Do not report cluster fields supplied by a cluster policy as drift - #6531
Open
denik wants to merge 22 commits into
Open
Do not report cluster fields supplied by a cluster policy as drift#6531denik wants to merge 22 commits into
denik wants to merge 22 commits into
Conversation
## Changes - New `ignore_remote_additions` rule in `bundle/direct/dresources/resources.yml`: an object plus a `when_set` field that gates it. Inside a gated object, a field absent from both state and config but present in the remote is skipped with reason `policy_managed`. A disagreement between config and remote, and a field the user removed from config, still report an update. - The gate is `policy_id`, applied to every place a cluster spec appears: `clusters`, `jobs.tasks[*].new_cluster`, `jobs.tasks[*].for_each_task.task.new_cluster`, `jobs.job_clusters[*].new_cluster` and `pipelines.clusters[*]`. - `libs/testserver` now applies cluster policies: `fixed` elements always, `defaultValue` elements only when the request sets `apply_policy_default_values`, never overriding a value the request supplied. Without this the new tests would pass locally for the wrong reason. - `jobFixUps` dropped `apply_policy_default_values` by zeroing it but left it in `ForceSendFields`, so it serialized as an explicit `false` where the Jobs API returns nothing. Fixed. ## Why A cluster policy supplies settings server-side, so the remote spec is a superset of what the bundle declares and every deploy planned an update that never converged. The gate is `policy_id` rather than `apply_policy_default_values`, because `fixed` policy elements are applied whether or not that flag is set — both reporters hit the bug with the flag unset. `acceptance/bundle/resources/cluster_policies/policy_value_semantics` records the measured backend behaviour for all four combinations. Fixes #5179 Fixes #6512 ## Tests - `policy_value_semantics` pins {`fixed`, `defaultValue`} x {flag, no flag} against a real workspace; `fixed_values_applied` shows a `fixed` element supplying `spark_version` and a tag the config never declares; `fixed_value_conflict` shows it rejecting a contradicting value with 400. - `policy_drift` covers the classification matrix in one golden: policy addition skipped, out-of-band change to a config-owned tag still an update, removal from config still an update, no `policy_id` still an update. - `policy_no_drift_variants` covers all five cluster-spec locations. - Unit tests for the classifier and for the testserver policy application; a config test validates every rule's pattern and gate against the state type. Co-authored-by: Isaac
Follow-up from re-recording the new tests against a real workspace. - The Pipelines API does not expand cluster policies into the stored spec: a pipeline cluster with a policy_id reads back exactly as authored. So there is nothing to suppress; the `pipelines.clusters[*]` rule is dropped and the testserver no longer applies policies there. `policy_no_drift_variants` keeps the pipeline case to record that. - `libs/testserver` now rejects a value that contradicts a `fixed` element, reproducing the backend's validation message. Without it `fixed_value_conflict` could not pass both locally and in cloud. Co-authored-by: Isaac
Co-authored-by: Isaac
The test proved the plan was clean but not why. spark_version is the field from issue 6512 that no backend_defaults rule covers, so print its classification to assert the suppression comes from policy_managed. Co-authored-by: Isaac
Collaborator
Integration test reportCommit: 8aeb76c
Top 4 slowest tests (at least 2 minutes):
|
Creating a real cluster made it the slowest test in the integration suite, at 6-8 minutes per env across six envs. It still runs locally on every test run and in full cloud runs; the cheap job-based tests in the same directory keep the core behaviour in every cloud run. Co-authored-by: Isaac
It was a bare string joined onto the matched prefix with NewDotString, which is asymmetric with field and silently accepts a dotted value as one field name. Resolving in two steps -- the matched object, then the gate relative to it -- needs no path-join helper and supports a multi-segment gate for free. Co-authored-by: Isaac
fixed_value_conflict, fixed_values_applied, policy_value_semantics and policy_drift attach a cluster policy to a job's cluster, so the resource under test is jobs, not cluster_policies. The old path implied the opposite. Their shared config (Cloud, direct-only engine matrix, no request recording) moves to a single test.toml at the new parent. policy_no_drift_variants stays under resources/cluster_policies: it spans clusters, jobs and pipelines, so no single resource owns it. Co-authored-by: Isaac
- Changelog no longer claims a field was "supplied by a cluster policy": the rule gates on policy_id being set and suppresses any config-absent remote field, without verifying the field's origin. Reworded to describe that rule. - Cluster policy definitions are authored as inline YAML instead of an embedded JSON string, the form the CLI already supports (see definition_yaml). - The plan/jobs-get assertions use gron.py | grep instead of inline jq, per the repo guidance; the flattened paths are self-documenting. Co-authored-by: Isaac
The rule gates on policy_id being set and suppresses a config-absent, remote-present field; it never verifies the value came from the policy. Name the reason after what is observed -- a remote-only addition -- to match the ignore_remote_additions rule and drop the unfounded provenance claim. Co-authored-by: Isaac
…-only The grepped gron lines were hard to read. Record the whole `bundle plan -o json` instead: with everything nested it is legible, and it shows the suppression at every cluster-spec location in one place, including that a policy-attached cluster suppresses backend-default fields (data_security_mode, lifecycle) as remote_addition too -- both still skip, so convergence is unchanged. The whole plan carries remote_state and *_attributes, which are cloud- and region-specific, so it cannot be recorded portably across the aws/azure/gcp sweep; the test becomes local-only. The suppression behavior stays cloud-verified by the projected jobs/cluster_policy tests. Co-authored-by: Isaac
…an.py Replaces the whole-plan and gron approaches. prune_plan.py drops the parts of `bundle plan -o json` that a testserver cannot reproduce and that differ by cloud -- remote_state, changes with reason in --ignore-reasons (managed, backend_default by default), and changes whose path matches --ignore-keys -- leaving the config- and policy-driven entries. What survives here is the policy custom_tags skipped as remote_addition at every cluster-spec location, plus lifecycle; all cloud-independent, so the test runs on all clouds. Also fixes the confusing comment above `clusters:` in the fixture. Co-authored-by: Isaac
It creates a standalone cluster, which boots at deploy time and repeatedly hit aws capacity limits, and whose full remote_state is a per-run cluster GET no testserver reproduces. Run it locally only, where the testserver is deterministic, and cover the four cluster-spec locations there. The suppression behavior on a real workspace stays covered by the jobs/cluster_policy tests. With cloud parity no longer required, the plan projection drops the cloud-portability --ignore-keys and keeps prune_plan.py's defaults. Co-authored-by: Isaac
…_variants read_id.py pol only registered an id replacement; after pruning remote_state the policy id no longer appears in the output, so it is dead. Rename the resource key pol to my_policy while here. Co-authored-by: Isaac
…ests Consistent, slightly more descriptive key than pol. Regenerated locally against the testserver; the rename is a resource-name string, independent of the cloud. Co-authored-by: Isaac
Co-authored-by: Isaac
The pull_request test workflow did not spawn on the previous few pushes (they landed during a GitHub Actions/JFrog incident window). Empty commit to force a fresh synchronize event; squashed at merge. Co-authored-by: Isaac
… main Merging main brought deployment-history recording, which adds a features block and deployment.version_id changes to the plan under DMS=true, diverging from the DMS="" variant. That recording is orthogonal to drift suppression and is covered by the jobs/cluster_policy tests, so run this local-only test once. Co-authored-by: Isaac
…o DMS="" Two CI failures on the post-merge run: - prune_plan.py was committed 100644, so on CI it ran as "Permission denied" (exit 126); the plan output was the error and .databricks leaked as an unexpected file. Committed 100755 like the other bin/ helpers. - EnvMatrix.DMS = [] gave the test no DMS tag, so it ran on BOTH the direct and directdms CI cells (the same trap config.go documents for DATABRICKS_BUNDLE_ENGINE=[]); on directdms the plan gained deployment-history fields the DMS-off golden lacks. Pin to DMS = "" to run once on the direct cell. Co-authored-by: Isaac
Co-authored-by: Isaac
Replace the fat, overlapping tests with focused ones, each pinning one dimension:
jobs/cluster_policy/
fixed_addition - fixed policy supplies an omitted scalar + map key
default_flag - defaultValue supplied only with apply_policy_default_values
config_conflicts_policy - config value contradicting a fixed policy -> 400
owned_tag_drift - out-of-band change/removal of a config-owned tag -> update
policy_id_scoping - policy_id gates per cluster spec, across every jobs location
clusters/cluster_policy - standalone cluster (CloudSlow; boots a real cluster)
pipelines/cluster_policy - pipeline cluster: no policy expansion, converges
Splitting per resource lets jobs and pipelines run on cloud (no cluster boot),
while the standalone clusters resource is isolated as CloudSlow. Outputs are
projected to the policy fields so they are cloud- and DMS-invariant, which also
retires prune_plan.py and the lumped local-only policy_no_drift_variants.
Co-authored-by: Isaac
From an adversarial review (cursor) plus manual review: - Descriptive resource/task keys everywhere (my_job, my_cluster, my_pipeline, gated/ungated, main) instead of c/j/p/t/small. - config_conflicts_policy asserts the rejection with musterr, not errcode, so an unexpected successful deploy fails the test. - Plan-classification output uses a focused `jq` over changes[…] instead of `gron.py | grep`, which also dumped new_state/remote_state. - fixed_addition now supplies the fixed scalar + map key at all three jobs cluster-spec locations (job_clusters, tasks[*].new_cluster, for_each), so the rule is shown firing at every jobs path; policy_id_scoping keeps the gate case. - clusters/cluster_policy stays tag-only, with a comment: the clusters resource force-sends spark_version, so an omitted scalar arrives as "" and a fixed policy rejects it; the scalar case is covered by fixed_addition (omitempty job-cluster fields). - Standardize hello_world.py to the jobs convention. Co-authored-by: Isaac
denik
marked this pull request as ready for review
September 11, 2026 10:33
This was referenced Sep 11, 2026
janniklasrose
approved these changes
Sep 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
Direct engine: a cluster field that a cluster policy supplies server-side is no longer reported as perpetual drift.
ignore_remote_additionsrule inbundle/direct/dresources/resources.yml: inside a cluster spec that setspolicy_id, a field present in the remote but absent from both state and config is skipped with reasonremote_addition. A disagreement with config, or a field removed from config, still reports anupdate.clusters,jobs.job_clusters[*].new_cluster,jobs.tasks[*].new_cluster,jobs.tasks[*].for_each_task.task.new_cluster. There is deliberately no rule forpipelines.clusters[*]— the Pipelines API does not expand policies into the stored spec, so there is nothing to suppress.libs/testservernow applies cluster policies so the tests run locally the same way they run on cloud:fixedalways,defaultValueonly withapply_policy_default_values, never overriding a value the request supplied, and rejecting a value that contradicts afixedelement.jobFixUpszeroedapply_policy_default_valuesbut left it inForceSendFields, serializing it as an explicitfalsewhere the Jobs API returns nothing.Why
A cluster policy supplies settings server-side, so the remote spec is a superset of what the bundle declares and every deploy planned an update that never converged.
The gate is
policy_id, notapply_policy_default_values:fixedpolicy elements are applied whether or not that flag is set, and both reporters hit the bug with it unset. Measured against a real workspace:apply_policy_default_valuesfixedfixedtruedefaultValuedefaultValuetrueThe reason is named
remote_addition, not "policy-managed": the rule only knows the field is a remote-only addition inside a policy-gated object — it does not verify the value came from the policy.Fixes #5179
Fixes #6512
Tests
One condition per test. Jobs and pipelines run on cloud (no cluster boot at deploy); the standalone
clustersresource boots a real cluster, so it isCloudSlow. All also run locally against the testserver.acceptance/bundle/resources/jobs/cluster_policy/:fixed_addition— afixedpolicy supplies an omitted scalar (spark_version) and map key (custom_tags) → bothremote_addition, converges.default_flag—defaultValueis supplied only withapply_policy_default_values(two clusters, off vs on).config_conflicts_policy— config value contradicting afixedelement → 400.owned_tag_drift— out-of-band change / removal of a config-owned tag →update, with the policy's own tag skipped beside it.policy_id_scoping—policy_idgates suppression per cluster spec (gated → skip, ungated → update), across all three jobs locations.acceptance/bundle/resources/clusters/cluster_policy/— standalone cluster (CloudSlow).acceptance/bundle/resources/pipelines/cluster_policy/— pipeline cluster: no expansion, converges.Plus unit tests for the classifier and the testserver policy application, and a config test validating every rule's pattern and
when_setgate against the resource's state type. The existinginvariant/no_driftsuite coversapply_policy_default_valuesfor all cluster-spec locations and still passes.This pull request and its description were written by Isaac.