Skip to content

[feat] Support LinodeObjectStorageBucket deletion if forceDeleteBucket is set #780

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jul 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions api/v1alpha2/linodeobjectstoragebucket_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ const (
ACLPublicRead ObjectStorageACL = "public-read"
ACLAuthenticatedRead ObjectStorageACL = "authenticated-read"
ACLPublicReadWrite ObjectStorageACL = "public-read-write"

BucketFinalizer = "linodeobjectstoragebucket.infrastructure.cluster.x-k8s.io"
)

// LinodeObjectStorageBucketSpec defines the desired state of LinodeObjectStorageBucket
Expand All @@ -55,6 +57,14 @@ type LinodeObjectStorageBucketSpec struct {
// If not supplied then the credentials of the controller will be used.
// +optional
CredentialsRef *corev1.SecretReference `json:"credentialsRef"`

// AccessKeyRef is a reference to a LinodeObjectStorageBucketKey for the bucket.
// +optional
AccessKeyRef *corev1.ObjectReference `json:"accessKeyRef"`

// ForceDeleteBucket enables the object storage bucket used to be deleted even if it contains objects.
// +optional
ForceDeleteBucket bool `json:"forceDeleteBucket,omitempty"`
}

// LinodeObjectStorageBucketStatus defines the observed state of LinodeObjectStorageBucket
Expand Down
5 changes: 5 additions & 0 deletions api/v1alpha2/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions clients/clients.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ type LinodeObjectStorageClient interface {
GetObjectStorageKey(ctx context.Context, keyID int) (*linodego.ObjectStorageKey, error)
CreateObjectStorageKey(ctx context.Context, opts linodego.ObjectStorageKeyCreateOptions) (*linodego.ObjectStorageKey, error)
DeleteObjectStorageKey(ctx context.Context, keyID int) error
DeleteObjectStorageBucket(ctx context.Context, regionID, label string) error
}

// LinodeDNSClient defines the methods that interact with Linode's Domains service.
Expand Down Expand Up @@ -128,6 +129,10 @@ type S3Client interface {
DeleteObject(ctx context.Context, params *s3.DeleteObjectInput, optFns ...func(*s3.Options)) (*s3.DeleteObjectOutput, error)
PutObject(ctx context.Context, params *s3.PutObjectInput, optFns ...func(*s3.Options)) (*s3.PutObjectOutput, error)
HeadObject(ctx context.Context, params *s3.HeadObjectInput, optFns ...func(*s3.Options)) (*s3.HeadObjectOutput, error)
GetBucketVersioning(ctx context.Context, params *s3.GetBucketVersioningInput, optFns ...func(*s3.Options)) (*s3.GetBucketVersioningOutput, error)
DeleteObjects(ctx context.Context, params *s3.DeleteObjectsInput, optFns ...func(*s3.Options)) (*s3.DeleteObjectsOutput, error)
ListObjectsV2(ctx context.Context, params *s3.ListObjectsV2Input, optFns ...func(*s3.Options)) (*s3.ListObjectsV2Output, error)
ListObjectVersions(ctx context.Context, params *s3.ListObjectVersionsInput, f ...func(*s3.Options)) (*s3.ListObjectVersionsOutput, error)
}

type S3PresignClient interface {
Expand Down
65 changes: 65 additions & 0 deletions cloud/scope/object_storage_bucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

"github.com/go-logr/logr"
"sigs.k8s.io/cluster-api/util/patch"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

infrav1alpha2 "github.com/linode/cluster-api-provider-linode/api/v1alpha2"
"github.com/linode/cluster-api-provider-linode/clients"
Expand Down Expand Up @@ -77,6 +79,16 @@
}, nil
}

