Skip to content

Honor --key in bundle generate dashboard and make lookup flags mutually exclusive#5492

Merged
simonfaltum merged 7 commits into
mainfrom
simonfaltum/b19-generate-dashboard-key
Jul 2, 2026
Merged

Honor --key in bundle generate dashboard and make lookup flags mutually exclusive#5492
simonfaltum merged 7 commits into
mainfrom
simonfaltum/b19-generate-dashboard-key

Conversation

@simonfaltum

Copy link
Copy Markdown
Member

Why

Found during a full-repo review of the CLI. bundle generate dashboard silently ignored the --key flag, even though its own help text documents it and every other generate command (job, pipeline, alert) honors it. In addition, the three lookup flags (--existing-path, --existing-id, --resource) could be combined freely, with one silently shadowing the others, so users got no signal that part of their invocation was ignored.

Changes

Before, the dashboard resource key was always derived from the dashboard's display name and conflicting lookup flags were silently accepted; now --key takes precedence over the derived key and passing more than one lookup flag fails with a validation error.

  • cmd/bundle/generate/dashboard.go: read cmd.Flag("key") and fall back to the normalized display name when it is empty, matching the pattern used by job.go. The key flag is a persistent flag on the parent generate command.
  • cmd/bundle/generate/dashboard.go: mark --existing-path, --existing-id, and --resource as mutually exclusive. Combined with the existing MarkFlagsOneRequired, exactly one lookup flag is now required, which is what the existing comment already promised.

Test plan

  • New unit test TestDashboard_LookupFlagsAreMutuallyExclusive covering all three conflicting flag pairs (go test ./cmd/bundle/generate/)
  • Extended the acceptance/bundle/generate/dashboard_existing_path_nominal acceptance test with a --key custom_key invocation; verified the generated config uses the custom key (go test ./acceptance -run TestAccept/bundle/generate/dashboard_existing_path_nominal, both engine variants pass)
  • ./task fmt-q, ./task lint-q, and ./task checks pass

This pull request and its description were written by Isaac.

The dashboard generator always derived the resource key from the display
name even though --key is documented in its help text and honored by the
other generate commands. Also enforce that only one of --existing-path,
--existing-id, and --resource is provided instead of silently preferring
one over the others.

Co-authored-by: Isaac
@simonfaltum simonfaltum requested a review from denik June 9, 2026 19:48
@simonfaltum simonfaltum temporarily deployed to test-trigger-is June 9, 2026 19:49 — with GitHub Actions Inactive
@simonfaltum simonfaltum temporarily deployed to test-trigger-is June 9, 2026 19:49 — with GitHub Actions Inactive
@eng-dev-ecosystem-bot

eng-dev-ecosystem-bot commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

Integration test report

Commit: 0b839b4

Run: 28578720454

Env 🟨​KNOWN 💚​RECOVERED 🙈​SKIP ✅​pass 🙈​skip Time
🟨​ aws linux 7 3 13 230 1040 5:20
🟨​ aws windows 7 3 13 232 1038 6:22
💚​ aws-ucws linux 10 13 314 958 4:48
💚​ aws-ucws windows 10 13 316 956 4:15
💚​ azure linux 4 15 230 1039 4:08
💚​ azure windows 4 15 232 1037 3:21
💚​ azure-ucws linux 4 15 316 955 4:50
💚​ azure-ucws windows 4 15 318 953 3:31
💚​ gcp linux 4 15 229 1041 3:55
💚​ gcp windows 4 15 231 1039 3:23
23 interesting tests: 13 SKIP, 7 KNOWN, 3 RECOVERED
Test Name aws linux aws windows aws-ucws linux aws-ucws windows azure linux azure windows azure-ucws linux azure-ucws windows gcp linux gcp windows
🟨​ TestAccept 🟨​K 🟨​K 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R
🙈​ TestAccept/bundle/invariant/no_drift 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/permissions 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🟨​ TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/with_permissions 🟨​K 🟨​K 💚​R 💚​R 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🟨​ TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/with_permissions/DATABRICKS_BUNDLE_ENGINE=direct 🟨​K 🟨​K 💚​R 💚​R
🟨​ TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/with_permissions/DATABRICKS_BUNDLE_ENGINE=terraform 🟨​K 🟨​K 💚​R 💚​R
🟨​ TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/without_permissions 🟨​K 🟨​K 💚​R 💚​R 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🟨​ TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/without_permissions/DATABRICKS_BUNDLE_ENGINE=direct 🟨​K 🟨​K 💚​R 💚​R
🟨​ TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/without_permissions/DATABRICKS_BUNDLE_ENGINE=terraform 🟨​K 🟨​K 💚​R 💚​R
🙈​ TestAccept/bundle/resources/postgres_branches/basic 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/postgres_branches/recreate 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/postgres_branches/replace_existing 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/postgres_branches/update_protected 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/postgres_branches/without_branch_id 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/postgres_endpoints/basic 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/postgres_projects/update_display_name 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/synced_database_tables/basic 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/vector_search_endpoints/drift/recreated_same_name 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/vector_search_indexes/recreate/embedding_dimension 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/ssh/connection 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
💚​ TestFetchRepositoryInfoAPI_FromRepo 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R
💚​ TestFetchRepositoryInfoAPI_FromRepo/root 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R
💚​ TestFetchRepositoryInfoAPI_FromRepo/subdir 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R
Top 4 slowest tests (at least 2 minutes):
duration env testname
2:52 aws-ucws windows TestAccept
2:34 gcp windows TestAccept
2:31 azure-ucws windows TestAccept
2:28 azure windows TestAccept

