Alternative approach with modification of desired state

Signed-off-by: Yury Tsarev <yury@upbound.io>

Author: ytsarev

README.md

@@ -7,8 +7,8 @@ A Crossplane Composition Function for implementing manual approval workflows.
 The `function-approve` provides a serverless approval mechanism at the Crossplane level that:
 
 1. Tracks changes to a specified field by computing a hash
-2. Pauses reconciliation when changes are detected (using either pause annotation or Synced=False condition)
-3. Requires explicit approval before allowing reconciliation to continue
+2. When changes need approval, overwrites Desired state with Observed state to prevent changes
+3. Requires explicit approval before allowing changes to proceed
 4. Prevents drift by storing previously approved state
 
 This function implements the approval workflow using entirely Crossplane-native mechanisms without external dependencies, making it lightweight and reliable.
@@ -30,9 +30,9 @@ spec:
 
 1. When a resource is created or updated, `function-approve` calculates a hash of the monitored field (e.g., `spec.resources`).
 2. The function stores this hash in `status.newHash` (or specified field).
-3. If there's a previous approved hash (`status.oldHash`) and it doesn't match the new hash, reconciliation is paused.
+3. If there's a previous approved hash (`status.oldHash`) and it doesn't match the new hash, the function replaces Desired state with Observed state.
 4. An operator must approve the change by setting `status.approved = true`.
-5. After approval, the new hash is stored as the approved hash, the approval flag is reset, and reconciliation resumes.
+5. After approval, the new hash is stored as the approved hash, the approval flag is reset, and changes are allowed to proceed.
 6. If a customer modifies an existing claim after approval, this will generate a new hash, requiring another approval.
 
 ## Example
@@ -70,10 +70,8 @@ spec:
 | `approvalField` | string | Status field to check for approval. Default: `status.approved` |
 | `oldHashField` | string | Status field to store previously approved hash. Default: `status.oldHash` |
 | `newHashField` | string | Status field to store current hash. Default: `status.newHash` |
-| `pauseAnnotation` | string | Annotation to use for pausing reconciliation. Default: `crossplane.io/paused` |
 | `detailedCondition` | bool | Whether to add detailed information to conditions. Default: `true` |
 | `approvalMessage` | string | Message to display when approval is required. Default: `Changes detected. Approval required.` |
-| `setSyncedFalse` | bool | Use Synced=False condition instead of pause annotation. Default: `false` |
 
 ## Using with Custom Resources
 
@@ -119,16 +117,15 @@ spec:
 
 ## Approving Changes
 
-When changes are detected, the resource's reconciliation is paused, and its condition will show `ApprovalRequired` status. To approve the changes, patch the resource's status:
+When changes are detected, the Desired state is replaced with Observed state (preventing any changes from being applied), and the resource will show an `ApprovalRequired` condition. To approve the changes, patch the resource's status:
 
 ```yaml
 kubectl patch xapproval example --type=merge --subresource=status -p '{"status":{"approved":true}}'
 ```
 
 After approval, the function will:
 1. Record the new state as the approved state
-2. Remove the pause annotation
-3. Allow reconciliation to continue
+2. Allow the changes to proceed normally without overwriting the Desired state
 
 ## Resetting Approval State
 
@@ -143,31 +140,20 @@ kubectl patch xapproval example --type=merge --subresource=status -p '{"status":
 - Use RBAC to control who can approve changes by restricting access to the status subresource
 - Consider implementing additional verification steps or multi-party approval in your workflow
 
-## Pausing Reconciliation Methods
+## How Changes Are Prevented
 
-The function supports two methods to pause reconciliation when changes are detected:
+The function uses a direct approach to prevent changes when approval is required:
 
-### 1. Pause Annotation (Default)
+1. When changes are detected but not yet approved, the function:
+   - Replaces the Desired composite resource with the Observed resource
+   - Replaces any Desired composed resources with the Observed resources
+   - Sets an ApprovalRequired condition for visibility
 
