Fixed issue-3709: Image verify rule gives error for non-existing configmap (#5272)

Signed-off-by: Pratik Shah <pratik@infracloud.io>

Signed-off-by: Pratik Shah <pratik@infracloud.io>

pkg/engine/imageVerify.go

@@ -25,9 +25,45 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
 
+func getMatchingImages(images map[string]map[string]apiutils.ImageInfo, rule *kyvernov1.Rule) ([]apiutils.ImageInfo, string) {
+	imageInfos := []apiutils.ImageInfo{}
+	imageRefs := []string{}
+	for _, infoMap := range images {
+		for _, imageInfo := range infoMap {
+			image := imageInfo.String()
+			for _, verifyImage := range rule.VerifyImages {
+				verifyImage = *verifyImage.Convert()
+				imageRefs = append(imageRefs, verifyImage.ImageReferences...)
+				if imageMatches(image, verifyImage.ImageReferences) {
+					imageInfos = append(imageInfos, imageInfo)
+				}
+			}
+		}
+	}
+	return imageInfos, strings.Join(imageRefs, ",")
+}
+
+func extractMatchingImages(policyContext *PolicyContext, rule *kyvernov1.Rule) ([]apiutils.ImageInfo, string, error) {
+	var (
+		images map[string]map[string]apiutils.ImageInfo
+		err    error
+	)
+	images = policyContext.JSONContext.ImageInfo()
+	if rule.ImageExtractors != nil {
+		images, err = policyContext.JSONContext.GenerateCustomImageInfo(
+			&policyContext.NewResource, rule.ImageExtractors)
+		if err != nil {
+			// if we get an error while generating custom images from image extractors,
+			// don't check for matching images in imageExtractors
+			return nil, "", err
+		}
+	}
+	matchingImages, imageRefs := getMatchingImages(images, rule)
+	return matchingImages, imageRefs, nil
+}
+
 func VerifyAndPatchImages(policyContext *PolicyContext) (*response.EngineResponse, *ImageVerificationMetadata) {
 	resp := &response.EngineResponse{}
-	images := policyContext.JSONContext.ImageInfo()
 
 	policy := policyContext.Policy
 	patchedResource := policyContext.NewResource
@@ -66,28 +102,28 @@ func VerifyAndPatchImages(policyContext *PolicyContext) (*response.EngineRespons
 
 		logger.V(3).Info("processing image verification rule", "ruleSelector", applyRules)
 
-		policyContext.JSONContext.Restore()
+		var err error
-		if err := LoadContext(logger, rule.Context, policyContext, rule.Name); err != nil {
+		ruleImages, imageRefs, err := extractMatchingImages(policyContext, rule)
-			appendError(resp, rule, fmt.Sprintf("failed to load context: %s", err.Error()), response.RuleStatusError)
+		if err != nil {
+			appendResponse(resp, rule, fmt.Sprintf("failed to extract images: %s", err.Error()), response.RuleStatusError)
+			continue
+		}
+		if len(ruleImages) == 0 {
+			appendResponse(resp, rule,
+				fmt.Sprintf("skip run verification as image in resource not found in imageRefs '%s'",
+					imageRefs), response.RuleStatusSkip)
 			continue
 		}
 
-		ruleImages := images
+		policyContext.JSONContext.Restore()
-		var err error
+		if err := LoadContext(logger, rule.Context, policyContext, rule.Name); err != nil {
-		if rule.ImageExtractors != nil {
+			appendResponse(resp, rule, fmt.Sprintf("failed to load context: %s", err.Error()), response.RuleStatusError)
-			if ruleImages, err = policyContext.JSONContext.GenerateCustomImageInfo(&policyContext.NewResource, rule.ImageExtractors); err != nil {
-				appendError(resp, rule, fmt.Sprintf("failed to extract images: %s", err.Error()), response.RuleStatusError)
-				continue
-			}
-		}
-
-		if ruleImages == nil {
 			continue
 		}
 
 		ruleCopy, err := substituteVariables(rule, policyContext.JSONContext, logger)
 		if err != nil {
-			appendError(resp, rule, fmt.Sprintf("failed to substitute variables: %s", err.Error()), response.RuleStatusError)
+			appendResponse(resp, rule, fmt.Sprintf("failed to substitute variables: %s", err.Error()), response.RuleStatusError)
 			continue
 		}
 
@@ -111,7 +147,7 @@ func VerifyAndPatchImages(policyContext *PolicyContext) (*response.EngineRespons
 	return resp, ivm
 }
 
-func appendError(resp *response.EngineResponse, rule *kyvernov1.Rule, msg string, status response.RuleStatus) {
+func appendResponse(resp *response.EngineResponse, rule *kyvernov1.Rule, msg string, status response.RuleStatus) {
 	rr := ruleResponse(*rule, response.ImageVerify, msg, status, nil)
 	resp.PolicyResponse.Rules = append(resp.PolicyResponse.Rules, *rr)
 	incrementErrorCount(resp)
@@ -148,68 +184,61 @@ type imageVerifier struct {
 
 // verify applies policy rules to each matching image. The policy rule results and annotation patches are
 // added to tme imageVerifier `resp` and `ivm` fields.
-func (iv *imageVerifier) verify(imageVerify kyvernov1.ImageVerification, images map[string]map[string]apiutils.ImageInfo) {
+func (iv *imageVerifier) verify(imageVerify kyvernov1.ImageVerification, matchedImageInfos []apiutils.ImageInfo) {
 	// for backward compatibility
 	imageVerify = *imageVerify.Convert()
 
-	for _, infoMap := range images {
+	for _, imageInfo := range matchedImageInfos {
-		for _, imageInfo := range infoMap {
+		image := imageInfo.String()
-			image := imageInfo.String()
 
-			if !imageMatches(image, imageVerify.ImageReferences) {
+		if hasImageVerifiedAnnotationChanged(iv.policyContext, iv.logger) {
-				iv.logger.V(4).Info("image does not match pattern", "image", image, "patterns", imageVerify.ImageReferences)
+			msg := imageVerifyAnnotationKey + " annotation cannot be changed"
-				continue
+			iv.logger.Info("image verification error", "reason", msg)
-			}
+			ruleResp := ruleResponse(*iv.rule, response.ImageVerify, msg, response.RuleStatusFail, nil)
+			iv.resp.PolicyResponse.Rules = append(iv.resp.PolicyResponse.Rules, *ruleResp)
+			incrementAppliedCount(iv.resp)
+			continue
+		}
 
-			if hasImageVerifiedAnnotationChanged(iv.policyContext, iv.logger) {
+		pointer := jsonpointer.ParsePath(imageInfo.Pointer).JMESPath()
-				msg := imageVerifyAnnotationKey + " annotation cannot be changed"
+		changed, err := iv.policyContext.JSONContext.HasChanged(pointer)
-				iv.logger.Info("image verification error", "reason", msg)
+		if err == nil && !changed {
-				ruleResp := ruleResponse(*iv.rule, response.ImageVerify, msg, response.RuleStatusFail, nil)
+			iv.logger.V(4).Info("no change in image, skipping check", "image", image)
-				iv.resp.PolicyResponse.Rules = append(iv.resp.PolicyResponse.Rules, *ruleResp)
+			continue
-				incrementAppliedCount(iv.resp)
+		}
-				continue
-			}
 
-			pointer := jsonpointer.ParsePath(imageInfo.Pointer).JMESPath()
+		verified, err := isImageVerified(iv.policyContext.NewResource, image, iv.logger)
-			changed, err := iv.policyContext.JSONContext.HasChanged(pointer)
+		if err == nil && verified {
-			if err == nil && !changed {
+			iv.logger.Info("image was previously verified, skipping check", "image", image)
-				iv.logger.V(4).Info("no change in image, skipping check", "image", image)
+			continue
-				continue
+		}
-			}
 
-			verified, err := isImageVerified(iv.policyContext.NewResource, image, iv.logger)
+		ruleResp, digest := iv.verifyImage(imageVerify, imageInfo)
-			if err == nil && verified {
-				iv.logger.Info("image was previously verified, skipping check", "image", image)
-				continue
-			}
 
-			ruleResp, digest := iv.verifyImage(imageVerify, imageInfo)
+		if imageVerify.MutateDigest {
-
+			patch, retrievedDigest, err := iv.handleMutateDigest(digest, imageInfo)
-			if imageVerify.MutateDigest {
+			if err != nil {
-				patch, retrievedDigest, err := iv.handleMutateDigest(digest, imageInfo)
+				ruleResp = ruleError(iv.rule, response.ImageVerify, "failed to update digest", err)
-				if err != nil {
+			} else if patch != nil {
-					ruleResp = ruleError(iv.rule, response.ImageVerify, "failed to update digest", err)
+				if ruleResp == nil {
-				} else if patch != nil {
+					ruleResp = ruleResponse(*iv.rule, response.ImageVerify, "mutated image digest", response.RuleStatusPass, nil)
-					if ruleResp == nil {
-						ruleResp = ruleResponse(*iv.rule, response.ImageVerify, "mutated image digest", response.RuleStatusPass, nil)
-					}
-
-					ruleResp.Patches = append(ruleResp.Patches, patch)
-					imageInfo.Digest = retrievedDigest
-					image = imageInfo.String()
-				}
-			}
-
-			if ruleResp != nil {
-				if len(imageVerify.Attestors) > 0 || len(imageVerify.Attestations) > 0 {
-					verified := ruleResp.Status == response.RuleStatusPass
-					iv.ivm.add(image, verified)
 				}
 
-				iv.resp.PolicyResponse.Rules = append(iv.resp.PolicyResponse.Rules, *ruleResp)
+				ruleResp.Patches = append(ruleResp.Patches, patch)
-				incrementAppliedCount(iv.resp)
+				imageInfo.Digest = retrievedDigest
+				image = imageInfo.String()
 			}
 		}
+
+		if ruleResp != nil {
+			if len(imageVerify.Attestors) > 0 || len(imageVerify.Attestations) > 0 {
+				verified := ruleResp.Status == response.RuleStatusPass
+				iv.ivm.add(image, verified)
+			}
+
+			iv.resp.PolicyResponse.Rules = append(iv.resp.PolicyResponse.Rules, *ruleResp)
+			incrementAppliedCount(iv.resp)
+		}
 	}
 }
 

pkg/engine/imageVerifyValidate.go

@@ -19,6 +19,13 @@ func processImageValidationRule(log logr.Logger, ctx *PolicyContext, rule *kyver
 	}
 
 	log = log.WithValues("rule", rule.Name)
+	matchingImages, _, err := extractMatchingImages(ctx, rule)
+	if err != nil {
+		return ruleResponse(*rule, response.Validation, err.Error(), response.RuleStatusError, nil)
+	}
+	if len(matchingImages) == 0 {
+		return ruleResponse(*rule, response.Validation, "image verified", response.RuleStatusSkip, nil)
+	}
 	if err := LoadContext(log, rule.Context, ctx, rule.Name); err != nil {
 		if _, ok := err.(gojmespath.NotFoundError); ok {
 			log.V(3).Info("failed to load context", "reason", err.Error())

pkg/engine/imageVerify_test.go

@@ -10,6 +10,7 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 
 	kyverno "github.com/kyverno/kyverno/api/kyverno/v1"
+	client "github.com/kyverno/kyverno/pkg/clients/dclient"
 	"github.com/kyverno/kyverno/pkg/cosign"
 	"github.com/kyverno/kyverno/pkg/engine/context"
 	"github.com/kyverno/kyverno/pkg/engine/response"
@@ -305,6 +306,68 @@ var testSampleMultipleKeyPolicy = `
 }
 `
 
+var testConfigMapMissing = `{
+    "apiVersion": "kyverno.io/v1",
+    "kind": "ClusterPolicy",
+    "metadata": {
+        "annotations": {
+            "pod-policies.kyverno.io/autogen-controllers": "none"
+        },
+        "name": "image-verify-polset"
+    },
+    "spec": {
+        "background": false,
+        "failurePolicy": "Fail",
+        "rules": [
+            {
+                "context": [
+                    {
+                        "configMap": {
+                            "name": "myconfigmap",
+                            "namespace": "mynamespace"
+                        },
+                        "name": "myconfigmap"
+                    }
+                ],
+                "match": {
+                    "any": [
+                        {
+                            "resources": {
+                                "kinds": [
+                                    "Pod"
+                                ]
+                            }
+                        }
+                    ]
+                },
+                "name": "image-verify-pol1",
+                "verifyImages": [
+                    {
+                        "imageReferences": [
+                            "ghcr.io/*"
+                        ],
+                        "mutateDigest": false,
+                        "verifyDigest": false,
+                        "attestors": [
+                            {
+                                "entries": [
+                                    {
+                                        "keys": {
+                                            "publicKeys": "{{myconfigmap.data.configmapkey}}"
+                                        }
+                                    }
+                                ]
+                            }
+                        ]
+                    }
+                ]
+            }
+        ],
+        "validationFailureAction": "Audit",
+        "webhookTimeoutSeconds": 30
+    }
+}`
+
 var testSampleResource = `{
   "apiVersion": "v1",
   "kind": "Pod",
@@ -319,9 +382,47 @@ var testSampleResource = `{
   }
 }`
 
+var testConfigMapMissingResource = `{
+    "apiVersion": "v1",
+    "kind": "Pod",
+    "metadata": {
+        "labels": {
+            "run": "test"
+        },
+        "name": "test"
+    },
+    "spec": {
+        "containers": [
+            {
+                "image": "nginx:latest",
+                "name": "test",
+                "resources": {}
+            }
+        ]
+    }
+}`
+
 var testVerifyImageKey = `-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE8nXRh950IZbRj8Ra/N9sbqOPZrfM5/KAQN0/KjHcorm/J5yctVd7iEcnessRQjU917hmKO6JWVGHpDguIyakZA==\n-----END PUBLIC KEY-----\n`
 var testOtherKey = `-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEpNlOGZ323zMlhs4bcKSpAKQvbcWi5ZLRmijm6SqXDy0Fp0z0Eal+BekFnLzs8rUXUaXlhZ3hNudlgFJH+nFNMw==\n-----END PUBLIC KEY-----\n`
 
+func Test_ConfigMapMissingSuccess(t *testing.T) {
+	policyContext := buildContext(t, testConfigMapMissing, testConfigMapMissingResource, "")
+	cosign.ClearMock()
+	err, _ := VerifyAndPatchImages(policyContext)
+	assert.Equal(t, len(err.PolicyResponse.Rules), 1)
+	assert.Equal(t, err.PolicyResponse.Rules[0].Status, response.RuleStatusSkip, err.PolicyResponse.Rules[0].Message)
+}
+
+func Test_ConfigMapMissingFailure(t *testing.T) {
+	ghcrImage := strings.Replace(testConfigMapMissingResource, "nginx:latest", "ghcr.io/kyverno/test-verify-image:signed", -1)
+	policyContext := buildContext(t, testConfigMapMissing, ghcrImage, "")
+	policyContext.Client = client.NewEmptyFakeClient()
+	cosign.ClearMock()
+	err, _ := VerifyAndPatchImages(policyContext)
+	assert.Equal(t, len(err.PolicyResponse.Rules), 1)
+	assert.Equal(t, err.PolicyResponse.Rules[0].Status, response.RuleStatusError, err.PolicyResponse.Rules[0].Message)
+}
+
 func Test_SignatureGoodSigned(t *testing.T) {
 	policyContext := buildContext(t, testSampleSingleKeyPolicy, testSampleResource, "")
 	cosign.ClearMock()

pkg/policy/validate.go

@@ -501,6 +501,11 @@ func ruleForbiddenSectionsHaveVariables(rule *kyvernov1.Rule) error {
 		return fmt.Errorf("rule \"%s\" should not have variables in match section", rule.Name)
 	}
 
+	err = imageRefHasVariables(rule.VerifyImages)
+	if err != nil {
+		return fmt.Errorf("rule \"%s\" should not have variables in image reference section", rule.Name)
+	}
+
 	return nil
 }
 
@@ -551,6 +556,19 @@ func objectHasVariables(object interface{}) error {
 	return nil
 }
 
+func imageRefHasVariables(verifyImages []kyvernov1.ImageVerification) error {
+	for _, verifyImage := range verifyImages {
+		verifyImage = *verifyImage.Convert()
+		for _, imageRef := range verifyImage.ImageReferences {
+			matches := variables.RegexVariables.FindAllString(imageRef, -1)
+			if len(matches) > 0 {
+				return fmt.Errorf("variables are not allowed in image reference")
+			}
+		}
+	}
+	return nil
+}
+
 func buildContext(rule *kyvernov1.Rule, background bool) *enginecontext.MockContext {
 	re := getAllowedVariables(background)
 	ctx := enginecontext.NewMockContext(re)

test/conformance/kuttl/verifyImages/clusterpolicy/standard/noconfigmap-diffimage-success/01-policy.yaml

@@ -0,0 +1,6 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+apply:
+- policy.yaml
+assert:
+- policy-ready.yaml

test/conformance/kuttl/verifyImages/clusterpolicy/standard/noconfigmap-diffimage-success/02-create-good-pod.yaml

@@ -0,0 +1,7 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+apply:
+- namespace.yaml
+- good-pod.yaml
+assert:
+- good-pod.yaml

test/conformance/kuttl/verifyImages/clusterpolicy/standard/noconfigmap-diffimage-success/03-create-bad-pod.yaml

@@ -0,0 +1,12 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+commands:
+- script: |
+    if kubectl apply -f bad-pod.yaml
+    then 
+      echo "Tested failed. Pod was created when it shouldn't have been."
+      exit 1 
+    else 
+      echo "Test succeeded. Pod was not created as intended."
+      exit 0
+    fi

test/conformance/kuttl/verifyImages/clusterpolicy/standard/noconfigmap-diffimage-success/04-update-policy.yaml

@@ -0,0 +1,6 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+apply:
+- update-policy.yaml
+assert:
+- update-policy.yaml

test/conformance/kuttl/verifyImages/clusterpolicy/standard/noconfigmap-diffimage-success/05-create-pod-with-configmap.yaml

@@ -0,0 +1,6 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+apply:
+- pod-with-configmap.yaml
+assert:
+- pod-with-configmap-ready.yaml

test/conformance/kuttl/verifyImages/clusterpolicy/standard/noconfigmap-diffimage-success/99-cleanup.yaml

@@ -0,0 +1,4 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+commands:
+  - command: kubectl delete -f policy.yaml,good-pod.yaml,pod-with-configmap.yaml,namespace.yaml --force --wait=true --ignore-not-found=true

test/conformance/kuttl/verifyImages/clusterpolicy/standard/noconfigmap-diffimage-success/README.md

@@ -0,0 +1,13 @@
+## Description
+
+This test verifies that resource creation is not blocked if resource image is different than policy image.
+
+## Expected Behavior
+
+This test should create a policy with missing configmap, a pod with different image than policy image. This shouldn't block pod creation.
+When pod is created with same image as policy image, pod creation should be blocked.
+When test tries to update any field in a policy, it should get updated properly.
+
+## Reference Issue(s)
+
+3709

test/conformance/kuttl/verifyImages/clusterpolicy/standard/noconfigmap-diffimage-success/bad-pod.yaml

@@ -0,0 +1,9 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: test-fail
+  namespace: mynamespace
+spec:
+  containers:
+  - image: ghcr.io/kyverno/test-verify-image:signed
+    name: test-fail

test/conformance/kuttl/verifyImages/clusterpolicy/standard/noconfigmap-diffimage-success/good-pod.yaml

@@ -0,0 +1,9 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: test-success
+  namespace: mynamespace
+spec:
+  containers:
+  - image: nginx:latest
+    name: test-success

test/conformance/kuttl/verifyImages/clusterpolicy/standard/noconfigmap-diffimage-success/namespace.yaml

@@ -0,0 +1,4 @@
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: mynamespace

test/conformance/kuttl/verifyImages/clusterpolicy/standard/noconfigmap-diffimage-success/pod-with-configmap-ready.yaml

@@ -0,0 +1,9 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  name: test-with-configmap
+  namespace: mynamespace
+spec:
+  containers:
+  - image: ghcr.io/kyverno/test-verify-image:signed
+    name: test-with-configmap

test/conformance/kuttl/verifyImages/clusterpolicy/standard/noconfigmap-diffimage-success/pod-with-configmap.yaml

@@ -0,0 +1,21 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: myconfigmap1
+  namespace: mynamespace
+data:
+  configmapkey: |
+    -----BEGIN PUBLIC KEY-----
+    MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE8nXRh950IZbRj8Ra/N9sbqOPZrfM
+    5/KAQN0/KjHcorm/J5yctVd7iEcnessRQjU917hmKO6JWVGHpDguIyakZA==
+    -----END PUBLIC KEY-----
+---
+apiVersion: v1
+kind: Pod
+metadata:
+  name: test-with-configmap
+  namespace: mynamespace
+spec:
+  containers:
+  - image: ghcr.io/kyverno/test-verify-image:signed
+    name: test-with-configmap

test/conformance/kuttl/verifyImages/clusterpolicy/standard/noconfigmap-diffimage-success/policy-ready.yaml

@@ -0,0 +1,9 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: image-verify-polset
+status:
+  conditions:
+  - reason: Succeeded
+    status: "True"
+    type: Ready

test/conformance/kuttl/verifyImages/clusterpolicy/standard/noconfigmap-diffimage-success/policy.yaml

@@ -0,0 +1,32 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  annotations:
+    pod-policies.kyverno.io/autogen-controllers: none
+  name: image-verify-polset
+spec:
+  background: false
+  failurePolicy: Fail
+  rules:
+    - context:
+        - configMap:
+            name: myconfigmap
+            namespace: mynamespace
+          name: myconfigmap
+      match:
+        any:
+        - resources:
+            kinds:
+              - Pod
+      name: image-verify-pol1
+      verifyImages:
+        - imageReferences:
+            - ghcr.io/*
+          mutateDigest: false
+          verifyDigest: false
+          attestors:
+            - entries:
+                - keys:
+                    publicKeys: '{{myconfigmap.data.configmapkey}}'
+  validationFailureAction: Audit
+  webhookTimeoutSeconds: 30

test/conformance/kuttl/verifyImages/clusterpolicy/standard/noconfigmap-diffimage-success/update-policy.yaml

@@ -0,0 +1,32 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  annotations:
+    pod-policies.kyverno.io/autogen-controllers: none
+  name: image-verify-polset
+spec:
+  background: false
+  failurePolicy: Fail
+  rules:
+    - context:
+        - configMap:
+            name: myconfigmap1
+            namespace: mynamespace
+          name: myconfigmap1
+      match:
+        any:
+        - resources:
+            kinds:
+              - Pod
+      name: image-verify-pol1
+      verifyImages:
+        - imageReferences:
+            - ghcr.io/*
+          mutateDigest: false
+          verifyDigest: false
+          attestors:
+            - entries:
+                - keys:
+                    publicKeys: '{{myconfigmap1.data.configmapkey}}'
+  validationFailureAction: Audit
+  webhookTimeoutSeconds: 30