add initial BucketAccess controller reconciliation

Add initial implementation for BucketAccess reconciliation based on
v1alpha2 KEP. This initial implementation covers only the first section
of Controller reconciliation for a new BucketAccess. Coverage ends at the
point where reconciliation is handed off to the Sidecar.

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

Author: BlaineEXE

client/apis/objectstorage/v1alpha2/definitions.go

@@ -16,12 +16,30 @@ 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 (
+	// 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"

controller/internal/reconciler/bucketaccess.go

@@ -18,12 +18,16 @@ package reconciler
 
 import (
 	"context"
+	"time"
 
+	"github.com/go-logr/logr"
+	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	logf "sigs.k8s.io/controller-runtime/pkg/log"
+	"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"
 )
 
@@ -33,25 +37,54 @@ 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/[email protected]/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 BucketClaim")
+			return ctrl.Result{}, nil
+		}
+		// no resource to add status to or report an event for
+		logger.Error(err, "failed to get BucketClaim")
+		return ctrl.Result{}, err
+	}
 
-	return ctrl.Result{}, nil
+	retryError, err := r.reconcile(ctx, logger, access)
+	if err != nil {
+		// 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
+	}
+
+	// On success, clear any errors in the status.
+	if access.Status.Error != nil {
+		access.Status.Error = nil
+		if err := r.Status().Update(ctx, access); err != nil {
+			logger.Error(err, "failed to update BucketClaim status after reconcile success")
+			// Retry the reconcile so status can be updated eventually.
+			return reconcile.Result{}, err
+		}
+	}
+
+	return reconcile.Result{}, err
 }
 
 // SetupWithManager sets up the controller with the Manager.
@@ -61,3 +94,9 @@ func (r *BucketAccessReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Named("bucketaccess").
 		Complete(r)
 }
+
+func (r *BucketAccessReconciler) reconcile(
+	ctx context.Context, logger logr.Logger, claim *cosiapi.BucketAccess,
+) (retryErrorType, error) {
+	return NoError, nil
+}

internal/handoff/handoff.go

@@ -0,0 +1,67 @@
+/*
+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. 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
+}

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)
+		})
+	}
+}