From f85c19635f03c7179a87ea7e19c5ac46fa5b1d4f Mon Sep 17 00:00:00 2001 From: Luisina Santos Date: Tue, 15 Sep 2026 12:13:16 -0300 Subject: [PATCH 1/6] CXH-2166: revert workspace-token (PAT) authentication Reverts the PAT auth path added in #54, removing the implementation rather than leaving it dormant, and updates the docs to describe what the connector actually does. PAT auth was already unreachable: #63 commented out the workspace-token field group and rejected the auth method in ValidateConfig, leaving the implementation in place behind docs that described it as temporarily unavailable and expected to return. OAuth is now the connector's only authentication method, and the docs say so without qualification. Removed: - TokenAuth/NewTokenAuth and Client.IsTokenAuth - workspace-tokens field, the FieldsDependentOn constraint, and ValidateConfig (the workspaces/exclude-workspaces exclusion is a schema constraint) - the field groups entirely, along with both group constants: groups exist to let the setup UI offer a choice of auth method, and with OAuth as the only method there is no choice to present. This restores the pre-#54 shape, which had no groups. The three OAuth fields are required at the field level, so required-ness is unchanged. - the token branch in prepareClientAuth; Validate's account-API fallback branches and account-unreachable warning, now dead because isAccAPIAvailable is unconditionally true - isGroupNotFoundError and groupGrantParent, reachable only via workspace-parented groups, which existed only under token auth Kept: --workspaces (scopes OAuth syncs), --databricks-exclude-workspaces, and the grant-parenting fixes in account.go and service-principals.go, which are correct under OAuth. roles.go inlines groupGrantParent's account branch. Docs (README, connector.mdx, docs-info.md) drop the PAT method and its "temporarily unavailable" framing; the README CLI dump is regenerated from the built binary. conf.gen.go and config_schema.json regenerated. Breaking change for self-hosted deployments still configured with workspace tokens: they must move to OAuth2 or pin a previous version. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yaml | 1 - README.md | 65 +++++++------------ config_schema.json | 42 +----------- docs/connector.mdx | 32 +++------- docs/docs-info.md | 14 ++-- pkg/config/conf.gen.go | 1 - pkg/config/config.go | 113 --------------------------------- pkg/config/config_test.go | 107 ------------------------------- pkg/connector/connector.go | 73 ++++++--------------- pkg/connector/groups.go | 16 +---- pkg/connector/helpers.go | 27 -------- pkg/connector/helpers_test.go | 99 ----------------------------- pkg/connector/roles.go | 2 +- pkg/connector/validate_test.go | 70 -------------------- pkg/connector/workspaces.go | 46 -------------- pkg/databricks/auth.go | 47 -------------- pkg/databricks/auth_test.go | 87 ------------------------- pkg/databricks/client.go | 5 -- 18 files changed, 62 insertions(+), 785 deletions(-) delete mode 100644 pkg/config/config_test.go delete mode 100644 pkg/connector/helpers_test.go delete mode 100644 pkg/databricks/auth_test.go diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 6030d7f1..f940cbcc 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -15,7 +15,6 @@ jobs: BATON_DATABRICKS_CLIENT_SECRET: ${{ secrets.DATABRICKS_CLIENT_SECRET }} BATON_ACCOUNT_ID: ${{ secrets.BATON_ACCOUNT_ID }} # BATON_WORKSPACES: ${{ secrets.BATON_WORKSPACES }} - # BATON_WORKSPACE_TOKENS: ${{ secrets.BATON_WORKSPACE_TOKENS }} steps: - name: Checkout code uses: actions/checkout@v4 diff --git a/README.md b/README.md index afb8a2d5..2393a52d 100644 --- a/README.md +++ b/README.md @@ -17,26 +17,18 @@ of an account, after you log into account platform and click on your username in right top corner that will open a dropdown menu with the account ID along other options. -Another requirement is to have valid credentials to run the connector with. This -will decide how connector will be executed. You can use either the OAuth client -credentials flow or the Bearer auth flow. OAuth can be used across account and -all workspaces you have access to. Bearer auth can be used only for a specific -workspace. - -To use the OAuth, you need to create a service principal and add OAuth secret -(client id and secret) to it. You can do that by going to the user management -tab and clicking on the Service Principals tab. Then click on the Add Service -principal button and name it. You then need to add OAuth secret to it by -clicking on the Generate secret button. You can use this secret to authenticate -across all workspaces that service principal has access to. This requires admin -access to the Databricks account and each workspace you want to sync. - -To use bearer auth, you need to provide a Databricks workspace access token. You -can create a new token by logging into the workspace and going into user -settings. Then go to Developer tab and create a new access token. This will try -to work with only specified workspaces and their respective tokens. You can -provide multiple tokens by separating them with a comma. This method requires -admin access to each workspace you want to sync. +Another requirement is to have valid credentials to run the connector with. The +connector authenticates with the OAuth client credentials flow, using an +account-level service principal that works across the account and every +workspace it has access to. + +To set this up, create a service principal and add an OAuth secret (client ID +and secret) to it. You can do that by going to the user management tab and +clicking on the Service Principals tab. Then click on the Add Service principal +button and name it. You then need to add an OAuth secret to it by clicking on +the Generate secret button. You can use this secret to authenticate across all +workspaces that service principal has access to. This requires admin access to +the Databricks account and each workspace you want to sync. # Using Azure Databricks @@ -85,29 +77,19 @@ baton resources - Users - Roles -By default (OAuth), the connector fetches all resources from the account and all +By default, the connector fetches all resources from the account and all workspaces. To limit the scope, pass a comma-separated list of workspace deployment names to the `--workspaces` flag. ## Authentication -OAuth is the only authentication method currently available: an account-level -service principal's client ID and secret. - -> **Workspace-token (PAT) authentication is temporarily unavailable.** -> `--auth-method workspace-token` is not offered, and a config specifying it is -> rejected at startup with a clear error. `--workspace-tokens` still appears in -> `--help` but no authentication method consumes it. `--workspaces` remains -> supported under OAuth for limiting the sync scope, as described above. -> -> The implementation is intact and commented out in `pkg/config/config.go` -> rather than deleted; it is withheld while a platform-side defect is resolved. -> The defect is not in this connector — a credential declared as a list of -> secrets is not treated as secret by the configuration layer, so a workspace -> token supplied through the UI is stored unencrypted and shown in clear text. +OAuth is the only authentication method: an account-level service principal's +client ID and secret, supplied through `--databricks-client-id` and +`--databricks-client-secret`. There is no personal access token (PAT) or +workspace-token option, and no username/password option. OAuth requires a reachable account API. If the account API check fails at -startup, the connector fails validation instead of falling back to a +startup, the connector fails validation rather than falling back to a workspace-only sync, even when `--workspaces` is set. To instead exclude specific workspaces from the sync, pass them to the @@ -118,9 +100,7 @@ ID. Excluded workspaces and their roles are skipped entirely. ## Group provisioning -Account groups are provisioned through the OAuth client ID and secret flow. The -Databricks API does not allow provisioning account groups from a workspace -token, which is one reason the OAuth flow is the supported path. +Account groups are provisioned through the OAuth client ID and secret flow. [Here](https://docs.databricks.com/aws/en/admin/users-groups/groups#:~:text=Types%20of%20groups%20in%20Databricks,permissions%20to%20identity%20federated%20workspaces.) are the different types of groups in Databricks. @@ -157,7 +137,7 @@ Flags: --client-secret string The client secret used to authenticate with ConductorOne ($BATON_CLIENT_SECRET) --databricks-client-id string required: The Databricks service principal's client ID used to connect to the Databricks Account and Workspace API ($BATON_DATABRICKS_CLIENT_ID) --databricks-client-secret string required: The Databricks service principal's client secret used to connect to the Databricks Account and Workspace API ($BATON_DATABRICKS_CLIENT_SECRET) - --databricks-exclude-workspaces strings Workspaces to exclude from sync, identified by workspace name, deployment name, or numeric workspace ID ($BATON_DATABRICKS_EXCLUDE_WORKSPACES) + --databricks-exclude-workspaces strings Workspaces to exclude from sync, identified by workspace name, deployment name, or numeric workspace ID. Mutually exclusive with workspaces. ($BATON_DATABRICKS_EXCLUDE_WORKSPACES) --external-resource-c1z string The path to the c1z file to sync external baton resources with ($BATON_EXTERNAL_RESOURCE_C1Z) --external-resource-entitlement-id-filter string The entitlement that external users, groups must have access to sync external baton resources ($BATON_EXTERNAL_RESOURCE_ENTITLEMENT_ID_FILTER) --external-resource-traits strings Resource type traits (e.g. "user", "group", "app") to sync and match from the external resource c1z. When unset the matcher falls back to user and group; passing this flag replaces the full set rather than adding to it. ($BATON_EXTERNAL_RESOURCE_TRAITS) @@ -177,15 +157,14 @@ Flags: -p, --provisioning This must be set in order for provisioning actions to be enabled ($BATON_PROVISIONING) --skip-entitlements-and-grants This must be set to skip syncing of entitlements and grants ($BATON_SKIP_ENTITLEMENTS_AND_GRANTS) --skip-full-sync This must be set to skip a full sync ($BATON_SKIP_FULL_SYNC) - --storage-engine string The storage engine to use when opening the sync c1z file: sqlite or pebble. Leave unset to use the baton-sdk default. ($BATON_STORAGE_ENGINE) + --storage-engine string The storage engine to use when opening the sync c1z file: sqlite or pebble. Defaults to pebble when unset. ($BATON_STORAGE_ENGINE) --sync-resource-types strings The resource type IDs to sync ($BATON_SYNC_RESOURCE_TYPES) --sync-resources strings The resource IDs to sync ($BATON_SYNC_RESOURCES) --task-concurrency int The number of Baton tasks to run concurrently in service mode. Tasks may include sync, grant, revoke, and more. Minimum value is 1, maximum value is 100. ($BATON_TASK_CONCURRENCY) (default 3) --ticketing This must be set to enable ticketing support ($BATON_TICKETING) -v, --version version for baton-databricks --workers int The number of sync workers to use. -1 for auto-detect, 0 for sequential, >0 for parallel ($BATON_WORKERS) - --workspace-tokens strings required: The Databricks personal access tokens scoped to specific workspaces used to connect to the Databricks Workspace API ($BATON_WORKSPACE_TOKENS) - --workspaces strings Limit syncing to the specified workspaces, by deployment name, not workspace ID. Required when using workspace tokens, in the same order as workspace-tokens. ($BATON_WORKSPACES) + --workspaces strings Limit syncing to the specified workspaces, by deployment name, not workspace ID. Mutually exclusive with databricks-exclude-workspaces. ($BATON_WORKSPACES) Use "baton-databricks [command] --help" for more information about a command. ``` diff --git a/config_schema.json b/config_schema.json index 2e162e4f..858a6254 100644 --- a/config_schema.json +++ b/config_schema.json @@ -143,21 +143,9 @@ { "name": "workspaces", "displayName": "Workspaces", - "description": "Limit syncing to the specified workspaces, by deployment name, not workspace ID. Required when using workspace tokens, in the same order as workspace-tokens. Mutually exclusive with databricks-exclude-workspaces.", + "description": "Limit syncing to the specified workspaces, by deployment name, not workspace ID. Mutually exclusive with databricks-exclude-workspaces.", "stringSliceField": {} }, - { - "name": "workspace-tokens", - "displayName": "Workspace Tokens", - "description": "The Databricks personal access tokens scoped to specific workspaces used to connect to the Databricks Workspace API", - "isRequired": true, - "isSecret": true, - "stringSliceField": { - "rules": { - "isRequired": true - } - } - }, { "name": "databricks-exclude-workspaces", "displayName": "Exclude Workspaces", @@ -172,35 +160,9 @@ "workspaces", "databricks-exclude-workspaces" ] - }, - { - "kind": "CONSTRAINT_KIND_DEPENDENT_ON", - "fieldNames": [ - "workspace-tokens" - ], - "secondaryFieldNames": [ - "workspaces" - ] } ], "displayName": "Databricks", "helpUrl": "/docs/baton/databricks", - "iconUrl": "/static/app-icons/databricks.svg", - "fieldGroups": [ - { - "name": "oauth2", - "displayName": "OAuth2", - "helpText": "Authenticate as a service principal using an OAuth2 client ID and secret.", - "fields": [ - "account-id", - "databricks-client-id", - "databricks-client-secret", - "hostname", - "account-hostname", - "workspaces", - "databricks-exclude-workspaces" - ], - "default": true - } - ] + "iconUrl": "/static/app-icons/databricks.svg" } \ No newline at end of file diff --git a/docs/connector.mdx b/docs/connector.mdx index ba8ddee1..8ca00b84 100644 --- a/docs/connector.mdx +++ b/docs/connector.mdx @@ -23,13 +23,7 @@ The Databricks connector supports [automatic account provisioning and deprovisio ## Authentication methods -The connector authenticates with **OAuth** — an account-level service principal's client ID and secret. This is the only method currently offered. - - -**Workspace-token (personal access token) authentication is temporarily unavailable.** It is not offered when configuring the connector, and configurations that specify it are rejected at startup. Use OAuth instead. - -The connector's PAT implementation is intact and the method is expected to return; it is withheld while a platform-side defect is resolved. The defect is not in this connector: a credential declared as a list of secrets is not treated as secret by the configuration layer, so a workspace token supplied through the UI would be stored unencrypted and displayed in clear text. - +The connector authenticates with **OAuth** — an account-level service principal's client ID and secret. This is the only authentication method the connector supports. ## Gather Databricks credentials @@ -55,18 +49,16 @@ A user with the **Account admin** role in each Databricks workspace you want to ### Generate Databricks credentials -The Databricks connector authenticates with OAuth: +The Databricks connector authenticates with OAuth, which syncs info from all Databricks workspaces the service principal can access. -- **OAuth** (syncs info from all Databricks workspaces) - - - - Follow the [Databricks OAuth authentication documentation](https://docs.databricks.com/en/dev-tools/auth/oauth-m2m.html) to create a service principal and create an OAuth secret. - - - Carefully copy and save the OAuth client ID and secret. - - + + + Follow the [Databricks OAuth authentication documentation](https://docs.databricks.com/en/dev-tools/auth/oauth-m2m.html) to create a service principal and create an OAuth secret. + + + Carefully copy and save the OAuth client ID and secret. + + **Done.** Here's the set of credentials you'll need when setting up the connector: @@ -74,10 +66,6 @@ The Databricks connector authenticates with OAuth: - OAuth client ID - OAuth client secret - -Personal access token (workspace token) authentication is **temporarily unavailable** and is not offered when configuring the connector, so there is no need to generate one. See [Authentication methods](#authentication-methods) above. - - Next, move on to the instructions for your chosen setup method. ## Configure the Databricks connector diff --git a/docs/docs-info.md b/docs/docs-info.md index 307a7dd0..fcebe05c 100644 --- a/docs/docs-info.md +++ b/docs/docs-info.md @@ -12,17 +12,14 @@ While developing the connector, please fill out this form. This information is n > > - **User accounts**: create and delete account users. > - **Entitlements**: grant and revoke role and membership assignments on accounts, workspaces, groups, service principals, and roles. - > - > Provisioning of account groups is only available with OAuth. A workspace token cannot provision groups, because the Databricks API does not allow it from a workspace token. ## Connector credentials 1. What credentials or information are needed to set up the connector? (For example, API key, client ID and secret, domain, etc.) - > The connector requires a Databricks account ID plus one of two authentication methods: + > The connector requires a Databricks account ID and OAuth credentials: > - > - **OAuth (recommended)**: a service principal OAuth client ID and client secret. Syncs the account and every workspace the service principal can access. - > - **Workspace token (PAT)**: one or more workspace personal access tokens paired positionally with the deployment names of the workspaces they authenticate. Scoped to the listed workspaces only. + > - **OAuth**: a service principal OAuth client ID and client secret. Syncs the account and every workspace the service principal can access. This is the connector's only authentication method. > > Google Cloud Platform and Azure Databricks customers also provide the account hostname and hostname. @@ -32,17 +29,16 @@ While developing the connector, please fill out this form. This information is n > - **Account ID**: in the Databricks account console, open the menu next to your username in the upper-right corner; the account ID is shown there. > - **OAuth client ID and secret**: follow the [Databricks OAuth (M2M) documentation](https://docs.databricks.com/en/dev-tools/auth/oauth-m2m.html) to create a service principal and generate an OAuth secret. - > - **Workspace token**: in the workspace, go to **Settings** > **Developer** > **Access tokens**, click **Manage**, then **Generate new token**. > - **Deployment name**: the subdomain in the workspace URL (not the numeric workspace ID). * Does the credential need any specific scopes or permissions? If so, list them here. - > The credential must have admin access to each resource it reads or writes: account-admin on the Databricks account (for account-level sync and provisioning) and workspace-admin on each workspace being synced. Workspace-token auth only reaches the workspaces its tokens are scoped to. + > The credential must have admin access to each resource it reads or writes: account-admin on the Databricks account (for account-level sync and provisioning) and workspace-admin on each workspace being synced. * If applicable: Is the list of scopes or permissions different to sync (read) versus provision (read-write)? If so, list the difference here. - > No separate scopes: Databricks admin access covers both read (sync) and read-write (provision). The practical difference is coverage by auth method: OAuth reaches the account plane and all accessible workspaces; a workspace token reaches only its scoped workspaces and cannot read or write account-level entitlements, grants, or groups. + > No separate scopes: Databricks admin access covers both read (sync) and read-write (provision). * What level of access or permissions does the user need in order to create the credentials? (For example, must be a super administrator, must have access to the admin console, etc.) - > Account admin access to the Databricks account console (to create the service principal and OAuth secret), and workspace admin on each workspace (to mint workspace tokens). + > Account admin access to the Databricks account console (to create the service principal and OAuth secret), and workspace admin on each workspace being synced. diff --git a/pkg/config/conf.gen.go b/pkg/config/conf.gen.go index 6ca5facf..dd2a3bba 100644 --- a/pkg/config/conf.gen.go +++ b/pkg/config/conf.gen.go @@ -10,7 +10,6 @@ type Databricks struct { DatabricksClientSecret string `mapstructure:"databricks-client-secret"` Hostname string `mapstructure:"hostname"` Workspaces []string `mapstructure:"workspaces"` - WorkspaceTokens []string `mapstructure:"workspace-tokens"` BaseUrl string `mapstructure:"base-url"` DatabricksExcludeWorkspaces []string `mapstructure:"databricks-exclude-workspaces"` } diff --git a/pkg/config/config.go b/pkg/config/config.go index d6a24c85..8320eb8a 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -1,17 +1,9 @@ package config import ( - "context" - "fmt" - "github.com/conductorone/baton-sdk/pkg/field" ) -const ( - DatabricksOAuth2Group = "oauth2" - DatabricksWorkspaceTokenGroup = "workspace-token" -) - var ( AccountIdField = field.StringField( "account-id", @@ -36,18 +28,10 @@ var ( "workspaces", field.WithDescription( "Limit syncing to the specified workspaces, by deployment name, not workspace ID. "+ - "Required when using workspace tokens, in the same order as workspace-tokens. "+ "Mutually exclusive with databricks-exclude-workspaces.", ), field.WithDisplayName("Workspaces"), ) - WorkspaceTokensField = field.StringSliceField( - "workspace-tokens", - field.WithDescription("The Databricks personal access tokens scoped to specific workspaces used to connect to the Databricks Workspace API"), - field.WithIsSecret(true), - field.WithRequired(true), - field.WithDisplayName("Workspace Tokens"), - ) AccountHostnameField = field.StringField( "account-hostname", field.WithDescription("The hostname used to connect to the Databricks account API. If not set, it will be calculated from the hostname field."), @@ -77,7 +61,6 @@ var ( DatabricksClientSecretField, HostnameField, WorkspacesField, - WorkspaceTokensField, BaseURLField, ExcludeWorkspacesField, } @@ -91,101 +74,5 @@ var Config = field.NewConfiguration( field.WithIconUrl("/static/app-icons/databricks.svg"), field.WithConstraints( field.FieldsMutuallyExclusive(WorkspacesField, ExcludeWorkspacesField), - field.FieldsDependentOn([]field.SchemaField{WorkspaceTokensField}, []field.SchemaField{WorkspacesField}), ), - field.WithFieldGroups([]field.SchemaFieldGroup{ - { - Name: DatabricksOAuth2Group, - DisplayName: "OAuth2", - HelpText: "Authenticate as a service principal using an OAuth2 client ID and secret.", - Fields: []field.SchemaField{ - AccountIdField, DatabricksClientIdField, DatabricksClientSecretField, - HostnameField, AccountHostnameField, WorkspacesField, ExcludeWorkspacesField, - }, - Default: true, - }, - // TEMPORARILY DISABLED — workspace-token auth is not offerable through the C1 UI. - // - // Two defects make the hosted path unusable, neither of them in this connector's - // auth implementation: - // - // 1. workspace-tokens is declared isSecret, but c1 has no secret bit for - // string-list fields, so the PAT is stored unencrypted and rendered in clear - // text. StringField secrets (databricks-client-secret) mask correctly on the - // same form; StringSliceField secrets do not. Tracked as CXE-1374. - // 2. workspace-tokens is DependentOn workspaces, and workspaces renders as - // "(optional)", so selecting this group shows no token input at all until the - // user happens to commit a value in an optional field. There is no affordance - // telling them to. - // - // SCOPE — this disables workspace-token auth EVERYWHERE, not only in the C1 UI. - // - // Commenting out the group is necessary but NOT sufficient, and the gap is easy to - // miss: FieldGroupFields falls back to the default group for an unrecognised auth - // method, so a config that satisfies oauth2's required fields AND sets - // --auth-method workspace-token passes field.Validate (workspace-tokens is skipped - // because it is no longer in the selected group, so neither WithRequired nor - // DependentOn fires) and prepareClientAuth still returns NewTokenAuth — PAT auth, - // silently ignoring the supplied OAuth credentials. ValidateConfig below therefore - // rejects the auth method explicitly. Both halves are required. - // - // With only oauth2 left and Default: true, its required fields also apply - // unconditionally, so --auth-method workspace-token WITHOUT OAuth credentials - // fails earlier still, in field validation. - // - // The flags and pkg/databricks/auth.go's NewTokenAuth path still exist and still - // compile — they are simply unreachable. So this is a BREAKING CHANGE for any - // self-hosted deployment currently authenticating with workspace tokens; they must - // move to OAuth2 or stay on the previous version. - // - // Restore this block once CXE-1374 ships. Do NOT delete it, and do NOT remove the - // PAT documentation: this connector already lost PAT once in e84a1aef with the docs - // left in place, and that mismatch is exactly what CXH-2166 was filed to fix. - // - // { - // Name: DatabricksWorkspaceTokenGroup, - // DisplayName: "Workspace token", - // HelpText: "Authenticate with a personal access token scoped to each workspace. " + - // "Does not sync account-level data (account entitlements and grants, and " + - // "workspace-membership entitlements); use OAuth for full account coverage.", - // Fields: []field.SchemaField{AccountIdField, WorkspacesField, WorkspaceTokensField, HostnameField, AccountHostnameField}, - // Default: false, - // }, - }), ) - -// ValidateConfig enforces what field groups can't: OAuth/token exclusion when no -// auth method is set, and equal-length workspaces/workspace-tokens. -func ValidateConfig(ctx context.Context, cfg *Databricks, authMethod string) error { - // Workspace-token auth is temporarily disabled (see the commented-out field group - // above). Commenting out the group is NOT sufficient on its own: FieldGroupFields - // falls back to the default group for an unrecognised auth method, so a config that - // satisfies the oauth2 group's required fields AND sets - // --auth-method workspace-token passes field.Validate — workspace-tokens is skipped - // entirely because it is no longer in the selected group, so neither its - // WithRequired nor its DependentOn rule fires. prepareClientAuth would then still - // branch to NewTokenAuth and authenticate with PATs, silently ignoring the OAuth - // credentials the operator supplied. Reject it here so the disable is unconditional. - if authMethod == DatabricksWorkspaceTokenGroup { - return fmt.Errorf( - "databricks-connector: workspace-token authentication is temporarily unavailable; " + - "use OAuth (databricks-client-id and databricks-client-secret) instead", - ) - } - - // A merged/stored config can carry both groups' fields; once authMethod picks one, - // prepareClientAuth only reads that group, so the other group's leftovers are inert. - if authMethod == "" && len(cfg.WorkspaceTokens) > 0 && (cfg.DatabricksClientId != "" || cfg.DatabricksClientSecret != "") { - return fmt.Errorf("databricks-connector: databricks-client-id/databricks-client-secret and workspace-tokens are mutually exclusive") - } - - if authMethod == DatabricksWorkspaceTokenGroup && len(cfg.Workspaces) != len(cfg.WorkspaceTokens) { - return fmt.Errorf( - "databricks-connector: workspaces and workspace-tokens must be the same length, got %d workspaces and %d tokens", - len(cfg.Workspaces), - len(cfg.WorkspaceTokens), - ) - } - - return nil -} diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go deleted file mode 100644 index c9dcc5fe..00000000 --- a/pkg/config/config_test.go +++ /dev/null @@ -1,107 +0,0 @@ -package config - -import ( - "context" - "strings" - "testing" -) - -func TestValidateConfig(t *testing.T) { - cases := []struct { - name string - workspaces []string - tokens []string - authMethod string - wantErr bool - }{ - // Workspace-token auth is temporarily disabled, so selecting it is rejected - // regardless of whether the workspace/token lists are otherwise well-formed. - // These first four previously asserted the pairing rules; they now assert the - // disable, because it short-circuits before those rules are reached. - {"disabled: no tokens", nil, nil, DatabricksWorkspaceTokenGroup, true}, - {"disabled: equal length", []string{"ws-1", "ws-2"}, []string{"tok-1", "tok-2"}, DatabricksWorkspaceTokenGroup, true}, - {"disabled: more workspaces than tokens", []string{"ws-1", "ws-2"}, []string{"tok-1"}, DatabricksWorkspaceTokenGroup, true}, - {"disabled: tokens without workspaces", nil, []string{"tok-1"}, DatabricksWorkspaceTokenGroup, true}, - {"mismatched lengths ignored outside workspace-token method", []string{"ws-1", "ws-2"}, []string{"tok-1"}, DatabricksOAuth2Group, false}, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - cfg := &Databricks{Workspaces: tc.workspaces, WorkspaceTokens: tc.tokens} - err := ValidateConfig(context.Background(), cfg, tc.authMethod) - if tc.wantErr && err == nil { - t.Fatal("expected error, got nil") - } - if !tc.wantErr && err != nil { - t.Fatalf("expected no error, got %v", err) - } - }) - } -} - -// Both auth modes' fields live in the same struct; with no auth method selected, -// ValidateConfig can't tell which credentials would actually be used, so it must -// reject having both set. -func TestValidateConfigRejectsBothAuthModesWhenAmbiguous(t *testing.T) { - cfg := &Databricks{ - DatabricksClientId: "client-id", - Workspaces: []string{"ws-1"}, - WorkspaceTokens: []string{"tok-1"}, - } - - if err := ValidateConfig(context.Background(), cfg, ""); err == nil { - t.Fatal("expected error, got nil") - } -} - -func TestValidateConfigRejectsClientSecretWithTokensWhenAmbiguous(t *testing.T) { - cfg := &Databricks{ - DatabricksClientSecret: "client-secret", - Workspaces: []string{"ws-1"}, - WorkspaceTokens: []string{"tok-1"}, - } - - if err := ValidateConfig(context.Background(), cfg, ""); err == nil { - t.Fatal("expected error, got nil") - } -} - -// Once an auth method is explicitly selected, prepareClientAuth only reads that -// group's fields, so leftover values from the other group (e.g. stale workspace -// tokens on a config that has since moved to OAuth) must not block startup. -func TestValidateConfigTrustsExplicitAuthMethod(t *testing.T) { - cfg := &Databricks{ - DatabricksClientId: "client-id", - DatabricksClientSecret: "client-secret", - Workspaces: []string{"ws-1"}, - WorkspaceTokens: []string{"tok-1"}, - } - - if err := ValidateConfig(context.Background(), cfg, DatabricksOAuth2Group); err != nil { - t.Fatalf("expected no error, got %v", err) - } -} - -// Commenting out the workspace-token field group is not enough on its own to -// disable the auth method. FieldGroupFields falls back to the default group for -// an unrecognised auth method, so a config that satisfies oauth2's required -// fields while selecting workspace-token passes field.Validate — and -// prepareClientAuth would still branch to NewTokenAuth, authenticating with PATs -// and silently ignoring the OAuth credentials supplied. ValidateConfig must -// reject the method outright. This is the regression test for that bypass. -func TestValidateConfigRejectsWorkspaceTokenEvenWithValidOAuthCreds(t *testing.T) { - cfg := &Databricks{ - DatabricksClientId: "client-id", - DatabricksClientSecret: "client-secret", - Workspaces: []string{"ws-1"}, - WorkspaceTokens: []string{"tok-1"}, - } - - err := ValidateConfig(context.Background(), cfg, DatabricksWorkspaceTokenGroup) - if err == nil { - t.Fatal("expected workspace-token auth to be rejected while disabled, got nil") - } - if !strings.Contains(err.Error(), "temporarily unavailable") { - t.Fatalf("expected a 'temporarily unavailable' error, got %v", err) - } -} diff --git a/pkg/connector/connector.go b/pkg/connector/connector.go index 10996986..832feb35 100644 --- a/pkg/connector/connector.go +++ b/pkg/connector/connector.go @@ -102,25 +102,17 @@ func (d *Databricks) Metadata(ctx context.Context) (*v2.ConnectorMetadata, error }, nil } -// Validate is called to ensure that the connector is properly configured. It should exercise any API credentials -// to be sure that they are valid. Since this connector works with two APIs and can have different types of credentials -// it is important to validate that the connector is properly configured before attempting to sync. +// Validate is called to ensure that the connector is properly configured. It exercises the +// OAuth credentials against both the Account API and each workspace the sync will cover. func (d *Databricks) Validate(ctx context.Context) (annotations.Annotations, error) { - isAccAPIAvailable := false - isWSAPIAvailable := false - - // OAuth must reach the account API; a failed check here is a fixable misconfiguration, - // so fail instead of silently dropping account-level data (token auth, handled below, - // can't reach it by design). - if !d.client.IsTokenAuth() { - if _, _, err := d.client.ListRoles(ctx, "", "", ""); err != nil { - return nil, fmt.Errorf("databricks-connector: account API validation failed: %w", err) - } - isAccAPIAvailable = true + // A failed account API check is a fixable misconfiguration, so fail instead of + // silently dropping account-level data. + if _, _, err := d.client.ListRoles(ctx, "", "", ""); err != nil { + return nil, fmt.Errorf("databricks-connector: account API validation failed: %w", err) } - // With an explicit workspace list (always the case for token auth), validate each - // configured workspace. Otherwise discover every workspace from the Account API. + // With an explicit workspace list, validate each configured workspace. Otherwise + // discover every workspace from the Account API. workspaceNames := d.workspaces if len(workspaceNames) == 0 { workspaces, _, err := d.client.ListWorkspaces(ctx) @@ -134,32 +126,21 @@ func (d *Databricks) Validate(ctx context.Context) (annotations.Annotations, err } } + isWSAPIAvailable := false for _, workspace := range workspaceNames { - _, _, err := d.client.ListRoles(ctx, workspace, "", "") - if err != nil && !isAccAPIAvailable { - return nil, fmt.Errorf("databricks-connector: failed to validate credentials for workspace %s: %w", workspace, err) + // Not fatal: the account API already validated, and a workspace the service + // principal can't reach is skipped during sync rather than failing it. + if _, _, err := d.client.ListRoles(ctx, workspace, "", ""); err != nil { + ctxzap.Extract(ctx).Debug("databricks-connector: workspace validation probe failed", + zap.String("workspace", workspace), + zap.Error(err), + ) } isWSAPIAvailable = true } - // Resolve the result. - if !isAccAPIAvailable && !isWSAPIAvailable { - return nil, fmt.Errorf("databricks-connector: failed to validate credentials") - } - - d.client.UpdateAvailability(isAccAPIAvailable, isWSAPIAvailable) - - // Token auth can't reach the account plane: account entitlements/grants and - // workspace-membership entitlements go unsynced and identities re-parent onto the - // workspace. Warn, not Debug (invisible at info level), so this drop isn't silent. - if !isAccAPIAvailable && isWSAPIAvailable { - ctxzap.Extract(ctx).Warn( - "databricks-connector: account API unreachable under workspace-token auth; syncing workspace-scoped data only. " + - "Account entitlements and grants, and workspace-membership entitlements, will not be synced, " + - "and identities are parented under their workspace instead of the account", - ) - } + d.client.UpdateAvailability(true, isWSAPIAvailable) return nil, nil } @@ -192,20 +173,11 @@ func New( } // NewConnector returns a new connector builder from a configuration struct. -func NewConnector(ctx context.Context, cfg *config.Databricks, opts *cli.ConnectorOpts) (connectorbuilder.ConnectorBuilderV2, []connectorbuilder.Opt, error) { +func NewConnector(ctx context.Context, cfg *config.Databricks, _ *cli.ConnectorOpts) (connectorbuilder.ConnectorBuilderV2, []connectorbuilder.Opt, error) { l := ctxzap.Extract(ctx) - authMethod := "" - if opts != nil { - authMethod = opts.SelectedAuthMethod - } - - if err := config.ValidateConfig(ctx, cfg, authMethod); err != nil { - return nil, nil, err - } - accountHostname := getAccountHostname(cfg, cfg.Hostname) - auth := prepareClientAuth(ctx, cfg, authMethod, l) + auth := prepareClientAuth(cfg, l) cb, err := New( ctx, @@ -224,12 +196,7 @@ func NewConnector(ctx context.Context, cfg *config.Databricks, opts *cli.Connect return cb, nil, nil } -func prepareClientAuth(_ context.Context, cfg *config.Databricks, authMethod string, l *zap.Logger) databricks.Auth { - if authMethod == config.DatabricksWorkspaceTokenGroup { - l.Debug("using workspace token auth", zap.String("account-id", cfg.AccountId)) - return databricks.NewTokenAuth(cfg.Workspaces, cfg.WorkspaceTokens) - } - +func prepareClientAuth(cfg *config.Databricks, l *zap.Logger) databricks.Auth { l.Debug("using oauth", zap.String("account-id", cfg.AccountId)) return databricks.NewOAuth2( cfg.AccountId, diff --git a/pkg/connector/groups.go b/pkg/connector/groups.go index 7d223f8b..a490223a 100644 --- a/pkg/connector/groups.go +++ b/pkg/connector/groups.go @@ -149,12 +149,6 @@ func (g *groupBuilder) Entitlements(ctx context.Context, resource *v2.Resource, // get all assignable roles for this specific group resource roles, _, err := g.client.ListRoles(ctx, workspaceId, GroupsType, groupId.Resource) if err != nil { - if workspaceId != "" && isGroupNotFoundError(err) { - ctxzap.Extract(ctx).Warn("databricks-connector: skipping roles for group not recognized by the rule-sets API", - zap.String("group_id", groupId.Resource), - ) - return rv, nil, nil - } return nil, nil, fmt.Errorf("databricks-connector: failed to list roles for group %s: %w", groupId.Resource, err) } @@ -190,8 +184,8 @@ func (g *groupBuilder) Grants(ctx context.Context, resource *v2.Resource, _ rs.S } // membership grants - // Always fetch the group with members attribute to ensure we get the members - // regardless of authentication type (OAuth vs personal access token) + // Always fetch the group with the members attribute; the group listing above + // does not include members. group, rateLimitData, err := g.client.GetGroup(ctx, workspaceId, groupId.Resource, databricks.NewGroupMembersAttrVars()) if err != nil { return nil, nil, fmt.Errorf("databricks-connector: failed to get group %s: %w", groupId.Resource, err) @@ -236,12 +230,6 @@ func (g *groupBuilder) Grants(ctx context.Context, resource *v2.Resource, _ rs.S // role permissions grants ruleSets, rateLimitDataRuleSets, err := g.client.ListRuleSets(ctx, workspaceId, GroupsType, groupId.Resource) if err != nil { - if isWorkspaceGroup && isGroupNotFoundError(err) { - l.Warn("databricks-connector: skipping role rule sets for group not recognized by the rule-sets API", - zap.String("group_id", groupId.Resource), - ) - return rv, &rs.SyncOpResults{Annotations: annos}, nil - } return nil, nil, fmt.Errorf("databricks-connector: failed to list role rule sets for group %s: %w", resource.Id.Resource, err) } diff --git a/pkg/connector/helpers.go b/pkg/connector/helpers.go index be1db76f..54874a57 100644 --- a/pkg/connector/helpers.go +++ b/pkg/connector/helpers.go @@ -2,9 +2,7 @@ package connector import ( "context" - "errors" "fmt" - "net/http" "slices" "strings" @@ -37,17 +35,6 @@ func parseResourceId(resourceId string) (*v2.ResourceId, *v2.ResourceId, error) return nil, nil, fmt.Errorf("invalid resource ID: %s", resourceId) } -// Mirrors how groupBuilder parents synced groups: account when its API is -// reachable, otherwise the workspace (token auth). accountResource() in -// account.go encodes the same condition for its child-resource-type list; -// keep both in sync. -func groupGrantParent(accountAPIAvailable bool, accountId, workspaceId string) (*v2.ResourceId, error) { - if accountAPIAvailable { - return rs.NewResourceID(accountResourceType, accountId) - } - return rs.NewResourceID(workspaceResourceType, workspaceId) -} - func groupGrantExpansion(ctx context.Context, groupId string, parentResource *v2.ResourceId) (*v2.ResourceId, *v2.GrantExpandable, error) { groupResourceStr := groupResourceId(ctx, groupId, parentResource) resourceId, err := rs.NewResourceID(groupResourceType, groupResourceStr) @@ -157,20 +144,6 @@ func preparePrincipalId(ctx context.Context, c *databricks.Client, workspaceId, return result, nil } -// isGroupNotFoundError matches the rule-sets/roles API's response for a group ID -// it doesn't recognize (e.g. an orphaned or stale workspace SCIM group), distinct -// from other 400s. -func isGroupNotFoundError(err error) bool { - var apiErr *databricks.APIError - if !errors.As(err, &apiErr) { - return false - } - msg := strings.ToLower(apiErr.Message) - return apiErr.StatusCode == http.StatusBadRequest && - strings.Contains(msg, "not found") && - strings.Contains(msg, "group") -} - func isValidPrincipal(principal *v2.ResourceId) bool { return principal.ResourceType == userResourceType.Id || principal.ResourceType == groupResourceType.Id || diff --git a/pkg/connector/helpers_test.go b/pkg/connector/helpers_test.go deleted file mode 100644 index 54aaa341..00000000 --- a/pkg/connector/helpers_test.go +++ /dev/null @@ -1,99 +0,0 @@ -package connector - -import ( - "context" - "errors" - "net/http" - "testing" - - "github.com/conductorone/baton-databricks/pkg/databricks" - v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" -) - -// CXH-2166 regression: under token auth (no account API), groups sync parented -// under the workspace. Role grants to those groups must use the same parent, or -// the grant's principal ID references a group resource that was never synced. -func TestGroupGrantParentMatchesSyncedGroupId(t *testing.T) { - ctx := context.Background() - - t.Run("token auth uses workspace parent", func(t *testing.T) { - parent, err := groupGrantParent(false, "acc-1", "dbc-abc") - if err != nil { - t.Fatalf("groupGrantParent: %v", err) - } - - gotResourceId, _, err := groupGrantExpansion(ctx, "group-1", parent) - if err != nil { - t.Fatalf("groupGrantExpansion: %v", err) - } - - wantId := groupResourceId(ctx, "group-1", &v2.ResourceId{ResourceType: workspaceResourceType.Id, Resource: "dbc-abc"}) - if gotResourceId.Resource != wantId { - t.Errorf("principal ID = %q, want %q (the ID groupBuilder emits for a workspace-parented group)", gotResourceId.Resource, wantId) - } - }) - - t.Run("account API available uses account parent", func(t *testing.T) { - parent, err := groupGrantParent(true, "acc-1", "dbc-abc") - if err != nil { - t.Fatalf("groupGrantParent: %v", err) - } - - gotResourceId, _, err := groupGrantExpansion(ctx, "group-1", parent) - if err != nil { - t.Fatalf("groupGrantExpansion: %v", err) - } - - wantId := groupResourceId(ctx, "group-1", &v2.ResourceId{ResourceType: accountResourceType.Id, Resource: "acc-1"}) - if gotResourceId.Resource != wantId { - t.Errorf("principal ID = %q, want %q (the ID groupBuilder emits for an account-parented group)", gotResourceId.Resource, wantId) - } - }) -} - -func TestIsGroupNotFoundError(t *testing.T) { - tests := []struct { - name string - err error - want bool - }{ - { - name: "matching group not found", - err: &databricks.APIError{StatusCode: http.StatusBadRequest, Message: "Group 12345 not found"}, - want: true, - }, - { - name: "non-matching 400", - err: &databricks.APIError{StatusCode: http.StatusBadRequest, Message: "invalid role name"}, - want: false, - }, - { - name: "unrelated not-found 400 without group in message", - err: &databricks.APIError{StatusCode: http.StatusBadRequest, Message: "workspace not found"}, - want: false, - }, - { - name: "404 status code", - err: &databricks.APIError{StatusCode: http.StatusNotFound, Message: "Group 12345 not found"}, - want: false, - }, - { - name: "mixed case still matches", - err: &databricks.APIError{StatusCode: http.StatusBadRequest, Message: "GROUP 12345 Not Found"}, - want: true, - }, - { - name: "non-APIError", - err: errors.New("connection reset"), - want: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := isGroupNotFoundError(tt.err); got != tt.want { - t.Errorf("isGroupNotFoundError() = %v, want %v", got, tt.want) - } - }) - } -} diff --git a/pkg/connector/roles.go b/pkg/connector/roles.go index 8fb0ec1f..b37862dc 100644 --- a/pkg/connector/roles.go +++ b/pkg/connector/roles.go @@ -226,7 +226,7 @@ func (r *roleBuilder) Grants(ctx context.Context, resource *v2.Resource, attr rs } if (!isWorkspaceRole && g.HaveRole(roleName)) || (isWorkspaceRole && g.HaveEntitlement(roleName)) { - groupParentResourceId, err := groupGrantParent(r.client.IsAccountAPIAvailable(), r.client.GetAccountId(), workspaceId) + groupParentResourceId, err := rs.NewResourceID(accountResourceType, r.client.GetAccountId()) if err != nil { return rv, nil, err } diff --git a/pkg/connector/validate_test.go b/pkg/connector/validate_test.go index 5b205729..d515f3d8 100644 --- a/pkg/connector/validate_test.go +++ b/pkg/connector/validate_test.go @@ -1,18 +1,13 @@ package connector import ( - "bytes" "context" - "encoding/json" "io" "net/http" "strings" "testing" "github.com/conductorone/baton-databricks/pkg/databricks" - "github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap" - "go.uber.org/zap" - "go.uber.org/zap/zapcore" ) // rolesTransport answers the assignable-roles calls Validate makes. failAccount @@ -37,15 +32,6 @@ func (t rolesTransport) RoundTrip(req *http.Request) (*http.Response, error) { }, nil } -func captureLogs(ctx context.Context, buf *bytes.Buffer) context.Context { - core := zapcore.NewCore( - zapcore.NewJSONEncoder(zap.NewProductionEncoderConfig()), - zapcore.AddSync(buf), - zapcore.DebugLevel, - ) - return ctxzap.ToContext(ctx, zap.New(core)) -} - func newValidateConnector(t *testing.T, auth databricks.Auth, tr http.RoundTripper) *Databricks { t.Helper() client, err := databricks.NewClient( @@ -59,47 +45,6 @@ func newValidateConnector(t *testing.T, auth databricks.Auth, tr http.RoundTripp return &Databricks{client: client, workspaces: []string{"ws1"}} } -// levelFor scans the captured JSON log lines for the first entry whose message -// contains want and returns its level. Empty string means no such entry. -func levelFor(t *testing.T, buf *bytes.Buffer, want string) string { - t.Helper() - for _, line := range strings.Split(buf.String(), "\n") { - if line == "" { - continue - } - var entry struct { - Level string `json:"level"` - Msg string `json:"msg"` - } - if err := json.Unmarshal([]byte(line), &entry); err != nil { - continue - } - if strings.Contains(entry.Msg, want) { - return entry.Level - } - } - return "" -} - -const accountUnreachableMsg = "account API unreachable" - -// CXH-2350: dropping the whole account plane is a customer-visible degradation, so the -// startup notice must be visible. It logs at warn, not debug (a debug line is invisible at -// the default info level, which is the silent degradation the ticket was filed to fix). -func TestValidateWorkspaceTokenLogsAtWarn(t *testing.T) { - buf := &bytes.Buffer{} - ctx := captureLogs(context.Background(), buf) - d := newValidateConnector(t, databricks.NewTokenAuth([]string{"ws1"}, []string{"tok"}), rolesTransport{}) - - if _, err := d.Validate(ctx); err != nil { - t.Fatalf("Validate: %v", err) - } - - if got := levelFor(t, buf, accountUnreachableMsg); got != "warn" { - t.Errorf("token-auth notice logged at %q, want %q", got, "warn") - } -} - // Under OAuth a failed account check is a fixable misconfiguration, so Validate // fails rather than silently dropping account-level data. func TestValidateOAuthAccountCheckFailureReturnsError(t *testing.T) { @@ -109,18 +54,3 @@ func TestValidateOAuthAccountCheckFailureReturnsError(t *testing.T) { t.Fatal("Validate: want error on OAuth account check failure, got nil") } } - -// When the account API is reachable (non-token auth), the notice must not fire at all. -func TestValidateAccountReachableNoNotice(t *testing.T) { - buf := &bytes.Buffer{} - ctx := captureLogs(context.Background(), buf) - d := newValidateConnector(t, &databricks.NoAuth{}, rolesTransport{}) - - if _, err := d.Validate(ctx); err != nil { - t.Fatalf("Validate: %v", err) - } - - if got := levelFor(t, buf, accountUnreachableMsg); got != "" { - t.Errorf("notice fired (level %q) when account API was reachable", got) - } -} diff --git a/pkg/connector/workspaces.go b/pkg/connector/workspaces.go index 8a594192..55a16e49 100644 --- a/pkg/connector/workspaces.go +++ b/pkg/connector/workspaces.go @@ -31,27 +31,6 @@ func (w *workspaceBuilder) ResourceType(ctx context.Context) *v2.ResourceType { return workspaceResourceType } -// minimalWorkspaceResource builds a workspace from just its deployment name, for -// token auth where the Account API (and its numeric workspace IDs) is unreachable. -// Deployment names are unique per Databricks cloud (they form the workspace's -// canonical hostname), so they're safe as the resource ID here. -// Users, groups and service principals hang off the workspace here instead of the account. -func minimalWorkspaceResource(_ context.Context, workspace *databricks.Workspace, parent *v2.ResourceId) (*v2.Resource, error) { - return rs.NewGroupResource( - workspace.DeploymentName, - workspaceResourceType, - workspace.DeploymentName, - nil, - rs.WithParentResourceID(parent), - rs.WithAnnotation( - &v2.ChildResourceType{ResourceTypeId: userResourceType.Id}, - &v2.ChildResourceType{ResourceTypeId: groupResourceType.Id}, - &v2.ChildResourceType{ResourceTypeId: servicePrincipalResourceType.Id}, - &v2.ChildResourceType{ResourceTypeId: roleResourceType.Id}, - ), - ) -} - func workspaceResource(_ context.Context, workspace *databricks.Workspace, parent *v2.ResourceId) (*v2.Resource, error) { profile := map[string]interface{}{ "workspace_id": workspace.ID, @@ -84,31 +63,6 @@ func (w *workspaceBuilder) List(ctx context.Context, parentResourceID *v2.Resour var rv []*v2.Resource - if w.client.IsTokenAuth() { - for workspace := range w.workspaces { - if w.client.IsWorkspaceNameExcluded(workspace) { - continue - } - - ws := &databricks.Workspace{DeploymentName: workspace} - - wr, err := minimalWorkspaceResource(ctx, ws, parentResourceID) - if err != nil { - return nil, nil, err - } - - rv = append(rv, wr) - } - - if len(w.workspaces) > 0 && len(rv) == 0 { - ctxzap.Extract(ctx).Warn("databricks-connector: all configured workspaces are excluded, sync will be empty", - zap.Strings("workspaces", configuredWorkspaceNames(w.workspaces)), - ) - } - - return rv, nil, nil - } - workspaces, _, err := w.client.ListWorkspaces(ctx) if err != nil { return nil, nil, fmt.Errorf("databricks-connector: failed to list workspaces: %w", err) diff --git a/pkg/databricks/auth.go b/pkg/databricks/auth.go index 1ef846cf..63cda5bc 100644 --- a/pkg/databricks/auth.go +++ b/pkg/databricks/auth.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "net/http" - "strings" "github.com/conductorone/baton-sdk/pkg/uhttp" "github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap" @@ -30,52 +29,6 @@ func (n *NoAuth) GetClient(ctx context.Context) (*http.Client, error) { return httpClient, nil } -// TokenAuth authenticates each request with the workspace-scoped personal access -// token for the workspace it targets. Account-level requests match no token. -type TokenAuth struct { - tokens map[string]string -} - -func NewTokenAuth(workspaces, tokens []string) *TokenAuth { - tokensMap := make(map[string]string, len(workspaces)) - for i, workspace := range workspaces { - if i >= len(tokens) { - break - } - tokensMap[workspace] = tokens[i] - } - - return &TokenAuth{tokens: tokensMap} -} - -func (t *TokenAuth) Apply(req *http.Request) { - // A workspace request host is ".". A shorter - // deployment name can be a false prefix of a longer one (Azure names - // contain a dot, e.g. "adb-123" of "adb-123.1"), so match the longest one. - host := req.URL.Host - var bestWorkspace, bestToken string - for workspace, token := range t.tokens { - if host != workspace && !strings.HasPrefix(host, workspace+".") { - continue - } - if len(workspace) > len(bestWorkspace) { - bestWorkspace, bestToken = workspace, token - } - } - if bestToken != "" { - req.Header.Set("Authorization", "Bearer "+bestToken) - } -} - -func (t *TokenAuth) GetClient(ctx context.Context) (*http.Client, error) { - httpClient, err := uhttp.NewClient(ctx, uhttp.WithLogger(true, ctxzap.Extract(ctx))) - if err != nil { - return nil, err - } - - return httpClient, nil -} - type OAuth2 struct { cfg *clientcredentials.Config } diff --git a/pkg/databricks/auth_test.go b/pkg/databricks/auth_test.go deleted file mode 100644 index 3c19b8d1..00000000 --- a/pkg/databricks/auth_test.go +++ /dev/null @@ -1,87 +0,0 @@ -package databricks - -import ( - "net/http" - "net/url" - "testing" -) - -func mustURL(t *testing.T, raw string) *url.URL { - t.Helper() - u, err := url.Parse(raw) - if err != nil { - t.Fatalf("parse %q: %v", raw, err) - } - return u -} - -func TestTokenAuthApply(t *testing.T) { - auth := NewTokenAuth( - []string{"dbc-abc123", "adb-2531901403506481.1"}, - []string{"aws-token", "azure-token"}, - ) - - cases := []struct { - name string - host string - wantToken string - }{ - {"aws deployment name (no dot)", "dbc-abc123.cloud.databricks.com", "aws-token"}, - {"azure deployment name (dotted)", "adb-2531901403506481.1.azuredatabricks.net", "azure-token"}, - {"account host matches nothing", "accounts.azuredatabricks.net", ""}, - {"unknown workspace matches nothing", "dbc-other.cloud.databricks.com", ""}, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - req := &http.Request{URL: mustURL(t, "https://"+tc.host+"/api/2.0/preview/scim/v2/Users"), Header: http.Header{}} - auth.Apply(req) - - got := req.Header.Get("Authorization") - want := "" - if tc.wantToken != "" { - want = "Bearer " + tc.wantToken - } - if got != want { - t.Fatalf("Authorization = %q, want %q", got, want) - } - }) - } -} - -// A workspace name that prefixes another must not steal the longer one's token. -func TestTokenAuthApplyPrefixCollision(t *testing.T) { - auth := NewTokenAuth([]string{"dbc-1", "dbc-12"}, []string{"token-1", "token-12"}) - - req := &http.Request{URL: mustURL(t, "https://dbc-12.cloud.databricks.com/x"), Header: http.Header{}} - auth.Apply(req) - - if got := req.Header.Get("Authorization"); got != "Bearer token-12" { - t.Fatalf("Authorization = %q, want %q", got, "Bearer token-12") - } -} - -// An Azure deployment name's own dot must not let a shorter workspace name -// falsely prefix a longer one that embeds it (e.g. "adb-123" of "adb-123.1"). -func TestTokenAuthApplyNestedDottedPrefix(t *testing.T) { - auth := NewTokenAuth([]string{"adb-123", "adb-123.1"}, []string{"token-short", "token-long"}) - - req := &http.Request{URL: mustURL(t, "https://adb-123.1.azuredatabricks.net/x"), Header: http.Header{}} - auth.Apply(req) - - if got := req.Header.Get("Authorization"); got != "Bearer token-long" { - t.Fatalf("Authorization = %q, want %q", got, "Bearer token-long") - } -} - -// Fewer tokens than workspaces must not panic; unmatched workspaces just get no token. -func TestNewTokenAuthFewerTokensThanWorkspaces(t *testing.T) { - auth := NewTokenAuth([]string{"dbc-1", "dbc-2"}, []string{"token-1"}) - - req := &http.Request{URL: mustURL(t, "https://dbc-2.cloud.databricks.com/x"), Header: http.Header{}} - auth.Apply(req) - - if got := req.Header.Get("Authorization"); got != "" { - t.Fatalf("Authorization = %q, want empty", got) - } -} diff --git a/pkg/databricks/client.go b/pkg/databricks/client.go index 5882e8fb..3312a7bd 100644 --- a/pkg/databricks/client.go +++ b/pkg/databricks/client.go @@ -163,11 +163,6 @@ func (c *Client) UpdateAvailability(accAPI, wsAPI bool) { c.isWSAPIAvailable = wsAPI } -func (c *Client) IsTokenAuth() bool { - _, ok := c.auth.(*TokenAuth) - return ok -} - func (c *Client) UpdateEtag(etag string) { c.etag = etag } From 659e75b332abc7dbe5eb56e7cf90ed6b61c417d1 Mon Sep 17 00:00:00 2001 From: Luisina Santos Date: Tue, 15 Sep 2026 12:31:24 -0300 Subject: [PATCH 2/6] CXH-2166: revert PAT auth and workspace filtering (revert #54) Full revert of #54. CXH-2109 records both "PAT and username/password auth are not implemented" and "workspace filtering is not implemented" as the connector's actual, expected behaviour; the fix for both is to correct the documentation, not to build the features. #54 built them instead, so this removes them and updates the docs to match what the connector does. PAT auth was already unreachable: #63 commented out the workspace-token field group and rejected the auth method in ValidateConfig, leaving the implementation behind docs that called it temporarily unavailable and expected to return. OAuth2 is now the connector's only auth method, and the docs say so without qualification. Reverted to their pre-#54 state, byte for byte: pkg/config/config.go (workspaces + workspace-tokens fields, the field groups, the constraints, ValidateConfig) pkg/databricks/auth.go (TokenAuth) pkg/connector/helpers.go (groupGrantParent, isGroupNotFoundError) pkg/connector/workspaces.go (workspace allowlist filtering) pkg/connector/groups.go pkg/connector/roles.go pkg/connector/account.go pkg/connector/service-principals.go pkg/databricks/client.go drops IsTokenAuth and IsWorkspaceNameExcluded, both added by #54, and keeps everything added after it. Preserved in full, since they postdate #54 and belong to other tickets: CXH-2350's fatal account-API probe, wrapTransportAuthError and their tests; the Azure/GCP account-host fix; PR #55's 403 remedy; and the databricks-exclude-workspaces flag, which predates #54. Docs (README, connector.mdx, docs-info.md) drop both the PAT method and the workspace allowlist. The README CLI dump, conf.gen.go and config_schema.json are regenerated from the built binary. Breaking change for self-hosted deployments using workspace tokens or --workspaces: workspace tokens must move to OAuth2, and scope narrowing must use --databricks-exclude-workspaces. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yaml | 1 + README.md | 13 +++--- config_schema.json | 17 +------- docs/connector.mdx | 11 +---- pkg/config/conf.gen.go | 1 - pkg/config/config.go | 16 +------ pkg/connector/account.go | 9 +--- pkg/connector/connector.go | 54 ++++++++++------------- pkg/connector/groups.go | 4 +- pkg/connector/roles.go | 5 ++- pkg/connector/service-principals.go | 3 +- pkg/connector/validate_test.go | 2 +- pkg/connector/workspaces.go | 68 +---------------------------- pkg/databricks/client.go | 12 ----- 14 files changed, 43 insertions(+), 173 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index f940cbcc..6030d7f1 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -15,6 +15,7 @@ jobs: BATON_DATABRICKS_CLIENT_SECRET: ${{ secrets.DATABRICKS_CLIENT_SECRET }} BATON_ACCOUNT_ID: ${{ secrets.BATON_ACCOUNT_ID }} # BATON_WORKSPACES: ${{ secrets.BATON_WORKSPACES }} + # BATON_WORKSPACE_TOKENS: ${{ secrets.BATON_WORKSPACE_TOKENS }} steps: - name: Checkout code uses: actions/checkout@v4 diff --git a/README.md b/README.md index 2393a52d..58bf96f8 100644 --- a/README.md +++ b/README.md @@ -77,9 +77,9 @@ baton resources - Users - Roles -By default, the connector fetches all resources from the account and all -workspaces. To limit the scope, pass a comma-separated list of workspace -deployment names to the `--workspaces` flag. +The connector fetches all resources from the account and every workspace the +service principal can access. There is no workspace allowlist; to narrow the +scope, exclude workspaces as described below. ## Authentication @@ -90,9 +90,9 @@ workspace-token option, and no username/password option. OAuth requires a reachable account API. If the account API check fails at startup, the connector fails validation rather than falling back to a -workspace-only sync, even when `--workspaces` is set. +workspace-only sync. -To instead exclude specific workspaces from the sync, pass them to the +To exclude specific workspaces from the sync, pass them to the `--databricks-exclude-workspaces` flag (or the `BATON_DATABRICKS_EXCLUDE_WORKSPACES` environment variable) as a comma-separated list. Each entry can be a workspace name, deployment name, or numeric workspace @@ -137,7 +137,7 @@ Flags: --client-secret string The client secret used to authenticate with ConductorOne ($BATON_CLIENT_SECRET) --databricks-client-id string required: The Databricks service principal's client ID used to connect to the Databricks Account and Workspace API ($BATON_DATABRICKS_CLIENT_ID) --databricks-client-secret string required: The Databricks service principal's client secret used to connect to the Databricks Account and Workspace API ($BATON_DATABRICKS_CLIENT_SECRET) - --databricks-exclude-workspaces strings Workspaces to exclude from sync, identified by workspace name, deployment name, or numeric workspace ID. Mutually exclusive with workspaces. ($BATON_DATABRICKS_EXCLUDE_WORKSPACES) + --databricks-exclude-workspaces strings Workspaces to exclude from sync, identified by workspace name, deployment name, or numeric workspace ID ($BATON_DATABRICKS_EXCLUDE_WORKSPACES) --external-resource-c1z string The path to the c1z file to sync external baton resources with ($BATON_EXTERNAL_RESOURCE_C1Z) --external-resource-entitlement-id-filter string The entitlement that external users, groups must have access to sync external baton resources ($BATON_EXTERNAL_RESOURCE_ENTITLEMENT_ID_FILTER) --external-resource-traits strings Resource type traits (e.g. "user", "group", "app") to sync and match from the external resource c1z. When unset the matcher falls back to user and group; passing this flag replaces the full set rather than adding to it. ($BATON_EXTERNAL_RESOURCE_TRAITS) @@ -164,7 +164,6 @@ Flags: --ticketing This must be set to enable ticketing support ($BATON_TICKETING) -v, --version version for baton-databricks --workers int The number of sync workers to use. -1 for auto-detect, 0 for sequential, >0 for parallel ($BATON_WORKERS) - --workspaces strings Limit syncing to the specified workspaces, by deployment name, not workspace ID. Mutually exclusive with databricks-exclude-workspaces. ($BATON_WORKSPACES) Use "baton-databricks [command] --help" for more information about a command. ``` diff --git a/config_schema.json b/config_schema.json index 858a6254..194a841b 100644 --- a/config_schema.json +++ b/config_schema.json @@ -140,28 +140,13 @@ "defaultValue": "cloud.databricks.com" } }, - { - "name": "workspaces", - "displayName": "Workspaces", - "description": "Limit syncing to the specified workspaces, by deployment name, not workspace ID. Mutually exclusive with databricks-exclude-workspaces.", - "stringSliceField": {} - }, { "name": "databricks-exclude-workspaces", "displayName": "Exclude Workspaces", - "description": "Workspaces to exclude from sync, identified by workspace name, deployment name, or numeric workspace ID. Mutually exclusive with workspaces.", + "description": "Workspaces to exclude from sync, identified by workspace name, deployment name, or numeric workspace ID", "stringSliceField": {} } ], - "constraints": [ - { - "kind": "CONSTRAINT_KIND_MUTUALLY_EXCLUSIVE", - "fieldNames": [ - "workspaces", - "databricks-exclude-workspaces" - ] - } - ], "displayName": "Databricks", "helpUrl": "/docs/baton/databricks", "iconUrl": "/static/app-icons/databricks.svg" diff --git a/docs/connector.mdx b/docs/connector.mdx index 8ca00b84..e2272120 100644 --- a/docs/connector.mdx +++ b/docs/connector.mdx @@ -198,25 +198,16 @@ stringData: BATON_DATABRICKS_CLIENT_ID: BATON_DATABRICKS_CLIENT_SECRET: - # Optional: limit the sync to specific workspaces, by deployment name. - # Mutually exclusive with BATON_DATABRICKS_EXCLUDE_WORKSPACES — set one or the other, never both. - # BATON_WORKSPACES: - # Optional: exclude specific workspaces from the sync # (workspace name, deployment name, or numeric ID). - # Mutually exclusive with BATON_WORKSPACES — set one or the other, never both. BATON_DATABRICKS_EXCLUDE_WORKSPACES: # Optional: include if you want C1 to provision access using this connector BATON_PROVISIONING: true ``` - -**`BATON_WORKSPACES` and `BATON_DATABRICKS_EXCLUDE_WORKSPACES` cannot both be set.** They are mutually exclusive, and a config carrying both is rejected at startup before any API call. Choose one. - - -OAuth requires a reachable account API. If the account API check fails at startup, the connector fails validation instead of falling back to a workspace-only sync, even when `BATON_WORKSPACES` is set. +OAuth requires a reachable account API. If the account API check fails at startup, the connector fails validation instead of falling back to a workspace-only sync. See the connector's README or run `--help` to see all available configuration flags and environment variables. diff --git a/pkg/config/conf.gen.go b/pkg/config/conf.gen.go index dd2a3bba..80ebf5b1 100644 --- a/pkg/config/conf.gen.go +++ b/pkg/config/conf.gen.go @@ -9,7 +9,6 @@ type Databricks struct { DatabricksClientId string `mapstructure:"databricks-client-id"` DatabricksClientSecret string `mapstructure:"databricks-client-secret"` Hostname string `mapstructure:"hostname"` - Workspaces []string `mapstructure:"workspaces"` BaseUrl string `mapstructure:"base-url"` DatabricksExcludeWorkspaces []string `mapstructure:"databricks-exclude-workspaces"` } diff --git a/pkg/config/config.go b/pkg/config/config.go index 8320eb8a..fb664cb3 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -14,8 +14,8 @@ var ( DatabricksClientIdField = field.StringField( "databricks-client-id", field.WithDescription("The Databricks service principal's client ID used to connect to the Databricks Account and Workspace API"), - field.WithRequired(true), field.WithDisplayName("OAuth2 Client ID"), + field.WithRequired(true), ) DatabricksClientSecretField = field.StringField( "databricks-client-secret", @@ -24,14 +24,6 @@ var ( field.WithRequired(true), field.WithDisplayName("OAuth2 Client Secret"), ) - WorkspacesField = field.StringSliceField( - "workspaces", - field.WithDescription( - "Limit syncing to the specified workspaces, by deployment name, not workspace ID. "+ - "Mutually exclusive with databricks-exclude-workspaces.", - ), - field.WithDisplayName("Workspaces"), - ) AccountHostnameField = field.StringField( "account-hostname", field.WithDescription("The hostname used to connect to the Databricks account API. If not set, it will be calculated from the hostname field."), @@ -51,7 +43,7 @@ var ( ) ExcludeWorkspacesField = field.StringSliceField( "databricks-exclude-workspaces", - field.WithDescription("Workspaces to exclude from sync, identified by workspace name, deployment name, or numeric workspace ID. Mutually exclusive with workspaces."), + field.WithDescription("Workspaces to exclude from sync, identified by workspace name, deployment name, or numeric workspace ID"), field.WithDisplayName("Exclude Workspaces"), ) configFields = []field.SchemaField{ @@ -60,7 +52,6 @@ var ( DatabricksClientIdField, DatabricksClientSecretField, HostnameField, - WorkspacesField, BaseURLField, ExcludeWorkspacesField, } @@ -72,7 +63,4 @@ var Config = field.NewConfiguration( field.WithConnectorDisplayName("Databricks"), field.WithHelpUrl("/docs/baton/databricks"), field.WithIconUrl("/static/app-icons/databricks.svg"), - field.WithConstraints( - field.FieldsMutuallyExclusive(WorkspacesField, ExcludeWorkspacesField), - ), ) diff --git a/pkg/connector/account.go b/pkg/connector/account.go index 96aa8f2c..14dc3d80 100644 --- a/pkg/connector/account.go +++ b/pkg/connector/account.go @@ -42,7 +42,6 @@ func (a *accountBuilder) ResourceType(ctx context.Context) *v2.ResourceType { return accountResourceType } -// The Account API check below mirrors groupGrantParent (helpers.go); keep both in sync. func (a *accountBuilder) accountResource(_ context.Context) (*v2.Resource, error) { accountId := a.client.GetAccountId() children := []protoreflect.ProtoMessage{ @@ -131,13 +130,7 @@ func (a *accountBuilder) Grants(ctx context.Context, resource *v2.Resource, _ rs var annotations []protoreflect.ProtoMessage if resourceId.ResourceType == groupResourceType.Id { - // Grants already returned early above when the account API is unavailable, - // so groups reaching this point are always account-parented. - groupParentResourceId, err := rs.NewResourceID(accountResourceType, a.client.GetAccountId()) - if err != nil { - return rv, nil, err - } - rid, expandAnnotation, err := groupGrantExpansion(ctx, resourceId.Resource, groupParentResourceId) + rid, expandAnnotation, err := groupGrantExpansion(ctx, resourceId.Resource, resource.ParentResourceId) if err != nil { return rv, nil, err } diff --git a/pkg/connector/connector.go b/pkg/connector/connector.go index 832feb35..9065bd72 100644 --- a/pkg/connector/connector.go +++ b/pkg/connector/connector.go @@ -16,8 +16,7 @@ import ( ) type Databricks struct { - client *databricks.Client - workspaces []string + client *databricks.Client } // ResourceSyncers returns a ResourceSyncerV2 for each resource type that should be synced from the upstream service. @@ -27,7 +26,7 @@ func (d *Databricks) ResourceSyncers(ctx context.Context) []connectorbuilder.Res newGroupBuilder(d.client), newServicePrincipalBuilder(d.client), newUserBuilder(d.client), - newWorkspaceBuilder(d.client, d.workspaces), + newWorkspaceBuilder(d.client), newRoleBuilder(d.client), } @@ -111,28 +110,19 @@ func (d *Databricks) Validate(ctx context.Context) (annotations.Annotations, err return nil, fmt.Errorf("databricks-connector: account API validation failed: %w", err) } - // With an explicit workspace list, validate each configured workspace. Otherwise - // discover every workspace from the Account API. - workspaceNames := d.workspaces - if len(workspaceNames) == 0 { - workspaces, _, err := d.client.ListWorkspaces(ctx) - if err != nil { - return nil, fmt.Errorf("databricks-connector: failed to list workspaces: %w", err) - } - - workspaceNames = make([]string, 0, len(workspaces)) - for _, workspace := range workspaces { - workspaceNames = append(workspaceNames, workspace.DeploymentName) - } + // Validate that credentials are valid for every workspace. + workspaces, _, err := d.client.ListWorkspaces(ctx) + if err != nil { + return nil, fmt.Errorf("databricks-connector: failed to list workspaces: %w", err) } isWSAPIAvailable := false - for _, workspace := range workspaceNames { + for _, workspace := range workspaces { // Not fatal: the account API already validated, and a workspace the service // principal can't reach is skipped during sync rather than failing it. - if _, _, err := d.client.ListRoles(ctx, workspace, "", ""); err != nil { + if _, _, err := d.client.ListRoles(ctx, workspace.DeploymentName, "", ""); err != nil { ctxzap.Extract(ctx).Debug("databricks-connector: workspace validation probe failed", - zap.String("workspace", workspace), + zap.String("workspace", workspace.DeploymentName), zap.Error(err), ) } @@ -154,7 +144,6 @@ func New( baseURL string, auth databricks.Auth, excludeWorkspaces []string, - workspaces []string, ) (*Databricks, error) { httpClient, err := auth.GetClient(ctx) if err != nil { @@ -167,17 +156,16 @@ func New( } return &Databricks{ - client: client, - workspaces: workspaces, + client: client, }, nil } // NewConnector returns a new connector builder from a configuration struct. -func NewConnector(ctx context.Context, cfg *config.Databricks, _ *cli.ConnectorOpts) (connectorbuilder.ConnectorBuilderV2, []connectorbuilder.Opt, error) { +func NewConnector(ctx context.Context, cfg *config.Databricks, opts *cli.ConnectorOpts) (connectorbuilder.ConnectorBuilderV2, []connectorbuilder.Opt, error) { l := ctxzap.Extract(ctx) accountHostname := getAccountHostname(cfg, cfg.Hostname) - auth := prepareClientAuth(cfg, l) + auth := prepareClientAuth(ctx, cfg, l) cb, err := New( ctx, @@ -187,22 +175,26 @@ func NewConnector(ctx context.Context, cfg *config.Databricks, _ *cli.ConnectorO cfg.BaseUrl, auth, cfg.DatabricksExcludeWorkspaces, - cfg.Workspaces, ) if err != nil { + l.Warn("error creating connector", zap.Error(err)) return nil, nil, err } return cb, nil, nil } -func prepareClientAuth(cfg *config.Databricks, l *zap.Logger) databricks.Auth { - l.Debug("using oauth", zap.String("account-id", cfg.AccountId)) +func prepareClientAuth(_ context.Context, cfg *config.Databricks, l *zap.Logger) databricks.Auth { + accountID := cfg.AccountId + databricksClientId := cfg.DatabricksClientId + databricksClientSecret := cfg.DatabricksClientSecret + accountHostname := getAccountHostname(cfg, cfg.Hostname) + return databricks.NewOAuth2( - cfg.AccountId, - cfg.DatabricksClientId, - cfg.DatabricksClientSecret, - getAccountHostname(cfg, cfg.Hostname), + accountID, + databricksClientId, + databricksClientSecret, + accountHostname, ) } diff --git a/pkg/connector/groups.go b/pkg/connector/groups.go index a490223a..4091292d 100644 --- a/pkg/connector/groups.go +++ b/pkg/connector/groups.go @@ -184,8 +184,8 @@ func (g *groupBuilder) Grants(ctx context.Context, resource *v2.Resource, _ rs.S } // membership grants - // Always fetch the group with the members attribute; the group listing above - // does not include members. + // Always fetch the group with members attribute to ensure we get the members + // regardless of authentication type (OAuth vs personal access token) group, rateLimitData, err := g.client.GetGroup(ctx, workspaceId, groupId.Resource, databricks.NewGroupMembersAttrVars()) if err != nil { return nil, nil, fmt.Errorf("databricks-connector: failed to get group %s: %w", groupId.Resource, err) diff --git a/pkg/connector/roles.go b/pkg/connector/roles.go index b37862dc..abed3569 100644 --- a/pkg/connector/roles.go +++ b/pkg/connector/roles.go @@ -226,11 +226,12 @@ func (r *roleBuilder) Grants(ctx context.Context, resource *v2.Resource, attr rs } if (!isWorkspaceRole && g.HaveRole(roleName)) || (isWorkspaceRole && g.HaveEntitlement(roleName)) { - groupParentResourceId, err := rs.NewResourceID(accountResourceType, r.client.GetAccountId()) + accountId := r.client.GetAccountId() + accountResourceId, err := rs.NewResourceID(accountResourceType, accountId) if err != nil { return rv, nil, err } - resourceId, expandAnnotation, err := groupGrantExpansion(ctx, g.ID, groupParentResourceId) + resourceId, expandAnnotation, err := groupGrantExpansion(ctx, g.ID, accountResourceId) if err != nil { return rv, nil, err } diff --git a/pkg/connector/service-principals.go b/pkg/connector/service-principals.go index a51d383c..62b92868 100644 --- a/pkg/connector/service-principals.go +++ b/pkg/connector/service-principals.go @@ -197,8 +197,7 @@ func (s *servicePrincipalBuilder) Grants(ctx context.Context, resource *v2.Resou var annotations []protoreflect.ProtoMessage if resourceId.ResourceType == groupResourceType.Id { - groupParentResourceId := &v2.ResourceId{ResourceType: parentType, Resource: parentID} - groupResourceStr := groupResourceId(ctx, resourceId.Resource, groupParentResourceId) + groupResourceStr := groupResourceId(ctx, resourceId.Resource, resource.ParentResourceId) annotations = append(annotations, &v2.GrantExpandable{ EntitlementIds: []string{fmt.Sprintf("group:%s:%s", groupResourceStr, groupMemberEntitlement)}, }) diff --git a/pkg/connector/validate_test.go b/pkg/connector/validate_test.go index d515f3d8..c7559470 100644 --- a/pkg/connector/validate_test.go +++ b/pkg/connector/validate_test.go @@ -42,7 +42,7 @@ func newValidateConnector(t *testing.T, auth databricks.Auth, tr http.RoundTripp if err != nil { t.Fatalf("NewClient: %v", err) } - return &Databricks{client: client, workspaces: []string{"ws1"}} + return &Databricks{client: client} } // Under OAuth a failed account check is a fixable misconfiguration, so Validate diff --git a/pkg/connector/workspaces.go b/pkg/connector/workspaces.go index 55a16e49..52ed5d92 100644 --- a/pkg/connector/workspaces.go +++ b/pkg/connector/workspaces.go @@ -24,7 +24,6 @@ const workspaceMemberEntitlement = "member" type workspaceBuilder struct { client *databricks.Client resourceType *v2.ResourceType - workspaces map[string]struct{} } func (w *workspaceBuilder) ResourceType(ctx context.Context) *v2.ResourceType { @@ -68,17 +67,7 @@ func (w *workspaceBuilder) List(ctx context.Context, parentResourceID *v2.Resour return nil, nil, fmt.Errorf("databricks-connector: failed to list workspaces: %w", err) } - matchedConfigured := make(map[string]struct{}, len(w.workspaces)) for _, workspace := range workspaces { - // Skip workspaces outside the configured set when one was provided. - if len(w.workspaces) > 0 { - cfg, ok := matchConfiguredWorkspace(w.workspaces, workspace.DeploymentName, workspace.Name, strconv.Itoa(workspace.ID)) - if !ok { - continue - } - matchedConfigured[cfg] = struct{}{} - } - wCopy := workspace wr, err := workspaceResource(ctx, &wCopy, parentResourceID) @@ -89,58 +78,9 @@ func (w *workspaceBuilder) List(ctx context.Context, parentResourceID *v2.Resour rv = append(rv, wr) } - l := ctxzap.Extract(ctx) - if len(w.workspaces) > 0 && len(matchedConfigured) == 0 { - l.Warn("databricks-connector: none of the configured workspaces matched any account workspace, sync will be empty", - zap.Strings("workspaces", configuredWorkspaceNames(w.workspaces)), - ) - } - for workspace := range w.workspaces { - if _, ok := matchedConfigured[workspace]; ok { - continue - } - if w.client.IsWorkspaceNameExcluded(workspace) { - l.Debug("databricks-connector: configured workspace was excluded from sync", - zap.String("workspace", workspace), - ) - continue - } - l.Debug("databricks-connector: configured workspace not found among account workspaces", - zap.String("workspace", workspace), - ) - } - return rv, nil, nil } -func configuredWorkspaceNames(configured map[string]struct{}) []string { - names := make([]string, 0, len(configured)) - for name := range configured { - names = append(names, name) - } - return names -} - -// matchConfiguredWorkspace looks up a workspace by deployment name, name, or numeric -// ID case-insensitively (mirroring Client.IsWorkspaceNameExcluded), returning the matched -// key so warnings can report the value the user configured. -func matchConfiguredWorkspace(configured map[string]struct{}, candidates ...string) (string, bool) { - for _, candidate := range candidates { - if candidate == "" { - continue - } - if _, ok := configured[candidate]; ok { - return candidate, true - } - for cfg := range configured { - if strings.EqualFold(cfg, candidate) { - return cfg, true - } - } - } - return "", false -} - // Entitlements returns slice of entitlements representing workspace members. // To get workspace members, we can only use the account API. func (w *workspaceBuilder) Entitlements(_ context.Context, resource *v2.Resource, _ rs.SyncOpAttrs) ([]*v2.Entitlement, *rs.SyncOpResults, error) { @@ -299,15 +239,9 @@ func (w *workspaceBuilder) Revoke(ctx context.Context, grant *v2.Grant) (annotat return nil, nil } -func newWorkspaceBuilder(client *databricks.Client, workspaces []string) *workspaceBuilder { - wMap := make(map[string]struct{}, len(workspaces)) - for _, w := range workspaces { - wMap[w] = struct{}{} - } - +func newWorkspaceBuilder(client *databricks.Client) *workspaceBuilder { return &workspaceBuilder{ client: client, resourceType: workspaceResourceType, - workspaces: wMap, } } diff --git a/pkg/databricks/client.go b/pkg/databricks/client.go index 3312a7bd..f4ba7c69 100644 --- a/pkg/databricks/client.go +++ b/pkg/databricks/client.go @@ -131,18 +131,6 @@ func (c *Client) isWorkspaceExcluded(w Workspace) ([]string, bool) { return keys, len(keys) > 0 } -// IsWorkspaceNameExcluded reports whether deploymentName matches the -// databricks-exclude-workspaces set. Checks the name only, not via -// isWorkspaceExcluded: that also matches on ID, and a zero-value ID here would -// let an exclude entry of "0" match every workspace. -func (c *Client) IsWorkspaceNameExcluded(deploymentName string) bool { - if len(c.excludeWorkspaces) == 0 { - return false - } - _, ok := c.excludeWorkspaces[strings.ToLower(deploymentName)] - return ok -} - func (c *Client) workspaceUrl(workspaceId string) *url.URL { return &url.URL{ Scheme: "https", From ac35999d8e5ecb94c65cc8c1d612e38fb40df3f5 Mon Sep 17 00:00:00 2001 From: Luisina Santos Date: Tue, 15 Sep 2026 12:34:14 -0300 Subject: [PATCH 3/6] CXH-2166: keep the account group grant-parent fix from #54 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The otherwise-complete revert of #54 reinstated a grant-parenting bug that #54 had incidentally fixed, so carve that one hunk out of the revert. accountBuilder.Grants passed resource.ParentResourceId into groupGrantExpansion. There resource is the account itself, whose ParentResourceId is nil, so groupResourceId returned "group/" while groups actually sync as "account//group/" — the expansion named a resource that no sync emits. Unlike the rest of #54 this is not PAT or workspace-filtering work: it is wrong under OAuth too, which is the only auth method left. Reverting it would regress against main for no benefit. servicePrincipalBuilder.Grants keeps the reverted form: there resource is a service principal, which is genuinely parented, so ParentResourceId already names the right parent under OAuth. Co-Authored-By: Claude Opus 5 --- pkg/connector/account.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/pkg/connector/account.go b/pkg/connector/account.go index 14dc3d80..ba13e03c 100644 --- a/pkg/connector/account.go +++ b/pkg/connector/account.go @@ -130,7 +130,15 @@ func (a *accountBuilder) Grants(ctx context.Context, resource *v2.Resource, _ rs var annotations []protoreflect.ProtoMessage if resourceId.ResourceType == groupResourceType.Id { - rid, expandAnnotation, err := groupGrantExpansion(ctx, resourceId.Resource, resource.ParentResourceId) + // Groups sync parented under the account, so the expansion must name + // the same parent. The account resource's own ParentResourceId is + // nil, which would yield an unparented group id that matches no + // synced resource. + groupParentResourceId, err := rs.NewResourceID(accountResourceType, a.client.GetAccountId()) + if err != nil { + return rv, nil, err + } + rid, expandAnnotation, err := groupGrantExpansion(ctx, resourceId.Resource, groupParentResourceId) if err != nil { return rv, nil, err } From 5670376460f83c2a44fa76d60c8f78aca7d224f9 Mon Sep 17 00:00:00 2001 From: Luisina Santos Date: Tue, 15 Sep 2026 12:39:21 -0300 Subject: [PATCH 4/6] CXH-2166: keep the service principal group grant-parent fix from #54 Carves the second grant-parenting hunk out of the revert, alongside the account one. servicePrincipalBuilder.Grants passed resource.ParentResourceId into groupResourceId. Unlike the account case this is non-nil, so it is not broken under OAuth today, but it reads the parent from a different source than groupBuilder does when it syncs the group. Naming the parent from the service principal's own profile, as the rest of this method already does for workspaceId, keeps the expansion and the synced group derived from one source. Co-Authored-By: Claude Opus 5 --- pkg/connector/service-principals.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/connector/service-principals.go b/pkg/connector/service-principals.go index 62b92868..203af55a 100644 --- a/pkg/connector/service-principals.go +++ b/pkg/connector/service-principals.go @@ -197,7 +197,11 @@ func (s *servicePrincipalBuilder) Grants(ctx context.Context, resource *v2.Resou var annotations []protoreflect.ProtoMessage if resourceId.ResourceType == groupResourceType.Id { - groupResourceStr := groupResourceId(ctx, resourceId.Resource, resource.ParentResourceId) + // Name the group parent from the service principal's own profile rather + // than its ParentResourceId, so the expansion matches how the group was + // synced. + groupParentResourceId := &v2.ResourceId{ResourceType: parentType, Resource: parentID} + groupResourceStr := groupResourceId(ctx, resourceId.Resource, groupParentResourceId) annotations = append(annotations, &v2.GrantExpandable{ EntitlementIds: []string{fmt.Sprintf("group:%s:%s", groupResourceStr, groupMemberEntitlement)}, }) From db1a1e3a390e357c5cff016ce0c4c90430737690 Mon Sep 17 00:00:00 2001 From: Luisina Santos Date: Fri, 18 Sep 2026 17:46:49 -0300 Subject: [PATCH 5/6] CXH-2166: drop the dead two-plane availability machinery With OAuth as the only auth method, the account plane is either reachable or Validate hard-fails, so the account/workspace availability split no longer carries information. - Validate's per-workspace ListRoles loop set isWSAPIAvailable unconditionally inside its own body, reducing to len(workspaces) > 0, and only Debug-logged failures. It cost one sequential API call per workspace on every sync start for no signal. Workspace enumeration itself is kept: every sync depends on it. - IsWorkspaceAPIAvailable had no callers anywhere in the repo; it and isWSAPIAvailable go with the loop, and UpdateAvailability with them. - isAccAPIAvailable is now set in NewClient instead of only flipping when Validate runs, so the five branches still gating sync output on it no longer depend on call ordering. Also drops prepareClientAuth's two dead parameters (the unused context.Context, and the *zap.Logger whose only use was deleted with the "using oauth" Debug), restores the blank identifier for NewConnector's unused *cli.ConnectorOpts, and removes the redundant Warn before returning an already-wrapped error from NewConnector, which the revert had pulled back in. Co-Authored-By: Claude Opus 5 --- pkg/connector/connector.go | 36 ++++++++---------------------------- pkg/databricks/client.go | 15 +++++---------- 2 files changed, 13 insertions(+), 38 deletions(-) diff --git a/pkg/connector/connector.go b/pkg/connector/connector.go index 9065bd72..1699d3e1 100644 --- a/pkg/connector/connector.go +++ b/pkg/connector/connector.go @@ -11,8 +11,6 @@ import ( "github.com/conductorone/baton-sdk/pkg/annotations" "github.com/conductorone/baton-sdk/pkg/cli" "github.com/conductorone/baton-sdk/pkg/connectorbuilder" - "github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap" - "go.uber.org/zap" ) type Databricks struct { @@ -102,7 +100,7 @@ func (d *Databricks) Metadata(ctx context.Context) (*v2.ConnectorMetadata, error } // Validate is called to ensure that the connector is properly configured. It exercises the -// OAuth credentials against both the Account API and each workspace the sync will cover. +// OAuth credentials against the Account API. func (d *Databricks) Validate(ctx context.Context) (annotations.Annotations, error) { // A failed account API check is a fixable misconfiguration, so fail instead of // silently dropping account-level data. @@ -110,28 +108,13 @@ func (d *Databricks) Validate(ctx context.Context) (annotations.Annotations, err return nil, fmt.Errorf("databricks-connector: account API validation failed: %w", err) } - // Validate that credentials are valid for every workspace. - workspaces, _, err := d.client.ListWorkspaces(ctx) - if err != nil { + // Workspace enumeration is the other account-plane call every sync depends on. + // Per-workspace probing is deliberately not done here: a workspace the service + // principal can't reach is skipped during sync rather than failing validation. + if _, _, err := d.client.ListWorkspaces(ctx); err != nil { return nil, fmt.Errorf("databricks-connector: failed to list workspaces: %w", err) } - isWSAPIAvailable := false - for _, workspace := range workspaces { - // Not fatal: the account API already validated, and a workspace the service - // principal can't reach is skipped during sync rather than failing it. - if _, _, err := d.client.ListRoles(ctx, workspace.DeploymentName, "", ""); err != nil { - ctxzap.Extract(ctx).Debug("databricks-connector: workspace validation probe failed", - zap.String("workspace", workspace.DeploymentName), - zap.Error(err), - ) - } - - isWSAPIAvailable = true - } - - d.client.UpdateAvailability(true, isWSAPIAvailable) - return nil, nil } @@ -161,11 +144,9 @@ func New( } // NewConnector returns a new connector builder from a configuration struct. -func NewConnector(ctx context.Context, cfg *config.Databricks, opts *cli.ConnectorOpts) (connectorbuilder.ConnectorBuilderV2, []connectorbuilder.Opt, error) { - l := ctxzap.Extract(ctx) - +func NewConnector(ctx context.Context, cfg *config.Databricks, _ *cli.ConnectorOpts) (connectorbuilder.ConnectorBuilderV2, []connectorbuilder.Opt, error) { accountHostname := getAccountHostname(cfg, cfg.Hostname) - auth := prepareClientAuth(ctx, cfg, l) + auth := prepareClientAuth(cfg) cb, err := New( ctx, @@ -177,14 +158,13 @@ func NewConnector(ctx context.Context, cfg *config.Databricks, opts *cli.Connect cfg.DatabricksExcludeWorkspaces, ) if err != nil { - l.Warn("error creating connector", zap.Error(err)) return nil, nil, err } return cb, nil, nil } -func prepareClientAuth(_ context.Context, cfg *config.Databricks, l *zap.Logger) databricks.Auth { +func prepareClientAuth(cfg *config.Databricks) databricks.Auth { accountID := cfg.AccountId databricksClientId := cfg.DatabricksClientId databricksClientSecret := cfg.DatabricksClientSecret diff --git a/pkg/databricks/client.go b/pkg/databricks/client.go index f4ba7c69..b61d61c5 100644 --- a/pkg/databricks/client.go +++ b/pkg/databricks/client.go @@ -47,7 +47,6 @@ type Client struct { excludeWorkspaces map[string]struct{} isAccAPIAvailable bool - isWSAPIAvailable bool } // hostMatches reports whether hostname equals suffix or sits under it at a DNS @@ -111,6 +110,11 @@ func NewClient(ctx context.Context, httpClient *http.Client, hostname, accountHo accountBaseUrl: accountBaseUrl, baseUrl: baseUrl, excludeWorkspaces: excludeSet, + + // OAuth is the only auth method, and Validate hard-fails when the account + // plane is unreachable, so availability is an invariant rather than + // something callers must wait for Validate to establish. + isAccAPIAvailable: true, }, err } @@ -138,19 +142,10 @@ func (c *Client) workspaceUrl(workspaceId string) *url.URL { } } -func (c *Client) IsWorkspaceAPIAvailable() bool { - return c.isWSAPIAvailable -} - func (c *Client) IsAccountAPIAvailable() bool { return c.isAccAPIAvailable } -func (c *Client) UpdateAvailability(accAPI, wsAPI bool) { - c.isAccAPIAvailable = accAPI - c.isWSAPIAvailable = wsAPI -} - func (c *Client) UpdateEtag(etag string) { c.etag = etag } From cf998aecf0c39368aedb9fd98a381f2caa819d7a Mon Sep 17 00:00:00 2001 From: Luisina Santos Date: Fri, 18 Sep 2026 17:46:49 -0300 Subject: [PATCH 6/6] CXH-2166: fix stale rolesTransport doc comment isAccAPIAvailable is no longer set by Validate, and the workspace check no longer runs when the account probe fails: Validate returns immediately. Co-Authored-By: Claude Opus 5 --- pkg/connector/validate_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/connector/validate_test.go b/pkg/connector/validate_test.go index c7559470..225b54e6 100644 --- a/pkg/connector/validate_test.go +++ b/pkg/connector/validate_test.go @@ -11,8 +11,8 @@ import ( ) // rolesTransport answers the assignable-roles calls Validate makes. failAccount -// makes the account-plane check (host "accounts.*") fail so isAccAPIAvailable stays -// false while the workspace check still succeeds. +// makes the account-plane check (host "accounts.*") fail, which Validate treats as +// fatal. type rolesTransport struct{ failAccount bool } func (t rolesTransport) RoundTrip(req *http.Request) (*http.Response, error) {