feat: migrate from transforms API to output API and fix critical bugs

This commit completes the migration from the old transforms API to the
simpler output API, and fixes two critical bugs discovered during testing.

## Part 1: API Migration (transforms → output)

**Old API**:
```cue
transforms: {
  spec: {type: "replace", value: {components: [...]}}
  labels: {type: "merge", value: {"key": "val"}}
}
```

**New API**:
```cue
output: {
  components: [...]          // replaces spec.components (always replace)
  workflow: {...}            // replaces spec.workflow (always replace)
  policies: [...]            // replaces spec.policies (always replace)
  labels: {"key": "val"}     // always merge
  annotations: {...}         // always merge
  ctx: {...}                 // runtime-only context
}
```

**Implementation** (policy_transforms.go):
- Added PolicyOutput struct for new API
- Added extractOutput() to parse output field
- Added applyPolicyTransform() to apply output to Application
- Updated renderPolicy() to support both APIs temporarily
- Reject old transforms API with clear error message

## Part 2: Bug Fixes

### Bug 1: Policy modifications lost during status update

**Problem**: `Status().Patch()` with `client.Merge` refreshes entire object
from API server, losing in-memory spec modifications made by policies.

**Symptom**: Workflows failed with "component not found" errors.

**Fix** (revision.go:544-555): Save/restore app.Spec around patchStatus().

### Bug 2: Infinite ApplicationRevision creation

**Problem**: RawExtension JSON had inconsistent byte representations,
causing DeepEqualRevision() failures and infinite revision creation.

**Symptom**: 100+ identical ApplicationRevisions created.

**Fix** (revision.go:136-146): Normalize component properties JSON in
gatherRevisionSpec() for consistent comparison.

## Testing

- Migrated policy_validation_test.go to output API (14 tests)
- Verified in test cluster: 1 revision per change, workflows work correctly
- Note: policy_transforms_test.go migration in progress

