Skip to content

Commit c6698a9

Browse files
AlexVulajAlexVulaj
committed
ROX-34024: Use name-based matching in populateImageMetadata
1 parent d721274 commit c6698a9

File tree

2 files changed

+44
-27
lines changed

2 files changed

+44
-27
lines changed

‎sensor/kubernetes/listener/resources/convert.go‎

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -310,11 +310,13 @@ func (w *deploymentWrap) populateImageMetadata(localImages set.StringSet, pods .
310310
// The downside to this is that if different pods have different versions then we will miss that fact that pods are running
311311
// different versions and clobber it. I've added a log to illustrate the clobbering so we can see how often it happens
312312

313-
// Sort the w.Deployment.Containers by name and p.Status.ContainerStatuses by name
314-
// This is because the order is not guaranteed
315-
sort.SliceStable(w.GetDeployment().GetContainers(), func(i, j int) bool {
316-
return w.GetDeployment().GetContainers()[i].GetName() < w.GetDeployment().GetContainers()[j].GetName()
317-
})
313+
// Build a map from container name to deployment container for name-based matching.
314+
// This avoids index-based alignment which breaks when other container types are
315+
//present in deployment.Containers but not in pod container statuses.
316+
containersByName := make(map[string]*storage.Container, len(w.GetDeployment().GetContainers()))
317+
for _, c := range w.GetDeployment().GetContainers() {
318+
containersByName[c.GetName()] = c
319+
}
318320

319321
// Sort the pods by time created as that pod will be most likely to have the most updated spec
320322
sort.SliceStable(pods, func(i, j int) bool {
@@ -323,19 +325,20 @@ func (w *deploymentWrap) populateImageMetadata(localImages set.StringSet, pods .
323325

324326
// Determine each image's ID, if not already populated, as well as if the image is pullable and/or cluster-local.
325327
for _, p := range pods {
326-
sort.SliceStable(p.Status.ContainerStatuses, func(i, j int) bool {
327-
return p.Status.ContainerStatuses[i].Name < p.Status.ContainerStatuses[j].Name
328-
})
329-
sort.SliceStable(p.Spec.Containers, func(i, j int) bool {
330-
return p.Spec.Containers[i].Name < p.Spec.Containers[j].Name
331-
})
332-
for i, c := range p.Status.ContainerStatuses {
333-
if i >= len(w.GetDeployment().GetContainers()) || i >= len(p.Spec.Containers) {
334-
// This should not happen, but could happen if w.Deployment.Containers and container status are out of sync
335-
break
328+
// Build a map from container name to pod spec container for name-based lookup.
329+
specContainersByName := make(map[string]v1.Container, len(p.Spec.Containers))
330+
for _, sc := range p.Spec.Containers {
331+
specContainersByName[sc.Name] = sc
332+
}
333+
334+
for _, c := range p.Status.ContainerStatuses {
335+
deployContainer, found := containersByName[c.Name]
336+
if !found {
337+
log.Debugf("Skipping container status %q with no matching deployment container for deploy %q, pod %q", c.Name, w.GetDeployment().GetName(), p.GetName())
338+
continue
336339
}
337340

338-
image := w.GetDeployment().GetContainers()[i].GetImage()
341+
image := deployContainer.GetImage()
339342

340343
var runtimeImageName *storage.ImageName
341344
if features.UnqualifiedSearchRegistries.Enabled() && c.ImageID != "" {
@@ -365,7 +368,12 @@ func (w *deploymentWrap) populateImageMetadata(localImages set.StringSet, pods .
365368
continue
366369
}
367370

368-
parsedName, err := imageUtils.GenerateImageFromStringWithOverride(p.Spec.Containers[i].Image, w.registryOverride)
371+
specContainer, found := specContainersByName[c.Name]
372+
if !found {
373+
continue
374+
}
375+
376+
parsedName, err := imageUtils.GenerateImageFromStringWithOverride(specContainer.Image, w.registryOverride)
369377
if err != nil {
370378
// This error will only happen if we could not parse the image, this is possible if the image in kubernetes is malformed
371379
// e.g. us.gcr.io/$PROJECT/xyz:latest is an example that we have seen

‎sensor/kubernetes/listener/resources/convert_test.go‎

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package resources
22

33
import (
4+
"fmt"
45
"testing"
56
"time"
67

@@ -390,23 +391,27 @@ func TestPopulateImageMetadata(t *testing.T) {
390391
wrap := deploymentWrap{
391392
Deployment: &storage.Deployment{},
392393
}
393-
for _, container := range c.wrap {
394+
for i, container := range c.wrap {
395+
name := fmt.Sprintf("container-%d", i)
394396
img, err := imageUtils.GenerateImageFromString(container.image)
395397
require.NoError(t, err)
396398
wrap.Containers = append(wrap.Containers, &storage.Container{
399+
Name: name,
397400
Image: img,
398401
})
399-
400402
}
401403

402404
pods := make([]*v1.Pod, 0, len(c.pods))
403405
for _, pod := range c.pods {
404406
k8sPod := &v1.Pod{}
405-
for _, img := range pod.images {
406-
k8sPod.Spec.Containers = append(k8sPod.Spec.Containers, v1.Container{Image: img})
407+
for i, img := range pod.images {
408+
name := fmt.Sprintf("container-%d", i)
409+
k8sPod.Spec.Containers = append(k8sPod.Spec.Containers, v1.Container{Name: name, Image: img})
407410
}
408-
for _, imageID := range pod.imageIDsInStatus {
411+
for i, imageID := range pod.imageIDsInStatus {
412+
name := fmt.Sprintf("container-%d", i)
409413
k8sPod.Status.ContainerStatuses = append(k8sPod.Status.ContainerStatuses, v1.ContainerStatus{
414+
Name: name,
410415
ImageID: imageID,
411416
})
412417
}
@@ -545,23 +550,27 @@ func TestPopulateImageMetadataWithUnqualified(t *testing.T) {
545550
wrap := deploymentWrap{
546551
Deployment: &storage.Deployment{},
547552
}
548-
for _, container := range c.wrap {
553+
for i, container := range c.wrap {
554+
name := fmt.Sprintf("container-%d", i)
549555
img, err := imageUtils.GenerateImageFromString(container.image)
550556
require.NoError(t, err)
551557
wrap.Containers = append(wrap.Containers, &storage.Container{
558+
Name: name,
552559
Image: img,
553560
})
554-
555561
}
556562

557563
pods := make([]*v1.Pod, 0, len(c.pods))
558564
for _, pod := range c.pods {
559565
k8sPod := &v1.Pod{}
560-
for _, img := range pod.images {
561-
k8sPod.Spec.Containers = append(k8sPod.Spec.Containers, v1.Container{Image: img})
566+
for i, img := range pod.images {
567+
name := fmt.Sprintf("container-%d", i)
568+
k8sPod.Spec.Containers = append(k8sPod.Spec.Containers, v1.Container{Name: name, Image: img})
562569
}
563-
for _, imageID := range pod.imageIDsInStatus {
570+
for i, imageID := range pod.imageIDsInStatus {
571+
name := fmt.Sprintf("container-%d", i)
564572
k8sPod.Status.ContainerStatuses = append(k8sPod.Status.ContainerStatuses, v1.ContainerStatus{
573+
Name: name,
565574
ImageID: imageID,
566575
})
567576
}

0 commit comments

Comments
 (0)