From eec78d8f61132356f28b2a9695cf7eb3c84f07ed Mon Sep 17 00:00:00 2001 From: "gcp-cherry-pick-bot[bot]" <98988430+gcp-cherry-pick-bot[bot]@users.noreply.github.com> Date: Wed, 20 Sep 2023 15:14:31 +0200 Subject: [PATCH] fix: image verify cache test (#8462) (#8467) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: image verify cache test * feat: print err message * feat: clear mock * feat: defer clear mock --------- Signed-off-by: Vishal Choudhary Co-authored-by: Vishal Choudhary Co-authored-by: Charles-Edouard Brétéché --- pkg/engine/image_verify_test.go | 245 +++++++++++++++++++------------- 1 file changed, 144 insertions(+), 101 deletions(-) diff --git a/pkg/engine/image_verify_test.go b/pkg/engine/image_verify_test.go index 7692aec78b..a529b01bb5 100644 --- a/pkg/engine/image_verify_test.go +++ b/pkg/engine/image_verify_test.go @@ -146,7 +146,8 @@ var testPolicyBad = `{ } ] } -}` +} +` var testResource = `{ "apiVersion": "v1", @@ -165,6 +166,130 @@ var testResource = `{ } }` +var cosignTestResource = `{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "name": "test", + "annotations": {} + }, + "spec": { + "containers": [ + { + "name": "pause2", + "image": "ghcr.io/kyverno/test-verify-image:signed" + } + ] + } +}` + +var cosignTestPolicy = `{ + "apiVersion": "kyverno.io/v1", + "kind": "ClusterPolicy", + "metadata": { + "name": "check-image", + "annotations": { + "pod-policies.kyverno.io/autogen-controllers": "none" + } + }, + "spec": { + "validationFailureAction": "enforce", + "background": false, + "webhookTimeoutSeconds": 30, + "failurePolicy": "Fail", + "rules": [ + { + "name": "check-signature", + "match": { + "resources": { + "kinds": [ + "Pod" + ] + } + }, + "verifyImages": [ + { + "imageReferences": [ + "ghcr.io/kyverno/test-verify-image:*" + ], + "attestors": [ + { + "entries": [ + { + "keys": { + "publicKeys": "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE8nXRh950IZbRj8Ra/N9sbqOPZrfM\n5/KAQN0/KjHcorm/J5yctVd7iEcnessRQjU917hmKO6JWVGHpDguIyakZA==\n-----END PUBLIC KEY-----", + "rekor": { + "url": "https://rekor.sigstore.dev", + "ignoreTlog": true + }, + "ctlog": { + "ignoreSCT": true + } + } + } + ] + } + ] + } + ] + } + ] + } +}` + +var cosignTestPolicyUpdated = `{ + "apiVersion": "kyverno.io/v1", + "kind": "ClusterPolicy", + "metadata": { + "name": "check-image", + "annotations": { + "pod-policies.kyverno.io/autogen-controllers": "none" + } + }, + "spec": { + "validationFailureAction": "enforce", + "background": false, + "webhookTimeoutSeconds": 30, + "failurePolicy": "Fail", + "rules": [ + { + "name": "check-signature-updated", + "match": { + "resources": { + "kinds": [ + "Pod" + ] + } + }, + "verifyImages": [ + { + "imageReferences": [ + "ghcr.io/kyverno/test-verify-image:*" + ], + "attestors": [ + { + "entries": [ + { + "keys": { + "publicKeys": "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE8nXRh950IZbRj8Ra/N9sbqOPZrfM\n5/KAQN0/KjHcorm/J5yctVd7iEcnessRQjU917hmKO6JWVGHpDguIyakZA==\n-----END PUBLIC KEY-----", + "rekor": { + "url": "https://rekor.sigstore.dev", + "ignoreTlog": true + }, + "ctlog": { + "ignoreSCT": true + } + } + } + ] + } + ] + } + ] + } + ] + } +}` var attestationPayloads = [][]byte{ []byte(`{"payloadType":"https://example.com/CodeReview/v1","payload":"eyJfdHlwZSI6Imh0dHBzOi8vaW4tdG90by5pby9TdGF0ZW1lbnQvdjAuMSIsInByZWRpY2F0ZVR5cGUiOiJodHRwczovL2V4YW1wbGUuY29tL0NvZGVSZXZpZXcvdjEiLCJzdWJqZWN0IjpbeyJuYW1lIjoiZ2hjci5pby9qaW1idWd3YWRpYS9wYXVzZTIiLCJkaWdlc3QiOnsic2hhMjU2IjoiYjMxYmZiNGQwMjEzZjI1NGQzNjFlMDA3OWRlYWFlYmVmYTRmODJiYTdhYTc2ZWY4MmU5MGI0OTM1YWQ1YjEwNSJ9fV0sInByZWRpY2F0ZSI6eyJhdXRob3IiOiJtYWlsdG86YWxpY2VAZXhhbXBsZS5jb20iLCJyZXBvIjp7ImJyYW5jaCI6Im1haW4iLCJ0eXBlIjoiZ2l0IiwidXJpIjoiaHR0cHM6Ly9naXRodWIuY29tL2V4YW1wbGUvbXktcHJvamVjdCJ9LCJyZXZpZXdlcnMiOlsibWFpbHRvOmJvYkBleGFtcGxlLmNvbSJdfX0=","signatures":[{"keyid":"","sig":"MEYCIQCrEr+vgPDmNCrqGDE/4z9iMLmCXMXcDlGKtSoiuMTSFgIhAN2riBaGk4accWzVl7ypi1XTRxyrPYHst8DesugPXgOf"}]}`), []byte(`{"payloadType":"cosign.sigstore.dev/attestation/v1","payload":"eyJfdHlwZSI6Imh0dHBzOi8vaW4tdG90by5pby9TdGF0ZW1lbnQvdjAuMSIsInByZWRpY2F0ZVR5cGUiOiJjb3NpZ24uc2lnc3RvcmUuZGV2L2F0dGVzdGF0aW9uL3YxIiwic3ViamVjdCI6W3sibmFtZSI6ImdoY3IuaW8vamltYnVnd2FkaWEvcGF1c2UyIiwiZGlnZXN0Ijp7InNoYTI1NiI6ImIzMWJmYjRkMDIxM2YyNTRkMzYxZTAwNzlkZWFhZWJlZmE0ZjgyYmE3YWE3NmVmODJlOTBiNDkzNWFkNWIxMDUifX1dLCJwcmVkaWNhdGUiOnsiRGF0YSI6ImhlbGxvIVxuIiwiVGltZXN0YW1wIjoiMjAyMS0xMC0wNVQwNToxODoxMVoifX0=","signatures":[{"keyid":"","sig":"MEQCIF5r9lf55rnYNPByZ9v6bortww694UEPvmyBIelIDYbIAiBNTGX4V64Oj6jZVRpkJQRxdzKUPYqC5GZTb4oS6eQ6aQ=="}]}`), @@ -208,6 +333,7 @@ func testVerifyAndPatchImages( func Test_CosignMockAttest(t *testing.T) { policyContext := buildContext(t, testPolicyGood, testResource, "") err := cosign.SetMock("ghcr.io/jimbugwadia/pause2:latest", attestationPayloads) + defer cosign.ClearMock() assert.NilError(t, err) er, ivm := testVerifyAndPatchImages(context.TODO(), registryclient.NewOrDie(), nil, policyContext, cfg) @@ -222,6 +348,7 @@ func Test_CosignMockAttest(t *testing.T) { func Test_CosignMockAttest_fail(t *testing.T) { policyContext := buildContext(t, testPolicyBad, testResource, "") err := cosign.SetMock("ghcr.io/jimbugwadia/pause2:latest", attestationPayloads) + defer cosign.ClearMock() assert.NilError(t, err) er, _ := testVerifyAndPatchImages(context.TODO(), registryclient.NewOrDie(), nil, policyContext, cfg) @@ -498,7 +625,6 @@ var ( func Test_NoMatch(t *testing.T) { policyContext := buildContext(t, testConfigMapMissing, testConfigMapMissingResource, "") - cosign.ClearMock() err, _ := testVerifyAndPatchImages(context.TODO(), registryclient.NewOrDie(), nil, policyContext, cfg) assert.Equal(t, len(err.PolicyResponse.Rules), 0) } @@ -508,7 +634,6 @@ func Test_ConfigMapMissingFailure(t *testing.T) { policyContext := buildContext(t, testConfigMapMissing, ghcrImage, "") resolver, err := resolvers.NewClientBasedResolver(kubefake.NewSimpleClientset()) assert.NilError(t, err) - cosign.ClearMock() resp, _ := testVerifyAndPatchImages(context.TODO(), registryclient.NewOrDie(), resolver, policyContext, cfg) assert.Equal(t, len(resp.PolicyResponse.Rules), 1) assert.Equal(t, resp.PolicyResponse.Rules[0].Status(), engineapi.RuleStatusError, resp.PolicyResponse.Rules[0].Message()) @@ -517,7 +642,6 @@ func Test_ConfigMapMissingFailure(t *testing.T) { func Test_SignatureGoodSigned(t *testing.T) { policyContext := buildContext(t, testSampleSingleKeyPolicy, testSampleResource, "") policyContext.Policy().GetSpec().Rules[0].VerifyImages[0].MutateDigest = true - cosign.ClearMock() engineResp, _ := testVerifyAndPatchImages(context.TODO(), registryclient.NewOrDie(), nil, policyContext, cfg) assert.Equal(t, len(engineResp.PolicyResponse.Rules), 1) assert.Equal(t, engineResp.PolicyResponse.Rules[0].Status(), engineapi.RuleStatusPass, engineResp.PolicyResponse.Rules[0].Message()) @@ -531,7 +655,6 @@ func Test_SignatureGoodSigned(t *testing.T) { } func Test_SignatureUnsigned(t *testing.T) { - cosign.ClearMock() unsigned := strings.Replace(testSampleResource, ":signed", ":unsigned", -1) policyContext := buildContext(t, testSampleSingleKeyPolicy, unsigned, "") engineResp, _ := testVerifyAndPatchImages(context.TODO(), registryclient.NewOrDie(), nil, policyContext, cfg) @@ -540,7 +663,6 @@ func Test_SignatureUnsigned(t *testing.T) { } func Test_SignatureWrongKey(t *testing.T) { - cosign.ClearMock() otherKey := strings.Replace(testSampleResource, ":signed", ":signed-by-someone-else", -1) policyContext := buildContext(t, testSampleSingleKeyPolicy, otherKey, "") engineResp, _ := testVerifyAndPatchImages(context.TODO(), registryclient.NewOrDie(), nil, policyContext, cfg) @@ -549,7 +671,6 @@ func Test_SignatureWrongKey(t *testing.T) { } func Test_SignaturesMultiKey(t *testing.T) { - cosign.ClearMock() policy := strings.Replace(testSampleMultipleKeyPolicy, "KEY1", testVerifyImageKey, -1) policy = strings.Replace(policy, "KEY2", testVerifyImageKey, -1) policy = strings.Replace(policy, "COUNT", "0", -1) @@ -560,7 +681,6 @@ func Test_SignaturesMultiKey(t *testing.T) { } func Test_SignaturesMultiKeyFail(t *testing.T) { - cosign.ClearMock() policy := strings.Replace(testSampleMultipleKeyPolicy, "KEY1", testVerifyImageKey, -1) policy = strings.Replace(policy, "COUNT", "0", -1) policyContext := buildContext(t, policy, testSampleResource, "") @@ -570,7 +690,6 @@ func Test_SignaturesMultiKeyFail(t *testing.T) { } func Test_SignaturesMultiKeyOneGoodKey(t *testing.T) { - cosign.ClearMock() policy := strings.Replace(testSampleMultipleKeyPolicy, "KEY1", testVerifyImageKey, -1) policy = strings.Replace(policy, "KEY2", testOtherKey, -1) policy = strings.Replace(policy, "COUNT", "1", -1) @@ -581,7 +700,6 @@ func Test_SignaturesMultiKeyOneGoodKey(t *testing.T) { } func Test_SignaturesMultiKeyZeroGoodKey(t *testing.T) { - cosign.ClearMock() policy := strings.Replace(testSampleMultipleKeyPolicy, "KEY1", testOtherKey, -1) policy = strings.Replace(policy, "KEY2", testOtherKey, -1) policy = strings.Replace(policy, "COUNT", "1", -1) @@ -592,7 +710,6 @@ func Test_SignaturesMultiKeyZeroGoodKey(t *testing.T) { } func Test_RuleSelectorImageVerify(t *testing.T) { - cosign.ClearMock() policyContext := buildContext(t, testSampleSingleKeyPolicy, testSampleResource, "") rule := newStaticKeyRule("match-all", "*", testOtherKey) @@ -721,7 +838,6 @@ var testNestedAttestorPolicy = ` ` func Test_NestedAttestors(t *testing.T) { - cosign.ClearMock() policy := strings.Replace(testNestedAttestorPolicy, "KEY1", testVerifyImageKey, -1) policy = strings.Replace(policy, "KEY2", testVerifyImageKey, -1) @@ -830,9 +946,9 @@ func Test_ChangedAnnotation(t *testing.T) { func Test_MarkImageVerified(t *testing.T) { image := "ghcr.io/jimbugwadia/pause2:latest" - cosign.ClearMock() policyContext := buildContext(t, testPolicyGood, testResource, "") err := cosign.SetMock(image, attestationPayloads) + defer cosign.ClearMock() assert.NilError(t, err) engineResponse, verifiedImages := testVerifyAndPatchImages(context.TODO(), registryclient.NewOrDie(), nil, policyContext, cfg) @@ -928,9 +1044,9 @@ func Test_ParsePEMDelimited(t *testing.T) { }` image := "ghcr.io/jimbugwadia/pause2:latest" - cosign.ClearMock() policyContext := buildContext(t, testPEMPolicy, testResource, "") err := cosign.SetMock(image, signaturePayloads) + defer cosign.ClearMock() assert.NilError(t, err) engineResponse, verifiedImages := testVerifyAndPatchImages(context.TODO(), registryclient.NewOrDie(), nil, policyContext, cfg) @@ -969,78 +1085,12 @@ func testImageVerifyCache( func errorAssertionUtil(t *testing.T, image string, ivm engineapi.ImageVerificationMetadata, er engineapi.EngineResponse) { assert.Equal(t, len(er.PolicyResponse.Rules), 1) - assert.Equal(t, er.PolicyResponse.Rules[0].Status(), engineapi.RuleStatusPass) + assert.Equal(t, er.PolicyResponse.Rules[0].Status(), engineapi.RuleStatusPass, er.PolicyResponse.Rules[0].Message()) assert.Equal(t, ivm.IsEmpty(), false) assert.Equal(t, ivm.IsVerified(image), true) } -var testUpdatedPolicyGood = `{ - "apiVersion": "kyverno.io/v1", - "kind": "ClusterPolicy", - "metadata": { - "name": "attest" - }, - "spec": { - "rules": [ - { - "name": "attest-testing", - "match": { - "resources": { - "kinds": [ - "Pod" - ] - } - }, - "verifyImages": [ - { - "image": "*", - "key": "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEHMmDjK65krAyDaGaeyWNzgvIu155JI50B2vezCw8+3CVeE0lJTL5dbL3OP98Za0oAEBJcOxky8Riy/XcmfKZbw==\n-----END PUBLIC KEY-----", - "attestations": [ - { - "predicateType": "https://example.com/CodeReview/v1", - "attestors": [ - { - "entries": [ - { - "keys": { - "publicKeys": "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEHMmDjK65krAyDaGaeyWNzgvIu155JI50B2vezCw8+3CVeE0lJTL5dbL3OP98Za0oAEBJcOxky8Riy/XcmfKZbw==\n-----END PUBLIC KEY-----", - "rekor": { - "url": "https://rekor.sigstore.dev", - "ignoreSCT": true, - "ignoreTlog": true - } - } - } - ] - } - ], - "conditions": [ - { - "all": [ - { - "key": "{{ repo.uri }}", - "operator": "Equals", - "value": "https://github.com/example/my-project" - }, - { - "key": "{{ repo.branch }}", - "operator": "Equals", - "value": "main" - } - ] - } - ] - } - ] - } - ] - } - ] - } - }` - func Test_ImageVerifyCacheCosign(t *testing.T) { - opts := []imageverifycache.Option{ imageverifycache.WithCacheEnableFlag(true), imageverifycache.WithMaxSize(1000), @@ -1049,10 +1099,8 @@ func Test_ImageVerifyCacheCosign(t *testing.T) { imageVerifyCache, err := imageverifycache.New(opts...) assert.NilError(t, err) - policyContext := buildContext(t, testPolicyGood, testResource, "") - image := "ghcr.io/jimbugwadia/pause2:latest" - err = cosign.SetMock(image, attestationPayloads) - assert.NilError(t, err) + image := "ghcr.io/kyverno/test-verify-image:signed" + policyContext := buildContext(t, cosignTestPolicy, cosignTestResource, "") start := time.Now() er, ivm := testImageVerifyCache(imageVerifyCache, context.TODO(), registryclient.NewOrDie(), nil, policyContext, cfg) @@ -1063,11 +1111,10 @@ func Test_ImageVerifyCacheCosign(t *testing.T) { er, ivm = testImageVerifyCache(imageVerifyCache, context.TODO(), registryclient.NewOrDie(), nil, policyContext, cfg) secondOperationTime := time.Since(start) errorAssertionUtil(t, image, ivm, er) - assert.Check(t, secondOperationTime < firstOperationTime/2, "cache entry is valid, so image verification should be from cache.", firstOperationTime, secondOperationTime) + assert.Check(t, secondOperationTime < firstOperationTime/10, "cache entry is valid, so image verification should be from cache.", firstOperationTime, secondOperationTime) } func Test_ImageVerifyCacheExpiredCosign(t *testing.T) { - opts := []imageverifycache.Option{ imageverifycache.WithCacheEnableFlag(true), imageverifycache.WithMaxSize(1000), @@ -1076,10 +1123,8 @@ func Test_ImageVerifyCacheExpiredCosign(t *testing.T) { imageVerifyCache, err := imageverifycache.New(opts...) assert.NilError(t, err) - policyContext := buildContext(t, testPolicyGood, testResource, "") - image := "ghcr.io/jimbugwadia/pause2:latest" - err = cosign.SetMock(image, attestationPayloads) - assert.NilError(t, err) + image := "ghcr.io/kyverno/test-verify-image:signed" + policyContext := buildContext(t, cosignTestPolicy, cosignTestResource, "") start := time.Now() er, ivm := testImageVerifyCache(imageVerifyCache, context.TODO(), registryclient.NewOrDie(), nil, policyContext, cfg) @@ -1092,7 +1137,7 @@ func Test_ImageVerifyCacheExpiredCosign(t *testing.T) { er, ivm = testImageVerifyCache(imageVerifyCache, context.TODO(), registryclient.NewOrDie(), nil, policyContext, cfg) secondOperationTime := time.Since(start) errorAssertionUtil(t, image, ivm, er) - assert.Check(t, secondOperationTime > firstOperationTime/2 && secondOperationTime < firstOperationTime*2, "cache entry is expired, so image verification should not be from cache.") + assert.Check(t, secondOperationTime > firstOperationTime/10 && secondOperationTime < firstOperationTime*10, "cache entry is expired, so image verification should not be from cache.") } func Test_changePolicyCacheVerificationCosign(t *testing.T) { @@ -1104,22 +1149,20 @@ func Test_changePolicyCacheVerificationCosign(t *testing.T) { imageVerifyCache, err := imageverifycache.New(opts...) assert.NilError(t, err) - policyContext := buildContext(t, testPolicyGood, testResource, "") - image := "ghcr.io/jimbugwadia/pause2:latest" - err = cosign.SetMock(image, attestationPayloads) - assert.NilError(t, err) + image := "ghcr.io/kyverno/test-verify-image:signed" + policyContext := buildContext(t, cosignTestPolicy, cosignTestResource, "") start := time.Now() er, ivm := testImageVerifyCache(imageVerifyCache, context.TODO(), registryclient.NewOrDie(), nil, policyContext, cfg) firstOperationTime := time.Since(start) errorAssertionUtil(t, image, ivm, er) - policyContext = buildContext(t, testUpdatedPolicyGood, testResource, "") + policyContext = buildContext(t, cosignTestPolicyUpdated, cosignTestResource, "") start = time.Now() er, ivm = testImageVerifyCache(imageVerifyCache, context.TODO(), registryclient.NewOrDie(), nil, policyContext, cfg) secondOperationTime := time.Since(start) errorAssertionUtil(t, image, ivm, er) - assert.Check(t, secondOperationTime > firstOperationTime/2 && secondOperationTime < firstOperationTime*2, "cache entry not found, so image verification should not be from cache.") + assert.Check(t, secondOperationTime > firstOperationTime/10 && secondOperationTime < firstOperationTime*10, "cache entry not found, so image verification should not be from cache.") } var verifyImageNotaryPolicy = `{ @@ -1266,7 +1309,7 @@ func Test_ImageVerifyCacheNotary(t *testing.T) { er, ivm = testImageVerifyCache(imageVerifyCache, context.TODO(), registryclient.NewOrDie(), nil, policyContext, cfg) secondOperationTime := time.Since(start) errorAssertionUtil(t, image, ivm, er) - assert.Check(t, secondOperationTime < firstOperationTime/2, "cache entry is valid, so image verification should be from cache.", firstOperationTime, secondOperationTime) + assert.Check(t, secondOperationTime < firstOperationTime/10, "cache entry is valid, so image verification should be from cache.", firstOperationTime, secondOperationTime) } func Test_ImageVerifyCacheExpiredNotary(t *testing.T) { @@ -1293,7 +1336,7 @@ func Test_ImageVerifyCacheExpiredNotary(t *testing.T) { er, ivm = testImageVerifyCache(imageVerifyCache, context.TODO(), registryclient.NewOrDie(), nil, policyContext, cfg) secondOperationTime := time.Since(start) errorAssertionUtil(t, image, ivm, er) - assert.Check(t, secondOperationTime > firstOperationTime/2 && secondOperationTime < firstOperationTime*2, "cache entry is expired, so image verification should not be from cache.") + assert.Check(t, secondOperationTime > firstOperationTime/10 && secondOperationTime < firstOperationTime*10, "cache entry is expired, so image verification should not be from cache.") } @@ -1318,6 +1361,6 @@ func Test_changePolicyCacheVerificationNotary(t *testing.T) { er, ivm = testImageVerifyCache(imageVerifyCache, context.TODO(), registryclient.NewOrDie(), nil, policyContext, cfg) secondOperationTime := time.Since(start) errorAssertionUtil(t, image, ivm, er) - assert.Check(t, secondOperationTime > firstOperationTime/2 && secondOperationTime < firstOperationTime*2, "cache entry not found, so image verification should not be from cache.") + assert.Check(t, secondOperationTime > firstOperationTime/10 && secondOperationTime < firstOperationTime*10, "cache entry not found, so image verification should not be from cache.") }