feat(validation-webhook): validate global context reference (#9678)

* feat(validation-webhook): validate global context reference

Signed-off-by: Khaled Emara <khaled.emara@nirmata.com>

* fix(validation-webhook): global reference name

Signed-off-by: Khaled Emara <khaled.emara@nirmata.com>

* test(globalcontext): fix tests after valdiation

Signed-off-by: Khaled Emara <khaled.emara@nirmata.com>

* fix(policycache): dont add NotReady Policies

Signed-off-by: Khaled Emara <khaled.emara@nirmata.com>

* chore(globalcontext): rename e2e tests

Signed-off-by: Khaled Emara <khaled.emara@nirmata.com>

* chore(globalcontext): add entry errors

Signed-off-by: Khaled Emara <khaled.emara@nirmata.com>

* test(globalcontext): fix chainsaw  test

Signed-off-by: Khaled Emara <khaled.emara@nirmata.com>

---------

Signed-off-by: Khaled Emara <khaled.emara@nirmata.com>

cmd/cli/kubectl-kyverno/commands/apply/command.go

@@ -257,7 +257,7 @@ func (c *ApplyCommandConfig) applyPolicytoResource(
 	var validPolicies []kyvernov1.PolicyInterface
 	for _, pol := range policies {
 		// TODO we should return this info to the caller
-		_, err := policyvalidation.Validate(pol, nil, nil, true, config.KyvernoUserName(config.KyvernoServiceAccountName()))
+		_, err := policyvalidation.Validate(pol, nil, nil, nil, true, config.KyvernoUserName(config.KyvernoServiceAccountName()))
 		if err != nil {
 			log.Log.Error(err, "policy validation error")
 			rc.IncrementError(1)

cmd/cli/kubectl-kyverno/commands/oci/push/options.go

@@ -40,7 +40,7 @@ func (o options) execute(ctx context.Context, dir string, keychain authn.Keychai
 		return fmt.Errorf("unable to read policy file or directory %s (%w)", dir, err)
 	}
 	for _, policy := range policies {
-		if _, err := policyvalidation.Validate(policy, nil, nil, true, config.KyvernoUserName(config.KyvernoServiceAccountName())); err != nil {
+		if _, err := policyvalidation.Validate(policy, nil, nil, nil, true, config.KyvernoUserName(config.KyvernoServiceAccountName())); err != nil {
 			return fmt.Errorf("validating policy %s: %v", policy.GetName(), err)
 		}
 	}

cmd/cli/kubectl-kyverno/commands/test/test.go

@@ -145,7 +145,7 @@ func runTest(out io.Writer, testCase test.TestCase, registryAccess bool, auditWa
 	var validPolicies []kyvernov1.PolicyInterface
 	for _, pol := range policies {
 		// TODO we should return this info to the caller
-		_, err := policyvalidation.Validate(pol, nil, nil, true, config.KyvernoUserName(config.KyvernoServiceAccountName()))
+		_, err := policyvalidation.Validate(pol, nil, nil, nil, true, config.KyvernoUserName(config.KyvernoServiceAccountName()))
 		if err != nil {
 			log.Log.Error(err, "skipping invalid policy", "name", pol.GetName())
 			continue

cmd/kyverno/main.go

@@ -508,6 +508,7 @@ func main() {
 	)
 	policyHandlers := webhookspolicy.NewHandlers(
 		setup.KyvernoDynamicClient,
+		kyvernoInformer.Kyverno().V2alpha1().GlobalContextEntries(),
 		backgroundServiceAccountName,
 	)
 	resourceHandlers := webhooksresource.NewHandlers(

pkg/controllers/policycache/controller.go

@@ -74,7 +74,12 @@ func (c *controller) WarmUp() error {
 		if key, err := cache.MetaNamespaceKeyFunc(policy); err != nil {
 			return err
 		} else {
-			return c.cache.Set(key, policy, c.client.Discovery())
+			if policy.IsReady() {
+				return c.cache.Set(key, policy, c.client.Discovery())
+			} else {
+				c.cache.Unset(key)
+				return nil
+			}
 		}
 	}
 	cpols, err := c.cpolLister.List(labels.Everything())
@@ -85,7 +90,12 @@ func (c *controller) WarmUp() error {
 		if key, err := cache.MetaNamespaceKeyFunc(policy); err != nil {
 			return err
 		} else {
-			return c.cache.Set(key, policy, c.client.Discovery())
+			if policy.IsReady() {
+				return c.cache.Set(key, policy, c.client.Discovery())
+			} else {
+				c.cache.Unset(key)
+				return nil
+			}
 		}
 	}
 	return nil
@@ -104,7 +114,12 @@ func (c *controller) reconcile(ctx context.Context, logger logr.Logger, key, nam
 		return err
 	}
 	if policy.AdmissionProcessingEnabled() && !policy.GetSpec().CustomWebhookConfiguration() {
-		return c.cache.Set(key, policy, c.client.Discovery())
+		if policy.IsReady() {
+			return c.cache.Set(key, policy, c.client.Discovery())
+		} else {
+			c.cache.Unset(key)
+			return nil
+		}
 	} else {
 		c.cache.Unset(key)
 		return nil

pkg/globalcontext/externalapi/entry.go

@@ -2,6 +2,7 @@ package externalapi
 
 import (
 	"context"
+	"fmt"
 	"sync"
 	"time"
 
@@ -14,6 +15,7 @@ import (
 type entry struct {
 	sync.Mutex
 	data any
+	err  error
 	stop func()
 }
 
@@ -39,12 +41,13 @@ func New(
 	group.StartWithContext(ctx, func(ctx context.Context) {
 		// TODO: make sure we have called it at least once before returning
 		config := apicall.NewAPICallConfiguration(maxResponseLength)
-		caller := apicall.NewCaller(logger, "TODO", client, config)
+		caller := apicall.NewCaller(logger, "globalcontext", client, config)
 		wait.UntilWithContext(ctx, func(ctx context.Context) {
 			if data, err := doCall(ctx, caller, call); err != nil {
 				logger.Error(err, "failed to get data from api caller")
+				e.setData(nil, err)
 			} else {
-				e.setData(data)
+				e.setData(data, nil)
 			}
 		}, period)
 	})
@@ -54,6 +57,15 @@ func New(
 func (e *entry) Get() (any, error) {
 	e.Lock()
 	defer e.Unlock()
+
+	if e.err != nil {
+		return nil, e.err
+	}
+
+	if e.data == nil {
+		return nil, fmt.Errorf("no data available")
+	}
+
 	return e.data, nil
 }
 
@@ -63,10 +75,15 @@ func (e *entry) Stop() {
 	e.stop()
 }
 
-func (e *entry) setData(data any) {
+func (e *entry) setData(data any, err error) {
 	e.Lock()
 	defer e.Unlock()
-	e.data = data
+
+	if err != nil {
+		e.err = err
+	} else {
+		e.data = data
+	}
 }
 
 func doCall(ctx context.Context, caller apicall.Caller, call kyvernov1.APICall) (any, error) {

pkg/globalcontext/k8sresource/entry.go

@@ -18,7 +18,6 @@ type entry struct {
 	stop   func()
 }
 
-// TODO: error handling
 func New(ctx context.Context, client dynamic.Interface, gvr schema.GroupVersionResource, namespace string) (*entry, error) {
 	indexers := cache.Indexers{
 		cache.NamespaceIndex: cache.MetaNamespaceIndexFunc,

pkg/validation/policy/fuzz_test.go

@@ -14,6 +14,6 @@ func FuzzValidatePolicy(f *testing.F) {
 		p := &kyverno.ClusterPolicy{}
 		ff.GenerateStruct(p)
 
-		Validate(p, nil, nil, true, "admin")
+		Validate(p, nil, nil, nil, true, "admin")
 	})
 }

pkg/validation/policy/validate.go

@@ -16,8 +16,10 @@ import (
 	"github.com/kyverno/go-jmespath"
 	"github.com/kyverno/kyverno/api/kyverno"
 	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
+	kyvernov2alpha1 "github.com/kyverno/kyverno/api/kyverno/v2alpha1"
 	"github.com/kyverno/kyverno/ext/wildcard"
 	"github.com/kyverno/kyverno/pkg/autogen"
+	kyvernov2alpha1listers "github.com/kyverno/kyverno/pkg/client/listers/kyverno/v2alpha1"
 	"github.com/kyverno/kyverno/pkg/clients/dclient"
 	enginecontext "github.com/kyverno/kyverno/pkg/engine/context"
 	"github.com/kyverno/kyverno/pkg/engine/variables"
@@ -31,6 +33,7 @@ import (
 	admissionregistrationv1alpha1 "k8s.io/api/admissionregistration/v1alpha1"
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/apimachinery/pkg/util/yaml"
@@ -125,7 +128,7 @@ func checkValidationFailureAction(spec *kyvernov1.Spec) []string {
 }
 
 // Validate checks the policy and rules declarations for required configurations
-func Validate(policy, oldPolicy kyvernov1.PolicyInterface, client dclient.Interface, mock bool, username string) ([]string, error) {
+func Validate(policy, oldPolicy kyvernov1.PolicyInterface, client dclient.Interface, gctxentryLister kyvernov2alpha1listers.GlobalContextEntryLister, mock bool, username string) ([]string, error) {
 	var warnings []string
 	spec := policy.GetSpec()
 	background := spec.BackgroundProcessingEnabled()
@@ -400,6 +403,26 @@ func Validate(policy, oldPolicy kyvernov1.PolicyInterface, client dclient.Interf
 		checkForDeprecatedOperatorsInRule(rule, &warnings)
 	}
 
+	// global context entry validation
+	if gctxentryLister != nil {
+		gctxentries, err := gctxentryLister.List(labels.Everything())
+		if err != nil {
+			return nil, err
+		}
+		for _, rule := range rules {
+			if rule.Context == nil {
+				continue
+			}
+			for _, ctxEntry := range rule.Context {
+				if ctxEntry.GlobalReference != nil {
+					if !isGlobalContextEntryReady(ctxEntry.GlobalReference.Name, gctxentries) {
+						return nil, fmt.Errorf("global context entry %s is not ready", ctxEntry.Name)
+					}
+				}
+			}
+		}
+	}
+
 	// check for CEL expression warnings in case of CEL subrules
 	if ok, _ := vaputils.CanGenerateVAP(spec); ok && client != nil {
 		resolver := &resolver.ClientDiscoveryResolver{
@@ -450,6 +473,15 @@ func Validate(policy, oldPolicy kyvernov1.PolicyInterface, client dclient.Interf
 	return warnings, nil
 }
 
+func isGlobalContextEntryReady(name string, gctxentries []*kyvernov2alpha1.GlobalContextEntry) bool {
+	for _, gctxentry := range gctxentries {
+		if gctxentry.Name == name {
+			return true
+		}
+	}
+	return false
+}
+
 func ValidateVariables(p kyvernov1.PolicyInterface, backgroundMode bool) error {
 	vars, err := hasVariables(p)
 	if err != nil {

pkg/webhooks/policy/handlers.go

@@ -5,6 +5,8 @@ import (
 	"time"
 
 	"github.com/go-logr/logr"
+	kyvernov2alpha1informers "github.com/kyverno/kyverno/pkg/client/informers/externalversions/kyverno/v2alpha1"
+	kyvernov2alpha1listers "github.com/kyverno/kyverno/pkg/client/listers/kyverno/v2alpha1"
 	"github.com/kyverno/kyverno/pkg/clients/dclient"
 	admissionutils "github.com/kyverno/kyverno/pkg/utils/admission"
 	policyvalidate "github.com/kyverno/kyverno/pkg/validation/policy"
@@ -14,12 +16,14 @@ import (
 
 type policyHandlers struct {
 	client                       dclient.Interface
+	gctxentryLister              kyvernov2alpha1listers.GlobalContextEntryLister
 	backgroundServiceAccountName string
 }
 
-func NewHandlers(client dclient.Interface, serviceaccount string) webhooks.PolicyHandlers {
+func NewHandlers(client dclient.Interface, gctxentryInformer kyvernov2alpha1informers.GlobalContextEntryInformer, serviceaccount string) webhooks.PolicyHandlers {
 	return &policyHandlers{
 		client:                       client,
+		gctxentryLister:              gctxentryInformer.Lister(),
 		backgroundServiceAccountName: serviceaccount,
 	}
 }
@@ -30,7 +34,7 @@ func (h *policyHandlers) Validate(ctx context.Context, logger logr.Logger, reque
 		logger.Error(err, "failed to unmarshal policies from admission request")
 		return admissionutils.Response(request.UID, err)
 	}
-	warnings, err := policyvalidate.Validate(policy, oldPolicy, h.client, false, h.backgroundServiceAccountName)
+	warnings, err := policyvalidate.Validate(policy, oldPolicy, h.client, h.gctxentryLister, false, h.backgroundServiceAccountName)
 	if err != nil {
 		logger.Error(err, "policy validation errors")
 	}

test/conformance/chainsaw/globalcontext/gctxentry-not-exist/chainsaw-test.yaml

@@ -2,7 +2,7 @@ apiVersion: chainsaw.kyverno.io/v1alpha1
 kind: Test
 metadata:
   creationTimestamp: null
-  name: resource-not-exist
+  name: gctxentry-not-exist
 spec:
   steps:
   - name: setup
@@ -13,14 +13,10 @@ spec:
         file: main-deployment.yaml
     - apply:
         file: gctxentry.yaml
-    - apply:
-        file: clusterpolicy.yaml
-    - assert:
-        file: clusterpolicy-failed.yaml
   - name: negative
     try:
     - apply:
         expect:
         - check:
             ($error != null): true
-        file: new-deployment.yaml
+        file: clusterpolicy.yaml

test/conformance/chainsaw/globalcontext/gctxentry-not-exist/clusterpolicy.yaml

@@ -10,7 +10,7 @@ spec:
     context:
     - name: deploymentCount
       globalReference:
-        name: non-existent-reference
+        name: non-existent-gctx
         jmesPath: "length(@)"
     match:
       all:

test/conformance/chainsaw/globalcontext/not-ready/chainsaw-test.yaml

@@ -2,7 +2,7 @@ apiVersion: chainsaw.kyverno.io/v1alpha1
 kind: Test
 metadata:
   creationTimestamp: null
-  name: resource-not-exist
+  name: not-ready
 spec:
   steps:
   - name: setup
@@ -13,14 +13,18 @@ spec:
         file: main-deployment.yaml
     - apply:
         file: gctxentry.yaml
+    - assert:
+        file: gctxentry-exists.yaml
     - apply:
         file: clusterpolicy.yaml
+    - delete:
+        ref:
+          apiVersion: kyverno.io/v2alpha1
+          kind: GlobalContextEntry
+          name: deployments
     - assert:
         file: clusterpolicy-failed.yaml
-  - name: negative
-    try:
     - apply:
-        expect:
-        - check:
-            ($error != null): true
         file: new-deployment.yaml
+    - assert:
+        file: new-deployment-exists.yaml

test/conformance/chainsaw/globalcontext/not-ready/clusterpolicy.yaml

@@ -10,7 +10,7 @@ spec:
     context:
     - name: deploymentCount
       globalReference:
-        name: non-existent-reference
+        name: deployments
         jmesPath: "items | length(@)"
     match:
       all:

test/conformance/chainsaw/globalcontext/not-ready/gctxentry-exists.yaml

@@ -0,0 +1,4 @@
+apiVersion: kyverno.io/v2alpha1
+kind: GlobalContextEntry
+metadata:
+  name: deployments

test/conformance/chainsaw/globalcontext/not-ready/new-deployment-exists.yaml

@@ -0,0 +1,7 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: new-deployment
+  namespace: test-globalcontext
+  labels:
+    app: new-deployment

test/conformance/chainsaw/globalcontext/resource-not-exist/clusterpolicy-failed.yaml

@@ -1,9 +0,0 @@
-apiVersion: kyverno.io/v1
-kind: ClusterPolicy
-metadata:
-  name: namespace-has-coordinator
-status:
-  conditions:
-  - reason: Failed
-    status: "False"
-    type: Ready