Skip to content

Do not report cluster fields supplied by a cluster policy as drift - #6531

Draft
denik wants to merge 5 commits into
mainfrom
denik/cluster-policy-no-drift
Draft

Do not report cluster fields supplied by a cluster policy as drift#6531
denik wants to merge 5 commits into
mainfrom
denik/cluster-policy-no-drift

Conversation

@denik

@denik denik commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

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 and jobs.job_clusters[*].new_cluster.
  • libs/testserver now applies cluster policies (fixed always, defaultValue only with apply_policy_default_values, never overriding a supplied value, rejecting a value that contradicts a fixed element). 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. Measured against a real workspace:

policy element apply_policy_default_values config omits attribute config sets conflicting value
fixed unset supplied 400 rejected
fixed true supplied 400 rejected
defaultValue unset not supplied allowed
defaultValue true supplied allowed

forbidden is the only element type that never supplies a value. policy_value_semantics records this so the gate's rationale stays checked rather than assumed.

There is deliberately no rule for pipelines.clusters[*]: the Pipelines API does not expand policies into the stored spec, so there is no addition to suppress.

Fixes #5179
Fixes #6512

Tests

New cloud tests under acceptance/bundle/resources/cluster_policies/ (all also run locally against the testserver):

  • policy_value_semantics — the table above, in one deploy.
  • fixed_values_applied — a fixed element supplying spark_version and a tag the config never declares.
  • fixed_value_conflict — the same element rejecting a contradicting value with 400.
  • policy_drift — 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 — every cluster-spec location converges.

Also unit tests for the classifier and the testserver policy application, and a config test that validates every rule's pattern and gate against the resource's state type. The existing invariant/no_drift suite already covers apply_policy_default_values for all five cluster-spec locations and still passes.

This pull request and its description were written by Isaac.

## 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
@eng-dev-ecosystem-bot

eng-dev-ecosystem-bot commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Integration test report

Commit: 1ffb64a

Run: 33900750398

Env 💚​RECOVERED ✅​pass 🙈​skip Time
💚​ aws linux 1 287 16 6:54
💚​ aws windows 1 289 14 5:57
💚​ azure linux 1 286 16 7:43
💚​ azure windows 1 288 14 5:56
💚​ gcp linux 1 287 16 7:52
💚​ gcp windows 1 289 14 5:50
Test Name aws linux aws windows azure linux azure windows gcp linux gcp windows
💚​ TestAccept 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R
Top 6 slowest tests (at least 2 minutes):
duration env testname
4:27 azure windows TestAccept
4:15 aws windows TestAccept
3:58 gcp windows TestAccept
3:48 azure linux TestAccept
3:45 aws linux TestAccept
3:38 gcp linux TestAccept

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants