diff --git a/client/apis/objectstorage/v1alpha2/bucketaccess_types.go b/client/apis/objectstorage/v1alpha2/bucketaccess_types.go
index b8ba2670..9a38c277 100644
--- a/client/apis/objectstorage/v1alpha2/bucketaccess_types.go
+++ b/client/apis/objectstorage/v1alpha2/bucketaccess_types.go
@@ -167,7 +167,7 @@ type BucketClaimAccess struct {
AccessSecretName string `json:"accessSecretName"`
}
-// AccessedBucket identifies a Bucket and corresponding access parameters.
+// AccessedBucket identifies a Bucket and correlates it to a BucketClaimAccess from the spec.
type AccessedBucket struct {
// bucketName is the name of a Bucket the access should have permissions for.
// +required
@@ -175,9 +175,11 @@ type AccessedBucket struct {
// +kubebuilder:validation:MaxLength=253
BucketName string `json:"bucketName"`
- // accessMode is the Read/Write access mode that the access should have for the bucket.
+ // bucketClaimName must match a BucketClaimAccess's BucketClaimName from the spec.
// +required
- AccessMode BucketAccessMode `json:"accessMode"`
+ // +kubebuilder:validation:MinLength=1
+ // +kubebuilder:validation:MaxLength=253
+ BucketClaimName string `json:"bucketClaimName"`
}
// +kubebuilder:object:root=true
diff --git a/client/apis/objectstorage/v1alpha2/definitions.go b/client/apis/objectstorage/v1alpha2/definitions.go
index 1db1edf5..162b76fc 100644
--- a/client/apis/objectstorage/v1alpha2/definitions.go
+++ b/client/apis/objectstorage/v1alpha2/definitions.go
@@ -16,12 +16,36 @@ limitations under the License.
package v1alpha2
+// Finalizers
const (
// ProtectionFinalizer is applied to a COSI resource object to protect it from deletion while
// COSI processes deletion of the object's intermediate and backend resources.
ProtectionFinalizer = `objectstorage.k8s.io/protection`
)
+// Annotations
+const (
+ // HasBucketAccessReferencesAnnotation : This annotation is applied by the COSI Controller to a
+ // BucketClaim when a BucketAccess that references the BucketClaim is created. The annotation
+ // remains for as long as any BucketAccess references the BucketClaim. Once all BucketAccesses
+ // that reference the BucketClaim are deleted, the annotation is removed.
+ HasBucketAccessReferencesAnnotation = `objectstorage.k8s.io/has-bucketaccess-references`
+
+ // SidecarCleanupFinishedAnnotation : This annotation is applied by a COSI Sidecar to a managed
+ // BucketAccess when the resources is being deleted. The Sidecar calls the Driver to perform
+ // backend deletion actions and then hands off final deletion cleanup to the COSI Controller
+ // by setting this annotation on the resource.
+ SidecarCleanupFinishedAnnotation = `objectstorage.k8s.io/sidecar-cleanup-finished`
+
+ // ControllerManagementOverrideAnnotation : This annotation can be applied to a resource by the
+ // COSI Controller in order to reclaim management of the resource temporarily when it would
+ // otherwise be managed by a COSI Sidecar. This is intended for scenarios where a bug in
+ // provisioning needs to be rectified by a newer version of the COSI Controller. Once the bug is
+ // resolved, the annotation should be removed to allow normal Sidecar handoff to occur.
+ ControllerManagementOverrideAnnotation = `objectstorage.k8s.io/controller-management-override`
+)
+
+// Sidecar RPC definitions
const (
// RpcEndpointDefault is the default RPC endpoint unix socket location.
RpcEndpointDefault = "unix:///var/lib/cosi/cosi.sock"
diff --git a/client/config/crd/objectstorage.k8s.io_bucketaccesses.yaml b/client/config/crd/objectstorage.k8s.io_bucketaccesses.yaml
index f1542913..4d28dc3c 100644
--- a/client/config/crd/objectstorage.k8s.io_bucketaccesses.yaml
+++ b/client/config/crd/objectstorage.k8s.io_bucketaccesses.yaml
@@ -142,16 +142,14 @@ spec:
with per-Bucket access options. This field is populated by the COSI Controller based on the
referenced BucketClaims in the spec.
items:
- description: AccessedBucket identifies a Bucket and corresponding
- access parameters.
+ description: AccessedBucket identifies a Bucket and correlates it
+ to a BucketClaimAccess from the spec.
properties:
- accessMode:
- description: accessMode is the Read/Write access mode that the
- access should have for the bucket.
- enum:
- - ReadWrite
- - ReadOnly
- - WriteOnly
+ bucketClaimName:
+ description: bucketClaimName must match a BucketClaimAccess's
+ BucketClaimName from the spec.
+ maxLength: 253
+ minLength: 1
type: string
bucketName:
description: bucketName is the name of a Bucket the access should
@@ -160,7 +158,7 @@ spec:
minLength: 1
type: string
required:
- - accessMode
+ - bucketClaimName
- bucketName
type: object
type: array
diff --git a/controller/internal/reconciler/bucketaccess.go b/controller/internal/reconciler/bucketaccess.go
index bf8b90f6..28e5152e 100644
--- a/controller/internal/reconciler/bucketaccess.go
+++ b/controller/internal/reconciler/bucketaccess.go
@@ -18,13 +18,24 @@ package reconciler
import (
"context"
+ "fmt"
+ "slices"
+ "time"
+ "github.com/go-logr/logr"
+ kerrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
+ "k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
- logf "sigs.k8s.io/controller-runtime/pkg/log"
+ ctrlutil "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+ ctrlpredicate "sigs.k8s.io/controller-runtime/pkg/predicate"
+ "sigs.k8s.io/controller-runtime/pkg/reconcile"
+ cosiapi "sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2"
objectstoragev1alpha2 "sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2"
+ "sigs.k8s.io/container-object-storage-interface/internal/handoff"
+ cosipredicate "sigs.k8s.io/container-object-storage-interface/internal/predicate"
)
// BucketAccessReconciler reconciles a BucketAccess object
@@ -33,25 +44,57 @@ type BucketAccessReconciler struct {
Scheme *runtime.Scheme
}
-// +kubebuilder:rbac:groups=objectstorage.k8s.io,resources=bucketaccesses,verbs=get;list;watch;create;update;patch;delete
-// +kubebuilder:rbac:groups=objectstorage.k8s.io,resources=bucketaccesses/status,verbs=get;update;patch
+// +kubebuilder:rbac:groups=objectstorage.k8s.io,resources=bucketaccesses,verbs=get;list;watch;create;update
+// +kubebuilder:rbac:groups=objectstorage.k8s.io,resources=bucketaccesses/status,verbs=get;update
// +kubebuilder:rbac:groups=objectstorage.k8s.io,resources=bucketaccesses/finalizers,verbs=update
// Reconcile is part of the main kubernetes reconciliation loop which aims to
// move the current state of the cluster closer to the desired state.
-// TODO(user): Modify the Reconcile function to compare the state specified by
-// the BucketAccess object against the actual cluster state, and then
-// perform operations to make the cluster state reflect the state specified by
-// the user.
-//
-// For more details, check Reconcile and its Result here:
-// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.21.0/pkg/reconcile
func (r *BucketAccessReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
- _ = logf.FromContext(ctx)
+ logger := ctrl.LoggerFrom(ctx)
- // TODO(user): your logic here
+ access := &cosiapi.BucketAccess{}
+ if err := r.Get(ctx, req.NamespacedName, access); err != nil {
+ if kerrors.IsNotFound(err) {
+ logger.V(1).Info("not reconciling nonexistent BucketAccess")
+ return ctrl.Result{}, nil
+ }
+ // no resource to add status to or report an event for
+ logger.Error(err, "failed to get BucketAccess")
+ return ctrl.Result{}, err
+ }
- return ctrl.Result{}, nil
+ if handoff.BucketAccessManagedBySidecar(access) {
+ logger.V(1).Info("not reconciling BucketAccess that should be managed by sidecar")
+ return ctrl.Result{}, nil
+ }
+
+ retryError, err := r.reconcile(ctx, logger, access)
+ if err != nil {
+ // Because the BucketAccess status is could be managed by either Sidecar or Controller,
+ // indicate that this error is coming from the Controller.
+ err = fmt.Errorf("COSI Controller error: %w", err)
+
+ // Record any error as a timestamped error in the status.
+ access.Status.Error = cosiapi.NewTimestampedError(time.Now(), err.Error())
+ if updErr := r.Status().Update(ctx, access); updErr != nil {
+ logger.Error(err, "failed to update BucketAccess status after reconcile error", "updateError", updErr)
+ // If status update fails, we must retry the error regardless of the reconcile return.
+ // The reconcile needs to run again to make sure the status is eventually be updated.
+ return reconcile.Result{}, err
+ }
+
+ if !retryError {
+ return reconcile.Result{}, reconcile.TerminalError(err)
+ }
+ return reconcile.Result{}, err
+ }
+
+ // NOTE: Do not clear the error in the status on success. Success indicates 1 of 2 things:
+ // 1. BucketAccess was initialized successfully, and it's now owned by the Sidecar
+ // 2. BucketAccess deletion cleanup was just finished, and no status update is needed
+
+ return reconcile.Result{}, err
}
// SetupWithManager sets up the controller with the Manager.
@@ -59,5 +102,366 @@ func (r *BucketAccessReconciler) SetupWithManager(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
For(&objectstoragev1alpha2.BucketAccess{}).
Named("bucketaccess").
+ WithEventFilter(
+ ctrlpredicate.And(
+ cosipredicate.BucketAccessManagedByController(r.Scheme), // only opt in to reconciles managed by controller
+ ctrlpredicate.Or(
+ // when managed by controller, we should reconcile ALL Create/Delete/Generic events
+ cosipredicate.AnyCreate(),
+ cosipredicate.AnyDelete(),
+ cosipredicate.AnyGeneric(),
+ // opt in to desired update events
+ cosipredicate.BucketAccessHandoffOccurred(r.Scheme), // reconcile any handoff change
+ cosipredicate.ProtectionFinalizerRemoved(r.Scheme), // re-add protection finalizer if removed
+ ),
+ ),
+ ).
Complete(r)
}
+
+func (r *BucketAccessReconciler) reconcile(
+ ctx context.Context, logger logr.Logger, access *cosiapi.BucketAccess,
+) (retryErrorType, error) {
+ if !access.GetDeletionTimestamp().IsZero() {
+ logger.V(1).Info("beginning BucketAccess deletion cleanup")
+
+ // TODO: deletion logic
+
+ ctrlutil.RemoveFinalizer(access, cosiapi.ProtectionFinalizer)
+ if err := r.Update(ctx, access); err != nil {
+ logger.Error(err, "failed to remove finalizer")
+ return RetryError, fmt.Errorf("failed to remove finalizer: %w", err)
+ }
+
+ return DoNotRetryError, fmt.Errorf("deletion is not yet implemented") // TODO
+ }
+
+ needInit, err := needsControllerInitialization(&access.Status)
+ if err != nil {
+ logger.Error(err, "processed a degraded BucketAccess")
+ return DoNotRetryError, fmt.Errorf("processed a degraded BucketAccess: %w", err)
+ }
+ if !needInit {
+ // BucketAccessClass info should only be copied to the BucketAccess status once, upon
+ // initial provisioning. After the info is copied, make no attempt to fill in any missing or
+ // lost info because we don't know whether the current Class is compatible with the info
+ // from the existing (old) Class info. If we reach this condition, something is systemically
+ // wrong. Sidecar should have ownership, but we determined otherwise, and the Sidecar will
+ // likely also determine us to be the owner.
+ logger.Error(nil, "processed a BucketAccess that should be managed by COSI Sidecar")
+ return DoNotRetryError, fmt.Errorf("processed a BucketAccess that should be managed by COSI Sidecar")
+ }
+
+ logger = logger.WithValues("bucketAccessClassName", access.Spec.BucketAccessClassName)
+
+ logger.V(1).Info("initializing BucketAccess")
+
+ didAdd := ctrlutil.AddFinalizer(access, cosiapi.ProtectionFinalizer)
+ if didAdd {
+ if err := r.Update(ctx, access); err != nil {
+ logger.Error(err, "failed to add protection finalizer")
+ return RetryError, fmt.Errorf("failed to add protection finalizer: %w", err)
+ }
+ }
+
+ claimsByName, retryErr, err := getAllBucketClaims(ctx, r.Client, access.Namespace, access.Spec.BucketClaims)
+ if err != nil {
+ logger.Error(err, "failed to get all referenced BucketClaims")
+ return retryErr, err
+ }
+
+ // Mark as many referenced BucketClaims as possible as soon as possible in the reconcile.
+ // This ensures that BucketClaims are marked to protect their data from deletion quickly.
+ if err := markAllBucketClaimsAsAccessed(ctx, r.Client, claimsByName); err != nil {
+ logger.Error(err, "failed to mark all referenced BucketClaims")
+ return RetryError, err
+ }
+
+ class := &cosiapi.BucketAccessClass{}
+ classNsName := types.NamespacedName{
+ Name: access.Spec.BucketAccessClassName,
+ Namespace: "", // global resource
+ }
+ if err := r.Get(ctx, classNsName, class); err != nil {
+ if kerrors.IsNotFound(err) {
+ // TODO: for now, return an error and allow the controller to exponential backoff
+ // until the access class exists. in the future, optimize this by adding a
+ // access class reconciler that enqueues requests for BucketAccesses that reference the
+ // class and aren't yet passed to the sidecar.
+ logger.Error(err, "BucketAccessClass not found")
+ return RetryError, err
+ }
+ logger.Error(err, "failed to get BucketAccessClass")
+ return RetryError, err
+ }
+
+ if err := validateAccessAgainstClass(&class.Spec, &access.Spec); err != nil {
+ logger.Error(err, "BucketAccess failed featureOptions validation")
+ return DoNotRetryError, err
+ }
+
+ blockers := cannotAccessBucketClaims(claimsByName, access.Spec)
+ if len(blockers) > 0 {
+ logger.Error(nil, "access cannot be provisioned for one or more BucketClaims", "blockers", blockers)
+ return DoNotRetryError, fmt.Errorf("access cannot be provisioned for one or more BucketClaims: %v", blockers)
+ }
+
+ waitlist := waitingOnBucketClaims(claimsByName)
+ if len(waitlist) > 0 {
+ logger.Error(nil, "waiting for prerequisites before provisioning access", "waitlist", waitlist)
+ // TODO: for now, return an error and allow the controller to exponential backoff until we
+ // are done waiting on the resources. in the future, optimize this by adding a bucketclaim
+ // reconciler that enqueues requests for BucketClaims when they finish provisioning.
+ return RetryError, fmt.Errorf("waiting for prerequisites before provisioning access: %v", waitlist)
+ }
+
+ accessedBuckets, err := generateAccessedBuckets(access.Spec.BucketClaims, claimsByName)
+ if err != nil {
+ logger.Error(err, "waiting for BucketClaims to finish provisioning")
+ return RetryError, fmt.Errorf("waiting for BucketClaims to finish provisioning: %w", err)
+ }
+
+ // After this status update, resource management should be handed off to the Sidecar
+ access.Status.AccessedBuckets = accessedBuckets
+ access.Status.DriverName = class.Spec.DriverName
+ access.Status.AuthenticationType = class.Spec.AuthenticationType
+ access.Status.Parameters = class.Spec.Parameters
+ access.Status.Error = nil
+ if err := r.Status().Update(ctx, access); err != nil {
+ logger.Error(err, "failed to update BucketClaim status after successful initialization")
+ return RetryError, err
+ }
+
+ return NoError, nil
+}
+
+// Return true if the Controller needs to initialize the BucketAccess with BucketClaim and
+// BucketAccessClass info. Return false if required info is set.
+// Return an error if any required info is only partially set. This indicates some sort of
+// degradation or bug.
+func needsControllerInitialization(s *cosiapi.BucketAccessStatus) (bool, error) {
+ requiredFields := map[string]bool{}
+ requiredFieldIsSet := func(fieldName string, isSet bool) {
+ requiredFields[fieldName] = isSet
+ }
+
+ requiredFieldIsSet("status.accessedBuckets", len(s.AccessedBuckets) > 0)
+ requiredFieldIsSet("status.driverName", s.DriverName != "")
+ requiredFieldIsSet("status.authenticationType", string(s.AuthenticationType) != "")
+
+ set := []string{}
+ for field, isSet := range requiredFields {
+ if isSet {
+ set = append(set, field)
+ }
+ }
+
+ if len(set) == 0 {
+ return true, nil
+ }
+
+ if len(set) == len(requiredFields) {
+ return false, nil
+ }
+
+ return false, fmt.Errorf("required Controller-managed fields are only partially set: %v", requiredFields)
+}
+
+// Get all BucketClaims that this BucketAccess references.
+// If any claims don't exist, assume they don't exist YET; mark them nil in the resulting map
+// without treating nonexistence as an error.
+// When no error is returned, the output map MUST have an entry for every given BucketClaimAccess.
+func getAllBucketClaims(
+ ctx context.Context, client client.Client, namespace string, claimAccesses []cosiapi.BucketClaimAccess,
+) (map[string]*cosiapi.BucketClaim, retryErrorType, error) {
+ claims := make(map[string]*cosiapi.BucketClaim, len(claimAccesses))
+ errs := []error{}
+ retryErr := RetryError
+
+ for _, ref := range claimAccesses {
+ if _, ok := claims[ref.BucketClaimName]; ok {
+ // In testing, the CEL validation rules prevent this case, but no duplicates is critical
+ // to the access initialization, so double check it.
+ errs = append(errs, fmt.Errorf("BucketClaim %q is referenced more than once", ref.BucketClaimName))
+ retryErr = DoNotRetryError
+ continue
+ }
+
+ c := cosiapi.BucketClaim{}
+ nsName := types.NamespacedName{
+ Namespace: namespace,
+ Name: ref.BucketClaimName,
+ }
+ err := client.Get(ctx, nsName, &c)
+ if kerrors.IsNotFound(err) {
+ // BucketClaim doesn't exist (yet)
+ claims[ref.BucketClaimName] = nil
+ } else if err != nil {
+ // Unspecified API server error that probably resolves after exponential backoff
+ errs = append(errs, err)
+ } else {
+ // No error
+ claims[ref.BucketClaimName] = &c
+ }
+ }
+
+ if len(errs) > 0 {
+ return nil, retryErr, fmt.Errorf("could not get one or more BucketClaims: %v", errs)
+ }
+
+ if len(claims) != len(claimAccesses) {
+ // Should never happen, but double check because the 1:1 requirement is critical.
+ return nil, retryErr, fmt.Errorf("did not get one or more BucketClaims, but no errors observed")
+ }
+
+ return claims, retryErr, nil
+}
+
+// Mark all (non-nil) BucketClaims as having a BucketAccess reference.
+func markAllBucketClaimsAsAccessed(
+ ctx context.Context,
+ client client.Client,
+ claimsByName map[string]*cosiapi.BucketClaim,
+) error {
+ errs := []error{}
+ for _, claim := range claimsByName {
+ if claim == nil {
+ continue
+ }
+
+ if claim.Annotations == nil {
+ claim.Annotations = map[string]string{}
+ }
+ if _, ok := claim.Annotations[cosiapi.HasBucketAccessReferencesAnnotation]; ok {
+ continue // already present
+ }
+ // Race condition: this will still attempt to apply the annotation even when the deletion
+ // timestamp is set. This may interrupt an in-progress BucketClaim deletion before the point
+ // of no return, preserving data, or it may be too late. The BucketClaim deletion logic must
+ // handle the unexpected appearance of this annotation at any point.
+ claim.Annotations[cosiapi.HasBucketAccessReferencesAnnotation] = ""
+ if err := client.Update(ctx, claim); err != nil {
+ errs = append(errs, err)
+ }
+ }
+ if len(errs) > 0 {
+ return fmt.Errorf("failed to mark one or more BucketClaims as having a BucketAccess reference: %v", errs)
+ }
+
+ return nil
+}
+
+// Return an error if the BucketAccess doesn't meet BucketAccessClass requirements.
+func validateAccessAgainstClass(
+ class *cosiapi.BucketAccessClassSpec,
+ access *cosiapi.BucketAccessSpec,
+) error {
+ errs := []string{}
+
+ needServiceAccount := class.AuthenticationType == cosiapi.BucketAccessAuthenticationTypeServiceAccount
+ if needServiceAccount && access.ServiceAccountName == "" {
+ errs = append(errs, "serviceAccountName must be specified")
+ }
+
+ if class.FeatureOptions.DisallowMultiBucketAccess && len(access.BucketClaims) > 1 {
+ errs = append(errs, "multi-bucket access is disallowed")
+ }
+
+ for _, claimRef := range access.BucketClaims {
+ if slices.Contains(class.FeatureOptions.DisallowedBucketAccessModes, claimRef.AccessMode) {
+ errs = append(errs,
+ fmt.Sprintf("accessMode %q requested for BucketClaim %q is disallowed",
+ claimRef.AccessMode, claimRef.BucketClaimName),
+ )
+ }
+ }
+
+ if len(errs) > 0 {
+ return fmt.Errorf("one or more features are disallowed by the BucketAccessClass: %v", errs)
+ }
+ return nil
+}
+
+// Ensure that all BucketClaims can request the access to be provisioned without known errors.
+// Return a list of messages that explain what is blocking provisioning.
+func cannotAccessBucketClaims(
+ claimsByName map[string]*cosiapi.BucketClaim,
+ spec cosiapi.BucketAccessSpec,
+) []string {
+ blockers := []string{}
+ for name, claim := range claimsByName {
+ if claim == nil {
+ continue
+ }
+ if !claim.DeletionTimestamp.IsZero() {
+ // The BucketClaim might not delete while this BucketAccess exists.
+ // The BucketAccess can't proceed for the in-deletion BucketClaim.
+ // Because this is a data safety race, rely on the user to resolve it as they desire.
+ // This race is probably rare in the real world, so going to excessive lengths to
+ // resolve it in COSI seems like premature optimization.
+ blockers = append(blockers,
+ fmt.Sprintf("stuck: data integrity for deleting BucketClaim %q is not guaranteed", name))
+ }
+ if len(claim.Status.Protocols) > 0 && !slices.Contains(claim.Status.Protocols, spec.Protocol) {
+ blockers = append(blockers,
+ fmt.Sprintf("BucketClaim %q does not support protocol %q", name, spec.Protocol))
+ }
+ }
+ return blockers
+}
+
+// Ensure that all BucketClaims are provisioned enough to continue with access initialization.
+// Return a list of messages that explain what needs to be waited on.
+func waitingOnBucketClaims(claimsByName map[string]*cosiapi.BucketClaim) []string {
+ waitMsgs := []string{}
+ for name, claim := range claimsByName {
+ if claim == nil {
+ waitMsgs = append(waitMsgs, fmt.Sprintf("BucketClaim %q does not (yet?) exist", name))
+ continue
+ }
+ if claim.Status.BoundBucketName == "" || len(claim.Status.Protocols) == 0 {
+ waitMsgs = append(waitMsgs, fmt.Sprintf("BucketClaim %q is still provisioning", name))
+ continue
+ }
+ }
+ return waitMsgs
+}
+
+// Generate the accessedBuckets status list for the BucketAccess.
+func generateAccessedBuckets(
+ claimAccesses []cosiapi.BucketClaimAccess,
+ claimsByName map[string]*cosiapi.BucketClaim,
+) (
+ []cosiapi.AccessedBucket,
+ error,
+) {
+ accessedBuckets := make([]cosiapi.AccessedBucket, len(claimAccesses))
+ unbound := []string{}
+
+ // It will be helpful for human readability if the ordering of AccessedBuckets in the status
+ // matches the ordering of BucketClaims in the spec.
+ for i, ref := range claimAccesses {
+ claim, ok := claimsByName[ref.BucketClaimName]
+ if !ok || claim == nil {
+ // Unexpected during runtime because getAllBucketClaims() requires that all input
+ // BucketAccessClaims must be represented in the claimsByName map.
+ return nil, fmt.Errorf("missing expected BucketClaim internally %q", ref.BucketClaimName)
+ }
+
+ if claim.Status.BoundBucketName == "" {
+ unbound = append(unbound, ref.BucketClaimName)
+ continue
+ }
+
+ accessedBuckets[i] = cosiapi.AccessedBucket{
+ BucketName: claim.Status.BoundBucketName,
+ BucketClaimName: claim.GetName(),
+ }
+ }
+
+ if len(unbound) > 0 {
+ return nil, fmt.Errorf("one or more BucketClaims are still unbound to a Bucket: %v", unbound)
+ }
+
+ return accessedBuckets, nil
+}
diff --git a/controller/internal/reconciler/bucketaccess_test.go b/controller/internal/reconciler/bucketaccess_test.go
new file mode 100644
index 00000000..ab18d561
--- /dev/null
+++ b/controller/internal/reconciler/bucketaccess_test.go
@@ -0,0 +1,883 @@
+/*
+Copyright 2025 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package reconciler
+
+import (
+ "context"
+ "testing"
+ "time"
+
+ "github.com/go-logr/logr"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+ meta "k8s.io/apimachinery/pkg/apis/meta/v1"
+ "k8s.io/apimachinery/pkg/runtime"
+ "k8s.io/apimachinery/pkg/types"
+ cosiapi "sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2"
+ "sigs.k8s.io/container-object-storage-interface/internal/handoff"
+ ctrl "sigs.k8s.io/controller-runtime"
+ "sigs.k8s.io/controller-runtime/pkg/client"
+ "sigs.k8s.io/controller-runtime/pkg/client/fake"
+ "sigs.k8s.io/controller-runtime/pkg/reconcile"
+)
+
+func TestBucketAccessReconcile(t *testing.T) {
+ // valid base claim used for subtests
+ baseAccess := cosiapi.BucketAccess{
+ ObjectMeta: meta.ObjectMeta{
+ Name: "my-access",
+ Namespace: "my-ns",
+ },
+ Spec: cosiapi.BucketAccessSpec{
+ BucketClaims: []cosiapi.BucketClaimAccess{
+ {
+ BucketClaimName: "readwrite-bucket",
+ AccessMode: cosiapi.BucketAccessModeReadWrite,
+ AccessSecretName: "readwrite-bucket-creds",
+ },
+ {
+ BucketClaimName: "readonly-bucket",
+ AccessMode: cosiapi.BucketAccessModeReadOnly,
+ AccessSecretName: "readonly-bucket-creds",
+ },
+ },
+ BucketAccessClassName: "s3-class",
+ Protocol: cosiapi.ObjectProtocolS3,
+ ServiceAccountName: "my-app-sa",
+ },
+ }
+
+ accessNsName := types.NamespacedName{
+ Namespace: baseAccess.Namespace,
+ Name: baseAccess.Name,
+ }
+
+ // valid base class used by subests
+ baseClass := cosiapi.BucketAccessClass{
+ ObjectMeta: meta.ObjectMeta{
+ Name: "s3-class",
+ },
+ Spec: cosiapi.BucketAccessClassSpec{
+ DriverName: "cosi.s3.internal",
+ AuthenticationType: cosiapi.BucketAccessAuthenticationTypeKey,
+ Parameters: map[string]string{
+ "maxSize": "100Gi",
+ "maxIops": "10",
+ },
+ FeatureOptions: cosiapi.BucketAccessFeatureOptions{}, // base: no options
+ },
+ }
+
+ // first valid bucketclaim referenced by above valid access
+ baseReadWriteClaim := cosiapi.BucketClaim{
+ ObjectMeta: meta.ObjectMeta{
+ Name: "readwrite-bucket",
+ Namespace: "my-ns",
+ UID: "qwerty",
+ Finalizers: []string{cosiapi.ProtectionFinalizer},
+ },
+ Spec: cosiapi.BucketClaimSpec{
+ BucketClassName: "s3-class",
+ Protocols: []cosiapi.ObjectProtocol{
+ cosiapi.ObjectProtocolS3,
+ },
+ },
+ Status: cosiapi.BucketClaimStatus{
+ BoundBucketName: "bc-qwerty",
+ Protocols: []cosiapi.ObjectProtocol{cosiapi.ObjectProtocolS3},
+ },
+ }
+
+ readWriteClaimNsName := types.NamespacedName{
+ Namespace: baseReadWriteClaim.Namespace,
+ Name: baseReadWriteClaim.Name,
+ }
+
+ // second valid bucketclaim referenced by above valid access
+ baseReadOnlyClaim := cosiapi.BucketClaim{
+ ObjectMeta: meta.ObjectMeta{
+ Name: "readonly-bucket",
+ Namespace: "my-ns",
+ UID: "asdfgh",
+ Finalizers: []string{cosiapi.ProtectionFinalizer},
+ },
+ Spec: cosiapi.BucketClaimSpec{
+ BucketClassName: "s3-class",
+ Protocols: []cosiapi.ObjectProtocol{
+ cosiapi.ObjectProtocolS3,
+ },
+ },
+ Status: cosiapi.BucketClaimStatus{
+ BoundBucketName: "bc-asdfgh",
+ Protocols: []cosiapi.ObjectProtocol{cosiapi.ObjectProtocolS3, cosiapi.ObjectProtocolAzure},
+ },
+ }
+
+ readOnlyClaimNsName := types.NamespacedName{
+ Namespace: baseReadOnlyClaim.Namespace,
+ Name: baseReadOnlyClaim.Name,
+ }
+
+ ctx := context.Background()
+ nolog := logr.Discard()
+ scheme := runtime.NewScheme()
+ err := cosiapi.AddToScheme(scheme)
+ require.NoError(t, err)
+
+ newClient := func(withObj ...client.Object) client.Client {
+ return fake.NewClientBuilder().
+ WithScheme(scheme).
+ WithObjects(withObj...).
+ WithStatusSubresource(withObj...). // assume all starting objects have status
+ Build()
+ }
+
+ t.Run("dynamic provisioning, happy path", func(t *testing.T) {
+ c := newClient(
+ baseAccess.DeepCopy(),
+ baseClass.DeepCopy(),
+ baseReadWriteClaim.DeepCopy(),
+ baseReadOnlyClaim.DeepCopy(),
+ )
+ r := BucketAccessReconciler{
+ Client: c,
+ Scheme: scheme,
+ }
+ nctx := logr.NewContext(ctx, nolog)
+
+ res, err := r.Reconcile(nctx, ctrl.Request{NamespacedName: accessNsName})
+ assert.NoError(t, err)
+ assert.Empty(t, res)
+
+ access := &cosiapi.BucketAccess{}
+ err = c.Get(ctx, accessNsName, access)
+ require.NoError(t, err)
+ assert.Contains(t, access.GetFinalizers(), cosiapi.ProtectionFinalizer)
+ status := access.Status
+ assert.False(t, status.ReadyToUse)
+ assert.Nil(t, status.Error)
+ assert.Equal(t, "", status.AccountID)
+ assert.Equal(t,
+ []cosiapi.AccessedBucket{
+ {
+ BucketName: "bc-qwerty",
+ BucketClaimName: "readwrite-bucket",
+ },
+ {
+ BucketName: "bc-asdfgh",
+ BucketClaimName: "readonly-bucket",
+ },
+ },
+ status.AccessedBuckets,
+ )
+ assert.Equal(t, "cosi.s3.internal", status.DriverName)
+ assert.Equal(t, "Key", string(status.AuthenticationType))
+ assert.Equal(t,
+ map[string]string{
+ "maxSize": "100Gi",
+ "maxIops": "10",
+ },
+ status.Parameters,
+ )
+
+ assert.True(t, handoff.BucketAccessManagedBySidecar(access)) // MUST hand off to sidecar
+ needInit, err := needsControllerInitialization(&access.Status) // MUST be fully initialized
+ assert.NoError(t, err)
+ assert.False(t, needInit)
+
+ crw := &cosiapi.BucketClaim{}
+ err = c.Get(ctx, readWriteClaimNsName, crw)
+ require.NoError(t, err)
+ assert.Contains(t, crw.Annotations, cosiapi.HasBucketAccessReferencesAnnotation)
+
+ cro := &cosiapi.BucketClaim{}
+ err = c.Get(ctx, readOnlyClaimNsName, cro)
+ require.NoError(t, err)
+ assert.Contains(t, cro.Annotations, cosiapi.HasBucketAccessReferencesAnnotation)
+
+ t.Log("run Reconcile() a second time to ensure nothing is modified")
+
+ // using the same client and stuff from before
+ res, err = r.Reconcile(nctx, ctrl.Request{NamespacedName: accessNsName})
+ assert.NoError(t, err)
+ assert.Empty(t, res)
+
+ secondAccess := &cosiapi.BucketAccess{}
+ err = c.Get(ctx, accessNsName, secondAccess)
+ require.NoError(t, err)
+ assert.Equal(t, access, secondAccess)
+
+ crw2 := &cosiapi.BucketClaim{}
+ err = c.Get(ctx, readWriteClaimNsName, crw2)
+ require.NoError(t, err)
+ assert.Equal(t, crw, crw2)
+
+ cro2 := &cosiapi.BucketClaim{}
+ err = c.Get(ctx, readOnlyClaimNsName, cro2)
+ require.NoError(t, err)
+ assert.Equal(t, cro, cro2)
+ })
+
+ t.Run("dynamic provisioning, a bucketclaim doesn't exist", func(t *testing.T) {
+ c := newClient(
+ baseAccess.DeepCopy(),
+ baseClass.DeepCopy(),
+ baseReadWriteClaim.DeepCopy(),
+ // readonly-bucket claim doesn't exist
+ )
+ r := BucketAccessReconciler{
+ Client: c,
+ Scheme: scheme,
+ }
+ nctx := logr.NewContext(ctx, nolog)
+
+ res, err := r.Reconcile(nctx, ctrl.Request{NamespacedName: accessNsName})
+ assert.Error(t, err)
+ assert.NotErrorIs(t, err, reconcile.TerminalError(nil))
+ assert.Empty(t, res)
+
+ access := &cosiapi.BucketAccess{}
+ err = c.Get(ctx, accessNsName, access)
+ require.NoError(t, err)
+ assert.Contains(t, access.GetFinalizers(), cosiapi.ProtectionFinalizer)
+ status := access.Status
+ assert.False(t, status.ReadyToUse)
+ require.NotNil(t, status.Error)
+ assert.NotNil(t, status.Error.Time)
+ assert.NotContains(t, *status.Error.Message, "readwrite-bucket")
+ assert.Contains(t, *status.Error.Message, "readonly-bucket")
+ assert.Equal(t, "", status.AccountID)
+ assert.Empty(t, status.AccessedBuckets)
+ assert.Empty(t, status.DriverName)
+ assert.Empty(t, status.AuthenticationType)
+ assert.Empty(t, status.Parameters)
+
+ assert.False(t, handoff.BucketAccessManagedBySidecar(access)) // MUST NOT hand off to sidecar
+ needInit, err := needsControllerInitialization(&access.Status) // MUST NOT be initialized
+ assert.NoError(t, err)
+ assert.True(t, needInit)
+
+ crw := &cosiapi.BucketClaim{}
+ err = c.Get(ctx, readWriteClaimNsName, crw)
+ require.NoError(t, err)
+ assert.Contains(t, crw.Annotations, cosiapi.HasBucketAccessReferencesAnnotation)
+ })
+
+ t.Run("dynamic provisioning, 1 claim ready, 1 claim provisioning", func(t *testing.T) {
+ rwc := baseReadWriteClaim.DeepCopy()
+ rwc.Status = cosiapi.BucketClaimStatus{}
+
+ c := newClient(
+ baseAccess.DeepCopy(),
+ baseClass.DeepCopy(),
+ rwc,
+ baseReadOnlyClaim.DeepCopy(),
+ )
+ r := BucketAccessReconciler{
+ Client: c,
+ Scheme: scheme,
+ }
+ nctx := logr.NewContext(ctx, nolog)
+
+ res, err := r.Reconcile(nctx, ctrl.Request{NamespacedName: accessNsName})
+ assert.Error(t, err)
+ assert.NotErrorIs(t, err, reconcile.TerminalError(nil))
+ assert.Empty(t, res)
+
+ access := &cosiapi.BucketAccess{}
+ err = c.Get(ctx, accessNsName, access)
+ require.NoError(t, err)
+ assert.Contains(t, access.GetFinalizers(), cosiapi.ProtectionFinalizer)
+ status := access.Status
+ assert.False(t, status.ReadyToUse)
+ require.NotNil(t, status.Error)
+ assert.NotNil(t, status.Error.Time)
+ assert.Contains(t, *status.Error.Message, "readwrite-bucket")
+ assert.NotContains(t, *status.Error.Message, "readonly-bucket")
+ assert.Equal(t, "", status.AccountID)
+ assert.Empty(t, status.AccessedBuckets)
+ assert.Empty(t, status.DriverName)
+ assert.Empty(t, status.AuthenticationType)
+ assert.Empty(t, status.Parameters)
+
+ assert.False(t, handoff.BucketAccessManagedBySidecar(access)) // MUST NOT hand off to sidecar
+ needInit, err := needsControllerInitialization(&access.Status) // MUST NOT be initialized
+ assert.NoError(t, err)
+ assert.True(t, needInit)
+
+ crw := &cosiapi.BucketClaim{}
+ err = c.Get(ctx, readWriteClaimNsName, crw)
+ require.NoError(t, err)
+ assert.Contains(t, crw.Annotations, cosiapi.HasBucketAccessReferencesAnnotation)
+
+ cro := &cosiapi.BucketClaim{}
+ err = c.Get(ctx, readOnlyClaimNsName, cro)
+ require.NoError(t, err)
+ assert.Contains(t, cro.Annotations, cosiapi.HasBucketAccessReferencesAnnotation)
+ })
+
+ t.Run("dynamic provisioning, 1 claim provisioning, 1 claim deleting", func(t *testing.T) {
+ rwc := baseReadWriteClaim.DeepCopy()
+ rwc.Status = cosiapi.BucketClaimStatus{}
+
+ roc := baseReadOnlyClaim.DeepCopy()
+ roc.DeletionTimestamp = &meta.Time{Time: time.Now()}
+
+ c := newClient(
+ baseAccess.DeepCopy(),
+ baseClass.DeepCopy(),
+ rwc,
+ roc,
+ )
+ r := BucketAccessReconciler{
+ Client: c,
+ Scheme: scheme,
+ }
+ nctx := logr.NewContext(ctx, nolog)
+
+ res, err := r.Reconcile(nctx, ctrl.Request{NamespacedName: accessNsName})
+ assert.Error(t, err)
+ assert.ErrorIs(t, err, reconcile.TerminalError(nil))
+ assert.Empty(t, res)
+
+ access := &cosiapi.BucketAccess{}
+ err = c.Get(ctx, accessNsName, access)
+ require.NoError(t, err)
+ assert.Contains(t, access.GetFinalizers(), cosiapi.ProtectionFinalizer)
+ status := access.Status
+ assert.False(t, status.ReadyToUse)
+ require.NotNil(t, status.Error)
+ assert.NotNil(t, status.Error.Time)
+ assert.Contains(t, *status.Error.Message,
+ "data integrity for deleting BucketClaim \"readonly-bucket\" is not guaranteed")
+ assert.Equal(t, "", status.AccountID)
+ assert.Empty(t, status.AccessedBuckets)
+ assert.Empty(t, status.DriverName)
+ assert.Empty(t, status.AuthenticationType)
+ assert.Empty(t, status.Parameters)
+
+ assert.False(t, handoff.BucketAccessManagedBySidecar(access)) // MUST NOT hand off to sidecar
+ needInit, err := needsControllerInitialization(&access.Status) // MUST NOT be initialized
+ assert.NoError(t, err)
+ assert.True(t, needInit)
+
+ crw := &cosiapi.BucketClaim{}
+ err = c.Get(ctx, readWriteClaimNsName, crw)
+ require.NoError(t, err)
+ assert.Contains(t, crw.Annotations, cosiapi.HasBucketAccessReferencesAnnotation)
+
+ // being deleted, but still needs to be marked
+ cro := &cosiapi.BucketClaim{}
+ err = c.Get(ctx, readOnlyClaimNsName, cro)
+ require.NoError(t, err)
+ assert.Contains(t, cro.Annotations, cosiapi.HasBucketAccessReferencesAnnotation)
+ })
+
+ t.Run("dynamic provisioning, 1 claim ready, 1 claim protocol unsupported", func(t *testing.T) {
+ roc := baseReadOnlyClaim.DeepCopy()
+ roc.Status.Protocols = []cosiapi.ObjectProtocol{cosiapi.ObjectProtocolGcs}
+
+ c := newClient(
+ baseAccess.DeepCopy(),
+ baseClass.DeepCopy(),
+ baseReadWriteClaim.DeepCopy(),
+ roc,
+ )
+ r := BucketAccessReconciler{
+ Client: c,
+ Scheme: scheme,
+ }
+ nctx := logr.NewContext(ctx, nolog)
+
+ res, err := r.Reconcile(nctx, ctrl.Request{NamespacedName: accessNsName})
+ assert.Error(t, err)
+ assert.ErrorIs(t, err, reconcile.TerminalError(nil))
+ assert.Empty(t, res)
+
+ access := &cosiapi.BucketAccess{}
+ err = c.Get(ctx, accessNsName, access)
+ require.NoError(t, err)
+ assert.Contains(t, access.GetFinalizers(), cosiapi.ProtectionFinalizer)
+ status := access.Status
+ assert.False(t, status.ReadyToUse)
+ require.NotNil(t, status.Error)
+ assert.NotNil(t, status.Error.Time)
+ assert.Contains(t, *status.Error.Message, "readonly-bucket")
+ assert.Equal(t, "", status.AccountID)
+ assert.Empty(t, status.AccessedBuckets)
+ assert.Empty(t, status.DriverName)
+ assert.Empty(t, status.AuthenticationType)
+ assert.Empty(t, status.Parameters)
+
+ assert.False(t, handoff.BucketAccessManagedBySidecar(access)) // MUST NOT hand off to sidecar
+ needInit, err := needsControllerInitialization(&access.Status) // MUST NOT be initialized
+ assert.NoError(t, err)
+ assert.True(t, needInit)
+
+ crw := &cosiapi.BucketClaim{}
+ err = c.Get(ctx, readWriteClaimNsName, crw)
+ require.NoError(t, err)
+ assert.Contains(t, crw.Annotations, cosiapi.HasBucketAccessReferencesAnnotation)
+
+ // being deleted, but still needs to be marked
+ cro := &cosiapi.BucketClaim{}
+ err = c.Get(ctx, readOnlyClaimNsName, cro)
+ require.NoError(t, err)
+ assert.Contains(t, cro.Annotations, cosiapi.HasBucketAccessReferencesAnnotation)
+ })
+
+ t.Run("dynamic provisioning, bucketaccessclass doesn't exist", func(t *testing.T) {
+ c := newClient(
+ baseAccess.DeepCopy(),
+ // class doesn't exist
+ baseReadWriteClaim.DeepCopy(),
+ baseReadOnlyClaim.DeepCopy(),
+ )
+ r := BucketAccessReconciler{
+ Client: c,
+ Scheme: scheme,
+ }
+ nctx := logr.NewContext(ctx, nolog)
+
+ res, err := r.Reconcile(nctx, ctrl.Request{NamespacedName: accessNsName})
+ assert.Error(t, err)
+ assert.NotErrorIs(t, err, reconcile.TerminalError(nil))
+ assert.Empty(t, res)
+
+ access := &cosiapi.BucketAccess{}
+ err = c.Get(ctx, accessNsName, access)
+ require.NoError(t, err)
+ assert.Contains(t, access.GetFinalizers(), cosiapi.ProtectionFinalizer)
+ status := access.Status
+ assert.False(t, status.ReadyToUse)
+ require.NotNil(t, status.Error)
+ assert.NotNil(t, status.Error.Time)
+ assert.Contains(t, *status.Error.Message, "s3-class")
+ assert.Equal(t, "", status.AccountID)
+ assert.Empty(t, status.AccessedBuckets)
+ assert.Empty(t, status.DriverName)
+ assert.Empty(t, status.AuthenticationType)
+ assert.Empty(t, status.Parameters)
+
+ assert.False(t, handoff.BucketAccessManagedBySidecar(access)) // MUST NOT hand off to sidecar
+ needInit, err := needsControllerInitialization(&access.Status) // MUST NOT be initialized
+ assert.NoError(t, err)
+ assert.True(t, needInit)
+
+ crw := &cosiapi.BucketClaim{}
+ err = c.Get(ctx, readWriteClaimNsName, crw)
+ require.NoError(t, err)
+ assert.Contains(t, crw.Annotations, cosiapi.HasBucketAccessReferencesAnnotation)
+
+ cro := &cosiapi.BucketClaim{}
+ err = c.Get(ctx, readOnlyClaimNsName, cro)
+ require.NoError(t, err)
+ assert.Contains(t, cro.Annotations, cosiapi.HasBucketAccessReferencesAnnotation)
+ })
+
+ t.Run("dynamic provisioning, bucketaccessclass disallows multi-bucket access", func(t *testing.T) {
+ class := baseClass.DeepCopy()
+ class.Spec.FeatureOptions.DisallowMultiBucketAccess = true
+
+ c := newClient(
+ baseAccess.DeepCopy(),
+ class,
+ baseReadWriteClaim.DeepCopy(),
+ baseReadOnlyClaim.DeepCopy(),
+ )
+ r := BucketAccessReconciler{
+ Client: c,
+ Scheme: scheme,
+ }
+ nctx := logr.NewContext(ctx, nolog)
+
+ res, err := r.Reconcile(nctx, ctrl.Request{NamespacedName: accessNsName})
+ assert.Error(t, err)
+ assert.ErrorIs(t, err, reconcile.TerminalError(nil))
+ assert.Empty(t, res)
+
+ access := &cosiapi.BucketAccess{}
+ err = c.Get(ctx, accessNsName, access)
+ require.NoError(t, err)
+ assert.Contains(t, access.GetFinalizers(), cosiapi.ProtectionFinalizer)
+ status := access.Status
+ assert.False(t, status.ReadyToUse)
+ require.NotNil(t, status.Error)
+ assert.NotNil(t, status.Error.Time)
+ assert.Contains(t, *status.Error.Message, "multi-bucket access")
+ assert.Equal(t, "", status.AccountID)
+ assert.Empty(t, status.AccessedBuckets)
+ assert.Empty(t, status.DriverName)
+ assert.Empty(t, status.AuthenticationType)
+ assert.Empty(t, status.Parameters)
+
+ assert.False(t, handoff.BucketAccessManagedBySidecar(access)) // MUST NOT hand off to sidecar
+ needInit, err := needsControllerInitialization(&access.Status) // MUST NOT be initialized
+ assert.NoError(t, err)
+ assert.True(t, needInit)
+
+ crw := &cosiapi.BucketClaim{}
+ err = c.Get(ctx, readWriteClaimNsName, crw)
+ require.NoError(t, err)
+ assert.Contains(t, crw.Annotations, cosiapi.HasBucketAccessReferencesAnnotation)
+
+ cro := &cosiapi.BucketClaim{}
+ err = c.Get(ctx, readOnlyClaimNsName, cro)
+ require.NoError(t, err)
+ assert.Contains(t, cro.Annotations, cosiapi.HasBucketAccessReferencesAnnotation)
+ })
+
+ t.Run("dynamic provisioning, single-bucket passes when multi-bucket access is disallowed", func(t *testing.T) {
+ access := baseAccess.DeepCopy()
+ access.Spec.BucketClaims = []cosiapi.BucketClaimAccess{
+ baseAccess.DeepCopy().Spec.BucketClaims[0],
+ }
+
+ class := baseClass.DeepCopy()
+ class.Spec.FeatureOptions.DisallowMultiBucketAccess = true
+
+ c := newClient(
+ access,
+ class,
+ baseReadWriteClaim.DeepCopy(),
+ baseReadOnlyClaim.DeepCopy(),
+ )
+ r := BucketAccessReconciler{
+ Client: c,
+ Scheme: scheme,
+ }
+ nctx := logr.NewContext(ctx, nolog)
+
+ res, err := r.Reconcile(nctx, ctrl.Request{NamespacedName: accessNsName})
+ assert.NoError(t, err)
+ assert.Empty(t, res)
+
+ access = &cosiapi.BucketAccess{}
+ err = c.Get(ctx, accessNsName, access)
+ require.NoError(t, err)
+ assert.Contains(t, access.GetFinalizers(), cosiapi.ProtectionFinalizer)
+ status := access.Status
+ assert.False(t, status.ReadyToUse)
+ assert.Nil(t, status.Error)
+ assert.Equal(t, "", status.AccountID)
+ assert.Equal(t,
+ []cosiapi.AccessedBucket{
+ {
+ BucketName: "bc-qwerty",
+ BucketClaimName: "readwrite-bucket",
+ },
+ },
+ status.AccessedBuckets,
+ )
+ assert.Equal(t, "cosi.s3.internal", status.DriverName)
+ assert.Equal(t, "Key", string(status.AuthenticationType))
+ assert.Equal(t,
+ map[string]string{
+ "maxSize": "100Gi",
+ "maxIops": "10",
+ },
+ status.Parameters,
+ )
+
+ assert.True(t, handoff.BucketAccessManagedBySidecar(access)) // MUST hand off to sidecar
+ needInit, err := needsControllerInitialization(&access.Status) // MUST be fully initialized
+ assert.NoError(t, err)
+ assert.False(t, needInit)
+
+ crw := &cosiapi.BucketClaim{}
+ err = c.Get(ctx, readWriteClaimNsName, crw)
+ require.NoError(t, err)
+ assert.Contains(t, crw.Annotations, cosiapi.HasBucketAccessReferencesAnnotation)
+
+ cro := &cosiapi.BucketClaim{}
+ err = c.Get(ctx, readOnlyClaimNsName, cro)
+ require.NoError(t, err)
+ assert.NotContains(t, cro.Annotations, cosiapi.HasBucketAccessReferencesAnnotation) // not referenced
+ })
+
+ t.Run("dynamic provisioning, bucketaccessclass disallows write modes", func(t *testing.T) {
+ class := baseClass.DeepCopy()
+ class.Spec.FeatureOptions.DisallowedBucketAccessModes = []cosiapi.BucketAccessMode{
+ cosiapi.BucketAccessModeReadWrite,
+ cosiapi.BucketAccessModeWriteOnly,
+ }
+
+ c := newClient(
+ baseAccess.DeepCopy(),
+ class,
+ baseReadWriteClaim.DeepCopy(),
+ baseReadOnlyClaim.DeepCopy(),
+ )
+ r := BucketAccessReconciler{
+ Client: c,
+ Scheme: scheme,
+ }
+ nctx := logr.NewContext(ctx, nolog)
+
+ res, err := r.Reconcile(nctx, ctrl.Request{NamespacedName: accessNsName})
+ assert.Error(t, err)
+ assert.ErrorIs(t, err, reconcile.TerminalError(nil))
+ assert.Empty(t, res)
+
+ access := &cosiapi.BucketAccess{}
+ err = c.Get(ctx, accessNsName, access)
+ require.NoError(t, err)
+ assert.Contains(t, access.GetFinalizers(), cosiapi.ProtectionFinalizer)
+ status := access.Status
+ assert.False(t, status.ReadyToUse)
+ require.NotNil(t, status.Error)
+ assert.NotNil(t, status.Error.Time)
+ assert.Contains(t, *status.Error.Message, "ReadWrite")
+ assert.Contains(t, *status.Error.Message, "readwrite-bucket")
+ assert.Equal(t, "", status.AccountID)
+ assert.Empty(t, status.AccessedBuckets)
+ assert.Empty(t, status.DriverName)
+ assert.Empty(t, status.AuthenticationType)
+ assert.Empty(t, status.Parameters)
+
+ assert.False(t, handoff.BucketAccessManagedBySidecar(access)) // MUST NOT hand off to sidecar
+ needInit, err := needsControllerInitialization(&access.Status) // MUST NOT be initialized
+ assert.NoError(t, err)
+ assert.True(t, needInit)
+
+ crw := &cosiapi.BucketClaim{}
+ err = c.Get(ctx, readWriteClaimNsName, crw)
+ require.NoError(t, err)
+ assert.Contains(t, crw.Annotations, cosiapi.HasBucketAccessReferencesAnnotation)
+
+ cro := &cosiapi.BucketClaim{}
+ err = c.Get(ctx, readOnlyClaimNsName, cro)
+ require.NoError(t, err)
+ assert.Contains(t, cro.Annotations, cosiapi.HasBucketAccessReferencesAnnotation)
+ })
+
+ t.Run("duplicate BucketClaim reference", func(t *testing.T) {
+ // In testing, CEL validation rules catch this, but test it here to be careful
+ access := baseAccess.DeepCopy()
+ access.Spec.BucketClaims = []cosiapi.BucketClaimAccess{
+ baseAccess.DeepCopy().Spec.BucketClaims[0],
+ baseAccess.DeepCopy().Spec.BucketClaims[0],
+ }
+
+ c := newClient(
+ access,
+ baseClass.DeepCopy(),
+ baseReadWriteClaim.DeepCopy(),
+ baseReadOnlyClaim.DeepCopy(),
+ )
+ r := BucketAccessReconciler{
+ Client: c,
+ Scheme: scheme,
+ }
+ nctx := logr.NewContext(ctx, nolog)
+
+ res, err := r.Reconcile(nctx, ctrl.Request{NamespacedName: accessNsName})
+ assert.Error(t, err)
+ assert.ErrorIs(t, err, reconcile.TerminalError(nil))
+ assert.Empty(t, res)
+
+ access = &cosiapi.BucketAccess{}
+ err = c.Get(ctx, accessNsName, access)
+ require.NoError(t, err)
+ assert.Contains(t, access.GetFinalizers(), cosiapi.ProtectionFinalizer)
+ status := access.Status
+ assert.False(t, status.ReadyToUse)
+ require.NotNil(t, status.Error)
+ assert.NotNil(t, status.Error.Time)
+ assert.Contains(t, *status.Error.Message, "readwrite-bucket")
+ assert.Equal(t, "", status.AccountID)
+ assert.Empty(t, status.AccessedBuckets)
+ assert.Empty(t, status.DriverName)
+ assert.Empty(t, status.AuthenticationType)
+ assert.Empty(t, status.Parameters)
+
+ assert.False(t, handoff.BucketAccessManagedBySidecar(access)) // MUST NOT hand off to sidecar
+ needInit, err := needsControllerInitialization(&access.Status) // MUST NOT be initialized
+ assert.NoError(t, err)
+ assert.True(t, needInit)
+
+ crw := &cosiapi.BucketClaim{}
+ err = c.Get(ctx, readWriteClaimNsName, crw)
+ require.NoError(t, err)
+ assert.NotContains(t, crw.Annotations, cosiapi.HasBucketAccessReferencesAnnotation)
+
+ cro := &cosiapi.BucketClaim{}
+ err = c.Get(ctx, readOnlyClaimNsName, cro)
+ require.NoError(t, err)
+ assert.NotContains(t, cro.Annotations, cosiapi.HasBucketAccessReferencesAnnotation)
+ })
+}
+
+func Test_validateAccessAgainstClass(t *testing.T) {
+ tests := []struct {
+ name string // description of this test case
+ // Named input parameters for target function.
+ class *cosiapi.BucketAccessClassSpec
+ access *cosiapi.BucketAccessSpec
+ wantErr bool
+ }{
+ {"key auth, disallow nothing",
+ &cosiapi.BucketAccessClassSpec{
+ AuthenticationType: cosiapi.BucketAccessAuthenticationTypeKey,
+ FeatureOptions: cosiapi.BucketAccessFeatureOptions{},
+ },
+ &cosiapi.BucketAccessSpec{
+ BucketClaims: []cosiapi.BucketClaimAccess{
+ {
+ BucketClaimName: "rw",
+ AccessMode: cosiapi.BucketAccessModeReadWrite,
+ AccessSecretName: "rw",
+ },
+ {
+ BucketClaimName: "ro",
+ AccessMode: cosiapi.BucketAccessModeReadOnly,
+ AccessSecretName: "ro",
+ },
+ },
+ ServiceAccountName: "",
+ },
+ false,
+ },
+ {"key auth, disallow multi-bucket",
+ &cosiapi.BucketAccessClassSpec{
+ AuthenticationType: cosiapi.BucketAccessAuthenticationTypeKey,
+ FeatureOptions: cosiapi.BucketAccessFeatureOptions{
+ DisallowMultiBucketAccess: true,
+ },
+ },
+ &cosiapi.BucketAccessSpec{
+ BucketClaims: []cosiapi.BucketClaimAccess{
+ {
+ BucketClaimName: "rw",
+ AccessMode: cosiapi.BucketAccessModeReadWrite,
+ AccessSecretName: "rw",
+ },
+ {
+ BucketClaimName: "ro",
+ AccessMode: cosiapi.BucketAccessModeReadOnly,
+ AccessSecretName: "ro",
+ },
+ },
+ ServiceAccountName: "",
+ },
+ true,
+ },
+ {"key auth, disallow write modes",
+ &cosiapi.BucketAccessClassSpec{
+ AuthenticationType: cosiapi.BucketAccessAuthenticationTypeKey,
+ FeatureOptions: cosiapi.BucketAccessFeatureOptions{
+ DisallowedBucketAccessModes: []cosiapi.BucketAccessMode{
+ cosiapi.BucketAccessModeReadWrite,
+ cosiapi.BucketAccessModeWriteOnly,
+ },
+ },
+ },
+ &cosiapi.BucketAccessSpec{
+ BucketClaims: []cosiapi.BucketClaimAccess{
+ {
+ BucketClaimName: "rw",
+ AccessMode: cosiapi.BucketAccessModeReadWrite,
+ AccessSecretName: "rw",
+ },
+ {
+ BucketClaimName: "ro",
+ AccessMode: cosiapi.BucketAccessModeReadOnly,
+ AccessSecretName: "ro",
+ },
+ },
+ ServiceAccountName: "",
+ },
+ true,
+ },
+ {"serviceaccount auth, sa given",
+ &cosiapi.BucketAccessClassSpec{
+ AuthenticationType: cosiapi.BucketAccessAuthenticationTypeServiceAccount,
+ },
+ &cosiapi.BucketAccessSpec{
+ BucketClaims: []cosiapi.BucketClaimAccess{
+ {
+ BucketClaimName: "rw",
+ AccessMode: cosiapi.BucketAccessModeReadWrite,
+ AccessSecretName: "rw",
+ },
+ {
+ BucketClaimName: "ro",
+ AccessMode: cosiapi.BucketAccessModeReadOnly,
+ AccessSecretName: "ro",
+ },
+ },
+ ServiceAccountName: "my-sa",
+ },
+ false,
+ },
+ {"serviceaccount auth, no sa",
+ &cosiapi.BucketAccessClassSpec{
+ AuthenticationType: cosiapi.BucketAccessAuthenticationTypeServiceAccount,
+ },
+ &cosiapi.BucketAccessSpec{
+ BucketClaims: []cosiapi.BucketClaimAccess{
+ {
+ BucketClaimName: "rw",
+ AccessMode: cosiapi.BucketAccessModeReadWrite,
+ AccessSecretName: "rw",
+ },
+ {
+ BucketClaimName: "ro",
+ AccessMode: cosiapi.BucketAccessModeReadOnly,
+ AccessSecretName: "ro",
+ },
+ },
+ ServiceAccountName: "",
+ },
+ true,
+ },
+ {"serviceaccount auth, disallow multi-bucket",
+ &cosiapi.BucketAccessClassSpec{
+ AuthenticationType: cosiapi.BucketAccessAuthenticationTypeServiceAccount,
+ FeatureOptions: cosiapi.BucketAccessFeatureOptions{
+ DisallowMultiBucketAccess: true,
+ },
+ },
+ &cosiapi.BucketAccessSpec{
+ BucketClaims: []cosiapi.BucketClaimAccess{
+ {
+ BucketClaimName: "rw",
+ AccessMode: cosiapi.BucketAccessModeReadWrite,
+ AccessSecretName: "rw",
+ },
+ {
+ BucketClaimName: "ro",
+ AccessMode: cosiapi.BucketAccessModeReadOnly,
+ AccessSecretName: "ro",
+ },
+ },
+ ServiceAccountName: "my-sa",
+ },
+ true,
+ },
+ }
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ gotErr := validateAccessAgainstClass(tt.class, tt.access)
+ if tt.wantErr {
+ assert.Error(t, gotErr)
+ } else {
+ assert.NoError(t, gotErr)
+ }
+ })
+ }
+}
diff --git a/controller/internal/reconciler/bucketclaim.go b/controller/internal/reconciler/bucketclaim.go
index f4396cf8..7e58b31e 100644
--- a/controller/internal/reconciler/bucketclaim.go
+++ b/controller/internal/reconciler/bucketclaim.go
@@ -81,7 +81,7 @@ func (r *BucketClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request)
}
// On success, clear any errors in the status.
- if claim.Status.Error != nil {
+ if claim.Status.Error != nil && !claim.DeletionTimestamp.IsZero() {
claim.Status.Error = nil
if err := r.Status().Update(ctx, claim); err != nil {
logger.Error(err, "failed to update BucketClaim status after reconcile success")
diff --git a/docs/src/api/out.md b/docs/src/api/out.md
index cc3f120d..d20ba371 100644
--- a/docs/src/api/out.md
+++ b/docs/src/api/out.md
@@ -26,7 +26,7 @@ Package v1alpha2 contains API Schema definitions for the objectstorage v1alpha2
-AccessedBucket identifies a Bucket and corresponding access parameters.
+AccessedBucket identifies a Bucket and correlates it to a BucketClaimAccess from the spec.
@@ -36,7 +36,7 @@ _Appears in:_
| Field | Description | Default | Validation |
| --- | --- | --- | --- |
| `bucketName` _string_ | bucketName is the name of a Bucket the access should have permissions for. | | MaxLength: 253
MinLength: 1
|
-| `accessMode` _[BucketAccessMode](#bucketaccessmode)_ | accessMode is the Read/Write access mode that the access should have for the bucket. | | Enum: [ReadWrite ReadOnly WriteOnly]
|
+| `bucketClaimName` _string_ | bucketClaimName must match a BucketClaimAccess's BucketClaimName from the spec. | | MaxLength: 253
MinLength: 1
|
#### Bucket
@@ -206,7 +206,6 @@ _Validation:_
- Enum: [ReadWrite ReadOnly WriteOnly]
_Appears in:_
-- [AccessedBucket](#accessedbucket)
- [BucketAccessFeatureOptions](#bucketaccessfeatureoptions)
- [BucketClaimAccess](#bucketclaimaccess)
diff --git a/internal/handoff/handoff.go b/internal/handoff/handoff.go
new file mode 100644
index 00000000..492301cf
--- /dev/null
+++ b/internal/handoff/handoff.go
@@ -0,0 +1,68 @@
+/*
+Copyright 2025 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// Package handoff defines logic needed for handing off control of resources between Controller and
+// Sidecar.
+package handoff
+
+import (
+ cosiapi "sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2"
+)
+
+// BucketAccessManagedBySidecar returns true if a BucketAccess should be managed by the Sidecar.
+// A false return value indicates that it should be managed by the Controller instead.
+//
+// In order for COSI Controller and any given Sidecar to work well together, they should avoid
+// managing the same BucketAccess resource at the same time. This will help prevent the Controller
+// and Sidecar from racing with each other and causing update conflicts.
+// Instances where a resource has no manager MUST be avoided without exception.
+//
+// Version skew between Controller and Sidecar should be assumed. In order for version skew issues
+// to be minimized, avoid updating this logic unless it is absolutely critical. If updates are made,
+// be sure to carefully consider all version skew cases below. Minimize dual-ownership scenarios,
+// and avoid no-owner scenarios.
+//
+// 1. Sidecar version low, Controller version low
+// 2. Sidecar version low, Controller version high
+// 3. Sidecar version high, Controller version low
+// 4. Sidecar version high, Controller version high
+func BucketAccessManagedBySidecar(ba *cosiapi.BucketAccess) bool {
+ // Allow a future-compatible mechanism by which the Controller can override the normal
+ // BucketAccess management handoff logic in order to resolve a bug.
+ // Instances where this is utilized should be infrequent -- ideally, never used.
+ if _, ok := ba.Annotations[cosiapi.ControllerManagementOverrideAnnotation]; ok {
+ return false
+ }
+
+ // During provisioning, there are several status fields that the Controller needs to set before
+ // the Sidecar can provision an access. However, tying this function's logic to ALL of the
+ // status items could make long-term Controller-Sidecar handoff logic fragile. More logic means
+ // more risk of unmanaged resources and more difficulty reasoning about how changes will impact
+ // ownership during version skew. Minimize risk by relying on a single determining status field.
+ if ba.Status.DriverName == "" {
+ return false
+ }
+
+ // During deletion, as long as the access was handed off to the Sidecar at some point, the
+ // Sidecar must first clean up the backend bucket, then hand back final deletion to the
+ // Controller by setting an annotation.
+ if !ba.DeletionTimestamp.IsZero() {
+ _, ok := ba.Annotations[cosiapi.SidecarCleanupFinishedAnnotation]
+ return !ok // ok means sidecar is done cleaning up
+ }
+
+ return true
+}
diff --git a/internal/handoff/handoff_test.go b/internal/handoff/handoff_test.go
new file mode 100644
index 00000000..006f6296
--- /dev/null
+++ b/internal/handoff/handoff_test.go
@@ -0,0 +1,154 @@
+/*
+Copyright 2025 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package handoff
+
+import (
+ "testing"
+ "time"
+
+ "github.com/stretchr/testify/assert"
+ meta "k8s.io/apimachinery/pkg/apis/meta/v1"
+ "k8s.io/apimachinery/pkg/types"
+
+ cosiapi "sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2"
+)
+
+func TestBucketAccessManagedBySidecar(t *testing.T) {
+ tests := []struct {
+ name string // description of this test case
+ // input parameters for target function.
+ isHandedOffToSidecar bool
+ hasDeletionTimestamp bool
+ hasSidecarCleanupFinishedAnnotation bool
+ // desired result
+ want bool
+ }{
+ // expected real-world scenarios
+ {name: "new BA",
+ isHandedOffToSidecar: false,
+ hasDeletionTimestamp: false,
+ hasSidecarCleanupFinishedAnnotation: false,
+ want: false,
+ },
+ {name: "BA handoff to sidecar",
+ isHandedOffToSidecar: true,
+ hasDeletionTimestamp: false,
+ hasSidecarCleanupFinishedAnnotation: false,
+ want: true,
+ },
+ {name: "sidecar-managed BA begins deleting",
+ isHandedOffToSidecar: true,
+ hasDeletionTimestamp: true,
+ hasSidecarCleanupFinishedAnnotation: false,
+ want: true,
+ },
+ {name: "controller hand-back after sidecar deletion cleanup",
+ isHandedOffToSidecar: true,
+ hasDeletionTimestamp: true,
+ hasSidecarCleanupFinishedAnnotation: true,
+ want: false,
+ },
+ {name: "BA deleted before sidecar handoff",
+ isHandedOffToSidecar: false,
+ hasDeletionTimestamp: true,
+ hasSidecarCleanupFinishedAnnotation: false,
+ want: false,
+ },
+ // degraded scenarios
+ {name: "new BA, erroneous sidecar cleanup annotation",
+ isHandedOffToSidecar: false,
+ hasDeletionTimestamp: false,
+ hasSidecarCleanupFinishedAnnotation: true, // erroneous
+ want: false,
+ },
+ {name: "sidecar-managed BA, erroneous sidecar cleanup annotation",
+ isHandedOffToSidecar: true,
+ hasDeletionTimestamp: false,
+ hasSidecarCleanupFinishedAnnotation: true, // erroneous
+ want: true,
+ },
+ {name: "BA deleted before sidecar handoff, erroneous sidecar cleanup annotation",
+ isHandedOffToSidecar: false,
+ hasDeletionTimestamp: true,
+ hasSidecarCleanupFinishedAnnotation: true, // erroneous
+ want: false,
+ },
+ }
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ base := &cosiapi.BucketAccess{
+ ObjectMeta: meta.ObjectMeta{
+ Name: "my-access",
+ Namespace: "tenant",
+ Finalizers: []string{
+ cosiapi.ProtectionFinalizer,
+ "something-else",
+ },
+ Annotations: map[string]string{
+ "user-annotation": "value",
+ "key-only": "",
+ },
+ CreationTimestamp: meta.NewTime(time.Now()),
+ Generation: 2,
+ UID: types.UID("qwerty"),
+ },
+ Spec: cosiapi.BucketAccessSpec{
+ BucketClaims: []cosiapi.BucketClaimAccess{
+ {
+ BucketClaimName: "bc-1",
+ AccessMode: cosiapi.BucketAccessModeReadWrite,
+ AccessSecretName: "bc-1-creds",
+ },
+ },
+ BucketAccessClassName: "bac-standard",
+ Protocol: cosiapi.ObjectProtocolS3,
+ ServiceAccountName: "my-app",
+ },
+ }
+
+ copy := base.DeepCopy()
+
+ if tt.isHandedOffToSidecar {
+ copy.Status.AccessedBuckets = []cosiapi.AccessedBucket{
+ {
+ BucketName: "bc-asdfgh",
+ AccessMode: cosiapi.BucketAccessModeReadWrite,
+ },
+ }
+ copy.Status.DriverName = "some.driver.io"
+ copy.Status.AuthenticationType = cosiapi.BucketAccessAuthenticationTypeKey
+ copy.Status.Parameters = map[string]string{}
+ }
+
+ if tt.hasDeletionTimestamp {
+ copy.DeletionTimestamp = &meta.Time{Time: time.Now()}
+ }
+
+ if tt.hasSidecarCleanupFinishedAnnotation {
+ copy.Annotations[cosiapi.SidecarCleanupFinishedAnnotation] = ""
+ }
+
+ got := BucketAccessManagedBySidecar(copy)
+ assert.Equal(t, tt.want, got)
+
+ // for all cases,applying the controller override annotation makes it controller-managed
+ copy.Annotations[cosiapi.ControllerManagementOverrideAnnotation] = ""
+ withOverride := BucketAccessManagedBySidecar(copy)
+ assert.False(t, withOverride)
+ })
+ }
+}
diff --git a/internal/predicate/predicate.go b/internal/predicate/predicate.go
index 3f5349c9..795f76d8 100644
--- a/internal/predicate/predicate.go
+++ b/internal/predicate/predicate.go
@@ -21,6 +21,9 @@ limitations under the License.
package predicate
import (
+ "fmt"
+
+ "github.com/go-logr/logr"
"k8s.io/apimachinery/pkg/runtime"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -29,6 +32,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/predicate"
cosiapi "sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2"
+ "sigs.k8s.io/container-object-storage-interface/internal/handoff"
)
// AnyCreate returns a predicate that enqueues a reconcile for any Create event.
@@ -109,6 +113,93 @@ func ProtectionFinalizerRemoved(s *runtime.Scheme) predicate.Funcs {
return funcs
}
+// BucketAccessHandoffOccurred implements a predicate that enqueues a BucketAccess reconcile for
+// Update events where the managing component of the BucketAccess changes, indicating that handoff
+// between Controller and Sidecar has occurred in either direction.
+//
+// The predicate does not enqueue requests for any Create/Delete/Generic events.
+// This ensures that other predicates can effectively filter out undesired non-Update events.
+func BucketAccessHandoffOccurred(s *runtime.Scheme) predicate.Funcs {
+ funcs := allFalseFuncs()
+ funcs.UpdateFunc = func(e event.UpdateEvent) bool {
+ old := e.ObjectOld
+ new := e.ObjectNew
+
+ logger := ctrl.Log.WithName("predicate")
+
+ oldBa, ok := toTypedOrLogError[*cosiapi.BucketAccess](logger.WithValues("oldOrNew", "old"), s, old)
+ if !ok {
+ return false // not a BucketAccess, so don't manage it
+ }
+ newBa, ok := toTypedOrLogError[*cosiapi.BucketAccess](logger.WithValues("oldOrNew", "new"), s, new)
+ if !ok {
+ return false // not a BucketAccess, so don't manage it
+ }
+
+ return handoffOccurred(logger, oldBa, newBa)
+ }
+ return funcs
+}
+
+// Internal logic for determining if BucketAccess Controller-Sidecar handoff has occurred.
+func handoffOccurred(logger logr.Logger, old, new *cosiapi.BucketAccess) bool {
+ oldIsSidecar := handoff.BucketAccessManagedBySidecar(old)
+ newIsSidecar := handoff.BucketAccessManagedBySidecar(new)
+ if oldIsSidecar != newIsSidecar {
+ toComponentName := func(isSidecar bool) string {
+ if isSidecar {
+ return "sidecar"
+ }
+ return "controller"
+ }
+ logger.Info("BucketAccess management handoff occurred",
+ "namespace", old.GetNamespace(), "name", old.GetName(),
+ "oldManagedBy", toComponentName(oldIsSidecar),
+ "newManagedBy", toComponentName(newIsSidecar))
+ return true
+ }
+ return false
+}
+
+// BucketAccessManagedBySidecar implements a predicate that enqueues a BucketAccess reconcile for
+// any event if (and only if) the BucketAccess should be managed by the COSI Sidecar.
+func BucketAccessManagedBySidecar(s *runtime.Scheme) predicate.Funcs {
+ return predicate.NewPredicateFuncs(func(object client.Object) bool {
+ ba, ok := toTypedOrLogError[*cosiapi.BucketAccess](ctrl.Log.WithName("predicate"), s, object)
+ if !ok {
+ return false // not a BucketAccess, so don't manage it
+ }
+ return handoff.BucketAccessManagedBySidecar(ba)
+ })
+}
+
+// BucketAccessManagedByController implements a predicate that enqueues a BucketAccess reconcile for
+// any event if (and only if) the BucketAccess should be managed by the COSI Controller.
+func BucketAccessManagedByController(s *runtime.Scheme) predicate.Funcs {
+ return predicate.NewPredicateFuncs(func(object client.Object) bool {
+ ba, ok := toTypedOrLogError[*cosiapi.BucketAccess](ctrl.Log.WithName("predicate"), s, object)
+ if !ok {
+ return false // not a BucketAccess, so don't manage it
+ }
+ // Note: cannot simply return predicate.Not() of BucketAccessManagedBySidecar() because
+ // any failed type conversion must return false for both Sidecar and Controller
+ return !handoff.BucketAccessManagedBySidecar(ba)
+ })
+}
+
+// Converts a client object to a typed object. Logs an error if conversion fails.
+func toTypedOrLogError[T client.Object](logger logr.Logger, s *runtime.Scheme, object client.Object) (T, bool) {
+ typed, ok := object.(T)
+ if !ok {
+ logger.Error(nil, "failed to convert object with unexpected type",
+ "expectedType", fmt.Sprintf("%T", *new(T)),
+ "receivedType", fmt.Sprintf("%T", object),
+ "kind", inferKind(object, s), "namespace", object.GetNamespace(), "name", object.GetName())
+ return *new(T), false
+ }
+ return typed, true
+}
+
// Makes a best-effort attempt to infer a likely Kind for the object in the schema.
// Useful because controller-runtime predicates don't have GVK info for objects, and logging object
// kind in reusable predicates can help disambiguate resources in logs.
diff --git a/internal/predicate/predicate_test.go b/internal/predicate/predicate_test.go
new file mode 100644
index 00000000..10ad3fe7
--- /dev/null
+++ b/internal/predicate/predicate_test.go
@@ -0,0 +1,95 @@
+/*
+Copyright 2025 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package predicate
+
+import (
+ "testing"
+
+ "github.com/go-logr/logr"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+ meta "k8s.io/apimachinery/pkg/apis/meta/v1"
+ "k8s.io/apimachinery/pkg/runtime"
+ ctrl "sigs.k8s.io/controller-runtime"
+ "sigs.k8s.io/controller-runtime/pkg/client"
+ "sigs.k8s.io/controller-runtime/pkg/log/zap"
+
+ cosiapi "sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2"
+)
+
+func Test_toTypedOrLogError(t *testing.T) {
+ // ctrl.SetLogger(zap.New(zap.UseDevMode(true)))
+ // logger := ctrl.Log.WithName("predicate")
+ logger := logr.Discard() // comment this and uncomment above to locally test log messages
+
+ scheme := runtime.NewScheme()
+ err := cosiapi.AddToScheme(scheme)
+ require.NoError(t, err)
+
+ t.Run("matching type", func(t *testing.T) {
+ access := &cosiapi.BucketAccess{
+ ObjectMeta: meta.ObjectMeta{
+ Namespace: "ns",
+ Name: "name",
+ },
+ }
+ accessObj := client.Object(access)
+
+ gotObj, ok := toTypedOrLogError[*cosiapi.BucketAccess](logger, scheme, accessObj)
+ assert.Equal(t, access, gotObj)
+ assert.True(t, ok)
+ })
+
+ t.Run("nonmatching type", func(t *testing.T) {
+ claim := &cosiapi.BucketClaim{
+ ObjectMeta: meta.ObjectMeta{
+ Namespace: "ns",
+ Name: "name",
+ },
+ }
+ claimObj := client.Object(claim)
+
+ gotObj, ok := toTypedOrLogError[*cosiapi.BucketAccess](logger, scheme, claimObj)
+ assert.Empty(t, gotObj)
+ assert.False(t, ok)
+ })
+}
+
+func Test_handoffOccurred(t *testing.T) {
+ ctrl.SetLogger(zap.New(zap.UseDevMode(true)))
+ logger := ctrl.Log.WithName("predicate")
+ // logger := logr.Discard() // comment this and uncomment above to locally test log messages
+
+ t.Run("no handoff", func(t *testing.T) {
+ old := &cosiapi.BucketAccess{}
+ new := &cosiapi.BucketAccess{}
+
+ assert.False(t, handoffOccurred(logger, old, new))
+ })
+
+ t.Run("handoff", func(t *testing.T) {
+ old := &cosiapi.BucketAccess{}
+ new := &cosiapi.BucketAccess{
+ Status: cosiapi.BucketAccessStatus{
+ DriverName: "something",
+ },
+ }
+
+ assert.True(t, handoffOccurred(logger, old, new))
+ })
+
+}
diff --git a/sidecar/internal/reconciler/bucket.go b/sidecar/internal/reconciler/bucket.go
index 746f41d3..8447665a 100644
--- a/sidecar/internal/reconciler/bucket.go
+++ b/sidecar/internal/reconciler/bucket.go
@@ -82,7 +82,7 @@ func (r *BucketReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
}
// On success, clear any errors in the status.
- if bucket.Status.Error != nil {
+ if bucket.Status.Error != nil && !bucket.DeletionTimestamp.IsZero() {
bucket.Status.Error = nil
if err := r.Status().Update(ctx, bucket); err != nil {
logger.Error(err, "failed to update BucketClaim status after reconcile success")
diff --git a/vendor/sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2/bucketaccess_types.go b/vendor/sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2/bucketaccess_types.go
index b8ba2670..9a38c277 100644
--- a/vendor/sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2/bucketaccess_types.go
+++ b/vendor/sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2/bucketaccess_types.go
@@ -167,7 +167,7 @@ type BucketClaimAccess struct {
AccessSecretName string `json:"accessSecretName"`
}
-// AccessedBucket identifies a Bucket and corresponding access parameters.
+// AccessedBucket identifies a Bucket and correlates it to a BucketClaimAccess from the spec.
type AccessedBucket struct {
// bucketName is the name of a Bucket the access should have permissions for.
// +required
@@ -175,9 +175,11 @@ type AccessedBucket struct {
// +kubebuilder:validation:MaxLength=253
BucketName string `json:"bucketName"`
- // accessMode is the Read/Write access mode that the access should have for the bucket.
+ // bucketClaimName must match a BucketClaimAccess's BucketClaimName from the spec.
// +required
- AccessMode BucketAccessMode `json:"accessMode"`
+ // +kubebuilder:validation:MinLength=1
+ // +kubebuilder:validation:MaxLength=253
+ BucketClaimName string `json:"bucketClaimName"`
}
// +kubebuilder:object:root=true
diff --git a/vendor/sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2/definitions.go b/vendor/sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2/definitions.go
index 1db1edf5..162b76fc 100644
--- a/vendor/sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2/definitions.go
+++ b/vendor/sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2/definitions.go
@@ -16,12 +16,36 @@ limitations under the License.
package v1alpha2
+// Finalizers
const (
// ProtectionFinalizer is applied to a COSI resource object to protect it from deletion while
// COSI processes deletion of the object's intermediate and backend resources.
ProtectionFinalizer = `objectstorage.k8s.io/protection`
)
+// Annotations
+const (
+ // HasBucketAccessReferencesAnnotation : This annotation is applied by the COSI Controller to a
+ // BucketClaim when a BucketAccess that references the BucketClaim is created. The annotation
+ // remains for as long as any BucketAccess references the BucketClaim. Once all BucketAccesses
+ // that reference the BucketClaim are deleted, the annotation is removed.
+ HasBucketAccessReferencesAnnotation = `objectstorage.k8s.io/has-bucketaccess-references`
+
+ // SidecarCleanupFinishedAnnotation : This annotation is applied by a COSI Sidecar to a managed
+ // BucketAccess when the resources is being deleted. The Sidecar calls the Driver to perform
+ // backend deletion actions and then hands off final deletion cleanup to the COSI Controller
+ // by setting this annotation on the resource.
+ SidecarCleanupFinishedAnnotation = `objectstorage.k8s.io/sidecar-cleanup-finished`
+
+ // ControllerManagementOverrideAnnotation : This annotation can be applied to a resource by the
+ // COSI Controller in order to reclaim management of the resource temporarily when it would
+ // otherwise be managed by a COSI Sidecar. This is intended for scenarios where a bug in
+ // provisioning needs to be rectified by a newer version of the COSI Controller. Once the bug is
+ // resolved, the annotation should be removed to allow normal Sidecar handoff to occur.
+ ControllerManagementOverrideAnnotation = `objectstorage.k8s.io/controller-management-override`
+)
+
+// Sidecar RPC definitions
const (
// RpcEndpointDefault is the default RPC endpoint unix socket location.
RpcEndpointDefault = "unix:///var/lib/cosi/cosi.sock"