Simplify updateBucketCR

internal/controller/bucket/create.go

@@ -63,7 +63,7 @@ func (c *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext
 	// Of course, this approach does not completely remove the possibility of us finding ourselves in
 	// the above scenario. It only mitigates it. As long as Crossplane persists with its existing logic
 	// then we can only make a "best-effort" to avoid it.
-	if err := c.updateBucketCR(ctx, bucket, func(_, bucketLatest *v1alpha1.Bucket) UpdateRequired {
+	if err := c.updateBucketCR(ctx, bucket, func(bucketLatest *v1alpha1.Bucket) UpdateRequired {
 		meta.RemoveAnnotations(bucket, meta.AnnotationKeyExternalCreatePending)
 
 		return NeedsObjectUpdate
@@ -195,7 +195,7 @@ func (c *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext
 	// Bucket CR while there are no backends for us to create on.
 	if backendCount == 0 {
 		c.log.Info("Failed to find any backend for bucket", consts.KeyBucketName, bucket.Name)
-		if err := c.updateBucketCR(ctx, bucket, func(_, bucketLatest *v1alpha1.Bucket) UpdateRequired {
+		if err := c.updateBucketCR(ctx, bucket, func(bucketLatest *v1alpha1.Bucket) UpdateRequired {
 			// Although no backends were found for the bucket, we still apply the backend
 			// label to the Bucket CR for each backend that the bucket was intended to be
 			// created on. This is to ensure the bucket will eventually be created on these
@@ -243,11 +243,11 @@ func (c *external) waitForCreationAndUpdateBucketCR(ctx context.Context, bucket
 			// 2. The Bucket CR Status with the Ready condition.
 			// 3. The Bucket CR Status Backends with a Ready condition for the backend the bucket
 			// was created on.
-			err := c.updateBucketCR(ctx, bucket, func(_, bucketLatest *v1alpha1.Bucket) UpdateRequired {
+			err := c.updateBucketCR(ctx, bucket, func(bucketLatest *v1alpha1.Bucket) UpdateRequired {
 				setAllBackendLabels(bucketLatest, allBackendsToCreateOn)
 
 				return NeedsObjectUpdate
-			}, func(_, bucketLatest *v1alpha1.Bucket) UpdateRequired {
+			}, func(bucketLatest *v1alpha1.Bucket) UpdateRequired {
 				bucketLatest.Status.SetConditions(xpv1.Available())
 				bucketLatest.Status.AtProvider.Backends = v1alpha1.Backends{
 					beName: &v1alpha1.BackendInfo{
@@ -281,7 +281,7 @@ func (c *external) waitForCreationAndUpdateBucketCR(ctx context.Context, bucket
 	// Update the Bucket CR Status condition to Unavailable. This means the Bucket CR will
 	// not be seen as Ready. If that update is successful, we return the createErr which will
 	// be the most recent error receieved from a backend's failed creation.
-	if err := c.updateBucketCR(ctx, bucket, func(_, bucketLatest *v1alpha1.Bucket) UpdateRequired {
+	if err := c.updateBucketCR(ctx, bucket, func(bucketLatest *v1alpha1.Bucket) UpdateRequired {
 		bucketLatest.Status.SetConditions(xpv1.Unavailable())
 
 		return NeedsStatusUpdate

internal/controller/bucket/delete.go

@@ -105,7 +105,7 @@ func (c *external) Delete(ctx context.Context, mg resource.Managed) (managed.Ext
 	// the bucket CR. This is done by setting the Disabled flag on the bucket
 	// CR spec. If the deletion is successful or unsuccessful, the bucket CR status must be
 	// updated.
-	if err := c.updateBucketCR(ctx, bucket, func(bucketDeepCopy, bucketLatest *v1alpha1.Bucket) UpdateRequired {
+	if err := c.updateBucketCR(ctx, bucket, func(bucketLatest *v1alpha1.Bucket) UpdateRequired {
 		setBucketStatus(bucketLatest, bucketBackends, providerNames, c.minReplicas)
 
 		return NeedsStatusUpdate
@@ -130,7 +130,7 @@ func (c *external) Delete(ctx context.Context, mg resource.Managed) (managed.Ext
 			if !bucket.Spec.Disabled {
 				return managed.ExternalDelete{}, nil
 			}
-			if err := c.updateBucketCR(ctx, bucket, func(bucketDeepCopy, bucketLatest *v1alpha1.Bucket) UpdateRequired {
+			if err := c.updateBucketCR(ctx, bucket, func(bucketLatest *v1alpha1.Bucket) UpdateRequired {
 				c.log.Info("Bucket CRs with non-empty buckets should not be disabled - setting 'disabled' flag to false", consts.KeyBucketName, bucket.Name)
 
 				bucketLatest.Spec.Disabled = false

internal/controller/bucket/helpers.go

@@ -211,70 +211,41 @@ const (
 // updateBucketCR updates the Bucket CR and/or the Bucket CR Status by applying a series of callbacks.
 // The function uses an exponential backoff retry mechanism to handle potential conflicts during updates.
 //
-// The callbacks take two Bucket parameters. Before the callbacks are called, the first Bucket
-// parameter will become a DeepCopy of bucket. The second will become the latest version of bucket, as it is fetched
-// from the Kube API. Each callback function should aim to update the latest version of the bucket (second parameter)
-// with the changes which will be persisted in bucket (and as a result, it's DeepCopy).
-//
 // Callbacks return an UpdateRequired status, depending on whether the update that is performed by the callback
 // requires a Bucket Status update (NeedsStatusUpdate) or a full Bucket object update (NeedsObjectUpdate).
 // This enables updateObject to make a decision on whether to perform kubeclient.Status().Update() or
 // kubeClient.Update() respectively.
 //
-// Callback example 1, updating the latest version of bucket Status with a field from your version of bucket.
-// This callback only performs an update to the Bucket Status, so NeedsStatusUpdate is returned to enabled
-// updateBucketCR to perform kubeClient.Status().Update().
-//
-//	 func(bucketDeepCopy, bucketLatest *v1alpha1.Bucket) UpdateRequired {
-//		  bucketLatest.Status.SomeField = bucketDeepCopy.Status.SomeField
-//
-//	   return NeedsStatusUpdate
-//	 },
-//
-// Callback example 2, updating the latest version of bucket Status with a string:
-//
-//		func(_, bucketLatest *v1alpha1.Bucket) {
-//		  bucketLatest.Status.SomeOtherField = "some-value"
+// Callback example, updating the latest version of bucket Status with a string, so NeedsStatusUpdate is
+// returned to enabled updateBucketCR to perform kubeClient.Status().Update().
 //
-//	   return NeedsStatusUpdate
-//		},
+//	func(bucketLatest *v1alpha1.Bucket) {
+//	  bucketLatest.Status.SomeOtherField = "some-value"
 //
-// Callback example 3, updating the latest version of bucket Spec with a field from your version of the bucket.
-// This callback performs an update to the Bucket Spec, so NeedsObjectUpdate is returned to enabled updateBucketCR
-// to perform a full kubeClient.Update().
+//	  return NeedsStatusUpdate
+//	},
 //
-//	 func(bucketDeepCopy, bucketLatest *v1alpha1.Bucket) UpdateRequired {
-//		  bucketLatest.Spec.SomeField = bucketDeepCopy.Spec.SomeField
+// Example usage with above callback example:
 //
-//	   return NeedsObjectUpdate
-//	 },
+//	err := updateBucketCR(ctx, bucket, func(bucketLatest *v1alpha1.Bucket) {
+//	  bucketLatest.Status.SomeOtherField = "some-value"
 //
-// Example usage with above callback example 3:
+//	  return NeedsStatusUpdate
+//	})
 //
-//		err := updateBucketCR(ctx, bucket, func(bucketDeepCopy, bucketLatest *v1alpha1.Bucket) {
-//		  bucketLatest.Spec.SomeField = bucketDeepCopy.Spec.SomeField
-//
-//	   return NeedsObjectUpdate
-//		})
-//
-//		if err != nil {
-//		  // Handle error
-//		}
-func (c *external) updateBucketCR(ctx context.Context, bucket *v1alpha1.Bucket, callbacks ...func(*v1alpha1.Bucket, *v1alpha1.Bucket) UpdateRequired) error {
+//	if err != nil {
+//	  // Handle error
+//	}
+func (c *external) updateBucketCR(ctx context.Context, bucket *v1alpha1.Bucket, callbacks ...func(*v1alpha1.Bucket) UpdateRequired) error {
 	ctx, span := otel.Tracer("").Start(ctx, "bucket.external.updateBucketCR")
 	defer span.End()
 
-	bucketDeepCopy := bucket.DeepCopy()
-
-	nn := types.NamespacedName{Name: bucket.GetName()}
-
 	for _, cb := range callbacks {
 		err := retry.OnError(retry.DefaultRetry, resource.IsAPIError, func() error {
-			if err := c.kubeClient.Get(ctx, nn, bucket); err != nil {
+			if err := c.kubeClient.Get(ctx, types.NamespacedName{Name: bucket.GetName()}, bucket); err != nil {
 				return err
 			}
-
-			switch cb(bucketDeepCopy, bucket) {
+			switch cb(bucket) {
 			case NeedsStatusUpdate:
 				return c.kubeClient.Status().Update(ctx, bucket)
 			case NeedsObjectUpdate:

internal/controller/bucket/update.go

@@ -86,7 +86,7 @@ func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext
 	// Bucket CR Status in all cases to represent the conditions of each individual bucket.
 	cls := c.backendStore.GetBackendS3Clients(allBackendsToUpdateOn)
 	if err := c.updateBucketCR(ctx, bucket,
-		func(bucketDeepCopy, bucketLatest *v1alpha1.Bucket) UpdateRequired {
+		func(bucketLatest *v1alpha1.Bucket) UpdateRequired {
 			setBucketStatus(bucketLatest, bucketBackends, allBackendsToUpdateOn, c.minReplicas)
 
 			return NeedsStatusUpdate
@@ -100,7 +100,7 @@ func (c *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext
 	// The buckets have been updated successfully on all backends, so we need to update the
 	// Bucket CR Spec accordingly.
 	err := c.updateBucketCR(ctx, bucket,
-		func(bucketDeepCopy, bucketLatest *v1alpha1.Bucket) UpdateRequired {
+		func(bucketLatest *v1alpha1.Bucket) UpdateRequired {
 			if bucketLatest.ObjectMeta.Labels == nil {
 				bucketLatest.ObjectMeta.Labels = map[string]string{}
 			}