From b7e2df7b349d048f6b6e2079de2d52542cfa9a99 Mon Sep 17 00:00:00 2001 From: Yi Rae Kim Date: Wed, 4 Oct 2023 16:10:35 -0400 Subject: [PATCH] Add overriding webhook operations Signed-off-by: Yi Rae Kim --- .github/workflows/ci_tests.yaml | 8 ++ api/v1alpha1/gatekeeper_types.go | 5 + api/v1alpha1/zz_generated.deepcopy.go | 9 ++ ...keeper-operator.clusterserviceversion.yaml | 4 + .../operator.gatekeeper.sh_gatekeepers.yaml | 11 ++ .../operator.gatekeeper.sh_gatekeepers.yaml | 11 ++ config/default/manager_auth_proxy_patch.yaml | 2 + config/manager/manager.yaml | 2 + config/samples/gatekeeper_operations.yaml | 12 ++ controllers/gatekeeper_controller.go | 46 +++++++ deploy/gatekeeper-operator.yaml | 4 + test/e2e/gatekeeper_controller_test.go | 118 ++++++++++++------ 12 files changed, 197 insertions(+), 35 deletions(-) create mode 100644 config/samples/gatekeeper_operations.yaml diff --git a/.github/workflows/ci_tests.yaml b/.github/workflows/ci_tests.yaml index 752dfb11d..96aa297eb 100644 --- a/.github/workflows/ci_tests.yaml +++ b/.github/workflows/ci_tests.yaml @@ -93,6 +93,10 @@ jobs: echo "::group::Operator Logs" cat operator.log echo "::endgroup::" + echo "::group::Deployments" + kubectl -n gatekeeper-system get deployments -o yaml + echo "::endgroup::" + configsync-e2e-test: name: Run configsync e2e tests @@ -134,6 +138,10 @@ jobs: echo "::group::Operator Logs" cat operator.log echo "::endgroup::" + echo "::group::Deployments" + kubectl -n gatekeeper-system get deployments -o yaml + echo "::endgroup::" + gatekeeper-e2e-tests: name: Run gatekeeper e2e tests diff --git a/api/v1alpha1/gatekeeper_types.go b/api/v1alpha1/gatekeeper_types.go index ee7458d39..ac5434b2c 100644 --- a/api/v1alpha1/gatekeeper_types.go +++ b/api/v1alpha1/gatekeeper_types.go @@ -127,9 +127,14 @@ type WebhookConfig struct { // +optional Resources *corev1.ResourceRequirements `json:"resources,omitempty"` // +optional + Operations *[]OperationType `json:"operations,omitempty"` + // +optional DisabledBuiltins []string `json:"disabledBuiltins,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 94565a921..1d7efa4ac 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -312,6 +312,15 @@ func (in *WebhookConfig) DeepCopyInto(out *WebhookConfig) { *out = new(v1.ResourceRequirements) (*in).DeepCopyInto(*out) } + if in.Operations != nil { + in, out := &in.Operations, &out.Operations + *out = new([]OperationType) + if **in != nil { + in, out := *in, *out + *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/gatekeeper-operator.clusterserviceversion.yaml b/bundle/manifests/gatekeeper-operator.clusterserviceversion.yaml index 2e5bf2f45..0c0dfd3b5 100644 --- a/bundle/manifests/gatekeeper-operator.clusterserviceversion.yaml +++ b/bundle/manifests/gatekeeper-operator.clusterserviceversion.yaml @@ -367,6 +367,8 @@ spec: capabilities: drop: - ALL + seccompProfile: + type: RuntimeDefault - args: - --health-probe-bind-address=:8081 - --metrics-bind-address=127.0.0.1:8080 @@ -403,6 +405,8 @@ spec: capabilities: drop: - ALL + seccompProfile: + type: RuntimeDefault securityContext: runAsNonRoot: true serviceAccountName: gatekeeper-operator-controller-manager diff --git a/bundle/manifests/operator.gatekeeper.sh_gatekeepers.yaml b/bundle/manifests/operator.gatekeeper.sh_gatekeepers.yaml index 1b0cfe1d9..4d3959f0b 100644 --- a/bundle/manifests/operator.gatekeeper.sh_gatekeepers.yaml +++ b/bundle/manifests/operator.gatekeeper.sh_gatekeepers.yaml @@ -1085,6 +1085,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 49574946a..e5d6a221c 100644 --- a/config/crd/bases/operator.gatekeeper.sh_gatekeepers.yaml +++ b/config/crd/bases/operator.gatekeeper.sh_gatekeepers.yaml @@ -1085,6 +1085,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/default/manager_auth_proxy_patch.yaml b/config/default/manager_auth_proxy_patch.yaml index f4642f464..4179673cb 100644 --- a/config/default/manager_auth_proxy_patch.yaml +++ b/config/default/manager_auth_proxy_patch.yaml @@ -25,6 +25,8 @@ spec: capabilities: drop: - ALL + seccompProfile: + type: RuntimeDefault - name: manager args: - "--health-probe-bind-address=:8081" diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index f023562cb..8e5cdce77 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -34,6 +34,8 @@ spec: imagePullPolicy: Always securityContext: allowPrivilegeEscalation: false + seccompProfile: + type: RuntimeDefault capabilities: drop: - ALL diff --git a/config/samples/gatekeeper_operations.yaml b/config/samples/gatekeeper_operations.yaml new file mode 100644 index 000000000..f18dd9325 --- /dev/null +++ b/config/samples/gatekeeper_operations.yaml @@ -0,0 +1,12 @@ +apiVersion: operator.gatekeeper.sh/v1alpha1 +kind: Gatekeeper +metadata: + name: gatekeeper +spec: + # Add fields here + webhook: + operations: + - CREATE + - UPDATE + - DELETE + - CONNECT diff --git a/controllers/gatekeeper_controller.go b/controllers/gatekeeper_controller.go index 7752f85ef..40cf984c7 100644 --- a/controllers/gatekeeper_controller.go +++ b/controllers/gatekeeper_controller.go @@ -38,6 +38,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/predicate" + "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" @@ -620,6 +621,7 @@ func webhookOverrides(obj *unstructured.Unstructured, webhook *operatorv1alpha1. return nil } +// override common properties func webhookConfigurationOverrides( obj *unstructured.Unstructured, webhook *operatorv1alpha1.WebhookConfig, @@ -644,9 +646,15 @@ func webhookConfigurationOverrides( return err } } + + if err := setOperators(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 } @@ -963,6 +971,44 @@ func setNamespaceSelector( return setWebhookConfigurationWithFn(obj, webhookName, setNamespaceSelectorFn) } +func setOperators( + obj *unstructured.Unstructured, operations *[]v1alpha1.OperationType, webhookName string, +) error { + // If no operations is provided, no override for operations + if operations == nil { + return nil + } + + setOperatorsFn := func(webhook map[string]interface{}) error { + rules := webhook["rules"].([]interface{}) + if rules[0] == nil { + return nil + } + + converted := make([]interface{}, len(*operations)) + for i, op := range *operations { + converted[i] = string(op) + } + + firtRuleObj := rules[0].(map[string]interface{}) + newfirstRule := map[string]interface{}{ + "apiGroups": firtRuleObj["apiGroups"], + "apiVersions": firtRuleObj["apiVersions"], + "operations": converted, + "resources": firtRuleObj["resources"], + "scope": firtRuleObj["scope"], + } + + if err := unstructured.SetNestedSlice(webhook, []interface{}{newfirstRule}, "rules"); err != nil { + return errors.Wrapf(err, "Failed to set webhook namespace selector") + } + + return nil + } + + return setWebhookConfigurationWithFn(obj, webhookName, setOperatorsFn) +} + // Generic setters func setAffinity(obj *unstructured.Unstructured, spec operatorv1alpha1.GatekeeperSpec) error { diff --git a/deploy/gatekeeper-operator.yaml b/deploy/gatekeeper-operator.yaml index 20b63c5cd..89c5a5bd5 100644 --- a/deploy/gatekeeper-operator.yaml +++ b/deploy/gatekeeper-operator.yaml @@ -1705,6 +1705,8 @@ spec: capabilities: drop: - ALL + seccompProfile: + type: RuntimeDefault - args: - --health-probe-bind-address=:8081 - --metrics-bind-address=127.0.0.1:8080 @@ -1738,6 +1740,8 @@ spec: memory: 20Mi securityContext: allowPrivilegeEscalation: false + seccompProfile: + type: RuntimeDefault capabilities: drop: - ALL diff --git a/test/e2e/gatekeeper_controller_test.go b/test/e2e/gatekeeper_controller_test.go index 99ee63b52..191a175a8 100644 --- a/test/e2e/gatekeeper_controller_test.go +++ b/test/e2e/gatekeeper_controller_test.go @@ -24,6 +24,7 @@ import ( "os" "strings" + . "github.com/gatekeeper/gatekeeper-operator/test/e2e/util" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" admregv1 "k8s.io/api/admissionregistration/v1" @@ -36,7 +37,6 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/yaml" - "sigs.k8s.io/controller-runtime/pkg/client" "github.com/gatekeeper/gatekeeper-operator/api/v1alpha1" "github.com/gatekeeper/gatekeeper-operator/controllers" @@ -48,6 +48,7 @@ const ( // Gatekeeper name and namespace gkName = "gatekeeper" gatekeeperWithAllValuesFile = "gatekeeper_with_all_values.yaml" + gatekeeperWithOperations = "gatekeeper_operations.yaml" ) var ( @@ -93,39 +94,37 @@ var _ = Describe("Gatekeeper", Label("gatekeeper-controller"), func() { } }) - AfterEach(func() { - Expect(K8sClient.Delete(ctx, emptyGatekeeper(), client.PropagationPolicy(v1.DeletePropagationForeground))).Should(Succeed()) - - // Once this succeeds, clean up has happened for all owned resources. - Eventually(func() bool { - err := K8sClient.Get(ctx, gatekeeperName, &v1alpha1.Gatekeeper{}) - if err == nil { - return false - } - - return apierrors.IsNotFound(err) - }, deleteTimeout, pollInterval).Should(BeTrue()) - - Eventually(func() bool { - err := K8sClient.Get(ctx, auditName, &appsv1.Deployment{}) - if err == nil { - return false - } - - return apierrors.IsNotFound(err) - }, deleteTimeout, pollInterval).Should(BeTrue()) - - Eventually(func() bool { - err := K8sClient.Get(ctx, controllerManagerName, &appsv1.Deployment{}) - if err == nil { - return false - } - - return apierrors.IsNotFound(err) - }, deleteTimeout, pollInterval).Should(BeTrue()) - }) - Describe("Overriding CR", Ordered, func() { + AfterEach(func() { + Kubectl("delete", "gatekeeper", "gatekeeper", "--ignore-not-found") + // Once this succeeds, clean up has happened for all owned resources. + Eventually(func() bool { + err := K8sClient.Get(ctx, gatekeeperName, &v1alpha1.Gatekeeper{}) + if err == nil { + return false + } + + return apierrors.IsNotFound(err) + }, deleteTimeout, pollInterval).Should(BeTrue()) + + Eventually(func() bool { + err := K8sClient.Get(ctx, auditName, &appsv1.Deployment{}) + if err == nil { + return false + } + + return apierrors.IsNotFound(err) + }, deleteTimeout, pollInterval).Should(BeTrue()) + + Eventually(func() bool { + err := K8sClient.Get(ctx, controllerManagerName, &appsv1.Deployment{}) + if err == nil { + return false + } + + return apierrors.IsNotFound(err) + }, deleteTimeout, pollInterval).Should(BeTrue()) + }) It("Creating an empty gatekeeper contains default values", func() { gatekeeper := emptyGatekeeper() err := loadGatekeeperFromFile(gatekeeper, "gatekeeper_empty.yaml") @@ -498,6 +497,55 @@ var _ = Describe("Gatekeeper", Label("gatekeeper-controller"), func() { auditDeployment, webhookDeployment = gatekeeperDeployments() byCheckingMutationDisabled(auditDeployment, webhookDeployment) }) + + It("Override Webhook operations with Create, Update, Delete, Connect", func() { + gatekeeper := &v1alpha1.Gatekeeper{} + gatekeeper.Namespace = gatekeeperNamespace + err := loadGatekeeperFromFile(gatekeeper, gatekeeperWithOperations) + Expect(err).ToNot(HaveOccurred()) + 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()) + }) }) }) @@ -509,7 +557,7 @@ func gatekeeperAuditDeployment() (auditDeployment *appsv1.Deployment) { auditDeployment = &appsv1.Deployment{} Eventually(func() error { return K8sClient.Get(ctx, auditName, auditDeployment) - }, timeout*2, pollInterval).ShouldNot(HaveOccurred()) + }, timeout, pollInterval).ShouldNot(HaveOccurred()) return } @@ -517,7 +565,7 @@ func gatekeeperWebhookDeployment() (webhookDeployment *appsv1.Deployment) { webhookDeployment = &appsv1.Deployment{} Eventually(func() error { return K8sClient.Get(ctx, controllerManagerName, webhookDeployment) - }, timeout*2, pollInterval).ShouldNot(HaveOccurred()) + }, timeout, pollInterval).ShouldNot(HaveOccurred()) return }