diff --git a/cmd/webhook/main.go b/cmd/webhook/main.go index 6f84894fd..35712cc39 100644 --- a/cmd/webhook/main.go +++ b/cmd/webhook/main.go @@ -103,6 +103,12 @@ var ( // trustrootResyncPeriod holds the interval which the TrustRoot will resync // This is essential for triggering a reconcile update for potentially stale TUF metadata. trustrootResyncPeriod = flag.Duration("trustroot-resync-period", 24*time.Hour, "The resync period for ClusterImagePolicies. The default is 24h.") + + // Cache configuration for validating webhook results. + // https://github.com/sigstore/policy-controller/issues/647 + enableCache = flag.Bool("enable-cache", false, "Enable in-memory LRU cache for validation results.") + cacheSize = flag.Int("cache-size", 1024, "Maximum number of entries in the validation result cache.") + cacheTTL = flag.Duration("cache-ttl", 1*time.Hour, "TTL for cached validation results.") ) func main() { @@ -238,6 +244,12 @@ func NewValidatingAdmissionController(ctx context.Context, cmw configmap.Watcher kc := kubeclient.Get(ctx) validator := cwebhook.NewValidator(ctx) + var cache cwebhook.ResultCache + if *enableCache { + cache = cwebhook.NewLRUCache(*cacheSize, *cacheTTL) + logging.FromContext(ctx).Infof("Validation result cache enabled: size=%d, ttl=%v", *cacheSize, *cacheTTL) + } + return validation.NewAdmissionController(ctx, // Name of the resource webhook. *webhookName, @@ -253,6 +265,9 @@ func NewValidatingAdmissionController(ctx context.Context, cmw configmap.Watcher ctx = context.WithValue(ctx, kubeclient.Key{}, kc) ctx = store.ToContext(ctx) ctx = policyControllerConfigStore.ToContext(ctx) + if cache != nil { + ctx = cwebhook.ToContext(ctx, cache) + } ctx = policyduckv1beta1.WithPodScalableValidator(ctx, validator.ValidatePodScalable) ctx = duckv1.WithPodValidator(ctx, validator.ValidatePod) ctx = duckv1.WithPodSpecValidator(ctx, validator.ValidatePodSpecable) diff --git a/go.mod b/go.mod index 18b850d87..a1e67c177 100644 --- a/go.mod +++ b/go.mod @@ -62,6 +62,7 @@ require ( github.com/docker/docker-credential-helpers v0.9.3 github.com/docker/go-connections v0.5.0 github.com/go-jose/go-jose/v4 v4.1.0 + github.com/hashicorp/golang-lru/v2 v2.0.7 github.com/sigstore/protobuf-specs v0.4.1 github.com/sigstore/scaffolding v0.7.22 github.com/sigstore/sigstore-go v0.7.2 diff --git a/pkg/webhook/lrucache.go b/pkg/webhook/lrucache.go new file mode 100644 index 000000000..fedb969a7 --- /dev/null +++ b/pkg/webhook/lrucache.go @@ -0,0 +1,64 @@ +// +// Copyright 2026 The Sigstore Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package webhook + +import ( + "context" + "fmt" + "time" + + expirable "github.com/hashicorp/golang-lru/v2/expirable" + "knative.dev/pkg/logging" +) + +// LRUCache implements ResultCache using an LRU cache with TTL expiration. +// Only successful validations (PolicyResult non-nil) are cached. +// Failed validations (PolicyResult nil) are not cached to allow retries. +type LRUCache struct { + cache *expirable.LRU[string, *CacheResult] +} + +// NewLRUCache creates a new LRU cache with the given size and TTL. +func NewLRUCache(size int, ttl time.Duration) *LRUCache { + return &LRUCache{ + cache: expirable.NewLRU[string, *CacheResult](size, nil, ttl), + } +} + +func cacheKeyFor(image, uid, resourceVersion string) string { + return fmt.Sprintf("%s/%s/%s", image, uid, resourceVersion) +} + +func (c *LRUCache) Get(ctx context.Context, image, uid, resourceVersion string) *CacheResult { + result, ok := c.cache.Get(cacheKeyFor(image, uid, resourceVersion)) + if !ok { + logging.FromContext(ctx).Debugf("cache miss for image %s, policy UID %s", image, uid) + return nil + } + logging.FromContext(ctx).Debugf("cache hit for image %s, policy UID %s", image, uid) + return result +} + +func (c *LRUCache) Set(_ context.Context, image, name, uid, resourceVersion string, cacheResult *CacheResult) { //nolint: revive + if cacheResult.PolicyResult == nil { + return + } + copied := &CacheResult{ + PolicyResult: cacheResult.PolicyResult, + Errors: append([]error(nil), cacheResult.Errors...), + } + c.cache.Add(cacheKeyFor(image, uid, resourceVersion), copied) +} diff --git a/pkg/webhook/lrucache_test.go b/pkg/webhook/lrucache_test.go new file mode 100644 index 000000000..e42d6bdd6 --- /dev/null +++ b/pkg/webhook/lrucache_test.go @@ -0,0 +1,190 @@ +// +// Copyright 2026 The Sigstore Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package webhook + +import ( + "context" + "errors" + "testing" + "time" +) + +func TestLRUCacheSetGet(t *testing.T) { + cache := NewLRUCache(10, 1*time.Hour) + ctx := context.Background() + + want := &CacheResult{ + PolicyResult: &PolicyResult{ + AuthorityMatches: map[string]AuthorityMatch{}, + }, + } + cache.Set(ctx, "gcr.io/foo/bar@sha256:abc", "my-policy", "uid-1", "v1", want) + + got := cache.Get(ctx, "gcr.io/foo/bar@sha256:abc", "uid-1", "v1") + if got == nil { + t.Fatal("expected cache hit, got nil") + } + if got.PolicyResult == nil { + t.Fatal("expected PolicyResult, got nil") + } +} + +func TestLRUCacheMiss(t *testing.T) { + cache := NewLRUCache(10, 1*time.Hour) + ctx := context.Background() + + got := cache.Get(ctx, "gcr.io/foo/bar@sha256:abc", "uid-1", "v1") + if got != nil { + t.Fatalf("expected cache miss (nil), got %v", got) + } +} + +func TestLRUCacheSkipsErrors(t *testing.T) { + cache := NewLRUCache(10, 1*time.Hour) + ctx := context.Background() + + // Failed validation: PolicyResult is nil, only errors present. + // This is the case when no authorities matched (validator.go:590-591). + cache.Set(ctx, "gcr.io/foo/bar@sha256:abc", "my-policy", "uid-1", "v1", &CacheResult{ + Errors: []error{errors.New("image not signed")}, + }) + + got := cache.Get(ctx, "gcr.io/foo/bar@sha256:abc", "uid-1", "v1") + if got != nil { + t.Fatalf("expected cache miss for failed validation, got %v", got) + } +} + +func TestLRUCachePartialSuccess(t *testing.T) { + cache := NewLRUCache(10, 1*time.Hour) + ctx := context.Background() + + // Partial success: PolicyResult is non-nil (at least one authority matched) + // but there are also errors from authorities that didn't match. + // This is the common case with multi-authority CIPs (validator.go:641). + cache.Set(ctx, "gcr.io/foo/bar@sha256:abc", "my-policy", "uid-1", "v1", &CacheResult{ + PolicyResult: &PolicyResult{ + AuthorityMatches: map[string]AuthorityMatch{ + "authority-0": {Static: true}, + }, + }, + Errors: []error{errors.New("authority-1: signature invalid")}, + }) + + got := cache.Get(ctx, "gcr.io/foo/bar@sha256:abc", "uid-1", "v1") + if got == nil { + t.Fatal("expected cache hit for partial success (PolicyResult non-nil), got nil") + } + if got.PolicyResult == nil { + t.Fatal("expected PolicyResult in cached result") + } + if len(got.Errors) != 1 { + t.Fatalf("expected 1 error in cached result, got %d", len(got.Errors)) + } +} + +func TestLRUCacheTTLExpiry(t *testing.T) { + cache := NewLRUCache(10, 50*time.Millisecond) + ctx := context.Background() + + cache.Set(ctx, "gcr.io/foo/bar@sha256:abc", "my-policy", "uid-1", "v1", &CacheResult{ + PolicyResult: &PolicyResult{ + AuthorityMatches: map[string]AuthorityMatch{}, + }, + }) + + // Should hit immediately + if got := cache.Get(ctx, "gcr.io/foo/bar@sha256:abc", "uid-1", "v1"); got == nil { + t.Fatal("expected cache hit before TTL expiry") + } + + // Wait for TTL to expire + time.Sleep(100 * time.Millisecond) + + if got := cache.Get(ctx, "gcr.io/foo/bar@sha256:abc", "uid-1", "v1"); got != nil { + t.Fatalf("expected cache miss after TTL expiry, got %v", got) + } +} + +func TestLRUCacheEviction(t *testing.T) { + cache := NewLRUCache(2, 1*time.Hour) + ctx := context.Background() + result := &CacheResult{ + PolicyResult: &PolicyResult{ + AuthorityMatches: map[string]AuthorityMatch{}, + }, + } + + cache.Set(ctx, "image-1", "p", "uid-1", "v1", result) + cache.Set(ctx, "image-2", "p", "uid-1", "v1", result) + cache.Set(ctx, "image-3", "p", "uid-1", "v1", result) // evicts image-1 + + if got := cache.Get(ctx, "image-1", "uid-1", "v1"); got != nil { + t.Fatal("expected image-1 to be evicted") + } + if got := cache.Get(ctx, "image-2", "uid-1", "v1"); got == nil { + t.Fatal("expected image-2 to still be cached") + } + if got := cache.Get(ctx, "image-3", "uid-1", "v1"); got == nil { + t.Fatal("expected image-3 to still be cached") + } +} + +func TestLRUCacheKeyIsolation(t *testing.T) { + cache := NewLRUCache(10, 1*time.Hour) + ctx := context.Background() + result := &CacheResult{ + PolicyResult: &PolicyResult{ + AuthorityMatches: map[string]AuthorityMatch{}, + }, + } + + cache.Set(ctx, "image-a", "p", "uid-1", "v1", result) + + // Different image + if got := cache.Get(ctx, "image-b", "uid-1", "v1"); got != nil { + t.Fatal("expected miss for different image") + } + // Different UID + if got := cache.Get(ctx, "image-a", "uid-2", "v1"); got != nil { + t.Fatal("expected miss for different UID") + } + // Correct key + if got := cache.Get(ctx, "image-a", "uid-1", "v1"); got == nil { + t.Fatal("expected hit for matching key") + } +} + +func TestLRUCacheResourceVersionInvalidation(t *testing.T) { + cache := NewLRUCache(10, 1*time.Hour) + ctx := context.Background() + result := &CacheResult{ + PolicyResult: &PolicyResult{ + AuthorityMatches: map[string]AuthorityMatch{}, + }, + } + + cache.Set(ctx, "image-a", "my-policy", "uid-1", "v1", result) + + // Same image+uid but new resourceVersion (policy was updated) + if got := cache.Get(ctx, "image-a", "uid-1", "v2"); got != nil { + t.Fatal("expected miss for updated resourceVersion") + } + // Original version still hits + if got := cache.Get(ctx, "image-a", "uid-1", "v1"); got == nil { + t.Fatal("expected hit for original resourceVersion") + } +} diff --git a/pkg/webhook/validator.go b/pkg/webhook/validator.go index c03a93a8c..68bb37fb8 100644 --- a/pkg/webhook/validator.go +++ b/pkg/webhook/validator.go @@ -418,11 +418,6 @@ func validatePolicies(ctx context.Context, namespace string, ref name.Reference, result := retChannelType{name: cipName} result.policyResult, result.errors = ValidatePolicy(ctx, namespace, ref, cip, kc, remoteOpts...) - // Cache the result. - FromContext(ctx).Set(ctx, ref.Name(), cipName, string(cip.UID), cip.ResourceVersion, &CacheResult{ - PolicyResult: result.policyResult, - Errors: result.errors, - }) results <- result }() } @@ -638,6 +633,11 @@ func ValidatePolicy(ctx context.Context, namespace string, ref name.Reference, c return nil, append(authorityErrors, asFieldError(cip.Mode == "warn", warn)) } } + // Cache the result. Set is a no-op when PolicyResult is nil. + FromContext(ctx).Set(ctx, ref.String(), "", string(cip.UID), cip.ResourceVersion, &CacheResult{ + PolicyResult: policyResult, + Errors: authorityErrors, + }) return policyResult, authorityErrors } diff --git a/pkg/webhook/validator_test.go b/pkg/webhook/validator_test.go index c04d42a96..d97bbb7e6 100644 --- a/pkg/webhook/validator_test.go +++ b/pkg/webhook/validator_test.go @@ -35,6 +35,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/authn/k8schain" "github.com/google/go-containerregistry/pkg/name" v1 "github.com/google/go-containerregistry/pkg/v1" @@ -4234,3 +4235,254 @@ func TestDiscoverAttestationsOCI11PartialProcessingFailure(t *testing.T) { t.Errorf("Expected 2 signatures (second failed), got %d", len(sigs)) } } + +// cacheTestFixtures returns common test fixtures for cache integration tests. +func cacheTestFixtures(t *testing.T) (*ecdsa.PublicKey, name.Reference, authn.Keychain) { + t.Helper() + + var authorityKeyCosignPub *ecdsa.PublicKey + pems := parsePems([]byte(authorityKeyCosignPubString)) + if len(pems) > 0 { + key, _ := x509.ParsePKIXPublicKey(pems[0].Bytes) + authorityKeyCosignPub = key.(*ecdsa.PublicKey) + } else { + t.Fatal("Error parsing authority key from string") + } + + digest := name.MustParseReference("gcr.io/distroless/static:nonroot@sha256:be5d77c62dbe7fedfb0a4e5ec2f91078080800ab1f18358e5f31fcc8faa023c4") + + kc, err := k8schain.NewNoClient(context.Background()) + if err != nil { + t.Fatal(err) + } + + return authorityKeyCosignPub, digest, kc +} + +func TestValidatePolicyCacheHit(t *testing.T) { + // Save and restore global mock functions + origCVS := cosignVerifySignatures + defer func() { cosignVerifySignatures = origCVS }() + + callCount := 0 + cosignVerifySignatures = func(_ context.Context, _ name.Reference, _ *cosign.CheckOpts) ([]oci.Signature, bool, error) { + callCount++ + sig, err := static.NewSignature(nil, "") + if err != nil { + return nil, false, err + } + return []oci.Signature{sig}, true, nil + } + + authorityKeyCosignPub, digest, kc := cacheTestFixtures(t) + + ctx := context.Background() + + // Inject a real cache into context + cache := NewLRUCache(10, 1*time.Hour) + ctx = ToContext(ctx, cache) + + cip := webhookcip.ClusterImagePolicy{ + Authorities: []webhookcip.Authority{{ + Key: &webhookcip.KeyRef{ + Data: authorityKeyCosignPubString, + PublicKeys: []crypto.PublicKey{authorityKeyCosignPub}, + HashAlgorithm: signaturealgo.DefaultSignatureAlgorithm, + HashAlgorithmCode: crypto.SHA256, + }, + }}, + } + cip.UID = "test-uid" + cip.ResourceVersion = "v1" + + // First call - should invoke cosign + result1, errs1 := ValidatePolicy(ctx, system.Namespace(), digest, cip, kc) + if len(errs1) > 0 { + t.Fatalf("unexpected errors: %v", errs1) + } + if result1 == nil { + t.Fatal("expected non-nil PolicyResult") + } + if callCount != 1 { + t.Fatalf("expected cosign to be called once, got %d", callCount) + } + + // Second call - should return cached result without calling cosign + result2, errs2 := ValidatePolicy(ctx, system.Namespace(), digest, cip, kc) + if len(errs2) > 0 { + t.Fatalf("unexpected errors: %v", errs2) + } + if result2 == nil { + t.Fatal("expected non-nil cached PolicyResult") + } + if callCount != 1 { + t.Fatalf("expected cosign NOT to be called again (cache hit), got %d calls", callCount) + } +} + +func TestValidatePolicyCacheSkipsErrors(t *testing.T) { + origCVS := cosignVerifySignatures + defer func() { cosignVerifySignatures = origCVS }() + + callCount := 0 + cosignVerifySignatures = func(_ context.Context, _ name.Reference, _ *cosign.CheckOpts) ([]oci.Signature, bool, error) { + callCount++ + return nil, false, errors.New("image not signed") + } + + authorityKeyCosignPub, digest, kc := cacheTestFixtures(t) + + ctx := context.Background() + + cache := NewLRUCache(10, 1*time.Hour) + ctx = ToContext(ctx, cache) + + cip := webhookcip.ClusterImagePolicy{ + Authorities: []webhookcip.Authority{{ + Key: &webhookcip.KeyRef{ + Data: authorityKeyCosignPubString, + PublicKeys: []crypto.PublicKey{authorityKeyCosignPub}, + HashAlgorithm: signaturealgo.DefaultSignatureAlgorithm, + HashAlgorithmCode: crypto.SHA256, + }, + }}, + } + cip.UID = "test-uid" + cip.ResourceVersion = "v1" + + // First call - should fail and NOT cache the error + _, errs1 := ValidatePolicy(ctx, system.Namespace(), digest, cip, kc) + if len(errs1) == 0 { + t.Fatal("expected errors on first call") + } + if callCount != 1 { + t.Fatalf("expected cosign to be called once, got %d", callCount) + } + + // Second call - should call cosign again because errors aren't cached + _, errs2 := ValidatePolicy(ctx, system.Namespace(), digest, cip, kc) + if len(errs2) == 0 { + t.Fatal("expected errors on second call") + } + if callCount != 2 { + t.Fatalf("expected cosign to be called again (errors not cached), got %d calls", callCount) + } +} + +func TestValidatePolicyCachePartialSuccess(t *testing.T) { + // Multi-authority CIP: one authority fails (cosign), one passes (static). + // ValidatePolicy returns non-nil PolicyResult AND non-empty errors. + // This partial success SHOULD be cached. + origCVS := cosignVerifySignatures + defer func() { cosignVerifySignatures = origCVS }() + + callCount := 0 + cosignVerifySignatures = func(_ context.Context, _ name.Reference, _ *cosign.CheckOpts) ([]oci.Signature, bool, error) { + callCount++ + return nil, false, errors.New("signature invalid") + } + + authorityKeyCosignPub, digest, kc := cacheTestFixtures(t) + + ctx := context.Background() + + cache := NewLRUCache(10, 1*time.Hour) + ctx = ToContext(ctx, cache) + + cip := webhookcip.ClusterImagePolicy{ + Authorities: []webhookcip.Authority{ + { + // This authority will fail (cosign mock returns error) + Key: &webhookcip.KeyRef{ + Data: authorityKeyCosignPubString, + PublicKeys: []crypto.PublicKey{authorityKeyCosignPub}, + HashAlgorithm: signaturealgo.DefaultSignatureAlgorithm, + HashAlgorithmCode: crypto.SHA256, + }, + }, + { + // This authority will pass (static action) + Static: &webhookcip.StaticRef{Action: "pass"}, + }, + }, + } + cip.UID = "test-uid" + cip.ResourceVersion = "v1" + + // First call - one authority fails, one passes -> partial success + result1, errs1 := ValidatePolicy(ctx, system.Namespace(), digest, cip, kc) + if result1 == nil { + t.Fatal("expected non-nil PolicyResult (partial success)") + } + if len(errs1) == 0 { + t.Fatal("expected errors from the failing authority") + } + if callCount != 1 { + t.Fatalf("expected cosign to be called once, got %d", callCount) + } + + // Second call - should return cached result, cosign NOT called again + result2, errs2 := ValidatePolicy(ctx, system.Namespace(), digest, cip, kc) + if result2 == nil { + t.Fatal("expected non-nil cached PolicyResult") + } + if len(errs2) == 0 { + t.Fatal("expected cached errors from the failing authority") + } + if callCount != 1 { + t.Fatalf("expected cosign NOT to be called again (cache hit), got %d calls", callCount) + } +} + +func TestValidatePolicyNoCacheDefault(t *testing.T) { + origCVS := cosignVerifySignatures + defer func() { cosignVerifySignatures = origCVS }() + + callCount := 0 + cosignVerifySignatures = func(_ context.Context, _ name.Reference, _ *cosign.CheckOpts) ([]oci.Signature, bool, error) { + callCount++ + sig, err := static.NewSignature(nil, "") + if err != nil { + return nil, false, err + } + return []oci.Signature{sig}, true, nil + } + + authorityKeyCosignPub, digest, kc := cacheTestFixtures(t) + + ctx := context.Background() + + // Do NOT inject a cache - this is the default behavior + // FromContext(ctx) should return NoCache + + cip := webhookcip.ClusterImagePolicy{ + Authorities: []webhookcip.Authority{{ + Key: &webhookcip.KeyRef{ + Data: authorityKeyCosignPubString, + PublicKeys: []crypto.PublicKey{authorityKeyCosignPub}, + HashAlgorithm: signaturealgo.DefaultSignatureAlgorithm, + HashAlgorithmCode: crypto.SHA256, + }, + }}, + } + cip.UID = "test-uid" + cip.ResourceVersion = "v1" + + // First call + _, errs1 := ValidatePolicy(ctx, system.Namespace(), digest, cip, kc) + if len(errs1) > 0 { + t.Fatalf("unexpected errors: %v", errs1) + } + if callCount != 1 { + t.Fatalf("expected cosign called once, got %d", callCount) + } + + // Second call - should call cosign again because there is no cache + _, errs2 := ValidatePolicy(ctx, system.Namespace(), digest, cip, kc) + if len(errs2) > 0 { + t.Fatalf("unexpected errors: %v", errs2) + } + if callCount != 2 { + t.Fatalf("expected cosign called twice (no cache), got %d calls", callCount) + } +}