diff --git a/packages/system/objectstorage-controller/images/objectstorage/Dockerfile b/packages/system/objectstorage-controller/images/objectstorage/Dockerfile index 01dafe83d..260446c30 100644 --- a/packages/system/objectstorage-controller/images/objectstorage/Dockerfile +++ b/packages/system/objectstorage-controller/images/objectstorage/Dockerfile @@ -1,16 +1,13 @@ # syntax=docker/dockerfile:1.2 FROM alpine AS source -ARG COMMIT_REF=c2f6e651eb58880627ccddafe0d84fb36e03a780 -RUN apk add --no-cache curl tar git +ARG COMMIT_REF=v0.2.2 +RUN apk add --no-cache curl tar WORKDIR /src RUN curl -sSL https://github.com/kubernetes-sigs/container-object-storage-interface/archive/${COMMIT_REF}.tar.gz \ | tar -xz --strip-components=1 -COPY patches /patches -RUN git apply /patches/*.diff - FROM --platform=$BUILDPLATFORM docker.io/golang:1.24 AS builder ARG TARGETOS ARG TARGETARCH diff --git a/packages/system/objectstorage-controller/images/objectstorage/patches/89-reconciliation.diff b/packages/system/objectstorage-controller/images/objectstorage/patches/89-reconciliation.diff deleted file mode 100644 index 8a3f11ab3..000000000 --- a/packages/system/objectstorage-controller/images/objectstorage/patches/89-reconciliation.diff +++ /dev/null @@ -1,398 +0,0 @@ -diff --git a/controller/pkg/bucketclaim/bucketclaim.go b/controller/pkg/bucketclaim/bucketclaim.go -index 2f4d565e..8ad7baed 100644 ---- a/controller/pkg/bucketclaim/bucketclaim.go -+++ b/controller/pkg/bucketclaim/bucketclaim.go -@@ -32,6 +32,10 @@ func NewBucketClaimListener() *BucketClaimListener { - - // Add creates a bucket in response to a bucketClaim - func (b *BucketClaimListener) Add(ctx context.Context, bucketClaim *v1alpha1.BucketClaim) error { -+ if !bucketClaim.GetDeletionTimestamp().IsZero() { -+ return b.handleDeletion(ctx, bucketClaim) -+ } -+ - klog.V(3).InfoS("Add BucketClaim", - "name", bucketClaim.ObjectMeta.Name, - "ns", bucketClaim.ObjectMeta.Namespace, -@@ -76,18 +80,11 @@ func (b *BucketClaimListener) Update(ctx context.Context, old, new *v1alpha1.Buc - bucketClaim := new.DeepCopy() - - if !new.GetDeletionTimestamp().IsZero() { -- if controllerutil.ContainsFinalizer(bucketClaim, util.BucketClaimFinalizer) { -- bucketName := bucketClaim.Status.BucketName -- err := b.buckets().Delete(ctx, bucketName, metav1.DeleteOptions{}) -- if err != nil { -- klog.V(3).ErrorS(err, "Error deleting bucket", -- "bucket", bucketName, -- "bucketClaim", bucketClaim.ObjectMeta.Name) -- return b.recordError(bucketClaim, v1.EventTypeWarning, v1alpha1.FailedDeleteBucket, err) -- } -- -- klog.V(5).Infof("Successfully deleted bucket: %s from bucketClaim: %s", bucketName, bucketClaim.ObjectMeta.Name) -- } -+ return b.handleDeletion(ctx, bucketClaim) -+ } -+ -+ if err := b.Add(ctx, bucketClaim); err != nil { -+ return err - } - - klog.V(3).InfoS("Update BucketClaim success", -@@ -96,6 +93,27 @@ func (b *BucketClaimListener) Update(ctx context.Context, old, new *v1alpha1.Buc - return nil - } - -+// handleDeletion processes the deletion of a bucketClaim. -+func (b *BucketClaimListener) handleDeletion(ctx context.Context, bucketClaim *v1alpha1.BucketClaim) error { -+ if !controllerutil.ContainsFinalizer(bucketClaim, util.BucketClaimFinalizer) { -+ return nil -+ } -+ -+ bucketName := bucketClaim.Status.BucketName -+ if bucketName != "" { -+ if err := b.buckets().Delete(ctx, bucketName, metav1.DeleteOptions{}); err != nil && !kubeerrors.IsNotFound(err) { -+ klog.V(3).ErrorS(err, "Error deleting bucket", -+ "bucket", bucketName, -+ "bucketClaim", bucketClaim.ObjectMeta.Name) -+ return b.recordError(bucketClaim, v1.EventTypeWarning, v1alpha1.FailedDeleteBucket, err) -+ } -+ klog.V(5).Infof("Successfully requested deletion of bucket: %s for bucketClaim: %s", -+ bucketName, bucketClaim.ObjectMeta.Name) -+ } -+ -+ return nil -+} -+ - // Delete processes a bucket for which bucket request is deleted - func (b *BucketClaimListener) Delete(ctx context.Context, bucketClaim *v1alpha1.BucketClaim) error { - klog.V(3).InfoS("Delete BucketClaim", -diff --git a/controller/pkg/bucketclaim/bucketclaim_test.go b/controller/pkg/bucketclaim/bucketclaim_test.go -index 284185b6..e2e2d3d2 100644 ---- a/controller/pkg/bucketclaim/bucketclaim_test.go -+++ b/controller/pkg/bucketclaim/bucketclaim_test.go -@@ -323,3 +323,32 @@ func TestRecordEvents(t *testing.T) { - func newEvent(eventType, reason, message string) string { - return fmt.Sprintf("%s %s %s", eventType, reason, message) - } -+ -+// Claim already marked for deletion must not create a bucket -+func TestAddDeletedBucketClaim(t *testing.T) { -+ ctx, cancel := context.WithCancel(context.Background()) -+ defer cancel() -+ -+ client := fakebucketclientset.NewSimpleClientset() -+ kubeClient := fakekubeclientset.NewSimpleClientset() -+ eventRecorder := record.NewFakeRecorder(3) -+ -+ listener := NewBucketClaimListener() -+ listener.InitializeKubeClient(kubeClient) -+ listener.InitializeBucketClient(client) -+ listener.InitializeEventRecorder(eventRecorder) -+ -+ _, _ = util.CreateBucketClass(ctx, client, &goldClass) -+ -+ claimToDelete := bucketClaim1.DeepCopy() -+ now := metav1.Now() -+ claimToDelete.ObjectMeta.DeletionTimestamp = &now -+ -+ if err := listener.Add(ctx, claimToDelete); err != nil { -+ t.Fatalf("Add returned error for deleted claim: %v", err) -+ } -+ -+ if bl := util.GetBuckets(ctx, client, 0); len(bl.Items) != 0 { -+ t.Fatalf("expected 0 buckets, got %d", len(bl.Items)) -+ } -+} -diff --git a/sidecar/pkg/bucket/bucket_controller.go b/sidecar/pkg/bucket/bucket_controller.go -index a934d0c5..bf8b5311 100644 ---- a/sidecar/pkg/bucket/bucket_controller.go -+++ b/sidecar/pkg/bucket/bucket_controller.go -@@ -68,6 +68,10 @@ func (b *BucketListener) Add(ctx context.Context, inputBucket *v1alpha1.Bucket) - - var err error - -+ if !bucket.GetDeletionTimestamp().IsZero() { -+ return b.handleDeletion(ctx, bucket) -+ } -+ - klog.V(3).InfoS("Add Bucket", - "name", bucket.ObjectMeta.Name) - -@@ -212,55 +216,60 @@ func (b *BucketListener) Update(ctx context.Context, old, new *v1alpha1.Bucket) - var err error - - if !bucket.GetDeletionTimestamp().IsZero() { -- if controllerutil.ContainsFinalizer(bucket, consts.BABucketFinalizer) { -- bucketClaimNs := bucket.Spec.BucketClaim.Namespace -- bucketClaimName := bucket.Spec.BucketClaim.Name -- bucketAccessList, err := b.bucketAccesses(bucketClaimNs).List(ctx, metav1.ListOptions{}) -- if err != nil { -- klog.V(3).ErrorS(err, "Error fetching BucketAccessList", -- "bucket", bucket.ObjectMeta.Name) -- return err -- } -- -- for _, bucketAccess := range bucketAccessList.Items { -- if strings.EqualFold(bucketAccess.Spec.BucketClaimName, bucketClaimName) { -- err = b.bucketAccesses(bucketClaimNs).Delete(ctx, bucketAccess.Name, metav1.DeleteOptions{}) -- if err != nil { -- klog.V(3).ErrorS(err, "Error deleting BucketAccess", -- "name", bucketAccess.Name, -- "bucket", bucket.ObjectMeta.Name) -- return err -- } -- } -- } -+ return b.handleDeletion(ctx, bucket) -+ } - -- klog.V(5).Infof("Successfully deleted dependent bucketAccess of bucket:%s", bucket.ObjectMeta.Name) -+ if err = b.Add(ctx, bucket); err != nil { -+ return err -+ } - -- controllerutil.RemoveFinalizer(bucket, consts.BABucketFinalizer) -- klog.V(5).Infof("Successfully removed finalizer: %s of bucket: %s", consts.BABucketFinalizer, bucket.ObjectMeta.Name) -- } -+ klog.V(3).InfoS("Update Bucket success", -+ "name", bucket.ObjectMeta.Name, -+ "ns", bucket.ObjectMeta.Namespace) -+ return nil -+} - -- if controllerutil.ContainsFinalizer(bucket, consts.BucketFinalizer) { -- err = b.deleteBucketOp(ctx, bucket) -- if err != nil { -- return b.recordError(bucket, v1.EventTypeWarning, v1alpha1.FailedDeleteBucket, err) -- } -+func (b *BucketListener) handleDeletion(ctx context.Context, bucket *v1alpha1.Bucket) error { -+ var err error - -- controllerutil.RemoveFinalizer(bucket, consts.BucketFinalizer) -- klog.V(5).Infof("Successfully removed finalizer: %s of bucket: %s", consts.BucketFinalizer, bucket.ObjectMeta.Name) -- } -+ if controllerutil.ContainsFinalizer(bucket, consts.BABucketFinalizer) { -+ bucketClaimNs := bucket.Spec.BucketClaim.Namespace -+ bucketClaimName := bucket.Spec.BucketClaim.Name - -- _, err = b.buckets().Update(ctx, bucket, metav1.UpdateOptions{}) -+ bucketAccessList, err := b.bucketAccesses(bucketClaimNs).List(ctx, metav1.ListOptions{}) - if err != nil { -- klog.V(3).ErrorS(err, "Error updating bucket after removing finalizers", -+ klog.V(3).ErrorS(err, "Error fetching BucketAccessList", - "bucket", bucket.ObjectMeta.Name) - return err - } -+ -+ for _, ba := range bucketAccessList.Items { -+ if strings.EqualFold(ba.Spec.BucketClaimName, bucketClaimName) { -+ if err = b.bucketAccesses(bucketClaimNs).Delete(ctx, ba.Name, metav1.DeleteOptions{}); err != nil { -+ klog.V(3).ErrorS(err, "Error deleting BucketAccess", -+ "name", ba.Name, -+ "bucket", bucket.ObjectMeta.Name) -+ return err -+ } -+ } -+ } -+ -+ controllerutil.RemoveFinalizer(bucket, consts.BABucketFinalizer) -+ } -+ -+ if controllerutil.ContainsFinalizer(bucket, consts.BucketFinalizer) { -+ if err = b.deleteBucketOp(ctx, bucket); err != nil { -+ return b.recordError(bucket, v1.EventTypeWarning, v1alpha1.FailedDeleteBucket, err) -+ } -+ controllerutil.RemoveFinalizer(bucket, consts.BucketFinalizer) -+ } -+ -+ if _, err = b.buckets().Update(ctx, bucket, metav1.UpdateOptions{}); err != nil { -+ klog.V(3).ErrorS(err, "Error updating bucket after removing finalizers", -+ "bucket", bucket.ObjectMeta.Name) -+ return err - } - -- klog.V(3).InfoS("Update Bucket success", -- "name", bucket.ObjectMeta.Name, -- "ns", bucket.ObjectMeta.Namespace) - return nil - } - -diff --git a/sidecar/pkg/bucket/bucket_controller_test.go b/sidecar/pkg/bucket/bucket_controller_test.go -index 9be6cc4a..ae63464e 100644 ---- a/sidecar/pkg/bucket/bucket_controller_test.go -+++ b/sidecar/pkg/bucket/bucket_controller_test.go -@@ -310,3 +310,43 @@ func TestRecordEvents(t *testing.T) { - func newEvent(eventType, reason, message string) string { - return fmt.Sprintf("%s %s %s", eventType, reason, message) - } -+ -+// TestAddDeletedBucket tests that the Add method does not call the driver -+func TestAddDeletedBucket(t *testing.T) { -+ driver := "driver1" -+ -+ mpc := struct{ fakespec.FakeProvisionerClient }{} -+ mpc.FakeDriverDeleteBucket = func( -+ _ context.Context, -+ _ *cosi.DriverDeleteBucketRequest, -+ _ ...grpc.CallOption, -+ ) (*cosi.DriverDeleteBucketResponse, error) { -+ t.Fatalf("driver should NOT be called from Add when object has DeletionTimestamp") -+ return nil, nil -+ } -+ -+ now := metav1.Now() -+ b := v1alpha1.Bucket{ -+ ObjectMeta: metav1.ObjectMeta{ -+ Name: "testbucket", -+ DeletionTimestamp: &now, -+ ResourceVersion: "1", -+ }, -+ Spec: v1alpha1.BucketSpec{ -+ DriverName: driver, -+ BucketClassName: "ignored", -+ }, -+ } -+ -+ client := fakebucketclientset.NewSimpleClientset(&b) -+ -+ bl := BucketListener{ -+ driverName: driver, -+ provisionerClient: &mpc, -+ } -+ bl.InitializeBucketClient(client) -+ -+ if err := bl.Add(context.TODO(), &b); err != nil { -+ t.Fatalf("Add returned error for deleted bucket: %v", err) -+ } -+} -diff --git a/sidecar/pkg/bucketaccess/bucketaccess_controller.go b/sidecar/pkg/bucketaccess/bucketaccess_controller.go -index c6d0ed07..dd18202f 100644 ---- a/sidecar/pkg/bucketaccess/bucketaccess_controller.go -+++ b/sidecar/pkg/bucketaccess/bucketaccess_controller.go -@@ -68,6 +68,12 @@ func NewBucketAccessListener(driverName string, client cosi.ProvisionerClient) * - func (bal *BucketAccessListener) Add(ctx context.Context, inputBucketAccess *v1alpha1.BucketAccess) error { - bucketAccess := inputBucketAccess.DeepCopy() - -+ if !bucketAccess.GetDeletionTimestamp().IsZero() { -+ klog.V(3).InfoS("BucketAccess has deletion timestamp, handling deletion", -+ "name", bucketAccess.ObjectMeta.Name) -+ return bal.deleteBucketAccessOp(ctx, bucketAccess) -+ } -+ - if bucketAccess.Status.AccessGranted && bucketAccess.Status.AccountID != "" { - klog.V(3).InfoS("BucketAccess already exists", bucketAccess.ObjectMeta.Name) - return nil -@@ -310,10 +316,13 @@ func (bal *BucketAccessListener) Update(ctx context.Context, old, new *v1alpha1. - - bucketAccess := new.DeepCopy() - if !bucketAccess.GetDeletionTimestamp().IsZero() { -- err := bal.deleteBucketAccessOp(ctx, bucketAccess) -- if err != nil { -+ if err := bal.deleteBucketAccessOp(ctx, bucketAccess); err != nil { - return bal.recordError(bucketAccess, v1.EventTypeWarning, v1alpha1.FailedRevokeAccess, err) - } -+ } else { -+ if err := bal.Add(ctx, bucketAccess); err != nil { -+ return bal.recordError(bucketAccess, v1.EventTypeWarning, v1alpha1.FailedGrantAccess, err) -+ } - } - - klog.V(3).InfoS("Update BucketAccess success", -diff --git a/sidecar/pkg/bucketaccess/bucketaccess_controller_test.go b/sidecar/pkg/bucketaccess/bucketaccess_controller_test.go -index 2371c81e..d8da44a2 100644 ---- a/sidecar/pkg/bucketaccess/bucketaccess_controller_test.go -+++ b/sidecar/pkg/bucketaccess/bucketaccess_controller_test.go -@@ -502,3 +502,94 @@ func TestRecordEvents(t *testing.T) { - func newEvent(eventType, reason, message string) string { - return fmt.Sprintf("%s %s %s", eventType, reason, message) - } -+ -+// TestAddDeletedBucketAccess tests that a deleted BucketAccess does not -+// trigger a call to the driver to grant access, and that no secrets are created. -+func TestAddDeletedBucketAccess(t *testing.T) { -+ driver := "driver" -+ baName := "bucketaccess-deleted" -+ ns := "testns" -+ -+ mpc := struct{ fakespec.FakeProvisionerClient }{} -+ mpc.FakeDriverGrantBucketAccess = func( -+ _ context.Context, -+ _ *cosi.DriverGrantBucketAccessRequest, -+ _ ...grpc.CallOption, -+ ) (*cosi.DriverGrantBucketAccessResponse, error) { -+ t.Fatalf("driver Grant should NOT be called on deleted BA") -+ return nil, nil -+ } -+ mpc.FakeDriverRevokeBucketAccess = func( -+ _ context.Context, -+ _ *cosi.DriverRevokeBucketAccessRequest, -+ _ ...grpc.CallOption, -+ ) (*cosi.DriverRevokeBucketAccessResponse, error) { -+ return &cosi.DriverRevokeBucketAccessResponse{}, nil -+ } -+ -+ // minimal stub objects just to satisfy look-ups inside delete-path -+ bac := &v1alpha1.BucketAccessClass{ -+ ObjectMeta: metav1.ObjectMeta{Name: "bac"}, -+ DriverName: driver, -+ } -+ claim := &v1alpha1.BucketClaim{ -+ ObjectMeta: metav1.ObjectMeta{Name: "claim", Namespace: ns}, -+ Status: v1alpha1.BucketClaimStatus{ -+ BucketReady: true, -+ BucketName: "bucket", -+ }, -+ } -+ bucket := &v1alpha1.Bucket{ -+ ObjectMeta: metav1.ObjectMeta{Name: "bucket"}, -+ Status: v1alpha1.BucketStatus{ -+ BucketReady: true, -+ BucketID: "id", -+ }, -+ } -+ -+ now := metav1.Now() -+ ba := &v1alpha1.BucketAccess{ -+ ObjectMeta: metav1.ObjectMeta{ -+ Name: baName, -+ Namespace: ns, -+ DeletionTimestamp: &now, -+ Finalizers: []string{consts.BAFinalizer}, -+ }, -+ Spec: v1alpha1.BucketAccessSpec{ -+ BucketClaimName: claim.Name, -+ BucketAccessClassName: bac.Name, -+ CredentialsSecretName: "creds", -+ }, -+ Status: v1alpha1.BucketAccessStatus{ -+ AccountID: "acc", -+ AccessGranted: true, -+ }, -+ } -+ -+ secret := &v1.Secret{ -+ ObjectMeta: metav1.ObjectMeta{ -+ Name: "creds", -+ Namespace: ns, -+ Finalizers: []string{consts.SecretFinalizer}, -+ }, -+ StringData: map[string]string{"dummy": "val"}, -+ } -+ -+ client := fakebucketclientset.NewSimpleClientset(bac, claim, bucket, ba) -+ kubeClient := fakekubeclientset.NewSimpleClientset(secret) -+ -+ bal := BucketAccessListener{ -+ driverName: driver, -+ provisionerClient: &mpc, -+ bucketClient: client, -+ kubeClient: kubeClient, -+ } -+ -+ if err := bal.Add(context.TODO(), ba); err != nil { -+ t.Fatalf("Add returned error for deleted BucketAccess: %v", err) -+ } -+ -+ if _, err := bal.secrets(ns).Get(context.TODO(), "creds", metav1.GetOptions{}); !kubeerrors.IsNotFound(err) { -+ t.Fatalf("secret was not cleaned up, err=%v", err) -+ } -+} diff --git a/packages/system/objectstorage-controller/images/objectstorage/patches/90-bucket-name.diff b/packages/system/objectstorage-controller/images/objectstorage/patches/90-bucket-name.diff deleted file mode 100644 index de2d2c6b3..000000000 --- a/packages/system/objectstorage-controller/images/objectstorage/patches/90-bucket-name.diff +++ /dev/null @@ -1,13 +0,0 @@ -diff --git a/controller/pkg/bucketclaim/bucketclaim.go b/controller/pkg/bucketclaim/bucketclaim.go -index 2f4d565e..2addcee8 100644 ---- a/controller/pkg/bucketclaim/bucketclaim.go -+++ b/controller/pkg/bucketclaim/bucketclaim.go -@@ -165,7 +165,7 @@ func (b *BucketClaimListener) provisionBucketClaimOperation(ctx context.Context, - return b.recordError(inputBucketClaim, v1.EventTypeWarning, v1alpha1.FailedCreateBucket, err) - } - -- bucketName = bucketClassName + string(bucketClaim.ObjectMeta.UID) -+ bucketName = fmt.Sprintf("bucket-%s", bucketClaim.ObjectMeta.UID) - - // create bucket - bucket := &v1alpha1.Bucket{}