Co-authored-by: Isaac
@simonfaltum simonfaltum requested review from mihaimitrea-db and removed request for denik June 12, 2026 10:45
Comment on lines +553 to +557
cmd.MarkFlagsMutuallyExclusive(
"existing-path",
"existing-id",
"resource",
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would happen before this? Based on a quick view it seems that previously there was some precedence between flags where some would get ignored. Not good.

However, now we'll throw an error. Is this a breaking changes? What if people are using multiple flags somewhere and it just works for them?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. Before this change the three lookup flags weren't validated together, so combining them silently applied a precedence: --resource beat --existing-path beat --existing-id. If you passed more than one, the lower-precedence flags were just ignored and you got output based on a flag you may not have intended, with no signal that part of your invocation was dropped.

So yes, it's technically a behavior change: an invocation that passed multiple lookup flags and "worked" before will now error. But it only ever "worked" by silently ignoring input, so any such invocation was already producing a result the user didn't fully specify. A couple of things keep the risk low:

  • The error fires at flag-parse time, before any API call or file write, and cobra names the conflicting flags, so the fix is obvious (drop the extra flag).
  • Exactly-one-required was already the documented contract here (the existing MarkFlagsOneRequired and the comment above it), this just enforces the other half of it.
  • It matches how the sibling generate commands treat their flags, and the repo's own CLI UX guidance: reject incompatible inputs early with an actionable error rather than silently ignoring a flag.

I'll add a NEXT_CHANGELOG.md entry noting the behavior change so it shows up in the release notes.

@mihaimitrea-db mihaimitrea-db left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@simonfaltum simonfaltum enabled auto-merge July 2, 2026 09:28
@simonfaltum simonfaltum added this pull request to the merge queue Jul 2, 2026
Merged via the queue into main with commit 4e30c6e Jul 2, 2026
25 checks passed
@simonfaltum simonfaltum deleted the simonfaltum/b19-generate-dashboard-key branch July 2, 2026 09:58
deco-sdk-tagging Bot added a commit that referenced this pull request Jul 2, 2026
## Release v1.6.0

### CLI

 * `ssh connect` now accepts a `--base-environment` flag to run a serverless session on a custom base environment. It takes an `env.yaml` path, a `workspace-base-environments/...` resource ID, or a base environment display name, and is rejected together with `--environment-version` or `--cluster` ([#5706](#5706)).
 * `databricks aitools install` is now plugin-first: it installs the Databricks plugin through each agent's own CLI (Claude Code, Codex, GitHub Copilot) instead of copying raw skill files. Agents without a plugin (OpenCode, Antigravity) still get skill files, and Cursor prints the `/add-plugin databricks` step. Use `--skills-only` to force raw skill files for every agent, or `--path <dir>` to write skills to a directory ([#5738](#5738)).
 * `databricks labs list` now only shows projects that can be installed ([#5560](#5560)).

### Bundles

 * direct: Fixed persistent drift on `model_serving_endpoints` caused by the `traffic_config` field ([#5708](#5708)).
 * direct: Fix spurious update when `apply_policy_default_values: true` is set on job task, for-each-task, or job cluster new_cluster ([#5731](#5731)). Also fix spurious updates for for-each-task clusters due to missing backend defaults for `data_security_mode`, `node_type_id`, `driver_node_type_id`, `driver_instance_pool_id`, `enable_elastic_disk`, and `enable_local_disk_encryption`.
 * direct: Cluster resize now falls back to regular update if resize fails due to `INVALID_STATE` ([#5716](#5716)).
 * `bundle generate dashboard` now honors the `--key` flag when naming the generated resource, and rejects combining `--existing-path`, `--existing-id`, and `--resource` instead of silently ignoring all but one ([#5492](#5492)).
 * Fixed `bundle deployment migrate` failing on `model_serving_endpoints`/`database_instances` with permissions (regression since v1.5.0) ([#5775](#5775)).
 * After a terraform deploy, the CLI now dry-runs a migration to the direct engine (writing nothing locally or remotely) and reports the outcome via telemetry, warning if the migration could not be completed ([#5797](#5797)).

### Dependency updates
 * Bump `github.com/databricks/databricks-sdk-go` from v0.147.0 to v0.152.0 ([#5773](#5773)).
 * Bump Terraform provider from v1.118.0 to v1.120.0 ([#5792](#5792)).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants