add bucketaccess sidecar reconciliation

Implement the final portion of BucketAccess provisioning after handoff
to the Sidecar.

Signed-off-by: Blaine Gardner <[email protected]>

Author: BlaineEXE

client/apis/objectstorage/v1alpha2/definitions.go

@@ -25,6 +25,10 @@ const (
 
 // Annotations
 const (
+	// BucketClaimBeingDeletedAnnotation : This annotation is applied by the COSI Controller to a
+	// Bucket when its BucketClaim is being deleted.
+	BucketClaimBeingDeletedAnnotation = `objectstorage.k8.io/bucketclaim-being-deleted`
+
 	// 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

controller/internal/reconciler/bucketaccess.go

@@ -36,8 +36,8 @@ import (
 
 	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/bucketaccess"
 	cosierr "sigs.k8s.io/container-object-storage-interface/internal/errors"
-	"sigs.k8s.io/container-object-storage-interface/internal/handoff"
 	cosipredicate "sigs.k8s.io/container-object-storage-interface/internal/predicate"
 )
 
@@ -67,7 +67,7 @@ func (r *BucketAccessReconciler) Reconcile(ctx context.Context, req ctrl.Request
 		return ctrl.Result{}, err
 	}
 
-	if handoff.BucketAccessManagedBySidecar(access) {
+	if bucketaccess.ManagedBySidecar(access) {
 		logger.V(1).Info("not reconciling BucketAccess that should be managed by sidecar")
 		return ctrl.Result{}, nil
 	}
@@ -142,12 +142,12 @@ func (r *BucketAccessReconciler) reconcile(
 		return cosierr.NonRetryableError(fmt.Errorf("deletion is not yet implemented")) // TODO
 	}
 
-	needInit, err := needsControllerInitialization(&access.Status)
+	initialized, err := bucketaccess.SidecarRequirementsPresent(&access.Status)
 	if err != nil {
 		logger.Error(err, "processed a degraded BucketAccess")
 		return cosierr.NonRetryableError(fmt.Errorf("processed a degraded BucketAccess: %w", err))
 	}
-	if !needInit {
+	if initialized {
 		// 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
@@ -244,38 +244,6 @@ func (r *BucketAccessReconciler) reconcile(
 	return 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.

controller/internal/reconciler/bucketaccess_test.go

@@ -29,7 +29,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/utils/ptr"
 	cosiapi "sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2"
-	"sigs.k8s.io/container-object-storage-interface/internal/handoff"
+	"sigs.k8s.io/container-object-storage-interface/internal/bucketaccess"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
@@ -195,10 +195,10 @@ func TestBucketAccessReconcile(t *testing.T) {
 			status.Parameters,
 		)
 
-		assert.True(t, handoff.BucketAccessManagedBySidecar(access))   // MUST hand off to sidecar
-		needInit, err := needsControllerInitialization(&access.Status) // MUST be fully initialized
+		assert.True(t, bucketaccess.ManagedBySidecar(access))                       // MUST hand off to sidecar
+		initialized, err := bucketaccess.SidecarRequirementsPresent(&access.Status) // MUST be fully initialized
 		assert.NoError(t, err)
-		assert.False(t, needInit)
+		assert.True(t, initialized)
 
 		crw := &cosiapi.BucketClaim{}
 		err = c.Get(ctx, readWriteClaimNsName, crw)
@@ -267,10 +267,10 @@ func TestBucketAccessReconcile(t *testing.T) {
 		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.False(t, bucketaccess.ManagedBySidecar(access))                      // MUST NOT hand off to sidecar
+		initialized, err := bucketaccess.SidecarRequirementsPresent(&access.Status) // MUST NOT be initialized
 		assert.NoError(t, err)
-		assert.True(t, needInit)
+		assert.False(t, initialized)
 
 		crw := &cosiapi.BucketClaim{}
 		err = c.Get(ctx, readWriteClaimNsName, crw)
@@ -315,10 +315,10 @@ func TestBucketAccessReconcile(t *testing.T) {
 		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.False(t, bucketaccess.ManagedBySidecar(access))                      // MUST NOT hand off to sidecar
+		initialized, err := bucketaccess.SidecarRequirementsPresent(&access.Status) // MUST NOT be initialized
 		assert.NoError(t, err)
-		assert.True(t, needInit)
+		assert.False(t, initialized)
 
 		crw := &cosiapi.BucketClaim{}
 		err = c.Get(ctx, readWriteClaimNsName, crw)
@@ -371,10 +371,10 @@ func TestBucketAccessReconcile(t *testing.T) {
 		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.False(t, bucketaccess.ManagedBySidecar(access))                      // MUST NOT hand off to sidecar
+		initialized, err := bucketaccess.SidecarRequirementsPresent(&access.Status) // MUST NOT be initialized
 		assert.NoError(t, err)
-		assert.True(t, needInit)
+		assert.False(t, initialized)
 
 		crw := &cosiapi.BucketClaim{}
 		err = c.Get(ctx, readWriteClaimNsName, crw)
@@ -424,10 +424,10 @@ func TestBucketAccessReconcile(t *testing.T) {
 		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.False(t, bucketaccess.ManagedBySidecar(access))                      // MUST NOT hand off to sidecar
+		initialized, err := bucketaccess.SidecarRequirementsPresent(&access.Status) // MUST NOT be initialized
 		assert.NoError(t, err)
-		assert.True(t, needInit)
+		assert.False(t, initialized)
 
 		crw := &cosiapi.BucketClaim{}
 		err = c.Get(ctx, readWriteClaimNsName, crw)
@@ -474,10 +474,10 @@ func TestBucketAccessReconcile(t *testing.T) {
 		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.False(t, bucketaccess.ManagedBySidecar(access))                      // MUST NOT hand off to sidecar
+		initialized, err := bucketaccess.SidecarRequirementsPresent(&access.Status) // MUST NOT be initialized
 		assert.NoError(t, err)
-		assert.True(t, needInit)
+		assert.False(t, initialized)
 
 		crw := &cosiapi.BucketClaim{}
 		err = c.Get(ctx, readWriteClaimNsName, crw)
@@ -526,10 +526,10 @@ func TestBucketAccessReconcile(t *testing.T) {
 		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.False(t, bucketaccess.ManagedBySidecar(access))                      // MUST NOT hand off to sidecar
+		initialized, err := bucketaccess.SidecarRequirementsPresent(&access.Status) // MUST NOT be initialized
 		assert.NoError(t, err)
-		assert.True(t, needInit)
+		assert.False(t, initialized)
 
 		crw := &cosiapi.BucketClaim{}
 		err = c.Get(ctx, readWriteClaimNsName, crw)
@@ -594,10 +594,10 @@ func TestBucketAccessReconcile(t *testing.T) {
 			status.Parameters,
 		)
 
-		assert.True(t, handoff.BucketAccessManagedBySidecar(access))   // MUST hand off to sidecar
-		needInit, err := needsControllerInitialization(&access.Status) // MUST be fully initialized
+		assert.True(t, bucketaccess.ManagedBySidecar(access))                       // MUST hand off to sidecar
+		initialized, err := bucketaccess.SidecarRequirementsPresent(&access.Status) // MUST be fully initialized
 		assert.NoError(t, err)
-		assert.False(t, needInit)
+		assert.True(t, initialized)
 
 		crw := &cosiapi.BucketClaim{}
 		err = c.Get(ctx, readWriteClaimNsName, crw)
@@ -650,10 +650,10 @@ func TestBucketAccessReconcile(t *testing.T) {
 		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.False(t, bucketaccess.ManagedBySidecar(access))                      // MUST NOT hand off to sidecar
+		initialized, err := bucketaccess.SidecarRequirementsPresent(&access.Status) // MUST NOT be initialized
 		assert.NoError(t, err)
-		assert.True(t, needInit)
+		assert.False(t, initialized)
 
 		crw := &cosiapi.BucketClaim{}
 		err = c.Get(ctx, readWriteClaimNsName, crw)
@@ -706,10 +706,10 @@ func TestBucketAccessReconcile(t *testing.T) {
 		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.False(t, bucketaccess.ManagedBySidecar(access))                      // MUST NOT hand off to sidecar
+		initialized, err := bucketaccess.SidecarRequirementsPresent(&access.Status) // MUST NOT be initialized
 		assert.NoError(t, err)
-		assert.True(t, needInit)
+		assert.False(t, initialized)
 
 		crw := &cosiapi.BucketClaim{}
 		err = c.Get(ctx, readWriteClaimNsName, crw)

go.mod

@@ -13,6 +13,7 @@ require (
 	github.com/go-logr/logr v1.4.3
 	github.com/stretchr/testify v1.11.1
 	google.golang.org/grpc v1.75.1
+	k8s.io/api v0.34.2
 	k8s.io/apimachinery v0.34.2
 	k8s.io/client-go v0.34.2
 	k8s.io/utils v0.0.0-20250820121507-0af2bda4dd1d
@@ -99,7 +100,6 @@ require (
 	gopkg.in/evanphx/json-patch.v4 v4.13.0 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
-	k8s.io/api v0.34.2 // indirect
 	k8s.io/apiextensions-apiserver v0.34.1 // indirect
 	k8s.io/apiserver v0.34.1 // indirect
 	k8s.io/component-base v0.34.1 // indirect

internal/handoff/handoff.go internal/bucketaccess/bucketaccess.gointernal/handoff/handoff.go renamed to internal/bucketaccess/bucketaccess.go

@@ -14,16 +14,18 @@ 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
+// Package bucketaccess defines logic for BucketAccess resources needed by both Controller and
+// Sidecar. BucketAccesses need to be handed off between the two, and some logic is shared.
+package bucketaccess
 
 import (
+	"fmt"
+
 	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.
+// ManagedBySidecar returns true if a BucketAccess should be managed by the Sidecar.
+// It returns false if 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
@@ -39,19 +41,19 @@ import (
 //  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 {
+func ManagedBySidecar(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.
+	// During provisioning, there are several status fields that the Controller needs to initialize
+	// 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 status field.
 	if ba.Status.DriverName == "" {
 		return false
 	}
@@ -66,3 +68,40 @@ func BucketAccessManagedBySidecar(ba *cosiapi.BucketAccess) bool {
 
 	return true
 }
+
+// SidecarRequirementsPresent verifies that BucketAccess status information required by the Sidecar
+// to provision the BucketAccess is fully set.
+//
+// Return true if the fields needed by the Sidecar are all set (Controller initialization finished).
+// Return false if the fields needed by the Sidecar are all unset (needs Controller initialization).
+// Return an error if required info is only partially set, indicating some sort of degradation/bug.
+//
+// Do not use this function to determine whether a BucketAccess should be managed by the Sidecar or
+// Controller, or whether handoff has occurred. Use ManagedBySidecar() for that purpose instead.
+// This function is appropriate for use within a controller to check requirements before/after
+// initialization/provisioning.
+func SidecarRequirementsPresent(s *cosiapi.BucketAccessStatus) (bool, error) {
+	requiredFields := map[string]bool{}
+	set := []string{}
+
+	requiredFieldIsSet := func(fieldName string, isSet bool) {
+		requiredFields[fieldName] = isSet
+		if isSet {
+			set = append(set, fieldName)
+		}
+	}
+
+	requiredFieldIsSet("status.accessedBuckets", len(s.AccessedBuckets) > 0)
+	requiredFieldIsSet("status.driverName", s.DriverName != "")
+	requiredFieldIsSet("status.authenticationType", string(s.AuthenticationType) != "")
+
+	if len(set) == 0 {
+		return false, nil
+	}
+
+	if len(set) == len(requiredFields) {
+		return true, nil
+	}
+
+	return false, fmt.Errorf("fields required for sidecar provisioning are only partially set: %v", requiredFields)
+}

internal/handoff/handoff_test.go …ternal/bucketaccess/bucketaccess_test.gointernal/handoff/handoff_test.go renamed to internal/bucketaccess/bucketaccess_test.go internal/handoff/handoff_test.go renamed to internal/bucketaccess/bucketaccess_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package handoff
+package bucketaccess
 
 import (
 	"testing"
@@ -27,7 +27,7 @@ import (
 	cosiapi "sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2"
 )
 
-func TestBucketAccessManagedBySidecar(t *testing.T) {
+func TestManagedBySidecar(t *testing.T) {
 	tests := []struct {
 		name string // description of this test case
 		// input parameters for target function.
@@ -125,8 +125,8 @@ func TestBucketAccessManagedBySidecar(t *testing.T) {
 			if tt.isHandedOffToSidecar {
 				copy.Status.AccessedBuckets = []cosiapi.AccessedBucket{
 					{
-						BucketName: "bc-asdfgh",
-						AccessMode: cosiapi.BucketAccessModeReadWrite,
+						BucketName:      "bc-asdfgh",
+						BucketClaimName: "bc-1",
 					},
 				}
 				copy.Status.DriverName = "some.driver.io"
@@ -142,12 +142,12 @@ func TestBucketAccessManagedBySidecar(t *testing.T) {
 				copy.Annotations[cosiapi.SidecarCleanupFinishedAnnotation] = ""
 			}
 
-			got := BucketAccessManagedBySidecar(copy)
+			got := ManagedBySidecar(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)
+			withOverride := ManagedBySidecar(copy)
 			assert.False(t, withOverride)
 		})
 	}