Repository navigation
[CXH-2028] - Support workload identity federation - Databricks Connector - #84
mateoHernandez123 wants to merge 6 commits into
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
sergiocorral-conductorone
left a comment
sergiocorral-conductorone
left a comment
There was a problem hiding this comment.
Automated review (follow-up/first pass) — drafted 1 → confirmed 1, low-confidence 0, dropped 0, added 0 (panel: opus-5.5 / gpt-6-astra / gemini-4-argon, all high).
…mselves Co-authored-by: Cursor <cursoragent@cursor.com>
sergiocorral-conductorone
left a comment
sergiocorral-conductorone
left a comment
There was a problem hiding this comment.
Automated review
drafted 5 → confirmed 4, low-confidence 1, dropped 0, added 0 unique (panel: opus-5.5 / gpt-6-astra / gemini-4-argon, all high).
…they carry databricks-token reads as a Databricks personal access token: that is what DATABRICKS_TOKEN means in Databricks' own tooling, and what a bare token field means across the connectors. This credential is an externally issued JWT, which Databricks itself calls DATABRICKS_OIDC_TOKEN, so the flags are now databricks-oidc-token and databricks-oidc-token-file. Nothing ships with the old names yet, so no deployed configuration changes. The Kubernetes example also promised a token file that nothing created: the secret pointed the token file at /var/run/secrets/databricks/token while the Deployment below mounted no such volume, so the first exchange failed unless the operator supplied the mount themselves. It now carries the matching projected serviceAccountToken volume and mount, commented out like the secret line it pairs with. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| field.WithRequired(true), | ||
| field.WithDisplayName("OAuth2 Client Secret"), | ||
| ) | ||
| DatabricksOIDCTokenField = field.StringField( |
There was a problem hiding this comment.
Hosted C1 is part of the request, and these fields don't cover it. A hosted connector has no file to point databricks-oidc-token-file at. A JWT pasted into databricks-oidc-token fails the sync once it expires (the exchanged token's lifetime follows the JWT's exp, typically about an hour). The service-mode Hello check runs before that.
For hosted, C1 itself has to issue the JWT, which the customer then trusts in the service principal federation policy. C1 already has most of the plumbing for AWS:
- A per-tenant OIDC issuer at
<tenant>/auth/v1/aws-sts, with discovery and JWKS (pkg/api/auth/http_aws_sts_oidc.go), behindFEATURE_FLAG_ID_AWS_STS_OIDC_ISSUER. - Per-purpose RS256 signer keys with rotation (
ExternalTokenSignerPurpose_AWS_STS_V1,pkg/temporal/activity/keyrotate). Databricks accepts RS256. - Delivery is a one-shot action argument today (baton-aws
assume_role_with_web_identity). A sync needs a fresh JWT for every exchange.
Could we open a CXE Linear issue (or ask for a c1 investigation) to add a Databricks purpose? It would need:
- A new signer purpose with its own issuer route, so a Databricks policy only trusts Databricks-purpose tokens.
- Minting with a stable
suband a configurableaudthat the customer copies into the federation policy. I couldn't find the AWS minting code on c1 main, so it's worth confirming where it lives. - Token-source delivery to the hosted runtime, so the connector can ask for a fresh JWT on each exchange instead of getting it once.
GetConnectorOauthTokenis the closest existing pattern.
Connector side, so this PR is ready for that:
- Use field groups instead of the mutually-exclusive/at-least-one constraints:
client-secret(default): client id plus client secret, withWithRequiredback on the secret.workload-identity-file(CLI-only): client id plusdatabricks-oidc-token-file.workload-identity-c1(hosted): client id only. The JWT comes from C1, and the docs give the issuer URL, audience and subject for the policy.
- Turn the JWT lookup in
subjectToken()into a small source interface (file now, C1 token source later), sofederationTokenSource.Token()stays the same. NewConnectorcurrently ignorescli.ConnectorOpts. The C1 group would read the token source from there, the same way OAuth2 connectors consumeopts.TokenSource.- Drop
databricks-oidc-token. A static JWT doesn't stay valid in either mode.
There was a problem hiding this comment.
You are right. Both workload-identity fields defaulted to gui, and neither works there: there is no filesystem for the file, and a static JWT cannot be refreshed because Token() asks for a fresh one on every exchange.
Done in 80463512: field groups instead of the constraints, client-secret as the default with WithRequired back on the secret, workload-identity-file as a CLI-only group, the JWT lookup behind a SubjectTokenSource, and databricks-oidc-token removed.
I left out workload-identity-c1. Without the minting it would be a third hosted option that does not work, so it lands with CXE-1557, which I filed on Connector Engine and linked to CXE-718, IGA-5233, IGA-2165, CXH-2028, CXH-2220 and CXH-116. On the minting code you could not find: it is not there. ExternalTokenSignerPurpose_EXTERNAL_TOKEN_SIGNER_PURPOSE_AWS_STS_V1 appears only in key rotation.
Verified against the tenant, with a temporary federation policy carrying an inline JWKS: the federated sync matched the client-secret sync exactly, and an expired JWT, a JWT signed by an untrusted key and a missing file are each refused. The policy is deleted.
The exported config schema gave neither workload identity field an export target, so both reached the hosted web form, and neither works there. A hosted connector has no file system the tenant admin controls, and a pasted JWT is never refreshed: Token() asks for a fresh subject token on every exchange, so a static string returns the same expired JWT once its hour is up. The Hello check runs while that JWT is still valid, so the configuration validates and the sync breaks later. Field groups replace the mutually exclusive and at-least-one constraints. The client secret is the default and is required again, which the constraints had forced off. The token file is its own CLI-only group, so hosted no longer offers it. The static token field is removed: it works in neither mode. Validation skips any field the selected group does not list, so each group repeats the common fields; account-id is required and would otherwise stop being checked. Selecting the file method on the command line needs --auth-method, which the README now shows. The JWT lookup moves behind SubjectTokenSource, with the file as its only implementation. A source that reads from ConductorOne lands when CXE-1557 adds the minting, and Token() will not change. Verified against the tenant with a temporary federation policy carrying an inline JWKS: a locally signed JWT exchanged for a Databricks token, and the sync through it matched the client-secret sync exactly (322 resources, 279 entitlements, 789 grants). An expired JWT, a JWT signed by an untrusted key and a missing file are each refused, and restoring the file syncs again, which is the rotation the source exists for. The policy was deleted afterwards. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Superseded — see the current review report for commit
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Superseded — see the current review report for commit
|
There was a problem hiding this comment.
No blocking issues found — see the full review report
…edential prepareClientAuth switched to federation whenever a token file was set, so a file left over from another deployment replaced the credential the operator had selected, and validation did not catch it: the file is not in the client secret group, so that group never checks it. The method is the explicit choice, so the branch reads it. The group names move to constants that both the group definitions and the branch share. The Kubernetes example set BATON_DATABRICKS_OIDC_TOKEN_FILE without BATON_AUTH_METHOD, which validates against the client secret group and fails on the missing secret, so copying it produced a deployment that could not start. Both variables are now shown together. docs-info.md still described the JWT as a value or a file path, and two lines still said exactly one of two credentials is supplied. The static value is gone and the method selects the credential. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Superseded — see the current review report for commit
|
Connector PR Review: [CXH-2028] - Support workload identity federation - Databricks ConnectorBlocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0 Review SummaryThe new commit makes the selected auth method choose the credential. I scanned the full PR diff for security and correctness, including
The repo-local criteria loaded. I applied E1/E2 (error wrapping and codes), H1–H5 (HTTP safety), L-series logging and G1–G3 (config) to the auth and config changes. Section S, provisioning (P/PR) and B10 (API spec drift) do not apply because no sync, provisioning or API endpoint code changed. No resource types or entitlements change, so this is not a breaking change (B1–B9). The PR description now matches the diff: two methods, field groups, and the client secret required inside its own group. The incremental diff was complete, with no dropped paths and no truncation. Security IssuesNone found. Correctness IssuesNone found. SuggestionsNone. Resolved prior findings
Reviewed commit: |
There was a problem hiding this comment.
No blocking issues found — see the full review report
Description
The connector can now authenticate a service principal with workload identity federation. The client secret stays the default. Existing deployments keep working. A saved config that sets the client secret still passes validation.
This pull request replaces #49. That pull request is stale and conflicting, and it is closed in favor of this one.
What changes for the operator
The resource types and the provisioning surface do not move. What changes is how the connector gets its token.
Before. One credential, and it is required.
After. The client id stays required. The auth method selects the credential, and the new method reads a JWT the connector exchanges.
flowchart LR id["--databricks-client-id (required)"] pick{"--auth-method"} sec["client-secret (default)<br/>--databricks-client-secret"] file["workload-identity-file (CLI only)<br/>--databricks-oidc-token-file"] id --> pick pick --> sec pick --> file sec --> cc["OAuth client credentials"] file --> ex["RFC 8693 token exchange"] cc --> ep["account token endpoint"] ex --> ep ep --> bearer["Bearer token"] style file fill:#1f6feb,color:#fff,stroke:#1f6feb style ex fill:#1f6feb,color:#fff,stroke:#1f6febThe blue path is new.
--databricks-client-secretkeeps the behaviour it has today, so a saved config that sets it still validates and still syncs.The connector reads the JWT from a file on every token refresh, so whatever rotates that file controls the lifetime and no restart is needed.
--databricks-client-idis still required. The credential is chosen by auth method rather than by constraints, so each method keeps its own required field.Sync:
user) — unchangedgroup) — unchangedrole) — unchangedservice_principal) — unchangedworkspace) — unchangedaccount) — unchangedProvisioning:
Auth:
The connector still authenticates as the service principal named by
--databricks-client-id. That flag stays required.Two authentication methods, selected as field groups:
client-secret(default):--databricks-client-secret. The connector uses the OAuth client-credentials flow.workload-identity-file(command line and service mode only):--databricks-oidc-token-file. The connector reads this file on every token refresh.On the command line, select the second with
--auth-method workload-identity-file. Without it the connector validates against the default method and asks for a client secret.A ConductorOne-hosted connector has no file system the tenant admin controls, so the web form offers the client secret only. Hosted workload identity needs ConductorOne to issue the JWT, which is tracked in CXE-1557.
The exchange is RFC 8693. The grant type is
urn:ietf:params:oauth:grant-type:token-exchange. The subject token type isurn:ietf:params:oauth:token-type:jwt. The scope isall-apis. The URL is the same account token endpoint the client secret already uses.Databricks must have a service principal federation policy that trusts the JWT issuer, subject, and audience. The connector does not create that policy.
The client secret is required inside its own group. Validation only checks the fields the selected group lists, so the secret is not asked for when the token file method is chosen, and the other way round.
Architecture highlights:
clientcredentials.Configcan overridegrant_type, but its public API caches the token before the connector can set an expiry. A missingexpires_inwould pin the first token for the life of the process.expires_inuses the access tokenexpclaim, then a five-minute fallback.uhttpclassifies the HTTP status. An HTTP 200 with no access token returnsFailedPrecondition.Validation:
baton-uc-cxh2341, then a full sync with no client secret, exit 0. The policy and the test key were deleted after the run.Useful links:
Merge order and release flow
The team does not merge to
stagingany more. A PR targetsmain, and the test release comes from the feature branch:main. Reviewers read it there.-test) from the feature branch and move the PR to draft. The ticket moves to validation.main.main. The ticket moves to rollout.This PR targets
stagingtoday, and it can move tomainat any time. The branch leftmainatc0a09af4, which is also where it leftstaging, and it carries nostaging-only commit. The diff therefore does not move: 4 commits and 10 files against either base, and it merges tomainwith no conflict.That is not true of the Unity Catalog stack (#71, #72, #73) or of #90.
stagingis 42 commits ahead ofmain, and those branches would swallow everystaging-only commit on a retarget. They wait untilstaginglands inmain. This PR does not depend on them, and they do not depend on it.