// AddFinalizer adds a finalizer if not present and immediately patches the
// object to avoid any race conditions.
func (s *ObjectStorageBucketScope) AddFinalizer(ctx context.Context) error {
if controllerutil.AddFinalizer(s.Bucket, infrav1alpha2.BucketFinalizer) {
return s.Close(ctx)
}

return nil

Check warning on line 89 in cloud/scope/object_storage_bucket.go

View check run for this annotation

Codecov / codecov/patch

cloud/scope/object_storage_bucket.go#L89

Added line #L89 was not covered by tests
}

// PatchObject persists the object storage bucket configuration and status.
func (s *ObjectStorageBucketScope) PatchObject(ctx context.Context) error {
return s.PatchHelper.Patch(ctx, s.Bucket)
Expand All @@ -86,3 +98,56 @@
func (s *ObjectStorageBucketScope) Close(ctx context.Context) error {
return s.PatchObject(ctx)
}

// AddAccessKeyRefFinalizer adds a finalizer to the linodeobjectstoragekey referenced in spec.AccessKeyRef.
func (s *ObjectStorageBucketScope) AddAccessKeyRefFinalizer(ctx context.Context, finalizer string) error {
obj, err := s.getAccessKey(ctx)
if err != nil {
return err
}

controllerutil.AddFinalizer(obj, finalizer)
if err := s.Client.Update(ctx, obj); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

patch instead of update?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is following the same logic as https://github.com/linode/cluster-api-provider-linode/blob/main/cloud/scope/common.go#L188-L199 which uses Update but I'm not opposed to using Patch if that works better

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just worried about potential conflicts - since this is a different object

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

conflicts are already be handled via retry/requeue on the bucket reconciler but maybe switching to patch would help with that

return fmt.Errorf("add linodeobjectstoragekey finalizer %s/%s: %w", s.Bucket.Spec.AccessKeyRef.Namespace, s.Bucket.Spec.AccessKeyRef.Name, err)
}

Check warning on line 112 in cloud/scope/object_storage_bucket.go

View check run for this annotation

Codecov / codecov/patch

cloud/scope/object_storage_bucket.go#L111-L112

Added lines #L111 - L112 were not covered by tests

return nil
}

// RemoveAccessKeyRefFinalizer removes a finalizer from the linodeobjectstoragekey referenced in spec.AccessKeyRef.
func (s *ObjectStorageBucketScope) RemoveAccessKeyRefFinalizer(ctx context.Context, finalizer string) error {
obj, err := s.getAccessKey(ctx)
if err != nil {
return err
}

controllerutil.RemoveFinalizer(obj, finalizer)
if err := s.Client.Update(ctx, obj); err != nil {
return fmt.Errorf("remove linodeobjectstoragekey finalizer %s/%s: %w", s.Bucket.Spec.AccessKeyRef.Namespace, s.Bucket.Spec.AccessKeyRef.Name, err)
}

Check warning on line 127 in cloud/scope/object_storage_bucket.go

View check run for this annotation

Codecov / codecov/patch

cloud/scope/object_storage_bucket.go#L126-L127

Added lines #L126 - L127 were not covered by tests

return nil
}

func (s *ObjectStorageBucketScope) getAccessKey(ctx context.Context) (*infrav1alpha2.LinodeObjectStorageKey, error) {
if s.Bucket.Spec.AccessKeyRef == nil {
return nil, fmt.Errorf("accessKeyRef is nil for bucket %s", s.Bucket.Name)
}

objKeyNamespace := s.Bucket.Spec.AccessKeyRef.Namespace
if s.Bucket.Spec.AccessKeyRef.Namespace == "" {
objKeyNamespace = s.Bucket.Namespace
}

objKey := client.ObjectKey{
Name: s.Bucket.Spec.AccessKeyRef.Name,
Namespace: objKeyNamespace,
}

objStorageKey := &infrav1alpha2.LinodeObjectStorageKey{}
if err := s.Client.Get(ctx, objKey, objStorageKey); err != nil {
return nil, fmt.Errorf("get linodeobjectstoragekey %s: %w", s.Bucket.Spec.AccessKeyRef.Name, err)
}

return objStorageKey, nil
}
Loading
Loading