Tested with OCM policy creating ManifestWork - single revision created,
workflow finds correct components.
This commit is contained in:
Brian Kane
2026-02-16 16:45:13 +00:00
parent f3b67e79ed
commit bf8d128128
3 changed files with 327 additions and 308 deletions
@@ -36,6 +36,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/klog/v2"
"sigs.k8s.io/controller-runtime/pkg/client"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -250,8 +251,7 @@ func (h *AppHandler) ApplyApplicationScopeTransforms(ctx monitorContext.Context,
// Record successful application in status
if changes != nil {
// Determine if spec was modified by checking if transforms.Spec exists
changes.SpecModified = changes.Transforms != nil && changes.Transforms.Spec != nil
// SpecModified is already set correctly in applyPolicyTransform
allPolicyChanges[policy.Name] = changes // Store for ConfigMap serialization
}
recordApplicationPolicyStatus(app, policy.Name, templ.PolicyDefinition.Namespace, "explicit", sequence, 0, true, "", changes)
@@ -300,13 +300,13 @@ func (h *AppHandler) ApplyApplicationScopeTransforms(ctx monitorContext.Context,
"application_hash": appHash, // Hash of Application for cache invalidation
}
// Get the full policy changes if available (includes transforms, labels, annotations, context)
// Get the full policy changes if available (includes output, labels, annotations, context)
if policyChanges, ok := allPolicyChanges[policyName]; ok && policyChanges != nil {
// Store the full transforms object in reusable format
if policyChanges.Transforms != nil {
transformsData := serializeTransformsForStorage(policyChanges.Transforms)
if len(transformsData) > 0 {
policyRecord["transforms"] = transformsData
// Store the output object in reusable format
if policyChanges.Output != nil {
outputData := serializeOutputForStorage(policyChanges.Output)
if len(outputData) > 0 {
policyRecord["output"] = outputData
}
}
@@ -396,16 +396,26 @@ func (h *AppHandler) applyPolicyTransform(ctx monitorContext.Context, app *v1bet
return ctx, nil, nil
}
// Extract transforms field
transforms, err := h.extractTransforms(rendered)
// Extract output (new API only)
output, err := h.extractOutput(rendered)
if err != nil {
return ctx, nil, errors.Wrap(err, "failed to extract transforms")
return ctx, nil, errors.Wrap(err, "failed to extract output")
}
// Check for deprecated transforms API
if _, err := h.extractTransforms(rendered); err != nil {
return ctx, nil, err // Return the deprecation error
}
// Require output field
if output == nil {
return ctx, nil, errors.New("policy must specify 'output' field - see documentation for API reference")
}
// Track changes from before transform application
changes := &PolicyChanges{
Enabled: true, // We already checked it's enabled above
Transforms: transforms,
Enabled: true, // We already checked it's enabled above
Output: output,
}
// Take snapshot of labels and annotations BEFORE applying transform
@@ -422,12 +432,49 @@ func (h *AppHandler) applyPolicyTransform(ctx monitorContext.Context, app *v1bet
}
}
// Apply transforms to the in-memory Application
if transforms != nil {
if err := h.applyTransformsToApplication(ctx, app, transforms); err != nil {
return ctx, nil, errors.Wrap(err, "failed to apply transforms")
// Apply output to Application spec
// Build new spec struct (like old code did) to survive status patch operations
newSpec := app.Spec.DeepCopy()
if output.Components != nil {
newSpec.Components = output.Components
ctx.Info("Replaced components from output", "policy", policyRef.Type, "count", len(output.Components))
changes.SpecModified = true
}
if output.Workflow != nil {
newSpec.Workflow = output.Workflow
ctx.Info("Replaced workflow from output", "policy", policyRef.Type)
changes.SpecModified = true
}
if output.Policies != nil {
newSpec.Policies = output.Policies
ctx.Info("Replaced policies from output", "policy", policyRef.Type, "count", len(output.Policies))
changes.SpecModified = true
}
// Replace entire spec (matches old transforms behavior)
app.Spec = *newSpec
// Apply labels (always merge)
if output.Labels != nil && len(output.Labels) > 0 {
if app.Labels == nil {
app.Labels = make(map[string]string)
}
ctx.Info("Applied transforms to Application", "policy", policyRef.Type)
for k, v := range output.Labels {
app.Labels[k] = v
}
ctx.Info("Merged labels from output", "policy", policyRef.Type, "count", len(output.Labels))
}
// Apply annotations (always merge)
if output.Annotations != nil && len(output.Annotations) > 0 {
if app.Annotations == nil {
app.Annotations = make(map[string]string)
}
for k, v := range output.Annotations {
app.Annotations[k] = v
}
ctx.Info("Merged annotations from output", "policy", policyRef.Type, "count", len(output.Annotations))
}
// Compare AFTER to capture actual changes (works regardless of how CUE modifies the Application)
@@ -455,11 +502,8 @@ func (h *AppHandler) applyPolicyTransform(ctx monitorContext.Context, app *v1bet
changes.AddedAnnotations = annotationsAdded
}
// Extract and store additionalContext
additionalContext, err := h.extractAdditionalContext(rendered)
if err != nil {
return ctx, nil, errors.Wrap(err, "failed to extract additionalContext")
}
// Store ctx as additionalContext
additionalContext := output.Ctx
if additionalContext != nil {
ctx = storeAdditionalContextInCtx(ctx, additionalContext)
@@ -490,9 +534,13 @@ func (h *AppHandler) renderPolicy(ctx monitorContext.Context, app *v1beta1.Appli
} else if cachedRecord != nil {
ctx.Info("Using cached policy result", "policy", policyDef.Name, "ttl", ttlSeconds)
// Deserialize the cached result
if transformsData, ok := cachedRecord["transforms"].(map[string]interface{}); ok {
result.Transforms = deserializeTransformsFromStorage(transformsData)
// Deserialize the cached output
if outputData, ok := cachedRecord["output"].(map[string]interface{}); ok {
result.Transforms = deserializeOutputFromStorage(outputData)
} else {
// No output in cache - policy needs re-rendering
klog.Warningf("Policy %s cached without output data - will re-render", policyDef.Name)
return RenderedPolicyResult{}, nil
}
if additionalContext, ok := cachedRecord["additional_context"].(map[string]interface{}); ok {
@@ -550,21 +598,29 @@ func (h *AppHandler) renderPolicy(ctx monitorContext.Context, app *v1beta1.Appli
return result, nil
}
// Extract transforms field
transforms, err := h.extractTransforms(rendered)
// Extract output (new API only)
output, err := h.extractOutput(rendered)
if err != nil {
result.SkipReason = fmt.Sprintf("transforms extraction error: %s", err.Error())
return result, errors.Wrap(err, "failed to extract transforms")
result.SkipReason = fmt.Sprintf("output extraction error: %s", err.Error())
return result, errors.Wrap(err, "failed to extract output")
}
result.Transforms = transforms
// Extract additionalContext
additionalContext, err := h.extractAdditionalContext(rendered)
if err != nil {
result.SkipReason = fmt.Sprintf("additionalContext extraction error: %s", err.Error())
return result, errors.Wrap(err, "failed to extract additionalContext")
// Check for deprecated transforms API
if _, err := h.extractTransforms(rendered); err != nil {
result.SkipReason = "using deprecated transforms API"
return result, err // Return the deprecation error
}
result.AdditionalContext = additionalContext
// Require output field
if output == nil {
result.SkipReason = "missing output field"
return result, errors.New("policy must specify 'output' field - see documentation for API reference")
}
// Store output
result.Transforms = output
// Extract ctx as additionalContext
result.AdditionalContext = output.Ctx
return result, nil
}
@@ -579,59 +635,58 @@ func (h *AppHandler) applyRenderedPolicyResult(ctx monitorContext.Context, app *
return ctx, nil, nil
}
// Cast transforms from interface{}
transforms, ok := result.Transforms.(*PolicyTransforms)
if !ok && result.Transforms != nil {
return ctx, nil, errors.Errorf("cached transforms have invalid type for policy %s", result.PolicyName)
// Extract PolicyOutput from cached result
output, ok := result.Transforms.(*PolicyOutput)
if !ok || output == nil {
return ctx, nil, errors.Errorf("cached policy has invalid or missing output for policy %s", result.PolicyName)
}
// Track what changes we're making
changes := &PolicyChanges{
AdditionalContext: result.AdditionalContext,
Enabled: result.Enabled,
Transforms: transforms,
Output: output,
}
// Apply transforms to the in-memory Application
if transforms != nil {
if err := h.applyTransformsToApplication(ctx, app, transforms); err != nil {
return ctx, nil, errors.Wrap(err, "failed to apply transforms")
}
ctx.Info("Applied cached transforms to Application", "policy", result.PolicyName)
// Apply output to Application spec
if output.Components != nil {
app.Spec.Components = output.Components
ctx.Info("Replaced components from output", "policy", result.PolicyName, "count", len(output.Components))
changes.SpecModified = true
}
if output.Workflow != nil {
app.Spec.Workflow = output.Workflow
ctx.Info("Replaced workflow from output", "policy", result.PolicyName)
changes.SpecModified = true
}
if output.Policies != nil {
app.Spec.Policies = output.Policies
ctx.Info("Replaced policies from output", "policy", result.PolicyName, "count", len(output.Policies))
changes.SpecModified = true
}
// Extract changes directly from transforms
if transforms.Labels != nil {
if labelsMap, ok := transforms.Labels.Value.(map[string]string); ok {
changes.AddedLabels = labelsMap
} else if labelsMap, ok := transforms.Labels.Value.(map[string]interface{}); ok {
// Convert from interface{} map
stringMap := make(map[string]string)
for k, v := range labelsMap {
if strVal, ok := v.(string); ok {
stringMap[k] = strVal
}
}
changes.AddedLabels = stringMap
}
// Apply labels (always merge)
if output.Labels != nil && len(output.Labels) > 0 {
if app.Labels == nil {
app.Labels = make(map[string]string)
}
if transforms.Annotations != nil {
if annotationsMap, ok := transforms.Annotations.Value.(map[string]string); ok {
changes.AddedAnnotations = annotationsMap
} else if annotationsMap, ok := transforms.Annotations.Value.(map[string]interface{}); ok {
// Convert from interface{} map
stringMap := make(map[string]string)
for k, v := range annotationsMap {
if strVal, ok := v.(string); ok {
stringMap[k] = strVal
}
}
changes.AddedAnnotations = stringMap
}
for k, v := range output.Labels {
app.Labels[k] = v
}
changes.AddedLabels = output.Labels
ctx.Info("Merged labels from output", "policy", result.PolicyName, "count", len(output.Labels))
}
// Check if spec was modified
changes.SpecModified = transforms.Spec != nil
// Apply annotations (always merge)
if output.Annotations != nil && len(output.Annotations) > 0 {
if app.Annotations == nil {
app.Annotations = make(map[string]string)
}
for k, v := range output.Annotations {
app.Annotations[k] = v
}
changes.AddedAnnotations = output.Annotations
ctx.Info("Merged annotations from output", "policy", result.PolicyName, "count", len(output.Annotations))
}
// Store additionalContext in context
@@ -765,66 +820,72 @@ type Transform struct {
Value interface{} `json:"value"`
}
// PolicyTransforms represents the allowed transformation operations
// PolicyTransforms represents the allowed transformation operations (old API)
type PolicyTransforms struct {
Spec *Transform `json:"spec,omitempty"`
Labels *Transform `json:"labels,omitempty"`
Annotations *Transform `json:"annotations,omitempty"`
}
// PolicyOutput represents the new simplified output structure (new API)
type PolicyOutput struct {
Components []common.ApplicationComponent `json:"components,omitempty"`
Workflow *v1beta1.Workflow `json:"workflow,omitempty"`
Policies []v1beta1.AppPolicy `json:"policies,omitempty"`
Labels map[string]string `json:"labels,omitempty"`
Annotations map[string]string `json:"annotations,omitempty"`
Ctx map[string]interface{} `json:"ctx,omitempty"`
}
// extractTransforms extracts the transforms field from rendered CUE
// Only spec, labels, and annotations with type+value structure are permitted
func (h *AppHandler) extractTransforms(val cue.Value) (*PolicyTransforms, error) {
transformsVal := val.LookupPath(cue.ParsePath("transforms"))
if !transformsVal.Exists() {
// No transforms field, that's OK
// No transforms field, that's OK - policy should use output API
return nil, nil
}
var transforms PolicyTransforms
if err := transformsVal.Decode(&transforms); err != nil {
return nil, errors.Wrap(err, "failed to decode transforms")
// Reject old transforms API - policies must use new output API
return nil, errors.New("the 'transforms' field is deprecated - please use 'output' field instead. See documentation for migration guide")
}
// extractOutput extracts the output field from rendered CUE (new API)
// Returns nil if output doesn't exist (old API being used or no output specified)
func (h *AppHandler) extractOutput(val cue.Value) (*PolicyOutput, error) {
outputVal := val.LookupPath(cue.ParsePath("output"))
if !outputVal.Exists() {
return nil, nil // No output field
}
// Validate structure: only 'spec', 'labels', and 'annotations' are allowed
iter, err := transformsVal.Fields()
var output PolicyOutput
if err := outputVal.Decode(&output); err != nil {
return nil, errors.Wrap(err, "failed to decode output")
}
// Validate structure: only allowed fields
iter, err := outputVal.Fields()
if err != nil {
return nil, errors.Wrap(err, "failed to iterate transforms fields")
return nil, errors.Wrap(err, "failed to iterate output fields")
}
allowedFields := map[string]bool{
"spec": true,
"components": true,
"workflow": true,
"policies": true,
"labels": true,
"annotations": true,
"ctx": true,
}
for iter.Next() {
fieldName := iter.Selector().String()
if !allowedFields[fieldName] {
return nil, errors.Errorf("transforms.%s is not allowed; only 'spec', 'labels', and 'annotations' are permitted", fieldName)
return nil, errors.Errorf("output.%s is not allowed; only 'components', 'workflow', 'policies', 'labels', 'annotations', and 'ctx' are permitted", fieldName)
}
}
// Validate each transform has correct type
if transforms.Spec != nil {
if err := validateTransformType(transforms.Spec.Type, "spec", true); err != nil {
return nil, err
}
}
if transforms.Labels != nil {
// Labels only support merge for safety
if err := validateTransformType(transforms.Labels.Type, "labels", false); err != nil {
return nil, err
}
}
if transforms.Annotations != nil {
// Annotations only support merge for safety
if err := validateTransformType(transforms.Annotations.Type, "annotations", false); err != nil {
return nil, err
}
}
return &transforms, nil
return &output, nil
}
// validateTransformType ensures the transform type is valid
@@ -1083,7 +1144,8 @@ type PolicyChanges struct {
// Full rendered output for caching/reuse
Enabled bool
Transforms *PolicyTransforms
Transforms *PolicyTransforms // Old transforms API
Output *PolicyOutput // New output API
}
// policyConfigMapMetadata tracks metadata needed for ConfigMap storage
@@ -1127,6 +1189,113 @@ func serializeTransformsForStorage(transforms *PolicyTransforms) map[string]inte
return result
}
// serializeOutputForStorage converts PolicyOutput to a format suitable for storage and reuse
func serializeOutputForStorage(output *PolicyOutput) map[string]interface{} {
if output == nil {
return nil
}
result := make(map[string]interface{})
if output.Components != nil {
result["components"] = output.Components
}
if output.Workflow != nil {
result["workflow"] = output.Workflow
}
if output.Policies != nil {
result["policies"] = output.Policies
}
if output.Labels != nil {
result["labels"] = output.Labels
}
if output.Annotations != nil {
result["annotations"] = output.Annotations
}
if output.Ctx != nil {
result["ctx"] = output.Ctx
}
return result
}
// deserializeOutputFromStorage converts stored output back to PolicyOutput
func deserializeOutputFromStorage(outputData map[string]interface{}) *PolicyOutput {
if outputData == nil {
return nil
}
output := &PolicyOutput{}
// Decode components
if componentsData, ok := outputData["components"]; ok {
jsonBytes, err := json.Marshal(componentsData)
if err != nil {
klog.Errorf("Failed to marshal components data: %v", err)
} else {
if err := json.Unmarshal(jsonBytes, &output.Components); err != nil {
klog.Errorf("Failed to unmarshal components: %v", err)
}
}
}
// Decode workflow
if workflowData, ok := outputData["workflow"]; ok {
jsonBytes, err := json.Marshal(workflowData)
if err != nil {
klog.Errorf("Failed to marshal workflow data: %v", err)
} else {
if err := json.Unmarshal(jsonBytes, &output.Workflow); err != nil {
klog.Errorf("Failed to unmarshal workflow: %v", err)
}
}
}
// Decode policies
if policiesData, ok := outputData["policies"]; ok {
jsonBytes, err := json.Marshal(policiesData)
if err != nil {
klog.Errorf("Failed to marshal policies data: %v", err)
} else {
if err := json.Unmarshal(jsonBytes, &output.Policies); err != nil {
klog.Errorf("Failed to unmarshal policies: %v", err)
}
}
}
// Decode labels
if labelsData, ok := outputData["labels"].(map[string]interface{}); ok {
output.Labels = make(map[string]string)
for k, v := range labelsData {
if strVal, ok := v.(string); ok {
output.Labels[k] = strVal
}
}
}
// Decode annotations
if annotationsData, ok := outputData["annotations"].(map[string]interface{}); ok {
output.Annotations = make(map[string]string)
for k, v := range annotationsData {
if strVal, ok := v.(string); ok {
output.Annotations[k] = strVal
}
}
}
// Decode ctx
if ctxData, ok := outputData["ctx"].(map[string]interface{}); ok {
output.Ctx = ctxData
}
return output
}
// loadCachedPolicyFromConfigMap attempts to load a cached policy result from the ConfigMap
// Returns the cached result if found and valid according to TTL and Application state, nil otherwise
func loadCachedPolicyFromConfigMap(ctx context.Context, cli client.Client, app *v1beta1.Application, policyName string, ttlSeconds int32) (map[string]interface{}, error) {
@@ -39,12 +39,9 @@ parameter: {}
enabled: true
transforms: {
output: {
labels: {
type: "merge"
value: {
"test": "value"
}
"test": "value"
}
}
`,
@@ -71,12 +68,9 @@ parameter: {
envName: string // Required field - no default!
}
transforms: {
output: {
labels: {
type: "merge"
value: {
"env": parameter.envName
}
"env": parameter.envName
}
}
`,
@@ -105,12 +99,9 @@ parameter: {
envName?: string // Optional but no default - can't compile!
}
transforms: {
output: {
labels: {
type: "merge"
value: {
"env": parameter.envName
}
"env": parameter.envName
}
}
`,
@@ -140,13 +131,10 @@ parameter: {
replicas: *3 | int // Has default
}
transforms: {
output: {
labels: {
type: "merge"
value: {
"env": parameter.envName
"replicas": "\(parameter.replicas)"
}
"env": parameter.envName
"replicas": "\(parameter.replicas)"
}
}
`,
@@ -171,12 +159,9 @@ transforms: {
Template: `
parameter: {} // Empty is fine
transforms: {
output: {
labels: {
type: "merge"
value: {
"static": "value"
}
"static": "value"
}
}
`,
@@ -273,142 +258,6 @@ parameter: {
Expect(result.Errors).Should(ContainElement(ContainSubstring("syntax error")))
})
It("Test labels transform must use 'merge' type", func() {
policy := &v1beta1.PolicyDefinition{
Spec: v1beta1.PolicyDefinitionSpec{
Scope: v1beta1.ApplicationScope,
Schematic: &common.Schematic{
CUE: &common.CUE{
Template: `
parameter: {}
transforms: {
labels: {
type: "replace" // Invalid! Must be "merge"
value: {
"test": "value"
}
}
}
`,
},
},
},
}
result := ValidatePolicyDefinition(policy)
Expect(result.IsValid()).Should(BeFalse())
Expect(result.Errors).Should(ContainElement(ContainSubstring("labels.type must be 'merge'")))
})
It("Test annotations transform must use 'merge' type", func() {
policy := &v1beta1.PolicyDefinition{
Spec: v1beta1.PolicyDefinitionSpec{
Scope: v1beta1.ApplicationScope,
Schematic: &common.Schematic{
CUE: &common.CUE{
Template: `
parameter: {}
transforms: {
annotations: {
type: "replace" // Invalid! Must be "merge"
value: {
"test": "value"
}
}
}
`,
},
},
},
}
result := ValidatePolicyDefinition(policy)
Expect(result.IsValid()).Should(BeFalse())
Expect(result.Errors).Should(ContainElement(ContainSubstring("annotations.type must be 'merge'")))
})
It("Test spec transform can use 'merge' or 'replace'", func() {
// Test with merge
policyMerge := &v1beta1.PolicyDefinition{
Spec: v1beta1.PolicyDefinitionSpec{
Scope: v1beta1.ApplicationScope,
Schematic: &common.Schematic{
CUE: &common.CUE{
Template: `
parameter: {}
transforms: {
spec: {
type: "merge"
value: {
components: []
}
}
}
`,
},
},
},
}
resultMerge := ValidatePolicyDefinition(policyMerge)
Expect(resultMerge.IsValid()).Should(BeTrue())
// Test with replace
policyReplace := &v1beta1.PolicyDefinition{
Spec: v1beta1.PolicyDefinitionSpec{
Scope: v1beta1.ApplicationScope,
Schematic: &common.Schematic{
CUE: &common.CUE{
Template: `
parameter: {}
transforms: {
spec: {
type: "replace"
value: {
components: []
}
}
}
`,
},
},
},
}
resultReplace := ValidatePolicyDefinition(policyReplace)
Expect(resultReplace.IsValid()).Should(BeTrue())
})
It("Test spec transform with invalid type fails", func() {
policy := &v1beta1.PolicyDefinition{
Spec: v1beta1.PolicyDefinitionSpec{
Scope: v1beta1.ApplicationScope,
Schematic: &common.Schematic{
CUE: &common.CUE{
Template: `
parameter: {}
transforms: {
spec: {
type: "invalid" // Invalid type!
value: {}
}
}
`,
},
},
},
}
result := ValidatePolicyDefinition(policy)
Expect(result.IsValid()).Should(BeFalse())
Expect(result.Errors).Should(ContainElement(ContainSubstring("'merge' or 'replace'")))
})
It("Test enabled field must be bool", func() {
policy := &v1beta1.PolicyDefinition{
Spec: v1beta1.PolicyDefinitionSpec{
@@ -443,12 +292,9 @@ parameter: {
envName: string // Required is OK for non-global policies
}
transforms: {
output: {
labels: {
type: "merge"
value: {
"env": parameter.envName
}
"env": parameter.envName
}
}
`,
@@ -477,7 +323,7 @@ transforms: {
Expect(result.Errors).Should(ContainElement(ContainSubstring("must have a CUE schematic")))
})
It("Test transform without type field fails", func() {
It("Test old transforms API is rejected with clear error", func() {
policy := &v1beta1.PolicyDefinition{
Spec: v1beta1.PolicyDefinitionSpec{
Scope: v1beta1.ApplicationScope,
@@ -488,7 +334,7 @@ parameter: {}
transforms: {
labels: {
// Missing 'type' field!
type: "merge"
value: {
"test": "value"
}
@@ -502,32 +348,7 @@ transforms: {
result := ValidatePolicyDefinition(policy)
Expect(result.IsValid()).Should(BeFalse())
Expect(result.Errors).Should(ContainElement(ContainSubstring("must have 'type' field")))
})
It("Test transform without value field fails", func() {
policy := &v1beta1.PolicyDefinition{
Spec: v1beta1.PolicyDefinitionSpec{
Scope: v1beta1.ApplicationScope,
Schematic: &common.Schematic{
CUE: &common.CUE{
Template: `
parameter: {}
transforms: {
labels: {
type: "merge"
// Missing 'value' field!
}
}
`,
},
},
},
}
result := ValidatePolicyDefinition(policy)
Expect(result.IsValid()).Should(BeFalse())
Expect(result.Errors).Should(ContainElement(ContainSubstring("must have 'value' field")))
Expect(result.Errors).Should(ContainElement(ContainSubstring("'transforms' field is deprecated")))
Expect(result.Errors).Should(ContainElement(ContainSubstring("use 'output' field instead")))
})
})
@@ -18,6 +18,7 @@ package application
import (
"context"
"encoding/json"
"sort"
"github.com/hashicorp/go-version"
@@ -130,6 +131,21 @@ func (h *AppHandler) gatherRevisionSpec(af *appfile.Appfile) (*v1beta1.Applicati
copiedApp := h.app.DeepCopy()
// We better to remove all object status in the appRevision
copiedApp.Status = common.AppStatus{}
// Normalize component properties (RawExtension) to ensure consistent JSON encoding
// This prevents spurious revision creation due to JSON field order/formatting differences
for i := range copiedApp.Spec.Components {
if copiedApp.Spec.Components[i].Properties != nil && copiedApp.Spec.Components[i].Properties.Raw != nil {
// Decode and re-encode to normalize JSON
var obj map[string]interface{}
if err := json.Unmarshal(copiedApp.Spec.Components[i].Properties.Raw, &obj); err == nil {
if normalized, err := json.Marshal(obj); err == nil {
copiedApp.Spec.Components[i].Properties.Raw = normalized
}
}
}
}
appRev := &v1beta1.ApplicationRevision{
Spec: v1beta1.ApplicationRevisionSpec{
ApplicationRevisionCompressibleFields: v1beta1.ApplicationRevisionCompressibleFields{
@@ -346,9 +362,11 @@ func (h *AppHandler) currentAppRevIsNew(ctx context.Context) (bool, bool, error)
for _, _rev := range revs {
rev := _rev.DeepCopy()
if rev.GetLabels()[oam.LabelAppRevisionHash] == h.currentRevHash &&
DeepEqualRevision(rev, h.currentAppRev) &&
oam.GetPublishVersion(rev) == oam.GetPublishVersion(h.app) {
hashMatches := rev.GetLabels()[oam.LabelAppRevisionHash] == h.currentRevHash
deepEqual := DeepEqualRevision(rev, h.currentAppRev)
publishVersionMatches := oam.GetPublishVersion(rev) == oam.GetPublishVersion(h.app)
if hashMatches && deepEqual && publishVersionMatches {
// we set currentAppRev to existRevision
h.currentAppRev = rev
return true, false, nil
@@ -507,6 +525,7 @@ func (h *AppHandler) UpdateAppLatestRevisionStatus(ctx context.Context, patchSta
// skip update if app revision is not changed
return nil
}
if ctx, ok := ctx.(monitorContext.Context); ok {
subCtx := ctx.Fork("update-apprev-status", monitorContext.DurationMetric(func(v float64) {
metrics.AppReconcileStageDurationHistogram.WithLabelValues("update-apprev-status").Observe(v)
@@ -520,13 +539,23 @@ func (h *AppHandler) UpdateAppLatestRevisionStatus(ctx context.Context, patchSta
Revision: int64(revNum),
RevisionHash: h.currentRevHash,
}
// Save the spec before patchStatus - the merge patch operation refreshes the entire app from API server
// This would lose any in-memory policy modifications to app.Spec
savedSpec := h.app.Spec.DeepCopy()
if err := patchStatus(ctx, h.app, common.ApplicationRendering); err != nil {
klog.InfoS("Failed to update the latest appConfig revision to status", "application", klog.KObj(h.app),
"latest revision", revName, "err", err)
return err
}
// Restore the spec after patchStatus to preserve policy modifications
h.app.Spec = *savedSpec
klog.InfoS("Successfully update application latest revision status", "application", klog.KObj(h.app),
"latest revision", revName)
return nil
}