diff --git a/config/crd/bases/addons.cluster.x-k8s.io_clusterresourcesetbindings.yaml b/config/crd/bases/addons.cluster.x-k8s.io_clusterresourcesetbindings.yaml index edf31d9af124..1fe1d47f60f2 100644 --- a/config/crd/bases/addons.cluster.x-k8s.io_clusterresourcesetbindings.yaml +++ b/config/crd/bases/addons.cluster.x-k8s.io_clusterresourcesetbindings.yaml @@ -272,6 +272,10 @@ spec: - clusterResourceSetName type: object type: array + clusterName: + description: 'ClusterName is the name of the Cluster this binding + applies to. Note: this field mandatory in v1beta2.' + type: string type: object type: object served: true diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index f38f2a1e673f..83174169ca30 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -410,6 +410,28 @@ webhooks: resources: - clusterresourcesets sideEffects: None +- admissionReviewVersions: + - v1 + - v1beta1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /validate-addons-cluster-x-k8s-io-v1beta1-clusterresourcesetbinding + failurePolicy: Fail + matchPolicy: Equivalent + name: validation.clusterresourcesetbinding.addons.cluster.x-k8s.io + rules: + - apiGroups: + - addons.cluster.x-k8s.io + apiVersions: + - v1beta1 + operations: + - CREATE + - UPDATE + resources: + - clusterresourcesetbindings + sideEffects: None - admissionReviewVersions: - v1 - v1beta1 diff --git a/exp/addons/api/v1alpha3/conversion.go b/exp/addons/api/v1alpha3/conversion.go index 25cd0b8b5db1..e7c38ca772c6 100644 --- a/exp/addons/api/v1alpha3/conversion.go +++ b/exp/addons/api/v1alpha3/conversion.go @@ -17,9 +17,11 @@ limitations under the License. package v1alpha3 import ( + apiconversion "k8s.io/apimachinery/pkg/conversion" "sigs.k8s.io/controller-runtime/pkg/conversion" addonsv1 "sigs.k8s.io/cluster-api/exp/addons/api/v1beta1" + utilconversion "sigs.k8s.io/cluster-api/util/conversion" ) func (src *ClusterResourceSet) ConvertTo(dstRaw conversion.Hub) error { @@ -49,13 +51,31 @@ func (dst *ClusterResourceSetList) ConvertFrom(srcRaw conversion.Hub) error { func (src *ClusterResourceSetBinding) ConvertTo(dstRaw conversion.Hub) error { dst := dstRaw.(*addonsv1.ClusterResourceSetBinding) - return Convert_v1alpha3_ClusterResourceSetBinding_To_v1beta1_ClusterResourceSetBinding(src, dst, nil) + if err := Convert_v1alpha3_ClusterResourceSetBinding_To_v1beta1_ClusterResourceSetBinding(src, dst, nil); err != nil { + return err + } + // Manually restore data. + restored := &addonsv1.ClusterResourceSetBinding{} + if ok, err := utilconversion.UnmarshalData(src, restored); err != nil || !ok { + return err + } + dst.Spec.ClusterName = restored.Spec.ClusterName + return nil } func (dst *ClusterResourceSetBinding) ConvertFrom(srcRaw conversion.Hub) error { src := srcRaw.(*addonsv1.ClusterResourceSetBinding) - return Convert_v1beta1_ClusterResourceSetBinding_To_v1alpha3_ClusterResourceSetBinding(src, dst, nil) + if err := Convert_v1beta1_ClusterResourceSetBinding_To_v1alpha3_ClusterResourceSetBinding(src, dst, nil); err != nil { + return err + } + + // Preserve Hub data on down-conversion except for metadata + if err := utilconversion.MarshalData(src, dst); err != nil { + return err + } + + return nil } func (src *ClusterResourceSetBindingList) ConvertTo(dstRaw conversion.Hub) error { @@ -69,3 +89,9 @@ func (dst *ClusterResourceSetBindingList) ConvertFrom(srcRaw conversion.Hub) err return Convert_v1beta1_ClusterResourceSetBindingList_To_v1alpha3_ClusterResourceSetBindingList(src, dst, nil) } + +// Convert_v1beta1_ClusterResourceSetBindingSpec_To_v1alpha3_ClusterResourceSetBindingSpec is a conversion function. +func Convert_v1beta1_ClusterResourceSetBindingSpec_To_v1alpha3_ClusterResourceSetBindingSpec(in *addonsv1.ClusterResourceSetBindingSpec, out *ClusterResourceSetBindingSpec, s apiconversion.Scope) error { + // Spec.ClusterName does not exist in ClusterResourceSetBinding v1alpha3 API. + return autoConvert_v1beta1_ClusterResourceSetBindingSpec_To_v1alpha3_ClusterResourceSetBindingSpec(in, out, s) +} diff --git a/exp/addons/api/v1alpha3/zz_generated.conversion.go b/exp/addons/api/v1alpha3/zz_generated.conversion.go index 48b5d5349e6a..b8514b7f909b 100644 --- a/exp/addons/api/v1alpha3/zz_generated.conversion.go +++ b/exp/addons/api/v1alpha3/zz_generated.conversion.go @@ -74,11 +74,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*v1beta1.ClusterResourceSetBindingSpec)(nil), (*ClusterResourceSetBindingSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1beta1_ClusterResourceSetBindingSpec_To_v1alpha3_ClusterResourceSetBindingSpec(a.(*v1beta1.ClusterResourceSetBindingSpec), b.(*ClusterResourceSetBindingSpec), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*ClusterResourceSetList)(nil), (*v1beta1.ClusterResourceSetList)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha3_ClusterResourceSetList_To_v1beta1_ClusterResourceSetList(a.(*ClusterResourceSetList), b.(*v1beta1.ClusterResourceSetList), scope) }); err != nil { @@ -139,6 +134,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*v1beta1.ClusterResourceSetBindingSpec)(nil), (*ClusterResourceSetBindingSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1beta1_ClusterResourceSetBindingSpec_To_v1alpha3_ClusterResourceSetBindingSpec(a.(*v1beta1.ClusterResourceSetBindingSpec), b.(*ClusterResourceSetBindingSpec), scope) + }); err != nil { + return err + } return nil } @@ -202,7 +202,17 @@ func Convert_v1beta1_ClusterResourceSetBinding_To_v1alpha3_ClusterResourceSetBin func autoConvert_v1alpha3_ClusterResourceSetBindingList_To_v1beta1_ClusterResourceSetBindingList(in *ClusterResourceSetBindingList, out *v1beta1.ClusterResourceSetBindingList, s conversion.Scope) error { out.ListMeta = in.ListMeta - out.Items = *(*[]v1beta1.ClusterResourceSetBinding)(unsafe.Pointer(&in.Items)) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]v1beta1.ClusterResourceSetBinding, len(*in)) + for i := range *in { + if err := Convert_v1alpha3_ClusterResourceSetBinding_To_v1beta1_ClusterResourceSetBinding(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Items = nil + } return nil } @@ -213,7 +223,17 @@ func Convert_v1alpha3_ClusterResourceSetBindingList_To_v1beta1_ClusterResourceSe func autoConvert_v1beta1_ClusterResourceSetBindingList_To_v1alpha3_ClusterResourceSetBindingList(in *v1beta1.ClusterResourceSetBindingList, out *ClusterResourceSetBindingList, s conversion.Scope) error { out.ListMeta = in.ListMeta - out.Items = *(*[]ClusterResourceSetBinding)(unsafe.Pointer(&in.Items)) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]ClusterResourceSetBinding, len(*in)) + for i := range *in { + if err := Convert_v1beta1_ClusterResourceSetBinding_To_v1alpha3_ClusterResourceSetBinding(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Items = nil + } return nil } @@ -234,14 +254,10 @@ func Convert_v1alpha3_ClusterResourceSetBindingSpec_To_v1beta1_ClusterResourceSe func autoConvert_v1beta1_ClusterResourceSetBindingSpec_To_v1alpha3_ClusterResourceSetBindingSpec(in *v1beta1.ClusterResourceSetBindingSpec, out *ClusterResourceSetBindingSpec, s conversion.Scope) error { out.Bindings = *(*[]*ResourceSetBinding)(unsafe.Pointer(&in.Bindings)) + // WARNING: in.ClusterName requires manual conversion: does not exist in peer-type return nil } -// Convert_v1beta1_ClusterResourceSetBindingSpec_To_v1alpha3_ClusterResourceSetBindingSpec is an autogenerated conversion function. -func Convert_v1beta1_ClusterResourceSetBindingSpec_To_v1alpha3_ClusterResourceSetBindingSpec(in *v1beta1.ClusterResourceSetBindingSpec, out *ClusterResourceSetBindingSpec, s conversion.Scope) error { - return autoConvert_v1beta1_ClusterResourceSetBindingSpec_To_v1alpha3_ClusterResourceSetBindingSpec(in, out, s) -} - func autoConvert_v1alpha3_ClusterResourceSetList_To_v1beta1_ClusterResourceSetList(in *ClusterResourceSetList, out *v1beta1.ClusterResourceSetList, s conversion.Scope) error { out.ListMeta = in.ListMeta if in.Items != nil { diff --git a/exp/addons/api/v1alpha4/conversion.go b/exp/addons/api/v1alpha4/conversion.go index 1de13aac78c0..b1863ff4eaf6 100644 --- a/exp/addons/api/v1alpha4/conversion.go +++ b/exp/addons/api/v1alpha4/conversion.go @@ -17,9 +17,11 @@ limitations under the License. package v1alpha4 import ( + apiconversion "k8s.io/apimachinery/pkg/conversion" "sigs.k8s.io/controller-runtime/pkg/conversion" addonsv1 "sigs.k8s.io/cluster-api/exp/addons/api/v1beta1" + utilconversion "sigs.k8s.io/cluster-api/util/conversion" ) func (src *ClusterResourceSet) ConvertTo(dstRaw conversion.Hub) error { @@ -49,13 +51,31 @@ func (dst *ClusterResourceSetList) ConvertFrom(srcRaw conversion.Hub) error { func (src *ClusterResourceSetBinding) ConvertTo(dstRaw conversion.Hub) error { dst := dstRaw.(*addonsv1.ClusterResourceSetBinding) - return Convert_v1alpha4_ClusterResourceSetBinding_To_v1beta1_ClusterResourceSetBinding(src, dst, nil) + if err := Convert_v1alpha4_ClusterResourceSetBinding_To_v1beta1_ClusterResourceSetBinding(src, dst, nil); err != nil { + return err + } + // Manually restore data. + restored := &addonsv1.ClusterResourceSetBinding{} + if ok, err := utilconversion.UnmarshalData(src, restored); err != nil || !ok { + return err + } + dst.Spec.ClusterName = restored.Spec.ClusterName + return nil } func (dst *ClusterResourceSetBinding) ConvertFrom(srcRaw conversion.Hub) error { src := srcRaw.(*addonsv1.ClusterResourceSetBinding) - return Convert_v1beta1_ClusterResourceSetBinding_To_v1alpha4_ClusterResourceSetBinding(src, dst, nil) + if err := Convert_v1beta1_ClusterResourceSetBinding_To_v1alpha4_ClusterResourceSetBinding(src, dst, nil); err != nil { + return err + } + + // Preserve Hub data on down-conversion except for metadata + if err := utilconversion.MarshalData(src, dst); err != nil { + return err + } + + return nil } func (src *ClusterResourceSetBindingList) ConvertTo(dstRaw conversion.Hub) error { @@ -69,3 +89,9 @@ func (dst *ClusterResourceSetBindingList) ConvertFrom(srcRaw conversion.Hub) err return Convert_v1beta1_ClusterResourceSetBindingList_To_v1alpha4_ClusterResourceSetBindingList(src, dst, nil) } + +// Convert_v1beta1_ClusterResourceSetBindingSpec_To_v1alpha4_ClusterResourceSetBindingSpec is a conversion function. +func Convert_v1beta1_ClusterResourceSetBindingSpec_To_v1alpha4_ClusterResourceSetBindingSpec(in *addonsv1.ClusterResourceSetBindingSpec, out *ClusterResourceSetBindingSpec, s apiconversion.Scope) error { + // Spec.ClusterName does not exist in ClusterResourceSetBinding v1alpha4 API. + return autoConvert_v1beta1_ClusterResourceSetBindingSpec_To_v1alpha4_ClusterResourceSetBindingSpec(in, out, s) +} diff --git a/exp/addons/api/v1alpha4/zz_generated.conversion.go b/exp/addons/api/v1alpha4/zz_generated.conversion.go index 72a5d116a2c5..3eafdc8da85a 100644 --- a/exp/addons/api/v1alpha4/zz_generated.conversion.go +++ b/exp/addons/api/v1alpha4/zz_generated.conversion.go @@ -74,11 +74,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*v1beta1.ClusterResourceSetBindingSpec)(nil), (*ClusterResourceSetBindingSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1beta1_ClusterResourceSetBindingSpec_To_v1alpha4_ClusterResourceSetBindingSpec(a.(*v1beta1.ClusterResourceSetBindingSpec), b.(*ClusterResourceSetBindingSpec), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*ClusterResourceSetList)(nil), (*v1beta1.ClusterResourceSetList)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha4_ClusterResourceSetList_To_v1beta1_ClusterResourceSetList(a.(*ClusterResourceSetList), b.(*v1beta1.ClusterResourceSetList), scope) }); err != nil { @@ -139,6 +134,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*v1beta1.ClusterResourceSetBindingSpec)(nil), (*ClusterResourceSetBindingSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1beta1_ClusterResourceSetBindingSpec_To_v1alpha4_ClusterResourceSetBindingSpec(a.(*v1beta1.ClusterResourceSetBindingSpec), b.(*ClusterResourceSetBindingSpec), scope) + }); err != nil { + return err + } return nil } @@ -202,7 +202,17 @@ func Convert_v1beta1_ClusterResourceSetBinding_To_v1alpha4_ClusterResourceSetBin func autoConvert_v1alpha4_ClusterResourceSetBindingList_To_v1beta1_ClusterResourceSetBindingList(in *ClusterResourceSetBindingList, out *v1beta1.ClusterResourceSetBindingList, s conversion.Scope) error { out.ListMeta = in.ListMeta - out.Items = *(*[]v1beta1.ClusterResourceSetBinding)(unsafe.Pointer(&in.Items)) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]v1beta1.ClusterResourceSetBinding, len(*in)) + for i := range *in { + if err := Convert_v1alpha4_ClusterResourceSetBinding_To_v1beta1_ClusterResourceSetBinding(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Items = nil + } return nil } @@ -213,7 +223,17 @@ func Convert_v1alpha4_ClusterResourceSetBindingList_To_v1beta1_ClusterResourceSe func autoConvert_v1beta1_ClusterResourceSetBindingList_To_v1alpha4_ClusterResourceSetBindingList(in *v1beta1.ClusterResourceSetBindingList, out *ClusterResourceSetBindingList, s conversion.Scope) error { out.ListMeta = in.ListMeta - out.Items = *(*[]ClusterResourceSetBinding)(unsafe.Pointer(&in.Items)) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]ClusterResourceSetBinding, len(*in)) + for i := range *in { + if err := Convert_v1beta1_ClusterResourceSetBinding_To_v1alpha4_ClusterResourceSetBinding(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Items = nil + } return nil } @@ -234,14 +254,10 @@ func Convert_v1alpha4_ClusterResourceSetBindingSpec_To_v1beta1_ClusterResourceSe func autoConvert_v1beta1_ClusterResourceSetBindingSpec_To_v1alpha4_ClusterResourceSetBindingSpec(in *v1beta1.ClusterResourceSetBindingSpec, out *ClusterResourceSetBindingSpec, s conversion.Scope) error { out.Bindings = *(*[]*ResourceSetBinding)(unsafe.Pointer(&in.Bindings)) + // WARNING: in.ClusterName requires manual conversion: does not exist in peer-type return nil } -// Convert_v1beta1_ClusterResourceSetBindingSpec_To_v1alpha4_ClusterResourceSetBindingSpec is an autogenerated conversion function. -func Convert_v1beta1_ClusterResourceSetBindingSpec_To_v1alpha4_ClusterResourceSetBindingSpec(in *v1beta1.ClusterResourceSetBindingSpec, out *ClusterResourceSetBindingSpec, s conversion.Scope) error { - return autoConvert_v1beta1_ClusterResourceSetBindingSpec_To_v1alpha4_ClusterResourceSetBindingSpec(in, out, s) -} - func autoConvert_v1alpha4_ClusterResourceSetList_To_v1beta1_ClusterResourceSetList(in *ClusterResourceSetList, out *v1beta1.ClusterResourceSetList, s conversion.Scope) error { out.ListMeta = in.ListMeta if in.Items != nil { diff --git a/exp/addons/api/v1beta1/clusterresourcesetbinding_types.go b/exp/addons/api/v1beta1/clusterresourcesetbinding_types.go index 47f8762ec3e6..3b0ba2ddb97a 100644 --- a/exp/addons/api/v1beta1/clusterresourcesetbinding_types.go +++ b/exp/addons/api/v1beta1/clusterresourcesetbinding_types.go @@ -133,6 +133,11 @@ type ClusterResourceSetBindingSpec struct { // Bindings is a list of ClusterResourceSets and their resources. // +optional Bindings []*ResourceSetBinding `json:"bindings,omitempty"` + + // ClusterName is the name of the Cluster this binding applies to. + // Note: this field mandatory in v1beta2. + // +optional + ClusterName string `json:"clusterName,omitempty"` } // ANCHOR_END: ClusterResourceSetBindingSpec diff --git a/exp/addons/api/v1beta1/clusterresourcesetbinding_webhook.go b/exp/addons/api/v1beta1/clusterresourcesetbinding_webhook.go new file mode 100644 index 000000000000..91a5b12c49f8 --- /dev/null +++ b/exp/addons/api/v1beta1/clusterresourcesetbinding_webhook.go @@ -0,0 +1,78 @@ +/* +Copyright 2022 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 v1beta1 + +import ( + "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/webhook" + + "sigs.k8s.io/cluster-api/feature" +) + +func (c *ClusterResourceSetBinding) SetupWebhookWithManager(mgr ctrl.Manager) error { + return ctrl.NewWebhookManagedBy(mgr). + For(c). + Complete() +} + +// +kubebuilder:webhook:verbs=create;update,path=/validate-addons-cluster-x-k8s-io-v1beta1-clusterresourcesetbinding,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=addons.cluster.x-k8s.io,resources=clusterresourcesetbindings,versions=v1beta1,name=validation.clusterresourcesetbinding.addons.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1 + +var _ webhook.Validator = &ClusterResourceSetBinding{} + +// ValidateCreate implements webhook.Validator so a webhook will be registered for the type. +func (c *ClusterResourceSetBinding) ValidateCreate() error { + return c.validate(nil) +} + +// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type. +func (c *ClusterResourceSetBinding) ValidateUpdate(old runtime.Object) error { + oldBinding, ok := old.(*ClusterResourceSetBinding) + if !ok { + return apierrors.NewBadRequest(fmt.Sprintf("expected a ClusterResourceSetBinding but got a %T", old)) + } + return c.validate(oldBinding) +} + +// ValidateDelete implements webhook.Validator so a webhook will be registered for the type. +func (c *ClusterResourceSetBinding) ValidateDelete() error { + return nil +} + +func (c *ClusterResourceSetBinding) validate(old *ClusterResourceSetBinding) error { + // NOTE: ClusterResourceSet is behind ClusterResourceSet feature gate flag; the web hook + // must prevent creating new objects in case the feature flag is disabled. + if !feature.Gates.Enabled(feature.ClusterResourceSet) { + return field.Forbidden( + field.NewPath("spec"), + "can be set only if the ClusterResourceSet feature flag is enabled", + ) + } + var allErrs field.ErrorList + if old != nil && old.Spec.ClusterName != "" && old.Spec.ClusterName != c.Spec.ClusterName { + allErrs = append(allErrs, + field.Invalid(field.NewPath("spec", "clusterName"), c.Spec.ClusterName, "field is immutable")) + } + if len(allErrs) == 0 { + return nil + } + return apierrors.NewInvalid(GroupVersion.WithKind("ClusterResourceSetBinding").GroupKind(), c.Name, allErrs) +} diff --git a/exp/addons/api/v1beta1/clusterresourcesetbinding_webhook_test.go b/exp/addons/api/v1beta1/clusterresourcesetbinding_webhook_test.go new file mode 100644 index 000000000000..0530a6183edc --- /dev/null +++ b/exp/addons/api/v1beta1/clusterresourcesetbinding_webhook_test.go @@ -0,0 +1,89 @@ +/* +Copyright 2022 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 v1beta1 + +import ( + "testing" + + . "github.com/onsi/gomega" +) + +func TestClusterResourceSetBindingClusterNameImmutable(t *testing.T) { + tests := []struct { + name string + oldClusterName string + newClusterName string + expectErr bool + }{ + { + name: "when ClusterName is empty", + oldClusterName: "", + newClusterName: "", + expectErr: false, + }, + { + name: "add ClusterName field", + oldClusterName: "", + newClusterName: "bar", + expectErr: false, + }, + { + name: "when the ClusterName has not changed", + oldClusterName: "bar", + newClusterName: "bar", + expectErr: false, + }, + { + name: "when the ClusterName has changed", + oldClusterName: "bar", + newClusterName: "different", + expectErr: true, + }, + { + name: "existing ClusterName field to be set to empty", + oldClusterName: "bar", + newClusterName: "", + expectErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + newClusterResourceSetBinding := &ClusterResourceSetBinding{ + Spec: ClusterResourceSetBindingSpec{ + ClusterName: tt.newClusterName, + }, + } + + oldClusterResourceSetBinding := &ClusterResourceSetBinding{ + Spec: ClusterResourceSetBindingSpec{ + ClusterName: tt.oldClusterName, + }, + } + + if tt.expectErr { + g.Expect(newClusterResourceSetBinding.ValidateCreate()).To(Succeed()) + g.Expect(newClusterResourceSetBinding.ValidateUpdate(oldClusterResourceSetBinding)).NotTo(Succeed()) + return + } + g.Expect(newClusterResourceSetBinding.ValidateCreate()).To(Succeed()) + g.Expect(newClusterResourceSetBinding.ValidateUpdate(oldClusterResourceSetBinding)).To(Succeed()) + }) + } +} diff --git a/exp/addons/internal/controllers/clusterresourceset_controller.go b/exp/addons/internal/controllers/clusterresourceset_controller.go index a029bface122..36dc8b12244a 100644 --- a/exp/addons/internal/controllers/clusterresourceset_controller.go +++ b/exp/addons/internal/controllers/clusterresourceset_controller.go @@ -269,7 +269,7 @@ func (r *ClusterResourceSetReconciler) ApplyClusterResourceSet(ctx context.Conte }() // Ensure that the owner references are set on the ClusterResourceSetBinding. - clusterResourceSetBinding.OwnerReferences = ensureOwnerRefs(clusterResourceSetBinding, clusterResourceSet, cluster) + clusterResourceSetBinding.OwnerReferences = ensureOwnerRefs(clusterResourceSetBinding, clusterResourceSet) errList := []error{} resourceSetBinding := clusterResourceSetBinding.GetOrCreateBinding(clusterResourceSet) diff --git a/exp/addons/internal/controllers/clusterresourceset_controller_test.go b/exp/addons/internal/controllers/clusterresourceset_controller_test.go index 60c53b2702f3..8f8bcb384f5c 100644 --- a/exp/addons/internal/controllers/clusterresourceset_controller_test.go +++ b/exp/addons/internal/controllers/clusterresourceset_controller_test.go @@ -235,7 +235,7 @@ metadata: testCluster.SetLabels(labels) g.Expect(env.Update(ctx, testCluster)).To(Succeed()) - t.Log("Verifying ClusterResourceSetBinding is created with cluster owner reference") + t.Log("Verifying ClusterResourceSetBinding is created with cluster name") g.Eventually(func() bool { binding := &addonsv1.ClusterResourceSetBinding{} clusterResourceSetBindingKey := client.ObjectKey{ @@ -247,12 +247,7 @@ metadata: return false } - return util.HasOwnerRef(binding.GetOwnerReferences(), metav1.OwnerReference{ - APIVersion: clusterv1.GroupVersion.String(), - Kind: "Cluster", - Name: testCluster.Name, - UID: testCluster.UID, - }) + return binding.Spec.ClusterName == testCluster.Name }, timeout).Should(BeTrue()) // Wait until ClusterResourceSetBinding is created for the Cluster @@ -528,8 +523,8 @@ metadata: if err != nil { return false } - return len(binding.Spec.Bindings) == 2 && len(binding.OwnerReferences) == 3 - }, timeout).Should(BeTrue(), "Expected 2 ClusterResourceSets and 3 OwnerReferences") + return len(binding.Spec.Bindings) == 2 && len(binding.OwnerReferences) == 2 + }, timeout).Should(BeTrue(), "Expected 2 ClusterResourceSets and 2 OwnerReferences") t.Log("Verifying deleted CRS is deleted from ClusterResourceSetBinding") // Delete one of the CRS instances and wait until it is removed from the binding list. @@ -544,8 +539,8 @@ metadata: if err != nil { return false } - return len(binding.Spec.Bindings) == 1 && len(binding.OwnerReferences) == 2 - }, timeout).Should(BeTrue(), "ClusterResourceSetBinding should have 1 ClusterResourceSet and 2 OwnerReferences") + return len(binding.Spec.Bindings) == 1 && len(binding.OwnerReferences) == 1 + }, timeout).Should(BeTrue(), "ClusterResourceSetBinding should have 1 ClusterResourceSet and 1 OwnerReferences") t.Log("Verifying ClusterResourceSetBinding is deleted after deleting all matching CRS objects") // Delete one of the CRS instances and wait until it is removed from the binding list. @@ -940,12 +935,7 @@ func clusterResourceSetBindingReady(env *envtest.Environment, cluster *clusterv1 return false } - return util.HasOwnerRef(binding.GetOwnerReferences(), metav1.OwnerReference{ - APIVersion: clusterv1.GroupVersion.String(), - Kind: "Cluster", - Name: cluster.Name, - UID: cluster.UID, - }) + return binding.Spec.ClusterName == cluster.Name } } diff --git a/exp/addons/internal/controllers/clusterresourceset_helpers.go b/exp/addons/internal/controllers/clusterresourceset_helpers.go index 2b25ceab3ef8..be0dcdb6d0cb 100644 --- a/exp/addons/internal/controllers/clusterresourceset_helpers.go +++ b/exp/addons/internal/controllers/clusterresourceset_helpers.go @@ -32,6 +32,7 @@ import ( 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" "k8s.io/apimachinery/pkg/types" "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" @@ -120,9 +121,9 @@ func (r *ClusterResourceSetReconciler) getOrCreateClusterResourceSetBinding(ctx } clusterResourceSetBinding.Name = cluster.Name clusterResourceSetBinding.Namespace = cluster.Namespace - clusterResourceSetBinding.OwnerReferences = ensureOwnerRefs(clusterResourceSetBinding, clusterResourceSet, cluster) + clusterResourceSetBinding.OwnerReferences = ensureOwnerRefs(clusterResourceSetBinding, clusterResourceSet) clusterResourceSetBinding.Spec.Bindings = []*addonsv1.ResourceSetBinding{} - + clusterResourceSetBinding.Spec.ClusterName = cluster.Name if err := r.Client.Create(ctx, clusterResourceSetBinding); err != nil { if apierrors.IsAlreadyExists(err) { if err = r.Client.Get(ctx, clusterResourceSetBindingKey, clusterResourceSetBinding); err != nil { @@ -136,16 +137,10 @@ func (r *ClusterResourceSetReconciler) getOrCreateClusterResourceSetBinding(ctx return clusterResourceSetBinding, nil } -// ensureOwnerRefs ensures Cluster and ClusterResourceSet owner references are set on the ClusterResourceSetBinding. -func ensureOwnerRefs(clusterResourceSetBinding *addonsv1.ClusterResourceSetBinding, clusterResourceSet *addonsv1.ClusterResourceSet, cluster *clusterv1.Cluster) []metav1.OwnerReference { +// ensureOwnerRefs ensure ClusterResourceSet owner references are set on the ClusterResourceSetBinding. +func ensureOwnerRefs(clusterResourceSetBinding *addonsv1.ClusterResourceSetBinding, clusterResourceSet *addonsv1.ClusterResourceSet) []metav1.OwnerReference { ownerRefs := make([]metav1.OwnerReference, len(clusterResourceSetBinding.GetOwnerReferences())) copy(ownerRefs, clusterResourceSetBinding.GetOwnerReferences()) - ownerRefs = util.EnsureOwnerRef(ownerRefs, metav1.OwnerReference{ - APIVersion: clusterv1.GroupVersion.String(), - Kind: "Cluster", - Name: cluster.Name, - UID: cluster.UID, - }) ownerRefs = util.EnsureOwnerRef(ownerRefs, metav1.OwnerReference{ APIVersion: clusterResourceSet.GroupVersionKind().GroupVersion().String(), @@ -234,3 +229,24 @@ func normalizeData(resource *unstructured.Unstructured) ([][]byte, error) { return dataList, nil } + +func getClusterNameFromOwnerRef(obj metav1.ObjectMeta) (string, error) { + for _, ref := range obj.OwnerReferences { + if ref.Kind != "Cluster" { + continue + } + gv, err := schema.ParseGroupVersion(ref.APIVersion) + if err != nil { + return "", errors.Wrap(err, "failed to find cluster name in ownerRefs") + } + + if gv.Group != clusterv1.GroupVersion.Group { + continue + } + if ref.Name == "" { + return "", errors.New("failed to find cluster name in ownerRefs: ref name is empty") + } + return ref.Name, nil + } + return "", errors.New("failed to find cluster name in ownerRefs: no cluster ownerRef") +} diff --git a/exp/addons/internal/controllers/clusterresourcesetbinding_controller.go b/exp/addons/internal/controllers/clusterresourcesetbinding_controller.go index b0e12e3dd3f5..f1798e321a13 100644 --- a/exp/addons/internal/controllers/clusterresourcesetbinding_controller.go +++ b/exp/addons/internal/controllers/clusterresourcesetbinding_controller.go @@ -21,6 +21,7 @@ import ( "github.com/pkg/errors" apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" @@ -33,6 +34,7 @@ import ( "sigs.k8s.io/cluster-api/feature" "sigs.k8s.io/cluster-api/internal/hooks" "sigs.k8s.io/cluster-api/util" + "sigs.k8s.io/cluster-api/util/patch" "sigs.k8s.io/cluster-api/util/predicates" ) @@ -77,8 +79,33 @@ func (r *ClusterResourceSetBindingReconciler) Reconcile(ctx context.Context, req // Error reading the object - requeue the request. return ctrl.Result{}, err } - - cluster, err := util.GetOwnerCluster(ctx, r.Client, binding.ObjectMeta) + // Before 1.4 cluster name was stored as ownerRef; this code migrated this value to + // the spec.clusterName field if necessary. + clusterName := binding.Spec.ClusterName + if clusterName == "" { + // Initialize the patch helper. + patchHelper, err := patch.NewHelper(binding, r.Client) + if err != nil { + return ctrl.Result{}, errors.Wrap(err, "failed to configure the patch helper") + } + // Update the clusterName field of the existing ClusterResourceSetBindings with ownerReferences. + // More details please refer to: https://github.com/kubernetes-sigs/cluster-api/issues/7669. + if clusterName, err = getClusterNameFromOwnerRef(binding.ObjectMeta); err != nil { + return ctrl.Result{}, err + } + binding.Spec.ClusterName = clusterName + binding.OwnerReferences = util.RemoveOwnerRef(binding.OwnerReferences, metav1.OwnerReference{ + APIVersion: clusterv1.GroupVersion.String(), + Kind: "Cluster", + Name: clusterName, + }) + if err = patchHelper.Patch(ctx, binding); err != nil { + return ctrl.Result{}, errors.Wrap(err, "failed to patch ClusterResourceSetBinding to add spec.clusterName") + } + // Will enter the next Reconcile + return ctrl.Result{}, nil + } + cluster, err := util.GetClusterByName(ctx, r.Client, req.Namespace, clusterName) if err != nil { if apierrors.IsNotFound(err) { // If the owner cluster is already deleted, delete its ClusterResourceSetBinding @@ -87,10 +114,6 @@ func (r *ClusterResourceSetBindingReconciler) Reconcile(ctx context.Context, req } return ctrl.Result{}, err } - if cluster == nil { - log.Info("ownerRef not found for the ClusterResourceSetBinding") - return ctrl.Result{}, nil - } // If the owner cluster is in deletion process, delete its ClusterResourceSetBinding if !cluster.DeletionTimestamp.IsZero() { if feature.Gates.Enabled(feature.RuntimeSDK) && feature.Gates.Enabled(feature.ClusterTopology) { diff --git a/internal/test/envtest/environment.go b/internal/test/envtest/environment.go index 89def53bfaa2..32520fc4a4f8 100644 --- a/internal/test/envtest/environment.go +++ b/internal/test/envtest/environment.go @@ -298,6 +298,9 @@ func newEnvironment(uncachedObjs ...client.Object) *Environment { if err := (&addonsv1.ClusterResourceSet{}).SetupWebhookWithManager(mgr); err != nil { klog.Fatalf("unable to create webhook for crs: %+v", err) } + if err := (&addonsv1.ClusterResourceSetBinding{}).SetupWebhookWithManager(mgr); err != nil { + klog.Fatalf("unable to create webhook for ClusterResourceSetBinding: %+v", err) + } if err := (&expv1.MachinePool{}).SetupWebhookWithManager(mgr); err != nil { klog.Fatalf("unable to create webhook for machinepool: %+v", err) } diff --git a/main.go b/main.go index cb53d83bc168..5d747724523a 100644 --- a/main.go +++ b/main.go @@ -529,6 +529,12 @@ func setupWebhooks(mgr ctrl.Manager) { setupLog.Error(err, "unable to create webhook", "webhook", "ClusterResourceSet") os.Exit(1) } + // NOTE: ClusterResourceSetBinding is behind ClusterResourceSet feature gate flag; the webhook + // is going to prevent creating or updating new objects in case the feature flag is disabled + if err := (&addonsv1.ClusterResourceSetBinding{}).SetupWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create webhook", "webhook", "ClusterResourceSetBinding") + os.Exit(1) + } if err := (&clusterv1.MachineHealthCheck{}).SetupWebhookWithManager(mgr); err != nil { setupLog.Error(err, "unable to create webhook", "webhook", "MachineHealthCheck") diff --git a/test/framework/ownerreference_helpers.go b/test/framework/ownerreference_helpers.go index a1c0cc1fd136..cb2c3d43b195 100644 --- a/test/framework/ownerreference_helpers.go +++ b/test/framework/ownerreference_helpers.go @@ -157,9 +157,9 @@ var ExpOwnerReferenceAssertions = map[string]func([]metav1.OwnerReference) error // ClusterResourcesSet doesn't have ownerReferences (it is a clusterctl move-hierarchy root). return hasExactOwnersByGVK(owners, []schema.GroupVersionKind{}) }, - // ClusterResourcesSetBinding has both Cluster and ClusterResourceSet set as owners on creation. + // ClusterResourcesSetBinding has ClusterResourceSet set as owners on creation. clusterResourceSetBindingKind: func(owners []metav1.OwnerReference) error { - return hasExactOwnersByGVK(owners, []schema.GroupVersionKind{clusterGVK, clusterResourceSetGVK}) + return hasExactOwnersByGVK(owners, []schema.GroupVersionKind{clusterResourceSetGVK}) }, }