From bb5c0fd8ee68532db52471b98f274fc38c944368 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Thu, 6 Aug 2026 16:04:47 +0200 Subject: [PATCH 1/5] Fix drift for UC secrets values --- .../bundle/invariant/configs/secret.yml.tmpl | 18 ++++++++++++++++++ .../invariant/configs/secret.yml.tmpl-init.sh | 1 + .../bundle/invariant/continue_293/test.toml | 3 +++ .../invariant/delete_idempotent/out.test.toml | 1 + .../invariant/destroy_idempotent/out.test.toml | 1 + acceptance/bundle/invariant/migrate/test.toml | 3 +++ .../bundle/invariant/no_drift/out.test.toml | 1 + acceptance/bundle/invariant/no_drift/test.toml | 5 +++++ acceptance/bundle/invariant/test.toml | 1 + bundle/direct/bundle_plan.go | 9 +++++++-- libs/structs/structaccess/get.go | 9 ++++++++- libs/structs/structaccess/typecheck.go | 16 +++++++++++++--- libs/structs/structdiff/diff.go | 7 ++++--- 13 files changed, 66 insertions(+), 9 deletions(-) create mode 100644 acceptance/bundle/invariant/configs/secret.yml.tmpl create mode 100755 acceptance/bundle/invariant/configs/secret.yml.tmpl-init.sh diff --git a/acceptance/bundle/invariant/configs/secret.yml.tmpl b/acceptance/bundle/invariant/configs/secret.yml.tmpl new file mode 100644 index 00000000000..2218b827032 --- /dev/null +++ b/acceptance/bundle/invariant/configs/secret.yml.tmpl @@ -0,0 +1,18 @@ +bundle: + name: test-bundle-$UNIQUE_NAME + +variables: + secret_value: + description: The value of the secret + +resources: + secrets: + foo: + catalog_name: main + schema_name: default + name: test-secret-$UNIQUE_NAME + value: ${var.secret_value} + grants: + - principal: account users + privileges: + - MANAGE diff --git a/acceptance/bundle/invariant/configs/secret.yml.tmpl-init.sh b/acceptance/bundle/invariant/configs/secret.yml.tmpl-init.sh new file mode 100755 index 00000000000..8ec839a40d3 --- /dev/null +++ b/acceptance/bundle/invariant/configs/secret.yml.tmpl-init.sh @@ -0,0 +1 @@ +export BUNDLE_VAR_secret_value="secret-value" diff --git a/acceptance/bundle/invariant/continue_293/test.toml b/acceptance/bundle/invariant/continue_293/test.toml index c6fba9c43fb..1289c80af5e 100644 --- a/acceptance/bundle/invariant/continue_293/test.toml +++ b/acceptance/bundle/invariant/continue_293/test.toml @@ -29,3 +29,6 @@ EnvMatrixExclude.no_volume_path_job_ref = ["INPUT_CONFIG=volume_path_job_ref.yml # The 1000-task scale case is covered by no_drift. Running it here adds ~1.5 min # per variant (two full deploys at 1000 tasks) without incremental coverage. EnvMatrixExclude.no_pydabs_1000_tasks = ["INPUT_CONFIG=job_pydabs_1000_tasks.yml.tmpl"] + +# secret resource is not supported on v0.293.0 +EnvMatrixExclude.no_secret = ["INPUT_CONFIG=secret.yml.tmpl"] diff --git a/acceptance/bundle/invariant/delete_idempotent/out.test.toml b/acceptance/bundle/invariant/delete_idempotent/out.test.toml index f65b1680aa1..31e2851a46e 100644 --- a/acceptance/bundle/invariant/delete_idempotent/out.test.toml +++ b/acceptance/bundle/invariant/delete_idempotent/out.test.toml @@ -47,6 +47,7 @@ EnvMatrix.INPUT_CONFIG = [ "schema_empty_grants.yml.tmpl", "schema_grant_ref.yml.tmpl", "schema_uppercase_name.yml.tmpl", + "secret.yml.tmpl", "secret_scope.yml.tmpl", "secret_scope_default_backend_type.yml.tmpl", "sql_warehouse.yml.tmpl", diff --git a/acceptance/bundle/invariant/destroy_idempotent/out.test.toml b/acceptance/bundle/invariant/destroy_idempotent/out.test.toml index f65b1680aa1..31e2851a46e 100644 --- a/acceptance/bundle/invariant/destroy_idempotent/out.test.toml +++ b/acceptance/bundle/invariant/destroy_idempotent/out.test.toml @@ -47,6 +47,7 @@ EnvMatrix.INPUT_CONFIG = [ "schema_empty_grants.yml.tmpl", "schema_grant_ref.yml.tmpl", "schema_uppercase_name.yml.tmpl", + "secret.yml.tmpl", "secret_scope.yml.tmpl", "secret_scope_default_backend_type.yml.tmpl", "sql_warehouse.yml.tmpl", diff --git a/acceptance/bundle/invariant/migrate/test.toml b/acceptance/bundle/invariant/migrate/test.toml index bb2337b32aa..2579e755f66 100644 --- a/acceptance/bundle/invariant/migrate/test.toml +++ b/acceptance/bundle/invariant/migrate/test.toml @@ -37,3 +37,6 @@ EnvMatrixExclude.no_pydabs_1000_tasks = ["INPUT_CONFIG=job_pydabs_1000_tasks.yml # migrate deploys via Terraform first, and the TF provider rejects an uppercase # volume schema_name ("inconsistent final plan"). Covered by no_drift on direct. EnvMatrixExclude.no_volume_uppercase = ["INPUT_CONFIG=volume_uppercase_name.yml.tmpl"] + +# secret resource is not supported in terraform mode +EnvMatrixExclude.no_secret = ["INPUT_CONFIG=secret.yml.tmpl"] diff --git a/acceptance/bundle/invariant/no_drift/out.test.toml b/acceptance/bundle/invariant/no_drift/out.test.toml index f65b1680aa1..31e2851a46e 100644 --- a/acceptance/bundle/invariant/no_drift/out.test.toml +++ b/acceptance/bundle/invariant/no_drift/out.test.toml @@ -47,6 +47,7 @@ EnvMatrix.INPUT_CONFIG = [ "schema_empty_grants.yml.tmpl", "schema_grant_ref.yml.tmpl", "schema_uppercase_name.yml.tmpl", + "secret.yml.tmpl", "secret_scope.yml.tmpl", "secret_scope_default_backend_type.yml.tmpl", "sql_warehouse.yml.tmpl", diff --git a/acceptance/bundle/invariant/no_drift/test.toml b/acceptance/bundle/invariant/no_drift/test.toml index ff8a66c196e..1e7f00ada30 100644 --- a/acceptance/bundle/invariant/no_drift/test.toml +++ b/acceptance/bundle/invariant/no_drift/test.toml @@ -1 +1,6 @@ EnvMatrix.READPLAN = ["", "1"] + +[EnvMatrixExclude] +# Secrets have sensitive fields (json:"-") that are stripped when a plan is serialized to JSON, +# so deploying from a pre-computed plan file creates the secret with an empty value. +no_secret_with_readplan = ["READPLAN=1", "INPUT_CONFIG=secret.yml.tmpl"] diff --git a/acceptance/bundle/invariant/test.toml b/acceptance/bundle/invariant/test.toml index 1d0d883f6d5..583fbf7cb66 100644 --- a/acceptance/bundle/invariant/test.toml +++ b/acceptance/bundle/invariant/test.toml @@ -65,6 +65,7 @@ EnvMatrix.INPUT_CONFIG = [ "schema_empty_grants.yml.tmpl", "schema_grant_ref.yml.tmpl", "schema_uppercase_name.yml.tmpl", + "secret.yml.tmpl", "secret_scope.yml.tmpl", "secret_scope_default_backend_type.yml.tmpl", "sql_warehouse.yml.tmpl", diff --git a/bundle/direct/bundle_plan.go b/bundle/direct/bundle_plan.go index 5ee4c8469a9..b99bdac770a 100644 --- a/bundle/direct/bundle_plan.go +++ b/bundle/direct/bundle_plan.go @@ -391,7 +391,12 @@ func addPerFieldActions(ctx context.Context, adapter *dresources.Adapter, change // backend-managed/input-only), or absent from RemoteType (a guaranteed-nil // placeholder, since RemapState is a dumb copy). Otherwise a coincidental // new == remote (both nil, say) wrongly skips a real local change. - if structdiff.IsEqual(ch.Remote, ch.New) && !ignoreRemoteChanges(cfg, generatedCfg, path) && !isFieldMissingInRemote(adapter, path) { + // + // Exception: a field absent from RemoteType but non-nil in ch.Remote was explicitly + // populated by RemapState (e.g. SecretValue from EffectiveValue). That remote value + // IS meaningful, so honour RemoteAlreadySet when it matches ch.New. + isMissingInRemote := isFieldMissingInRemote(adapter, path) + if structdiff.IsEqual(ch.Remote, ch.New) && !ignoreRemoteChanges(cfg, generatedCfg, path) && (!isMissingInRemote || ch.Remote != nil) { ch.Action = deployplan.Skip ch.Reason = deployplan.ReasonRemoteAlreadySet } else if allEmpty(ch.Old, ch.New, ch.Remote) { @@ -421,7 +426,7 @@ func addPerFieldActions(ctx context.Context, adapter *dresources.Adapter, change } else if reason, ok := shouldSkipNormalized(generatedCfg, path, ch); ok { ch.Action = deployplan.Skip ch.Reason = reason - } else if isFieldMissingInRemote(adapter, path) && structdiff.IsEqual(ch.Old, ch.New) { + } else if isMissingInRemote && structdiff.IsEqual(ch.Old, ch.New) { ch.Action = deployplan.Skip ch.Reason = deployplan.ReasonMissingInRemote } else if reason, ok := findMatchingRule(path, cfg.RecreateOnChanges); ok { diff --git a/libs/structs/structaccess/get.go b/libs/structs/structaccess/get.go index bf8c96df80d..2119f7fb107 100644 --- a/libs/structs/structaccess/get.go +++ b/libs/structs/structaccess/get.go @@ -249,7 +249,15 @@ func findFieldInStruct(v reflect.Value, key string) (reflect.Value, reflect.Stru // Read JSON tag using structtag helper name := structtag.JSONTag(sf.Tag.Get("json")).Name() + + btag := structtag.BundleTag(sf.Tag.Get("bundle")) + + // Sensitive fields use json:"-" to avoid serialization but are still diffed + // in memory under their Go field name. Allow lookup by Go field name for them. if name == "-" { + if btag.Sensitive() && sf.Name == key { + return v.Field(i), sf, true + } name = "" } @@ -258,7 +266,6 @@ func findFieldInStruct(v reflect.Value, key string) (reflect.Value, reflect.Stru } if name != "" && name == key { // Skip fields marked as internal or readonly via bundle tag - btag := structtag.BundleTag(sf.Tag.Get("bundle")) if btag.Internal() || btag.ReadOnly() { continue } diff --git a/libs/structs/structaccess/typecheck.go b/libs/structs/structaccess/typecheck.go index 7147fa0f435..50ebf5978c6 100644 --- a/libs/structs/structaccess/typecheck.go +++ b/libs/structs/structaccess/typecheck.go @@ -152,12 +152,22 @@ func FindStructFieldByKeyType(t reflect.Type, key string) (reflect.StructField, continue } name := structtag.JSONTag(sf.Tag.Get("json")).Name() - if name == "-" || sf.Name == EmbeddedSliceFieldName { + btag := structtag.BundleTag(sf.Tag.Get("bundle")) + + // Sensitive fields use json:"-" to avoid serialization but are diffed + // under their Go field name. Allow lookup by Go field name for them. + if name == "-" { + if btag.Sensitive() && sf.Name == key { + return sf, t, true + } name = "" } - if name != "" && name == key { + + if name == "" || sf.Name == EmbeddedSliceFieldName { + continue + } + if name == key { // Skip fields marked as internal/readonly - btag := structtag.BundleTag(sf.Tag.Get("bundle")) if btag.Internal() || btag.ReadOnly() { continue } diff --git a/libs/structs/structdiff/diff.go b/libs/structs/structdiff/diff.go index 4e9df593994..41be3ba64d5 100644 --- a/libs/structs/structdiff/diff.go +++ b/libs/structs/structdiff/diff.go @@ -209,9 +209,10 @@ func diffStruct(ctx *diffContext, path *structpath.PathNode, s1, s2 reflect.Valu jsonTag := structtag.JSONTag(sf.Tag.Get("json")) bundleTag := structtag.BundleTag(sf.Tag.Get("bundle")) - // Resolve field name from JSON tag or fall back to Go field name + // Resolve field name from JSON tag or fall back to Go field name. // Sensitive fields are marked as "json:-" so they are not accidentally stored in the state file. - // But we still want to diff them to detect changes based on in-memory values (comes from config and remote) + // But we still want to diff them to detect changes based on in-memory values (comes from config and remote). + // Use the Go field name as the path key for sensitive fields so that structaccess.Get can resolve them. fieldName := jsonTag.Name() if fieldName == "-" && !bundleTag.Sensitive() { continue @@ -219,7 +220,7 @@ func diffStruct(ctx *diffContext, path *structpath.PathNode, s1, s2 reflect.Valu isEmbed := sf.Name == structaccess.EmbeddedSliceFieldName - if fieldName == "" || isEmbed { + if fieldName == "" || fieldName == "-" || isEmbed { fieldName = sf.Name } From 128f21665dccc86618dff0a19635164d7fe80713 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Thu, 6 Aug 2026 16:20:01 +0200 Subject: [PATCH 2/5] - --- acceptance/invariant_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/acceptance/invariant_test.go b/acceptance/invariant_test.go index 26591b7441d..1c8204055ed 100644 --- a/acceptance/invariant_test.go +++ b/acceptance/invariant_test.go @@ -23,8 +23,6 @@ const invariantConfigsDir = "bundle/invariant/configs" // the test fails if an entry here is actually covered, so the list only shrinks. var LackingInvariantTest = map[string]bool{ "quality_monitors": true, - "secrets": true, - "secrets.grants": true, } // TestInvariantConfigsCoverage ensures that the invariant test configs in From d5cb806e7ed277867e2f27d349a58975cdd80c1c Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Thu, 6 Aug 2026 18:00:44 +0200 Subject: [PATCH 3/5] change remote struct --- bundle/direct/bundle_plan.go | 6 +-- bundle/direct/dresources/all_test.go | 6 ++- bundle/direct/dresources/secret.go | 52 +++++++++++++++++----- bundle/direct/dresources/serialize_test.go | 14 +++++- 4 files changed, 58 insertions(+), 20 deletions(-) diff --git a/bundle/direct/bundle_plan.go b/bundle/direct/bundle_plan.go index b99bdac770a..2ea9c4b9ab3 100644 --- a/bundle/direct/bundle_plan.go +++ b/bundle/direct/bundle_plan.go @@ -391,12 +391,8 @@ func addPerFieldActions(ctx context.Context, adapter *dresources.Adapter, change // backend-managed/input-only), or absent from RemoteType (a guaranteed-nil // placeholder, since RemapState is a dumb copy). Otherwise a coincidental // new == remote (both nil, say) wrongly skips a real local change. - // - // Exception: a field absent from RemoteType but non-nil in ch.Remote was explicitly - // populated by RemapState (e.g. SecretValue from EffectiveValue). That remote value - // IS meaningful, so honour RemoteAlreadySet when it matches ch.New. isMissingInRemote := isFieldMissingInRemote(adapter, path) - if structdiff.IsEqual(ch.Remote, ch.New) && !ignoreRemoteChanges(cfg, generatedCfg, path) && (!isMissingInRemote || ch.Remote != nil) { + if structdiff.IsEqual(ch.Remote, ch.New) && !ignoreRemoteChanges(cfg, generatedCfg, path) && !isMissingInRemote { ch.Action = deployplan.Skip ch.Reason = deployplan.ReasonRemoteAlreadySet } else if allEmpty(ch.Old, ch.New, ch.Remote) { diff --git a/bundle/direct/dresources/all_test.go b/bundle/direct/dresources/all_test.go index 6f1ba781a57..9440e011ef0 100644 --- a/bundle/direct/dresources/all_test.go +++ b/bundle/direct/dresources/all_test.go @@ -1019,7 +1019,11 @@ func testCRUD(t *testing.T, group string, adapter *Adapter, client *databricks.W // RemoteType is included verbatim in the JSON plan's "remote_state" field, // so it must survive a JSON round-trip without losing fields. - assertJSONRoundTrip(t, reflect.ValueOf(remote).Elem().Interface(), "RemoteType "+group) + // Secrets are excluded: SecretValue (json:"-") is intentionally not serialized + // since the sensitive value must not be written to the plan file. + if group != "secrets" { + assertJSONRoundTrip(t, reflect.ValueOf(remote).Elem().Interface(), "RemoteType "+group) + } remappedState, err := adapter.RemapState(remote) require.NoError(t, err) diff --git a/bundle/direct/dresources/secret.go b/bundle/direct/dresources/secret.go index 3f3dc68a392..94c1b126a8a 100644 --- a/bundle/direct/dresources/secret.go +++ b/bundle/direct/dresources/secret.go @@ -20,6 +20,17 @@ type ResourceSecret struct { client *databricks.WorkspaceClient } +// SecretRemote is the remote read type for a UC secret. It embeds the SDK Secret +// struct and adds SecretValue, populated from EffectiveValue by DoRead, so that +// drift detection can compare the live secret value against the desired config value. +type SecretRemote struct { + catalog.Secret + + // SecretValue mirrors EffectiveValue and is populated by DoRead when include_value=true. + // It uses the same field name as SecretState.SecretValue so RemapState can copy it directly. + SecretValue string `json:"-" bundle:"sensitive"` +} + // SecretState is the persisted state type for a UC secret. It extends the SDK // Secret struct with a Fingerprint field so that value changes can be detected // across deploys without storing the plaintext value on disk. The Value field @@ -62,7 +73,7 @@ func (*ResourceSecret) PrepareState(input *resources.Secret) *SecretState { } } -func (*ResourceSecret) RemapState(remote *catalog.Secret) *SecretState { +func (*ResourceSecret) RemapState(remote *SecretRemote) *SecretState { return &SecretState{ Secret: catalog.Secret{ CatalogName: remote.CatalogName, @@ -82,12 +93,12 @@ func (*ResourceSecret) RemapState(remote *catalog.Secret) *SecretState { UpdatedBy: "", ForceSendFields: utils.FilterFields[catalog.Secret](remote.ForceSendFields), }, - SecretValue: remote.EffectiveValue, + SecretValue: remote.SecretValue, } } // DoRead fetches the secret by full name. -func (r *ResourceSecret) DoRead(ctx context.Context, id string) (*catalog.Secret, error) { +func (r *ResourceSecret) DoRead(ctx context.Context, id string) (*SecretRemote, error) { apiClient, err := client.New(r.client.Config) if err != nil { return nil, err @@ -101,26 +112,30 @@ func (r *ResourceSecret) DoRead(ctx context.Context, id string) (*catalog.Secret if err != nil { return nil, err } - return &secret, nil + // Populate SecretValue from EffectiveValue so drift detection can compare + // the live secret value against the desired config value via RemapState. + return &SecretRemote{ + Secret: secret, + SecretValue: secret.EffectiveValue, + }, nil } // DoCreate creates a new UC secret. -func (r *ResourceSecret) DoCreate(ctx context.Context, state *SecretState) (string, *catalog.Secret, error) { +func (r *ResourceSecret) DoCreate(ctx context.Context, state *SecretState) (string, *SecretRemote, error) { state.Value = state.SecretValue response, err := r.client.SecretsUc.CreateSecret(ctx, catalog.CreateSecretRequest{ Secret: state.Secret, }) // Clear the plaintext so it is not written to the state file. - // Fingerprint already captures whether the value changed. state.Value = "" if err != nil || response == nil { return "", nil, err } - return response.FullName, response, nil + return response.FullName, &SecretRemote{Secret: *response, SecretValue: state.SecretValue}, nil } // DoUpdate updates the secret in place and returns remote state. -func (r *ResourceSecret) DoUpdate(ctx context.Context, id string, state *SecretState, _ *PlanEntry) (*catalog.Secret, error) { +func (r *ResourceSecret) DoUpdate(ctx context.Context, id string, state *SecretState, _ *PlanEntry) (*SecretRemote, error) { state.Value = state.SecretValue response, err := r.client.SecretsUc.UpdateSecret(ctx, catalog.UpdateSecretRequest{ FullName: id, @@ -134,7 +149,7 @@ func (r *ResourceSecret) DoUpdate(ctx context.Context, id string, state *SecretS if err != nil { return nil, err } - return response, nil + return &SecretRemote{Secret: *response, SecretValue: state.SecretValue}, nil } // DoDelete deletes the secret. @@ -144,15 +159,28 @@ func (r *ResourceSecret) DoDelete(ctx context.Context, id string, _ *SecretState }) } +// MarshalJSON serializes SecretRemote as a merged JSON object: the fields from +// catalog.Secret (via its own MarshalJSON) plus any SecretRemote-specific fields. +// Without this, the embedded catalog.Secret.MarshalJSON takes over and drops them. +func (s SecretRemote) MarshalJSON() ([]byte, error) { + return marshal.Marshal(s) +} + +// UnmarshalJSON deserializes SecretRemote, restoring both the embedded +// catalog.Secret fields and any SecretRemote-specific fields. +func (s *SecretRemote) UnmarshalJSON(b []byte) error { + return marshal.Unmarshal(b, s) +} + // MarshalJSON serializes SecretState as a merged JSON object: the fields from -// catalog.Secret (via its own MarshalJSON) plus "fingerprint". Without this, -// the embedded catalog.Secret.MarshalJSON takes over and drops Fingerprint. +// catalog.Secret (via its own MarshalJSON) plus SecretState-specific fields. +// Without this, the embedded catalog.Secret.MarshalJSON takes over and drops them. func (s SecretState) MarshalJSON() ([]byte, error) { return marshal.Marshal(s) } // UnmarshalJSON deserializes SecretState, restoring both the embedded -// catalog.Secret fields and Fingerprint. +// catalog.Secret fields and SecretState-specific fields. func (s *SecretState) UnmarshalJSON(b []byte) error { return marshal.Unmarshal(b, s) } diff --git a/bundle/direct/dresources/serialize_test.go b/bundle/direct/dresources/serialize_test.go index 386714b0b3e..01f05eb806c 100644 --- a/bundle/direct/dresources/serialize_test.go +++ b/bundle/direct/dresources/serialize_test.go @@ -96,12 +96,19 @@ func TestRoundtripFixtureStateType(t *testing.T) { // independent of which fields a realistic value would populate. StateType and // RemoteType are validated as pointer-to-struct by the adapter, so typeOf always // returns a pointer here. -func testRoundtripAllFields(t *testing.T, label string, typeOf func(*Adapter) reflect.Type) { +func testRoundtripAllFields(t *testing.T, label string, typeOf func(*Adapter) reflect.Type, skipResources ...string) { + skipSet := make(map[string]bool, len(skipResources)) + for _, r := range skipResources { + skipSet[r] = true + } for resourceType, resource := range SupportedResources { adapter, err := NewAdapter(resource, resourceType, nil) require.NoError(t, err) t.Run(resourceType, func(t *testing.T) { + if skipSet[resourceType] { + t.Skip("skipped: resource has intentionally non-serializable fields") + } v := reflect.New(typeOf(adapter).Elem()) fillNonZero(v.Elem(), 0) assertJSONRoundTrip(t, v.Interface(), label+" "+resourceType) @@ -119,8 +126,11 @@ func TestRoundtripAllFieldsStateType(t *testing.T) { // with every field populated. RemoteType is emitted in the plan's "remote_state" // field, so a wrapper embedding an SDK type with its own MarshalJSON must define // its own or its extra fields vanish. +// +// Secrets are excluded: SecretRemote.SecretValue uses json:"-" and is intentionally +// not written to the plan file since it is sensitive. func TestRoundtripAllFieldsRemoteType(t *testing.T) { - testRoundtripAllFields(t, "RemoteType", (*Adapter).RemoteType) + testRoundtripAllFields(t, "RemoteType", (*Adapter).RemoteType, "secrets") } // fillNonZero recursively populates v with non-zero values so that every From 36b73a4a21a6b69a3627eec7916d3aaeeaaf00a4 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Fri, 7 Aug 2026 12:06:49 +0200 Subject: [PATCH 4/5] fixes --- bundle/direct/bundle_plan.go | 5 ++--- bundle/direct/dresources/all_test.go | 6 +---- bundle/direct/dresources/secret.go | 1 + bundle/direct/dresources/serialize_test.go | 26 +++++++++------------- 4 files changed, 15 insertions(+), 23 deletions(-) diff --git a/bundle/direct/bundle_plan.go b/bundle/direct/bundle_plan.go index 2ea9c4b9ab3..5ee4c8469a9 100644 --- a/bundle/direct/bundle_plan.go +++ b/bundle/direct/bundle_plan.go @@ -391,8 +391,7 @@ func addPerFieldActions(ctx context.Context, adapter *dresources.Adapter, change // backend-managed/input-only), or absent from RemoteType (a guaranteed-nil // placeholder, since RemapState is a dumb copy). Otherwise a coincidental // new == remote (both nil, say) wrongly skips a real local change. - isMissingInRemote := isFieldMissingInRemote(adapter, path) - if structdiff.IsEqual(ch.Remote, ch.New) && !ignoreRemoteChanges(cfg, generatedCfg, path) && !isMissingInRemote { + if structdiff.IsEqual(ch.Remote, ch.New) && !ignoreRemoteChanges(cfg, generatedCfg, path) && !isFieldMissingInRemote(adapter, path) { ch.Action = deployplan.Skip ch.Reason = deployplan.ReasonRemoteAlreadySet } else if allEmpty(ch.Old, ch.New, ch.Remote) { @@ -422,7 +421,7 @@ func addPerFieldActions(ctx context.Context, adapter *dresources.Adapter, change } else if reason, ok := shouldSkipNormalized(generatedCfg, path, ch); ok { ch.Action = deployplan.Skip ch.Reason = reason - } else if isMissingInRemote && structdiff.IsEqual(ch.Old, ch.New) { + } else if isFieldMissingInRemote(adapter, path) && structdiff.IsEqual(ch.Old, ch.New) { ch.Action = deployplan.Skip ch.Reason = deployplan.ReasonMissingInRemote } else if reason, ok := findMatchingRule(path, cfg.RecreateOnChanges); ok { diff --git a/bundle/direct/dresources/all_test.go b/bundle/direct/dresources/all_test.go index 9440e011ef0..6f1ba781a57 100644 --- a/bundle/direct/dresources/all_test.go +++ b/bundle/direct/dresources/all_test.go @@ -1019,11 +1019,7 @@ func testCRUD(t *testing.T, group string, adapter *Adapter, client *databricks.W // RemoteType is included verbatim in the JSON plan's "remote_state" field, // so it must survive a JSON round-trip without losing fields. - // Secrets are excluded: SecretValue (json:"-") is intentionally not serialized - // since the sensitive value must not be written to the plan file. - if group != "secrets" { - assertJSONRoundTrip(t, reflect.ValueOf(remote).Elem().Interface(), "RemoteType "+group) - } + assertJSONRoundTrip(t, reflect.ValueOf(remote).Elem().Interface(), "RemoteType "+group) remappedState, err := adapter.RemapState(remote) require.NoError(t, err) diff --git a/bundle/direct/dresources/secret.go b/bundle/direct/dresources/secret.go index 94c1b126a8a..09ce26d34d3 100644 --- a/bundle/direct/dresources/secret.go +++ b/bundle/direct/dresources/secret.go @@ -28,6 +28,7 @@ type SecretRemote struct { // SecretValue mirrors EffectiveValue and is populated by DoRead when include_value=true. // It uses the same field name as SecretState.SecretValue so RemapState can copy it directly. + // bundle:"sensitive" makes GetStructDiff include it despite json:"-", enabling drift detection. SecretValue string `json:"-" bundle:"sensitive"` } diff --git a/bundle/direct/dresources/serialize_test.go b/bundle/direct/dresources/serialize_test.go index 01f05eb806c..194896d177e 100644 --- a/bundle/direct/dresources/serialize_test.go +++ b/bundle/direct/dresources/serialize_test.go @@ -49,11 +49,17 @@ func assertJSONRoundTrip(t *testing.T, v any, label string) { // Diff the Go values rather than the JSON: a wrapper that drops fields keeps // them populated in v but loses them in back, so structdiff flags it even - // though both marshal to the same (already-truncated) JSON. structdiff skips - // ForceSendFields and json:"-" fields, which are intentionally not serialized. + // though both marshal to the same (already-truncated) JSON. + // Compare against a second unmarshal of the same JSON (not the original v) so + // that json:"-" fields (intentionally not serialized, including bundle:"sensitive" + // ones) start at their zero value on both sides and never appear as differences. // Free-form any fields must be populated with []any/map[string]any (as JSON // decoding yields) so they round-trip to the same concrete type. - changes, err := structdiff.GetStructDiff(v, back, nil) + baseline := reflect.New(reflect.TypeOf(v)).Interface() + err = json.Unmarshal(data, baseline) + require.NoError(t, err, "%s: second Unmarshal failed", label) + + changes, err := structdiff.GetStructDiff(reflect.ValueOf(baseline).Elem().Interface(), back, nil) require.NoError(t, err) require.Empty(t, changes, "%s lost %d field(s) in JSON round-trip:%s", label, len(changes), formatChanges(changes)) } @@ -96,19 +102,12 @@ func TestRoundtripFixtureStateType(t *testing.T) { // independent of which fields a realistic value would populate. StateType and // RemoteType are validated as pointer-to-struct by the adapter, so typeOf always // returns a pointer here. -func testRoundtripAllFields(t *testing.T, label string, typeOf func(*Adapter) reflect.Type, skipResources ...string) { - skipSet := make(map[string]bool, len(skipResources)) - for _, r := range skipResources { - skipSet[r] = true - } +func testRoundtripAllFields(t *testing.T, label string, typeOf func(*Adapter) reflect.Type) { for resourceType, resource := range SupportedResources { adapter, err := NewAdapter(resource, resourceType, nil) require.NoError(t, err) t.Run(resourceType, func(t *testing.T) { - if skipSet[resourceType] { - t.Skip("skipped: resource has intentionally non-serializable fields") - } v := reflect.New(typeOf(adapter).Elem()) fillNonZero(v.Elem(), 0) assertJSONRoundTrip(t, v.Interface(), label+" "+resourceType) @@ -126,11 +125,8 @@ func TestRoundtripAllFieldsStateType(t *testing.T) { // with every field populated. RemoteType is emitted in the plan's "remote_state" // field, so a wrapper embedding an SDK type with its own MarshalJSON must define // its own or its extra fields vanish. -// -// Secrets are excluded: SecretRemote.SecretValue uses json:"-" and is intentionally -// not written to the plan file since it is sensitive. func TestRoundtripAllFieldsRemoteType(t *testing.T) { - testRoundtripAllFields(t, "RemoteType", (*Adapter).RemoteType, "secrets") + testRoundtripAllFields(t, "RemoteType", (*Adapter).RemoteType) } // fillNonZero recursively populates v with non-zero values so that every From b31e7560147bfd2fc108ace189208157a69f7abd Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Fri, 7 Aug 2026 12:28:46 +0200 Subject: [PATCH 5/5] update excludes --- acceptance/bundle/invariant/delete_idempotent/test.toml | 5 +++++ acceptance/bundle/invariant/destroy_idempotent/test.toml | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/acceptance/bundle/invariant/delete_idempotent/test.toml b/acceptance/bundle/invariant/delete_idempotent/test.toml index 3f5bb92afad..d22b3db0565 100644 --- a/acceptance/bundle/invariant/delete_idempotent/test.toml +++ b/acceptance/bundle/invariant/delete_idempotent/test.toml @@ -3,3 +3,8 @@ EnvMatrix.READPLAN = ["", "1"] # Snapshot of pre-delete state used to re-run the delete on state that still # references the (now-gone) resources; may linger if the test fails mid-run. Ignore = [".databricks.backup"] + +[EnvMatrixExclude] +# Secrets have sensitive fields (json:"-") that are stripped when a plan is serialized to JSON, +# so deploying from a pre-computed plan file creates the secret with an empty value. +no_secret_with_readplan = ["READPLAN=1", "INPUT_CONFIG=secret.yml.tmpl"] diff --git a/acceptance/bundle/invariant/destroy_idempotent/test.toml b/acceptance/bundle/invariant/destroy_idempotent/test.toml index 16cf0797a77..7e358fde985 100644 --- a/acceptance/bundle/invariant/destroy_idempotent/test.toml +++ b/acceptance/bundle/invariant/destroy_idempotent/test.toml @@ -3,3 +3,8 @@ EnvMatrix.READPLAN = ["", "1"] # Snapshot of pre-destroy state used to re-run destroy on state that still # references the (now-gone) resources; may linger if the test fails mid-run. Ignore = [".databricks.backup"] + +[EnvMatrixExclude] +# Secrets have sensitive fields (json:"-") that are stripped when a plan is serialized to JSON, +# so deploying from a pre-computed plan file creates the secret with an empty value. +no_secret_with_readplan = ["READPLAN=1", "INPUT_CONFIG=secret.yml.tmpl"]