Repository: kurator-dev/kurator
Pull request #366 (2023-08-02 09:14:18)

cluster operator: add customization for deployment
**What type of PR is this?**


/kind feature

ref [#359 ](https://github.com/kurator-dev/kurator/pull/359#discussion_r1271690404)

**What this PR does / why we need it**:

**Which issue(s) this PR fixes**:
Fixes #

**Special notes for your reviewer**:

**Does this PR introduce a user-facing change?**:
<!--
If no, just write "NONE" in the release-note block below.
If yes, a release note is required.
-->
```release-note
cluster operator: add customization for deployment
```



kurator-bot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by:
**Once this PR has been reviewed and has the lgtm label**, please assign zirain for approval. For more information see [the Kubernetes Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kurator-dev%2Fkurator).

<details open>
Needs approval from an approver in each of these files:

- **[manifests/OWNERS](https://github.com/kurator-dev/kurator/blob/main/manifests/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["zirain"]} -->
netlify[bot]: ### <span aria-hidden="true">👷</span> Deploy Preview for *kurator-dev* processing.


|  Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 120ed35d8d5c5e4b077a38b31016207c643b4f21 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/sites/kurator-dev/deploys/64ca1e6cf60ac900080509b6 |
kurator-bot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kurator-dev/kurator/pull/366#pullrequestreview-1559022069" title="Approved">zirain</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kurator-dev%2Fkurator).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[manifests/OWNERS](https://github.com/kurator-dev/kurator/blob/main/manifests/OWNERS)~~ [zirain]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/cmd/fleet-manager/application/application.go b/cmd/fleet-manager/application/application.go
index 728a25f..10cde37 100644
--- a/cmd/fleet-manager/application/application.go
+++ b/cmd/fleet-manager/application/application.go
@@ -24,6 +24,7 @@ import (
 
 	"kurator.dev/kurator/cmd/fleet-manager/options"
 	"kurator.dev/kurator/pkg/fleet-manager"
+	"kurator.dev/kurator/pkg/webhooks"
 )
 
 var log = ctrl.Log.WithName("application")
@@ -37,5 +38,12 @@ func InitControllers(ctx context.Context, opts *options.Options, mgr ctrl.Manage
 		return err
 	}
 
+	if err := (&webhooks.ApplicationWebhook{
+		Client: mgr.GetClient(),
+	}).SetupWebhookWithManager(mgr); err != nil {
+		log.Error(err, "unable to create Application webhook", "Webhook", "Application")
+		return err
+	}
+
 	return nil
 }
diff --git a/cmd/fleet-manager/main.go b/cmd/fleet-manager/main.go
index 80db18e..082ccdc 100644
--- a/cmd/fleet-manager/main.go
+++ b/cmd/fleet-manager/main.go
@@ -118,6 +118,8 @@ func run(ctx context.Context, opts *options.Options) error {
 		LeaderElectionResourceLock: resourcelock.LeasesResourceLock,
 		LeaderElectionID:           "kurator-fleet-manager-leader-elect",
 		LeaderElectionNamespace:    opts.LeaderElectionNamespace,
+		Port:                       opts.WebhookPort,
+		CertDir:                    opts.WebhookCertDir,
 		EventBroadcaster:           broadcaster,
 	})
 	if err != nil {
diff --git a/cmd/fleet-manager/options/options.go b/cmd/fleet-manager/options/options.go
index 3ed7e96..c3671ef 100644
--- a/cmd/fleet-manager/options/options.go
+++ b/cmd/fleet-manager/options/options.go
@@ -27,6 +27,8 @@ type Options struct {
 	LeaderElectionNamespace string
 	ProfilerAddress         string
 	Concurrency             int
+	WebhookPort             int
+	WebhookCertDir          string
 }
 
 func (opt *Options) AddFlags(fs *pflag.FlagSet) {
@@ -71,4 +73,17 @@ func (opt *Options) AddFlags(fs *pflag.FlagSet) {
 		"",
 		"Path to the directory containing the Fleet manifests, built-in manifests will be used if not specified",
 	)
+
+	fs.IntVar(
+		&opt.WebhookPort,
+		"webhook-port",
+		9443,
+		"Webhook Server port.",
+	)
+
+	fs.StringVar(
+		&opt.WebhookCertDir,
+		"webhook-cert-dir",
+		"/tmp/k8s-webhook-server/serving-certs/",
+		"Webhook cert dir, only used when webhook-port is specified.")
 }
diff --git a/manifests/charts/cluster-operator/templates/certmanager.yaml b/manifests/charts/cluster-operator/templates/certmanager.yaml
index 4b23eef..6c5aa91 100644
--- a/manifests/charts/cluster-operator/templates/certmanager.yaml
+++ b/manifests/charts/cluster-operator/templates/certmanager.yaml
@@ -7,6 +7,7 @@ metadata:
 spec:
   dnsNames:
   - kurator-webhook-service.{{ .Release.Namespace }}.svc
+  - kurator-webhook-service-fleet.{{ .Release.Namespace }}.svc
   - kurator-webhook-service.{{ .Release.Namespace }}.svc.cluster.local
   issuerRef:
     kind: Issuer
diff --git a/manifests/charts/cluster-operator/templates/deployment.yaml b/manifests/charts/cluster-operator/templates/deployment.yaml
index fdef9dc..ca1ccad 100644
--- a/manifests/charts/cluster-operator/templates/deployment.yaml
+++ b/manifests/charts/cluster-operator/templates/deployment.yaml
@@ -22,6 +22,8 @@ spec:
         - --v={{ .Values.logging.level }}
         - --metrics-bind-addr=127.0.0.1:8080
         - --requeue-after={{ .Values.requeueAfter }}
+        - --webhook-port={{ .Values.webhook.port }}
+        - --webhook-cert-dir={{ .Values.webhook.certMountPath }}
         env:
         - name: AWS_SHARED_CREDENTIALS_FILE
           value: /home/.aws/credentials
@@ -35,7 +37,7 @@ spec:
           periodSeconds: 10
         name: manager
         ports:
-        - containerPort: 9443
+        - containerPort: {{ .Values.webhook.port }}
           name: webhook-server
           protocol: TCP
         - containerPort: 9440
@@ -46,7 +48,7 @@ spec:
             path: /readyz
             port: healthz
         volumeMounts:
-        - mountPath: /tmp/k8s-webhook-server/serving-certs
+        - mountPath: {{ .Values.webhook.certMountPath }}
           name: cert
           readOnly: true
         - mountPath: /home/.aws
diff --git a/manifests/charts/cluster-operator/values.yaml b/manifests/charts/cluster-operator/values.yaml
index d9abc0e..4b1d3ae 100644
--- a/manifests/charts/cluster-operator/values.yaml
+++ b/manifests/charts/cluster-operator/values.yaml
@@ -21,3 +21,7 @@ feautreGates:
 aws:
   # AWS_BASE64ENCODED_CREDENTIALS
   credentials: "Cg==" # zero credential
+
+webhook:
+  port: 9443
+  certMountPath: /tmp/k8s-webhook-server/serving-certs
diff --git a/manifests/charts/fleet-manager/templates/deployment.yaml b/manifests/charts/fleet-manager/templates/deployment.yaml
index 3b4b6fd..59703a2 100644
--- a/manifests/charts/fleet-manager/templates/deployment.yaml
+++ b/manifests/charts/fleet-manager/templates/deployment.yaml
@@ -19,9 +19,19 @@ spec:
       - args:
         - --leader-elect
         - --v={{ .Values.logging.level }}
+        - --webhook-port={{ .Values.webhook.port }}
+        - --webhook-cert-dir={{ .Values.webhook.certMountPath }}
         image: {{ .Values.image.hub }}/fleet-manager:{{ .Values.image.tag }}
         imagePullPolicy: {{ .Values.image.pullPolicy }}
         name: manager
+        ports:
+          - containerPort: {{ .Values.webhook.port }}
+            name: webhook-server
+            protocol: TCP
+        volumeMounts:
+          - mountPath: {{ .Values.webhook.certMountPath }}
+            name: cert
+            readOnly: true
       serviceAccountName: kurator-fleet-manager
       terminationGracePeriodSeconds: 10
       tolerations:
@@ -29,3 +39,8 @@ spec:
         key: node-role.kubernetes.io/master
       - effect: NoSchedule
         key: node-role.kubernetes.io/control-plane
+      volumes:
+        - name: cert
+          secret:
+            defaultMode: 420
+            secretName: kurator-webhook-service-cert
diff --git a/manifests/charts/fleet-manager/templates/service.yaml b/manifests/charts/fleet-manager/templates/service.yaml
new file mode 100644
index 0000000..7187cc3
--- /dev/null
+++ b/manifests/charts/fleet-manager/templates/service.yaml
@@ -0,0 +1,11 @@
+apiVersion: v1
+kind: Service
+metadata:
+  name: kurator-webhook-service-fleet
+  namespace: {{ .Release.Namespace }}
+spec:
+  ports:
+    - port: 443
+      targetPort: webhook-server
+  selector:
+    app.kubernetes.io/name: kurator-fleet-manager
diff --git a/manifests/charts/fleet-manager/templates/webhooks.yaml b/manifests/charts/fleet-manager/templates/webhooks.yaml
new file mode 100644
index 0000000..b2c4fc7
--- /dev/null
+++ b/manifests/charts/fleet-manager/templates/webhooks.yaml
@@ -0,0 +1,30 @@
+apiVersion: admissionregistration.k8s.io/v1
+kind: ValidatingWebhookConfiguration
+metadata:
+  annotations:
+    cert-manager.io/inject-ca-from: {{ .Release.Namespace }}/kurator-serving-cert
+  creationTimestamp: null
+  name: fleet-manager-validating-webhook-configuration
+webhooks:
+  - admissionReviewVersions:
+      - v1
+      - v1beta1
+    clientConfig:
+      service:
+        name: kurator-webhook-service-fleet
+        namespace: {{ .Release.Namespace }}
+        path: /validate-apps-kurator-dev-v1alpha1-application # do not change this
+    failurePolicy: Fail
+    matchPolicy: Equivalent
+    name: validation.application.apps.kurator.dev
+    rules:
+      - apiGroups:
+          - apps.kurator.dev
+        apiVersions:
+          - v1alpha1
+        operations:
+          - CREATE
+          - UPDATE
+        resources:
+          - applications
+    sideEffects: None
diff --git a/manifests/charts/fleet-manager/values.yaml b/manifests/charts/fleet-manager/values.yaml
index 0d347b6..e91a688 100644
--- a/manifests/charts/fleet-manager/values.yaml
+++ b/manifests/charts/fleet-manager/values.yaml
@@ -7,3 +7,7 @@ image:
 
 logging:
   level: 0
+
+webhook:
+  port: 9443
+  certMountPath: /tmp/k8s-webhook-server/serving-certs
diff --git a/pkg/webhooks/application_webhook.go b/pkg/webhooks/application_webhook.go
new file mode 100644
index 0000000..96c60d3
--- /dev/null
+++ b/pkg/webhooks/application_webhook.go
@@ -0,0 +1,128 @@
+/*
+Copyright Kurator Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package webhooks
+
+import (
+	"context"
+	"fmt"
+
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/validation/field"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/webhook"
+
+	"kurator.dev/kurator/pkg/apis/apps/v1alpha1"
+)
+
+var _ webhook.CustomValidator = &ApplicationWebhook{}
+
+type ApplicationWebhook struct {
+	Client client.Reader
+}
+
+func (wh *ApplicationWebhook) SetupWebhookWithManager(mgr ctrl.Manager) error {
+	return ctrl.NewWebhookManagedBy(mgr).
+		For(&v1alpha1.Application{}).
+		WithValidator(wh).
+		Complete()
+}
+
+func (wh *ApplicationWebhook) ValidateCreate(_ context.Context, obj runtime.Object) error {
+	in, ok := obj.(*v1alpha1.Application)
+	if !ok {
+		return apierrors.NewBadRequest(fmt.Sprintf("expected a Application but got a %T", obj))
+	}
+
+	return wh.validate(in)
+}
+
+func (wh *ApplicationWebhook) validate(in *v1alpha1.Application) error {
+	var allErrs field.ErrorList
+
+	allErrs = append(allErrs, validateFleet(in)...)
+
+	if len(allErrs) > 0 {
+		return apierrors.NewInvalid(v1alpha1.SchemeGroupVersion.WithKind("Application").GroupKind(), in.Name, allErrs)
+	}
+
+	return nil
+}
+
+// validateFleet validates the fleet in the application with the following rules:
+// 1 if defaultFleet is set, make sure all policy fleet(if set) is same as the defaultFleet
+// 2 if defaultFleet is not set, every individual policies must be set and must be same as the first policy fleet
+func validateFleet(in *v1alpha1.Application) field.ErrorList {
+	var allErrs field.ErrorList
+
+	defaultFleet := ""
+	if in.Spec.Destination != nil {
+		defaultFleet = in.Spec.Destination.Fleet
+	}
+
+	// if defaultFleet is set, make sure all policy fleet(if set) is same as the defaultFleet
+	if defaultFleet != "" {
+		for i, policy := range in.Spec.SyncPolicies {
+			if policy.Destination != nil && policy.Destination.Fleet != "" && defaultFleet != policy.Destination.Fleet {
+				allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "syncPolicies").Index(i).Child("destination", "fleet"), policy.Destination.Fleet, fmt.Sprintf("must be same as application.spec.destination.fleet:%v, because fleet must be consistent throughout the application", defaultFleet)))
+			}
+		}
+	}
+
+	// if defaultFleet is not set, every individual policies must be set and must be same as the first policy fleet
+	if defaultFleet == "" {
+		var (
+			firstPolicyFleet string
+			isFirst          = true
+		)
+		for i, policy := range in.Spec.SyncPolicies {
+			// if individual policy fleet is not set, return err
+			if policy.Destination == nil || policy.Destination.Fleet == "" {
+				allErrs = append(allErrs, field.Required(field.NewPath("spec", "syncPolicies").Index(i).Child("destination", "fleet"), "must be set when application.spec.destination.fleet is not set"))
+				return allErrs
+			}
+			if isFirst {
+				firstPolicyFleet = policy.Destination.Fleet
+				isFirst = false
+			}
+			if !isFirst && firstPolicyFleet != policy.Destination.Fleet {
+				allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "syncPolicies").Index(i).Child("destination", "fleet"), policy.Destination.Fleet, fmt.Sprintf("must be same as firstPolicyFleet:%v, because fleet must be consistent throughout the application", firstPolicyFleet)))
+			}
+		}
+	}
+
+	return allErrs
+}
+
+func (wh *ApplicationWebhook) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) error {
+	_, ok := oldObj.(*v1alpha1.Application)
+	if !ok {
+		return apierrors.NewBadRequest(fmt.Sprintf("expected a Application but got a %T", oldObj))
+	}
+
+	newApplication, ok := newObj.(*v1alpha1.Application)
+	if !ok {
+		return apierrors.NewBadRequest(fmt.Sprintf("expected a Application but got a %T", newObj))
+	}
+
+	return wh.validate(newApplication)
+}
+
+func (wh *ApplicationWebhook) ValidateDelete(_ context.Context, obj runtime.Object) error {
+	return nil
+}


Test patch:
diff --git a/pkg/webhooks/application_webhook_test.go b/pkg/webhooks/application_webhook_test.go
new file mode 100644
index 0000000..eed30eb
--- /dev/null
+++ b/pkg/webhooks/application_webhook_test.go
@@ -0,0 +1,101 @@
+/*
+Copyright Kurator Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package webhooks
+
+import (
+	"io/fs"
+	"os"
+	"path"
+	"path/filepath"
+	"testing"
+
+	. "github.com/onsi/gomega"
+	"github.com/stretchr/testify/assert"
+	"sigs.k8s.io/yaml"
+
+	"kurator.dev/kurator/pkg/apis/apps/v1alpha1"
+)
+
+func TestValidApplicationValidation(t *testing.T) {
+	// read configuration from examples directory to test valid application configuration
+	r := path.Join("../../examples", "application")
+	caseNames := make([]string, 0)
+	err := filepath.WalkDir(r, func(path string, d fs.DirEntry, err error) error {
+		if d.IsDir() {
+			return nil
+		}
+
+		caseNames = append(caseNames, path)
+
+		return nil
+	})
+	assert.NoError(t, err)
+
+	wh := &ApplicationWebhook{}
+	for _, tt := range caseNames {
+		t.Run(tt, func(t *testing.T) {
+			g := NewWithT(t)
+			c, err := readApplication(tt)
+			g.Expect(err).NotTo(HaveOccurred())
+
+			err = wh.validate(c)
+			g.Expect(err).NotTo(HaveOccurred())
+		})
+	}
+}
+
+func TestInvalidApplicationValidation(t *testing.T) {
+	r := path.Join("testdata", "application")
+	caseNames := make([]string, 0)
+	err := filepath.WalkDir(r, func(path string, d fs.DirEntry, err error) error {
+		if d.IsDir() {
+			return nil
+		}
+
+		caseNames = append(caseNames, path)
+
+		return nil
+	})
+	assert.NoError(t, err)
+
+	wh := &ApplicationWebhook{}
+	for _, tt := range caseNames {
+		t.Run(tt, func(t *testing.T) {
+			g := NewWithT(t)
+			c, err := readApplication(tt)
+			g.Expect(err).NotTo(HaveOccurred())
+
+			err = wh.validate(c)
+			g.Expect(err).To(HaveOccurred())
+			t.Logf("%v", err)
+		})
+	}
+}
+
+func readApplication(filename string) (*v1alpha1.Application, error) {
+	b, err := os.ReadFile(filename)
+	if err != nil {
+		return nil, err
+	}
+
+	c := &v1alpha1.Application{}
+	if err := yaml.Unmarshal(b, c); err != nil {
+		return nil, err
+	}
+
+	return c, nil
+}
diff --git a/pkg/webhooks/testdata/application/invalid-fleet1.yaml b/pkg/webhooks/testdata/application/invalid-fleet1.yaml
new file mode 100644
index 0000000..2090fd9
--- /dev/null
+++ b/pkg/webhooks/testdata/application/invalid-fleet1.yaml
@@ -0,0 +1,38 @@
+apiVersion: apps.kurator.dev/v1alpha1
+kind: Application
+metadata:
+  name: gitrepo-kustomization-demo
+  namespace: default
+spec:
+  destination:
+    fleet: quickstart1
+    clusterSelector:
+  source:
+    gitRepository:
+      interval: 3m0s
+      ref:
+        branch: master
+      timeout: 1m0s
+      url: https://github.com/stefanprodan/podinfo
+  syncPolicies:
+    - destination:
+        fleet: quickstart2
+        clusterSelector:
+          matchLabels:
+            env: test
+      kustomization:
+        interval: 5m0s
+        path: ./deploy/webapp
+        prune: true
+        timeout: 2m0s
+    - destination:
+        fleet: quickstart3
+        clusterSelector:
+          matchLabels:
+            env: dev
+      kustomization:
+        targetNamespace: default
+        interval: 5m0s
+        path: ./kustomize
+        prune: true
+        timeout: 2m0s
diff --git a/pkg/webhooks/testdata/application/invalid-fleet2.yaml b/pkg/webhooks/testdata/application/invalid-fleet2.yaml
new file mode 100644
index 0000000..2f1e279
--- /dev/null
+++ b/pkg/webhooks/testdata/application/invalid-fleet2.yaml
@@ -0,0 +1,35 @@
+apiVersion: apps.kurator.dev/v1alpha1
+kind: Application
+metadata:
+  name: gitrepo-kustomization-demo
+  namespace: default
+spec:
+  source:
+    gitRepository:
+      interval: 3m0s
+      ref:
+        branch: master
+      timeout: 1m0s
+      url: https://github.com/stefanprodan/podinfo
+  syncPolicies:
+    - destination:
+        fleet: quickstart1
+        clusterSelector:
+          matchLabels:
+            env: test
+      kustomization:
+        interval: 5m0s
+        path: ./deploy/webapp
+        prune: true
+        timeout: 2m0s
+    - destination:
+        fleet: quickstart2
+        clusterSelector:
+          matchLabels:
+            env: dev
+      kustomization:
+        targetNamespace: default
+        interval: 5m0s
+        path: ./kustomize
+        prune: true
+        timeout: 2m0s
diff --git a/pkg/webhooks/testdata/application/miss-default-and-policy-fleet.yaml b/pkg/webhooks/testdata/application/miss-default-and-policy-fleet.yaml
new file mode 100644
index 0000000..1a7a138
--- /dev/null
+++ b/pkg/webhooks/testdata/application/miss-default-and-policy-fleet.yaml
@@ -0,0 +1,34 @@
+apiVersion: apps.kurator.dev/v1alpha1
+kind: Application
+metadata:
+  name: gitrepo-kustomization-demo
+  namespace: default
+spec:
+  source:
+    gitRepository:
+      interval: 3m0s
+      ref:
+        branch: master
+      timeout: 1m0s
+      url: https://github.com/stefanprodan/podinfo
+  syncPolicies:
+    - destination:
+        fleet: quickstart
+        clusterSelector:
+          matchLabels:
+            env: test
+      kustomization:
+        interval: 5m0s
+        path: ./deploy/webapp
+        prune: true
+        timeout: 2m0s
+    - destination:
+        clusterSelector:
+          matchLabels:
+            env: dev
+      kustomization:
+        targetNamespace: default
+        interval: 5m0s
+        path: ./kustomize
+        prune: true
+        timeout: 2m0s