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/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/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/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"] 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/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 diff --git a/bundle/direct/dresources/secret.go b/bundle/direct/dresources/secret.go index 3f3dc68a392..09ce26d34d3 100644 --- a/bundle/direct/dresources/secret.go +++ b/bundle/direct/dresources/secret.go @@ -20,6 +20,18 @@ 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 @@ -62,7 +74,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 +94,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 +113,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 +150,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 +160,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..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)) } 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 }