-By default, the function adds the `crossplane.io/paused` annotation (or specified annotation) to pause reconciliation:
-
-```yaml
-pauseAnnotation: "crossplane.io/paused"
-setSyncedFalse: false  # Or omit this field as it defaults to false
-```
-
-### 2. Synced=False Condition
-
-Alternatively, the function can set the Synced condition to False instead of using an annotation:
-
-```yaml
-setSyncedFalse: true
-```
-
-This approach may be preferred in environments where:
-- Annotations are subject to stricter validation or policies
-- You want to leverage Crossplane's native condition-based reconciliation control
-- You need better integration with monitoring systems that use conditions
+2. This approach has several benefits:
+   - Deterministic behavior - changes are physically prevented
+   - No reliance on pause annotations or condition interpretation by the reconciler
+   - Works consistently across different Crossplane versions
+   - Clear separation of approval status and reconciliation mechanics
 
 ## Complete Example
 
@@ -196,7 +182,6 @@ spec:
       oldHashField: "status.approvedHash"
       detailedCondition: true
       approvalMessage: "Cluster changes require admin approval"
-      setSyncedFalse: true  # Use Synced=False condition instead of pause annotation
   - step: create-resources
     functionRef:
       name: function-patch-and-transform

example/composition.yaml

@@ -8,20 +8,6 @@ spec:
     kind: XApproval
   mode: Pipeline
   pipeline:
-  - step: require-approval
-    functionRef:
-      name: function-approve
-    input:
-      apiVersion: approve.fn.crossplane.io/v1alpha1
-      kind: Input
-      dataField: "spec.resources"
-      approvalField: "status.approved"
-      newHashField: "status.newHash"
-      oldHashField: "status.oldHash"
-      pauseAnnotation: "crossplane.io/paused"
-      detailedCondition: true
-      approvalMessage: "Changes detected. Administrative approval required."
-      setSyncedFalse: true
   - step: render-resources
     functionRef:
       name: function-patch-and-transform
@@ -41,3 +27,15 @@ spec:
             fromFieldPath: Required
         readinessChecks:
         - type: None
+  - step: require-approval
+    functionRef:
+      name: function-approve
+    input:
+      apiVersion: approve.fn.crossplane.io/v1alpha1
+      kind: Input
+      dataField: "spec.resources"
+      approvalField: "status.approved"
+      newHashField: "status.newHash"
+      oldHashField: "status.oldHash"
+      detailedCondition: true
+      approvalMessage: "Changes detected. Administrative approval required."

example/functions.yaml

@@ -6,7 +6,8 @@ metadata:
   annotations:
     render.crossplane.io/runtime: Development
 spec:
-  package: xpkg.upbound.io/upbound/function-approve:v0.1.0
+  #package: xpkg.upbound.io/upbound/function-approve:v0.1.0
+  package: xpkg.upbound.io/upbound/function-approve:v0.0.0-20250521103443-546a0fc537a8
 ---
 apiVersion: pkg.crossplane.io/v1beta1
 kind: Function

fn.go

@@ -130,17 +130,17 @@ func (f *Function) handleUnapprovedChanges(req *fnv1.RunFunctionRequest, in *v1b
 			"Approve this change by setting " + *in.ApprovalField + " to true"
 	}
 
-	// Changes detected and not approved, pause reconciliation
-	if err := f.pauseReconciliation(req, in, rsp); err != nil {
+	// Changes detected and not approved, overwrite Desired State with Observed State
+	if err := f.overwriteDesiredWithObserved(req, rsp); err != nil {
 		return err
 	}
 
-	// After pauseReconciliation, add ApprovalRequired condition
+	// Add ApprovalRequired condition for visibility
 	response.ConditionFalse(rsp, "ApprovalRequired", "WaitingForApproval").
 		WithMessage(detailedMsg).
 		TargetCompositeAndClaim()
 
-	f.log.Info("Pausing reconciliation asking for approval", "message", msg, "setSyncedFalse", *in.SetSyncedFalse)
+	f.log.Info("Preventing desired state changes until approval", "message", msg)
 	return nil
 }
 
