Add unit, kuttl test; Refactoring

Signed-off-by: anushkamittal2001 <[email protected]>
nirmata · Sep 28, 2023 · 8c3f49e · 8c3f49e
1 parent 1c6e376
commit 8c3f49e
Show file tree

Hide file tree

Showing 13 changed files with 291 additions and 18 deletions.
diff --git a/pkg/controllers/webhook/controller.go b/pkg/controllers/webhook/controller.go
@@ -642,7 +642,7 @@ func (c *controller) buildResourceMutatingWebhookConfiguration(cfg config.Config
 				}
 			}
 			rules := p.GetSpec().Rules
-			operationStatusMap = analyseOperationStatusMapMutatingWebhookConfiguration(rules, operationStatusMap)
+			operationStatusMap = computeOperationsForMutatingWebhookConf(rules, operationStatusMap)
 		}
 		operationReq := getMinimumOperations(operationStatusMap)
 		webhookCfg := config.WebhookConfig{}
@@ -744,6 +744,19 @@ func (c *controller) buildDefaultResourceValidatingWebhookConfiguration(cfg conf
 		nil
 }
 
+func scanResourceFilter(resFilter kyvernov1.ResourceFilters, operationStatusMap map[string]bool) (bool, map[string]bool) {
+	opFound := false
+	for _, rf := range resFilter {
+		if rf.ResourceDescription.Operations != nil {
+			for _, o := range rf.ResourceDescription.Operations {
+				opFound = true
+				operationStatusMap[string(o)] = true
+			}
+		}
+	}
+	return opFound, operationStatusMap
+}
+
 func getOperationStatusMap() map[string]bool {
 	operationStatusMap := make(map[string]bool)
 	operationStatusMap[webhookCreate] = false
@@ -753,19 +766,34 @@ func getOperationStatusMap() map[string]bool {
 	return operationStatusMap
 }
 
-func analyseOperationStatusMapValidatingWebhookConfiguration(rules []kyvernov1.Rule, operationStatusMap map[string]bool) map[string]bool {
+func computeOperationsForValidatingWebhookConf(rules []kyvernov1.Rule, operationStatusMap map[string]bool) map[string]bool {
 	for _, r := range rules {
+		opFound := false
+		if len(r.MatchResources.Any) != 0 {
+			opFound, operationStatusMap = scanResourceFilter(r.MatchResources.Any, operationStatusMap)
+		}
+		if len(r.MatchResources.All) != 0 {
+			opFound, operationStatusMap = scanResourceFilter(r.MatchResources.All, operationStatusMap)
+		}
+		if len(r.ExcludeResources.Any) != 0 {
+			opFound, operationStatusMap = scanResourceFilter(r.ExcludeResources.Any, operationStatusMap)
+		}
+		if len(r.ExcludeResources.All) != 0 {
+			opFound, operationStatusMap = scanResourceFilter(r.ExcludeResources.All, operationStatusMap)
+		}
 		if r.MatchResources.ResourceDescription.Operations != nil {
 			for _, o := range r.MatchResources.ResourceDescription.Operations {
+				opFound = true
 				operationStatusMap[string(o)] = true
 			}
 		}
 		if r.ExcludeResources.ResourceDescription.Operations != nil {
-			for _, o := range r.MatchResources.ResourceDescription.Operations {
+			for _, o := range r.ExcludeResources.ResourceDescription.Operations {
+				opFound = true
 				operationStatusMap[string(o)] = true
 			}
 		}
-		if r.MatchResources.ResourceDescription.Operations == nil && r.ExcludeResources.ResourceDescription.Operations == nil {
+		if !opFound {
 			operationStatusMap[webhookCreate] = true
 			operationStatusMap[webhookUpdate] = true
 
@@ -777,19 +805,34 @@ func analyseOperationStatusMapValidatingWebhookConfiguration(rules []kyvernov1.R
 	return operationStatusMap
 }
 
-func analyseOperationStatusMapMutatingWebhookConfiguration(rules []kyvernov1.Rule, operationStatusMap map[string]bool) map[string]bool {
+func computeOperationsForMutatingWebhookConf(rules []kyvernov1.Rule, operationStatusMap map[string]bool) map[string]bool {
 	for _, r := range rules {
+		opFound := false
+		if len(r.MatchResources.Any) != 0 {
+			opFound, operationStatusMap = scanResourceFilter(r.MatchResources.Any, operationStatusMap)
+		}
+		if len(r.MatchResources.All) != 0 {
+			opFound, operationStatusMap = scanResourceFilter(r.MatchResources.All, operationStatusMap)
+		}
+		if len(r.ExcludeResources.Any) != 0 {
+			opFound, operationStatusMap = scanResourceFilter(r.ExcludeResources.Any, operationStatusMap)
+		}
+		if len(r.ExcludeResources.All) != 0 {
+			opFound, operationStatusMap = scanResourceFilter(r.ExcludeResources.All, operationStatusMap)
+		}
 		if r.MatchResources.ResourceDescription.Operations != nil {
 			for _, o := range r.MatchResources.ResourceDescription.Operations {
+				opFound = true
 				operationStatusMap[string(o)] = true
 			}
 		}
 		if r.ExcludeResources.ResourceDescription.Operations != nil {
-			for _, o := range r.MatchResources.ResourceDescription.Operations {
+			for _, o := range r.ExcludeResources.ResourceDescription.Operations {
+				opFound = true
 				operationStatusMap[string(o)] = true
 			}
 		}
-		if r.MatchResources.ResourceDescription.Operations == nil && r.ExcludeResources.ResourceDescription.Operations == nil {
+		if !opFound {
 			operationStatusMap[webhookCreate] = true
 			operationStatusMap[webhookUpdate] = true
 		}
@@ -832,7 +875,7 @@ func (c *controller) buildResourceValidatingWebhookConfiguration(cfg config.Conf
 				}
 			}
 			rules := p.GetSpec().Rules
-			operationStatusMap = analyseOperationStatusMapValidatingWebhookConfiguration(rules, operationStatusMap)
+			operationStatusMap = computeOperationsForValidatingWebhookConf(rules, operationStatusMap)
 		}
 		operationReq := getMinimumOperations(operationStatusMap)
 

diff --git a/pkg/controllers/webhook/controller_test.go b/pkg/controllers/webhook/controller_test.go
@@ -0,0 +1,177 @@
+package webhook
+
+import (
+	"reflect"
+	"testing"
+
+	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
+	v1 "github.com/kyverno/kyverno/api/kyverno/v1"
+	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
+)
+
+func TestGetMinimumOperations(t *testing.T) {
+	testCases := []struct {
+		name           string
+		inputMap       map[string]bool
+		expectedResult []admissionregistrationv1.OperationType
+	}{
+		{
+			name: "Test Case 1",
+			inputMap: map[string]bool{
+				"CREATE": true,
+				"UPDATE": false,
+				"DELETE": true,
+			},
+			expectedResult: []admissionregistrationv1.OperationType{"CREATE", "DELETE"},
+		},
+		{
+			name: "Test Case 2",
+			inputMap: map[string]bool{
+				"CREATE":  false,
+				"UPDATE":  false,
+				"DELETE":  false,
+				"CONNECT": true,
+			},
+			expectedResult: []admissionregistrationv1.OperationType{"CONNECT"},
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			result := getMinimumOperations(testCase.inputMap)
+			if !reflect.DeepEqual(result, testCase.expectedResult) {
+				t.Errorf("Expected %v, but got %v", testCase.expectedResult, result)
+			}
+		})
+	}
+}
+
+func TestComputeOperationsForMutatingWebhookConf(t *testing.T) {
+	testCases := []struct {
+		name           string
+		rules          []kyvernov1.Rule
+		expectedResult map[string]bool
+	}{
+		{
+			name: "Test Case 1",
+			rules: []kyvernov1.Rule{
+				{
+					MatchResources: kyvernov1.MatchResources{
+						ResourceDescription: kyvernov1.ResourceDescription{
+							Operations: []v1.AdmissionOperation{"CREATE"},
+						},
+					},
+				},
+			},
+			expectedResult: map[string]bool{
+				"CREATE": true,
+			},
+		},
+		{
+			name: "Test Case 2",
+			rules: []kyvernov1.Rule{
+				{
+					MatchResources:   kyvernov1.MatchResources{},
+					ExcludeResources: kyvernov1.MatchResources{},
+				},
+				{
+					MatchResources:   kyvernov1.MatchResources{},
+					ExcludeResources: kyvernov1.MatchResources{},
+				},
+			},
+			expectedResult: map[string]bool{
+				webhookCreate: true,
+				webhookUpdate: true,
+			},
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			result := computeOperationsForMutatingWebhookConf(testCase.rules, make(map[string]bool))
+			if !reflect.DeepEqual(result, testCase.expectedResult) {
+				t.Errorf("Expected %v, but got %v", testCase.expectedResult, result)
+			}
+		})
+	}
+}
+
+func TestComputeOperationsForValidatingWebhookConf(t *testing.T) {
+	testCases := []struct {
+		name           string
+		rules          []kyvernov1.Rule
+		expectedResult map[string]bool
+	}{
+		{
+			name: "Test Case 1",
+			rules: []kyvernov1.Rule{
+				{
+					MatchResources: kyvernov1.MatchResources{
+						ResourceDescription: kyvernov1.ResourceDescription{
+							Operations: []v1.AdmissionOperation{"CREATE"},
+						},
+					},
+				},
+				{
+					ExcludeResources: kyvernov1.MatchResources{
+						ResourceDescription: kyvernov1.ResourceDescription{
+							Operations: []v1.AdmissionOperation{"DELETE"},
+						},
+					},
+				},
+			},
+			expectedResult: map[string]bool{
+				"CREATE": true,
+				"DELETE": true,
+			},
+		},
+		{
+			name: "Test Case 2",
+			rules: []kyvernov1.Rule{
+				{
+					MatchResources:   kyvernov1.MatchResources{},
+					ExcludeResources: kyvernov1.MatchResources{},
+				},
+				{
+					MatchResources:   kyvernov1.MatchResources{},
+					ExcludeResources: kyvernov1.MatchResources{},
+				},
+			},
+			expectedResult: map[string]bool{
+				webhookCreate: true,
+				webhookUpdate: true,
+			},
+		},
+		{
+			name: "Test Case 3",
+			rules: []kyvernov1.Rule{
+				{
+					MatchResources: kyvernov1.MatchResources{
+						ResourceDescription: kyvernov1.ResourceDescription{
+							Operations: []v1.AdmissionOperation{"CREATE", "UPDATE"},
+						},
+					},
+					ExcludeResources: kyvernov1.MatchResources{
+						ResourceDescription: kyvernov1.ResourceDescription{
+							Operations: []v1.AdmissionOperation{"DELETE"},
+						},
+					},
+				},
+			},
+			expectedResult: map[string]bool{
+				webhookCreate: true,
+				webhookUpdate: true,
+				"DELETE":      true,
+			},
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			result := computeOperationsForValidatingWebhookConf(testCase.rules, make(map[string]bool))
+			if !reflect.DeepEqual(result, testCase.expectedResult) {
+				t.Errorf("Expected %v, but got %v", testCase.expectedResult, result)
+			}
+		})
+	}
+}
diff --git a/test/conformance/kuttl/webhooks/all-scale/webhooks.yaml b/test/conformance/kuttl/webhooks/all-scale/webhooks.yaml
@@ -13,8 +13,6 @@ webhooks:
     operations:
     - CREATE
     - UPDATE
-    - DELETE
-    - CONNECT
     resources:
     - '*/scale'
     scope: '*'
diff --git a/test/conformance/kuttl/webhooks/double-wildcard/webhooks.yaml b/test/conformance/kuttl/webhooks/double-wildcard/webhooks.yaml
@@ -14,8 +14,6 @@ webhooks:
     operations:
     - CREATE
     - UPDATE
-    - DELETE
-    - CONNECT
     resources:
     - '*/*'
     scope: '*'
diff --git a/test/conformance/kuttl/webhooks/dynamic-op/01-policy.yaml b/test/conformance/kuttl/webhooks/dynamic-op/01-policy.yaml
@@ -0,0 +1,6 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+apply:
+- policy.yaml
+assert:
+- policy-assert.yaml
diff --git a/test/conformance/kuttl/webhooks/dynamic-op/02-webhooks.yaml b/test/conformance/kuttl/webhooks/dynamic-op/02-webhooks.yaml
@@ -0,0 +1,4 @@
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+assert:
+- webhooks.yaml
diff --git a/test/conformance/kuttl/webhooks/dynamic-op/README.md b/test/conformance/kuttl/webhooks/dynamic-op/README.md
@@ -0,0 +1,3 @@
+## Description
+
+This test verifies that operations configured dynmically are correct
diff --git a/test/conformance/kuttl/webhooks/dynamic-op/policy-assert.yaml b/test/conformance/kuttl/webhooks/dynamic-op/policy-assert.yaml
@@ -0,0 +1,9 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: require-labels
+status:
+  conditions:
+    - reason: Succeeded
+      status: "True"
+      type: Ready
diff --git a/test/conformance/kuttl/webhooks/dynamic-op/policy.yaml b/test/conformance/kuttl/webhooks/dynamic-op/policy.yaml
@@ -0,0 +1,24 @@
+apiVersion: kyverno.io/v1
+kind: ClusterPolicy
+metadata:
+  name: require-labels
+  annotations:
+    pod-policies.kyverno.io/autogen-controllers: none
+spec:
+  validationFailureAction: Audit
+  background: false
+  rules:
+    - name: require-team
+      match:
+        any:
+          - resources:
+              kinds:
+                - '*/scale'
+              operations:
+              - CREATE
+      validate:
+        message: 'The label `team` is required.'
+        pattern:
+          metadata:
+            labels:
+              team: '?*'
diff --git a/test/conformance/kuttl/webhooks/dynamic-op/webhooks.yaml b/test/conformance/kuttl/webhooks/dynamic-op/webhooks.yaml
@@ -0,0 +1,17 @@
+apiVersion: admissionregistration.k8s.io/v1
+kind: ValidatingWebhookConfiguration
+metadata:
+  labels:
+    webhook.kyverno.io/managed-by: kyverno
+  name: kyverno-resource-validating-webhook-cfg
+webhooks:
+- rules:
+  - apiGroups:
+    - '*'
+    apiVersions:
+    - '*'
+    operations:
+    - CREATE
+    resources:
+    - '*/scale'
+    scope: '*'
diff --git a/test/conformance/kuttl/webhooks/only-pod/webhooks.yaml b/test/conformance/kuttl/webhooks/only-pod/webhooks.yaml
@@ -13,8 +13,6 @@ webhooks:
     operations:
     - CREATE
     - UPDATE
-    - DELETE
-    - CONNECT
     resources:
     - pods
     - pods/ephemeralcontainers

diff --git a/test/conformance/kuttl/webhooks/pod-all-subresources/webhooks.yaml b/test/conformance/kuttl/webhooks/pod-all-subresources/webhooks.yaml
@@ -13,8 +13,6 @@ webhooks:
     operations:
     - CREATE
     - UPDATE
-    - DELETE
-    - CONNECT
     resources:
     - pods/attach
     - pods/binding