diff --git a/api/v1alpha6/openstackcluster_conversion.go b/api/v1alpha6/openstackcluster_conversion.go index 9192af795c..695d7c3248 100644 --- a/api/v1alpha6/openstackcluster_conversion.go +++ b/api/v1alpha6/openstackcluster_conversion.go @@ -365,7 +365,10 @@ func Convert_v1beta1_OpenStackClusterSpec_To_v1alpha6_OpenStackClusterSpec(in *i } out.CloudName = in.IdentityRef.CloudName - out.IdentityRef = &OpenStackIdentityReference{Name: in.IdentityRef.Name} + out.IdentityRef = &OpenStackIdentityReference{} + if err := Convert_v1beta1_OpenStackIdentityReference_To_v1alpha6_OpenStackIdentityReference(&in.IdentityRef, out.IdentityRef, s); err != nil { + return err + } if in.APIServerLoadBalancer != nil { if err := Convert_v1beta1_APIServerLoadBalancer_To_v1alpha6_APIServerLoadBalancer(in.APIServerLoadBalancer, &out.APIServerLoadBalancer, s); err != nil { diff --git a/api/v1alpha6/openstackmachine_conversion.go b/api/v1alpha6/openstackmachine_conversion.go index aed2d3b7d4..47a757bd7d 100644 --- a/api/v1alpha6/openstackmachine_conversion.go +++ b/api/v1alpha6/openstackmachine_conversion.go @@ -373,7 +373,6 @@ func Convert_v1beta1_OpenStackMachineSpec_To_v1alpha6_OpenStackMachineSpec(in *i } if in.IdentityRef != nil { - out.IdentityRef = &OpenStackIdentityReference{Name: in.IdentityRef.Name} out.CloudName = in.IdentityRef.CloudName } diff --git a/api/v1alpha6/types_conversion.go b/api/v1alpha6/types_conversion.go index 041ce9eb3a..30c8618cc8 100644 --- a/api/v1alpha6/types_conversion.go +++ b/api/v1alpha6/types_conversion.go @@ -534,6 +534,8 @@ func Convert_v1alpha6_OpenStackIdentityReference_To_v1beta1_OpenStackIdentityRef func Convert_v1beta1_OpenStackIdentityReference_To_v1alpha6_OpenStackIdentityReference(in *infrav1.OpenStackIdentityReference, out *OpenStackIdentityReference, _ apiconversion.Scope) error { out.Name = in.Name + // Kind will be overwritten during restore if it was previously set to some other value, but if not then some value is still required + out.Kind = "Secret" return nil } diff --git a/api/v1alpha7/openstackcluster_conversion.go b/api/v1alpha7/openstackcluster_conversion.go index 6224af306b..7b46f589a7 100644 --- a/api/v1alpha7/openstackcluster_conversion.go +++ b/api/v1alpha7/openstackcluster_conversion.go @@ -362,7 +362,10 @@ func Convert_v1beta1_OpenStackClusterSpec_To_v1alpha7_OpenStackClusterSpec(in *i } out.CloudName = in.IdentityRef.CloudName - out.IdentityRef = &OpenStackIdentityReference{Name: in.IdentityRef.Name} + out.IdentityRef = &OpenStackIdentityReference{} + if err := Convert_v1beta1_OpenStackIdentityReference_To_v1alpha7_OpenStackIdentityReference(&in.IdentityRef, out.IdentityRef, s); err != nil { + return err + } if in.APIServerLoadBalancer != nil { if err := Convert_v1beta1_APIServerLoadBalancer_To_v1alpha7_APIServerLoadBalancer(in.APIServerLoadBalancer, &out.APIServerLoadBalancer, s); err != nil { diff --git a/api/v1alpha7/types_conversion.go b/api/v1alpha7/types_conversion.go index 806e145806..db640e1e95 100644 --- a/api/v1alpha7/types_conversion.go +++ b/api/v1alpha7/types_conversion.go @@ -536,6 +536,8 @@ func Convert_v1alpha7_OpenStackIdentityReference_To_v1beta1_OpenStackIdentityRef func Convert_v1beta1_OpenStackIdentityReference_To_v1alpha7_OpenStackIdentityReference(in *infrav1.OpenStackIdentityReference, out *OpenStackIdentityReference, _ apiconversion.Scope) error { out.Name = in.Name + // Kind will be overwritten during restore if it was previously set to some other value, but if not then some value is still required + out.Kind = "Secret" return nil } diff --git a/test/e2e/suites/apivalidations/openstackcluster_test.go b/test/e2e/suites/apivalidations/openstackcluster_test.go index 84e4eac521..6ab495f8c9 100644 --- a/test/e2e/suites/apivalidations/openstackcluster_test.go +++ b/test/e2e/suites/apivalidations/openstackcluster_test.go @@ -19,6 +19,7 @@ package apivalidations import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/onsi/gomega/format" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" @@ -32,16 +33,6 @@ import ( var _ = Describe("OpenStackCluster API validations", func() { var namespace *corev1.Namespace - create := func(obj client.Object) error { - err := k8sClient.Create(ctx, obj) - if err == nil { - DeferCleanup(func() error { - return k8sClient.Delete(ctx, obj) - }) - } - return err - } - BeforeEach(func() { namespace = createNamespace() }) @@ -57,12 +48,12 @@ var _ = Describe("OpenStackCluster API validations", func() { }) It("should allow the smallest permissible cluster spec", func() { - Expect(create(cluster)).To(Succeed(), "OpenStackCluster creation should succeed") + Expect(createObj(cluster)).To(Succeed(), "OpenStackCluster creation should succeed") }) It("should only allow controlPlaneEndpoint to be set once", func() { By("Creating a bare cluster") - Expect(create(cluster)).To(Succeed(), "OpenStackCluster creation should succeed") + Expect(createObj(cluster)).To(Succeed(), "OpenStackCluster creation should succeed") By("Setting the control plane endpoint") cluster.Spec.ControlPlaneEndpoint = &clusterv1.APIEndpoint{ @@ -78,12 +69,12 @@ var _ = Describe("OpenStackCluster API validations", func() { It("should allow an empty managed security groups definition", func() { cluster.Spec.ManagedSecurityGroups = &infrav1.ManagedSecurityGroups{} - Expect(create(cluster)).To(Succeed(), "OpenStackCluster creation should succeed") + Expect(createObj(cluster)).To(Succeed(), "OpenStackCluster creation should succeed") }) It("should default enabled to true if APIServerLoadBalancer is specified without enabled=true", func() { cluster.Spec.APIServerLoadBalancer = &infrav1.APIServerLoadBalancer{} - Expect(create(cluster)).To(Succeed(), "OpenStackCluster creation should succeed") + Expect(createObj(cluster)).To(Succeed(), "OpenStackCluster creation should succeed") // Fetch the cluster and check the defaulting fetchedCluster := &infrav1.OpenStackCluster{} @@ -94,7 +85,7 @@ var _ = Describe("OpenStackCluster API validations", func() { }) It("should not default APIServerLoadBalancer if it is not specifid", func() { - Expect(create(cluster)).To(Succeed(), "OpenStackCluster creation should succeed") + Expect(createObj(cluster)).To(Succeed(), "OpenStackCluster creation should succeed") // Fetch the cluster and check the defaulting fetchedCluster := &infrav1.OpenStackCluster{} @@ -115,19 +106,19 @@ var _ = Describe("OpenStackCluster API validations", func() { }, }, } - Expect(create(cluster)).To(Succeed(), "OpenStackCluster creation should succeed") + Expect(createObj(cluster)).To(Succeed(), "OpenStackCluster creation should succeed") }) It("should not allow bastion.enabled=true without a spec", func() { cluster.Spec.Bastion = &infrav1.Bastion{ Enabled: ptr.To(true), } - Expect(create(cluster)).NotTo(Succeed(), "OpenStackCluster creation should not succeed") + Expect(createObj(cluster)).NotTo(Succeed(), "OpenStackCluster creation should not succeed") }) It("should not allow an empty Bastion", func() { cluster.Spec.Bastion = &infrav1.Bastion{} - Expect(create(cluster)).NotTo(Succeed(), "OpenStackCluster creation should not succeed") + Expect(createObj(cluster)).NotTo(Succeed(), "OpenStackCluster creation should not succeed") }) It("should default bastion.enabled=true", func() { @@ -140,7 +131,7 @@ var _ = Describe("OpenStackCluster API validations", func() { }, }, } - Expect(create(cluster)).To(Succeed(), "OpenStackCluster creation should not succeed") + Expect(createObj(cluster)).To(Succeed(), "OpenStackCluster creation should not succeed") // Fetch the cluster and check the defaulting fetchedCluster := &infrav1.OpenStackCluster{} @@ -161,7 +152,7 @@ var _ = Describe("OpenStackCluster API validations", func() { }, FloatingIP: ptr.To("10.0.0.0"), } - Expect(create(cluster)).To(Succeed(), "OpenStackCluster creation should succeed") + Expect(createObj(cluster)).To(Succeed(), "OpenStackCluster creation should succeed") }) It("should not allow non-IPv4 as bastion floating IP", func() { @@ -175,7 +166,7 @@ var _ = Describe("OpenStackCluster API validations", func() { }, FloatingIP: ptr.To("foobar"), } - Expect(create(cluster)).NotTo(Succeed(), "OpenStackCluster creation should not succeed") + Expect(createObj(cluster)).NotTo(Succeed(), "OpenStackCluster creation should not succeed") }) }) @@ -195,7 +186,7 @@ var _ = Describe("OpenStackCluster API validations", func() { Kind: "FakeKind", Name: "identity-ref", } - Expect(create(cluster)).To(Succeed(), "OpenStackCluster creation should succeed") + Expect(createObj(cluster)).To(Succeed(), "OpenStackCluster creation should succeed") // Fetch the infrav1 version of the cluster infrav1Cluster := &infrav1.OpenStackCluster{} @@ -217,7 +208,7 @@ var _ = Describe("OpenStackCluster API validations", func() { It("should not enable an explicitly disabled bastion when converting to v1beta1", func() { cluster.Spec.Bastion = &infrav1alpha7.Bastion{Enabled: false} - Expect(create(cluster)).To(Succeed(), "OpenStackCluster creation should succeed") + Expect(createObj(cluster)).To(Succeed(), "OpenStackCluster creation should succeed") // Fetch the infrav1 version of the cluster infrav1Cluster := &infrav1.OpenStackCluster{} @@ -232,5 +223,25 @@ var _ = Describe("OpenStackCluster API validations", func() { Expect(infrav1Bastion).ToNot(BeNil(), "Bastion should not have been removed") Expect(infrav1Bastion.Enabled).To(Equal(ptr.To(false)), "Bastion should remain disabled") }) + + It("should downgrade cleanly from infrav1", func() { + infrav1Cluster := &infrav1.OpenStackCluster{} + infrav1Cluster.Namespace = namespace.Name + infrav1Cluster.GenerateName = clusterNamePrefix + infrav1Cluster.Spec.IdentityRef.CloudName = "test-cloud" + infrav1Cluster.Spec.IdentityRef.Name = "test-credentials" + Expect(createObj(infrav1Cluster)).To(Succeed(), "infrav1 OpenStackCluster creation should succeed") + + // Just fetching the object as v1alpha6 doesn't trigger + // validation failure, so we first fetch it and then + // patch the object with identical contents. The patch + // triggers a validation failure. + cluster := &infrav1alpha7.OpenStackCluster{} //nolint: staticcheck + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: infrav1Cluster.Name, Namespace: infrav1Cluster.Namespace}, cluster)).To(Succeed(), "OpenStackCluster fetch should succeed") + + setObjectGVK(cluster) + cluster.ManagedFields = nil + Expect(k8sClient.Patch(ctx, cluster, client.Apply, client.FieldOwner("test"), client.ForceOwnership)).To(Succeed(), format.Object(cluster, 4)) + }) }) }) diff --git a/test/e2e/suites/apivalidations/openstackmachine_test.go b/test/e2e/suites/apivalidations/openstackmachine_test.go index c1d8415d42..0e978a1a97 100644 --- a/test/e2e/suites/apivalidations/openstackmachine_test.go +++ b/test/e2e/suites/apivalidations/openstackmachine_test.go @@ -21,9 +21,13 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/onsi/gomega/format" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + infrav1alpha7 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha7" infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1" ) @@ -332,4 +336,29 @@ var _ = Describe("OpenStackMachine API validations", func() { Expect(k8sClient.Create(ctx, machine)).NotTo(Succeed(), "Creating a machine with an additional block device with an empty name availability zone should fail") }) }) + + Context("v1alpha7", func() { + It("should downgrade cleanly from infrav1", func() { + infrav1Machine := &infrav1.OpenStackMachine{} + infrav1Machine.Namespace = namespace.Name + infrav1Machine.GenerateName = clusterNamePrefix + infrav1Machine.Spec.IdentityRef = &infrav1.OpenStackIdentityReference{ + CloudName: "test-cloud", + Name: "test-credentials", + } + infrav1Machine.Spec.Image.ID = ptr.To("de9872ee-0c2c-44ed-9414-90163c8b0e0d") + Expect(createObj(infrav1Machine)).To(Succeed(), "infrav1 OpenStackMachine creation should succeed") + + // Just fetching the object as v1alpha7 doesn't trigger + // validation failure, so we first fetch it and then + // patch the object with identical contents. The patch + // triggers a validation failure. + machine := &infrav1alpha7.OpenStackMachine{} //nolint: staticcheck + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: infrav1Machine.Name, Namespace: infrav1Machine.Namespace}, machine)).To(Succeed(), "OpenStackMachine fetch should succeed") + + setObjectGVK(machine) + machine.ManagedFields = nil + Expect(k8sClient.Patch(ctx, machine, client.Apply, client.FieldOwner("test"), client.ForceOwnership)).To(Succeed(), format.Object(machine, 4)) + }) + }) }) diff --git a/test/e2e/suites/apivalidations/suite_test.go b/test/e2e/suites/apivalidations/suite_test.go index 043b74284e..4000c0795c 100644 --- a/test/e2e/suites/apivalidations/suite_test.go +++ b/test/e2e/suites/apivalidations/suite_test.go @@ -161,3 +161,21 @@ func createNamespace() *corev1.Namespace { By(fmt.Sprintf("Using namespace %s", namespace.Name)) return &namespace } + +func createObj(obj client.Object) error { + err := k8sClient.Create(ctx, obj) + if err == nil { + DeferCleanup(func() error { + return k8sClient.Delete(ctx, obj) + }) + } + return err +} + +func setObjectGVK(obj runtime.Object) { + gvk, unversioned, err := testScheme.ObjectKinds(obj) + Expect(unversioned).To(BeFalse(), "Object is considered unversioned") + Expect(err).ToNot(HaveOccurred(), "Error fetching gvk for Object") + Expect(gvk).To(HaveLen(1), "Object should have only one gvk") + obj.GetObjectKind().SetGroupVersionKind(gvk[0]) +}