From da795873a4847e84ce1cbae95212368f5f650b69 Mon Sep 17 00:00:00 2001 From: Johann Gnaucke Date: Thu, 5 Jun 2025 12:48:52 +0200 Subject: [PATCH 1/2] Revert "Remove Extension Validation for Provider in ETCD-Backup" This reverts commit 13bbc31080c9e2ec736eb48a5d683938ea56d8fc. --- .../extension/required/runtime/add_test.go | 1 + pkg/utils/gardener/operator/garden.go | 9 +++------ pkg/utils/gardener/operator/garden_test.go | 5 ++++- .../extension/required/runtime/runtime_test.go | 13 +++++++++++-- 4 files changed, 19 insertions(+), 9 deletions(-) diff --git a/pkg/operator/controller/extension/required/runtime/add_test.go b/pkg/operator/controller/extension/required/runtime/add_test.go index 59ed6891ed5..69c64185ca4 100644 --- a/pkg/operator/controller/extension/required/runtime/add_test.go +++ b/pkg/operator/controller/extension/required/runtime/add_test.go @@ -122,6 +122,7 @@ var _ = Describe("Add", func() { It("should return the expected extensions", func() { Expect(mapperFunc(ctx, garden)).To(ConsistOf( + Equal(reconcile.Request{NamespacedName: types.NamespacedName{Name: infraExtension.Name}}), Equal(reconcile.Request{NamespacedName: types.NamespacedName{Name: dnsExtension.Name}}), )) }) diff --git a/pkg/utils/gardener/operator/garden.go b/pkg/utils/gardener/operator/garden.go index 43aae3a8887..f6939ccf601 100644 --- a/pkg/utils/gardener/operator/garden.go +++ b/pkg/utils/gardener/operator/garden.go @@ -58,12 +58,9 @@ func IsServedByKubeAPIServer(resource string) bool { func ComputeRequiredExtensionsForGarden(garden *operatorv1alpha1.Garden, extensionList *operatorv1alpha1.ExtensionList) sets.Set[string] { requiredExtensions := sets.New[string]() - /* - // TODO: reapply validation when STACKITSKE-830 is resolved - if operatorv1alpha1helper.GetETCDMainBackup(garden) != nil { - requiredExtensions.Insert(gardener.ExtensionsID(extensionsv1alpha1.BackupBucketResource, garden.Spec.VirtualCluster.ETCD.Main.Backup.Provider)) - } - */ + if operatorv1alpha1helper.GetETCDMainBackup(garden) != nil { + requiredExtensions.Insert(gardener.ExtensionsID(extensionsv1alpha1.BackupBucketResource, garden.Spec.VirtualCluster.ETCD.Main.Backup.Provider)) + } for _, provider := range operatorv1alpha1helper.GetDNSProviders(garden) { requiredExtensions.Insert(gardener.ExtensionsID(extensionsv1alpha1.DNSRecordResource, provider.Type)) diff --git a/pkg/utils/gardener/operator/garden_test.go b/pkg/utils/gardener/operator/garden_test.go index b3a61e2b037..b81a70b2286 100644 --- a/pkg/utils/gardener/operator/garden_test.go +++ b/pkg/utils/gardener/operator/garden_test.go @@ -100,7 +100,9 @@ var _ = Describe("Garden", func() { }, } - Expect(ComputeRequiredExtensionsForGarden(garden, extensionList).UnsortedList()).To(BeEmpty()) + Expect(ComputeRequiredExtensionsForGarden(garden, extensionList).UnsortedList()).To(ConsistOf( + "BackupBucket/local-infrastructure", + )) }) It("should return required DNSRecord extension types", func() { @@ -159,6 +161,7 @@ var _ = Describe("Garden", func() { } Expect(ComputeRequiredExtensionsForGarden(garden, extensionList).UnsortedList()).To(ConsistOf( + "BackupBucket/local-infrastructure", "DNSRecord/local-dns", "Extension/local-extension-1", "Extension/local-extension-2", diff --git a/test/integration/operator/extension/required/runtime/runtime_test.go b/test/integration/operator/extension/required/runtime/runtime_test.go index 16e140c6299..1c4d28ccfdc 100644 --- a/test/integration/operator/extension/required/runtime/runtime_test.go +++ b/test/integration/operator/extension/required/runtime/runtime_test.go @@ -189,10 +189,10 @@ var _ = Describe("Extension Required Runtime controller tests", Ordered, func() It("should report extensions as required after garden was created", func() { Expect(testClient.Create(ctx, garden)).To(Succeed()) - for _, ext := range []client.Object{dnsExtension} { + for _, ext := range []client.Object{providerExtension, dnsExtension} { Eventually(func(g Gomega) []gardencorev1beta1.Condition { g.Expect(testClient.Get(ctx, client.ObjectKeyFromObject(ext), ext)).To(Succeed()) - return dnsExtension.Status.Conditions + return providerExtension.Status.Conditions }).Should(ContainCondition( OfType(operatorv1alpha1.ExtensionRequiredRuntime), WithStatus(gardencorev1beta1.ConditionTrue), @@ -300,6 +300,15 @@ var _ = Describe("Extension Required Runtime controller tests", Ordered, func() WithStatus(gardencorev1beta1.ConditionFalse), WithReason("ExtensionNotRequired"), )) + + Consistently(func(g Gomega) []gardencorev1beta1.Condition { + g.Expect(testClient.Get(ctx, client.ObjectKeyFromObject(providerExtension), providerExtension)).To(Succeed()) + return providerExtension.Status.Conditions + }).Should(ContainCondition( + OfType(operatorv1alpha1.ExtensionRequiredRuntime), + WithStatus(gardencorev1beta1.ConditionTrue), + WithReason("ExtensionRequired"), + )) }) It("should report provider extension as not required during garden deletion after backupbucket is gone", func() { From 8b5ce634b04352ddadbff944827ecf764d901530 Mon Sep 17 00:00:00 2001 From: Johann Gnaucke Date: Thu, 5 Jun 2025 13:36:18 +0200 Subject: [PATCH 2/2] Allow virtual cluster ETCd bucket name to be mutated This can be dropped with G/G v1.121 --- .../operator/v1alpha1/validation/garden.go | 5 +- .../v1alpha1/validation/garden_test.go | 86 ++++++++++--------- 2 files changed, 48 insertions(+), 43 deletions(-) diff --git a/pkg/apis/operator/v1alpha1/validation/garden.go b/pkg/apis/operator/v1alpha1/validation/garden.go index 1fee22ae961..a4bb4347f5b 100644 --- a/pkg/apis/operator/v1alpha1/validation/garden.go +++ b/pkg/apis/operator/v1alpha1/validation/garden.go @@ -137,9 +137,10 @@ func validateVirtualClusterUpdate(oldGarden, newGarden *operatorv1alpha1.Garden) if oldVirtualCluster.ETCD != nil && oldVirtualCluster.ETCD.Main != nil && oldVirtualCluster.ETCD.Main.Backup != nil && newVirtualCluster.ETCD != nil && newVirtualCluster.ETCD.Main != nil { fldBackup := fldPath.Child("etcd", "main", "backup") - if newVirtualCluster.ETCD.Main.Backup != nil { + // TODO: reapply validation when https://github.com/stackitcloud/ske-base/pull/3088 is rolled out + /*if newVirtualCluster.ETCD.Main.Backup != nil { allErrs = append(allErrs, apivalidation.ValidateImmutableField(oldVirtualCluster.ETCD.Main.Backup.BucketName, newVirtualCluster.ETCD.Main.Backup.BucketName, fldBackup.Child("bucketName"))...) - } + }*/ if newVirtualCluster.ETCD.Main.Backup == nil { allErrs = append(allErrs, field.Forbidden(fldBackup, "backup must not be deactivated if it was set before")) } diff --git a/pkg/apis/operator/v1alpha1/validation/garden_test.go b/pkg/apis/operator/v1alpha1/validation/garden_test.go index 79240a281f6..29487f647c4 100644 --- a/pkg/apis/operator/v1alpha1/validation/garden_test.go +++ b/pkg/apis/operator/v1alpha1/validation/garden_test.go @@ -2459,54 +2459,58 @@ var _ = Describe("Validation Tests", func() { }) Context("ETCD", func() { - It("should not be possible to set the backup bucket name if it was unset initially", func() { - oldGarden.Spec.VirtualCluster.ETCD = &operatorv1alpha1.ETCD{ - Main: &operatorv1alpha1.ETCDMain{ - Backup: &operatorv1alpha1.Backup{ - Provider: "foo-provider", - ProviderConfig: &runtime.RawExtension{ - Raw: []byte(`{"foo":"bar"}`), + /* + It("should not be possible to set the backup bucket name if it was unset initially", func() { + oldGarden.Spec.VirtualCluster.ETCD = &operatorv1alpha1.ETCD{ + Main: &operatorv1alpha1.ETCDMain{ + Backup: &operatorv1alpha1.Backup{ + Provider: "foo-provider", + ProviderConfig: &runtime.RawExtension{ + Raw: []byte(`{"foo":"bar"}`), + }, }, }, - }, - } - newGarden.Spec.VirtualCluster.ETCD = &operatorv1alpha1.ETCD{ - Main: &operatorv1alpha1.ETCDMain{ - Backup: &operatorv1alpha1.Backup{ - BucketName: ptr.To("foo-bucket"), - Provider: "foo-provider", + } + newGarden.Spec.VirtualCluster.ETCD = &operatorv1alpha1.ETCD{ + Main: &operatorv1alpha1.ETCDMain{ + Backup: &operatorv1alpha1.Backup{ + BucketName: ptr.To("foo-bucket"), + Provider: "foo-provider", + }, }, - }, - } - - Expect(ValidateGardenUpdate(oldGarden, newGarden, extensions)).To(ContainElement(PointTo(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(field.ErrorTypeInvalid), - "Field": Equal("spec.virtualCluster.etcd.main.backup.bucketName"), - })))) - }) + } - It("should not be possible to delete the backup bucket name if it was set initially", func() { - oldGarden.Spec.VirtualCluster.ETCD = &operatorv1alpha1.ETCD{ - Main: &operatorv1alpha1.ETCDMain{ - Backup: &operatorv1alpha1.Backup{ - Provider: "foo-provider", - BucketName: ptr.To("foo-bucket"), + Expect(ValidateGardenUpdate(oldGarden, newGarden, extensions)).To(ContainElement(PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeInvalid), + "Field": Equal("spec.virtualCluster.etcd.main.backup.bucketName"), + })))) + }) + */ + + /* + It("should not be possible to delete the backup bucket name if it was set initially", func() { + oldGarden.Spec.VirtualCluster.ETCD = &operatorv1alpha1.ETCD{ + Main: &operatorv1alpha1.ETCDMain{ + Backup: &operatorv1alpha1.Backup{ + Provider: "foo-provider", + BucketName: ptr.To("foo-bucket"), + }, }, - }, - } - newGarden.Spec.VirtualCluster.ETCD = &operatorv1alpha1.ETCD{ - Main: &operatorv1alpha1.ETCDMain{ - Backup: &operatorv1alpha1.Backup{ - Provider: "foo-provider", + } + newGarden.Spec.VirtualCluster.ETCD = &operatorv1alpha1.ETCD{ + Main: &operatorv1alpha1.ETCDMain{ + Backup: &operatorv1alpha1.Backup{ + Provider: "foo-provider", + }, }, - }, - } + } - Expect(ValidateGardenUpdate(oldGarden, newGarden, extensions)).To(ContainElement(PointTo(MatchFields(IgnoreExtras, Fields{ - "Type": Equal(field.ErrorTypeInvalid), - "Field": Equal("spec.virtualCluster.etcd.main.backup.bucketName"), - })))) - }) + Expect(ValidateGardenUpdate(oldGarden, newGarden, extensions)).To(ContainElement(PointTo(MatchFields(IgnoreExtras, Fields{ + "Type": Equal(field.ErrorTypeInvalid), + "Field": Equal("spec.virtualCluster.etcd.main.backup.bucketName"), + })))) + }) + */ It("should not be possible to delete the backup if it was set initially", func() { oldGarden.Spec.VirtualCluster.ETCD = &operatorv1alpha1.ETCD{