From 8ca4720c10f7f944533e9cba77681dc43b4f1086 Mon Sep 17 00:00:00 2001 From: Yi Rae Kim Date: Tue, 5 Dec 2023 09:32:13 -0500 Subject: [PATCH] Add editable operations Ref: https://issues.redhat.com/browse/ACM-7470 Signed-off-by: Yi Rae Kim --- api/v1alpha1/gatekeeper_types.go | 5 ++ api/v1alpha1/zz_generated.deepcopy.go | 5 ++ .../operator.gatekeeper.sh_gatekeepers.yaml | 11 ++++ .../operator.gatekeeper.sh_gatekeepers.yaml | 11 ++++ controllers/gatekeeper_controller.go | 43 ++++++++++++++ controllers/gatekeeper_controller_test.go | 46 +++++++++++++++ test/e2e/gatekeeper_controller_test.go | 58 +++++++++++++++++++ 7 files changed, 179 insertions(+) diff --git a/api/v1alpha1/gatekeeper_types.go b/api/v1alpha1/gatekeeper_types.go index 6a3f413db..a2f06d574 100644 --- a/api/v1alpha1/gatekeeper_types.go +++ b/api/v1alpha1/gatekeeper_types.go @@ -145,6 +145,8 @@ type WebhookConfig struct { // +optional Resources *corev1.ResourceRequirements `json:"resources,omitempty"` // +optional + Operations []OperationType `json:"operations,omitempty"` + // +optional DisabledBuiltins []string `json:"disabledBuiltins,omitempty"` // +optional // Sets the --log-mutations flag which enables logging of mutation events and errors. This defaults to Disabled. @@ -155,6 +157,9 @@ type WebhookConfig struct { MutationAnnotations *Mode `json:"mutationAnnotations,omitempty"` } +// +kubebuilder:validation:Enum:=CONNECT;CREATE;UPDATE;DELETE;* +type OperationType admregv1.OperationType + // +kubebuilder:validation:Enum:=DEBUG;INFO;WARNING;ERROR type LogLevelMode string diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 78843b580..548ca2ea1 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -322,6 +322,11 @@ func (in *WebhookConfig) DeepCopyInto(out *WebhookConfig) { *out = new(v1.ResourceRequirements) (*in).DeepCopyInto(*out) } + if in.Operations != nil { + in, out := &in.Operations, &out.Operations + *out = make([]OperationType, len(*in)) + copy(*out, *in) + } if in.DisabledBuiltins != nil { in, out := &in.DisabledBuiltins, &out.DisabledBuiltins *out = make([]string, len(*in)) diff --git a/bundle/manifests/operator.gatekeeper.sh_gatekeepers.yaml b/bundle/manifests/operator.gatekeeper.sh_gatekeepers.yaml index 7065a2cce..c39d6e67b 100644 --- a/bundle/manifests/operator.gatekeeper.sh_gatekeepers.yaml +++ b/bundle/manifests/operator.gatekeeper.sh_gatekeepers.yaml @@ -1114,6 +1114,17 @@ spec: "value". The requirements are ANDed. type: object type: object + operations: + items: + description: OperationType specifies an operation for a request. + enum: + - CONNECT + - CREATE + - UPDATE + - DELETE + - '*' + type: string + type: array replicas: format: int32 minimum: 0 diff --git a/config/crd/bases/operator.gatekeeper.sh_gatekeepers.yaml b/config/crd/bases/operator.gatekeeper.sh_gatekeepers.yaml index 552f2127d..5efb35fac 100644 --- a/config/crd/bases/operator.gatekeeper.sh_gatekeepers.yaml +++ b/config/crd/bases/operator.gatekeeper.sh_gatekeepers.yaml @@ -1114,6 +1114,17 @@ spec: "value". The requirements are ANDed. type: object type: object + operations: + items: + description: OperationType specifies an operation for a request. + enum: + - CONNECT + - CREATE + - UPDATE + - DELETE + - '*' + type: string + type: array replicas: format: int32 minimum: 0 diff --git a/controllers/gatekeeper_controller.go b/controllers/gatekeeper_controller.go index d67dbc527..9c9de144a 100644 --- a/controllers/gatekeeper_controller.go +++ b/controllers/gatekeeper_controller.go @@ -43,6 +43,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/source" + "github.com/gatekeeper/gatekeeper-operator/api/v1alpha1" operatorv1alpha1 "github.com/gatekeeper/gatekeeper-operator/api/v1alpha1" "github.com/gatekeeper/gatekeeper-operator/controllers/merge" "github.com/gatekeeper/gatekeeper-operator/pkg/platform" @@ -670,6 +671,7 @@ func webhookOverrides(obj *unstructured.Unstructured, webhook *operatorv1alpha1. return nil } +// override common properties func webhookConfigurationOverrides( obj *unstructured.Unstructured, webhook *operatorv1alpha1.WebhookConfig, @@ -694,9 +696,15 @@ func webhookConfigurationOverrides( return err } } + + if err := setOperations(obj, webhook.Operations, webhookName); err != nil { + return err + } + if err := setNamespaceSelector(obj, webhook.NamespaceSelector, gatekeeperNamespace, webhookName); err != nil { return err } + } else if err := setNamespaceSelector(obj, nil, gatekeeperNamespace, webhookName); err != nil { return err } @@ -1046,6 +1054,41 @@ func setNamespaceSelector( return setWebhookConfigurationWithFn(obj, webhookName, setNamespaceSelectorFn) } +func setOperations( + obj *unstructured.Unstructured, operations []v1alpha1.OperationType, webhookName string, +) error { + // If no operations is provided, no override for operations + if operations == nil { + return nil + } + + setOperationsFn := func(webhook map[string]interface{}) error { + rules := webhook["rules"].([]interface{}) + if len(rules) == 0 { + return nil + } + + converted := make([]interface{}, 0, len(operations)) + for _, op := range operations { + converted = append(converted, string(op)) + } + + for i, r := range rules { + firstRuleObj := r.(map[string]interface{}) + firstRuleObj["operations"] = converted + rules[i] = firstRuleObj + } + + if err := unstructured.SetNestedSlice(webhook, rules, "rules"); err != nil { + return errors.Wrapf(err, "Failed to set webhook operations") + } + + return nil + } + + return setWebhookConfigurationWithFn(obj, webhookName, setOperationsFn) +} + // Generic setters func setAffinity(obj *unstructured.Unstructured, spec operatorv1alpha1.GatekeeperSpec) error { diff --git a/controllers/gatekeeper_controller_test.go b/controllers/gatekeeper_controller_test.go index e30490b15..5cb607d2f 100644 --- a/controllers/gatekeeper_controller_test.go +++ b/controllers/gatekeeper_controller_test.go @@ -16,6 +16,7 @@ package controllers import ( "os" + "reflect" "testing" "time" @@ -1373,6 +1374,51 @@ func expectObjContainerArgument(g *WithT, containerName string, obj *unstructure return g.Expect(args) } +func TestWebhookOperations(t *testing.T) { + g := NewWithT(t) + + operations := []operatorv1alpha1.OperationType{"CREATE", "UPDATE", "DELETE"} + + gatekeeper := &operatorv1alpha1.Gatekeeper{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + Spec: operatorv1alpha1.GatekeeperSpec{ + Webhook: &operatorv1alpha1.WebhookConfig{ + Operations: operations, + }, + }, + } + + // test WebhookOperations override + valObj, err := util.GetManifestObject(ValidatingWebhookConfiguration) + g.Expect(err).ToNot(HaveOccurred()) + mutObj, err := util.GetManifestObject(MutatingWebhookConfiguration) + g.Expect(err).ToNot(HaveOccurred()) + + err = crOverrides(gatekeeper, ValidatingWebhookConfiguration, valObj, namespace, false, false) + g.Expect(err).ToNot(HaveOccurred()) + overrideWebhookOperations(g, valObj, ValidatingWebhookConfiguration, operations) + err = crOverrides(gatekeeper, MutatingWebhookConfiguration, mutObj, namespace, false, false) + g.Expect(err).ToNot(HaveOccurred()) + overrideWebhookOperations(g, mutObj, MutatingWebhookConfiguration, operations) +} + +func overrideWebhookOperations(g *WithT, obj *unstructured.Unstructured, webhookName string, expected []operatorv1alpha1.OperationType) { + assertWebhooksWithFn(g, obj, func(webhook map[string]interface{}) { + if webhook["name"] == webhookName { + current, found, err := unstructured.NestedSlice(webhook, "rules") + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(found).To(BeTrue()) + + for _, rule := range current { + operations := rule.(map[string]interface{})["operations"].([]string) + g.Expect(reflect.DeepEqual(expected, operations)).To(BeTrue()) + } + } + }) +} + func getContainerArgumentsMap(g *WithT, containerName string, obj *unstructured.Unstructured) map[string]string { argsMap := make(map[string]string) args := getContainerArgumentsSlice(g, containerName, obj) diff --git a/test/e2e/gatekeeper_controller_test.go b/test/e2e/gatekeeper_controller_test.go index 54f8c7e11..de30dd353 100644 --- a/test/e2e/gatekeeper_controller_test.go +++ b/test/e2e/gatekeeper_controller_test.go @@ -494,6 +494,64 @@ var _ = Describe("Gatekeeper", func() { auditDeployment, webhookDeployment = gatekeeperDeployments() byCheckingMutationDisabled(auditDeployment, webhookDeployment) }) + + It("Override Webhook operations with Create, Update, Delete, Connect", func() { + gatekeeper := &v1alpha1.Gatekeeper{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: gatekeeperNamespace, + Name: "gatekeeper", + }, + Spec: v1alpha1.GatekeeperSpec{ + Webhook: &v1alpha1.WebhookConfig{ + Operations: []v1alpha1.OperationType{ + "CREATE", "UPDATE", "CONNECT", "DELETE", + }, + }, + }, + } + Expect(K8sClient.Create(ctx, gatekeeper)).Should(Succeed()) + + By("Wait until new Deployments loaded") + gatekeeperDeployments() + + By("ValidatingWebhookConfiguration Rules should have 4 operations") + validatingWebhookConfiguration := &admregv1.ValidatingWebhookConfiguration{} + Eventually(func(g Gomega) { + err := K8sClient.Get(ctx, validatingWebhookName, validatingWebhookConfiguration) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(validatingWebhookConfiguration.Webhooks[0].Rules[0].Operations).Should(HaveLen(4)) + g.Expect(validatingWebhookConfiguration.Webhooks[1].Rules[0].Operations).Should(HaveLen(4)) + }, timeout, pollInterval).Should(Succeed()) + + By("MutatingWebhookConfiguration Rules should have 4 operations") + mutatingWebhookConfiguration := &admregv1.MutatingWebhookConfiguration{} + Eventually(func(g Gomega) { + err := K8sClient.Get(ctx, mutatingWebhookName, mutatingWebhookConfiguration) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(mutatingWebhookConfiguration.Webhooks[0].Rules[0].Operations).Should(HaveLen(4)) + }, timeout, pollInterval).Should(Succeed()) + + gatekeeper.Spec.Webhook.Operations = []v1alpha1.OperationType{"*"} + Expect(K8sClient.Update(ctx, gatekeeper)).Should(Succeed()) + + By("ValidatingWebhookConfiguration Rules should have 1 operations") + Eventually(func(g Gomega) { + err := K8sClient.Get(ctx, validatingWebhookName, validatingWebhookConfiguration) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(validatingWebhookConfiguration.Webhooks[0].Rules[0].Operations).Should(HaveLen(1)) + g.Expect(validatingWebhookConfiguration.Webhooks[0].Rules[0].Operations[0]).Should(BeEquivalentTo("*")) + g.Expect(validatingWebhookConfiguration.Webhooks[1].Rules[0].Operations).Should(HaveLen(1)) + g.Expect(validatingWebhookConfiguration.Webhooks[1].Rules[0].Operations[0]).Should(BeEquivalentTo("*")) + }, timeout*2, pollInterval).Should(Succeed()) + + By("MutatingWebhookConfiguration Rules should have 1 operations") + Eventually(func(g Gomega) { + err := K8sClient.Get(ctx, mutatingWebhookName, mutatingWebhookConfiguration) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(mutatingWebhookConfiguration.Webhooks[0].Rules[0].Operations).Should(HaveLen(1)) + g.Expect(mutatingWebhookConfiguration.Webhooks[0].Rules[0].Operations[0]).Should(BeEquivalentTo("*")) + }, timeout, pollInterval).Should(Succeed()) + }) }) })