Repository navigation
Avoid syncing cluster policy values into shared bundle config - #7170
ilyakuz-db wants to merge 12 commits into
Conversation
|
Commit: Regression test reportSelected 6 changed tests (limit=20, 0 not selected) 12 tests: regression=10 unreleased=0 coverage=2 failing=0
TestAccept/bundle/config-remote-sync/cluster_policy_cross_target/DMS=: branch ✅ | main (aae07ca) ❌ | latest (v1.23.0) ❌main (aae07ca): latest (v1.23.0): same output as main (aae07ca) TestAccept/bundle/config-remote-sync/cluster_policy_cross_target/DMS=true: branch ✅ | main (aae07ca) ❌ | latest (v1.23.0) ❌main (aae07ca): latest (v1.23.0): same output as main (aae07ca) TestAccept/bundle/config-remote-sync/cluster_policy_remote_addition/DMS=: branch ✅ | main (aae07ca) ❌ | latest (v1.23.0) ❌main (aae07ca): latest (v1.23.0): same output as main (aae07ca) TestAccept/bundle/config-remote-sync/cluster_policy_remote_addition/DMS=true: branch ✅ | main (aae07ca) ❌ | latest (v1.23.0) ❌main (aae07ca): latest (v1.23.0): same output as main (aae07ca) TestAccept/bundle/config-remote-sync/cluster_policy_replaces_array/DMS=: branch ✅ | main (aae07ca) ❌ | latest (v1.23.0) ❌main (aae07ca): latest (v1.23.0): same output as main (aae07ca) TestAccept/bundle/config-remote-sync/cluster_policy_replaces_array/DMS=true: branch ✅ | main (aae07ca) ❌ | latest (v1.23.0) ❌main (aae07ca): latest (v1.23.0): same output as main (aae07ca) TestAccept/bundle/config-remote-sync/external_cluster_policy_cross_target/DMS=: branch ✅ | main (aae07ca) ❌ | latest (v1.23.0) ❌main (aae07ca): latest (v1.23.0): same output as main (aae07ca) TestAccept/bundle/config-remote-sync/external_cluster_policy_cross_target/DMS=true: branch ✅ | main (aae07ca) ❌ | latest (v1.23.0) ❌main (aae07ca): latest (v1.23.0): same output as main (aae07ca) TestAccept/bundle/config-remote-sync/missing_cluster_policy/DMS=: branch ✅ | main (aae07ca) ❌ | latest (v1.23.0) ❌main (aae07ca): latest (v1.23.0): same output as main (aae07ca) TestAccept/bundle/config-remote-sync/missing_cluster_policy/DMS=true: branch ✅ | main (aae07ca) ❌ | latest (v1.23.0) ❌main (aae07ca): latest (v1.23.0): same output as main (aae07ca) |
Integration test reportCommit: 614f1e2
Top 8 slowest tests (at least 2 minutes):
|
…ial sync status Co-authored-by: Isaac <no-reply@databricks.com>
…model, drop keyed paths Co-authored-by: Isaac <no-reply@databricks.com>
… permission denied Co-authored-by: Isaac <no-reply@databricks.com>
| // one environment's policy values into (often shared) config and breaks deploys in | ||
| // other environments. TODO: move to backend_defaults in jobs.yml once | ||
| // configsync filtering is migrated to the direct engine lifecycle metadata. | ||
| "resources.jobs.*.tasks[*].new_cluster.custom_tags": backendDefault, |
There was a problem hiding this comment.
Context for removing the custom_tags / cluster_log_conf blanket skip (originally added in #5610 as a quick fix for policy-injected values leaking into shared YAML).
The new policy-aware filter covers the cases that skip was protecting, and the skip itself turned out to be incomplete: it only applied when the field was entirely absent from config. A cluster with a single authored tag still got the policy's tags synced in beside it.
Verified against a real AWS workspace with two policies (dev/prod) fixing custom_tags.*, spark_env_vars.*, workload_type.clients.* and an S3 cluster_log_conf (type/path/region), a shared job with one authored tag, and a UI edit (description + user-added tag) applied via jobs reset:
| Scenario | v1.23.0 (blanket skip) | this PR |
|---|---|---|
| Policy-fixed tags next to an authored tag | synced, prod deploy fails | filtered |
Policy-fixed spark_env_vars, workload_type |
synced, prod deploy fails | filtered |
Policy-fixed S3 cluster_log_conf |
skipped | filtered |
| User tag added in UI | synced | synced |
| Description edited in UI | synced | synced |
| Shared YAML deployed to prod | Cluster validation error: ... must be prod (is "dev") |
succeeds, next sync and plan report no changes |
The backend materialized the S3 log conf as exactly s3.destination + s3.region, both mapped back to the policy's path / region rules, so nothing is left to leak. With that, these entries are safe to drop.
Changes
config-remote-syncno longer copies cluster policy values into bundle YAML. After changes are extracted and selected, job and cluster specs whose remotepolicy_idis set are checked against that policy (read lazily, once per sync):fixedrules, including*wildcards, are never synced. The backend rejects any other value for them, so the remote value always comes from the policy.defaultValueandapply_policy_default_valuesis set; any other value is a user edit and syncs.skippedChangeswith the newpartialstatus, and other edits still sync.Clusters without a policy are not filtered. The blanket
custom_tags/cluster_log_confskip is removed, so tags added in the UI now sync.Why
Syncing a UI edit could copy one target's policy values into shared YAML, and deploying that YAML to a target with a different policy then failed validation.
Tests
Unit tests for rule matching, plus acceptance tests for cross-target deploys, missing policies and deploy-and-plan after sync.
This PR was written with assistance from Codex.
This pull request and its description were written by Isaac.