feat: prevent node movement by label modification (#1444)

* feat: prevent node movement by label modification Signed-off-by: ricky <[email protected]> * comment on nodepool related e2e test temporarily Signed-off-by: ricky <[email protected]> --------- Signed-off-by: ricky <[email protected]>
openyurtio · Jul 13, 2023 · bc168c4 · bc168c4
1 parent 7f41a78
commit bc168c4
Show file tree

Hide file tree

Showing 9 changed files with 648 additions and 511 deletions.
diff --git a/charts/yurt-manager/templates/yurt-manager-auto-generated.yaml b/charts/yurt-manager/templates/yurt-manager-auto-generated.yaml
@@ -588,6 +588,26 @@ webhooks:
     resources:
     - gateways
   sideEffects: None
+- admissionReviewVersions:
+  - v1
+  - v1beta1
+  clientConfig:
+    service:
+      name: yurt-manager-webhook-service
+      namespace: {{ .Release.Namespace }}
+      path: /validate-core-openyurt-io-v1-node
+  failurePolicy: Fail
+  name: validate.core.v1.node.openyurt.io
+  rules:
+  - apiGroups:
+    - ""
+    apiVersions:
+    - v1
+    operations:
+    - UPDATE
+    resources:
+    - nodes
+  sideEffects: None
 - admissionReviewVersions:
   - v1
   clientConfig:

diff --git a/go.mod b/go.mod
@@ -43,7 +43,6 @@ require (
 	k8s.io/utils v0.0.0-20210930125809-cb0fa318a74b
 	sigs.k8s.io/apiserver-network-proxy v0.0.15
 	sigs.k8s.io/controller-runtime v0.10.3
-	sigs.k8s.io/yaml v1.3.0
 )
 
 require (
@@ -158,6 +157,7 @@ require (
 	k8s.io/kube-openapi v0.0.0-20211115234752-e816edb12b65 // indirect
 	sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.0.22 // indirect
 	sigs.k8s.io/structured-merge-diff/v4 v4.1.2 // indirect
+	sigs.k8s.io/yaml v1.3.0 // indirect
 )
 
 replace (

diff --git a/pkg/webhook/node/v1/node_handler.go b/pkg/webhook/node/v1/node_handler.go
@@ -0,0 +1,57 @@
+/*
+Copyright 2023 The OpenYurt 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 v1
+
+import (
+	v1 "k8s.io/api/core/v1"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
+
+	"github.com/openyurtio/openyurt/pkg/webhook/builder"
+	"github.com/openyurtio/openyurt/pkg/webhook/util"
+)
+
+const (
+	WebhookName = "node"
+)
+
+// SetupWebhookWithManager sets up Cluster webhooks. 	mutate path, validatepath, error
+func (webhook *NodeHandler) SetupWebhookWithManager(mgr ctrl.Manager) (string, string, error) {
+	// init
+	webhook.Client = mgr.GetClient()
+
+	gvk, err := apiutil.GVKForObject(&v1.Node{}, mgr.GetScheme())
+	if err != nil {
+		return "", "", err
+	}
+	return util.GenerateMutatePath(gvk),
+		util.GenerateValidatePath(gvk),
+		builder.WebhookManagedBy(mgr).
+			For(&v1.Node{}).
+			WithValidator(webhook).
+			Complete()
+}
+
+// +kubebuilder:webhook:path=/validate-core-openyurt-io-v1-node,mutating=false,failurePolicy=fail,sideEffects=None,admissionReviewVersions=v1;v1beta1,groups="",resources=nodes,verbs=update,versions=v1,name=validate.core.v1.node.openyurt.io
+
+// Cluster implements a validating and defaulting webhook for Cluster.
+type NodeHandler struct {
+	Client client.Client
+}
+
+var _ builder.CustomValidator = &NodeHandler{}
diff --git a/pkg/webhook/node/v1/node_validation.go b/pkg/webhook/node/v1/node_validation.go
@@ -0,0 +1,78 @@
+/*
+Copyright 2023 The OpenYurt 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 v1
+
+import (
+	"context"
+	"fmt"
+
+	v1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/validation/field"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+
+	"github.com/openyurtio/openyurt/pkg/apis/apps"
+)
+
+// ValidateCreate implements webhook.CustomValidator so a webhook will be registered for the type.
+func (webhook *NodeHandler) ValidateCreate(_ context.Context, obj runtime.Object, req admission.Request) error {
+	return nil
+}
+
+// ValidateUpdate implements webhook.CustomValidator so a webhook will be registered for the type.
+func (webhook *NodeHandler) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object, req admission.Request) error {
+	newNode, ok := newObj.(*v1.Node)
+	if !ok {
+		return apierrors.NewBadRequest(fmt.Sprintf("expected a Node but got a %T", newObj))
+	}
+	oldNode, ok := oldObj.(*v1.Node)
+	if !ok {
+		return apierrors.NewBadRequest(fmt.Sprintf("expected a Node} but got a %T", oldObj))
+	}
+
+	if allErrs := validateNodeUpdate(newNode, oldNode, req); len(allErrs) > 0 {
+		return apierrors.NewInvalid(v1.SchemeGroupVersion.WithKind("Node").GroupKind(), newNode.Name, allErrs)
+	}
+
+	return nil
+}
+
+// ValidateDelete implements webhook.CustomValidator so a webhook will be registered for the type.
+func (webhook *NodeHandler) ValidateDelete(_ context.Context, obj runtime.Object, req admission.Request) error {
+	return nil
+}
+
+func validateNodeUpdate(newNode, oldNode *v1.Node, req admission.Request) field.ErrorList {
+	oldNp := oldNode.Labels[apps.LabelDesiredNodePool]
+	newNp := newNode.Labels[apps.LabelDesiredNodePool]
+
+	if len(oldNp) == 0 {
+		return nil
+	}
+
+	// can not change LabelDesiredNodePool if it has been set
+	if oldNp != newNp {
+		return field.ErrorList([]*field.Error{
+			field.Forbidden(
+				field.NewPath("metadata").Child("labels").Child(apps.LabelDesiredNodePool),
+				"apps.openyurt.io/desired-nodepool can not be changed"),
+		})
+	}
+
+	return nil
+}
diff --git a/pkg/webhook/server.go b/pkg/webhook/server.go
@@ -34,6 +34,7 @@ import (
 	"github.com/openyurtio/openyurt/pkg/controller/yurtappset"
 	"github.com/openyurtio/openyurt/pkg/controller/yurtstaticset"
 	v1alpha1gateway "github.com/openyurtio/openyurt/pkg/webhook/gateway/v1alpha1"
+	v1node "github.com/openyurtio/openyurt/pkg/webhook/node/v1"
 	v1alpha1nodepool "github.com/openyurtio/openyurt/pkg/webhook/nodepool/v1alpha1"
 	v1beta1nodepool "github.com/openyurtio/openyurt/pkg/webhook/nodepool/v1beta1"
 	v1alpha1platformadmin "github.com/openyurtio/openyurt/pkg/webhook/platformadmin/v1alpha1"
@@ -82,6 +83,7 @@ func init() {
 	addControllerWebhook(platformadmin.ControllerName, &v1alpha2platformadmin.PlatformAdminHandler{})
 
 	independentWebhooks[v1pod.WebhookName] = &v1pod.PodHandler{}
+	independentWebhooks[v1node.WebhookName] = &v1node.NodeHandler{}
 }
 
 // Note !!! @kadisi
@@ -108,6 +110,9 @@ func SetupWithManager(c *config.CompletedConfig, mgr manager.Manager) error {
 		return nil
 	}
 
+	// set up webhook namespace
+	util.SetNamespace(c.ComponentConfig.Generic.WorkingNamespace)
+
 	// set up independent webhooks
 	for name, s := range independentWebhooks {
 		if util.IsWebhookDisabled(name, c.ComponentConfig.Generic.DisabledWebhooks) {

diff --git a/pkg/webhook/util/controller/webhook_controller.go b/pkg/webhook/util/controller/webhook_controller.go
@@ -70,8 +70,6 @@ type Controller struct {
 }
 
 func New(handlers map[string]struct{}, cc *config.CompletedConfig) (*Controller, error) {
-	webhookutil.SetNamespace(cc.ComponentConfig.Generic.WorkingNamespace)
-
 	c := &Controller{
 		kubeClient:  extclient.GetGenericClientWithName("webhook-controller").KubeClient,
 		handlers:    handlers,