@@ -152,10 +152,7 @@ func (f *Function) handleApprovedChanges(req *fnv1.RunFunctionRequest, in *v1bet
 		return err
 	}
 
-	// Remove pause annotation if it exists
-	if err := f.resumeReconciliation(req, in, rsp); err != nil {
-		return err
-	}
+	// No need to modify desired state when approved - let changes proceed
 
 	// Set success condition
 	response.ConditionTrue(rsp, "FunctionSuccess", "Success").
@@ -194,21 +191,11 @@ func (f *Function) parseInput(req *fnv1.RunFunctionRequest, rsp *fnv1.RunFunctio
 		in.NewHashField = &defaultField
 	}
 
-	if in.PauseAnnotation == nil {
-		defaultAnnotation := "crossplane.io/paused"
-		in.PauseAnnotation = &defaultAnnotation
-	}
-
 	if in.DetailedCondition == nil {
 		defaultValue := true
 		in.DetailedCondition = &defaultValue
 	}
 
-	if in.SetSyncedFalse == nil {
-		defaultValue := false
-		in.SetSyncedFalse = &defaultValue
-	}
-
 	return in, nil
 }
 
@@ -409,14 +396,22 @@ func SetNestedValue(root map[string]interface{}, key string, value interface{})
 	return nil
 }
 
