nhi: sync SP OAuth secrets + workspace PATs as STATIC_SECRET resources (Phase-1 class-B)#48
Conversation
…s (Phase-1 class-B)
Adds two new TRAIT_SECRET resource types to baton-databricks (pqprime NHI
program, bd pqprime-16u):
A1 — service_principal_secret (SP OAuth client secrets)
- New client method ListServicePrincipalSecrets: account-level API
GET /api/2.0/accounts/{acct}/servicePrincipals/{sp_id}/credentials/secrets
with cursor-based page_token/next_page_token pagination.
- Builder fans out per synced SP (parent = service_principal resource).
- STATIC_SECRET, detail "databricks.sp_oauth_secret", created_at from
create_time, identity_id back-ref to the parent SP resource (K3
APP_REGISTRATION already emitted by the SP builder).
- Secret value is never returned by the API; metadata only.
A2 — workspace_pat (workspace personal access tokens)
- New client method ListTokenManagementTokens: workspace-level admin endpoint
GET /api/2.0/token-management/tokens; fans out per synced workspace.
- Builder lists all workspace PATs (parent = workspace resource).
- STATIC_SECRET, detail "databricks.pat", created_at from creation_time,
expires_at from expiry_time (skipped when ≤0), identity_id from
created_by_id → user resource.
- Graceful degradation: 401/403 responses are logged and the workspace is
skipped; the rest of the sync continues.
Also adds:
- PageTokenVars to pkg/databricks/vars.go for cursor-based pagination.
- 13 unit tests covering basic listing, pagination, 401/403 graceful degrade,
wrong/nil parent types, timestamp parsing, and no-entitlements invariant.
- Both resource types registered in ResourceSyncers.
Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
| newServicePrincipalSecretBuilder(d.client), | ||
| newTokenBuilder(d.client), |
There was a problem hiding this comment.
🔴 Bug: These builders expect fan-out over parent resources (servicePrincipalResourceType and workspaceResourceType respectively), but neither parent resource declares the new types as children via ChildResourceType annotations. Without those annotations, the SDK will never call these builders' List methods with a parent resource ID — so no SP secrets or workspace PATs will be synced.
Fix: Add &v2.ChildResourceType{ResourceTypeId: servicePrincipalSecretResourceType.Id} to the SP resource in service-principals.go:servicePrincipalResource, and &v2.ChildResourceType{ResourceTypeId: workspacePATResourceType.Id} to the workspace resource in workspaces.go:workspaceResource.
| secrets, nextToken, _, err := b.client.ListServicePrincipalSecrets(ctx, spID, pageToken) | ||
| if err != nil { | ||
| return nil, nil, fmt.Errorf("databricks-connector: failed to list SP secrets for %s: %w", spID, err) | ||
| } | ||
|
|
||
| l.Debug("listed SP OAuth secrets", zap.String("sp_id", spID), zap.Int("count", len(secrets))) | ||
|
|
||
| for i := range secrets { | ||
| r, err := b.spSecretResource(&secrets[i], parentResourceID) | ||
| if err != nil { | ||
| return nil, nil, err | ||
| } | ||
| rv = append(rv, r) | ||
| } | ||
|
|
||
| if nextToken == "" { | ||
| break | ||
| } | ||
| pageToken = nextToken | ||
| } |
There was a problem hiding this comment.
🟡 Suggestion: This loop fetches all pages in-connector before returning, which buffers all secrets in memory. Per F2/R8, the SDK should drive pagination one page at a time for checkpointing and cancellation. In practice SP secret counts are tiny so this is unlikely to cause real problems, but consider using SDK pagination bags (like servicePrincipalBuilder.List does) to stay consistent with the codebase pattern and let the SDK checkpoint between pages.
Connector PR Review: nhi: sync SP OAuth secrets + workspace PATs as STATIC_SECRET resources (Phase-1 class-B)Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0 Review SummaryThe new commits add documentation updates: README.md now lists the two new resource types, and Security IssuesNone found. Correctness IssuesNone found. SuggestionsNone. |
servicePrincipalSecretBuilder.List was silently unrolling all pages for a given SP inside a single sync-engine call. Use opts.PageToken.Token as the API cursor and return SyncOpResults.NextPageToken so the sync engine drives pagination one page per call — bounded memory, checkpointable. Tests updated to exercise the two-call round-trip instead of the internal-loop behaviour. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
…otations - Wrap json.NewEncoder(w).Encode calls in require.NoError(t, ...) in sp_secrets_test.go and tokens_test.go to satisfy errcheck linter. - Add //nolint:gosec to workspaceTokensEndpoint URL constant (G101 false positive — path string, not a credential). - Add ChildResourceType annotations: SP resource declares servicePrincipalSecretResourceType as a child so the SDK fans out List calls per SP; workspace resource adds workspacePATResourceType alongside the existing roleResourceType child. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
gosec v2.11.4 (used by CI) flags the hardcoded "cursor-page-2" string as a potential credential when it appears as the value of a key whose name contains "token". Rename it to paginatorCursor so the variable name carries no sensitive semantics. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
…s table The PR introduced two new sync-only resource types (service_principal_secret and workspace_pat) but the docs/connector.mdx capabilities table and README data model list were not updated. Add rows for both types with sync enabled and provision disabled. Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
🔗 Preview: C1
Summary
Adds two new
TRAIT_SECRETresource types to baton-databricks. This is Phase-1 of the pqprime NHI program (bd pqprime-16u), closing the class-B credential gap: the connector previously synced zero secret-shaped resources.A1 —
service_principal_secret(SP OAuth client secrets)GET /api/2.0/accounts/{account_id}/servicePrincipals/{service_principal_id}/credentials/secretsRef: Databricks Service Principal Secrets API
page_token/next_page_tokenpagination.STATIC_SECRET, detaildatabricks.sp_oauth_secret,created_atfromcreate_time.identity_idback-ref → the parent SP resource (K3APP_REGISTRATIONalready emitted).secret_hash).expires_at/last_usedabsent fromSecretInfo→ omitted per spec.A2 —
workspace_pat(workspace personal access tokens)GET /api/2.0/token-management/tokens(workspace-admin endpoint)Ref: Databricks Token Management API
STATIC_SECRET, detaildatabricks.pat,created_atfromcreation_time,expires_atfromexpiry_time(skipped when ≤0 / "never expires").identity_idback-ref → owning user resource viacreated_by_id.Implementation notes
PageTokenVarsadded topkg/databricks/vars.gofor cursor pagination.V1Identifierset anywhere (per precedent baton-temporalcloud#82 r3380796810).Test plan
go test ./pkg/connector/...— 13 new tests, all passgo build ./...— cleango vet ./pkg/...— cleanBuilt with pqprime · bd pqprime-16u · spec
Exposed endpoints
🏰 Squire environment: sunny-dragon-90701
Task: 1870ba14-58ea-4835-8fb5-affd78ad616e