Added auth_type for multi-cloud states (#1000)

Added optional `auth_type` provider conf to enforce specific auth type to be used in very rare cases, where a single Terraform state manages Databricks workspaces on more than one cloud and `More than one authorization method configured` error is a false positive.

Valid values are `pat`, `basic`, `azure-client-secret`, `azure-msi`, `azure-cli`, and `databricks-cli`.

Author: nfx

CHANGELOG.md

@@ -3,6 +3,13 @@
 ## 0.4.2
 
 * Added `DBC` format support for `databricks_notebook` ([#989](https://github.com/databrickslabs/terraform-provider-databricks/pull/989)). 
+* Added optional `auth_type` provider conf to enforce specific auth type to be used in very rare cases, where a single Terraform state manages Databricks workspaces on more than one cloud and `More than one authorization method configured` error is a false positive. Valid values are `pat`, `basic`, `azure-client-secret`, `azure-msi`, `azure-cli`, and `databricks-cli`.
+* Added automated documentation formatting with `make fmt-docs`, so that all HCL examples look consistent.
+* Increased codebase unit test coverage to 91% to improve stability.
+
+Updated dependency versions:
+
+* Bump github.com/hashicorp/terraform-plugin-sdk/v2 from 2.10.0 to 2.10.1 
 
 ## 0.4.1
 

common/azure_auth_test.go

@@ -374,7 +374,7 @@ func TestGetJWTProperty_Authenticate_Fail(t *testing.T) {
 		Host: "https://adb-1232.azuredatabricks.net",
 	}
 	_, err := client.GetAzureJwtProperty("tid")
-	require.EqualError(t, err, "cannot configure Azure CLI auth: "+
+	require.EqualError(t, err, "cannot configure azure-cli auth: "+
 		"Invoking Azure CLI failed with the following error: "+
 		"This is just a failing script.\n. "+
 		"Please check https://registry.terraform.io/providers/"+

common/client.go

@@ -61,6 +61,10 @@ type DatabricksClient struct {
 	AzureTenantID      string `name:"azure_tenant_id" env:"ARM_TENANT_ID" auth:"azure"`
 	AzurermEnvironment string `name:"azure_environment" env:"ARM_ENVIRONMENT"`
 
+	// When multiple auth attributes are available in the environment, use the auth type
+	// specified by this argument. This argument also holds currently selected auth.
+	AuthType string `name:"auth_type" auth:"-"`
+
 	// Azure Enviroment endpoints
 	AzureEnvironment *azure.Environment
 
@@ -233,28 +237,39 @@ func (c *DatabricksClient) Authenticate(ctx context.Context) error {
 		name      string
 	}
 	providers := []auth{
-		{c.configureWithDirectParams, "direct"},
-		{c.configureWithAzureClientSecret, "Azure Service Principal"},
-		{c.configureWithAzureManagedIdentity, "Azure MSI"},
-		{c.configureWithAzureCLI, "Azure CLI"},
-		{c.configureWithGoogleForAccountsAPI, "Databricks Account on GCP"},
-		{c.configureWithGoogleForWorkspace, "Databricks on GCP"},
-		{c.configureWithDatabricksCfg, "Databricks CLI"},
+		{c.configureWithPat, "pat"},
+		{c.configureWithBasicAuth, "basic"},
+		{c.configureWithAzureClientSecret, "azure-client-secret"},
+		{c.configureWithAzureManagedIdentity, "azure-msi"},
+		{c.configureWithAzureCLI, "azure-cli"},
+		{c.configureWithGoogleForAccountsAPI, "google-accounts"},
+		{c.configureWithGoogleForWorkspace, "google-workspace"},
+		{c.configureWithDatabricksCfg, "databricks-cli"},
 	}
 	// try configuring authentication with different methods
 	for _, auth := range providers {
+		if c.AuthType != "" && auth.name != c.AuthType {
+			// ignore other auth types if one is explicitly enforced
+			log.Printf("[INFO] Ignoring %s auth, because %s is preferred", auth.name, c.AuthType)
+			continue
+		}
 		authorizer, err := auth.configure(ctx)
 		if err != nil {
 			return c.niceAuthError(fmt.Sprintf("cannot configure %s auth: %s", auth.name, err))
 		}
 		if authorizer == nil {
 			continue
 		}
+		// even though this may complain about clear text loggin, passwords are replaced with `***`
 		log.Printf("[INFO] Configured %s auth: %s", auth.name, c.configDebugString()) // lgtm[go/clear-text-logging]
 		c.authVisitor = authorizer
+		c.AuthType = auth.name
 		c.fixHost()
 		return nil
 	}
+	if c.AuthType != "" {
+		return c.niceAuthError(fmt.Sprintf("cannot configure %s auth", c.AuthType))
+	}
 	if c.Host == "" && IsData.GetOrUnknown(ctx) == "yes" {
 		return c.niceAuthError("workspace is most likely not created yet, because the `host` " +
 			"is empty. Please add `depends_on = [databricks_mws_workspaces.this]` or " +
@@ -310,25 +325,21 @@ func (c *DatabricksClient) fixHost() {
 	}
 }
 
-func (c *DatabricksClient) configureWithDirectParams(ctx context.Context) (func(*http.Request) error, error) {
-	authType := "Bearer"
-	var needsHostBecause string
-	if c.Username != "" && c.Password != "" {
-		authType = "Basic"
-		needsHostBecause = "basic_auth"
-		c.Token = c.encodeBasicAuth(c.Username, c.Password)
-		log.Printf("[INFO] Using basic auth for user '%s'", c.Username)
-	} else if c.Token != "" {
-		needsHostBecause = "token"
-	}
-	if needsHostBecause != "" && c.Host == "" {
-		return nil, fmt.Errorf("host is empty, but is required by %s", needsHostBecause)
+func (c *DatabricksClient) configureWithPat(ctx context.Context) (func(*http.Request) error, error) {
+	if !(c.Token != "" && c.Host != "") {
+		return nil, nil
 	}
-	if c.Token == "" || c.Host == "" {
+	log.Printf("[INFO] Using directly configured PAT authentication")
+	return c.authorizer("Bearer", c.Token), nil
+}
+
+func (c *DatabricksClient) configureWithBasicAuth(ctx context.Context) (func(*http.Request) error, error) {
+	if !(c.Username != "" && c.Password != "" && c.Host != "") {
 		return nil, nil
 	}
-	log.Printf("[INFO] Using directly configured host+%s authentication", needsHostBecause)
-	return c.authorizer(authType, c.Token), nil
+	b64 := c.encodeBasicAuth(c.Username, c.Password)
+	log.Printf("[INFO] Using directly configured basic authentication")
+	return c.authorizer("Basic", b64), nil
 }
 
 func (c *DatabricksClient) configureWithDatabricksCfg(ctx context.Context) (func(r *http.Request) error, error) {
@@ -342,8 +353,8 @@ func (c *DatabricksClient) configureWithDatabricksCfg(ctx context.Context) (func
 	}
 	_, err = os.Stat(configFile)
 	if os.IsNotExist(err) {
-		log.Printf("[INFO] ~/.databrickscfg not found on current host")
 		// early return for non-configured machines
+		log.Printf("[INFO] %s not found on current host", configFile)
 		return nil, nil
 	}
 	cfg, err := ini.Load(configFile)

common/client_test.go

@@ -2,17 +2,15 @@ package common
 
 import (
 	"context"
+	"log"
 	"os"
+	"reflect"
 	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 )
 
-func AssertErrorStartsWith(t *testing.T, err error, message string) bool {
-	return assert.True(t, strings.HasPrefix(err.Error(), message), err.Error())
-}
-
 func configureAndAuthenticate(dc *DatabricksClient) (*DatabricksClient, error) {
 	err := dc.Configure()
 	if err != nil {
@@ -21,22 +19,29 @@ func configureAndAuthenticate(dc *DatabricksClient) (*DatabricksClient, error) {
 	return dc, dc.Authenticate(context.Background())
 }
 
+func failsToAuthenticateWith(t *testing.T, dc *DatabricksClient, message string) {
+	_, err := configureAndAuthenticate(dc)
+	if dc.AuthType != "" {
+		log.Printf("[INFO] Auth is: %s", dc.AuthType)
+	}
+	if assert.NotNil(t, err, "expected to have error: %s", message) {
+		assert.True(t, strings.HasPrefix(err.Error(), message), err.Error())
+	}
+}
+
 func TestDatabricksClientConfigure_Nothing(t *testing.T) {
 	defer CleanupEnvironment()()
 	os.Setenv("PATH", "testdata:/bin")
-
-	_, err := configureAndAuthenticate(&DatabricksClient{})
-	AssertErrorStartsWith(t, err, "authentication is not configured for provider")
+	failsToAuthenticateWith(t, &DatabricksClient{},
+		"authentication is not configured for provider")
 }
 
 func TestDatabricksClientConfigure_BasicAuth_NoHost(t *testing.T) {
-	dc, err := configureAndAuthenticate(&DatabricksClient{
+	defer CleanupEnvironment()()
+	failsToAuthenticateWith(t, &DatabricksClient{
 		Username: "foo",
 		Password: "bar",
-	})
-
-	AssertErrorStartsWith(t, err, "cannot configure direct auth: host is empty, but is required by basic_auth")
-	assert.Equal(t, "Zm9vOmJhcg==", dc.Token)
+	}, "authentication is not configured for provider.")
 }
 
 func TestDatabricksClientConfigure_BasicAuth(t *testing.T) {
@@ -45,103 +50,101 @@ func TestDatabricksClientConfigure_BasicAuth(t *testing.T) {
 		Username: "foo",
 		Password: "bar",
 	})
-
-	assert.Equal(t, "Zm9vOmJhcg==", dc.Token)
 	assert.NoError(t, err)
+	assert.Equal(t, "basic", dc.AuthType)
 }
 
 func TestDatabricksClientConfigure_HostWithoutScheme(t *testing.T) {
 	dc, err := configureAndAuthenticate(&DatabricksClient{
 		Host:  "localhost:443",
 		Token: "...",
 	})
-
+	assert.NoError(t, err)
+	assert.Equal(t, "pat", dc.AuthType)
 	assert.Equal(t, "...", dc.Token)
 	assert.Equal(t, "https://localhost:443", dc.Host)
-	assert.NoError(t, err)
 }
 
 func TestDatabricksClientConfigure_Token_NoHost(t *testing.T) {
-	dc, err := configureAndAuthenticate(&DatabricksClient{
+	defer CleanupEnvironment()()
+	failsToAuthenticateWith(t, &DatabricksClient{
 		Token: "dapi345678",
-	})
-
-	AssertErrorStartsWith(t, err, "cannot configure direct auth: host is empty, but is required by token")
-	assert.Equal(t, "dapi345678", dc.Token)
+	}, "authentication is not configured for provider.")
 }
 
 func TestDatabricksClientConfigure_HostTokensTakePrecedence(t *testing.T) {
-	_, err := configureAndAuthenticate(&DatabricksClient{
+	dc, err := configureAndAuthenticate(&DatabricksClient{
 		Host:       "foo",
 		Token:      "connfigured",
 		ConfigFile: "testdata/.databrickscfg",
 	})
 	assert.NoError(t, err)
+	assert.Equal(t, "pat", dc.AuthType)
 }
 
 func TestDatabricksClientConfigure_BasicAuthTakePrecedence(t *testing.T) {
 	dc, err := configureAndAuthenticate(&DatabricksClient{
 		Host:       "foo",
-		Token:      "connfigured",
+		Token:      "configured",
 		Username:   "foo",
 		Password:   "bar",
 		ConfigFile: "testdata/.databrickscfg",
 	})
 	assert.NoError(t, err)
-	assert.Equal(t, "Zm9vOmJhcg==", dc.Token)
+	assert.Equal(t, "pat", dc.AuthType)
+	assert.Equal(t, "configured", dc.Token)
 }
 
 func TestDatabricksClientConfigure_ConfigRead(t *testing.T) {
 	dc, err := configureAndAuthenticate(&DatabricksClient{
 		ConfigFile: "testdata/.databrickscfg",
 	})
 	assert.NoError(t, err)
+	assert.Equal(t, "databricks-cli", dc.AuthType)
 	assert.Equal(t, "PT0+IC9kZXYvdXJhbmRvbSA8PT0KYFZ", dc.Token)
 }
 
 func TestDatabricksClientConfigure_NoHostGivesError(t *testing.T) {
-	_, err := configureAndAuthenticate(&DatabricksClient{
+	failsToAuthenticateWith(t, &DatabricksClient{
 		Token:      "connfigured",
 		ConfigFile: "testdata/.databrickscfg",
 		Profile:    "nohost",
-	})
-	assert.Error(t, err)
+	}, "cannot configure databricks-cli auth: config file "+
+		"testdata/.databrickscfg is corrupt: cannot find host in nohost profile.")
 }
 
 func TestDatabricksClientConfigure_NoTokenGivesError(t *testing.T) {
-	_, err := configureAndAuthenticate(&DatabricksClient{
+	failsToAuthenticateWith(t, &DatabricksClient{
 		Token:      "connfigured",
 		ConfigFile: "testdata/.databrickscfg",
 		Profile:    "notoken",
-	})
-	assert.Error(t, err)
+	}, "cannot configure databricks-cli auth: config file "+
+		"testdata/.databrickscfg is corrupt: cannot find token in notoken profile.")
 }
 
 func TestDatabricksClientConfigure_InvalidProfileGivesError(t *testing.T) {
-	_, err := configureAndAuthenticate(&DatabricksClient{
+	failsToAuthenticateWith(t, &DatabricksClient{
 		Token:      "connfigured",
 		ConfigFile: "testdata/.databrickscfg",
 		Profile:    "invalidhost",
-	})
-	assert.Error(t, err)
+	}, "cannot configure databricks-cli auth: testdata/.databrickscfg "+
+		"has no invalidhost profile configured")
 }
 
 func TestDatabricksClientConfigure_MissingFile(t *testing.T) {
-	_, err := configureAndAuthenticate(&DatabricksClient{
+	failsToAuthenticateWith(t, &DatabricksClient{
 		Token:      "connfigured",
 		ConfigFile: "testdata/.invalid file",
 		Profile:    "invalidhost",
-	})
-	assert.Error(t, err)
+	}, "authentication is not configured for provider.")
 }
 
 func TestDatabricksClientConfigure_InvalidConfigFilePath(t *testing.T) {
-	_, err := configureAndAuthenticate(&DatabricksClient{
+	failsToAuthenticateWith(t, &DatabricksClient{
 		Token:      "connfigured",
-		ConfigFile: "testdata/policy01.json",
+		ConfigFile: "testdata/az",
 		Profile:    "invalidhost",
-	})
-	assert.Error(t, err)
+	}, "cannot configure databricks-cli auth: cannot parse config file")
 }
 
 func TestDatabricksClient_FormatURL(t *testing.T) {
@@ -151,7 +154,7 @@ func TestDatabricksClient_FormatURL(t *testing.T) {
 
 func TestClientAttributes(t *testing.T) {
 	ca := ClientAttributes()
-	assert.Len(t, ca, 19)
+	assert.Len(t, ca, 20)
 }
 
 func TestDatabricksClient_Authenticate(t *testing.T) {
@@ -223,7 +226,10 @@ func TestClientForHostAuthError(t *testing.T) {
 		Profile:    "notoken",
 	}
 	_, err := c.ClientForHost(context.Background(), "https://e2-workspace.cloud.databricks.com/")
-	AssertErrorStartsWith(t, err, "cannot authenticate parent client: cannot configure direct auth")
+	if assert.NotNil(t, err) {
+		assert.True(t, strings.HasPrefix(err.Error(),
+			"cannot authenticate parent client: cannot configure databricks-cli auth"), err.Error())
+	}
 }
 
 func TestDatabricksCliCouldNotFindHomeDir(t *testing.T) {
@@ -237,7 +243,10 @@ func TestDatabricksCliCouldNotParseIni(t *testing.T) {
 	_, err := (&DatabricksClient{
 		ConfigFile: "testdata/az",
 	}).configureWithDatabricksCfg(context.Background())
-	AssertErrorStartsWith(t, err, "cannot parse config file: key-value delimiter not found")
+	if assert.NotNil(t, err) {
+		assert.True(t, strings.HasPrefix(err.Error(),
+			"cannot parse config file: key-value delimiter not found"), err.Error())
+	}
 }
 
 func TestDatabricksCliWrongProfile(t *testing.T) {
@@ -274,3 +283,17 @@ func TestDatabricksBasicAuth(t *testing.T) {
 	assert.Equal(t, "abc", c.Username)
 	assert.Equal(t, "bcd", c.Password)
 }
+
+func TestDatabricksClientConfigure_NonsenseAuth(t *testing.T) {
+	defer CleanupEnvironment()()
+	failsToAuthenticateWith(t, &DatabricksClient{
+		AuthType: "nonsense",
+	}, "cannot configure nonsense auth.")
+}
+
+func TestConfigAttributeSetNonsense(t *testing.T) {
+	err := (&ConfigAttribute{
+		Kind: reflect.Chan,
+	}).Set(&DatabricksClient{}, 1)
+	assert.EqualError(t, err, "cannot set  of unknown type Chan")
+}

common/resource.go

@@ -161,7 +161,7 @@ func makeEmptyBlockSuppressFunc(name string) func(k, old, new string, d *schema.
 	re := MustCompileKeyRE(name)
 	return func(k, old, new string, d *schema.ResourceData) bool {
 		if re.Match([]byte(name)) && old == "1" && new == "0" {
-			log.Printf("[DEBUG] Suppressing diff for name=%s k=%#v patform=%#v config=%#v", name, k, old, new)
+			log.Printf("[DEBUG] Suppressing diff for name=%s k=%#v platform=%#v config=%#v", name, k, old, new)
 			return true
 		}
 		return false

docs/index.md

@@ -176,6 +176,7 @@ Alternatively, you can provide this value as an environment variable `DATABRICKS
 * `profile` - (optional) Connection profile specified within ~/.databrickscfg. Please check [connection profiles section](https://docs.databricks.com/dev-tools/cli/index.html#connection-profiles) for more details. This field defaults to 
 `DEFAULT`.
 * `account_id` - (optional) Account Id that could be found in the bottom left corner of [Accounts Console](https://accounts.cloud.databricks.com/). Alternatively, you can provide this value as an environment variable `DATABRICKS_ACCOUNT_ID`. Only has effect when `host = "https://accounts.cloud.databricks.com/"` and currently used to provision account admins via [databricks_user](resources/user.md). In the future releases of the provider this property will also be used specify account for `databricks_mws_*` resources as well.
+* `auth_type` - (optional) enforce specific auth type to be used in very rare cases, where a single Terraform state manages Databricks workspaces on more than one cloud and `More than one authorization method configured` error is a false positive. Valid values are `pat`, `basic`, `azure-client-secret`, `azure-msi`, `azure-cli`, and `databricks-cli`.
 
 ## Special configurations for Azure
 

provider/provider.go

@@ -173,7 +173,7 @@ func configureDatabricksClient(ctx context.Context, d *schema.ResourceData) (int
 			authorizationMethodsUsed = append(authorizationMethodsUsed, name)
 		}
 	}
-	if len(authorizationMethodsUsed) > 1 {
+	if pc.AuthType == "" && len(authorizationMethodsUsed) > 1 {
 		sort.Strings(authorizationMethodsUsed)
 		return nil, diag.Errorf("More than one authorization method configured: %s",
 			strings.Join(authorizationMethodsUsed, " and "))