From 0895335b73aeaa0feca16d86744c686f0d6d11ca Mon Sep 17 00:00:00 2001 From: zgjia Date: Thu, 21 May 2026 11:41:40 +0800 Subject: [PATCH] add cluster name label admission webhook Add a generic core admission handler that enforces the cluster.x-k8s.io/cluster-name label for explicitly configured resources. The initial generated webhook rule targets Cluster create/update requests, requiring Cluster objects to carry the label themselves while still allowing future provider resources to inherit the label from labeled owners. Register the webhook in the core manager and envtest setup, expose it through the public webhook alias package, and generate the corresponding webhook/RBAC manifests. Add unit coverage for existing labels, owner-based mutation, missing owners, missing labels, multiple owners, and invalid owner references. Tested: - make GO_VERSION=1.25.9 generate-manifests-core - go test ./internal/webhooks ./webhooks . - git diff --check Co-Authored-By: Claude Opus 4.7 --- config/rbac/role.yaml | 6 + config/webhook/manifests.yaml | 22 ++ internal/test/envtest/environment.go | 3 + internal/webhooks/cluster_name_label.go | 130 ++++++++++ internal/webhooks/cluster_name_label_test.go | 243 +++++++++++++++++++ main.go | 5 + webhooks/alias.go | 12 + 7 files changed, 421 insertions(+) create mode 100644 internal/webhooks/cluster_name_label.go create mode 100644 internal/webhooks/cluster_name_label_test.go diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index af20d5eb5ac1..14eddcc66ea4 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -148,6 +148,12 @@ rules: - patch - update - watch +- apiGroups: + - exp.cluster.x-k8s.io + resources: + - machinepools + verbs: + - get - apiGroups: - ipam.cluster.x-k8s.io resources: diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index 4b3858713e87..552ee85b4257 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -4,6 +4,28 @@ kind: MutatingWebhookConfiguration metadata: name: mutating-webhook-configuration webhooks: +- admissionReviewVersions: + - v1 + - v1beta1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /mutate-cluster-x-k8s-io-v1beta1-cluster-name-label + failurePolicy: Fail + matchPolicy: Equivalent + name: cluster-name-label.cluster.cluster.x-k8s.io + rules: + - apiGroups: + - cluster.x-k8s.io + apiVersions: + - v1beta1 + operations: + - CREATE + - UPDATE + resources: + - clusters + sideEffects: None - admissionReviewVersions: - v1 - v1beta1 diff --git a/internal/test/envtest/environment.go b/internal/test/envtest/environment.go index b829c032892e..7b8f84619475 100644 --- a/internal/test/envtest/environment.go +++ b/internal/test/envtest/environment.go @@ -325,6 +325,9 @@ func newEnvironment(managerCacheOptions cache.Options, uncachedObjs ...client.Ob // Set minNodeStartupTimeout for Test, so it does not need to be at least 30s internalwebhooks.SetMinNodeStartupTimeout(metav1.Duration{Duration: 1 * time.Millisecond}) + if err := (&webhooks.ClusterNameLabel{Client: mgr.GetAPIReader()}).SetupWebhookWithManager(mgr); err != nil { + klog.Fatalf("unable to create webhook: %+v", err) + } if err := (&webhooks.Cluster{Client: mgr.GetClient()}).SetupWebhookWithManager(mgr); err != nil { klog.Fatalf("unable to create webhook: %+v", err) } diff --git a/internal/webhooks/cluster_name_label.go b/internal/webhooks/cluster_name_label.go new file mode 100644 index 000000000000..7591af986dd1 --- /dev/null +++ b/internal/webhooks/cluster_name_label.go @@ -0,0 +1,130 @@ +/* +Copyright 2026 The Kubernetes 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" + "encoding/json" + "fmt" + "net/http" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" +) + +const clusterNameLabelWebhookPath = "/mutate-cluster-x-k8s-io-v1beta1-cluster-name-label" + +// ClusterNameLabel enforces the cluster name label for explicitly configured resources. +type ClusterNameLabel struct { + Client client.Reader +} + +func (h *ClusterNameLabel) SetupWebhookWithManager(mgr ctrl.Manager) error { + if h.Client == nil { + h.Client = mgr.GetAPIReader() + } + + mgr.GetWebhookServer().Register(clusterNameLabelWebhookPath, &webhook.Admission{Handler: h}) + return nil +} + +// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=clusters;machines;machinesets;machinedeployments;machinehealthchecks,verbs=get +// +kubebuilder:rbac:groups=exp.cluster.x-k8s.io,resources=machinepools,verbs=get +// +kubebuilder:webhook:verbs=create;update,path=/mutate-cluster-x-k8s-io-v1beta1-cluster-name-label,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=cluster.x-k8s.io,resources=clusters,versions=v1beta1,name=cluster-name-label.cluster.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1 + +var _ admission.Handler = &ClusterNameLabel{} + +func (webhook *ClusterNameLabel) Handle(ctx context.Context, req admission.Request) admission.Response { + obj := &unstructured.Unstructured{} + if err := json.Unmarshal(req.Object.Raw, obj); err != nil { + return admission.Errored(http.StatusBadRequest, fmt.Errorf("failed to decode object: %w", err)) + } + + if clusterName := obj.GetLabels()[clusterv1.ClusterNameLabel]; clusterName != "" { + return admission.Allowed("") + } + + clusterName, err := webhook.clusterNameFromOwner(ctx, obj.GetNamespace(), obj.GetOwnerReferences()) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + if clusterName == "" { + return admission.Denied(fmt.Sprintf("%s label must be set on the object or one of its owners", clusterv1.ClusterNameLabel)) + } + + labels := obj.GetLabels() + if labels == nil { + labels = map[string]string{} + } + labels[clusterv1.ClusterNameLabel] = clusterName + obj.SetLabels(labels) + + mutatedRaw, err := json.Marshal(obj) + if err != nil { + return admission.Errored(http.StatusInternalServerError, fmt.Errorf("failed to encode mutated object: %w", err)) + } + return admission.PatchResponseFromRaw(req.Object.Raw, mutatedRaw) +} + +func (webhook *ClusterNameLabel) clusterNameFromOwner(ctx context.Context, namespace string, ownerRefs []metav1.OwnerReference) (string, error) { + for _, ownerRef := range sortedOwnerReferences(ownerRefs) { + gv, err := schema.ParseGroupVersion(ownerRef.APIVersion) + if err != nil { + return "", fmt.Errorf("failed to parse owner apiVersion %q: %w", ownerRef.APIVersion, err) + } + + owner := &unstructured.Unstructured{} + owner.SetGroupVersionKind(gv.WithKind(ownerRef.Kind)) + if err := webhook.Client.Get(ctx, client.ObjectKey{Namespace: namespace, Name: ownerRef.Name}, owner); err != nil { + if apierrors.IsNotFound(err) { + continue + } + return "", fmt.Errorf("failed to get owner %s %s/%s: %w", ownerRef.Kind, namespace, ownerRef.Name, err) + } + + if clusterName := owner.GetLabels()[clusterv1.ClusterNameLabel]; clusterName != "" { + return clusterName, nil + } + } + return "", nil +} + +func sortedOwnerReferences(ownerRefs []metav1.OwnerReference) []metav1.OwnerReference { + if len(ownerRefs) <= 1 { + return ownerRefs + } + + controllerOwners := make([]metav1.OwnerReference, 0, 1) + otherOwners := make([]metav1.OwnerReference, 0, len(ownerRefs)) + for _, ownerRef := range ownerRefs { + if ownerRef.Controller != nil && *ownerRef.Controller { + controllerOwners = append(controllerOwners, ownerRef) + continue + } + otherOwners = append(otherOwners, ownerRef) + } + + return append(controllerOwners, otherOwners...) +} diff --git a/internal/webhooks/cluster_name_label_test.go b/internal/webhooks/cluster_name_label_test.go new file mode 100644 index 000000000000..5c4c38ee2668 --- /dev/null +++ b/internal/webhooks/cluster_name_label_test.go @@ -0,0 +1,243 @@ +/* +Copyright 2026 The Kubernetes 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" + "encoding/json" + "testing" + + jsonpatch "github.com/evanphx/json-patch/v5" + . "github.com/onsi/gomega" + admissionv1 "k8s.io/api/admission/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/uuid" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" +) + +func TestClusterNameLabelHandle(t *testing.T) { + scheme := runtime.NewScheme() + _ = clusterv1.AddToScheme(scheme) + + ownerWithLabel := newUnstructured("cluster.x-k8s.io/v1beta1", "Machine", "default", "owner-with-label", map[string]string{ + clusterv1.ClusterNameLabel: "owner-cluster", + }) + otherOwnerWithLabel := newUnstructured("cluster.x-k8s.io/v1beta1", "MachineSet", "default", "other-owner-with-label", map[string]string{ + clusterv1.ClusterNameLabel: "other-cluster", + }) + ownerWithoutLabel := newUnstructured("cluster.x-k8s.io/v1beta1", "Machine", "default", "owner-without-label", nil) + + tests := []struct { + name string + object *unstructured.Unstructured + owners []client.Object + expectAllowed bool + expectPatched bool + expectLabel string + expectMessage string + }{ + { + name: "allows cluster with cluster name label", + object: newUnstructured("cluster.x-k8s.io/v1beta1", "Cluster", "default", "cluster-with-label", map[string]string{ + clusterv1.ClusterNameLabel: "cluster-with-label", + }), + expectAllowed: true, + expectLabel: "cluster-with-label", + }, + { + name: "denies cluster without cluster name label and owner", + object: newUnstructured("cluster.x-k8s.io/v1beta1", "Cluster", "default", "cluster-without-label", nil), + expectAllowed: false, + expectMessage: "cluster.x-k8s.io/cluster-name label must be set on the object or one of its owners", + }, + { + name: "copies label from controller owner", + object: withOwnerReferences(newUnstructured("infrastructure.cluster.x-k8s.io/v1beta1", "InfraMachine", "default", "infra-machine", nil), []metav1.OwnerReference{ + ownerRef("cluster.x-k8s.io/v1beta1", "Machine", "owner-with-label", true), + }), + owners: []client.Object{ownerWithLabel}, + expectAllowed: true, + expectPatched: true, + expectLabel: "owner-cluster", + }, + { + name: "creates labels map when copying from owner", + object: withOwnerReferences(newUnstructured("infrastructure.cluster.x-k8s.io/v1beta1", "InfraCluster", "default", "infra-cluster", nil), []metav1.OwnerReference{ + ownerRef("cluster.x-k8s.io/v1beta1", "Machine", "owner-with-label", true), + }), + owners: []client.Object{ownerWithLabel}, + expectAllowed: true, + expectPatched: true, + expectLabel: "owner-cluster", + }, + { + name: "does not overwrite existing label", + object: withOwnerReferences(newUnstructured("infrastructure.cluster.x-k8s.io/v1beta1", "InfraMachine", "default", "infra-machine", map[string]string{ + clusterv1.ClusterNameLabel: "existing-cluster", + }), []metav1.OwnerReference{ + ownerRef("cluster.x-k8s.io/v1beta1", "Machine", "owner-with-label", true), + }), + owners: []client.Object{ownerWithLabel}, + expectAllowed: true, + expectLabel: "existing-cluster", + }, + { + name: "checks multiple owners", + object: withOwnerReferences(newUnstructured("infrastructure.cluster.x-k8s.io/v1beta1", "InfraMachine", "default", "infra-machine", nil), []metav1.OwnerReference{ + ownerRef("cluster.x-k8s.io/v1beta1", "Machine", "owner-without-label", true), + ownerRef("cluster.x-k8s.io/v1beta1", "MachineSet", "other-owner-with-label", false), + }), + owners: []client.Object{ownerWithoutLabel, otherOwnerWithLabel}, + expectAllowed: true, + expectPatched: true, + expectLabel: "other-cluster", + }, + { + name: "denies when owner lacks cluster name label", + object: withOwnerReferences(newUnstructured("infrastructure.cluster.x-k8s.io/v1beta1", "InfraMachine", "default", "infra-machine", nil), []metav1.OwnerReference{ + ownerRef("cluster.x-k8s.io/v1beta1", "Machine", "owner-without-label", true), + }), + owners: []client.Object{ownerWithoutLabel}, + expectAllowed: false, + expectMessage: "cluster.x-k8s.io/cluster-name label must be set on the object or one of its owners", + }, + { + name: "denies when owner is not found", + object: withOwnerReferences(newUnstructured("infrastructure.cluster.x-k8s.io/v1beta1", "InfraMachine", "default", "infra-machine", nil), []metav1.OwnerReference{ + ownerRef("cluster.x-k8s.io/v1beta1", "Machine", "missing-owner", true), + }), + expectAllowed: false, + expectMessage: "cluster.x-k8s.io/cluster-name label must be set on the object or one of its owners", + }, + { + name: "not found owner does not block other owner", + object: withOwnerReferences(newUnstructured("infrastructure.cluster.x-k8s.io/v1beta1", "InfraMachine", "default", "infra-machine", nil), []metav1.OwnerReference{ + ownerRef("cluster.x-k8s.io/v1beta1", "Machine", "missing-owner", true), + ownerRef("cluster.x-k8s.io/v1beta1", "MachineSet", "other-owner-with-label", false), + }), + owners: []client.Object{otherOwnerWithLabel}, + expectAllowed: true, + expectPatched: true, + expectLabel: "other-cluster", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(tt.owners...).Build() + handler := &ClusterNameLabel{Client: fakeClient} + + raw := mustMarshal(t, tt.object) + resp := handler.Handle(context.Background(), admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{ + UID: uuid.NewUUID(), + Kind: metav1.GroupVersionKind(tt.object.GroupVersionKind()), + Operation: admissionv1.Create, + Object: runtime.RawExtension{Raw: raw}, + }}) + + g.Expect(resp.Allowed).To(Equal(tt.expectAllowed)) + if tt.expectMessage != "" { + g.Expect(resp.Result.Message).To(Equal(tt.expectMessage)) + } + + if !tt.expectAllowed { + return + } + + mutatedRaw := raw + if tt.expectPatched { + g.Expect(resp.Patches).ToNot(BeEmpty()) + patchRaw, err := json.Marshal(resp.Patches) + g.Expect(err).ToNot(HaveOccurred()) + patch, err := jsonpatch.DecodePatch(patchRaw) + g.Expect(err).ToNot(HaveOccurred()) + mutatedRaw, err = patch.Apply(raw) + g.Expect(err).ToNot(HaveOccurred()) + } else { + g.Expect(resp.Patches).To(BeEmpty()) + } + + mutated := &unstructured.Unstructured{} + g.Expect(json.Unmarshal(mutatedRaw, mutated)).To(Succeed()) + g.Expect(mutated.GetLabels()[clusterv1.ClusterNameLabel]).To(Equal(tt.expectLabel)) + }) + } +} + +func TestClusterNameLabelHandleWithInvalidOwnerAPIVersion(t *testing.T) { + g := NewWithT(t) + handler := &ClusterNameLabel{Client: fake.NewClientBuilder().WithScheme(runtime.NewScheme()).Build()} + obj := withOwnerReferences(newUnstructured("infrastructure.cluster.x-k8s.io/v1beta1", "InfraMachine", "default", "infra-machine", nil), []metav1.OwnerReference{ + {APIVersion: "invalid/api/version", Kind: "Machine", Name: "owner", Controller: ptr.To(true)}, + }) + + resp := handler.Handle(context.Background(), admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{ + UID: uuid.NewUUID(), + Kind: metav1.GroupVersionKind(obj.GroupVersionKind()), + Operation: admissionv1.Create, + Object: runtime.RawExtension{Raw: mustMarshal(t, obj)}, + }}) + + g.Expect(resp.Allowed).To(BeFalse()) + g.Expect(resp.Result.Code).To(Equal(int32(500))) + g.Expect(resp.Result.Message).To(ContainSubstring("failed to parse owner apiVersion")) +} + +func newUnstructured(apiVersion, kind, namespace, name string, labels map[string]string) *unstructured.Unstructured { + gv, err := schema.ParseGroupVersion(apiVersion) + if err != nil { + panic(err) + } + obj := &unstructured.Unstructured{} + obj.SetGroupVersionKind(gv.WithKind(kind)) + obj.SetNamespace(namespace) + obj.SetName(name) + obj.SetLabels(labels) + return obj +} + +func withOwnerReferences(obj *unstructured.Unstructured, ownerRefs []metav1.OwnerReference) *unstructured.Unstructured { + obj.SetOwnerReferences(ownerRefs) + return obj +} + +func ownerRef(apiVersion, kind, name string, controller bool) metav1.OwnerReference { + return metav1.OwnerReference{ + APIVersion: apiVersion, + Kind: kind, + Name: name, + Controller: &controller, + } +} + +func mustMarshal(t *testing.T, obj *unstructured.Unstructured) []byte { + t.Helper() + raw, err := json.Marshal(obj) + if err != nil { + t.Fatal(err) + } + return raw +} diff --git a/main.go b/main.go index 06a263cef021..5d980758f469 100644 --- a/main.go +++ b/main.go @@ -759,6 +759,11 @@ func setupReconcilers(ctx context.Context, mgr ctrl.Manager, watchNamespaces map } func setupWebhooks(mgr ctrl.Manager, clusterCacheReader webhooks.ClusterCacheReader) { + if err := (&webhooks.ClusterNameLabel{Client: mgr.GetAPIReader()}).SetupWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "Unable to create webhook", "webhook", "ClusterNameLabel") + os.Exit(1) + } + // NOTE: ClusterClass and managed topologies are behind ClusterTopology feature gate flag; the webhook // is going to prevent creating or updating new objects in case the feature flag is disabled. if err := (&webhooks.ClusterClass{Client: mgr.GetClient()}).SetupWebhookWithManager(mgr); err != nil { diff --git a/webhooks/alias.go b/webhooks/alias.go index 4df0a852a81a..3c8f1b77663c 100644 --- a/webhooks/alias.go +++ b/webhooks/alias.go @@ -58,6 +58,18 @@ func (webhook *Cluster) DefaultAndValidateVariables(ctx context.Context, cluster return webhooks.DefaultAndValidateVariables(ctx, cluster, oldCluster, clusterClass) } +// ClusterNameLabel implements a mutating webhook for the cluster name label. +type ClusterNameLabel struct { + Client client.Reader +} + +// SetupWebhookWithManager sets up ClusterNameLabel webhooks. +func (webhook *ClusterNameLabel) SetupWebhookWithManager(mgr ctrl.Manager) error { + return (&webhooks.ClusterNameLabel{ + Client: webhook.Client, + }).SetupWebhookWithManager(mgr) +} + // ClusterClass implements a validation and defaulting webhook for ClusterClass. type ClusterClass struct { Client client.Reader