From bb5c0fd8ee68532db52471b98f274fc38c944368 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Thu, 6 Aug 2026 16:04:47 +0200 Subject: [PATCH 1/9] 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/9] - --- 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/9] 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/9] 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/9] 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"] From fcba41f8ecdd737128387c46834b6fdb41a0f1f0 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 10 Aug 2026 15:22:39 +0200 Subject: [PATCH 6/9] redact value from state and plan --- .../bundle/resources/secrets/basic/output.txt | 24 +-- .../bundle/resources/secrets/basic/script | 2 +- .../resources/secrets/update-value/output.txt | 59 ++++++- .../resources/secrets/update-value/script | 1 + bundle/direct/apply.go | 29 +++- bundle/direct/bundle_plan.go | 10 ++ bundle/direct/dresources/adapter.go | 11 ++ bundle/direct/dresources/config.go | 4 + bundle/direct/dresources/resources.yml | 5 + bundle/direct/dresources/secret.go | 160 +++++------------- bundle/direct/redact.go | 122 +++++++++++++ 11 files changed, 292 insertions(+), 135 deletions(-) create mode 100644 bundle/direct/redact.go diff --git a/acceptance/bundle/resources/secrets/basic/output.txt b/acceptance/bundle/resources/secrets/basic/output.txt index 1e3d1b9f46b..83aadf57f98 100644 --- a/acceptance/bundle/resources/secrets/basic/output.txt +++ b/acceptance/bundle/resources/secrets/basic/output.txt @@ -1,12 +1,14 @@ ->>> [CLI] bundle validate --var secret_value=initial-secret-value -Name: test-bundle-[UNIQUE_NAME] -Target: default -Workspace: - User: [USERNAME] - Path: /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default - -Validation OK! +>>> [CLI] bundle validate --var secret_value=initial-secret-value -o json +{ + "secret1": { + "catalog_name": "main", + "comment": "Test secret for acceptance testing", + "name": "test_secret_[UNIQUE_NAME]", + "schema_name": "default", + "value": "initial-secret-value" + } +} >>> [CLI] bundle summary --var secret_value=initial-secret-value Name: test-bundle-[UNIQUE_NAME] @@ -38,7 +40,7 @@ Deployment complete! "value": "initial-secret-value" } } -secrets secret1 catalog_name='main' schema_name='default' name='test_secret_[UNIQUE_NAME]' comment='Test secret for acceptance testing' value='' effective_value=None +secrets secret1 catalog_name='main' schema_name='default' name='test_secret_[UNIQUE_NAME]' comment='Test secret for acceptance testing' value='[redacted]' effective_value=None === Update comment and re-deploy >>> update_file.py databricks.yml Test secret for acceptance testing Updated comment @@ -64,7 +66,7 @@ Deployment complete! "value": "initial-secret-value" } } -secrets secret1 catalog_name='main' schema_name='default' name='test_secret_[UNIQUE_NAME]' comment='Updated comment' value='' effective_value=None +secrets secret1 catalog_name='main' schema_name='default' name='test_secret_[UNIQUE_NAME]' comment='Updated comment' value='[redacted]' effective_value=None === Restore comment and re-deploy >>> update_file.py databricks.yml Updated comment Test secret for acceptance testing @@ -90,7 +92,7 @@ Deployment complete! "value": "initial-secret-value" } } -secrets secret1 catalog_name='main' schema_name='default' name='test_secret_[UNIQUE_NAME]' comment='Test secret for acceptance testing' value='' effective_value=None +secrets secret1 catalog_name='main' schema_name='default' name='test_secret_[UNIQUE_NAME]' comment='Test secret for acceptance testing' value='[redacted]' effective_value=None >>> [CLI] bundle destroy --auto-approve --var secret_value=initial-secret-value The following resources will be deleted: diff --git a/acceptance/bundle/resources/secrets/basic/script b/acceptance/bundle/resources/secrets/basic/script index d7dd7b53f6f..606e1fb95e0 100755 --- a/acceptance/bundle/resources/secrets/basic/script +++ b/acceptance/bundle/resources/secrets/basic/script @@ -6,7 +6,7 @@ cleanup() { } trap cleanup EXIT -trace $CLI bundle validate --var secret_value=initial-secret-value +trace $CLI bundle validate --var secret_value=initial-secret-value -o json | jq ".resources.secrets" trace $CLI bundle summary --var secret_value=initial-secret-value trace $CLI bundle deploy --var secret_value=initial-secret-value diff --git a/acceptance/bundle/resources/secrets/update-value/output.txt b/acceptance/bundle/resources/secrets/update-value/output.txt index 98f89e9d6e8..bd7d240b187 100644 --- a/acceptance/bundle/resources/secrets/update-value/output.txt +++ b/acceptance/bundle/resources/secrets/update-value/output.txt @@ -12,7 +12,7 @@ "comment": "Test secret", "name": "test_secret", "schema_name": "default", - "value": "" + "value": "[redacted]" } } } @@ -37,9 +37,60 @@ Deployment complete! "value": "initial-secret-value" } } -secrets secret1 catalog_name='main' schema_name='default' name='test_secret' comment='Test secret' value='' +secrets secret1 catalog_name='main' schema_name='default' name='test_secret' comment='Test secret' value='[redacted]' === Update secret value by passing a different variable value +>>> [CLI] bundle plan --var secret_value=updated-secret-value -o json +{ + "plan_version": 2, + "cli_version": "[CLI_VERSION]", + "lineage": "[UUID]", + "serial": 1, + "plan": { + "resources.secrets.secret1": { + "action": "update", + "new_state": { + "value": { + "catalog_name": "main", + "comment": "Test secret", + "name": "test_secret", + "schema_name": "default", + "value": "[redacted]" + } + }, + "remote_state": { + "catalog_name": "main", + "comment": "Test secret", + "create_time": "[TIMESTAMP]", + "created_by": "test-user@databricks.com", + "effective_owner": "test-user@databricks.com", + "effective_value": "[redacted]", + "full_name": "main.default.test_secret", + "metastore_id": "test-metastore-id", + "name": "test_secret", + "owner": "test-user@databricks.com", + "schema_name": "default", + "update_time": "[TIMESTAMP]", + "updated_by": "test-user@databricks.com", + "value": "[redacted]" + }, + "changes": { + "owner": { + "action": "skip", + "reason": "spec:input_only", + "remote": "test-user@databricks.com" + }, + "value": { + "action": "update", + "old": "[redacted]", + "new": "[redacted]", + "remote": "[redacted]" + } + } + } + } +} + >>> [CLI] bundle deploy --var secret_value=updated-secret-value Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... Deploying resources... @@ -61,7 +112,7 @@ Deployment complete! "value": "updated-secret-value" } } -secrets secret1 catalog_name='main' schema_name='default' name='test_secret' comment='Test secret' value='' +secrets secret1 catalog_name='main' schema_name='default' name='test_secret' comment='Test secret' value='[redacted]' === Verify state does not contain actual secret value >>> print_state.py @@ -78,7 +129,7 @@ secrets secret1 catalog_name='main' schema_name='default' name='test_secret' com "comment": "Test secret", "name": "test_secret", "schema_name": "default", - "value": "" + "value": "[redacted]" } } } diff --git a/acceptance/bundle/resources/secrets/update-value/script b/acceptance/bundle/resources/secrets/update-value/script index 56ae4f03f94..a5c1596e29c 100755 --- a/acceptance/bundle/resources/secrets/update-value/script +++ b/acceptance/bundle/resources/secrets/update-value/script @@ -5,6 +5,7 @@ trace print_requests.py //unity-catalog read_state.py secrets secret1 catalog_name schema_name name comment value title "Update secret value by passing a different variable value" +trace $CLI bundle plan --var secret_value=updated-secret-value -o json trace $CLI bundle deploy --var secret_value=updated-secret-value trace print_requests.py //unity-catalog read_state.py secrets secret1 catalog_name schema_name name comment value diff --git a/bundle/direct/apply.go b/bundle/direct/apply.go index cbb0a2d45ff..4bf53418d04 100644 --- a/bundle/direct/apply.go +++ b/bundle/direct/apply.go @@ -75,7 +75,7 @@ func (d *DeploymentUnit) Create(ctx context.Context, db *dstate.DeploymentState, return err } - err = db.SaveState(d.ResourceKey, newID, newState, d.DependsOn) + err = d.saveStateRedacted(db, newID, newState, d.DependsOn) if err != nil { return fmt.Errorf("saving state after creating id=%s: %w", newID, err) } @@ -163,7 +163,7 @@ func (d *DeploymentUnit) Update(ctx context.Context, db *dstate.DeploymentState, return fmt.Errorf("deleting state id=%s: %w", id, err) } } else { - err = db.SaveState(d.ResourceKey, id, newState, d.DependsOn) + err = d.saveStateRedacted(db, id, newState, d.DependsOn) if err != nil { return fmt.Errorf("saving state id=%s: %w", id, err) } @@ -208,7 +208,7 @@ func (d *DeploymentUnit) UpdateWithID(ctx context.Context, db *dstate.Deployment return err } - err = db.SaveState(d.ResourceKey, newID, newState, d.DependsOn) + err = d.saveStateRedacted(db, newID, newState, d.DependsOn) if err != nil { return fmt.Errorf("saving state id=%s: %w", oldID, err) } @@ -291,7 +291,7 @@ func (d *DeploymentUnit) Resize(ctx context.Context, db *dstate.DeploymentState, return fmt.Errorf("resizing id=%s: %w", id, err) } - err = db.SaveState(d.ResourceKey, id, newState, d.DependsOn) + err = d.saveStateRedacted(db, id, newState, d.DependsOn) if err != nil { return fmt.Errorf("saving state id=%s: %w", id, err) } @@ -299,6 +299,27 @@ func (d *DeploymentUnit) Resize(ctx context.Context, db *dstate.DeploymentState, return nil } +// saveStateRedacted saves a copy of state to the deployment state file with +// sensitive fields replaced by a placeholder value so secrets are never written +// to disk in plaintext. +func (d *DeploymentUnit) saveStateRedacted(db *dstate.DeploymentState, newID string, state any, dependsOn []deployplan.DependsOnEntry) error { + // Round-trip through JSON to get an independent copy so the original struct + // (still held in memory for post-deploy use) is not modified. + data, err := json.Marshal(state) + if err != nil { + return fmt.Errorf("marshaling state for redaction: %w", err) + } + stateType := d.Adapter.StateType() + ptr := reflect.New(stateType.Elem()).Interface() + if err := json.Unmarshal(data, ptr); err != nil { + return fmt.Errorf("unmarshaling state copy for redaction: %w", err) + } + if err := redactStruct(d.Adapter, ptr); err != nil { + return fmt.Errorf("redacting state: %w", err) + } + return db.SaveState(d.ResourceKey, newID, ptr, dependsOn) +} + func parseState(destType reflect.Type, raw json.RawMessage) (any, error) { destPtr := reflect.New(destType).Interface() err := json.Unmarshal(raw, destPtr) diff --git a/bundle/direct/bundle_plan.go b/bundle/direct/bundle_plan.go index 5ee4c8469a9..4a9724fbb8c 100644 --- a/bundle/direct/bundle_plan.go +++ b/bundle/direct/bundle_plan.go @@ -320,6 +320,16 @@ func (b *DeploymentBundle) CalculatePlan(ctx context.Context, client *databricks } } + for resourceKey, entry := range plan.Plan { + adapter, err := b.getAdapterForKey(resourceKey) + if err != nil { + return nil, fmt.Errorf("redacting plan entry %s: %w", resourceKey, err) + } + if err := redactPlanEntry(adapter, entry); err != nil { + return nil, fmt.Errorf("redacting plan entry %s: %w", resourceKey, err) + } + } + return plan, nil } diff --git a/bundle/direct/dresources/adapter.go b/bundle/direct/dresources/adapter.go index ea8876c4d52..6c64cebf443 100644 --- a/bundle/direct/dresources/adapter.go +++ b/bundle/direct/dresources/adapter.go @@ -413,6 +413,17 @@ func (a *Adapter) GeneratedResourceConfig() *ResourceLifecycleConfig { return a.generatedResourceConfig } +// IsSensitive reports whether any path in v that has this path as a prefix should +// be treated as sensitive (redacted from plan output and deployment state). +func (a *Adapter) IsSensitive(path *structpath.PathNode) bool { + for _, r := range a.resourceConfig.SensitiveFields { + if path.HasPatternPrefix(r.Field) { + return true + } + } + return false +} + // FieldTriggersRecreate reports whether a local change to the field forces a // delete + create. Both recreate_on_changes and provided_id_fields do this, so a // caller that knows the ID is preserved can conclude the field is unchanged. diff --git a/bundle/direct/dresources/config.go b/bundle/direct/dresources/config.go index 91175594a6a..33a00e0a2f7 100644 --- a/bundle/direct/dresources/config.go +++ b/bundle/direct/dresources/config.go @@ -79,6 +79,9 @@ type ResourceLifecycleConfig struct { // BackendDefaults: fields where the backend may set defaults. // When old and new are nil but remote is set, and the remote value matches allowed values (if specified), the change is skipped. BackendDefaults []BackendDefaultRule `yaml:"backend_defaults,omitempty"` + + // SensitiveFields: fields that are sensitive and should not be logged. + SensitiveFields []FieldRule `yaml:"sensitive_fields,omitempty"` } // Config is the root configuration structure for resource lifecycle behavior. @@ -100,6 +103,7 @@ var empty = ResourceLifecycleConfig{ UpdatableIDFields: nil, NormalizeSlash: nil, BackendDefaults: nil, + SensitiveFields: nil, } func mustParseConfig(data []byte) func() *Config { diff --git a/bundle/direct/dresources/resources.yml b/bundle/direct/dresources/resources.yml index 0c9c9bf5319..7b1e006904d 100644 --- a/bundle/direct/dresources/resources.yml +++ b/bundle/direct/dresources/resources.yml @@ -558,6 +558,11 @@ resources: - field: scope_name reason: id_changes + secrets: + sensitive_fields: + - field: value + - field: effective_value + clusters: ignore_remote_changes: # https://github.com/databricks/terraform-provider-databricks/blob/4eba541abe1a9f50993ea7b9dd83874207e224a1/clusters/resource_cluster.go#L361-L363 diff --git a/bundle/direct/dresources/secret.go b/bundle/direct/dresources/secret.go index 09ce26d34d3..f6213e622ef 100644 --- a/bundle/direct/dresources/secret.go +++ b/bundle/direct/dresources/secret.go @@ -10,7 +10,6 @@ import ( "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/client" "github.com/databricks/databricks-sdk-go/common/types/fieldmask" - "github.com/databricks/databricks-sdk-go/marshal" "github.com/databricks/databricks-sdk-go/service/catalog" ) @@ -20,86 +19,54 @@ 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. - // bundle:"sensitive" makes GetStructDiff include it despite json:"-", enabling drift detection. - 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 -// is always cleared after the API call (see DoCreate/DoUpdate). -type SecretState struct { - catalog.Secret - - // SecretValue is the plaintext value of the secret. It is not stored in the state file. - // It is carried here so DoCreate/DoUpdate can send it to the API. - SecretValue string `json:"-" bundle:"sensitive"` -} - func (*ResourceSecret) New(client *databricks.WorkspaceClient) *ResourceSecret { return &ResourceSecret{client: client} } -func (*ResourceSecret) PrepareState(input *resources.Secret) *SecretState { - return &SecretState{ - Secret: catalog.Secret{ - CatalogName: input.CatalogName, - SchemaName: input.SchemaName, - Name: input.Name, - Value: "", - Comment: input.Comment, - ExpireTime: input.ExpireTime, - Owner: "", - CreateTime: nil, - CreatedBy: "", - EffectiveOwner: "", - EffectiveValue: "", - FullName: "", - MetastoreId: "", - UpdateTime: nil, - UpdatedBy: "", - ForceSendFields: utils.FilterFields[catalog.Secret](nil), - }, - // Value is carried here so DoCreate/DoUpdate can send it to the API. - // It is cleared from state after the API call (see DoCreate/DoUpdate). - SecretValue: input.Value, +func (*ResourceSecret) PrepareState(input *resources.Secret) *catalog.Secret { + return &catalog.Secret{ + CatalogName: input.CatalogName, + SchemaName: input.SchemaName, + Name: input.Name, + Value: input.Value, + Comment: input.Comment, + ExpireTime: input.ExpireTime, + Owner: "", + CreateTime: nil, + CreatedBy: "", + EffectiveOwner: "", + EffectiveValue: "", + FullName: "", + MetastoreId: "", + UpdateTime: nil, + UpdatedBy: "", + ForceSendFields: utils.FilterFields[catalog.Secret](nil), } } -func (*ResourceSecret) RemapState(remote *SecretRemote) *SecretState { - return &SecretState{ - Secret: catalog.Secret{ - CatalogName: remote.CatalogName, - SchemaName: remote.SchemaName, - Name: remote.Name, - Comment: remote.Comment, - Owner: remote.Owner, - ExpireTime: remote.ExpireTime, - Value: "", - CreateTime: nil, - CreatedBy: "", - EffectiveOwner: "", - EffectiveValue: remote.EffectiveValue, - FullName: "", - MetastoreId: "", - UpdateTime: nil, - UpdatedBy: "", - ForceSendFields: utils.FilterFields[catalog.Secret](remote.ForceSendFields), - }, - SecretValue: remote.SecretValue, +func (*ResourceSecret) RemapState(remote *catalog.Secret) *catalog.Secret { + return &catalog.Secret{ + CatalogName: remote.CatalogName, + SchemaName: remote.SchemaName, + Name: remote.Name, + Value: remote.EffectiveValue, + Comment: remote.Comment, + ExpireTime: remote.ExpireTime, + Owner: remote.EffectiveOwner, + CreateTime: nil, + CreatedBy: "", + EffectiveOwner: "", + EffectiveValue: "", + FullName: "", + MetastoreId: "", + UpdateTime: nil, + UpdatedBy: "", + ForceSendFields: utils.FilterFields[catalog.Secret](nil), } } // DoRead fetches the secret by full name. -func (r *ResourceSecret) DoRead(ctx context.Context, id string) (*SecretRemote, error) { +func (r *ResourceSecret) DoRead(ctx context.Context, id string) (*catalog.Secret, error) { apiClient, err := client.New(r.client.Config) if err != nil { return nil, err @@ -113,75 +80,38 @@ func (r *ResourceSecret) DoRead(ctx context.Context, id string) (*SecretRemote, if err != nil { return nil, err } - // 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 + return &secret, nil } // DoCreate creates a new UC secret. -func (r *ResourceSecret) DoCreate(ctx context.Context, state *SecretState) (string, *SecretRemote, error) { - state.Value = state.SecretValue +func (r *ResourceSecret) DoCreate(ctx context.Context, state *catalog.Secret) (string, *catalog.Secret, error) { response, err := r.client.SecretsUc.CreateSecret(ctx, catalog.CreateSecretRequest{ - Secret: state.Secret, + Secret: *state, }) - // Clear the plaintext so it is not written to the state file. - state.Value = "" if err != nil || response == nil { return "", nil, err } - return response.FullName, &SecretRemote{Secret: *response, SecretValue: state.SecretValue}, nil + return response.FullName, response, nil } // DoUpdate updates the secret in place and returns remote state. -func (r *ResourceSecret) DoUpdate(ctx context.Context, id string, state *SecretState, _ *PlanEntry) (*SecretRemote, error) { - state.Value = state.SecretValue +func (r *ResourceSecret) DoUpdate(ctx context.Context, id string, state *catalog.Secret, _ *PlanEntry) (*catalog.Secret, error) { response, err := r.client.SecretsUc.UpdateSecret(ctx, catalog.UpdateSecretRequest{ FullName: id, - Secret: state.Secret, + Secret: *state, UpdateMask: fieldmask.FieldMask{ Paths: []string{"*"}, }, }) - // Clear the plaintext so it is not written to the state file. - state.Value = "" if err != nil { return nil, err } - return &SecretRemote{Secret: *response, SecretValue: state.SecretValue}, nil + return response, nil } // DoDelete deletes the secret. -func (r *ResourceSecret) DoDelete(ctx context.Context, id string, _ *SecretState) error { +func (r *ResourceSecret) DoDelete(ctx context.Context, id string, _ *catalog.Secret) error { return r.client.SecretsUc.DeleteSecret(ctx, catalog.DeleteSecretRequest{ FullName: id, }) } - -// 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 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 SecretState-specific fields. -func (s *SecretState) UnmarshalJSON(b []byte) error { - return marshal.Unmarshal(b, s) -} diff --git a/bundle/direct/redact.go b/bundle/direct/redact.go new file mode 100644 index 00000000000..2d69be32149 --- /dev/null +++ b/bundle/direct/redact.go @@ -0,0 +1,122 @@ +package direct + +import ( + "encoding/json" + "fmt" + "reflect" + + "github.com/databricks/cli/bundle/deployplan" + "github.com/databricks/cli/bundle/direct/dresources" + "github.com/databricks/cli/libs/structs/structaccess" + "github.com/databricks/cli/libs/structs/structpath" + "github.com/databricks/cli/libs/structs/structwalk" + "github.com/databricks/cli/libs/structs/structvar" +) + +const sensitiveRedactedValue = "[redacted]" + +// redactStruct zeros all scalar fields in s (a pointer to a typed struct) that +// the adapter marks as sensitive. The struct is modified in place. +func redactStruct(adapter *dresources.Adapter, s any) error { + if s == nil { + return nil + } + + var toRedact []*structpath.PathNode + err := structwalk.Walk(s, func(path *structpath.PathNode, _ any, _ *reflect.StructField) { + if adapter.IsSensitive(path) { + toRedact = append(toRedact, path) + } + }) + if err != nil { + return fmt.Errorf("walking struct: %w", err) + } + + for _, path := range toRedact { + if err := structaccess.Set(s, path, sensitiveRedactedValue); err != nil { + // Field might not be a string (e.g. it could be an int or bool); try + // setting it to its zero value instead. + fv, ferr := structaccess.Get(s, path) + if ferr != nil { + continue + } + zero := reflect.Zero(reflect.TypeOf(fv)).Interface() + _ = structaccess.Set(s, path, zero) + } + } + + return nil +} + +// redactChanges replaces the Old, New, and Remote values in any ChangeDesc whose +// path is marked sensitive by the adapter, so the plan output does not leak them. +func redactChanges(adapter *dresources.Adapter, changes deployplan.Changes) error { + for pathString, ch := range changes { + path, err := structpath.ParsePath(pathString) + if err != nil { + return fmt.Errorf("parsing change path %q: %w", pathString, err) + } + if adapter.IsSensitive(path) { + if ch.Old != nil { + ch.Old = sensitiveRedactedValue + } + if ch.New != nil { + ch.New = sensitiveRedactedValue + } + if ch.Remote != nil { + ch.Remote = sensitiveRedactedValue + } + } + } + return nil +} + +// redactNewStateJSON redacts sensitive fields inside a StructVarJSON by round-tripping +// through the adapter's state type so path matching works correctly. +func redactNewStateJSON(adapter *dresources.Adapter, svj *structvar.StructVarJSON) error { + if svj == nil || len(svj.Value) == 0 { + return nil + } + + stateType := adapter.StateType() + // StateType returns a pointer type; Elem gives the concrete struct type. + ptr := reflect.New(stateType.Elem()).Interface() + if err := json.Unmarshal(svj.Value, ptr); err != nil { + return fmt.Errorf("unmarshaling new_state: %w", err) + } + + if err := redactStruct(adapter, ptr); err != nil { + return fmt.Errorf("redacting new_state: %w", err) + } + + redacted, err := json.Marshal(ptr) + if err != nil { + return fmt.Errorf("re-marshaling new_state: %w", err) + } + svj.Value = redacted + return nil +} + +// redactPlanEntry redacts sensitive fields from all three sections of a plan entry: +// new_state, remote_state, and changes. The entry is modified in place. +func redactPlanEntry(adapter *dresources.Adapter, entry *deployplan.PlanEntry) error { + if entry.NewState != nil { + if err := redactNewStateJSON(adapter, entry.NewState); err != nil { + return err + } + } + + if entry.RemoteState != nil { + if err := redactStruct(adapter, entry.RemoteState); err != nil { + return fmt.Errorf("redacting remote_state: %w", err) + } + } + + if entry.Changes != nil { + if err := redactChanges(adapter, entry.Changes); err != nil { + return fmt.Errorf("redacting changes: %w", err) + } + } + + return nil +} From 5ff895e1d9a08a451b67dd8f41f782158f340c65 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 10 Aug 2026 15:32:59 +0200 Subject: [PATCH 7/9] remove structaccess changes --- libs/structs/structaccess/get.go | 9 +-------- libs/structs/structaccess/typecheck.go | 16 +++------------- libs/structs/structdiff/diff.go | 7 +++---- 3 files changed, 7 insertions(+), 25 deletions(-) diff --git a/libs/structs/structaccess/get.go b/libs/structs/structaccess/get.go index 2119f7fb107..bf8c96df80d 100644 --- a/libs/structs/structaccess/get.go +++ b/libs/structs/structaccess/get.go @@ -249,15 +249,7 @@ 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 = "" } @@ -266,6 +258,7 @@ 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 50ebf5978c6..7147fa0f435 100644 --- a/libs/structs/structaccess/typecheck.go +++ b/libs/structs/structaccess/typecheck.go @@ -152,22 +152,12 @@ func FindStructFieldByKeyType(t reflect.Type, key string) (reflect.StructField, continue } name := structtag.JSONTag(sf.Tag.Get("json")).Name() - 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 - } + if name == "-" || sf.Name == EmbeddedSliceFieldName { name = "" } - - if name == "" || sf.Name == EmbeddedSliceFieldName { - continue - } - if name == key { + if name != "" && 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 41be3ba64d5..4e9df593994 100644 --- a/libs/structs/structdiff/diff.go +++ b/libs/structs/structdiff/diff.go @@ -209,10 +209,9 @@ 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). - // Use the Go field name as the path key for sensitive fields so that structaccess.Get can resolve them. + // But we still want to diff them to detect changes based on in-memory values (comes from config and remote) fieldName := jsonTag.Name() if fieldName == "-" && !bundleTag.Sensitive() { continue @@ -220,7 +219,7 @@ func diffStruct(ctx *diffContext, path *structpath.PathNode, s1, s2 reflect.Valu isEmbed := sf.Name == structaccess.EmbeddedSliceFieldName - if fieldName == "" || fieldName == "-" || isEmbed { + if fieldName == "" || isEmbed { fieldName = sf.Name } From 20bb9f4896de0d7ce7b8a37933b0b487bdccbe8e Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 10 Aug 2026 16:24:51 +0200 Subject: [PATCH 8/9] no drift --- .../bundle/resources/secrets/basic/output.txt | 6 ++-- .../resources/secrets/update-value/output.txt | 8 ++--- bundle/direct/apply.go | 2 +- bundle/direct/dresources/secret.go | 22 ++++++++++++ bundle/direct/redact.go | 36 +++++++++++++------ 5 files changed, 55 insertions(+), 19 deletions(-) diff --git a/acceptance/bundle/resources/secrets/basic/output.txt b/acceptance/bundle/resources/secrets/basic/output.txt index 83aadf57f98..fd9ce29485d 100644 --- a/acceptance/bundle/resources/secrets/basic/output.txt +++ b/acceptance/bundle/resources/secrets/basic/output.txt @@ -40,7 +40,7 @@ Deployment complete! "value": "initial-secret-value" } } -secrets secret1 catalog_name='main' schema_name='default' name='test_secret_[UNIQUE_NAME]' comment='Test secret for acceptance testing' value='[redacted]' effective_value=None +secrets secret1 catalog_name='main' schema_name='default' name='test_secret_[UNIQUE_NAME]' comment='Test secret for acceptance testing' value='' effective_value=None === Update comment and re-deploy >>> update_file.py databricks.yml Test secret for acceptance testing Updated comment @@ -66,7 +66,7 @@ Deployment complete! "value": "initial-secret-value" } } -secrets secret1 catalog_name='main' schema_name='default' name='test_secret_[UNIQUE_NAME]' comment='Updated comment' value='[redacted]' effective_value=None +secrets secret1 catalog_name='main' schema_name='default' name='test_secret_[UNIQUE_NAME]' comment='Updated comment' value='' effective_value=None === Restore comment and re-deploy >>> update_file.py databricks.yml Updated comment Test secret for acceptance testing @@ -92,7 +92,7 @@ Deployment complete! "value": "initial-secret-value" } } -secrets secret1 catalog_name='main' schema_name='default' name='test_secret_[UNIQUE_NAME]' comment='Test secret for acceptance testing' value='[redacted]' effective_value=None +secrets secret1 catalog_name='main' schema_name='default' name='test_secret_[UNIQUE_NAME]' comment='Test secret for acceptance testing' value='' effective_value=None >>> [CLI] bundle destroy --auto-approve --var secret_value=initial-secret-value The following resources will be deleted: diff --git a/acceptance/bundle/resources/secrets/update-value/output.txt b/acceptance/bundle/resources/secrets/update-value/output.txt index bd7d240b187..3af0d0e46b0 100644 --- a/acceptance/bundle/resources/secrets/update-value/output.txt +++ b/acceptance/bundle/resources/secrets/update-value/output.txt @@ -37,7 +37,7 @@ Deployment complete! "value": "initial-secret-value" } } -secrets secret1 catalog_name='main' schema_name='default' name='test_secret' comment='Test secret' value='[redacted]' +secrets secret1 catalog_name='main' schema_name='default' name='test_secret' comment='Test secret' value='' === Update secret value by passing a different variable value >>> [CLI] bundle plan --var secret_value=updated-secret-value -o json @@ -82,7 +82,7 @@ secrets secret1 catalog_name='main' schema_name='default' name='test_secret' com }, "value": { "action": "update", - "old": "[redacted]", + "old": "", "new": "[redacted]", "remote": "[redacted]" } @@ -112,7 +112,7 @@ Deployment complete! "value": "updated-secret-value" } } -secrets secret1 catalog_name='main' schema_name='default' name='test_secret' comment='Test secret' value='[redacted]' +secrets secret1 catalog_name='main' schema_name='default' name='test_secret' comment='Test secret' value='' === Verify state does not contain actual secret value >>> print_state.py @@ -129,7 +129,7 @@ secrets secret1 catalog_name='main' schema_name='default' name='test_secret' com "comment": "Test secret", "name": "test_secret", "schema_name": "default", - "value": "[redacted]" + "value": "" } } } diff --git a/bundle/direct/apply.go b/bundle/direct/apply.go index 4bf53418d04..b877ebad0b2 100644 --- a/bundle/direct/apply.go +++ b/bundle/direct/apply.go @@ -314,7 +314,7 @@ func (d *DeploymentUnit) saveStateRedacted(db *dstate.DeploymentState, newID str if err := json.Unmarshal(data, ptr); err != nil { return fmt.Errorf("unmarshaling state copy for redaction: %w", err) } - if err := redactStruct(d.Adapter, ptr); err != nil { + if err := zeroSensitiveFields(d.Adapter, ptr); err != nil { return fmt.Errorf("redacting state: %w", err) } return db.SaveState(d.ResourceKey, newID, ptr, dependsOn) diff --git a/bundle/direct/dresources/secret.go b/bundle/direct/dresources/secret.go index f6213e622ef..b3ab6dbf53c 100644 --- a/bundle/direct/dresources/secret.go +++ b/bundle/direct/dresources/secret.go @@ -5,7 +5,10 @@ import ( "net/http" "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/bundle/deployplan" "github.com/databricks/cli/libs/auth" + "github.com/databricks/cli/libs/structs/structdiff" + "github.com/databricks/cli/libs/structs/structpath" "github.com/databricks/cli/libs/utils" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/client" @@ -115,3 +118,22 @@ func (r *ResourceSecret) DoDelete(ctx context.Context, id string, _ *catalog.Sec FullName: id, }) } + +// OverrideChangeDesc handles the "value" field, which is write-only (the API never +// returns it in GET responses — only effective_value is readable). The state file +// stores "" for this field (never the plaintext), so old is always "" regardless of +// the actual stored value. We compare new vs remote (via effective_value from DoRead) +// to decide whether the secret actually changed: if they are equal, the user's config +// already matches what is stored remotely and no update is needed. +func (*ResourceSecret) OverrideChangeDesc(_ context.Context, path *structpath.PathNode, ch *ChangeDesc, _ *catalog.Secret) error { + if path.String() != "value" { + return nil + } + if structdiff.IsEqual(ch.Remote, ch.New) { + ch.Action = deployplan.Skip + ch.Reason = deployplan.ReasonRemoteAlreadySet + } else { + ch.Action = deployplan.Update + } + return nil +} diff --git a/bundle/direct/redact.go b/bundle/direct/redact.go index 2d69be32149..55ac061e34e 100644 --- a/bundle/direct/redact.go +++ b/bundle/direct/redact.go @@ -9,21 +9,22 @@ import ( "github.com/databricks/cli/bundle/direct/dresources" "github.com/databricks/cli/libs/structs/structaccess" "github.com/databricks/cli/libs/structs/structpath" - "github.com/databricks/cli/libs/structs/structwalk" "github.com/databricks/cli/libs/structs/structvar" + "github.com/databricks/cli/libs/structs/structwalk" ) const sensitiveRedactedValue = "[redacted]" -// redactStruct zeros all scalar fields in s (a pointer to a typed struct) that -// the adapter marks as sensitive. The struct is modified in place. -func redactStruct(adapter *dresources.Adapter, s any) error { +// redactSensitiveFields replaces (or zeros) scalar fields in s (a pointer to a typed struct) +// that the adapter marks as sensitive. replacement is what to set the field to; pass +// sensitiveRedactedValue for plan output display, or "" for state-file storage. +func redactSensitiveFields(adapter *dresources.Adapter, s any, replacement string) error { if s == nil { return nil } var toRedact []*structpath.PathNode - err := structwalk.Walk(s, func(path *structpath.PathNode, _ any, _ *reflect.StructField) { + err := structwalk.Walk(s, func(path *structpath.PathNode, val any, _ *reflect.StructField) { if adapter.IsSensitive(path) { toRedact = append(toRedact, path) } @@ -33,9 +34,8 @@ func redactStruct(adapter *dresources.Adapter, s any) error { } for _, path := range toRedact { - if err := structaccess.Set(s, path, sensitiveRedactedValue); err != nil { - // Field might not be a string (e.g. it could be an int or bool); try - // setting it to its zero value instead. + if err := structaccess.Set(s, path, replacement); err != nil { + // Field is not a string; fall back to its zero value. fv, ferr := structaccess.Get(s, path) if ferr != nil { continue @@ -48,8 +48,22 @@ func redactStruct(adapter *dresources.Adapter, s any) error { return nil } +// redactStruct replaces sensitive fields with "[redacted]" for display in plan output. +func redactStruct(adapter *dresources.Adapter, s any) error { + return redactSensitiveFields(adapter, s, sensitiveRedactedValue) +} + +// zeroSensitiveFields clears sensitive fields to their zero value for safe storage in +// the state file. This avoids a false "local change" diff on the next plan (the state +// stores "" instead of the actual value, so old==new==nil/zero and drift detection +// falls back to remote comparison via RemoteAlreadySet). +func zeroSensitiveFields(adapter *dresources.Adapter, s any) error { + return redactSensitiveFields(adapter, s, "") +} + // redactChanges replaces the Old, New, and Remote values in any ChangeDesc whose // path is marked sensitive by the adapter, so the plan output does not leak them. +// Empty/nil values are left as-is (they carry no secret). func redactChanges(adapter *dresources.Adapter, changes deployplan.Changes) error { for pathString, ch := range changes { path, err := structpath.ParsePath(pathString) @@ -57,13 +71,13 @@ func redactChanges(adapter *dresources.Adapter, changes deployplan.Changes) erro return fmt.Errorf("parsing change path %q: %w", pathString, err) } if adapter.IsSensitive(path) { - if ch.Old != nil { + if v, ok := ch.Old.(string); ok && v != "" { ch.Old = sensitiveRedactedValue } - if ch.New != nil { + if v, ok := ch.New.(string); ok && v != "" { ch.New = sensitiveRedactedValue } - if ch.Remote != nil { + if v, ok := ch.Remote.(string); ok && v != "" { ch.Remote = sensitiveRedactedValue } } From eaf8020c3176ed1176eb2ae16e32b1246755578c Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 10 Aug 2026 16:34:11 +0200 Subject: [PATCH 9/9] also redact validate --- .../bundle/resources/secrets/basic/output.txt | 2 +- bundle/direct/dresources/redact_config.go | 81 +++++++++++++++++++ cmd/bundle/validate.go | 7 +- 3 files changed, 88 insertions(+), 2 deletions(-) create mode 100644 bundle/direct/dresources/redact_config.go diff --git a/acceptance/bundle/resources/secrets/basic/output.txt b/acceptance/bundle/resources/secrets/basic/output.txt index fd9ce29485d..40e535d678c 100644 --- a/acceptance/bundle/resources/secrets/basic/output.txt +++ b/acceptance/bundle/resources/secrets/basic/output.txt @@ -6,7 +6,7 @@ "comment": "Test secret for acceptance testing", "name": "test_secret_[UNIQUE_NAME]", "schema_name": "default", - "value": "initial-secret-value" + "value": "[redacted]" } } diff --git a/bundle/direct/dresources/redact_config.go b/bundle/direct/dresources/redact_config.go new file mode 100644 index 00000000000..679001b2f09 --- /dev/null +++ b/bundle/direct/dresources/redact_config.go @@ -0,0 +1,81 @@ +package dresources + +import ( + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/structs/structpath" +) + +// RedactSensitiveConfigValues walks the bundle config dyn.Value and replaces the +// value of every field declared as sensitive_fields for its resource type with +// "[redacted]". This is used before printing the full config to stdout (e.g. +// `bundle validate -o json`) so plaintext secrets are never shown to the user. +// +// The function only handles resource types that have at least one sensitive_field +// declared in resources.yml. Each sensitive field pattern is translated to a +// dyn.Pattern of the form: resources..*.. +func RedactSensitiveConfigValues(v dyn.Value) (dyn.Value, error) { + patterns := buildSensitivePatterns() + for _, pat := range patterns { + var err error + v, err = dyn.MapByPattern(v, pat, func(p dyn.Path, _ dyn.Value) (dyn.Value, error) { + return dyn.V(sensitiveRedactedMarker), nil + }) + if err != nil { + return dyn.InvalidValue, err + } + } + return v, nil +} + +const sensitiveRedactedMarker = "[redacted]" + +// buildSensitivePatterns returns one dyn.Pattern per sensitive field rule across +// all resource types. Each pattern covers: resources..*.. +func buildSensitivePatterns() []dyn.Pattern { + cfg := MustLoadConfig() + var patterns []dyn.Pattern + for resourceType, rc := range cfg.Resources { + for _, rule := range rc.SensitiveFields { + if rule.Field == nil { + continue + } + fieldPat := structPathToDynPattern(rule.Field) + if fieldPat == nil { + continue + } + // resources..* + field pattern components + base := dyn.NewPattern( + dyn.Key("resources"), + dyn.Key(resourceType), + dyn.AnyKey(), + ) + full := append(base, fieldPat...) + patterns = append(patterns, full) + } + } + return patterns +} + +// structPathToDynPattern converts a structpath.PatternNode to a slice of +// dyn.patternComponent values. Returns nil if conversion is not possible. +func structPathToDynPattern(node *structpath.PatternNode) dyn.Pattern { + if node == nil || node.IsRoot() { + return nil + } + + segments := node.AsSlice() + pat := make(dyn.Pattern, 0, len(segments)) + for _, seg := range segments { + if seg.BracketStar() || seg.DotStar() { + pat = append(pat, dyn.AnyKey()) + } else if idx, ok := seg.Index(); ok { + pat = append(pat, dyn.Index(idx)) + } else if key, ok := seg.StringKey(); ok { + pat = append(pat, dyn.Key(key)) + } else { + // Unsupported segment type; skip entire pattern. + return nil + } + } + return pat +} diff --git a/cmd/bundle/validate.go b/cmd/bundle/validate.go index a2ec31f721b..ec05e8f2be7 100644 --- a/cmd/bundle/validate.go +++ b/cmd/bundle/validate.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/direct/dresources" "github.com/databricks/cli/bundle/render" "github.com/databricks/cli/cmd/bundle/utils" "github.com/databricks/cli/cmd/root" @@ -17,7 +18,11 @@ func renderJsonOutput(cmd *cobra.Command, b *bundle.Bundle) error { if b == nil { return nil } - buf, err := json.MarshalIndent(b.Config.Value().AsAny(), "", " ") + configValue, err := dresources.RedactSensitiveConfigValues(b.Config.Value()) + if err != nil { + return err + } + buf, err := json.MarshalIndent(configValue.AsAny(), "", " ") if err != nil { return err }