diff --git a/acceptance/bundle/config-remote-sync/select_basic/output.txt b/acceptance/bundle/config-remote-sync/select_basic/output.txt index 2909fc4e403..712ce3e3988 100644 --- a/acceptance/bundle/config-remote-sync/select_basic/output.txt +++ b/acceptance/bundle/config-remote-sync/select_basic/output.txt @@ -59,14 +59,14 @@ Resource: resources.jobs.job_two === 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 +Error: no deployed jobs resource with id no-such-id-123; deployed jobs ids in state: [JOB_ID], [JOB_ID] Exit code: 1 === An id that exists under a different type matches nothing and is rejected too >>> [CLI] bundle config-remote-sync --select-ids pipelines:[JOB_ONE_ID] -Error: no deployed pipelines resource with id [JOB_ONE_ID] +Error: no deployed pipelines resource with id [JOB_ONE_ID]; the deployment state contains no pipelines resources (deployed resources by type: jobs=2) Exit code: 1 diff --git a/acceptance/bundle/config-remote-sync/select_basic/test.toml b/acceptance/bundle/config-remote-sync/select_basic/test.toml index 1067069b9fa..42f2e63abeb 100644 --- a/acceptance/bundle/config-remote-sync/select_basic/test.toml +++ b/acceptance/bundle/config-remote-sync/select_basic/test.toml @@ -8,3 +8,10 @@ DATABRICKS_BUNDLE_ENABLE_EXPERIMENTAL_YAML_SYNC = "true" [EnvMatrix] DATABRICKS_BUNDLE_ENGINE = ["direct", "terraform"] + +# The two job ids are server-assigned, so their sort order in the +# "deployed jobs ids in state" list varies run to run. Collapse the pair to a +# single stable token so the expectation stays hermetic. +[[Repls]] +Old = 'deployed jobs ids in state: \[JOB_(ONE|TWO)_ID\], \[JOB_(ONE|TWO)_ID\]' +New = 'deployed jobs ids in state: [JOB_ID], [JOB_ID]' diff --git a/bundle/configsync/select.go b/bundle/configsync/select.go index 443c94423ae..8e35ce35b72 100644 --- a/bundle/configsync/select.go +++ b/bundle/configsync/select.go @@ -77,7 +77,7 @@ func ResolveResourceSelectors(ctx context.Context, state *dstate.DeploymentState // 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) + return nil, fmt.Errorf("no deployed %s resource with id %s; %s", resourceType, id, describeStateIDs(byTypeID, resourceType)) } // Some selectors matched: skip the stale ones so the matched resources still @@ -88,6 +88,66 @@ func ResolveResourceSelectors(ctx context.Context, state *dstate.DeploymentState return keys, nil } +// maxReportedIDs bounds how many ids describeStateIDs lists, so the message +// stays readable (and within the 500-char error_message telemetry limit) for +// bundles with many resources of one type. +const maxReportedIDs = 10 + +// describeStateIDs summarizes the ids present in the deployment state index so +// a failed selector lookup says what the state DID contain, not just what was +// missing. Without this, "no deployed jobs resource with id X" cannot be told +// apart from an empty state, a state for a different bundle, or a genuinely +// stale id — the three have very different fixes. +// +// byTypeID is keyed ":" as built by ResolveResourceSelectors, mapping +// to the plan key. Only resource types and ids are reported: ids are opaque +// workspace identifiers, while resource keys/names come from user configuration +// and are never included. +// +// Permissions and grants sub-resources are excluded. They are indexed under +// their parent's type with a path-shaped object id ("jobs:/jobs/123"), which is +// never selectable, would inflate the per-type counts, and would be redacted as +// a path when the error is recorded in telemetry. +func describeStateIDs(byTypeID map[string]string, resourceType string) string { + countsByType := make(map[string]int) + var sameTypeIDs []string + for typeAndID, resourceKey := range byTypeID { + if isPermissionsOrGrantsSubResource(resourceKey) { + continue + } + t, id, ok := strings.Cut(typeAndID, ":") + if !ok { + continue + } + countsByType[t]++ + if t == resourceType { + sameTypeIDs = append(sameTypeIDs, id) + } + } + + if len(countsByType) == 0 { + return "the deployment state contains no resources with ids (the bundle may not be deployed, or its resource state is missing)" + } + + if len(sameTypeIDs) == 0 { + types := make([]string, 0, len(countsByType)) + for t, n := range countsByType { + types = append(types, fmt.Sprintf("%s=%d", t, n)) + } + slices.Sort(types) + return fmt.Sprintf("the deployment state contains no %s resources (deployed resources by type: %s)", resourceType, strings.Join(types, ", ")) + } + + slices.Sort(sameTypeIDs) + listed := sameTypeIDs + suffix := "" + if len(listed) > maxReportedIDs { + listed = listed[:maxReportedIDs] + suffix = fmt.Sprintf(" (and %d more)", len(sameTypeIDs)-maxReportedIDs) + } + return fmt.Sprintf("deployed %s ids in state: %s%s", resourceType, strings.Join(listed, ", "), suffix) +} + // FilterChanges returns the subset of changes that belong to the resources in // selected, a list of plan keys ("resources..") as returned by // ResolveResourceSelectors. Change keys are plan keys too. diff --git a/bundle/configsync/select_test.go b/bundle/configsync/select_test.go index 7b6b45e3f38..8fb1c787050 100644 --- a/bundle/configsync/select_test.go +++ b/bundle/configsync/select_test.go @@ -1,6 +1,7 @@ package configsync import ( + "fmt" "maps" "slices" "testing" @@ -92,3 +93,90 @@ func TestFilterChanges(t *testing.T) { // The input map is never mutated. assert.Len(t, changes, 7) } + +func TestDescribeStateIDs(t *testing.T) { + tests := []struct { + name string + byTypeID map[string]string + resourceType string + want string + }{ + { + name: "empty state is called out explicitly", + byTypeID: map[string]string{}, + resourceType: "jobs", + want: "the deployment state contains no resources with ids (the bundle may not be deployed, or its resource state is missing)", + }, + { + // Permissions/grants are indexed under the parent's type with a + // path-shaped object id; reporting them would leak a path and + // inflate the counts. + name: "permissions and grants sub-resources are excluded", + byTypeID: map[string]string{ + "jobs:111": "resources.jobs.a", + "jobs:/jobs/111": "resources.jobs.a.permissions", + "schemas:s1": "resources.schemas.s", + "schemas:/schemas/s1": "resources.schemas.s.grants", + }, + resourceType: "jobs", + want: "deployed jobs ids in state: 111", + }, + { + name: "state holding only sub-resources reads as no resources with ids", + byTypeID: map[string]string{ + "jobs:/jobs/111": "resources.jobs.a.permissions", + }, + resourceType: "jobs", + want: "the deployment state contains no resources with ids (the bundle may not be deployed, or its resource state is missing)", + }, + { + // A resource literally named "permissions" is a real resource, not a + // sub-resource, so its id must still be reported. + name: "resource named permissions is still reported", + byTypeID: map[string]string{ + "jobs:333": "resources.jobs.permissions", + }, + resourceType: "jobs", + want: "deployed jobs ids in state: 333", + }, + { + name: "state holds other types only", + byTypeID: map[string]string{ + "pipelines:abc-123": "resources.pipelines.p", + "schemas:99": "resources.schemas.s", + "pipelines:def-456": "resources.pipelines.q", + }, + resourceType: "jobs", + want: "the deployment state contains no jobs resources (deployed resources by type: pipelines=2, schemas=1)", + }, + { + name: "ids of the requested type are listed sorted", + byTypeID: map[string]string{ + "jobs:222": "resources.jobs.b", + "jobs:111": "resources.jobs.a", + "schemas:7": "resources.schemas.s", + }, + resourceType: "jobs", + want: "deployed jobs ids in state: 111, 222", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, describeStateIDs(tt.byTypeID, tt.resourceType)) + }) + } +} + +func TestDescribeStateIDsTruncatesLongLists(t *testing.T) { + byTypeID := make(map[string]string) + for i := range maxReportedIDs + 3 { + // Ids are zero-padded so lexical sort order is also numeric order, + // keeping the assertion independent of id formatting. + id := fmt.Sprintf("%03d", i) + byTypeID["jobs:"+id] = "resources.jobs.j" + id + } + + got := describeStateIDs(byTypeID, "jobs") + assert.Equal(t, "deployed jobs ids in state: 000, 001, 002, 003, 004, 005, 006, 007, 008, 009 (and 3 more)", got) +}