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 04295a71d21..0ea874aac37 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 04295a71d21..0ea874aac37 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 9d79e11bef9..cb927f4a69a 100644 --- a/acceptance/bundle/invariant/migrate/test.toml +++ b/acceptance/bundle/invariant/migrate/test.toml @@ -38,5 +38,7 @@ EnvMatrixExclude.no_pydabs_1000_tasks = ["INPUT_CONFIG=job_pydabs_1000_tasks.yml # volume schema_name ("inconsistent final plan"). Covered by no_drift on direct. EnvMatrixExclude.no_volume_uppercase = ["INPUT_CONFIG=volume_uppercase_name.yml.tmpl"] +EnvMatrixExclude.no_secret = ["INPUT_CONFIG=secret.yml.tmpl"] + # Terraform types sampling_fraction as an integer and truncates 0.5; covered by no_drift. EnvMatrixExclude.no_model_serving_endpoint_telemetry = ["INPUT_CONFIG=model_serving_endpoint_telemetry.yml.tmpl"] diff --git a/acceptance/bundle/invariant/no_drift/out.test.toml b/acceptance/bundle/invariant/no_drift/out.test.toml index 04295a71d21..0ea874aac37 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 4d0777fdea3..faa2872a3b0 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/bundle/resources/secrets/basic/output.txt b/acceptance/bundle/resources/secrets/basic/output.txt index 1e3d1b9f46b..d1e25bdf8cf 100644 --- a/acceptance/bundle/resources/secrets/basic/output.txt +++ b/acceptance/bundle/resources/secrets/basic/output.txt @@ -1,12 +1,24 @@ ->>> [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 +>>> [CLI] bundle validate --var secret_value=initial-secret-value -o json + +>>> cat validate.json +{ + "secret1": { + "catalog_name": "main", + "comment": "Test secret for acceptance testing", + "name": "test_secret_[UNIQUE_NAME]", + "schema_name": "default", + "value": "[redacted]" + } +} -Validation OK! +>>> cat validate.json +{ + "secret_value": { + "description": "Test secret value", + "value": "[redacted]" + } +} >>> [CLI] bundle summary --var secret_value=initial-secret-value Name: test-bundle-[UNIQUE_NAME] diff --git a/acceptance/bundle/resources/secrets/basic/script b/acceptance/bundle/resources/secrets/basic/script index d7dd7b53f6f..07ddb03ddf5 100755 --- a/acceptance/bundle/resources/secrets/basic/script +++ b/acceptance/bundle/resources/secrets/basic/script @@ -6,7 +6,11 @@ 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 > validate.json +trace cat validate.json | jq -r ".resources.secrets" +trace cat validate.json | jq -r ".variables" +rm validate.json + 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..3af0d0e46b0 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]" } } } @@ -40,6 +40,57 @@ Deployment complete! 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 +{ + "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": "", + "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... 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/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/apply.go b/bundle/direct/apply.go index cbb0a2d45ff..3ed9d233f60 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.saveState(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.saveState(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.saveState(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.saveState(db, id, newState, d.DependsOn) if err != nil { return fmt.Errorf("saving state id=%s: %w", id, err) } @@ -299,6 +299,15 @@ func (d *DeploymentUnit) Resize(ctx context.Context, db *dstate.DeploymentState, return nil } +// saveState saves a state with sensitive fields replaced by a placeholder value so secrets are never written +// to disk in plaintext. +func (d *DeploymentUnit) saveState(db *dstate.DeploymentState, newID string, state any, dependsOn []deployplan.DependsOnEntry) error { + if err := zeroSensitiveFields(d.Adapter, state); err != nil { + return fmt.Errorf("redacting state: %w", err) + } + return db.SaveState(d.ResourceKey, newID, state, 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..21c29d78d33 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 } @@ -1070,15 +1080,31 @@ func (b *DeploymentBundle) makePlan(ctx context.Context, configRoot *config.Root return strings.Compare(a.Label, b.Label) }) + // Store an unredacted copy in the cache so Apply can deploy with the + // actual values. The original newStateConfig is redacted below and used + // only for the plan output. + stateType := adapter.StateType() + cacheCopyPtr := reflect.New(stateType.Elem()) + cacheCopyPtr.Elem().Set(reflect.ValueOf(newStateConfig).Elem()) + b.StateCache.Store(node, &structvar.StructVar{ + Value: cacheCopyPtr.Interface(), + Refs: refs, + }) + + // Redact sensitive fields before serialising. Sensitive values always come + // from bundle variables (enforced by ValidateSecretValueIsVariable), which + // are resolved before plan time, so SyncToJSON (called when cross-resource + // refs are resolved during planNode) will never re-serialise these fields. + if err := redactStruct(adapter, newStateConfig); err != nil { + return nil, fmt.Errorf("%s: cannot redact state: %w", node, err) + } + newState := &structvar.StructVar{ Value: newStateConfig, Refs: refs, } - // Store in cache for use during planning phase - b.StateCache.Store(node, newState) - - // Convert to JSON for serialization in plan + // Convert to JSON for serialization in plan (values already redacted above). newStateJSON, err := newState.ToJSON() if err != nil { return nil, fmt.Errorf("%s: cannot serialize state: %w", node, err) diff --git a/bundle/direct/dresources/adapter.go b/bundle/direct/dresources/adapter.go index ea8876c4d52..63f56479055 100644 --- a/bundle/direct/dresources/adapter.go +++ b/bundle/direct/dresources/adapter.go @@ -312,7 +312,8 @@ func (a *Adapter) validate() error { // If RemapState is implemented, validate its signature. // Otherwise require remote type to equal state type so remapping isn't needed. if a.remapState != nil { - validations = append(validations, + validations = append( + validations, "RemapState input", a.remapState.InTypes[0], remoteType, "RemapState return", a.remapState.OutTypes[0], stateType, ) @@ -413,6 +414,15 @@ func (a *Adapter) GeneratedResourceConfig() *ResourceLifecycleConfig { return a.generatedResourceConfig } +// GetSensitiveFields returns the list of sensitive fields for the resource. +func (a *Adapter) GetSensitiveFields() []string { + var fields []string + for _, r := range a.resourceConfig.SensitiveFields { + fields = append(fields, r.Field.String()) + } + return fields +} + // 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/redact_config.go b/bundle/direct/dresources/redact_config.go new file mode 100644 index 00000000000..dbd6ccda83d --- /dev/null +++ b/bundle/direct/dresources/redact_config.go @@ -0,0 +1,72 @@ +package dresources + +import ( + "reflect" + + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/libs/structs/structaccess" + "github.com/databricks/cli/libs/structs/structpath" + "github.com/databricks/cli/libs/structs/structwalk" +) + +// RedactSensitiveConfigValues walks the bundle config and replaces the +// value of every field declared as sensitive_fields for its resource type with +// "[redacted]". It also redacts the entire variables section when any sensitive +// resource fields are present, since variables are the standard mechanism for +// supplying secret values and their resolved values must not appear in the +// output of `bundle validate -o json`. +func RedactSensitiveConfigValues(root *config.Root) (*config.Root, error) { + fields := getSensitiveFields() + + var hasSensitiveFields bool + for resourceType, fieldRules := range fields { + resources, err := structaccess.GetByString(root, "resources."+resourceType) + if err != nil { + return nil, err + } + err = structwalk.Walk(resources, func(path *structpath.PathNode, val any, _ *reflect.StructField) { + for _, fieldRule := range fieldRules { + // The first segment of the path is the resource key, so we need to skip it and check the rest of the path + rest := path.SkipPrefix(1) + if rest.HasPatternPrefix(fieldRule.Field) { + hasSensitiveFields = true + _ = structaccess.SetByString(root, "resources."+resourceType+path.String(), sensitiveRedactedMarker) + } + } + }) + if err != nil { + return nil, err + } + } + + // Redact all variable values when any sensitive resource fields are present. + // Variables are the standard mechanism for supplying secret values and their + // resolved values must not appear in plaintext in the validate output. + if hasSensitiveFields { + for _, variable := range root.Variables { + if variable == nil { + continue + } + if _, ok := variable.Value.(string); ok { + variable.Value = sensitiveRedactedMarker + } + } + } + + return root, nil +} + +const sensitiveRedactedMarker = "[redacted]" + +// getSensitiveFields returns the map of resource type to list of sensitive fields. +func getSensitiveFields() map[string][]FieldRule { + cfg := MustLoadConfig() + fields := make(map[string][]FieldRule) + for resourceType, rc := range cfg.Resources { + if rc.SensitiveFields == nil { + continue + } + fields[resourceType] = rc.SensitiveFields + } + return fields +} diff --git a/bundle/direct/dresources/resources.yml b/bundle/direct/dresources/resources.yml index 06502dd2017..e98a1ac2330 100644 --- a/bundle/direct/dresources/resources.yml +++ b/bundle/direct/dresources/resources.yml @@ -593,6 +593,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 3f3dc68a392..122ea0440e9 100644 --- a/bundle/direct/dresources/secret.go +++ b/bundle/direct/dresources/secret.go @@ -5,12 +5,14 @@ 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" "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,69 +22,49 @@ type ResourceSecret struct { client *databricks.WorkspaceClient } -// 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 *catalog.Secret) *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.EffectiveValue, +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), } } @@ -105,14 +87,10 @@ func (r *ResourceSecret) DoRead(ctx context.Context, id string) (*catalog.Secret } // DoCreate creates a new UC secret. -func (r *ResourceSecret) DoCreate(ctx context.Context, state *SecretState) (string, *catalog.Secret, 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. - // Fingerprint already captures whether the value changed. - state.Value = "" if err != nil || response == nil { return "", nil, err } @@ -120,17 +98,14 @@ func (r *ResourceSecret) DoCreate(ctx context.Context, state *SecretState) (stri } // 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) { - 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 } @@ -138,21 +113,27 @@ func (r *ResourceSecret) DoUpdate(ctx context.Context, id string, state *SecretS } // 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 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. -func (s SecretState) MarshalJSON() ([]byte, error) { - return marshal.Marshal(s) -} - -// UnmarshalJSON deserializes SecretState, restoring both the embedded -// catalog.Secret fields and Fingerprint. -func (s *SecretState) UnmarshalJSON(b []byte) error { - return marshal.Unmarshal(b, s) +// 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.ReasonCustom + } else { + ch.Action = deployplan.Update + } + return nil } diff --git a/bundle/direct/redact.go b/bundle/direct/redact.go new file mode 100644 index 00000000000..b44ab74e7b4 --- /dev/null +++ b/bundle/direct/redact.go @@ -0,0 +1,99 @@ +package direct + +import ( + "fmt" + "slices" + + "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" +) + +const sensitiveRedactedValue = "[redacted]" + +// 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 + } + + fields := adapter.GetSensitiveFields() + for _, field := range fields { + path, err := structpath.ParsePath(field) + if err != nil { + return fmt.Errorf("parsing sensitive field path %q: %w", field, err) + } + + fv, err := structaccess.Get(s, path) + if err != nil { + continue + } + if fv == nil { + // structaccess.Get returns nil for omitempty fields whose value is zero. + // Such fields are absent from JSON, so there is nothing to redact or zero. + continue + } + _ = structaccess.Set(s, path, replacement) + } + + 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 { + fields := adapter.GetSensitiveFields() + for pathString, ch := range changes { + if slices.Contains(fields, pathString) { + if v, ok := ch.Old.(string); ok && v != "" { + ch.Old = sensitiveRedactedValue + } + if v, ok := ch.New.(string); ok && v != "" { + ch.New = sensitiveRedactedValue + } + if v, ok := ch.Remote.(string); ok && v != "" { + ch.Remote = sensitiveRedactedValue + } + } + } + return nil +} + +// redactPlanEntry redacts sensitive fields from remote_state and changes of a plan +// entry. new_state is already redacted before serialization in makePlan. +func redactPlanEntry(adapter *dresources.Adapter, entry *deployplan.PlanEntry) error { + if len(adapter.GetSensitiveFields()) == 0 { + return nil + } + + 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 +} diff --git a/cmd/bundle/validate.go b/cmd/bundle/validate.go index a2ec31f721b..c9dcb162345 100644 --- a/cmd/bundle/validate.go +++ b/cmd/bundle/validate.go @@ -5,9 +5,11 @@ 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" + "github.com/databricks/cli/libs/dyn/convert" "github.com/databricks/cli/libs/flags" "github.com/databricks/cli/libs/logdiag" "github.com/spf13/cobra" @@ -17,7 +19,15 @@ func renderJsonOutput(cmd *cobra.Command, b *bundle.Bundle) error { if b == nil { return nil } - buf, err := json.MarshalIndent(b.Config.Value().AsAny(), "", " ") + redactedRoot, err := dresources.RedactSensitiveConfigValues(&b.Config) + if err != nil { + return err + } + converted, err := convert.FromTyped(redactedRoot, b.Config.Value()) + if err != nil { + return err + } + buf, err := json.MarshalIndent(converted.AsAny(), "", " ") if err != nil { return err }