Repository: kurator-dev/kurator
Pull request #356 (2023-07-03 09:38:32)

custommachine: init webhook
**What type of PR is this?**


/kind feature


**What this PR does / why we need it**:

**Which issue(s) this PR fixes**:
add webhook for custommachine
part of #336 

**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
add webhook for custommachine

```



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 hzxuzhonghu 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:

- **[OWNERS](https://github.com/kurator-dev/kurator/blob/main/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":["hzxuzhonghu"]} -->
netlify[bot]: ### <span aria-hidden="true">👷</span> Deploy Preview for *kurator-dev* processing.


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

This pull-request has been approved by: *<a href="https://github.com/kurator-dev/kurator/pull/356#pullrequestreview-1540586141" title="Approved">hzxuzhonghu</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:

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

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":[]} -->
Xieql: 
/label tide/merge-method-squash


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

This pull-request has been approved by: *<a href="https://github.com/kurator-dev/kurator/pull/356#pullrequestreview-1542582077" title="LGTM">hzxuzhonghu</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:

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

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/cluster-operator/customcluster/customcluster.go b/cmd/cluster-operator/customcluster/customcluster.go
index 47d1c9a..150a52e 100644
--- a/cmd/cluster-operator/customcluster/customcluster.go
+++ b/cmd/cluster-operator/customcluster/customcluster.go
@@ -55,5 +55,12 @@ func InitControllers(ctx context.Context, opts *options.Options, mgr ctrl.Manage
 		return err
 	}
 
+	if err := (&webhooks.CustomMachineWebhook{
+		Client: mgr.GetClient(),
+	}).SetupWebhookWithManager(mgr); err != nil {
+		log.Error(err, "unable to create CustomMachine webhook", "Webhook", "CustomMachine")
+		return err
+	}
+
 	return nil
 }
diff --git a/manifests/charts/cluster-operator/templates/webhooks.yaml b/manifests/charts/cluster-operator/templates/webhooks.yaml
index 04928a0..6225dc7 100644
--- a/manifests/charts/cluster-operator/templates/webhooks.yaml
+++ b/manifests/charts/cluster-operator/templates/webhooks.yaml
@@ -50,6 +50,28 @@ webhooks:
     resources:
     - customclusters
   sideEffects: None
+- admissionReviewVersions:
+    - v1
+    - v1beta1
+  clientConfig:
+    service:
+      name: kurator-webhook-service
+      namespace: {{ .Release.Namespace }}
+      path: /validate-infrastructure-cluster-x-k8s-io-v1alpha1-custommachine # get this path from webhook service log
+  failurePolicy: Fail
+  matchPolicy: Equivalent
+  name: validation.custommachine.infrastructure.cluster.x-k8s.io
+  rules:
+    - apiGroups:
+        - infrastructure.cluster.x-k8s.io
+      apiVersions:
+        - v1alpha1
+      operations:
+        - CREATE
+        - UPDATE
+      resources:
+        - custommachines
+  sideEffects: None
 - admissionReviewVersions:
     - v1
     - v1beta1
diff --git a/pkg/webhooks/custommachine_webhook.go b/pkg/webhooks/custommachine_webhook.go
new file mode 100644
index 0000000..e10005a
--- /dev/null
+++ b/pkg/webhooks/custommachine_webhook.go
@@ -0,0 +1,118 @@
+/*
+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/infra/v1alpha1"
+)
+
+var _ webhook.CustomValidator = &CustomMachineWebhook{}
+
+type CustomMachineWebhook struct {
+	Client client.Reader
+}
+
+func (wh *CustomMachineWebhook) SetupWebhookWithManager(mgr ctrl.Manager) error {
+	return ctrl.NewWebhookManagedBy(mgr).
+		For(&v1alpha1.CustomMachine{}).
+		WithValidator(wh).
+		Complete()
+}
+
+func (wh *CustomMachineWebhook) ValidateCreate(_ context.Context, obj runtime.Object) error {
+	in, ok := obj.(*v1alpha1.CustomMachine)
+	if !ok {
+		return apierrors.NewBadRequest(fmt.Sprintf("expected a CustomMachine but got a %T", obj))
+	}
+
+	return wh.validate(in)
+}
+
+func (wh *CustomMachineWebhook) validate(in *v1alpha1.CustomMachine) error {
+	var allErrs field.ErrorList
+
+	if len(in.Spec.Master) == 0 {
+		allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "master"), in.Spec.Master,
+			"at least one master must be configured"))
+	} else if len(in.Spec.Master)%2 == 0 {
+		// etcd nodes must be set to an odd number, see https://github.com/kubernetes-sigs/kubespray/blob/0405af11077bc271529f9eca790a7dac4edf3891/docs/nodes.md
+		// we do not have a dedicated etcd configuration, so we are using the master node as the etcd node
+		allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "master"), len(in.Spec.Master),
+			"the number of master nodes need to be set to an odd number due to the restrictions of etcd cluster."))
+	}
+
+	if len(in.Spec.Nodes) == 0 {
+		allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "nodes"), in.Spec.Nodes,
+			"at least one node must be configured"))
+	}
+
+	allErrs = append(allErrs, validateMachine(in.Spec.Master, field.NewPath("spec", "master"))...)
+	allErrs = append(allErrs, validateMachine(in.Spec.Nodes, field.NewPath("spec", "node"))...)
+
+	if len(allErrs) > 0 {
+		return apierrors.NewInvalid(v1alpha1.SchemeGroupVersion.WithKind("CustomMachine").GroupKind(), in.Name, allErrs)
+	}
+
+	return nil
+}
+
+func validateMachine(machineArr []v1alpha1.Machine, fldPath *field.Path) field.ErrorList {
+	var allErrs field.ErrorList
+
+	for i, machine := range machineArr {
+		if machine.PrivateIP == "" {
+			allErrs = append(allErrs, field.Required(fldPath.Index(i).Child("privateIP"), "must be set"))
+		} else {
+			allErrs = append(allErrs, validateIP(machine.PrivateIP, fldPath.Index(i).Child("privateIP"))...)
+		}
+		if machine.PublicIP == "" {
+			allErrs = append(allErrs, field.Required(fldPath.Index(i).Child("publicIP"), "must be set"))
+		} else {
+			allErrs = append(allErrs, validateIP(machine.PublicIP, fldPath.Index(i).Child("publicIP"))...)
+		}
+	}
+
+	return allErrs
+}
+
+func (wh *CustomMachineWebhook) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) error {
+	_, ok := oldObj.(*v1alpha1.CustomMachine)
+	if !ok {
+		return apierrors.NewBadRequest(fmt.Sprintf("expected a CustomMachine but got a %T", oldObj))
+	}
+
+	newCustomMachine, ok := newObj.(*v1alpha1.CustomMachine)
+	if !ok {
+		return apierrors.NewBadRequest(fmt.Sprintf("expected a CustomMachine but got a %T", newObj))
+	}
+
+	return wh.validate(newCustomMachine)
+}
+
+func (wh *CustomMachineWebhook) ValidateDelete(_ context.Context, obj runtime.Object) error {
+	return nil
+}


Test patch:
diff --git a/pkg/webhooks/custommachine_webhook_test.go b/pkg/webhooks/custommachine_webhook_test.go
new file mode 100644
index 0000000..bbf6825
--- /dev/null
+++ b/pkg/webhooks/custommachine_webhook_test.go
@@ -0,0 +1,73 @@
+/*
+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/infra/v1alpha1"
+)
+
+func TestInvalidCustomMachineValidation(t *testing.T) {
+	r := path.Join("testdata", "custommachine")
+	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 := &CustomMachineWebhook{}
+	for _, tt := range caseNames {
+		t.Run(tt, func(t *testing.T) {
+			g := NewWithT(t)
+			c, err := readCustomMachine(tt)
+			g.Expect(err).NotTo(HaveOccurred())
+
+			err = wh.validate(c)
+			g.Expect(err).To(HaveOccurred())
+			t.Logf("%v", err)
+		})
+	}
+}
+
+func readCustomMachine(filename string) (*v1alpha1.CustomMachine, error) {
+	b, err := os.ReadFile(filename)
+	if err != nil {
+		return nil, err
+	}
+
+	c := &v1alpha1.CustomMachine{}
+	if err := yaml.Unmarshal(b, c); err != nil {
+		return nil, err
+	}
+
+	return c, nil
+}
diff --git a/pkg/webhooks/testdata/custommachine/invalid-master-nums.yaml b/pkg/webhooks/testdata/custommachine/invalid-master-nums.yaml
new file mode 100644
index 0000000..836f0ee
--- /dev/null
+++ b/pkg/webhooks/testdata/custommachine/invalid-master-nums.yaml
@@ -0,0 +1,43 @@
+apiVersion: infrastructure.cluster.x-k8s.io/v1alpha1
+kind: CustomMachine
+metadata:
+  name: cc-custommachine
+  namespace: default
+spec:
+  master:
+    - hostName: master1
+      publicIP: 202.1.1.1
+      privateIP: 192.1.1.1
+      sshKey:
+        apiVersion: v1
+        kind: Secret
+        name: cluster-secret
+    - hostName: master2
+      publicIP: 202.1.2.2
+      privateIP: 192.1.2.2
+      sshKey:
+        apiVersion: v1
+        kind: Secret
+        name: cluster-secret
+    - hostName: master3
+      publicIP: 202.1.3.3
+      privateIP: 192.1.3.3
+      sshKey:
+        apiVersion: v1
+        kind: Secret
+        name: cluster-secret
+    - hostName: master4
+      publicIP: 202.1.4.4
+      privateIP: 192.1.4.4
+      sshKey:
+        apiVersion: v1
+        kind: Secret
+        name: cluster-secret
+  node:
+    - hostName: node1
+      publicIP: 202.2.1.1
+      privateIP: 192.2.1.1
+      sshKey:
+        apiVersion: v1
+        kind: Secret
+        name: cluster-secret
diff --git a/pkg/webhooks/testdata/custommachine/invalid-master-public-ip.yaml b/pkg/webhooks/testdata/custommachine/invalid-master-public-ip.yaml
new file mode 100644
index 0000000..3a90661
--- /dev/null
+++ b/pkg/webhooks/testdata/custommachine/invalid-master-public-ip.yaml
@@ -0,0 +1,36 @@
+apiVersion: infrastructure.cluster.x-k8s.io/v1alpha1
+kind: CustomMachine
+metadata:
+  name: cc-custommachine
+  namespace: default
+spec:
+  master:
+    - hostName: master1
+      publicIP: 202.1.1.1
+      privateIP: 192.1.1.1
+      sshKey:
+        apiVersion: v1
+        kind: Secret
+        name: cluster-secret
+    - hostName: master2
+      publicIP: 202.1.2.2.2
+      privateIP: 192.1.2.2
+      sshKey:
+        apiVersion: v1
+        kind: Secret
+        name: cluster-secret
+    - hostName: master3
+      publicIP: 202.1.3.3
+      privateIP: 192.1.3.3
+      sshKey:
+        apiVersion: v1
+        kind: Secret
+        name: cluster-secret
+  node:
+    - hostName: node1
+      publicIP: 202.2.1.1
+      privateIP: 192.2.1.1
+      sshKey:
+        apiVersion: v1
+        kind: Secret
+        name: cluster-secret
diff --git a/pkg/webhooks/testdata/custommachine/invalid-node-private-ip.yaml b/pkg/webhooks/testdata/custommachine/invalid-node-private-ip.yaml
new file mode 100644
index 0000000..071183b
--- /dev/null
+++ b/pkg/webhooks/testdata/custommachine/invalid-node-private-ip.yaml
@@ -0,0 +1,36 @@
+apiVersion: infrastructure.cluster.x-k8s.io/v1alpha1
+kind: CustomMachine
+metadata:
+  name: cc-custommachine
+  namespace: default
+spec:
+  master:
+    - hostName: master1
+      publicIP: 202.1.1.1
+      privateIP: 192.1.1.1
+      sshKey:
+        apiVersion: v1
+        kind: Secret
+        name: cluster-secret
+    - hostName: master2
+      publicIP: 202.1.2.2
+      privateIP: 192.1.2.2
+      sshKey:
+        apiVersion: v1
+        kind: Secret
+        name: cluster-secret
+    - hostName: master3
+      publicIP: 202.1.3.3
+      privateIP: 192.1.3.3
+      sshKey:
+        apiVersion: v1
+        kind: Secret
+        name: cluster-secret
+  node:
+    - hostName: node1
+      publicIP: 202.2.1.1
+      privateIP: 192-2-1-1
+      sshKey:
+        apiVersion: v1
+        kind: Secret
+        name: cluster-secret
diff --git a/pkg/webhooks/testdata/custommachine/miss-master.yaml b/pkg/webhooks/testdata/custommachine/miss-master.yaml
new file mode 100644
index 0000000..bcc0fa2
--- /dev/null
+++ b/pkg/webhooks/testdata/custommachine/miss-master.yaml
@@ -0,0 +1,22 @@
+apiVersion: infrastructure.cluster.x-k8s.io/v1alpha1
+kind: CustomMachine
+metadata:
+  name: cc-custommachine
+  namespace: default
+spec:
+  master:
+  node:
+    - hostName: node1
+      publicIP: 202.2.1.1
+      privateIP: 192.2.1.1
+      sshKey:
+        apiVersion: v1
+        kind: Secret
+        name: cluster-secret
+    - hostName: node2
+      publicIP: 202.2.2.2
+      privateIP: 192.2.2.2
+      sshKey:
+        apiVersion: v1
+        kind: Secret
+        name: cluster-secret
diff --git a/pkg/webhooks/testdata/custommachine/miss-node.yaml b/pkg/webhooks/testdata/custommachine/miss-node.yaml
new file mode 100644
index 0000000..16e2db8
--- /dev/null
+++ b/pkg/webhooks/testdata/custommachine/miss-node.yaml
@@ -0,0 +1,30 @@
+apiVersion: infrastructure.cluster.x-k8s.io/v1alpha1
+kind: CustomMachine
+metadata:
+  name: cc-custommachine
+  namespace: default
+spec:
+  master:
+    - hostName: master1
+      publicIP: 202.1.1.1
+      privateIP: 192.1.1.1
+      sshKey:
+        apiVersion: v1
+        kind: Secret
+        name: cluster-secret
+    - hostName: master2
+      publicIP: 202.1.2.2
+      privateIP: 192.1.2.2
+      sshKey:
+        apiVersion: v1
+        kind: Secret
+        name: cluster-secret
+    - hostName: master3
+      publicIP: 202.1.3.3
+      privateIP: 192.1.3.3
+      sshKey:
+        apiVersion: v1
+        kind: Secret
+        name: cluster-secret
+  node:
+