diff --git a/internal/metastructure/patch/empty_value_fidelity.go b/internal/metastructure/patch/empty_value_fidelity.go new file mode 100644 index 000000000..3a3f26f39 --- /dev/null +++ b/internal/metastructure/patch/empty_value_fidelity.go @@ -0,0 +1,104 @@ +// © 2026 Platform Engineering Labs Inc. +// +// SPDX-License-Identifier: FSL-1.1-ALv2 + +package patch + +import ( + "strings" + + "github.com/tidwall/gjson" + + pkgmodel "github.com/platform-engineering-labs/formae/pkg/model" +) + +// firstPointerSegment returns the first segment of an RFC 6901 JSON Pointer, +// unescaped (~1 -> /, then ~0 -> ~). ok is false for anything that is not a +// pointer with at least one non-empty segment. Matching fidelity roots by +// first segment (never by string prefix) is what keeps "/Specification" from +// matching a root named "Spec". +func firstPointerSegment(path string) (string, bool) { + rest, found := strings.CutPrefix(path, "/") + if !found || rest == "" { + return "", false + } + seg, _, _ := strings.Cut(rest, "/") + if seg == "" { + return "", false + } + seg = strings.ReplaceAll(seg, "~1", "/") + seg = strings.ReplaceAll(seg, "~0", "~") + return seg, true +} + +// isKeepableReferenceEnvelope reports whether a desired-side value is a +// reference envelope well-formed enough that its flattened placeholder must +// survive the top-level empty-value drop: either a $ref that parses to a +// formae URI with a KSUID, or a complete $res declaration. $ref wins when +// both appear (the translated form is authoritative). Deliberately stricter +// than occurrence collection: an envelope with no usable reference identity +// is not kept, so a malformed object can never mint a placeholder op. +func isKeepableReferenceEnvelope(value gjson.Result) bool { + if !value.IsObject() { + return false + } + if ref := value.Get("$ref"); ref.Exists() { + return ref.Type == gjson.String && pkgmodel.FormaeURI(ref.String()).KSUID() != "" + } + if res := value.Get("$res"); res.Exists() { + if !res.IsBool() || !res.Bool() { + return false + } + for _, key := range []string{"$label", "$type", "$stack", "$property"} { + member := value.Get(key) + if member.Type != gjson.String || member.String() == "" { + return false + } + } + return true + } + return false +} + +// referenceEnvelopeFields returns the schema fields whose desired pre-flatten +// value is a keepable reference envelope. Absent, malformed, or non-object +// input contributes nothing: the failure direction is always "field dropped +// as today", never "non-envelope kept". CreateOnly destinations are excluded +// outright: a kept placeholder there could only ever surface as a +// replacement, and a replacement must never be planned from a value that has +// not resolved - an import-shaped source whose property is never written +// would otherwise destroy its consumer. +func referenceEnvelopeFields(desiredEnvelopes []byte, schema pkgmodel.Schema) map[string]bool { + fields := map[string]bool{} + if len(desiredEnvelopes) == 0 { + return fields + } + parsed := gjson.ParseBytes(desiredEnvelopes) + if !parsed.IsObject() { + return fields + } + for _, field := range schema.Fields { + if schema.Hints[field].CreateOnly { + continue + } + if isKeepableReferenceEnvelope(parsed.Get(field)) { + fields[field] = true + } + } + return fields +} + +// PreserveEmptyRootFields returns the literal top-level rendered property +// names whose FieldHint sets PreserveEmptyValues, derived from schema.Hints +// keys directly. Dotted hint keys (nested subresource hints) are skipped: +// fidelity is top-level-scoped, and a nested hint keeps whatever other +// meaning it carries with no fidelity behavior anywhere. +func PreserveEmptyRootFields(schema pkgmodel.Schema) map[string]bool { + fields := map[string]bool{} + for name, hint := range schema.Hints { + if hint.PreserveEmptyValues && !strings.Contains(name, ".") { + fields[name] = true + } + } + return fields +} diff --git a/internal/metastructure/patch/empty_value_fidelity_test.go b/internal/metastructure/patch/empty_value_fidelity_test.go new file mode 100644 index 000000000..50c1066ef --- /dev/null +++ b/internal/metastructure/patch/empty_value_fidelity_test.go @@ -0,0 +1,294 @@ +// © 2026 Platform Engineering Labs Inc. +// +// SPDX-License-Identifier: FSL-1.1-ALv2 + +//go:build unit + +package patch + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/tidwall/gjson" + + pkgmodel "github.com/platform-engineering-labs/formae/pkg/model" + + "github.com/platform-engineering-labs/formae/internal/metastructure/resolver" +) + +func TestFirstPointerSegment(t *testing.T) { + cases := []struct { + path string + want string + ok bool + }{ + {"/Spec", "Spec", true}, + {"/Spec/x", "Spec", true}, + {"/Spec/0/y", "Spec", true}, + {"/Specification", "Specification", true}, + {"/a~1b/c", "a/b", true}, + {"/a~0b", "a~b", true}, + {"/a~01", "a~1", true}, + {"", "", false}, + {"Spec", "", false}, + {"/", "", false}, + } + for _, c := range cases { + got, ok := firstPointerSegment(c.path) + assert.Equal(t, c.ok, ok, "path %q ok", c.path) + if c.ok { + assert.Equal(t, c.want, got, "path %q", c.path) + } + } +} + +func TestIsKeepableReferenceEnvelope(t *testing.T) { + keep := []string{ + `{"$ref":"formae://2ABcDeFgHiJkLmNoPqRsTuVwXyZ#/SecretString"}`, + `{"$ref":"formae://2ABcDeFgHiJkLmNoPqRsTuVwXyZ#/S","$value":"x","$visibility":"Opaque"}`, + `{"$res":true,"$label":"a","$type":"T::B","$stack":"s","$property":"P"}`, + `{"$res":true,"$label":"a","$type":"T::B","$stack":"s","$property":"P","$json":"host"}`, + } + drop := []string{ + `{"$res":false,"$label":"a","$type":"T::B","$stack":"s","$property":"P"}`, + `{"$res":true,"$label":"a","$type":"T::B","$stack":"s"}`, + `{"$res":true,"$label":"","$type":"T::B","$stack":"s","$property":"P"}`, + `{"$ref":42}`, + `{"$ref":"formae://#/S"}`, + `{"$ref":"not-a-uri"}`, + `{"$ref":"formae://#/S","$res":true,"$label":"a","$type":"T","$stack":"s","$property":"P"}`, + `"plain"`, + `{}`, + `[]`, + } + for _, s := range keep { + assert.True(t, isKeepableReferenceEnvelope(gjson.Parse(s)), "must keep %s", s) + } + for _, s := range drop { + assert.False(t, isKeepableReferenceEnvelope(gjson.Parse(s)), "must drop %s", s) + } +} + +func TestReferenceEnvelopeFields(t *testing.T) { + desired := []byte(`{ + "Token": {"$ref": "formae://2ABcDeFgHiJkLmNoPqRsTuVwXyZ#/S"}, + "Bad": {"$res": true}, + "Plain": "", + "Extra": {"$ref": "formae://2ABcDeFgHiJkLmNoPqRsTuVwXyZ#/T"} + }`) + fields := referenceEnvelopeFields(desired, pkgmodel.Schema{Fields: []string{"Token", "Bad", "Plain"}}) + assert.Equal(t, map[string]bool{"Token": true}, fields, + "only well-formed envelopes on schema fields enter the keep-set") + assert.Empty(t, referenceEnvelopeFields(nil, pkgmodel.Schema{Fields: []string{"Token"}})) + assert.Empty(t, referenceEnvelopeFields([]byte(`not json`), pkgmodel.Schema{Fields: []string{"Token"}})) + assert.Empty(t, referenceEnvelopeFields(desired, pkgmodel.Schema{ + Fields: []string{"Token"}, + Hints: map[string]pkgmodel.FieldHint{"Token": {CreateOnly: true}}, + }), "createOnly destinations never enter the keep-set") +} + +func TestPreserveEmptyRootFields(t *testing.T) { + schema := pkgmodel.Schema{ + Fields: []string{"Spec", "Other", "Nested"}, + Hints: map[string]pkgmodel.FieldHint{ + "Spec": {UpdateMethod: pkgmodel.FieldUpdateMethodAtomic, PreserveEmptyValues: true}, + "Other": {UpdateMethod: pkgmodel.FieldUpdateMethodAtomic}, + "Nested.sub": {PreserveEmptyValues: true}, + "Bare": {PreserveEmptyValues: true}, + }, + } + assert.Equal(t, map[string]bool{"Spec": true, "Bare": true}, PreserveEmptyRootFields(schema), + "only preserveEmptyValues hints on non-dotted keys enter the root set") + assert.Empty(t, PreserveEmptyRootFields(pkgmodel.Schema{})) +} + +func fidelitySchema() pkgmodel.Schema { + return pkgmodel.Schema{ + Identifier: "Name", + Fields: []string{"Name", "Spec", "Other"}, + Hints: map[string]pkgmodel.FieldHint{ + "Spec": {UpdateMethod: pkgmodel.FieldUpdateMethodAtomic, PreserveEmptyValues: true}, + }, + } +} + +// The headline shape: a hinted field's empty-object member survives to the +// single whole-value replace op. Requires both the diff-input exemption and +// the op-value exemption; red until both land. +func TestGeneratePatch_PreserveEmpty_ReplaceCarriesVerbatimValue(t *testing.T) { + document := json.RawMessage(`{"Name":"x","Spec":{"acme":{"server":"https://old"}}}`) + desired := json.RawMessage(`{"Name":"x","Spec":{"selfSigned":{}}}`) + + patchDoc, _, _, err := GeneratePatch(document, desired, document, desired, resolver.ResolvableProperties{}, fidelitySchema(), pkgmodel.FormaApplyModeReconcile) + require.NoError(t, err) + + var ops []struct { + Op string `json:"op"` + Path string `json:"path"` + Value json.RawMessage `json:"value"` + } + require.NoError(t, json.Unmarshal(patchDoc, &ops)) + require.Len(t, ops, 1) + assert.Equal(t, "replace", ops[0].Op) + assert.Equal(t, "/Spec", ops[0].Path) + assert.JSONEq(t, `{"selfSigned":{}}`, string(ops[0].Value), + "the empty-object member is the declaration and must survive") +} + +// Symmetry: identical values incl. empties on both sides plan nothing. +func TestGeneratePatch_PreserveEmpty_IdenticalSidesPlanNothing(t *testing.T) { + doc := json.RawMessage(`{"Name":"x","Spec":{"selfSigned":{}}}`) + + patchDoc, _, _, err := GeneratePatch(doc, doc, doc, doc, resolver.ResolvableProperties{}, fidelitySchema(), pkgmodel.FormaApplyModeReconcile) + require.NoError(t, err) + assert.Empty(t, patchDoc) +} + +// The default is pinned, not just the exemption: a field without the hint +// keeps today's stripping even when another field carries it. +func TestGeneratePatch_NonHintedFieldStillStripped(t *testing.T) { + document := json.RawMessage(`{"Name":"x","Other":{"acme":{"server":"https://old"}}}`) + desired := json.RawMessage(`{"Name":"x","Other":{"selfSigned":{}}}`) + + patchDoc, _, _, err := GeneratePatch(document, desired, document, desired, resolver.ResolvableProperties{}, fidelitySchema(), pkgmodel.FormaApplyModeReconcile) + require.NoError(t, err) + assert.NotContains(t, string(patchDoc), "selfSigned", + "unhinted fields keep the rendering-noise strip") +} + +func refSchema() pkgmodel.Schema { + return pkgmodel.Schema{ + Identifier: "Name", + Fields: []string{"Name", "Token", "Other"}, + Hints: map[string]pkgmodel.FieldHint{"Name": {CreateOnly: true}}, + } +} + +// A first-declared unresolvable reference survives as a placeholder add op; +// a plain empty string is still dropped as rendering noise. +func TestGeneratePatch_ReferenceEnvelopeAddSurvives(t *testing.T) { + document := json.RawMessage(`{"Name":"x"}`) + desired := json.RawMessage(`{"Name":"x","Token":{"$ref":"formae://2ABcDeFgHiJkLmNoPqRsTuVwXyZ#/S"}}`) + + patchDoc, _, _, err := GeneratePatch(document, desired, document, desired, resolver.ResolvableProperties{}, refSchema(), pkgmodel.FormaApplyModeReconcile) + require.NoError(t, err) + assert.JSONEq(t, `[{"op":"add","path":"/Token","value":""}]`, string(patchDoc)) + + plain := json.RawMessage(`{"Name":"x","Token":""}`) + patchDoc, _, _, err = GeneratePatch(document, plain, document, plain, resolver.ResolvableProperties{}, refSchema(), pkgmodel.FormaApplyModeReconcile) + require.NoError(t, err) + assert.Empty(t, patchDoc, "a plain empty string is still rendering noise") +} + +// A malformed envelope contributes nothing to the keep-set: current behavior +// (silent drop) is preserved rather than minting a placeholder. +func TestGeneratePatch_MalformedEnvelopeNotKept(t *testing.T) { + document := json.RawMessage(`{"Name":"x"}`) + desired := json.RawMessage(`{"Name":"x","Token":{"$res":true}}`) + + patchDoc, _, _, err := GeneratePatch(document, desired, document, desired, resolver.ResolvableProperties{}, refSchema(), pkgmodel.FormaApplyModeReconcile) + require.NoError(t, err) + assert.NotContains(t, string(patchDoc), `"value":""`, + "the keep-set must never mint an empty-string placeholder for a malformed envelope") + assert.JSONEq(t, `[{"op":"add","path":"/Token","value":{"$res":true}}]`, string(patchDoc), + "a malformed envelope keeps its pre-existing plain-map diff behavior, unchanged by the keep-set") +} + +// Keeping a field only permits diffing: equal values still produce no op. +func TestGeneratePatch_KeptFieldEqualValuesNoOp(t *testing.T) { + document := json.RawMessage(`{"Name":"x","Token":"v"}`) + desired := json.RawMessage(`{"Name":"x","Token":{"$ref":"formae://2ABcDeFgHiJkLmNoPqRsTuVwXyZ#/S","$value":"v"}}`) + + patchDoc, _, _, err := GeneratePatch(document, desired, document, desired, resolver.ResolvableProperties{}, refSchema(), pkgmodel.FormaApplyModeReconcile) + require.NoError(t, err) + assert.Empty(t, patchDoc) +} + +// GREEN-FIRST GUARD: a preserveEmptyValues root omitted from desired while +// the document holds a bare empty stays invisible (the absence-scoped +// provider-echo tolerance). Fails exactly if an implementation wrongly +// exempts preserved roots from that tolerance. +func TestGeneratePatch_PreservedRootAbsentDesired_NoRemove(t *testing.T) { + document := json.RawMessage(`{"Name":"x","Spec":{}}`) + desired := json.RawMessage(`{"Name":"x"}`) + + patchDoc, _, _, err := GeneratePatch(document, desired, document, desired, resolver.ResolvableProperties{}, fidelitySchema(), pkgmodel.FormaApplyModeReconcile) + require.NoError(t, err) + assert.NotContains(t, string(patchDoc), "remove") +} + +// The reference placeholder add survives under Patch mode too. +func TestGeneratePatch_ReferenceAddSurvivesPatchMode(t *testing.T) { + document := json.RawMessage(`{"Name":"x"}`) + desired := json.RawMessage(`{"Name":"x","Token":{"$ref":"formae://2ABcDeFgHiJkLmNoPqRsTuVwXyZ#/S"}}`) + + patchDoc, _, _, err := GeneratePatch(document, desired, document, desired, resolver.ResolvableProperties{}, refSchema(), pkgmodel.FormaApplyModePatch) + require.NoError(t, err) + assert.JSONEq(t, `[{"op":"add","path":"/Token","value":""}]`, string(patchDoc)) +} + +// A reference on a createOnly destination never mints a placeholder op: a +// kept placeholder there could only surface as a replacement, and a +// replacement is never planned from a value that has not resolved. +func TestGeneratePatch_ReferenceOnCreateOnlyDestination_NoPlaceholder(t *testing.T) { + schema := pkgmodel.Schema{ + Identifier: "Name", + Fields: []string{"Name", "Token"}, + Hints: map[string]pkgmodel.FieldHint{"Token": {CreateOnly: true}}, + } + document := json.RawMessage(`{"Name":"x"}`) + desired := json.RawMessage(`{"Name":"x","Token":{"$ref":"formae://2ABcDeFgHiJkLmNoPqRsTuVwXyZ#/S"}}`) + + patchDoc, createOnly, _, err := GeneratePatch(document, desired, document, desired, resolver.ResolvableProperties{}, schema, pkgmodel.FormaApplyModeReconcile) + require.NoError(t, err) + assert.Empty(t, createOnly, "no replacement may be planned from an unresolved placeholder") + assert.NotContains(t, string(patchDoc), "/Token") +} + +// The accepted churn contract: an actual-side empty-shaped extra inside a +// preserved field makes the whole value differ, producing exactly one +// replace carrying the desired value verbatim. +func TestGeneratePatch_PreserveEmpty_ActualExtraEmptyChurnsAsWholeReplace(t *testing.T) { + document := json.RawMessage(`{"Name":"x","Spec":{"selfSigned":{},"defaulted":{}}}`) + desired := json.RawMessage(`{"Name":"x","Spec":{"selfSigned":{}}}`) + + patchDoc, _, _, err := GeneratePatch(document, desired, document, desired, resolver.ResolvableProperties{}, fidelitySchema(), pkgmodel.FormaApplyModeReconcile) + require.NoError(t, err) + assert.JSONEq(t, `[{"op":"replace","path":"/Spec","value":{"selfSigned":{}}}]`, string(patchDoc)) +} + +// The no-change guarantee for existing plugins: Atomic WITHOUT +// preserveEmptyValues keeps exactly today's behavior - nested empties are +// stripped on both sides and equalize the diff. +func TestGeneratePatch_AtomicWithoutPreserve_KeepsStripBehavior(t *testing.T) { + schema := pkgmodel.Schema{ + Identifier: "Name", + Fields: []string{"Name", "Doc"}, + Hints: map[string]pkgmodel.FieldHint{"Doc": {UpdateMethod: pkgmodel.FieldUpdateMethodAtomic}}, + } + document := json.RawMessage(`{"Name":"x","Doc":{"stmt":{"cond":{}}}}`) + desired := json.RawMessage(`{"Name":"x","Doc":{"stmt":{}}}`) + + patchDoc, _, _, err := GeneratePatch(document, desired, document, desired, resolver.ResolvableProperties{}, schema, pkgmodel.FormaApplyModeReconcile) + require.NoError(t, err) + assert.Empty(t, patchDoc, + "symmetric stripping still equalizes empty-shaped differences for atomic-only fields") +} + +// A dotted (nested subresource) hint key grants no fidelity anywhere. +func TestGeneratePatch_DottedPreserveHint_NoFidelity(t *testing.T) { + schema := pkgmodel.Schema{ + Identifier: "Name", + Fields: []string{"Name", "Config"}, + Hints: map[string]pkgmodel.FieldHint{"Config.records": {PreserveEmptyValues: true}}, + } + document := json.RawMessage(`{"Name":"x","Config":{"records":{"a":{}}}}`) + desired := json.RawMessage(`{"Name":"x","Config":{"records":{}}}`) + + patchDoc, _, _, err := GeneratePatch(document, desired, document, desired, resolver.ResolvableProperties{}, schema, pkgmodel.FormaApplyModeReconcile) + require.NoError(t, err) + assert.Empty(t, patchDoc, "nested hints are out of fidelity scope; today's stripping applies") +} diff --git a/internal/metastructure/patch/patch_document.go b/internal/metastructure/patch/patch_document.go index 857bd870a..3599382b8 100644 --- a/internal/metastructure/patch/patch_document.go +++ b/internal/metastructure/patch/patch_document.go @@ -112,7 +112,18 @@ func generatePatch(document []byte, patch []byte, storedEnvelopes []byte, desire } requiredOnUpdateFields := schema.RequiredOnUpdate() - patchOps, err := createPatchDocument(flattenedDocument, flattenedPatch, schema.Fields, requiredOnUpdateFields, schema.HasProviderDefault(), entitySetProviderDefaultsFromHints(schema.Hints), collectionSemanticsFromFieldHints(schema.Hints), defaultIgnoredFields, strategy, topLevelConvergeFields(schema.Fields, properties)) + // Fields hinted preserveEmptyValues carry meaningful empty collections: + // every empty-collection normalization below skips their subtrees, on + // both sides and in op values. The top-level empty-STRING drop is + // deliberately not exempted: the hint speaks about collections, which + // pass that filter anyway, and a preserved field rendered "" by an unset + // nullable declaration is still rendering noise. + preserveRoots := PreserveEmptyRootFields(schema) + keepFields := topLevelConvergeFields(schema.Fields, properties) + for field := range referenceEnvelopeFields(desiredEnvelopes, schema) { + keepFields[field] = true + } + patchOps, err := createPatchDocument(flattenedDocument, flattenedPatch, schema.Fields, requiredOnUpdateFields, schema.HasProviderDefault(), entitySetProviderDefaultsFromHints(schema.Hints), collectionSemanticsFromFieldHints(schema.Hints), defaultIgnoredFields, strategy, keepFields, preserveRoots) if err != nil { return nil, nil, false, fmt.Errorf("failed to create patch document: %w", err) } @@ -122,14 +133,14 @@ func generatePatch(document []byte, patch []byte, storedEnvelopes []byte, desire // []/{}. An "add" of an empty collection to a field absent in the actual // state is always PKL rendering noise — a user clearing a field would // produce a "replace" (field exists in actual), not an "add". - patchOps = filterSpuriousEmptyAdds(patchOps) + patchOps = filterSpuriousEmptyAdds(patchOps, preserveRoots) // Strip empty collections from inside all patch operation values. This // cleans up phantom []/{} values inside nested objects (e.g., empty // ResponseParameters inside an IntegrationResponse). Without this, // EntitySet array elements may not match their actual counterparts and // produce "array items are not unique" errors. - patchOps = stripEmptyCollectionsFromOps(patchOps) + patchOps = stripEmptyCollectionsFromOps(patchOps, preserveRoots) // Drop serialization-only ops on Format-hinted fields before the createOnly // split, so a cosmetic diff on a (possibly createOnly) hinted field neither @@ -194,7 +205,7 @@ func generatePatch(document []byte, patch []byte, storedEnvelopes []byte, desire return json.RawMessage(patchJson), createOnlyJson, onlyForceResent, nil } -func createPatchDocument(document []byte, patch []byte, schemaFields []string, requiredOnUpdateFields []string, hasProviderDefaultFields []string, entitySetProviderDefaults map[string]string, collections jsonpatch.Collections, ignoredFields []jsonpatch.Path, strategy jsonpatch.PatchStrategy, convergeFields map[string]bool) ([]jsonpatch.JsonPatchOperation, error) { +func createPatchDocument(document []byte, patch []byte, schemaFields []string, requiredOnUpdateFields []string, hasProviderDefaultFields []string, entitySetProviderDefaults map[string]string, collections jsonpatch.Collections, ignoredFields []jsonpatch.Path, strategy jsonpatch.PatchStrategy, convergeFields map[string]bool, preserveRoots map[string]bool) ([]jsonpatch.JsonPatchOperation, error) { patchWithSchemaFieldsOnly, err := removeNonSchemaFields(patch, schemaFields, convergeFields) if err != nil { return nil, err @@ -245,7 +256,7 @@ func createPatchDocument(document []byte, patch []byte, schemaFields []string, r // nullable Listing/Mapping fields as []/{}. Without stripping, EntitySet // element matching fails because elements have different shapes (one has // phantom empty fields, the other doesn't), causing duplicate entries. - cleanedDesired, err := StripNestedEmptyCollections(patchWithSchemaFieldsOnly) + cleanedDesired, err := StripNestedEmptyCollectionsExcept(patchWithSchemaFieldsOnly, preserveRoots) if err != nil { return nil, err } @@ -259,7 +270,7 @@ func createPatchDocument(document []byte, patch []byte, schemaFields []string, r return nil, err } - cleanedDocument, err := StripNestedEmptyCollections(documentMinusTopEmpties) + cleanedDocument, err := StripNestedEmptyCollectionsExcept(documentMinusTopEmpties, preserveRoots) if err != nil { return nil, err } @@ -735,12 +746,23 @@ func removeNonSchemaFields(patch []byte, schemaFields []string, convergeFields m // resource updater (before sending Properties to plugins for Create/Update) // to clean PKL rendering artifacts (null → []/{} from nullable Listing/Mapping fields). func StripNestedEmptyCollections(data []byte) ([]byte, error) { + return StripNestedEmptyCollectionsExcept(data, nil) +} + +// StripNestedEmptyCollectionsExcept is StripNestedEmptyCollections with an +// exemption set: subtrees rooted at a top-level key in preserveRoots are left +// byte-for-byte intact — their empty collections are values, not rendering +// noise (the preserveEmptyValues field hint). +func StripNestedEmptyCollectionsExcept(data []byte, preserveRoots map[string]bool) ([]byte, error) { var doc map[string]any if err := json.Unmarshal(data, &doc); err != nil { return nil, fmt.Errorf("StripNestedEmptyCollections: invalid JSON: %w", err) } for k, v := range doc { + if preserveRoots[k] { + continue + } doc[k] = stripEmptyCollectionsFromValue(v) } @@ -752,9 +774,13 @@ func StripNestedEmptyCollections(data []byte) ([]byte, error) { // as []/{}. An "add" means the field is absent in the actual state, so adding // an empty collection is never user intent — it's PKL rendering noise. A user // clearing an existing field produces a "replace" (field exists), not "add". -func filterSpuriousEmptyAdds(patchOps []jsonpatch.JsonPatchOperation) []jsonpatch.JsonPatchOperation { +func filterSpuriousEmptyAdds(patchOps []jsonpatch.JsonPatchOperation, preserveRoots map[string]bool) []jsonpatch.JsonPatchOperation { filtered := make([]jsonpatch.JsonPatchOperation, 0, len(patchOps)) for _, op := range patchOps { + if opUnderPreservedRoot(op.Path, preserveRoots) { + filtered = append(filtered, op) + continue + } if op.Operation == "add" && isEmptyCollection(op.Value) { continue } @@ -809,13 +835,27 @@ func dropCanonicallyEqualHintedOps(ops []jsonpatch.JsonPatchOperation, document, // stripEmptyCollectionsFromOps recursively removes empty arrays and maps from // inside all patch operation values. This ensures that EntitySet element // matching works correctly when elements contain phantom []/{} values. -func stripEmptyCollectionsFromOps(patchOps []jsonpatch.JsonPatchOperation) []jsonpatch.JsonPatchOperation { +func stripEmptyCollectionsFromOps(patchOps []jsonpatch.JsonPatchOperation, preserveRoots map[string]bool) []jsonpatch.JsonPatchOperation { for i := range patchOps { + if opUnderPreservedRoot(patchOps[i].Path, preserveRoots) { + continue + } patchOps[i].Value = stripEmptyCollectionsFromValue(patchOps[i].Value) } return patchOps } +// opUnderPreservedRoot reports whether an op's JSON Pointer path is at or +// under a preserveEmptyValues root, matched by first segment (never by +// string prefix). +func opUnderPreservedRoot(path string, preserveRoots map[string]bool) bool { + if len(preserveRoots) == 0 { + return false + } + seg, ok := firstPointerSegment(path) + return ok && preserveRoots[seg] +} + func stripEmptyCollectionsFromValue(val any) any { switch v := val.(type) { case map[string]any: diff --git a/internal/metastructure/patch/patch_document_test.go b/internal/metastructure/patch/patch_document_test.go index 43d7e38e7..5ee1885b8 100644 --- a/internal/metastructure/patch/patch_document_test.go +++ b/internal/metastructure/patch/patch_document_test.go @@ -158,7 +158,7 @@ func TestCreatePatchDocument_PrimitiveArray(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - patches, err := createPatchDocument(tc.jsonA, tc.jsonB, []string{"label", "tags"}, nil, nil, nil, jsonpatch.Collections{}, nil, jsonpatch.PatchStrategyEnsureExists, nil) + patches, err := createPatchDocument(tc.jsonA, tc.jsonB, []string{"label", "tags"}, nil, nil, nil, jsonpatch.Collections{}, nil, jsonpatch.PatchStrategyEnsureExists, nil, nil) if err != nil { t.Fatalf("Error comparing JSONs: %v", err) } @@ -230,7 +230,7 @@ func TestCreatePatchDocument_ObjectArrayWithKeyValues(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - patches, err := createPatchDocument(tc.jsonA, tc.jsonB, []string{"label", "tags"}, nil, nil, nil, jsonpatch.Collections{EntitySets: jsonpatch.EntitySets{jsonpatch.Path("$.tags"): jsonpatch.Key("key")}}, nil, jsonpatch.PatchStrategyEnsureExists, nil) + patches, err := createPatchDocument(tc.jsonA, tc.jsonB, []string{"label", "tags"}, nil, nil, nil, jsonpatch.Collections{EntitySets: jsonpatch.EntitySets{jsonpatch.Path("$.tags"): jsonpatch.Key("key")}}, nil, jsonpatch.PatchStrategyEnsureExists, nil, nil) if err != nil { t.Fatalf("Error comparing JSONs: %v", err) } @@ -304,7 +304,7 @@ func TestCreatePatchDocument_ObjectArrayWithValues(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - patches, err := createPatchDocument(tc.jsonA, tc.jsonB, []string{"label", "tags"}, nil, nil, nil, jsonpatch.Collections{}, nil, jsonpatch.PatchStrategyEnsureExists, nil) + patches, err := createPatchDocument(tc.jsonA, tc.jsonB, []string{"label", "tags"}, nil, nil, nil, jsonpatch.Collections{}, nil, jsonpatch.PatchStrategyEnsureExists, nil, nil) if err != nil { t.Fatalf("Error comparing JSONs: %v", err) } diff --git a/internal/metastructure/resource_update/atomic_conversion_test.go b/internal/metastructure/resource_update/atomic_conversion_test.go new file mode 100644 index 000000000..ed1646a37 --- /dev/null +++ b/internal/metastructure/resource_update/atomic_conversion_test.go @@ -0,0 +1,72 @@ +// © 2026 Platform Engineering Labs Inc. +// +// SPDX-License-Identifier: FSL-1.1-ALv2 + +//go:build unit + +package resource_update + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + pkgmodel "github.com/platform-engineering-labs/formae/pkg/model" +) + +// Plugin-bound conversion preserves empty collections inside a +// preserveEmptyValues-hinted field, in both the write converter and the +// Read-context converter; unhinted fields keep the rendering-noise strip. +func TestConvertResourceForPlugin_PreserveEmptyFieldSurvives(t *testing.T) { + res := pkgmodel.Resource{ + Label: "cr", Type: "Test::Custom", + Schema: pkgmodel.Schema{ + Identifier: "Name", + Fields: []string{"Name", "Spec", "Other"}, + Hints: map[string]pkgmodel.FieldHint{ + "Spec": {UpdateMethod: pkgmodel.FieldUpdateMethodAtomic, PreserveEmptyValues: true}, + }, + }, + Properties: json.RawMessage(`{"Name":"cr","Spec":{"selfSigned":{}},"Other":{"empty":{}}}`), + } + + converted, err := convertResourceForPlugin(res) + require.NoError(t, err) + assert.JSONEq(t, `{"selfSigned":{}}`, extractField(t, converted.Properties, "Spec"), + "write payload keeps the hinted field verbatim") + assert.JSONEq(t, `{}`, extractField(t, converted.Properties, "Other"), + "unhinted fields keep the strip") + + readConverted, err := convertResourceForPluginRead(res) + require.NoError(t, err) + assert.JSONEq(t, `{"selfSigned":{}}`, extractField(t, readConverted.Properties, "Spec"), + "Read/sync/delete context carries the true value too") +} + +func extractField(t *testing.T, props json.RawMessage, field string) string { + t.Helper() + var m map[string]json.RawMessage + require.NoError(t, json.Unmarshal(props, &m)) + return string(m[field]) +} + +// The persist merge keeps empty collections under a preserveEmptyValues root +// even when the plugin echoes nothing; elsewhere the leaf-only walk drops +// them as before. +func TestMerge_PreserveEmptyRootSurvivesEmptyPluginEcho(t *testing.T) { + schema := pkgmodel.Schema{ + Fields: []string{"ApiVersion", "Spec", "Other"}, + Hints: map[string]pkgmodel.FieldHint{"Spec": {UpdateMethod: pkgmodel.FieldUpdateMethodAtomic, PreserveEmptyValues: true}}, + } + user := json.RawMessage(`{"ApiVersion":"v1","Spec":{"selfSigned":{"crl":[]}},"Other":{"e":{}}}`) + + merged, err := mergeRefsPreservingUserRefs(user, json.RawMessage(`{}`), schema, true, nil) + require.NoError(t, err) + assert.JSONEq(t, `{"selfSigned":{"crl":[]}}`, extractField(t, merged, "Spec"), + "the hinted subtree persists verbatim, nested empty list included") + var m map[string]json.RawMessage + require.NoError(t, json.Unmarshal(merged, &m)) + assert.NotContains(t, m, "Other", "unhinted empty-leaved objects keep today's drop") +} diff --git a/internal/metastructure/resource_update/resource_update.go b/internal/metastructure/resource_update/resource_update.go index b25bee288..0b6a58fe4 100644 --- a/internal/metastructure/resource_update/resource_update.go +++ b/internal/metastructure/resource_update/resource_update.go @@ -746,6 +746,15 @@ func (m *propertyMerger) mergeObject(path string, userVal, pluginVal gjson.Resul return } + // An empty user object writes no leaves, so the recursion below would + // drop it from the merged document entirely. Under a preserveEmptyValues + // root the empty object IS the value and must persist. + if len(userVal.Map()) == 0 && m.underPreservedRoot(path) { + cleanPath := m.cleanPath(path) + *m.result, _ = sjson.SetRaw(*m.result, cleanPath, "{}") + return + } + // Not a $ref or $embed object - recursively merge each field userVal.ForEach(func(key, val gjson.Result) bool { childPath := m.buildChildPath(path, key.String()) @@ -755,6 +764,16 @@ func (m *propertyMerger) mergeObject(path string, userVal, pluginVal gjson.Resul }) } +// underPreservedRoot reports whether a merge path's top-level field carries +// the preserveEmptyValues hint. +func (m *propertyMerger) underPreservedRoot(path string) bool { + if path == "" { + return false + } + root, _, _ := strings.Cut(path, ".") + return patch.PreserveEmptyRootFields(m.schema)[root] +} + // mergeRefObject handles merging of $ref objects (resolvable references) func (m *propertyMerger) mergeRefObject(path string, userVal, pluginVal gjson.Result) { cleanPath := m.cleanPath(path) @@ -977,6 +996,14 @@ func (m *propertyMerger) mergeArray(path string, userVal, pluginVal gjson.Result userArray := userVal.Array() pluginArray := pluginVal.Array() + // An empty user array writes no elements; under a preserveEmptyValues + // root it is the value and must persist (mirror of the empty-object case). + if len(userArray) == 0 && m.underPreservedRoot(path) { + cleanPath := m.cleanPath(path) + *m.result, _ = sjson.SetRaw(*m.result, cleanPath, "[]") + return + } + // Resolve this array's own hint by its index-less full path (e.g. // "ContainerDefinitions.0.Environment" -> "ContainerDefinitions.Environment"), // mirroring the diff-calculator's hint-key convention. A nested array must resolve diff --git a/internal/metastructure/resource_update/resource_updater.go b/internal/metastructure/resource_update/resource_updater.go index b605d0552..de0071ac8 100644 --- a/internal/metastructure/resource_update/resource_updater.go +++ b/internal/metastructure/resource_update/resource_updater.go @@ -74,8 +74,10 @@ func convertResourceForPluginWith(res pkgmodel.Resource, convert func(json.RawMe } // Strip nested empty collections from PKL null rendering artifacts. - // Top-level empty collections are preserved (may be intentional clears). - cleanedProps, err := patch.StripNestedEmptyCollections(convertedProps) + // Top-level empty collections are preserved (may be intentional clears), + // and preserveEmptyValues-hinted fields keep their subtrees verbatim in + // every plugin-bound context: their empties are values, not artifacts. + cleanedProps, err := patch.StripNestedEmptyCollectionsExcept(convertedProps, patch.PreserveEmptyRootFields(res.Schema)) if err != nil { return res, err } diff --git a/internal/schema/pkl/schema/formae.pkl b/internal/schema/pkl/schema/formae.pkl index 27e07eec0..ff97cb582 100644 --- a/internal/schema/pkl/schema/formae.pkl +++ b/internal/schema/pkl/schema/formae.pkl @@ -263,7 +263,10 @@ open class FieldHint extends Annotation { hidden edgeKind: EdgeKind = "default" // NEW hidden indexField: String? hidden updateMethod: FieldUpdateMethod? - hidden format: String = "" // NEW: serialized-content format (e.g. "json"); PLA-196 + /// Opts this field's subtree out of empty-value normalization: empty + /// collections inside it are treated as values and preserved end to end. + hidden preserveEmptyValues: Boolean = false + hidden format: String = "" // serialized-content format (e.g. "json") hidden outputField: String? hidden outputTransformation: ((Any) -> Any)? @@ -280,6 +283,7 @@ open class FieldHint extends Annotation { fixed AttachesTo: Boolean = attachesTo fixed EdgeKind: String = edgeKind fixed UpdateMethod: String = updateMethod ?? "" + fixed PreserveEmptyValues: Boolean = preserveEmptyValues fixed IndexField: String = indexField ?? "" fixed Format: String = format // NEW } diff --git a/internal/schema/pkl/schema/tests/formae.pkl-expected.pcf b/internal/schema/pkl/schema/tests/formae.pkl-expected.pcf index 122387892..09566c10e 100644 --- a/internal/schema/pkl/schema/tests/formae.pkl-expected.pcf +++ b/internal/schema/pkl/schema/tests/formae.pkl-expected.pcf @@ -123,6 +123,7 @@ examples { AttachesTo = false EdgeKind = "default" UpdateMethod = "" + PreserveEmptyValues = false IndexField = "" Format = "" } @@ -137,6 +138,7 @@ examples { AttachesTo = false EdgeKind = "default" UpdateMethod = "" + PreserveEmptyValues = false IndexField = "" Format = "" } @@ -151,6 +153,7 @@ examples { AttachesTo = false EdgeKind = "default" UpdateMethod = "" + PreserveEmptyValues = false IndexField = "" Format = "" } @@ -176,6 +179,7 @@ examples { AttachesTo = false EdgeKind = "default" UpdateMethod = "" + PreserveEmptyValues = false IndexField = "" Format = "" } @@ -190,6 +194,7 @@ examples { AttachesTo = false EdgeKind = "default" UpdateMethod = "" + PreserveEmptyValues = false IndexField = "" Format = "" } @@ -204,6 +209,7 @@ examples { AttachesTo = false EdgeKind = "default" UpdateMethod = "" + PreserveEmptyValues = false IndexField = "" Format = "" } @@ -260,6 +266,7 @@ examples { AttachesTo = false EdgeKind = "default" UpdateMethod = "" + PreserveEmptyValues = false IndexField = "" Format = "" } @@ -274,6 +281,7 @@ examples { AttachesTo = false EdgeKind = "default" UpdateMethod = "" + PreserveEmptyValues = false IndexField = "" Format = "" } @@ -288,6 +296,7 @@ examples { AttachesTo = false EdgeKind = "default" UpdateMethod = "" + PreserveEmptyValues = false IndexField = "" Format = "" } @@ -352,6 +361,7 @@ examples { AttachesTo = false EdgeKind = "default" UpdateMethod = "" + PreserveEmptyValues = false IndexField = "" Format = "" } @@ -366,6 +376,7 @@ examples { AttachesTo = false EdgeKind = "default" UpdateMethod = "" + PreserveEmptyValues = false IndexField = "" Format = "" } @@ -380,6 +391,7 @@ examples { AttachesTo = false EdgeKind = "default" UpdateMethod = "" + PreserveEmptyValues = false IndexField = "" Format = "" } @@ -428,6 +440,7 @@ examples { AttachesTo = false EdgeKind = "default" UpdateMethod = "" + PreserveEmptyValues = false IndexField = "" Format = "" } @@ -442,6 +455,7 @@ examples { AttachesTo = false EdgeKind = "default" UpdateMethod = "" + PreserveEmptyValues = false IndexField = "" Format = "" } @@ -456,6 +470,7 @@ examples { AttachesTo = false EdgeKind = "default" UpdateMethod = "" + PreserveEmptyValues = false IndexField = "" Format = "" } @@ -470,6 +485,7 @@ examples { AttachesTo = false EdgeKind = "default" UpdateMethod = "" + PreserveEmptyValues = false IndexField = "" Format = "" } @@ -484,6 +500,7 @@ examples { AttachesTo = false EdgeKind = "default" UpdateMethod = "" + PreserveEmptyValues = false IndexField = "" Format = "" } @@ -498,6 +515,7 @@ examples { AttachesTo = false EdgeKind = "default" UpdateMethod = "" + PreserveEmptyValues = false IndexField = "" Format = "" } @@ -512,6 +530,7 @@ examples { AttachesTo = false EdgeKind = "default" UpdateMethod = "" + PreserveEmptyValues = false IndexField = "" Format = "" } @@ -555,6 +574,7 @@ examples { AttachesTo = false EdgeKind = "default" UpdateMethod = "" + PreserveEmptyValues = false IndexField = "" Format = "" } @@ -569,6 +589,7 @@ examples { AttachesTo = false EdgeKind = "default" UpdateMethod = "" + PreserveEmptyValues = false IndexField = "" Format = "" } @@ -583,6 +604,7 @@ examples { AttachesTo = false EdgeKind = "default" UpdateMethod = "" + PreserveEmptyValues = false IndexField = "" Format = "" } @@ -601,6 +623,7 @@ examples { AttachesTo = false EdgeKind = "default" UpdateMethod = "" + PreserveEmptyValues = false IndexField = "" Format = "" } @@ -615,6 +638,7 @@ examples { AttachesTo = false EdgeKind = "default" UpdateMethod = "" + PreserveEmptyValues = false IndexField = "" Format = "" } @@ -633,6 +657,7 @@ examples { AttachesTo = false EdgeKind = "default" UpdateMethod = "" + PreserveEmptyValues = false IndexField = "" Format = "" } @@ -647,6 +672,7 @@ examples { AttachesTo = false EdgeKind = "default" UpdateMethod = "" + PreserveEmptyValues = false IndexField = "" Format = "" } @@ -672,6 +698,7 @@ examples { AttachesTo = false EdgeKind = "default" UpdateMethod = "" + PreserveEmptyValues = false IndexField = "" Format = "" } @@ -686,6 +713,7 @@ examples { AttachesTo = false EdgeKind = "default" UpdateMethod = "" + PreserveEmptyValues = false IndexField = "" Format = "" } @@ -700,6 +728,7 @@ examples { AttachesTo = false EdgeKind = "default" UpdateMethod = "" + PreserveEmptyValues = false IndexField = "" Format = "" } diff --git a/internal/testplugin/fakeaws/fake_aws.go b/internal/testplugin/fakeaws/fake_aws.go index 991ef5ea7..dca2e0b8a 100644 --- a/internal/testplugin/fakeaws/fake_aws.go +++ b/internal/testplugin/fakeaws/fake_aws.go @@ -117,6 +117,13 @@ func (s *FakeAWS) SupportedResources() []plugin.ResourceDescriptor { Type: secretsManagerSecretType, Discoverable: false, }, + // Custom::Resource models an opaque-bodied resource whose Spec is a + // user-owned document: hinted Atomic (whole-value diff) and + // preserveEmptyValues (empty collections inside it are values). + { + Type: "FakeAWS::Custom::Resource", + Discoverable: false, + }, } } @@ -172,6 +179,14 @@ func (s *FakeAWS) SchemaForResourceType(resourceType string) (model.Schema, erro "SecretString": {Opaque: true}, }, }, nil + case "FakeAWS::Custom::Resource": + return model.Schema{ + Identifier: "FormaeId", + Fields: []string{"ApiVersion", "Kind", "FormaeId", "Spec"}, + Hints: map[string]model.FieldHint{ + "Spec": {UpdateMethod: model.FieldUpdateMethodAtomic, PreserveEmptyValues: true}, + }, + }, nil default: return model.Schema{ Identifier: "BucketName", diff --git a/internal/workflow_tests/local/apply_forma/atomic_fidelity_test.go b/internal/workflow_tests/local/apply_forma/atomic_fidelity_test.go new file mode 100644 index 000000000..0af4496b8 --- /dev/null +++ b/internal/workflow_tests/local/apply_forma/atomic_fidelity_test.go @@ -0,0 +1,113 @@ +// © 2026 Platform Engineering Labs Inc. +// +// SPDX-License-Identifier: FSL-1.1-ALv2 + +//go:build unit + +package workflow_tests_local + +import ( + "encoding/json" + "sync/atomic" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/tidwall/gjson" + + "github.com/platform-engineering-labs/formae/internal/metastructure/config" + "github.com/platform-engineering-labs/formae/internal/metastructure/testutil" + "github.com/platform-engineering-labs/formae/internal/metastructure/util" + "github.com/platform-engineering-labs/formae/internal/workflow_tests/test_helpers" + pkgmodel "github.com/platform-engineering-labs/formae/pkg/model" + "github.com/platform-engineering-labs/formae/pkg/plugin" + "github.com/platform-engineering-labs/formae/pkg/plugin/resource" +) + +func customResource(stack, spec string) pkgmodel.Resource { + return pkgmodel.Resource{ + Label: "issuer", + Type: "FakeAWS::Custom::Resource", + Stack: stack, + Target: "test-target", + Schema: pkgmodel.Schema{ + Identifier: "FormaeId", + Fields: []string{"ApiVersion", "Kind", "FormaeId", "Spec"}, + Hints: map[string]pkgmodel.FieldHint{ + "Spec": {UpdateMethod: pkgmodel.FieldUpdateMethodAtomic, PreserveEmptyValues: true}, + }, + }, + Properties: json.RawMessage(`{ + "ApiVersion": "cert-manager.io/v1", + "Kind": "ClusterIssuer", + "FormaeId": "cert-manager.io/v1/ClusterIssuer//issuer", + "Spec": ` + spec + ` + }`), + } +} + +// A preserveEmptyValues-hinted field whose declaration IS an empty object +// member reaches the plugin verbatim on create, re-applies clean, and a later +// change arrives as one whole-field value still carrying the empty member. +func TestApplyForma_PreserveEmptySpec_VerbatimCreateCleanReapplyWholeValueUpdate(t *testing.T) { + testutil.RunTestFromProjectRoot(t, func(t *testing.T) { + var createSpec, updateSpec atomic.Value + var updateCalls atomic.Int32 + overrides := &plugin.ResourcePluginOverrides{ + Create: func(req *resource.CreateRequest) (*resource.CreateResult, error) { + createSpec.Store(gjson.GetBytes(req.Properties, "Spec").Raw) + return &resource.CreateResult{ProgressResult: &resource.ProgressResult{ + Operation: resource.OperationCreate, + OperationStatus: resource.OperationStatusSuccess, + RequestID: "cr-create-1", + NativeID: "cr-native-1", + }}, nil + }, + Update: func(req *resource.UpdateRequest) (*resource.UpdateResult, error) { + updateCalls.Add(1) + updateSpec.Store(gjson.GetBytes(req.DesiredProperties, "Spec").Raw) + return &resource.UpdateResult{ProgressResult: &resource.ProgressResult{ + Operation: resource.OperationUpdate, + OperationStatus: resource.OperationStatusSuccess, + RequestID: "cr-update-1", + NativeID: "cr-native-1", + }}, nil + }, + } + + cfg := test_helpers.NewTestMetastructureConfig() + cfg.Agent.Synchronization.Enabled = false + m, def, err := test_helpers.NewTestMetastructureWithConfig(t, overrides, cfg) + defer def() + require.NoError(t, err) + + stack := "test-stack-" + util.NewID() + targets := []pkgmodel.Target{{Label: "test-target", Namespace: "test-namespace"}} + forma := func(spec string) *pkgmodel.Forma { + return &pkgmodel.Forma{ + Stacks: []pkgmodel.Stack{{Label: stack}}, + Resources: []pkgmodel.Resource{customResource(stack, spec)}, + Targets: targets, + } + } + + _, err = m.ApplyForma(forma(`{"selfSigned":{}}`), &config.FormaCommandConfig{Mode: pkgmodel.FormaApplyModeReconcile}, "test-client-id", "", "") + require.NoError(t, err) + waitForApplyComplete(t, m) + created, _ := createSpec.Load().(string) + assert.JSONEq(t, `{"selfSigned":{}}`, created, + "the plugin must receive the empty-object member verbatim on create") + + resp, err := m.ApplyForma(forma(`{"selfSigned":{}}`), &config.FormaCommandConfig{Mode: pkgmodel.FormaApplyModeReconcile}, "test-client-id", "", "") + require.NoError(t, err) + assert.False(t, resp.Simulation.ChangesRequired, "identical re-apply must plan nothing") + + _, err = m.ApplyForma(forma(`{"selfSigned":{},"other":"x"}`), &config.FormaCommandConfig{Mode: pkgmodel.FormaApplyModeReconcile}, "test-client-id", "", "") + require.NoError(t, err) + waitForApplyComplete(t, m) + require.Greater(t, updateCalls.Load(), int32(0), "the change must reach the plugin") + updated, _ := updateSpec.Load().(string) + assert.JSONEq(t, `{"selfSigned":{},"other":"x"}`, updated, + "the update carries the whole field still holding the empty member") + }) +} diff --git a/internal/workflow_tests/local/apply_forma/secret_consumer_convergence_test.go b/internal/workflow_tests/local/apply_forma/secret_consumer_convergence_test.go index d4b42fcb5..216dd2208 100644 --- a/internal/workflow_tests/local/apply_forma/secret_consumer_convergence_test.go +++ b/internal/workflow_tests/local/apply_forma/secret_consumer_convergence_test.go @@ -8,6 +8,7 @@ package workflow_tests_local import ( "encoding/json" + "strings" "sync" "sync/atomic" "testing" @@ -373,3 +374,87 @@ func TestApplyForma_SecretConsumer_RepointPlansAndDelivers(t *testing.T) { "the consumer must receive the new source's value") }) } + +// An existing resource gains a NEW reference field to a hashed secret in the +// same apply as an ordinary field change: the update plans, dispatch waits +// for resolution, and the plugin receives BOTH the resolved secret and the +// ordinary change, with no placeholder left anywhere. +func TestApplyForma_NewSecretReferenceOnExistingResource_PlansAndDelivers(t *testing.T) { + testutil.RunTestFromProjectRoot(t, func(t *testing.T) { + const secretV1 = "first-declared-secret" + + var consumerUpdateCalls atomic.Int32 + var consumerUpdateProps atomic.Value + overrides := secretConsumerOverrides(&consumerUpdateCalls, &consumerUpdateProps) + + cfg := test_helpers.NewTestMetastructureConfig() + cfg.Agent.Synchronization.Enabled = false + m, def, err := test_helpers.NewTestMetastructureWithConfig(t, overrides, cfg) + defer def() + require.NoError(t, err) + + stack := "test-stack-" + util.NewID() + targets := []pkgmodel.Target{{Label: "test-target", Namespace: "test-namespace"}} + + bareConsumer := pkgmodel.Resource{ + Label: "my-bucket", Type: "FakeAWS::S3::Bucket", Stack: stack, Target: "test-target", + Schema: secretConsumerSchema(), + Properties: json.RawMessage(`{"BucketName":"my-bucket","AccessControl":"Private"}`), + } + _, err = m.ApplyForma(&pkgmodel.Forma{ + Stacks: []pkgmodel.Stack{{Label: stack}}, + Resources: []pkgmodel.Resource{secretResource(stack, "my-secret", secretV1), bareConsumer}, + Targets: targets, + }, &config.FormaCommandConfig{Mode: pkgmodel.FormaApplyModeReconcile}, "test-client-id", "", "") + require.NoError(t, err) + waitForApplyComplete(t, m) + + // The mixed change: the consumer gains DbPassword (a reference that + // cannot resolve at plan time) AND changes AccessControl. + withRef := secretConsumer(stack, "my-secret") + withRefProps := string(withRef.Properties) + withRef.Properties = json.RawMessage(strings.Replace(withRefProps, `"AccessControl": "Private"`, `"AccessControl": "PublicRead"`, 1)) + resp, err := m.ApplyForma(&pkgmodel.Forma{ + Stacks: []pkgmodel.Stack{{Label: stack}}, + Resources: []pkgmodel.Resource{secretResource(stack, "my-secret", secretV1), withRef}, + Targets: targets, + }, &config.FormaCommandConfig{Mode: pkgmodel.FormaApplyModeReconcile}, "test-client-id", "", "") + require.NoError(t, err) + require.True(t, resp.Simulation.ChangesRequired, "the new reference must plan") + waitForApplyComplete(t, m) + + require.Greater(t, consumerUpdateCalls.Load(), int32(0), "the consumer plugin must be invoked") + props, _ := consumerUpdateProps.Load().(json.RawMessage) + assert.Equal(t, secretV1, gjson.GetBytes(props, "DbPassword").String(), + "the plugin receives the resolved secret, never a placeholder") + assert.Equal(t, "PublicRead", gjson.GetBytes(props, "AccessControl").String(), + "the ordinary change rides the same update") + + // Steady state: the written occurrence is stamped; re-apply plans nothing. + resp, err = m.ApplyForma(&pkgmodel.Forma{ + Stacks: []pkgmodel.Stack{{Label: stack}}, + Resources: []pkgmodel.Resource{secretResource(stack, "my-secret", secretV1), withRef}, + Targets: targets, + }, &config.FormaCommandConfig{Mode: pkgmodel.FormaApplyModeReconcile}, "test-client-id", "", "") + require.NoError(t, err) + assert.False(t, resp.Simulation.ChangesRequired, "re-apply after the first declaration plans nothing") + + // The persisted executed patch keeps the placeholder, never the + // plaintext: the live value exists only in memory and the in-flight + // plugin request (the delivery assertions above), and at rest the + // occurrence op stays a resolution placeholder. + cmds, err := m.Datastore.LoadFormaCommands() + require.NoError(t, err) + for _, c := range cmds { + updates, err := m.Datastore.LoadResourceUpdates(c.ID) + require.NoError(t, err) + for _, ru := range updates { + if ru.DesiredState.Label != "my-bucket" { + continue + } + assert.NotContains(t, string(ru.DesiredState.PatchDocument), secretV1, + "the persisted executed patch must never carry the plaintext") + } + } + }) +} diff --git a/pkg/model/schema.go b/pkg/model/schema.go index b6b9ad410..d5e5482bc 100644 --- a/pkg/model/schema.go +++ b/pkg/model/schema.go @@ -51,7 +51,12 @@ type FieldHint struct { IndexField string `json:"IndexField" pkl:"IndexField"` UpdateMethod FieldUpdateMethod `json:"UpdateMethod" pkl:"UpdateMethod"` - Format string `json:"Format" pkl:"Format"` // "" = opaque String; "json" = serialized JSON document + // PreserveEmptyValues opts a top-level field's subtree out of empty-value + // normalization: empty collections inside it are values, preserved on both + // diff sides, in op values, and in plugin-bound payloads. Orthogonal to + // UpdateMethod; typically paired with Atomic on opaque document fields. + PreserveEmptyValues bool `json:"PreserveEmptyValues" pkl:"PreserveEmptyValues"` + Format string `json:"Format" pkl:"Format"` // "" = opaque String; "json" = serialized JSON document } // UnmarshalJSON normalizes the deprecated AttachesTo alias into EdgeKind so