CXH-2352: Name the inaccessible workspace and how to skip it on a 403 - #60
CXH-2352: Name the inaccessible workspace and how to skip it on a 403#60al-conductorone wants to merge 2 commits into
Conversation
| var apiErr *APIError | ||
| if errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusForbidden { | ||
| return fmt.Errorf( | ||
| "workspace %s is inaccessible (403); scope it out with --databricks-exclude-workspaces (BATON_DATABRICKS_EXCLUDE_WORKSPACES): %w", |
There was a problem hiding this comment.
🟡 Suggestion: databricks-exclude-workspaces is declared mutually exclusive with workspaces (pkg/config/config.go:93), and workspace-tokens requires workspaces (line 94). So for a workspace-token install — exactly the setup most likely to hit a per-workspace 403 — following this remedy produces a config-validation failure; the correct fix there is to drop the workspace from --databricks-workspaces. Consider mentioning both paths, e.g. "remove it from --databricks-workspaces, or scope it out with --databricks-exclude-workspaces".
Also note ListRoles/ListRuleSets are called from the group Grant/Revoke paths (pkg/connector/groups.go:397, :541), so a provisioning 403 will surface "scope it out of the sync" as its remedy, which reads as misleading advice for a grant failure.
There was a problem hiding this comment.
+1 on both points. exclude is a dead end under token auth (needs --workspaces, mutually exclusive with exclude) — should mention removing from --workspaces too. and yeah, same string on Grant/Revoke via ListRoles/ListRuleSets reads like sync advice for a provisioning failure.
Connector PR Review: CXH-2352: Name the inaccessible workspace and how to skip it on a 403Blocking Issues: 1 | Suggestions: 1 | Threads Resolved: 0 Review SummaryThe new commit reworks the 403 remedy string in Security IssuesNone found. Correctness Issues
Suggestions
Prompt for AI agents |
| // fix: excluding the workspace scopes it out of the sync. workspaceId is empty | ||
| // for account-scoped calls, where a 403 is not a per-workspace access problem, | ||
| // so those pass through untouched. | ||
| func nameWorkspace403Remedy(workspaceId string, err error) error { |
There was a problem hiding this comment.
heads up vs the customer/CXH-2109 shape: this still returns the 403, so sync dies — it just has a better message. the 400 path in workspaces.go logs + returns empty and continues. for Gemini (and the pebble "no full sync found" follow-on), workspace-scoped role Grants (roles.go ListUsers/Groups/SPs) need the same skip-on-403, not only this remedy wrapper.
| ratelimitData, err := c.Get(ctx, u, &res, vars...) | ||
| if err != nil { | ||
| return nil, 0, ratelimitData, err | ||
| return nil, 0, ratelimitData, nameWorkspace403Remedy(workspaceId, err) |
There was a problem hiding this comment.
this is the call behind the customer failure (failed to list service principals / Unauthorized access to Org). enriching here helps ops pick an exclude, but roleBuilder.Grants still propagates the error and aborts the whole sync — skip+empty at the Grants layer is what actually unblocks a full sync.
--databricks-exclude-workspaces is mutually exclusive with --databricks-workspaces, which workspace-token auth requires, so those installs cannot follow that advice. Add the 'remove it from --databricks-workspaces' path so the remedy is actionable for both auth modes (and both sync and grant/revoke 403s).
| var apiErr *APIError | ||
| if errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusForbidden { | ||
| return fmt.Errorf( | ||
| "workspace %s is inaccessible (403); remove it from --databricks-workspaces, or scope it out with --databricks-exclude-workspaces (BATON_DATABRICKS_EXCLUDE_WORKSPACES): %w", |
There was a problem hiding this comment.
🟠 Bug: --databricks-workspaces is not a real flag. The field is declared as "workspaces" in pkg/config/config.go:35, so the CLI flag is --workspaces and the env var is BATON_WORKSPACES (see README.md:175) — only the exclude field carries the databricks- prefix. An operator following this message gets unknown flag. Also worth noting: on workspace-token installs workspaces and workspace-tokens must stay the same length (ValidateConfig), so removing an entry means removing its paired token too.
| "workspace %s is inaccessible (403); remove it from --databricks-workspaces, or scope it out with --databricks-exclude-workspaces (BATON_DATABRICKS_EXCLUDE_WORKSPACES): %w", | |
| "workspace %s is inaccessible (403); remove it from --workspaces (BATON_WORKSPACES) along with its paired workspace token, or scope it out with --databricks-exclude-workspaces (BATON_DATABRICKS_EXCLUDE_WORKSPACES): %w", |
When a workspace can't be accessed (403), the sync now names that workspace and tells the operator to exclude it, instead of aborting with no guidance.