[redhat-3.10] fetching digest from image field when using cri-o (PROJ…

…QUAY-6512) (#123) As of OCP 4.15 the cri-o runtime now replaces the pod.status.imageID field with an internal cri-o ID instead of the resolved digest. This prevents CSO retrieving scan results since it's unable to fetch a digest for the resolved image. This change checks for a cri-o image ID and in which case will use the pod.status.image field. This allows images referenced by digest to be scanned. Images referenced by tag will not be able to be scanned. --------- Co-authored-by: bcaton <[email protected]>
quay · Feb 7, 2024 · ea48b1f · ea48b1f
1 parent 2fa9802
commit ea48b1f
Show file tree

Hide file tree

Showing 4 changed files with 102 additions and 24 deletions.
diff --git a/image/image.go b/image/image.go
@@ -91,9 +91,10 @@ func ParsePullSecrets(ctx context.Context, secretClient corev1.SecretInterface,
 }
 
 // Formats:
-//     {scheme}://{repo}@{digest} (images from dockerhub)
-//     {scheme}://{namespace}/{repo}@{digest} (images from dockerhub)
-//     {scheme}://{host}/{namespace}/{repo}@{digest}
+//
+//	{scheme}://{repo}@{digest} (images from dockerhub)
+//	{scheme}://{namespace}/{repo}@{digest} (images from dockerhub)
+//	{scheme}://{host}/{namespace}/{repo}@{digest}
 type Image struct {
 	ContainerName string
 	ContainerID   string
@@ -248,28 +249,65 @@ func ParseImageID(imageID string) (*Image, error) {
 	return image, nil
 }
 
-func ParseContainerStatus(containerStatus v1.ContainerStatus) (*Image, error) {
+func ParseContainer(pod *v1.Pod, containerName string) (*Image, error) {
+	// Get the container
+	var container v1.Container
+	for _, c := range pod.Spec.Containers {
+		if c.Name == containerName {
+			container = c
+			break
+		}
+	}
+	if container.Name != containerName {
+		return nil, fmt.Errorf("unable to find container: %s", containerName)
+	}
+
+	// Get the container status
+	var containerStatus v1.ContainerStatus
+	for _, cs := range pod.Status.ContainerStatuses {
+		if cs.Name == containerName {
+			containerStatus = cs
+			break
+		}
+	}
+	if containerStatus.Name != containerName {
+		return nil, fmt.Errorf("unable to find container status for container: %s", containerName)
+	}
+
 	// Parse imageID (digest)
-	image, err := ParseImageID(containerStatus.ImageID)
+	// cri-o will set the imageID to a random digest, in which case fallback to
+	// container.image. We cannot rely on containerstatus.image as it will not always
+	// point to the image that was specified in the pod spec
+	var imageID string
+	if regexp.MustCompile("^[a-zA-Z0-9_]*$").MatchString(containerStatus.ImageID) {
+		imageID = container.Image
+		digest := strings.SplitN(imageID, "@", 2)
+		if len(digest) != 2 {
+			return nil, fmt.Errorf("both image fields in container and containerStatus do not contain digest: %s", imageID)
+		}
+	} else {
+		imageID = containerStatus.ImageID
+	}
+	image, err := ParseImageID(imageID)
 	if err != nil {
 		return nil, err
 	}
 
 	// Set container name
-	image.ContainerName = containerStatus.Name
+	image.ContainerName = container.Name
 
 	// Set container id
 	image.ContainerID = containerStatus.ContainerID
 
 	// Check if image was pulled by digest or tag
-	if len(strings.SplitN(containerStatus.Image, "@", 2)) > 1 {
+	if len(strings.SplitN(container.Image, "@", 2)) > 1 {
 		return image, nil
 	}
 
 	// Set tag name
-	s := strings.Split(containerStatus.Image, ":")
+	s := strings.Split(container.Image, ":")
 	if len(s) != 2 && len(s) != 3 {
-		return nil, fmt.Errorf("Wrong image format")
+		return nil, fmt.Errorf("Wrong image format: %s", container.Image)
 	}
 
 	tagname := s[len(s)-1]

diff --git a/image/image_test.go b/image/image_test.go
@@ -10,13 +10,26 @@ import (
 	v1 "k8s.io/api/core/v1"
 )
 
-func generateContainerStatus(name, image, imageID string) v1.ContainerStatus {
-	cs := v1.ContainerStatus{
-		Name:    name,
-		Image:   image,
-		ImageID: imageID,
+func generatePod(name, image, imageID string) v1.Pod {
+	return v1.Pod{
+		Spec: v1.PodSpec{
+			Containers: []v1.Container{
+				{
+					Name:  name,
+					Image: image,
+				},
+			},
+		},
+		Status: v1.PodStatus{
+			ContainerStatuses: []v1.ContainerStatus{
+				{
+					Name:    name,
+					Image:   image,
+					ImageID: imageID,
+				},
+			},
+		},
 	}
-	return cs
 }
 
 var imageTable = []struct {
@@ -129,6 +142,20 @@ var containerStatusTable = []struct {
 
 	expectedError error
 }{
+	{
+		"my-test-repository",
+		"QUAY:443/my-test-namespace/my-test-repository@sha256:c549c6151dd8f4098fd02198913c0f6c55b240b156475588257f19d57e7b1fba",
+		"cf879a45faaacd2806705321f157c4c77682c7599589fed65d80f19bb61615a6",
+
+		"my-test-repository",
+		"QUAY:443",
+		"my-test-namespace",
+		"my-test-repository",
+		"sha256:c549c6151dd8f4098fd02198913c0f6c55b240b156475588257f19d57e7b1fba",
+		"",
+
+		nil,
+	},
 	{
 		"my-test-repository",
 		"QUAY:443/my-test-namespace/my-test-repository:latest",
@@ -239,11 +266,25 @@ var containerStatusTable = []struct {
 
 		fmt.Errorf("Invalid imageID format: %s", "sha256:94033a42da840b970fd9d2b04dae5fec56add2714ca674a758d030ce5acba27e"),
 	},
+	{
+		"my-test-repository",
+		"QUAY:443/my-test-namespace/my-test-repository:latest",
+		"cf879a45faaacd2806705321f157c4c77682c7599589fed65d80f19bb61615a6",
+
+		"",
+		"",
+		"",
+		"",
+		"",
+		"",
+
+		fmt.Errorf("both image fields in container and containerStatus do not contain digest: %s", "QUAY:443/my-test-namespace/my-test-repository:latest"),
+	},
 }
 
 func TestParseContainerStatus(t *testing.T) {
 	for _, tt := range containerStatusTable {
-		containerStatus := generateContainerStatus(tt.name, tt.image, tt.imageID)
+		pod := generatePod(tt.name, tt.image, tt.imageID)
 		var image = &Image{
 			ContainerName: tt.containername,
 			Host:          tt.host,
@@ -253,12 +294,12 @@ func TestParseContainerStatus(t *testing.T) {
 			Tag:           tt.tag,
 		}
 
-		parsedContainerStatus, err := ParseContainerStatus(containerStatus)
+		parsedContainerStatus, err := ParseContainer(&pod, tt.name)
 		if tt.expectedError != nil {
 			assert.Error(t, err)
 			assert.Equal(t, tt.expectedError, err)
 		} else if !reflect.DeepEqual(image, parsedContainerStatus) {
-			t.Errorf("Incorrectly parsed %+v as %+v", containerStatus, parsedContainerStatus)
+			t.Errorf("Incorrectly parsed %+v as %+v", pod, parsedContainerStatus)
 		}
 	}
 }

diff --git a/labeller/labeller.go b/labeller/labeller.go
@@ -403,7 +403,7 @@ func (l *Labeller) scan(ctx context.Context, pod *corev1.Pod, img *image.Image,
 			return fmt.Errorf("error updating image manifest vuln: %w", err)
 		}
 
-		level.Info(l.logger).Log("msg", "image manifest vuln creted", "image", img.String())
+		level.Info(l.logger).Log("msg", "image manifest vuln created", "image", img.String())
 		return nil
 	}
 
@@ -520,14 +520,13 @@ func (l *Labeller) Reconcile(ctx context.Context, key string) error {
 
 	// Add pod containers' images to scan
 	imagesToScan := make(map[string][]*image.Image)
-	for _, containerStatus := range pod.Status.ContainerStatuses {
-		img, err := image.ParseContainerStatus(containerStatus)
+	for _, container := range pod.Spec.Containers {
+		img, err := image.ParseContainer(pod, container.Name)
 		if err != nil {
-			level.Error(l.logger).Log("msg", "Error parsing imageID", "imageID", containerStatus.ImageID)
+			level.Error(l.logger).Log("msg", "Error parsing imageID", "err", err)
 			continue
 		}
 
-		img.ContainerName = containerStatus.Name
 		images := l.MirroredImages(img, mirrors)
 		images = append(images, img)
 

diff --git a/labeller/manifest_test.go b/labeller/manifest_test.go
@@ -97,7 +97,7 @@ func generateManifest(namespace, name string, pods []*corev1.Pod) (*secscanv1alp
 	for _, pod := range pods {
 		containerIds := []string{}
 		for _, containerStatus := range pod.Status.ContainerStatuses {
-			img, err := image.ParseContainerStatus(containerStatus)
+			img, err := image.ParseContainer(pod, containerStatus.Name)
 			if err != nil {
 				return nil, err
 			}