-// extractDataToHash extracts the data to hash from the specified field
+// extractDataToHash extracts the data to hash from either the desired state or specified field
 func (f *Function) extractDataToHash(req *fnv1.RunFunctionRequest, in *v1beta1.Input, rsp *fnv1.RunFunctionResponse) (interface{}, error) {
-	oxr, err := request.GetObservedCompositeResource(req)
+	dxr, err := request.GetDesiredCompositeResource(req)
 	if err != nil {
-		response.Fatal(rsp, errors.Wrap(err, "cannot get observed composite resource"))
+		response.Fatal(rsp, errors.Wrap(err, "cannot get desired composite resource"))
 		return nil, err
 	}
 
+	// Extract from desired resources
+	// Check if desired composed resources exist first
+	if drs := req.GetDesired().GetResources(); len(drs) > 0 {
+		f.log.Debug("Calculating hash of desired composed resources")
+		return req.GetDesired().GetResources(), nil
+	}
+
+	// Fallback to specific field if desired resources aren't available
 	// Determine if we're looking in spec or status
 	parts := strings.SplitN(in.DataField, ".", 2)
 	if len(parts) != 2 {
@@ -427,8 +422,8 @@ func (f *Function) extractDataToHash(req *fnv1.RunFunctionRequest, in *v1beta1.I
 	section, field := parts[0], parts[1]
 
 	var sectionData map[string]interface{}
-	if err := oxr.Resource.GetValueInto(section, &sectionData); err != nil {
-		response.Fatal(rsp, errors.Wrapf(err, "cannot get %s from observed XR", section))
+	if err := dxr.Resource.GetValueInto(section, &sectionData); err != nil {
+		response.Fatal(rsp, errors.Wrapf(err, "cannot get %s from desired XR", section))
 		return nil, err
 	}
 
@@ -598,92 +593,29 @@ func (f *Function) checkApprovalStatus(req *fnv1.RunFunctionRequest, in *v1beta1
 	return boolValue, nil
 }
 
-// pauseReconciliation pauses resource reconciliation
-func (f *Function) pauseReconciliation(req *fnv1.RunFunctionRequest, in *v1beta1.Input, rsp *fnv1.RunFunctionResponse) error {
-	// Check if we should use Synced=False condition instead of annotation
-	if in.SetSyncedFalse != nil && *in.SetSyncedFalse {
-		// Set Synced condition to False with high priority to pause reconciliation
-		f.log.Info("Setting Synced=False condition to pause reconciliation")
-		rsp.Conditions = nil // Clear any existing conditions to ensure ours takes precedence
-		response.ConditionFalse(rsp, "Synced", "ReconciliationPaused").
-			WithMessage("Resource synchronization paused due to pending approval").
-			TargetCompositeAndClaim()
-
-		return nil
-	}
-
-	// Otherwise use the pause annotation as before
-	_, dxr, err := f.getXRAndStatus(req)
-	if err != nil {
-		response.Fatal(rsp, err)
-		return err
-	}
-
-	// Get current annotations
-	annotations := dxr.Resource.GetAnnotations()
-	if annotations == nil {
-		annotations = map[string]string{}
-	}
-
-	// Add pause annotation
-	annotations[*in.PauseAnnotation] = "true"
-	dxr.Resource.SetAnnotations(annotations)
-
-	// Save the updated desired composite resource
-	if err := response.SetDesiredCompositeResource(rsp, dxr); err != nil {
-		response.Fatal(rsp, errors.Wrapf(err, "cannot set desired composite resource in %T", rsp))
-		return err
-	}
-
-	return nil
-}
-
-// resumeReconciliation removes pause mechanisms from the resource
-func (f *Function) resumeReconciliation(req *fnv1.RunFunctionRequest, in *v1beta1.Input, rsp *fnv1.RunFunctionResponse) error {
-	// If using Synced=False condition, set Synced=True to resume
-	if in.SetSyncedFalse != nil && *in.SetSyncedFalse {
-		f.log.Info("Setting Synced=True condition to resume reconciliation")
-
-		// Find and remove any existing Synced=False conditions
-		var filteredConditions []*fnv1.Condition
-		for _, c := range rsp.GetConditions() {
-			if c.GetType() != "Synced" {
-				filteredConditions = append(filteredConditions, c)
-			}
-		}
-		rsp.Conditions = filteredConditions
-
-		// Set Synced condition to True to resume reconciliation
-		response.ConditionTrue(rsp, "Synced", "ReconciliationResumed").
-			WithMessage("Resource synchronization resumed after approval").
-			TargetCompositeAndClaim()
-
-		return nil
-	}
-
-	// Otherwise remove the pause annotation as before
-	_, dxr, err := f.getXRAndStatus(req)
+// overwriteDesiredWithObserved overwrites the desired state with the observed state to prevent changes
+func (f *Function) overwriteDesiredWithObserved(req *fnv1.RunFunctionRequest, rsp *fnv1.RunFunctionResponse) error {
+	// Get observed composite resource
+	oxr, err := request.GetObservedCompositeResource(req)
 	if err != nil {
-		response.Fatal(rsp, err)
-		return err
+		return errors.Wrap(err, "cannot get observed composite resource")
 	}
 
-	// Get current annotations
-	annotations := dxr.Resource.GetAnnotations()
-	if annotations == nil || annotations[*in.PauseAnnotation] == "" {
-		// No pause annotation, nothing to do
-		return nil
+	// Set the observed state as the desired state
+	if err := response.SetDesiredCompositeResource(rsp, oxr); err != nil {
+		return errors.Wrap(err, "cannot set desired composite resource with observed state")
 	}
 
-	// Remove pause annotation
-	delete(annotations, *in.PauseAnnotation)
-	dxr.Resource.SetAnnotations(annotations)
+	// If there are any desired composed resources, clear them out
+	// We're going to just use the observed state for everything
+	if len(req.GetDesired().GetResources()) > 0 {
+		// Get the observed resources map
+		observedResources := req.GetObserved().GetResources()
 
-	// Save the updated desired composite resource
-	if err := response.SetDesiredCompositeResource(rsp, dxr); err != nil {
-		response.Fatal(rsp, errors.Wrapf(err, "cannot set desired composite resource in %T", rsp))
-		return err
+		// Set the response's desired resources to match observed
+		rsp.Desired.Resources = observedResources
 	}
 
+	f.log.Info("Overwrote desired state with observed state until approval")
 	return nil
 }