Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[redhat-3.10] fetching digest from image field when using cri-o (PROJQUAY-6512) #123

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 47 additions & 9 deletions image/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]
Expand Down
59 changes: 50 additions & 9 deletions image/image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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,
Expand All @@ -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)
}
}
}
Expand Down
9 changes: 4 additions & 5 deletions labeller/labeller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion labeller/manifest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Loading