Address copilot comments

Author: pedjak

internal/operator-controller/controllers/clusterextension_controller.go

@@ -61,9 +61,11 @@ const (
 // ReconcileStepFunc represents a single step in the ClusterExtension reconciliation process.
 // It takes a context and ClusterExtension object as input and returns:
 // - A potentially modified context for the next step
-// - An optional reconciliation result that if non-nil will stop reconciliation
 // - Any error that occurred during reconciliation, which will be returned to the caller
 type ReconcileStepFunc func(context.Context, *ocv1.ClusterExtension) (context.Context, *ctrl.Result, error)
+
+// ReconcileSteps is an ordered collection of reconciliation steps that are executed sequentially.
+// Each step receives the output context from the previous step, allowing data to flow through the pipeline.
 type ReconcileSteps []ReconcileStepFunc
 
 // Reconcile executes a series of reconciliation steps in sequence for a ClusterExtension.
@@ -75,16 +77,16 @@ type ReconcileSteps []ReconcileStepFunc
 func (steps *ReconcileSteps) Reconcile(ctx context.Context, ext *ocv1.ClusterExtension) (ctrl.Result, error) {
 	var res *ctrl.Result
 	var err error
-	stepCtx := ctx
+	nextStepCtx := ctx
 	for _, step := range *steps {
-		stepCtx, res, err = step(stepCtx, ext)
+		nextStepCtx, res, err = step(nextStepCtx, ext)
 		if err != nil {
 			return ctrl.Result{}, err
 		}
 		if res != nil {
 			return *res, nil
 		}
-		if stepCtx == nil {
+		if nextStepCtx == nil {
 			return ctrl.Result{}, nil
 		}
 	}
@@ -309,6 +311,7 @@ func (r *ClusterExtensionReconciler) SetupWithManager(mgr ctrl.Manager, opts ...
 	for _, applyOpt := range opts {
 		applyOpt(ctrlBuilder)
 	}
+
 	return ctrlBuilder.Build(r)
 }
 

internal/operator-controller/controllers/clusterextension_reconcile_steps.go

@@ -19,6 +19,7 @@ package controllers
 import (
 	"context"
 	"errors"
+	"fmt"
 	"io/fs"
 
 	apimeta "k8s.io/apimachinery/pkg/api/meta"
@@ -39,6 +40,22 @@ type revisionStatesKey struct{}
 type resolvedRevisionMetadataKey struct{}
 type imageFSKey struct{}
 
+func valueFromContext[T any](ctx context.Context, key any) (*T, error) {
+	v := ctx.Value(key)
+	if v == nil {
+		return nil, fmt.Errorf("value not found in context under key %v", key)
+	}
+	ptv, ok := v.(*T)
+	if !ok {
+		tv, ok := v.(T)
+		if !ok {
+			return nil, fmt.Errorf("value found in context under key %v is not of type %T or pointer to it", key, v)
+		}
+		return &tv, nil
+	}
+	return ptv, nil
+}
+
 func HandleFinalizers(f finalizer.Finalizer) ReconcileStepFunc {
 	return func(ctx context.Context, ext *ocv1.ClusterExtension) (context.Context, *ctrl.Result, error) {
 		l := log.FromContext(ctx)
@@ -90,7 +107,10 @@ func RetrieveRevisionStates(r RevisionStatesGetter) ReconcileStepFunc {
 func RetrieveRevisionMetadata(r resolve.Resolver) ReconcileStepFunc {
 	return func(ctx context.Context, ext *ocv1.ClusterExtension) (context.Context, *ctrl.Result, error) {
 		l := log.FromContext(ctx)
-		revisionStates := ctx.Value(revisionStatesKey{}).(*RevisionStates)
+		revisionStates, err := valueFromContext[RevisionStates](ctx, revisionStatesKey{})
+		if err != nil {
+			return ctx, nil, err
+		}
 		var resolvedRevisionMetadata *RevisionMetadata
 		if len(revisionStates.RollingOut) == 0 {
 			l.Info("resolving bundle")
@@ -138,7 +158,10 @@ func UnpackBundle(i imageutil.Puller, cache imageutil.Cache) ReconcileStepFunc {
 	return func(ctx context.Context, ext *ocv1.ClusterExtension) (context.Context, *ctrl.Result, error) {
 		l := log.FromContext(ctx)
 		l.Info("unpacking resolved bundle")
-		resolvedRevisionMetadata := ctx.Value(resolvedRevisionMetadataKey{}).(*RevisionMetadata)
+		resolvedRevisionMetadata, err := valueFromContext[RevisionMetadata](ctx, resolvedRevisionMetadataKey{})
+		if err != nil {
+			return ctx, nil, err
+		}
 		imageFS, _, _, err := i.Pull(ctx, ext.GetName(), resolvedRevisionMetadata.Image, cache)
 		if err != nil {
 			revisionStates := ctx.Value(revisionStatesKey{}).(*RevisionStates)
@@ -156,9 +179,18 @@ func UnpackBundle(i imageutil.Puller, cache imageutil.Cache) ReconcileStepFunc {
 func ApplyBundle(a Applier) ReconcileStepFunc {
 	return func(ctx context.Context, ext *ocv1.ClusterExtension) (context.Context, *ctrl.Result, error) {
 		l := log.FromContext(ctx)
-		resolvedRevisionMetadata := ctx.Value(resolvedRevisionMetadataKey{}).(*RevisionMetadata)
-		imageFS := ctx.Value(imageFSKey{}).(fs.FS)
-		revisionStates := ctx.Value(revisionStatesKey{}).(*RevisionStates)
+		resolvedRevisionMetadata, err := valueFromContext[RevisionMetadata](ctx, resolvedRevisionMetadataKey{})
+		if err != nil {
+			return ctx, nil, err
+		}
+		imageFS, err := valueFromContext[fs.FS](ctx, imageFSKey{})
+		if err != nil {
+			return ctx, nil, err
+		}
+		revisionStates, err := valueFromContext[RevisionStates](ctx, revisionStatesKey{})
+		if err != nil {
+			return ctx, nil, err
+		}
 		storeLbls := map[string]string{
 			labels.BundleNameKey:      resolvedRevisionMetadata.Name,
 			labels.PackageNameKey:     resolvedRevisionMetadata.Package,
@@ -180,7 +212,7 @@ func ApplyBundle(a Applier) ReconcileStepFunc {
 		// to ensure exponential backoff can occur:
 		//   - Permission errors (it is not possible to watch changes to permissions.
 		//     The only way to eventually recover from permission errors is to keep retrying).
-		rolloutSucceeded, rolloutStatus, err := a.Apply(ctx, imageFS, ext, objLbls, storeLbls)
+		rolloutSucceeded, rolloutStatus, err := a.Apply(ctx, *imageFS, ext, objLbls, storeLbls)
 
 		// Set installed status
 		if rolloutSucceeded {