diff --git a/.github/workflows/kind.yml b/.github/workflows/kind.yml index 950a4b5a..42e6d305 100644 --- a/.github/workflows/kind.yml +++ b/.github/workflows/kind.yml @@ -81,8 +81,13 @@ jobs: - name: Verify Deployment Configuration run: | + make webhook make build-images make kind-deploy-controller-dev + + - name: E2E Tests for Webhook + run: | + make e2e-test-webhook - name: Debug if: ${{ failure() }} @@ -93,3 +98,4 @@ jobs: if: ${{ always() }} run: | make kind-delete-cluster + \ No newline at end of file diff --git a/Makefile b/Makefile index 5436b813..7cabd8a6 100644 --- a/Makefile +++ b/Makefile @@ -217,11 +217,21 @@ kustomize: ## Download kustomize locally if necessary. GINKGO = $(LOCAL_BIN)/ginkgo .PHONY: kind-bootstrap-cluster -kind-bootstrap-cluster: kind-create-cluster install-crds kind-deploy-controller install-resources +kind-bootstrap-cluster: kind-create-cluster install-crds webhook kind-deploy-controller install-resources .PHONY: kind-bootstrap-cluster-dev kind-bootstrap-cluster-dev: kind-create-cluster install-crds install-resources +webhook: + -kubectl create ns $(KIND_NAMESPACE) + @echo installing cert-manager + kubectl apply -f https://github.com/cert-manager/cert-manager/releases/download/v1.12.0/cert-manager.yaml + @echo "wait until pods are up" + kubectl wait deployment -n cert-manager cert-manager --for condition=Available=True --timeout=90s + kubectl wait --for=condition=Ready pod -l app.kubernetes.io/instance=cert-manager -n cert-manager --timeout=30s + sed 's/namespace: open-cluster-management/namespace: $(KIND_NAMESPACE)/g' deploy/webhook.yaml | kubectl apply -f- + + .PHONY: kind-deploy-controller kind-deploy-controller: manifests @echo installing $(IMG) @@ -281,7 +291,7 @@ e2e-dependencies: .PHONY: e2e-test e2e-test: e2e-dependencies - $(GINKGO) -v --fail-fast $(E2E_TEST_ARGS) test/e2e + $(GINKGO) -v --fail-fast $(E2E_TEST_ARGS) --label-filter="!webhook" test/e2e .PHONY: e2e-test-coverage e2e-test-coverage: E2E_TEST_ARGS = --json-report=report_e2e.json --output-dir=. @@ -299,6 +309,9 @@ e2e-run-instrumented: e2e-build-instrumented e2e-stop-instrumented: ps -ef | grep '$(IMG)' | grep -v grep | awk '{print $$2}' | xargs kill +e2e-test-webhook: + $(GINKGO) -v --fail-fast --label-filter="webhook" test/e2e + .PHONY: e2e-debug e2e-debug: @echo local controller log: diff --git a/PROJECT b/PROJECT index 10850299..949a0e63 100644 --- a/PROJECT +++ b/PROJECT @@ -15,6 +15,9 @@ resources: group: policy kind: Policy path: open-cluster-management.io/governance-policy-propagator/api/v1 + webhooks: + validation: true + webhookVersion: v1 version: v1 - api: crdVersion: v1 diff --git a/README.md b/README.md index 0344ce7a..51c17053 100644 --- a/README.md +++ b/README.md @@ -62,6 +62,15 @@ make e2e-dependencies make e2e-test ``` +### How to run webhook locally + +```bash +--enable-webhooks=true +``` + +> **Limit**: If you want to run the webhook locally, you need to generate certificates and place them in `/tmp/k8s-webhook-server/serving-certs/tls.{crt,key}`. If you’re not running a local API server, you’ll also need to figure out how to proxy traffic from the remote cluster to your local webhook server. For this reason, Kubebuilder generally recommends disabling webhooks when doing your local code-run-test cycle. To disable it, please supply the `--enable-webhooks=false` argument when running the controller. +> For more information, visit https://book.kubebuilder.io/cronjob-tutorial/running.html + ### Clean up ``` @@ -92,6 +101,3 @@ The following environment variables can be set to configure the controller: - The `governance-policy-propagator` is part of the `open-cluster-management` community. For more information, visit: [open-cluster-management.io](https://open-cluster-management.io). - diff --git a/api/v1/policy_webhook.go b/api/v1/policy_webhook.go new file mode 100644 index 00000000..1926d49f --- /dev/null +++ b/api/v1/policy_webhook.go @@ -0,0 +1,66 @@ +// Copyright (c) 2021 Red Hat, Inc. +// Copyright Contributors to the Open Cluster Management project + +package v1 + +import ( + "errors" + "unicode/utf8" + + "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +var ( + // log is for logging in this package. + policylog = logf.Log.WithName("policy-validating-webhook") + errName = errors.New("the combined length of the policy namespace and name " + + ". cannot exceed 63 characters") +) + +func (r *Policy) SetupWebhookWithManager(mgr ctrl.Manager) error { + return ctrl.NewWebhookManagedBy(mgr). + For(r). + Complete() +} + +//+kubebuilder:webhook:path=/validate-policy-open-cluster-management-io-v1-policy,mutating=false,failurePolicy=Ignore,sideEffects=None,groups=policy.open-cluster-management.io,resources=policies,verbs=create,versions=v1,name=policy.open-cluster-management.io.webhook,admissionReviewVersions=v1 + +var _ webhook.Validator = &Policy{} + +// ValidateCreate implements webhook.Validator so a webhook will be registered for the type +func (r *Policy) ValidateCreate() (admission.Warnings, error) { + policylog.Info("Validate policy creation request", "name", r.Name) + + return r.validateName() +} + +// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type +func (r *Policy) ValidateUpdate(_ runtime.Object) (admission.Warnings, error) { + return nil, nil +} + +// ValidateDelete implements webhook.Validator so a webhook will be registered for the type +func (r *Policy) ValidateDelete() (admission.Warnings, error) { + return nil, nil +} + +// validate the policy name and namespace length +func (r *Policy) validateName() (admission.Warnings, error) { + policylog.Info("Validating the policy name through a validating webhook") + + // replicated policies don't need pass this validation + if _, ok := r.GetLabels()["policy.open-cluster-management.io/root-policy"]; ok { + return nil, nil + } + + // 1 character for "." + if (utf8.RuneCountInString(r.Name) + utf8.RuneCountInString(r.Namespace)) > 62 { + return nil, errName + } + + return nil, nil +} diff --git a/deploy/manager/manager.yaml b/deploy/manager/manager.yaml index 2e259ca2..7e615fd1 100644 --- a/deploy/manager/manager.yaml +++ b/deploy/manager/manager.yaml @@ -2,16 +2,22 @@ apiVersion: apps/v1 kind: Deployment metadata: + labels: + webhook-origin: governance-policy-propagator name: governance-policy-propagator spec: replicas: 1 selector: matchLabels: name: governance-policy-propagator + webhook-origin: governance-policy-propagator template: metadata: + annotations: + kubectl.kubernetes.io/default-container: governance-policy-propagator labels: name: governance-policy-propagator + webhook-origin: governance-policy-propagator spec: serviceAccountName: governance-policy-propagator containers: @@ -27,7 +33,14 @@ spec: - containerPort: 8383 protocol: TCP name: http + - containerPort: 9443 + protocol: TCP + name: webhook-http imagePullPolicy: Always + volumeMounts: + - mountPath: /tmp/k8s-webhook-server/serving-certs + name: cert + readOnly: true env: - name: WATCH_NAMESPACE value: "" @@ -37,6 +50,11 @@ spec: fieldPath: metadata.name - name: OPERATOR_NAME value: "governance-policy-propagator" + volumes: + - name: cert + secret: + defaultMode: 420 + secretName: propagator-webhook-server-cert --- kind: ClusterRoleBinding apiVersion: rbac.authorization.k8s.io/v1 diff --git a/deploy/operator.yaml b/deploy/operator.yaml index dccc1cdb..cb3a0d63 100644 --- a/deploy/operator.yaml +++ b/deploy/operator.yaml @@ -231,16 +231,22 @@ subjects: apiVersion: apps/v1 kind: Deployment metadata: + labels: + webhook-origin: governance-policy-propagator name: governance-policy-propagator spec: replicas: 1 selector: matchLabels: name: governance-policy-propagator + webhook-origin: governance-policy-propagator template: metadata: + annotations: + kubectl.kubernetes.io/default-container: governance-policy-propagator labels: name: governance-policy-propagator + webhook-origin: governance-policy-propagator spec: containers: - args: @@ -265,4 +271,16 @@ spec: - containerPort: 8383 name: http protocol: TCP + - containerPort: 9443 + name: webhook-http + protocol: TCP + volumeMounts: + - mountPath: /tmp/k8s-webhook-server/serving-certs + name: cert + readOnly: true serviceAccountName: governance-policy-propagator + volumes: + - name: cert + secret: + defaultMode: 420 + secretName: propagator-webhook-server-cert diff --git a/deploy/webhook.yaml b/deploy/webhook.yaml new file mode 100644 index 00000000..02065f96 --- /dev/null +++ b/deploy/webhook.yaml @@ -0,0 +1,61 @@ +apiVersion: v1 +kind: Service +metadata: + name: propagator-webhook-service + namespace: open-cluster-management +spec: + ports: + - port: 443 + protocol: TCP + targetPort: 9443 + selector: + webhook-origin: governance-policy-propagator +--- +apiVersion: cert-manager.io/v1 +kind: Certificate +metadata: + name: propagator-webhook-serving-cert + namespace: open-cluster-management +spec: + dnsNames: + - propagator-webhook-service.open-cluster-management.svc + - propagator-webhook-service.open-cluster-management.svc.cluster.local + issuerRef: + kind: Issuer + name: propagator-webhook-selfsigned-issuer + secretName: propagator-webhook-server-cert +--- +apiVersion: cert-manager.io/v1 +kind: Issuer +metadata: + name: propagator-webhook-selfsigned-issuer + namespace: open-cluster-management +spec: + selfSigned: {} +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingWebhookConfiguration +metadata: + annotations: + cert-manager.io/inject-ca-from: open-cluster-management/propagator-webhook-serving-cert + name: propagator-webhook-validating-configuration +webhooks: +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: propagator-webhook-service + namespace: open-cluster-management + path: /validate-policy-open-cluster-management-io-v1-policy + failurePolicy: Ignore + name: policy.open-cluster-management.io.webhook + rules: + - apiGroups: + - policy.open-cluster-management.io + apiVersions: + - v1 + operations: + - CREATE + resources: + - policies + sideEffects: None diff --git a/main.go b/main.go index a3c8eedb..e0d187c6 100644 --- a/main.go +++ b/main.go @@ -90,12 +90,15 @@ func main() { var enableLeaderElection bool var probeAddr string var keyRotationDays, keyRotationMaxConcurrency, policyMetricsMaxConcurrency, policyStatusMaxConcurrency uint + var enableWebhooks bool pflag.StringVar(&metricsAddr, "metrics-bind-address", ":8383", "The address the metric endpoint binds to.") pflag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") pflag.BoolVar(&enableLeaderElection, "leader-elect", true, "Enable leader election for controller manager. "+ "Enabling this will ensure there is only one active controller manager.") + pflag.BoolVar(&enableWebhooks, "enable-webhooks", true, + "Enable the policy validating webhook") pflag.UintVar( &keyRotationDays, "encryption-key-rotation", @@ -301,6 +304,13 @@ func main() { os.Exit(1) } + if enableWebhooks { + if err = (&policyv1.Policy{}).SetupWebhookWithManager(mgr); err != nil { + log.Error(err, "unable to create webhook", "webhook", "Policy") + os.Exit(1) + } + } + //+kubebuilder:scaffold:builder if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { diff --git a/main_test.go b/main_test.go index 081497c8..64a0d2d4 100644 --- a/main_test.go +++ b/main_test.go @@ -14,7 +14,10 @@ import ( // TestRunMain wraps the main() function in order to build a test binary and collection coverage for // E2E/Integration tests. Controller CLI flags are also passed in here. func TestRunMain(t *testing.T) { - os.Args = append(os.Args, "--leader-elect=false") + os.Args = append(os.Args, + "--leader-elect=false", + "--enable-webhooks=false", + ) main() } diff --git a/test/e2e/case17_policy_webhook_test.go b/test/e2e/case17_policy_webhook_test.go new file mode 100644 index 00000000..c69ec5b7 --- /dev/null +++ b/test/e2e/case17_policy_webhook_test.go @@ -0,0 +1,85 @@ +// Copyright (c) 2020 Red Hat, Inc. +// Copyright Contributors to the Open Cluster Management project + +package e2e + +import ( + "context" + "unicode/utf8" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "open-cluster-management.io/governance-policy-propagator/test/utils" +) + +const ( + case17PolicyLongYaml string = "../resources/case17_policy_webhook/case17_policy_long.yaml" + case17PolicyReplicatedYaml string = "../resources/case17_policy_webhook/" + + "case17_policy_replicate.yaml" + longNamesapce string = "long-long-long-long-long-long-long" + case17PolicyReplicatedName string = "case17-test-policy-replicated-longlong" + errMsg string = `admission webhook "policy.open-cluster-management.io.webhook" denied the ` + + `request: the combined length of the policy namespace and name . ` + + `cannot exceed 63 characters` +) + +var _ = Describe("Test policy webhook", Label("webhook"), Ordered, func() { + BeforeAll(func() { + _, err := utils.KubectlWithOutput("create", + "ns", longNamesapce, + ) + Expect(err).ShouldNot(HaveOccurred()) + }) + AfterAll(func() { + // cleanup + _, err := utils.KubectlWithOutput("delete", + "ns", longNamesapce, + "--ignore-not-found", + ) + Expect(err).ShouldNot(HaveOccurred()) + + _, err = utils.KubectlWithOutput("delete", + "policy", case17PolicyReplicatedName, + "-n", testNamespace, + "--ignore-not-found", + ) + Expect(err).ShouldNot(HaveOccurred()) + }) + Describe("Test name + namespace over 63", func() { + It("Should the error message is presented", func() { + output, err := utils.KubectlWithOutput("apply", + "-f", case17PolicyLongYaml, + "-n", longNamesapce) + Expect(err).Should(HaveOccurred()) + Expect(output).Should(ContainSubstring(errMsg)) + }) + It("Should replicated policy should not be validated", func() { + _, err := utils.KubectlWithOutput("apply", + "-f", case17PolicyReplicatedYaml, + "-n", testNamespace) + Expect(err).ShouldNot(HaveOccurred()) + plr := utils.GetWithTimeout( + clientHubDynamic, gvrPlacementRule, case17PolicyReplicatedName+"-plr", + testNamespace, true, defaultTimeoutSeconds, + ) + plr.Object["status"] = utils.GeneratePlrStatus("managed1") + _, err = clientHubDynamic.Resource(gvrPlacementRule).Namespace(testNamespace).UpdateStatus( + context.TODO(), plr, metav1.UpdateOptions{}, + ) + Expect(err).ToNot(HaveOccurred()) + + By("Replicated policy name and cluster namespace should be over 63 character") + Expect(utf8.RuneCountInString("managed1." + testNamespace + + "." + case17PolicyReplicatedName)).Should(BeNumerically(">", 63)) + + By("Replicated policy should be created") + plc := utils.GetWithTimeout( + clientHubDynamic, gvrPolicy, testNamespace+"."+case17PolicyReplicatedName, + "managed1", true, defaultTimeoutSeconds, + ) + Expect(plc).ToNot(BeNil()) + }) + }) +}) diff --git a/test/resources/case17_policy_webhook/case17_policy_long.yaml b/test/resources/case17_policy_webhook/case17_policy_long.yaml new file mode 100644 index 00000000..8a909cd3 --- /dev/null +++ b/test/resources/case17_policy_webhook/case17_policy_long.yaml @@ -0,0 +1,45 @@ +apiVersion: policy.open-cluster-management.io/v1 +kind: Policy +metadata: + name: case17-test-policy-long-long-long +spec: + remediationAction: inform + disabled: false + policy-templates: + - objectDefinition: + apiVersion: policies.ibm.com/v1alpha1 + kind: TrustedContainerPolicy + metadata: + name: case17-test-policy-trustedcontainerpolicy + spec: + severity: low + namespaceSelector: + include: ["default"] + exclude: ["kube-system"] + remediationAction: inform + imageRegistry: quay.io +--- +apiVersion: policy.open-cluster-management.io/v1 +kind: PlacementBinding +metadata: + name: case17-test-policy-pb +placementRef: + apiGroup: apps.open-cluster-management.io + kind: PlacementRule + name: case17-test-policy-plr +subjects: +- apiGroup: policy.open-cluster-management.io + kind: Policy + name: case17-test-policy +--- +apiVersion: apps.open-cluster-management.io/v1 +kind: PlacementRule +metadata: + name: case17-test-policy-plr +spec: + clusterConditions: + - status: "True" + type: ManagedClusterConditionAvailable + clusterSelector: + matchExpressions: + [] diff --git a/test/resources/case17_policy_webhook/case17_policy_replicate.yaml b/test/resources/case17_policy_webhook/case17_policy_replicate.yaml new file mode 100644 index 00000000..0de6134d --- /dev/null +++ b/test/resources/case17_policy_webhook/case17_policy_replicate.yaml @@ -0,0 +1,45 @@ +apiVersion: policy.open-cluster-management.io/v1 +kind: Policy +metadata: + name: case17-test-policy-replicated-longlong +spec: + remediationAction: inform + disabled: false + policy-templates: + - objectDefinition: + apiVersion: policies.ibm.com/v1alpha1 + kind: TrustedContainerPolicy + metadata: + name: case17-test-policy-replicated-trustedcontainerpolicy + spec: + severity: low + namespaceSelector: + include: ["default"] + exclude: ["kube-system"] + remediationAction: inform + imageRegistry: quay.io +--- +apiVersion: policy.open-cluster-management.io/v1 +kind: PlacementBinding +metadata: + name: case17-test-policy-replicated-longlong-pb +placementRef: + apiGroup: apps.open-cluster-management.io + kind: PlacementRule + name: case17-test-policy-replicated-longlong-plr +subjects: +- apiGroup: policy.open-cluster-management.io + kind: Policy + name: case17-test-policy-replicated-longlong +--- +apiVersion: apps.open-cluster-management.io/v1 +kind: PlacementRule +metadata: + name: case17-test-policy-replicated-longlong-plr +spec: + clusterConditions: + - status: "True" + type: ManagedClusterConditionAvailable + clusterSelector: + matchExpressions: + []