Skip to content

Commit 60d068b

Browse files
maryamtahhanmaryamtahhan
committed
feat: make Kyverno verification configurable (on by default)
Added KYVERNO_VERIFICATION_ENABLED environment variable to control signature verification behavior: - When enabled (default): Full verification with Kyverno annotation checking - When disabled: Skip signature verification, only resolve image digest (for development/testing) Signed-off-by: Maryam Tahhan <[email protected]>
1 parent efe1ecb commit 60d068b

File tree

5 files changed

+183
-40
lines changed

5 files changed

+183
-40
lines changed

‎api/v1alpha1/clustergkmcache_webhook.go‎

Lines changed: 49 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package v1alpha1
33
import (
44
"context"
55
"fmt"
6+
"time"
67

78
apierrors "k8s.io/apimachinery/pkg/api/errors"
89
"k8s.io/apimachinery/pkg/runtime"
@@ -55,17 +56,35 @@ func (w *ClusterGKMCache) Default(ctx context.Context, obj runtime.Object) error
5556
return nil
5657
}
5758

58-
// First check if the image already contains a digest (e.g., from Kyverno mutation)
59+
// Resolve & verify image -> digest
60+
cctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
61+
defer cancel()
62+
63+
kyvernoEnabled := isKyvernoVerificationEnabled()
5964
var digest string
60-
if extractedDigest := extractDigestFromImage(cache.Spec.Image); extractedDigest != "" {
61-
clustergkmcacheLog.Info("Image already contains digest (likely from Kyverno)", "image", cache.Spec.Image, "digest", extractedDigest)
62-
digest = extractedDigest
63-
}
64-
resolvedDigest, digestFound := cache.Annotations[utils.GMKCacheAnnotationResolvedDigest]
65-
if digestFound {
66-
// Digest hasn't changed so just return
67-
if digest == resolvedDigest {
68-
return nil
65+
var err error
66+
if kyvernoEnabled {
67+
// First check if the image already contains a digest (e.g., from Kyverno mutation)
68+
if extractedDigest := extractDigestFromImage(cache.Spec.Image); extractedDigest != "" {
69+
clustergkmcacheLog.Info("Image already contains digest (likely from Kyverno)", "image", cache.Spec.Image, "digest", extractedDigest)
70+
digest = extractedDigest
71+
}
72+
resolvedDigest, digestFound := cache.Annotations[utils.GMKCacheAnnotationResolvedDigest]
73+
if digestFound && digest != "" {
74+
// Digest hasn't changed so just return
75+
if digest == resolvedDigest {
76+
return nil
77+
}
78+
}
79+
} else {
80+
clustergkmcacheLog.V(1).Info("Resolving image digest (Kyverno verification disabled)", "image", cache.Spec.Image)
81+
digest, err = resolveImageDigest(cctx, cache.Spec.Image)
82+
if err != nil {
83+
clustergkmcacheLog.Error(err, "failed to resolve image digest")
84+
return apierrors.NewBadRequest(fmt.Sprintf(
85+
"image digest resolution failed for '%s': %s",
86+
cache.Spec.Image, err.Error(),
87+
))
6988
}
7089
}
7190
cache.Annotations[utils.GMKCacheAnnotationResolvedDigest] = digest
@@ -89,21 +108,23 @@ func (w *ClusterGKMCache) ValidateCreate(ctx context.Context, obj runtime.Object
89108
return nil, fmt.Errorf("%s must be set by mutating webhook", utils.GMKCacheAnnotationResolvedDigest)
90109
}
91110

92-
if _, exists := cache.Annotations[utils.KyvernoVerifyImagesAnnotation]; !exists {
93-
return nil, fmt.Errorf("%s must be set by kyverno", utils.KyvernoVerifyImagesAnnotation)
94-
}
111+
if isKyvernoVerificationEnabled() {
112+
if _, exists := cache.Annotations[utils.KyvernoVerifyImagesAnnotation]; !exists {
113+
return nil, fmt.Errorf("%s must be set by kyverno", utils.KyvernoVerifyImagesAnnotation)
114+
}
95115

96-
// Check Kyverno verification status if present
97-
if err := verifyKyvernoAnnotation(cache.Annotations); err != nil {
98-
return nil, fmt.Errorf("kyverno verification failed: %w", err)
116+
// Check Kyverno verification status if present
117+
if err := verifyKyvernoAnnotation(cache.Annotations); err != nil {
118+
return nil, fmt.Errorf("kyverno verification failed: %w", err)
119+
}
99120
}
100121

101122
return nil, nil
102123
}
103124

104125
// ValidateUpdate implements validation for update events.
105126
func (w *ClusterGKMCache) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
106-
gkmcachelog.Info("Validating Webhook called", "oldObj", oldObj, "newObj", newObj)
127+
clustergkmcacheLog.Info("Validating Webhook called", "oldObj", oldObj, "newObj", newObj)
107128
oldCache, ok1 := oldObj.(*ClusterGKMCache)
108129
newCache, ok2 := newObj.(*ClusterGKMCache)
109130
if !ok1 || !ok2 {
@@ -132,6 +153,17 @@ func (w *ClusterGKMCache) ValidateUpdate(_ context.Context, oldObj, newObj runti
132153
return nil, fmt.Errorf("%s must be set by mutating webhook when spec.image changes", utils.GMKCacheAnnotationResolvedDigest)
133154
}
134155

156+
// Validate Kyverno verification if enabled
157+
if isKyvernoVerificationEnabled() {
158+
if _, exists := newCache.Annotations[utils.KyvernoVerifyImagesAnnotation]; !exists {
159+
return nil, fmt.Errorf("%s must be set by kyverno", utils.KyvernoVerifyImagesAnnotation)
160+
}
161+
162+
if err := verifyKyvernoAnnotation(newCache.Annotations); err != nil {
163+
return nil, fmt.Errorf("kyverno verification failed: %w", err)
164+
}
165+
}
166+
135167
return nil, nil
136168
}
137169

‎api/v1alpha1/gkmcache_webhook.go‎

Lines changed: 108 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@ import (
44
"context"
55
"encoding/json"
66
"fmt"
7+
"os"
8+
"strings"
9+
"time"
710

811
apierrors "k8s.io/apimachinery/pkg/api/errors"
912
"k8s.io/apimachinery/pkg/runtime"
@@ -12,13 +15,15 @@ import (
1215
"sigs.k8s.io/controller-runtime/pkg/webhook"
1316
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
1417

18+
"github.com/google/go-containerregistry/pkg/authn"
1519
"github.com/google/go-containerregistry/pkg/name"
20+
gcrremote "github.com/google/go-containerregistry/pkg/v1/remote"
1621

1722
"github.com/redhat-et/GKM/pkg/utils"
1823
)
1924

2025
var (
21-
gkmcachelog = logf.Log.WithName("webhook-ns")
26+
gkmcacheLog = logf.Log.WithName("webhook-ns")
2227
_ webhook.CustomDefaulter = &GKMCache{}
2328
_ webhook.CustomValidator = &GKMCache{}
2429
)
@@ -41,39 +46,58 @@ func (w *GKMCache) SetupWebhookWithManager(mgr ctrl.Manager) error {
4146

4247
// Default implements the defaulting logic (mutating webhook)
4348
func (w *GKMCache) Default(ctx context.Context, obj runtime.Object) error {
44-
gkmcachelog.V(1).Info("Mutating Webhook called", "object", obj)
49+
gkmcacheLog.V(1).Info("Mutating Webhook called", "object", obj)
4550

4651
cache, ok := obj.(*GKMCache)
4752
if !ok {
4853
return apierrors.NewBadRequest(fmt.Sprintf("expected GKMCache, got %T", obj))
4954
}
50-
gkmcachelog.V(1).Info("Decoded GKMCache object", "name", cache.Name, "namespace", cache.Namespace)
55+
gkmcacheLog.V(1).Info("Decoded GKMCache object", "name", cache.Name, "namespace", cache.Namespace)
5156

5257
if cache.Annotations == nil {
5358
cache.Annotations = map[string]string{}
5459
}
5560

5661
if cache.Spec.Image == "" {
57-
gkmcachelog.Info("spec.image is empty, skipping")
62+
gkmcacheLog.Info("spec.image is empty, skipping")
5863
return nil
5964
}
6065

61-
// First check if the image already contains a digest (e.g., from Kyverno mutation)
66+
// Resolve & verify image -> digest
67+
cctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
68+
defer cancel()
69+
70+
kyvernoEnabled := isKyvernoVerificationEnabled()
6271
var digest string
63-
if extractedDigest := extractDigestFromImage(cache.Spec.Image); extractedDigest != "" {
64-
gkmcachelog.Info("Image already contains digest (likely from Kyverno)", "image", cache.Spec.Image, "digest", extractedDigest)
65-
digest = extractedDigest
66-
}
67-
resolvedDigest, digestFound := cache.Annotations[utils.GMKCacheAnnotationResolvedDigest]
68-
if digestFound {
69-
// Digest hasn't changed so just return
70-
if digest == resolvedDigest {
71-
return nil
72+
var err error
73+
if kyvernoEnabled {
74+
// First check if the image already contains a digest (e.g., from Kyverno mutation)
75+
if extractedDigest := extractDigestFromImage(cache.Spec.Image); extractedDigest != "" {
76+
gkmcacheLog.Info("Image already contains digest (likely from Kyverno)", "image", cache.Spec.Image, "digest", extractedDigest)
77+
digest = extractedDigest
78+
}
79+
resolvedDigest, digestFound := cache.Annotations[utils.GMKCacheAnnotationResolvedDigest]
80+
if digestFound && digest != "" {
81+
// Digest hasn't changed so just return
82+
if digest == resolvedDigest {
83+
return nil
84+
}
85+
}
86+
} else {
87+
gkmcacheLog.V(1).Info("Resolving image digest (Kyverno verification disabled)", "image", cache.Spec.Image)
88+
digest, err = resolveImageDigest(cctx, cache.Spec.Image)
89+
if err != nil {
90+
gkmcacheLog.Error(err, "failed to resolve image digest")
91+
return apierrors.NewBadRequest(fmt.Sprintf(
92+
"image digest resolution failed for '%s': %s",
93+
cache.Spec.Image, err.Error(),
94+
))
7295
}
7396
}
97+
7498
cache.Annotations[utils.GMKCacheAnnotationResolvedDigest] = digest
7599

76-
gkmcachelog.Info("added/updated resolvedDigest", "image", cache.Spec.Image, "digest", digest)
100+
gkmcacheLog.Info("added/updated resolvedDigest", "image", cache.Spec.Image, "digest", digest)
77101
return nil
78102
}
79103

@@ -96,21 +120,23 @@ func (w *GKMCache) ValidateCreate(ctx context.Context, obj runtime.Object) (admi
96120
return nil, fmt.Errorf("%s must be set by mutating webhook", utils.GMKCacheAnnotationResolvedDigest)
97121
}
98122

99-
if _, exists := cache.Annotations[utils.KyvernoVerifyImagesAnnotation]; !exists {
100-
return nil, fmt.Errorf("%s must be set by kyverno", utils.KyvernoVerifyImagesAnnotation)
101-
}
123+
if isKyvernoVerificationEnabled() {
124+
if _, exists := cache.Annotations[utils.KyvernoVerifyImagesAnnotation]; !exists {
125+
return nil, fmt.Errorf("%s must be set by kyverno", utils.KyvernoVerifyImagesAnnotation)
126+
}
102127

103-
// Check Kyverno verification status if present
104-
if err := verifyKyvernoAnnotation(cache.Annotations); err != nil {
105-
return nil, fmt.Errorf("kyverno verification failed: %w", err)
128+
// Check Kyverno verification status if present
129+
if err := verifyKyvernoAnnotation(cache.Annotations); err != nil {
130+
return nil, fmt.Errorf("kyverno verification failed: %w", err)
131+
}
106132
}
107133

108134
return nil, nil
109135
}
110136

111137
// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
112138
func (w *GKMCache) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
113-
gkmcachelog.V(1).Info("Validating Webhook called", "oldObj", oldObj, "newObj", newObj)
139+
gkmcacheLog.V(1).Info("Validating Webhook called", "oldObj", oldObj, "newObj", newObj)
114140
oldCache, ok1 := oldObj.(*GKMCache)
115141
newCache, ok2 := newObj.(*GKMCache)
116142
if !ok1 || !ok2 {
@@ -139,6 +165,17 @@ func (w *GKMCache) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Ob
139165
return nil, fmt.Errorf("%s must be set by mutating webhook when spec.image changes", utils.GMKCacheAnnotationResolvedDigest)
140166
}
141167

168+
// Validate Kyverno verification if enabled
169+
if isKyvernoVerificationEnabled() {
170+
if _, exists := newCache.Annotations[utils.KyvernoVerifyImagesAnnotation]; !exists {
171+
return nil, fmt.Errorf("%s must be set by kyverno", utils.KyvernoVerifyImagesAnnotation)
172+
}
173+
174+
if err := verifyKyvernoAnnotation(newCache.Annotations); err != nil {
175+
return nil, fmt.Errorf("kyverno verification failed: %w", err)
176+
}
177+
}
178+
142179
return nil, nil
143180
}
144181

@@ -149,7 +186,7 @@ func (w *GKMCache) ValidateDelete(_ context.Context, obj runtime.Object) (admiss
149186
return nil, apierrors.NewBadRequest(fmt.Sprintf("expected GKMCache, got %T", obj))
150187
}
151188

152-
clustergkmcacheLog.Info("validating GKMCache delete", "name", cache.Name)
189+
gkmcacheLog.Info("validating GKMCache delete", "name", cache.Name)
153190

154191
// Add delete validation logic here if needed.
155192
return nil, nil
@@ -198,3 +235,51 @@ func verifyKyvernoAnnotation(annotations map[string]string) error {
198235

199236
return nil
200237
}
238+
239+
// isKyvernoVerificationEnabled checks if Kyverno verification is enabled.
240+
// It reads from the KYVERNO_VERIFICATION_ENABLED environment variable.
241+
// Defaults to true (enabled) if not set or invalid.
242+
func isKyvernoVerificationEnabled() bool {
243+
envValue := os.Getenv(utils.EnvKyvernoEnabled)
244+
if envValue == "" {
245+
// Default to enabled
246+
return true
247+
}
248+
// Parse the value - accept "true", "1", "yes" as enabled
249+
// Everything else (including "false", "0", "no") is disabled
250+
switch strings.ToLower(envValue) {
251+
case "true", "1", "yes":
252+
return true
253+
case "false", "0", "no":
254+
return false
255+
default:
256+
// Invalid value, default to enabled and log a warning
257+
gkmcacheLog.Info("Invalid value for KYVERNO_VERIFICATION_ENABLED, defaulting to enabled", "value", envValue)
258+
return true
259+
}
260+
}
261+
262+
// resolveImageDigest resolves an image reference to its digest without verifying signatures.
263+
// This is used when Kyverno verification is disabled (development/testing mode).
264+
// It returns the image digest string (sha256:...) if successful.
265+
func resolveImageDigest(ctx context.Context, imageRef string) (string, error) {
266+
// Parse the image reference (tag or digest).
267+
ref, err := name.ParseReference(imageRef)
268+
if err != nil {
269+
return "", fmt.Errorf("parse image reference: %w", err)
270+
}
271+
272+
// Registry access options (authn.DefaultKeychain covers most cases).
273+
remoteOpts := []gcrremote.Option{
274+
gcrremote.WithAuthFromKeychain(authn.DefaultKeychain),
275+
gcrremote.WithContext(ctx),
276+
}
277+
278+
// Get the image descriptor to retrieve the digest
279+
descriptor, err := gcrremote.Get(ref, remoteOpts...)
280+
if err != nil {
281+
return "", fmt.Errorf("fetch image descriptor: %w", err)
282+
}
283+
284+
return descriptor.Digest.String(), nil
285+
}

‎pkg/database/cache.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,20 @@ func replaceUrlTag(imageURL, digest string) string {
307307
return ""
308308
}
309309

310+
// Check if the image already has a digest (e.g., from Kyverno mutation)
311+
// Format: registry/image:tag@sha256:digest
312+
if strings.Contains(imageURL, "@") {
313+
// Image already has a digest, check if it matches
314+
atIndex := strings.Index(imageURL, "@")
315+
existingDigest := imageURL[atIndex+1:]
316+
if existingDigest == digest {
317+
// Same digest, return as-is
318+
return imageURL
319+
}
320+
// Different digest, replace it
321+
return imageURL[:atIndex] + "@" + digest
322+
}
323+
310324
// Tokenize the Image URL
311325
lastColonIndex := strings.LastIndex(imageURL, ":")
312326
if lastColonIndex == -1 {

‎pkg/database/cache_test.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,14 @@ func TestReplaceUrlTag(t *testing.T) {
5252
t.Logf("TEST: replaceUrlTag() with no digest - Should fail (empty string)")
5353
updateUrl = replaceUrlTag("quay.io/test-33/image", "")
5454
require.Equal(t, updateUrl, "")
55+
56+
t.Logf("TEST: replaceUrlTag() with existing digest (Kyverno case) - same digest - Should return unchanged")
57+
updateUrl = replaceUrlTag("quay.io/gkm/cache-examples:vector-add-cache-rocm@sha256:bf6f7ea60274882031ad81434aa9c9ac0e4ff280cd1513db239dbbd705b6511c", "sha256:bf6f7ea60274882031ad81434aa9c9ac0e4ff280cd1513db239dbbd705b6511c")
58+
require.Equal(t, updateUrl, "quay.io/gkm/cache-examples:vector-add-cache-rocm@sha256:bf6f7ea60274882031ad81434aa9c9ac0e4ff280cd1513db239dbbd705b6511c")
59+
60+
t.Logf("TEST: replaceUrlTag() with existing digest (Kyverno case) - different digest - Should replace digest")
61+
updateUrl = replaceUrlTag("quay.io/gkm/cache-examples:vector-add-cache-rocm@sha256:olddigest", "sha256:newdigest")
62+
require.Equal(t, updateUrl, "quay.io/gkm/cache-examples:vector-add-cache-rocm@sha256:newdigest")
5563
})
5664
}
5765

‎pkg/utils/contants.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ const (
7272
ConfigMapIndexCsiImage = "gkm.csi.image"
7373
ConfigMapIndexCsiLogLevel = "gkm.csi.log.level"
7474
ConfigMapIndexNoGpu = "gkm.nogpu"
75+
ConfigMapIndexKyvernoEnabled = "gkm.kyverno.enabled"
7576

7677
// Duration for Kubernetes to Retry a failed request
7778
RetryOperatorConfigMapFailure = 5 * time.Second
@@ -82,4 +83,7 @@ const (
8283
RetryAgentNextStep = 1 * time.Second // KubeAPI call updated object so restart Reconcile
8384
RetryAgentUsagePoll = 5 * time.Second // Polling Cache to refresh GKMCacheNode Status usage data
8485
RetryAgentNodeStatusUpdate = 1 * time.Second // Status Updates not kicking Reconcile
86+
87+
// Environment Variables
88+
EnvKyvernoEnabled = "KYVERNO_VERIFICATION_ENABLED"
8589
)

0 commit comments

Comments
 (0)