feat(controller): added nameOverride to buckets (#111)

api/v1alpha1/bucket_types.go

@@ -41,7 +41,7 @@ type ExistingBucketSpec struct {
 
 // BucketSpec defines the desired state of Bucket
 //
-// +kubebuilder:validation:XValidation:rule="(has(self.name) && size(self.name) > 0) != has(self.existingBucket)",message="exactly one of spec.name or spec.existingBucket must be set"
+// +kubebuilder:validation:ExactlyOneOf=name;nameOverride;existingBucket
 // +kubebuilder:validation:XValidation:rule="!has(self.existingBucket) || !has(oldSelf.existingBucket) || self.existingBucket.name == oldSelf.existingBucket.name",message="spec.existingBucket.name is immutable"
 type BucketSpec struct {
 	// Name is the desired bucket name.
@@ -54,8 +54,20 @@ type BucketSpec struct {
 	// +optional
 	Name string `json:"name,omitempty"`
 
+	// NameOverride is the exact global alias assigned to a newly created Garage bucket.
+	// Mutually exclusive with Name and ExistingBucket.
+	//
+	// The controller does not adopt an existing bucket with this alias. Once the bucket ID
+	// is recorded in status, the controller will not recreate a missing bucket.
+	//
+	// +kubebuilder:validation:MinLength=3
+	// +kubebuilder:validation:MaxLength=63
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="spec.nameOverride is immutable"
+	// +optional
+	NameOverride string `json:"nameOverride,omitempty"`
+
 	// ExistingBucket identifies a pre-existing Garage bucket to import or reclaim.
-	// Mutually exclusive with Name.
+	// Mutually exclusive with Name and NameOverride.
 	// +optional
 	ExistingBucket *ExistingBucketSpec `json:"existingBucket,omitempty"`
 

config/crd/bases/garage.getclustered.net_buckets.yaml

@@ -54,7 +54,7 @@ spec:
               existingBucket:
                 description: |-
                   ExistingBucket identifies a pre-existing Garage bucket to import or reclaim.
-                  Mutually exclusive with Name.
+                  Mutually exclusive with Name and NameOverride.
                 properties:
                   name:
                     description: Name is the exact bucket name as it exists in Garage.
@@ -99,13 +99,28 @@ spec:
                 x-kubernetes-validations:
                 - message: spec.name is immutable
                   rule: self == oldSelf
+              nameOverride:
+                description: |-
+                  NameOverride is the exact global alias assigned to a newly created Garage bucket.
+                  Mutually exclusive with Name and ExistingBucket.
+
+                  The controller does not adopt an existing bucket with this alias. Once the bucket ID
+                  is recorded in status, the controller will not recreate a missing bucket.
+                maxLength: 63
+                minLength: 3
+                type: string
+                x-kubernetes-validations:
+                - message: spec.nameOverride is immutable
+                  rule: self == oldSelf
             type: object
             x-kubernetes-validations:
-            - message: exactly one of spec.name or spec.existingBucket must be set
-              rule: (has(self.name) && size(self.name) > 0) != has(self.existingBucket)
             - message: spec.existingBucket.name is immutable
               rule: '!has(self.existingBucket) || !has(oldSelf.existingBucket) ||
                 self.existingBucket.name == oldSelf.existingBucket.name'
+            - message: exactly one of the fields in [name nameOverride existingBucket]
+                must be set
+              rule: '[has(self.name),has(self.nameOverride),has(self.existingBucket)].filter(x,x==true).size()
+                == 1'
           status:
             description: status defines the observed state of Bucket
             properties:

internal/controller/bucket_controller.go

@@ -152,7 +152,11 @@ func (r *BucketReconciler) reconcileBucket(ctx context.Context, bucket *garagev1
 			}, nil
 		}
 	} else {
-		s3Bucket, alias, err = r.resolveNewBucket(ctx, bucket)
+		if bucket.Spec.NameOverride != "" {
+			s3Bucket, alias, err = r.resolveNewNameOverrideBucket(ctx, bucket)
+		} else {
+			s3Bucket, alias, err = r.resolveNewBucket(ctx, bucket)
+		}
 		if err != nil {
 			return ctrl.Result{}, err
 		}
@@ -315,6 +319,47 @@ func (r *BucketReconciler) resolveNewBucket(ctx context.Context, bucket *garagev
 	return s3Bucket, alias, nil
 }
 
+func (r *BucketReconciler) resolveNewNameOverrideBucket(ctx context.Context, bucket *garagev1alpha1.Bucket) (s3.Bucket, string, error) {
+	alias := bucket.Spec.NameOverride
+	s3Bucket, err := r.bucket.Get(ctx, alias)
+	if err != nil {
+		if !errors.Is(err, s3.ErrResourceNotFound) {
+			markBucketNotReady(bucket, "UnknownState", "S3 API error: %v", err)
+			return s3.Bucket{}, "", fmt.Errorf("retrieving existing bucket: %w", err)
+		}
+
+		if bucket.Status.BucketID != "" {
+			markBucketNotReady(bucket, "BucketMissing", "Bucket %q is missing: it has been deleted and will not be recreated", alias)
+			r.recorder.Eventf(bucket, corev1.EventTypeWarning, ReasonBucketMissing, "Bucket %q is missing: it has been deleted and will not be recreated", alias)
+			return s3.Bucket{}, "", fmt.Errorf("bucket %q is missing: it has been deleted and will not be recreated", alias)
+		}
+
+		s3Bucket, err = r.bucket.Create(ctx, alias)
+		if err != nil {
+			markBucketNotReady(bucket, "CreateFailed", "failed to create bucket '%s': %v", alias, err)
+			r.recorder.Eventf(bucket, corev1.EventTypeWarning, ReasonBucketCreateFailed, "failed to create Garage bucket %q: %v", alias, err)
+			return s3.Bucket{}, "", fmt.Errorf("create new bucket: %w", err)
+		}
+
+		r.recorder.Eventf(bucket, corev1.EventTypeNormal, ReasonBucketCreated, "Created Garage bucket %q", alias)
+		return s3Bucket, alias, nil
+	}
+
+	if bucket.Status.BucketID == "" {
+		markBucketNotReady(bucket, "CreateFailed", "Failed to create bucket %q: bucket already exists. Use spec.existingBucket to adopt it", alias)
+		r.recorder.Eventf(bucket, corev1.EventTypeWarning, ReasonBucketCreateFailed, "Failed to create bucket %q: bucket already exists. Use spec.existingBucket to adopt it", alias)
+		return s3.Bucket{}, "", fmt.Errorf("failed to create bucket %q: bucket already exists. Use spec.existingBucket to adopt it", alias)
+	}
+
+	if bucket.Status.BucketID != s3Bucket.ID {
+		markBucketNotReady(bucket, "BucketMissing", "Bucket %q is missing: its bucket ID changed and it will not be adopted", alias)
+		r.recorder.Eventf(bucket, corev1.EventTypeWarning, ReasonBucketMissing, "Bucket %q is missing: its bucket ID changed and it will not be adopted", alias)
+		return s3.Bucket{}, "", fmt.Errorf("bucket %q is missing: its bucket ID changed and it will not be adopted", alias)
+	}
+
+	return s3Bucket, alias, nil
+}
+
 func (r *BucketReconciler) resolveExistingBucket(ctx context.Context, bucket *garagev1alpha1.Bucket) (s3.Bucket, string, error) {
 	spec := bucket.Spec.ExistingBucket
 

internal/controller/bucket_controller_test.go

@@ -102,6 +102,49 @@ var _ = Describe("Bucket Controller", func() {
 			}).Should(Succeed())
 		})
 
+		It("creates an external S3 bucket matching spec.nameOverride", func() {
+			By("creating a Bucket custom resource")
+			bucket := newBucket(namespace)
+			bucket.Spec.Name = ""
+			bucket.Spec.NameOverride = fixture.RandAlpha(8)
+			Expect(k8sClient.Create(ctx, &bucket)).To(Succeed())
+
+			By("reconciling")
+			s3Fake := newS3APIFake()
+			s3Endpoint := "https://foo.bar:3456/baz"
+			controllerReconciler := NewBucketReconciler(k8sClient, k8sClient.Scheme(), s3Fake, s3Endpoint, nil, record.NewFakeRecorder(10))
+
+			_, err := controllerReconciler.Reconcile(ctx,
+				reconcile.Request{NamespacedName: namespacedName(bucket.ObjectMeta)})
+			Expect(err).ShouldNot(HaveOccurred())
+
+			By("waiting for external bucket to be provisioned")
+			Eventually(func(g Gomega) {
+				g.Expect(k8sClient.Get(ctx, namespacedName(bucket.ObjectMeta), &bucket)).To(Succeed())
+				g.Expect(checkCondition(bucket.Status.Conditions, Ready, metav1.ConditionTrue)).To(Succeed())
+				bucketCond := meta.FindStatusCondition(bucket.Status.Conditions, Ready)
+				g.Expect(bucketCond.ObservedGeneration).To(Equal(bucket.Generation))
+			}).Should(Succeed())
+
+			By("retrieving bucket with name override")
+			expectedName := bucket.Spec.NameOverride
+			created, err := s3Fake.Get(ctx, expectedName)
+			Expect(err).ToNot(HaveOccurred(), "bucket should exist: %s", expectedName)
+			Expect(created.GlobalAliases).To(ContainElement(expectedName))
+
+			By("creating a ConfigMap with bucket details")
+			expectedCMName := bucket.Name
+			Eventually(func(g Gomega) {
+				var configmap corev1.ConfigMap
+				g.Expect(k8sClient.Get(ctx,
+					types.NamespacedName{Namespace: namespace, Name: expectedCMName},
+					&configmap)).To(Succeed())
+
+				g.Expect(configmap.Data[configMapKeyBucketName]).To(Equal(bucket.Spec.NameOverride))
+				g.Expect(configmap.Data[configMapKeyEndpoint]).To(Equal(s3Endpoint))
+			}).Should(Succeed())
+		})
+
 		It("should create ConfigMap with connection details", func() {
 			By("creating the bucket")
 			resource := newBucket(namespace)
@@ -259,6 +302,52 @@ var _ = Describe("Bucket Controller", func() {
 			Consistently(rec.Events).ShouldNot(Receive())
 		})
 
+		It("allows a bound nameOverride bucket", func() {
+			By("creating a Bucket resource")
+			bucket := newBucket(namespace)
+			bucket.Spec.Name = ""
+			bucket.Spec.NameOverride = fixture.RandAlpha(63)
+			Expect(k8sClient.Create(ctx, &bucket)).To(Succeed())
+			bucket.Status.BucketID = "existing-id"
+			Expect(k8sClient.Status().Update(ctx, &bucket)).To(Succeed())
+
+			By("pre-seeding existing bucket in the Garage instance")
+			rec := record.NewFakeRecorder(10)
+			s3Fake := newS3APIFake()
+			alias := bucket.Spec.NameOverride
+			s3Fake.buckets["existing-id"] = s3.Bucket{ID: "existing-id", GlobalAliases: []string{alias}}
+			sut := NewBucketReconciler(k8sClient, k8sClient.Scheme(), s3Fake, "https://s3.test:9001", nil, rec)
+
+			By("reconciling")
+			_, err := sut.Reconcile(ctx, reconcile.Request{NamespacedName: namespacedName(bucket.ObjectMeta)})
+			Expect(err).ToNot(HaveOccurred())
+
+			By("asserting no events were emitted")
+			Consistently(rec.Events).ShouldNot(Receive())
+		})
+
+		It("rejects an unbound nameOverride bucket that already exists", func() {
+			By("creating a Bucket resource")
+			bucket := newBucket(namespace)
+			bucket.Spec.Name = ""
+			bucket.Spec.NameOverride = fixture.RandAlpha(8)
+			Expect(k8sClient.Create(ctx, &bucket)).To(Succeed())
+
+			By("pre-seeding existing bucket in the Garage instance")
+			rec := record.NewFakeRecorder(10)
+			s3Fake := newS3APIFake()
+			alias := bucket.Spec.NameOverride
+			s3Fake.buckets["existing-id"] = s3.Bucket{ID: "existing-id", GlobalAliases: []string{alias}}
+			sut := NewBucketReconciler(k8sClient, k8sClient.Scheme(), s3Fake, "https://s3.test:9001", nil, rec)
+
+			By("reconciling")
+			_, err := sut.Reconcile(ctx, reconcile.Request{NamespacedName: namespacedName(bucket.ObjectMeta)})
+			By("asserting failure err was passed and event was emitted")
+			Expect(err).To(HaveOccurred())
+			Expect(err).Should(MatchError(ContainSubstring("failed")))
+			Eventually(rec.Events).Should(Receive(ContainSubstring("Warning BucketCreateFailed")))
+		})
+
 		It("should emit BucketCreated exactly once across several reconciles", func() {
 			By("creating a Bucket")
 			bucket := newBucket(namespace)
@@ -301,6 +390,52 @@ var _ = Describe("Bucket Controller", func() {
 			Eventually(rec.Events).Should(Receive(ContainSubstring("Warning BucketCreateFailed")))
 		})
 
+		It("reports a nameOverride bucket with a changed ID as missing", func() {
+			By("creating a Bucket resource")
+			bucket := newBucket(namespace)
+			bucket.Spec.Name = ""
+			bucket.Spec.NameOverride = fixture.RandAlpha(63)
+			Expect(k8sClient.Create(ctx, &bucket)).To(Succeed())
+			bucket.Status.BucketID = "old-id"
+			Expect(k8sClient.Status().Update(ctx, &bucket)).To(Succeed())
+
+			By("pre-seeding existing bucket in the Garage instance")
+			rec := record.NewFakeRecorder(10)
+			s3Fake := newS3APIFake()
+			alias := bucket.Spec.NameOverride
+			s3Fake.buckets["existing-id"] = s3.Bucket{ID: "existing-id", GlobalAliases: []string{alias}}
+			sut := NewBucketReconciler(k8sClient, k8sClient.Scheme(), s3Fake, "https://s3.test:9001", nil, rec)
+
+			By("reconciling")
+			_, err := sut.Reconcile(ctx, reconcile.Request{NamespacedName: namespacedName(bucket.ObjectMeta)})
+			By("asserting failure err was passed and event was emitted")
+			Expect(err).To(HaveOccurred())
+			Expect(err).Should(MatchError(ContainSubstring("missing")))
+			Eventually(rec.Events).Should(Receive(ContainSubstring("missing")))
+		})
+
+		It("reports a missing bound nameOverride bucket", func() {
+			By("creating a Bucket resource")
+			bucket := newBucket(namespace)
+			bucket.Spec.Name = ""
+			bucket.Spec.NameOverride = fixture.RandAlpha(63)
+			Expect(k8sClient.Create(ctx, &bucket)).To(Succeed())
+			bucket.Status.BucketID = "old-id"
+			Expect(k8sClient.Status().Update(ctx, &bucket)).To(Succeed())
+
+			By("creating controller")
+			rec := record.NewFakeRecorder(10)
+			s3Fake := newS3APIFake()
+			sut := NewBucketReconciler(k8sClient, k8sClient.Scheme(), s3Fake, "https://s3.test:9001", nil, rec)
+
+			By("reconciling")
+			_, err := sut.Reconcile(ctx, reconcile.Request{NamespacedName: namespacedName(bucket.ObjectMeta)})
+			By("asserting failure err was passed and event was emitted")
+			Expect(err).To(HaveOccurred())
+			Expect(err).Should(MatchError(ContainSubstring("missing")))
+			Eventually(rec.Events).Should(Receive(ContainSubstring("missing")))
+		})
+
 		It("recovers from conflict once configMapName is set", func() {
 			conflictingName := "foobar"
 			originalData := map[string]string{"foo": "bar"}
@@ -500,7 +635,7 @@ var _ = Describe("Bucket Controller", func() {
 			Entry("starts with hyphen", "-invalid", false),
 		)
 
-		DescribeTable("validates existingBucket field combinations",
+		DescribeTable("validates bucket source field combinations",
 			func(spec garagev1alpha1.BucketSpec, isValid bool) {
 				resource := garagev1alpha1.Bucket{
 					ObjectMeta: metav1.ObjectMeta{
@@ -531,7 +666,11 @@ var _ = Describe("Bucket Controller", func() {
 					}},
 				true,
 			),
-			Entry("neither name nor existingBucket",
+			Entry("nameOverride only",
+				garagev1alpha1.BucketSpec{NameOverride: fixture.RandAlpha(8)},
+				true,
+			),
+			Entry("no bucket source",
 				garagev1alpha1.BucketSpec{},
 				false,
 			),
@@ -544,6 +683,34 @@ var _ = Describe("Bucket Controller", func() {
 					},
 				},
 				false),
+			Entry("both name and nameOverride",
+				garagev1alpha1.BucketSpec{
+					Name:         fixture.RandAlpha(8),
+					NameOverride: fixture.RandAlpha(8),
+				},
+				false,
+			),
+			Entry("both nameOverride and existingBucket",
+				garagev1alpha1.BucketSpec{
+					NameOverride: fixture.RandAlpha(8),
+					ExistingBucket: &garagev1alpha1.ExistingBucketSpec{
+						Name:           fixture.RandAlpha(8),
+						OwnerKeySecret: fixture.RandAlpha(6),
+					},
+				},
+				false,
+			),
+			Entry("all bucket sources",
+				garagev1alpha1.BucketSpec{
+					Name:         fixture.RandAlpha(8),
+					NameOverride: fixture.RandAlpha(8),
+					ExistingBucket: &garagev1alpha1.ExistingBucketSpec{
+						Name:           fixture.RandAlpha(8),
+						OwnerKeySecret: fixture.RandAlpha(6),
+					},
+				},
+				false,
+			),
 			Entry("existingBucket without ownerKeySecret",
 				garagev1alpha1.BucketSpec{
 					ExistingBucket: &garagev1alpha1.ExistingBucketSpec{