diff --git a/acceptance/bundle/config-remote-sync/select_basic/output.txt b/acceptance/bundle/config-remote-sync/select_basic/output.txt index 14987921858..2909fc4e403 100644 --- a/acceptance/bundle/config-remote-sync/select_basic/output.txt +++ b/acceptance/bundle/config-remote-sync/select_basic/output.txt @@ -37,24 +37,45 @@ Resource: resources.jobs.job_two -=== An unknown resource id is rejected +=== A stale selector is skipped, not fatal: job_two still syncs alongside it +Detected changes in 1 resource(s): + +Resource: resources.jobs.job_two + max_concurrent_runs: replace + + + +>>> diff.py databricks.yml.backup databricks.yml +--- databricks.yml.backup ++++ databricks.yml +@@ -16,5 +16,5 @@ + + job_two: +- max_concurrent_runs: 2 ++ max_concurrent_runs: 10 + tasks: + - task_key: main + +=== An unknown resource id alone is rejected (no match at all must not silently succeed) + >>> [CLI] bundle config-remote-sync --select-ids jobs:no-such-id-123 Error: no deployed jobs resource with id no-such-id-123 Exit code: 1 -=== A selector without a type is rejected ->>> [CLI] bundle config-remote-sync --select-ids no-such-id-123 -Error: invalid --select-ids value "no-such-id-123", expected : (e.g. jobs:[NUMID]) - -Exit code: 1 +=== An id that exists under a different type matches nothing and is rejected too -=== An id that exists under a different type is rejected (no cross-type collision) >>> [CLI] bundle config-remote-sync --select-ids pipelines:[JOB_ONE_ID] Error: no deployed pipelines resource with id [JOB_ONE_ID] Exit code: 1 +=== A selector without a type is still rejected +>>> [CLI] bundle config-remote-sync --select-ids no-such-id-123 +Error: invalid --select-ids value "no-such-id-123", expected : (e.g. jobs:[NUMID]) + +Exit code: 1 + >>> [CLI] bundle destroy --auto-approve The following resources will be deleted: delete resources.jobs.job_one diff --git a/acceptance/bundle/config-remote-sync/select_basic/script b/acceptance/bundle/config-remote-sync/select_basic/script index a33b9f9e57c..a85bb0b46a6 100644 --- a/acceptance/bundle/config-remote-sync/select_basic/script +++ b/acceptance/bundle/config-remote-sync/select_basic/script @@ -38,11 +38,20 @@ title "Unfiltered sync still detects the job_two drift (no lost updates)" echo $CLI bundle config-remote-sync -title "An unknown resource id is rejected" -errcode trace $CLI bundle config-remote-sync --select-ids jobs:no-such-id-123 +title "A stale selector is skipped, not fatal: job_two still syncs alongside it" +echo +cp databricks.yml databricks.yml.backup +$CLI bundle config-remote-sync --select-ids "jobs:no-such-id-123,jobs:$job_two_id" --save +trace diff.py databricks.yml.backup databricks.yml +rm databricks.yml.backup -title "A selector without a type is rejected" -errcode trace $CLI bundle config-remote-sync --select-ids no-such-id-123 +title "An unknown resource id alone is rejected (no match at all must not silently succeed)" +echo +errcode trace $CLI bundle config-remote-sync --select-ids jobs:no-such-id-123 -title "An id that exists under a different type is rejected (no cross-type collision)" +title "An id that exists under a different type matches nothing and is rejected too" +echo errcode trace $CLI bundle config-remote-sync --select-ids "pipelines:$job_one_id" + +title "A selector without a type is still rejected" +errcode trace $CLI bundle config-remote-sync --select-ids no-such-id-123 diff --git a/bundle/configsync/select.go b/bundle/configsync/select.go index fcd8549d0be..443c94423ae 100644 --- a/bundle/configsync/select.go +++ b/bundle/configsync/select.go @@ -1,11 +1,13 @@ package configsync import ( + "context" "fmt" "slices" "strings" "github.com/databricks/cli/bundle/direct/dstate" + "github.com/databricks/cli/libs/log" ) // ResolveResourceSelectors maps ":" selectors to their plan keys @@ -21,11 +23,18 @@ import ( // is also why selection is independent from `bundle deploy --select`, which // matches "type.name" keys. // -// A selector that matches no deployed resource is an error: only deployed -// resources have an id, so a selector matching nothing is a caller mistake. +// A selector that matches no deployed resource is skipped rather than failing +// the run — but only when at least one other selector did match. The workspace +// UI batches every edited resource into one sync, so a single stale selector +// (a resource deleted remotely, or whose deploy state has drifted) must not +// drop the valid resources' edits. When NO selector matches, the error is +// returned instead: silently reporting "no changes" would let the UI show a +// success while nothing synced, hiding a real state problem. A malformed +// selector (missing ":" shape) is always an error, because that is a +// caller mistake rather than drift. // Duplicate selectors are deduplicated; the returned keys preserve the order in // which their selectors first appear. -func ResolveResourceSelectors(state *dstate.DeploymentState, selectors []string) ([]string, error) { +func ResolveResourceSelectors(ctx context.Context, state *dstate.DeploymentState, selectors []string) ([]string, error) { // Index deployed resources by ":". State keys have the form // "resources.."; indexing by the component means a // selector can only ever match a resource of that exact type, never an id @@ -48,6 +57,7 @@ func ResolveResourceSelectors(state *dstate.DeploymentState, selectors []string) } keys := make([]string, 0, len(selectors)) + var missing []string for _, selector := range selectors { resourceType, id, ok := strings.Cut(selector, ":") if !ok || resourceType == "" || id == "" { @@ -55,12 +65,26 @@ func ResolveResourceSelectors(state *dstate.DeploymentState, selectors []string) } key, ok := byTypeID[selector] if !ok { - return nil, fmt.Errorf("no deployed %s resource with id %s", resourceType, id) + missing = append(missing, selector) + continue } if !slices.Contains(keys, key) { keys = append(keys, key) } } + + // No selector matched a deployed resource: fail loudly instead of syncing + // nothing, so the caller does not report a spurious success. + if len(keys) == 0 { + resourceType, id, _ := strings.Cut(missing[0], ":") + return nil, fmt.Errorf("no deployed %s resource with id %s", resourceType, id) + } + + // Some selectors matched: skip the stale ones so the matched resources still + // sync (the UI batches several resources into one run). + for _, selector := range missing { + log.Debugf(ctx, "config-remote-sync: skipping selector %q, no deployed resource with that id", selector) + } return keys, nil } diff --git a/cmd/bundle/config_remote_sync.go b/cmd/bundle/config_remote_sync.go index 93d9bbb14c3..e5231fef58a 100644 --- a/cmd/bundle/config_remote_sync.go +++ b/cmd/bundle/config_remote_sync.go @@ -104,7 +104,7 @@ Examples: // Filter after planning, never before: the plan must cover every // resource so ${resources.*} references resolve; only the emitted // changes are restricted to the selected resources. - selected, err := configsync.ResolveResourceSelectors(&deployBundle.StateDB, selectIDs) + selected, err := configsync.ResolveResourceSelectors(ctx, &deployBundle.StateDB, selectIDs) if err != nil { return err }