Owner optional

boxcutter.go

@@ -10,16 +10,34 @@ import (
 
 	"pkg.package-operator.run/boxcutter/machinery"
 	"pkg.package-operator.run/boxcutter/machinery/types"
-	"pkg.package-operator.run/boxcutter/ownerhandling"
 	"pkg.package-operator.run/boxcutter/validation"
 )
 
 // Revision represents multiple phases at a given point in time.
 type Revision = types.Revision
 
+// RevisionBuilder is a Revision with methods to attach options.
+type RevisionBuilder = types.RevisionBuilder
+
+// NewRevision creates a new RevisionBuilder with the given name, rev and phases.
+var NewRevision = types.NewRevision
+
+// NewRevisionWithOwner creates a new RevisionBuilder
+// with the given name, rev, phases and owner.
+var NewRevisionWithOwner = types.NewRevisionWithOwner
+
 // Phase represents a collection of objects lifecycled together.
 type Phase = types.Phase
 
+// PhaseBuilder is a Phase with methods to attach options.
+type PhaseBuilder = types.PhaseBuilder
+
+// NewPhase creates a new PhaseBuilder with the given name and objects.
+var NewPhase = types.NewPhase
+
+// NewPhaseWithOwner creates a new PhaseBuilder with the given name, objects and owner.
+var NewPhaseWithOwner = types.NewPhaseWithOwner
+
 // ObjectReconcileOption is the common interface for object reconciliation options.
 type ObjectReconcileOption = types.ObjectReconcileOption
 
@@ -86,6 +104,11 @@ var WithPhaseReconcileOptions = types.WithPhaseReconcileOptions
 // WithPhaseTeardownOptions applies the given options only to the given Phase.
 var WithPhaseTeardownOptions = types.WithPhaseTeardownOptions
 
+// WithOwner sets an owning object and the strategy to use with it.
+// Ensures controller-refs are set to track the owner and
+// enables handover between owners.
+var WithOwner = types.WithOwner
+
 // ProgressProbeType is a well-known probe type used to guard phase progression.
 const ProgressProbeType = types.ProgressProbeType
 
@@ -114,7 +137,6 @@ type RevisionEngineOptions struct {
 
 	// Optional
 
-	OwnerStrategy  OwnerStrategy
 	PhaseValidator *validation.PhaseValidator
 }
 
@@ -124,20 +146,16 @@ func NewPhaseEngine(opts RevisionEngineOptions) (*machinery.PhaseEngine, error)
 		return nil, err
 	}
 
-	if opts.OwnerStrategy == nil {
-		opts.OwnerStrategy = ownerhandling.NewNative(opts.Scheme)
-	}
-
 	if opts.PhaseValidator == nil {
 		opts.PhaseValidator = validation.NewNamespacedPhaseValidator(opts.RestMapper, opts.Writer)
 	}
 
 	comp := machinery.NewComparator(
-		opts.OwnerStrategy, opts.DiscoveryClient, opts.Scheme, opts.FieldOwner)
+		opts.DiscoveryClient, opts.Scheme, opts.FieldOwner)
 
 	oe := machinery.NewObjectEngine(
 		opts.Scheme, opts.Reader, opts.Writer,
-		opts.OwnerStrategy, comp, opts.FieldOwner, opts.SystemPrefix,
+		comp, opts.FieldOwner, opts.SystemPrefix,
 	)
 
 	return machinery.NewPhaseEngine(oe, opts.PhaseValidator), nil
@@ -149,19 +167,15 @@ func NewRevisionEngine(opts RevisionEngineOptions) (*RevisionEngine, error) {
 		return nil, err
 	}
 
-	if opts.OwnerStrategy == nil {
-		opts.OwnerStrategy = ownerhandling.NewNative(opts.Scheme)
-	}
-
 	pval := validation.NewNamespacedPhaseValidator(opts.RestMapper, opts.Writer)
 	rval := validation.NewRevisionValidator()
 
 	comp := machinery.NewComparator(
-		opts.OwnerStrategy, opts.DiscoveryClient, opts.Scheme, opts.FieldOwner)
+		opts.DiscoveryClient, opts.Scheme, opts.FieldOwner)
 
 	oe := machinery.NewObjectEngine(
 		opts.Scheme, opts.Reader, opts.Writer,
-		opts.OwnerStrategy, comp, opts.FieldOwner, opts.SystemPrefix,
+		comp, opts.FieldOwner, opts.SystemPrefix,
 	)
 	pe := machinery.NewPhaseEngine(oe, pval)
 

cmd/reference/internal/deploy.go

@@ -28,6 +28,7 @@ import (
 
 	"pkg.package-operator.run/boxcutter"
 	"pkg.package-operator.run/boxcutter/managedcache"
+	"pkg.package-operator.run/boxcutter/ownerhandling"
 	"pkg.package-operator.run/boxcutter/probing"
 	"pkg.package-operator.run/boxcutter/util"
 )
@@ -48,8 +49,9 @@ type Reconciler struct {
 	discoveryClient *discovery.DiscoveryClient
 	restMapper      meta.RESTMapper
 
-	cache  managedcache.ObjectBoundAccessManager[*corev1.ConfigMap]
-	scheme *runtime.Scheme
+	cache         managedcache.ObjectBoundAccessManager[*corev1.ConfigMap]
+	scheme        *runtime.Scheme
+	ownerStrategy boxcutter.OwnerStrategy
 }
 
 func NewReconciler(
@@ -65,6 +67,7 @@ func NewReconciler(
 		restMapper:      restMapper,
 		cache:           cache,
 		scheme:          scheme,
+		ownerStrategy:   ownerhandling.NewNative(scheme),
 	}
 }
 
@@ -130,7 +133,7 @@ func (c *Reconciler) handleDeployment(ctx context.Context, cm *corev1.ConfigMap)
 			return res, fmt.Errorf("to revision: %w", err)
 		}
 
-		existingRevisions = append(existingRevisions, *r)
+		existingRevisions = append(existingRevisions, r)
 	}
 
 	sort.Sort(revisionAscending(existingRevisions))
@@ -146,7 +149,7 @@ func (c *Reconciler) handleDeployment(ctx context.Context, cm *corev1.ConfigMap)
 	if len(existingRevisions) > 0 {
 		maybeCurrentObjectSet := existingRevisions[len(existingRevisions)-1]
 
-		annotations := maybeCurrentObjectSet.GetOwner().GetAnnotations()
+		annotations := getOwnerFromRev(maybeCurrentObjectSet).GetAnnotations()
 		if annotations != nil {
 			if hash, ok := annotations[hashAnnotation]; ok &&
 				hash == currentHash {
@@ -197,7 +200,7 @@ func (c *Reconciler) handleDeployment(ctx context.Context, cm *corev1.ConfigMap)
 			break
 		}
 
-		if err := client.IgnoreNotFound(c.client.Delete(ctx, prevRev.GetOwner())); err != nil {
+		if err := client.IgnoreNotFound(c.client.Delete(ctx, getOwnerFromRev(prevRev))); err != nil {
 			return res, fmt.Errorf("failed to delete revision (history limit): %w", err)
 		}
 
@@ -218,9 +221,7 @@ func (c *Reconciler) handleRevision(
 	var objects []client.Object
 
 	for _, phase := range revision.GetPhases() {
-		for _, pobj := range phase.GetObjects() {
-			objects = append(objects, &pobj)
-		}
+		objects = append(objects, phase.GetObjects()...)
 	}
 
 	accessor, err := c.cache.GetWithUser(ctx, deploy, revisionCM, objects)
@@ -243,7 +244,7 @@ func (c *Reconciler) handleRevision(
 
 	if !revisionCM.DeletionTimestamp.IsZero() ||
 		revisionCM.Data[cmStateKey] == "Archived" {
-		tres, err := re.Teardown(ctx, *revision)
+		tres, err := re.Teardown(ctx, revision)
 		if err != nil {
 			return res, fmt.Errorf("revision teardown: %w", err)
 		}
@@ -270,7 +271,7 @@ func (c *Reconciler) handleRevision(
 		return res, err
 	}
 
-	rres, err := re.Reconcile(ctx, *revision, opts...)
+	rres, err := re.Reconcile(ctx, revision, opts...)
 	if err != nil {
 		return res, fmt.Errorf("revision reconcile: %w", err)
 	}
@@ -330,19 +331,19 @@ func (e RevisionNumberNotSetError) Error() string {
 }
 
 func (c *Reconciler) toRevision(deployName string, cm *corev1.ConfigMap) (
-	r *boxcutter.Revision, opts []boxcutter.RevisionReconcileOption, previous []client.Object, err error,
+	r boxcutter.Revision, opts []boxcutter.RevisionReconcileOption, previous []client.Object, err error,
 ) {
 	var (
-		phases        []string
+		phaseNames    []string
 		previousUnstr []unstructured.Unstructured
 		revision      int64
 	)
 
-	objects := map[string][]unstructured.Unstructured{}
+	objects := map[string][]client.Object{}
 
 	for k, v := range cm.Data {
 		if k == cmPhasesKey {
-			if err := json.Unmarshal([]byte(v), &phases); err != nil {
+			if err := json.Unmarshal([]byte(v), &phaseNames); err != nil {
 				return nil, nil, nil, fmt.Errorf("json unmarshal key %s: %w", k, err)
 			}
 
@@ -375,8 +376,8 @@ func (c *Reconciler) toRevision(deployName string, cm *corev1.ConfigMap) (
 
 		phase := parts[0]
 
-		obj := unstructured.Unstructured{}
-		if err := json.Unmarshal([]byte(v), &obj); err != nil {
+		obj := &unstructured.Unstructured{}
+		if err := json.Unmarshal([]byte(v), obj); err != nil {
 			return nil, nil, nil, fmt.Errorf("json unmarshal key %s: %w", k, err)
 		}
 
@@ -399,25 +400,25 @@ func (c *Reconciler) toRevision(deployName string, cm *corev1.ConfigMap) (
 		return nil, nil, nil, RevisionNumberNotSetError{msg: "revision not set"}
 	}
 
-	rev := &boxcutter.Revision{
-		Name:     cm.Name,
-		Owner:    cm,
-		Revision: revision,
-	}
-
 	for _, obj := range previousUnstr {
 		previous = append(previous, &obj)
 	}
 
-	for _, phase := range phases {
-		p := boxcutter.Phase{
-			Name:    phase,
-			Objects: objects[phase],
-		}
+	phases := make([]boxcutter.Phase, 0, len(phaseNames))
+	for _, phaseName := range phaseNames {
+		p := boxcutter.NewPhase(
+			phaseName,
+			objects[phaseName],
+		)
 
-		rev.Phases = append(rev.Phases, p)
+		phases = append(phases, p)
 	}
 
+	rev := boxcutter.NewRevisionWithOwner(
+		cm.Name, revision, phases,
+		cm, c.ownerStrategy,
+	)
+
 	opts = []boxcutter.RevisionReconcileOption{
 		boxcutter.WithPreviousOwners(previous),
 		boxcutter.WithProbe(

cmd/reference/internal/util.go

@@ -7,6 +7,7 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
+	"pkg.package-operator.run/boxcutter"
 	bctypes "pkg.package-operator.run/boxcutter/machinery/types"
 )
 
@@ -33,7 +34,7 @@ func prevJSON(prevRevisions []bctypes.Revision) string {
 	data := make([]unstructured.Unstructured, 0, len(prevRevisions))
 
 	for _, rev := range prevRevisions {
-		refObj := rev.GetOwner()
+		refObj := getOwnerFromRev(rev)
 		ref := unstructured.Unstructured{}
 		ref.SetGroupVersionKind(refObj.GetObjectKind().GroupVersionKind())
 		ref.SetName(refObj.GetName())
@@ -59,3 +60,12 @@ func getOwner(obj client.Object) (metav1.OwnerReference, bool) {
 
 	return metav1.OwnerReference{}, false
 }
+
+func getOwnerFromRev(rev boxcutter.Revision) client.Object {
+	var options bctypes.RevisionReconcileOptions
+	for _, opt := range rev.GetReconcileOptions() {
+		opt.ApplyToRevisionReconcileOptions(&options)
+	}
+
+	return options.GetOwner()
+}

machinery/comparator.go

@@ -21,14 +21,15 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
 	"sigs.k8s.io/structured-merge-diff/v6/fieldpath"
 	"sigs.k8s.io/structured-merge-diff/v6/typed"
+
+	"pkg.package-operator.run/boxcutter/machinery/types"
 )
 
 // Comparator detects divergent state between desired and actual
 // by comparing managed field ownerships.
 // If not all fields from desired are owned by the same field owner in actual,
 // we know that the object has been updated by another actor.
 type Comparator struct {
-	ownerStrategy   divergeDetectorOwnerStrategy
 	openAPIAccessor openAPIAccessor
 	scheme          *runtime.Scheme
 	fieldOwner      string
@@ -38,23 +39,17 @@ type discoveryClient interface {
 	OpenAPIV3() openapi.Client
 }
 
-type divergeDetectorOwnerStrategy interface {
-	SetControllerReference(owner, obj metav1.Object) error
-}
-
 type openAPIAccessor interface {
 	Get(gv schema.GroupVersion) (*spec3.OpenAPI, error)
 }
 
 // NewComparator returns a new Comparator instance.
 func NewComparator(
-	ownerStrategy divergeDetectorOwnerStrategy,
 	discoveryClient discoveryClient,
 	scheme *runtime.Scheme,
 	fieldOwner string,
 ) *Comparator {
 	return &Comparator{
-		ownerStrategy: ownerStrategy,
 		openAPIAccessor: &defaultOpenAPIAccessor{
 			c: discoveryClient.OpenAPIV3(),
 		},
@@ -178,9 +173,14 @@ func (d CompareResult) Modified() []string {
 
 // Compare checks if a resource has been changed from desired.
 func (d *Comparator) Compare(
-	owner client.Object,
 	desiredObject, actualObject Object,
+	opts ...types.ComparatorOption,
 ) (res CompareResult, err error) {
+	var options types.ComparatorOptions
+	for _, opt := range opts {
+		opt.ApplyToComparatorOptions(&options)
+	}
+
 	if err := ensureGVKIsSet(desiredObject, d.scheme); err != nil {
 		return res, err
 	}
@@ -227,8 +227,12 @@ func (d *Comparator) Compare(
 
 	// Extrapolate a field set from desired.
 	desiredObject = desiredObject.DeepCopyObject().(Object)
-	if err := d.ownerStrategy.SetControllerReference(owner, desiredObject); err != nil {
-		return res, err
+	if options.Owner != nil {
+		if err := options.OwnerStrategy.SetControllerReference(
+			options.Owner, desiredObject,
+		); err != nil {
+			return res, err
+		}
 	}
 
 	tName, err := openAPICanonicalName(desiredObject)
@@ -435,7 +439,7 @@ func ensureGVKIsSet(obj client.Object, scheme *runtime.Scheme) error {
 const statusSubresourceSuffix = "{name}/status"
 
 // Determines if the schema has a Status subresource defined.
-// If so the comperator has to ignore .status, because the API server will also ignore these fields.
+// If so the Comparator has to ignore .status, because the API server will also ignore these fields.
 func hasStatusSubresource(openAPISchema *spec3.OpenAPI) bool {
 	if openAPISchema.Paths == nil {
 		return false