Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions NEXT_CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

### Bug Fixes

* Restore `NewCredentialsChain` return type to `CredentialsStrategy` interface ([#1516](https://github.com/databricks/databricks-sdk-go/pull/1516)).

### Documentation

### Internal Changes
Expand Down
6 changes: 6 additions & 0 deletions config/auth_azure_cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"golang.org/x/oauth2"

"github.com/databricks/databricks-sdk-go/common/environment"
"github.com/databricks/databricks-sdk-go/config/credentials"
"github.com/databricks/databricks-sdk-go/logger"
)
Expand All @@ -28,6 +29,11 @@ func (c AzureCliCredentials) Name() string {
return "azure-cli"
}

// Cloud implements [CloudScoped.Cloud].
func (c AzureCliCredentials) Cloud() environment.Cloud {
return environment.CloudAzure
}

// implementing azureHostResolver for ensureWorkspaceUrl to work
func (c AzureCliCredentials) tokenSourceFor(
ctx context.Context, cfg *Config, _, resource string) oauth2.TokenSource {
Expand Down
6 changes: 6 additions & 0 deletions config/auth_azure_client_secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"golang.org/x/oauth2"
"golang.org/x/oauth2/clientcredentials"

"github.com/databricks/databricks-sdk-go/common/environment"
"github.com/databricks/databricks-sdk-go/config/credentials"
"github.com/databricks/databricks-sdk-go/logger"
)
Expand All @@ -19,6 +20,11 @@ func (c AzureClientSecretCredentials) Name() string {
return "azure-client-secret"
}

// Cloud implements [CloudScoped.Cloud].
func (c AzureClientSecretCredentials) Cloud() environment.Cloud {
return environment.CloudAzure
}

func (c AzureClientSecretCredentials) tokenSourceFor(
ctx context.Context, cfg *Config, aadEndpoint, resource string) oauth2.TokenSource {
return (&clientcredentials.Config{
Expand Down
6 changes: 6 additions & 0 deletions config/auth_azure_github_oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"time"

"github.com/databricks/databricks-sdk-go/common/environment"
"github.com/databricks/databricks-sdk-go/config/credentials"
"github.com/databricks/databricks-sdk-go/config/experimental/auth"
"github.com/databricks/databricks-sdk-go/config/experimental/auth/oidc"
Expand All @@ -22,6 +23,11 @@ func (c AzureGithubOIDCCredentials) Name() string {
return "github-oidc-azure"
}

// Cloud implements [CloudScoped.Cloud].
func (c AzureGithubOIDCCredentials) Cloud() environment.Cloud {
return environment.CloudAzure
}

// Configure implements [CredentialsStrategy.Configure].
func (c AzureGithubOIDCCredentials) Configure(ctx context.Context, cfg *Config) (credentials.CredentialsProvider, error) {
if cfg.AzureClientID == "" || cfg.Host == "" || cfg.AzureTenantID == "" || cfg.ActionsIDTokenRequestURL == "" || cfg.ActionsIDTokenRequestToken == "" {
Expand Down
6 changes: 6 additions & 0 deletions config/auth_azure_msi.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"net/http"
"time"

"github.com/databricks/databricks-sdk-go/common/environment"
"github.com/databricks/databricks-sdk-go/config/credentials"
"github.com/databricks/databricks-sdk-go/httpclient"
"github.com/databricks/databricks-sdk-go/logger"
Expand All @@ -31,6 +32,11 @@ func (c AzureMsiCredentials) Name() string {
return "azure-msi"
}

// Cloud implements [CloudScoped.Cloud].
func (c AzureMsiCredentials) Cloud() environment.Cloud {
return environment.CloudAzure
}

func (c AzureMsiCredentials) Configure(ctx context.Context, cfg *Config) (credentials.CredentialsProvider, error) {
if !cfg.AzureUseMSI || (cfg.AzureResourceID == "" && cfg.ConfigType() == WorkspaceConfig) {
return nil, nil
Expand Down
36 changes: 6 additions & 30 deletions config/auth_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"fmt"
"net/http"

"github.com/databricks/databricks-sdk-go/common/environment"
"github.com/databricks/databricks-sdk-go/config/credentials"
"github.com/databricks/databricks-sdk-go/logger"
)
Expand All @@ -23,25 +22,13 @@ var ErrCannotConfigureDefault = fmt.Errorf("cannot configure default credentials
// INTERNAL: This function is not part of the public API and is subject to
// change. Users are encouraged to use an explicit credentials strategy rather
// than relying on a custom credentials chain.
func NewCredentialsChain(strategies ...CredentialsStrategy) *credentialsChain {
func NewCredentialsChain(strategies ...CredentialsStrategy) CredentialsStrategy {
return &credentialsChain{strategies: strategies}
}

type credentialsChain struct {
strategies []CredentialsStrategy
// cloudRequirements maps a strategy name to the cloud it requires. When
// set, the auto-detect loop skips strategies whose required cloud does not
// match the configured host. The map is not consulted when AuthType is
// explicitly set — in that case the named strategy is always attempted.
cloudRequirements map[string]environment.Cloud
name string
}

// WithCloudRequirements sets the cloud requirements for the chain and returns
// the chain for method chaining.
func (c *credentialsChain) WithCloudRequirements(m map[string]environment.Cloud) *credentialsChain {
c.cloudRequirements = m
return c
name string
}

func (c *credentialsChain) Name() string {
Expand Down Expand Up @@ -76,9 +63,9 @@ func (c *credentialsChain) Configure(ctx context.Context, cfg *Config) (credenti
// In auto-detect mode, skip cloud-specific strategies that don't match
// the detected cloud. This prevents Azure strategies from being
// attempted silently on GCP hosts and vice-versa.
if requiredCloud, ok := c.cloudRequirements[s.Name()]; ok {
if cfg.Environment().Cloud != requiredCloud {
logger.Debugf(ctx, "Skipping %q: not configured for %s", s.Name(), requiredCloud)
if cs, ok := s.(CloudScoped); ok {

@renaudhartert-db renaudhartert-db Mar 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you foresee any reason to generalize this beyond cloud checking?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only if we decide to do a full rewrite and align the 3 SDKs. Then we may want to completely change the logic not use nil, nil / nil, error when it does not apply and use explicit functions like this one.

But that would be a big behavioral breaking change, so I don't think redoing this interface would be much of a big deal.

if cs.Cloud() != cfg.Environment().Cloud {
logger.Debugf(ctx, "Skipping %q: requires %s", s.Name(), cs.Cloud())
continue
}
}
Expand Down Expand Up @@ -144,18 +131,7 @@ func (c *DefaultCredentials) Configure(ctx context.Context, cfg *Config) (creden
// Google strategies.
GoogleCredentials{},
GoogleDefaultCredentials{},
).WithCloudRequirements(map[string]environment.Cloud{
// cloudRequirements declares the cloud each strategy requires.
// DefaultCredentials uses this to skip cloud-specific strategies in
// auto-detect mode when the host cloud does not match. Cloud filtering
// is bypassed when AuthType is explicitly set.
"github-oidc-azure": environment.CloudAzure,
"azure-msi": environment.CloudAzure,
"azure-client-secret": environment.CloudAzure,
"azure-cli": environment.CloudAzure,
"google-credentials": environment.CloudGCP,
"google-id": environment.CloudGCP,
})
)
cp, err := chain.Configure(ctx, cfg)
if err != nil {
return nil, err
Expand Down
31 changes: 12 additions & 19 deletions config/auth_default_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,29 +10,27 @@ import (
"github.com/databricks/databricks-sdk-go/config/credentials"
)

// recordingStrategy is a test helper that records whether Configure was called.
type recordingStrategy struct {
// cloudScopedStrategy is a test helper that implements both [CredentialsStrategy]
// and [CloudScoped], recording whether Configure was called.
type cloudScopedStrategy struct {
name string
called bool
cloud environment.Cloud
}

func (r *recordingStrategy) Name() string { return r.name }
func (r *recordingStrategy) Configure(_ context.Context, _ *Config) (credentials.CredentialsProvider, error) {
r.called = true
func (s *cloudScopedStrategy) Name() string { return s.name }
func (s *cloudScopedStrategy) Cloud() environment.Cloud { return s.cloud }
func (s *cloudScopedStrategy) Configure(_ context.Context, _ *Config) (credentials.CredentialsProvider, error) {
s.called = true
return nil, nil
}

// TestCredentialsChain_CloudFiltering_SkipsOnCloudMismatch verifies that the
// chain skips a cloud-specific strategy in auto-detect mode when the detected
// cloud does not match the strategy's required cloud.
func TestCredentialsChain_CloudFiltering_SkipsOnCloudMismatch(t *testing.T) {
azureStrategy := &recordingStrategy{name: "azure-cli"}
chain := &credentialsChain{
strategies: []CredentialsStrategy{azureStrategy},
cloudRequirements: map[string]environment.Cloud{
"azure-cli": environment.CloudAzure,
},
}
azureStrategy := &cloudScopedStrategy{name: "azure-cli", cloud: environment.CloudAzure}
chain := &credentialsChain{strategies: []CredentialsStrategy{azureStrategy}}

// GCP host: azure-cli must be skipped in auto-detect mode.
cfg := &Config{Host: "https://xyz.gcp.databricks.com/", resolved: true}
Expand All @@ -47,13 +45,8 @@ func TestCredentialsChain_CloudFiltering_SkipsOnCloudMismatch(t *testing.T) {
// the cloud filter is bypassed when AuthType is explicitly set, so that a user
// can request "azure-cli" even on a GCP host.
func TestCredentialsChain_CloudFiltering_BypassesOnExplicitAuthType(t *testing.T) {
azureStrategy := &recordingStrategy{name: "azure-cli"}
chain := &credentialsChain{
strategies: []CredentialsStrategy{azureStrategy},
cloudRequirements: map[string]environment.Cloud{
"azure-cli": environment.CloudAzure,
},
}
azureStrategy := &cloudScopedStrategy{name: "azure-cli", cloud: environment.CloudAzure}
chain := &credentialsChain{strategies: []CredentialsStrategy{azureStrategy}}

// GCP host but auth_type is explicitly set: cloud filter must be bypassed.
cfg := &Config{Host: "https://xyz.gcp.databricks.com/", AuthType: "azure-cli", resolved: true}
Expand Down
6 changes: 6 additions & 0 deletions config/auth_gcp_google_credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"io/ioutil"
"os"

"github.com/databricks/databricks-sdk-go/common/environment"
"github.com/databricks/databricks-sdk-go/config/credentials"
"github.com/databricks/databricks-sdk-go/logger"
"golang.org/x/oauth2/google"
Expand All @@ -20,6 +21,11 @@ func (c GoogleCredentials) Name() string {
return "google-credentials"
}

// Cloud implements [CloudScoped.Cloud].
func (c GoogleCredentials) Cloud() environment.Cloud {
return environment.CloudGCP
}

func (c GoogleCredentials) Configure(ctx context.Context, cfg *Config) (credentials.CredentialsProvider, error) {
if cfg.GoogleCredentials == "" {
return nil, nil
Expand Down
6 changes: 6 additions & 0 deletions config/auth_gcp_google_id.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"

"github.com/databricks/databricks-sdk-go/common/environment"
"github.com/databricks/databricks-sdk-go/config/credentials"
"github.com/databricks/databricks-sdk-go/logger"
"golang.org/x/oauth2"
Expand All @@ -20,6 +21,11 @@ func (c GoogleDefaultCredentials) Name() string {
return "google-id"
}

// Cloud implements [CloudScoped.Cloud].
func (c GoogleDefaultCredentials) Cloud() environment.Cloud {
return environment.CloudGCP
}

func (c GoogleDefaultCredentials) Configure(ctx context.Context, cfg *Config) (credentials.CredentialsProvider, error) {
if cfg.GoogleServiceAccount == "" {
return nil, nil
Expand Down
9 changes: 9 additions & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,15 @@ type CredentialsStrategy interface {
Configure(context.Context, *Config) (credentials.CredentialsProvider, error)
}

// CloudScoped is an optional interface that a [CredentialsStrategy] can
// implement to declare which cloud it supports. When implemented, the
// credentials chain skips this strategy in auto-detect mode if the configured
// host's cloud does not match. Cloud filtering is bypassed when
// [Config.AuthType] is explicitly set.
type CloudScoped interface {
Cloud() environment.Cloud
}

type Loader interface {
// Name is human-addressable representation of this config resolver
Name() string
Expand Down
Loading