Clarify empty host error. Fixes #836 (#847)

Author: nfx

common/client.go

@@ -178,24 +178,6 @@ func ClientAttributes() (attrs []ConfigAttribute) {
 	return
 }
 
-// envVariablesUsed returns coma-separated list of the used relevant variable names
-func envVariablesUsed() string {
-	names := []string{}
-	for _, attr := range ClientAttributes() {
-		if len(attr.EnvVars) == 0 {
-			continue
-		}
-		for _, envVar := range attr.EnvVars {
-			value := os.Getenv(envVar)
-			if value == "" {
-				continue
-			}
-			names = append(names, envVar)
-		}
-	}
-	return strings.Join(names, ", ")
-}
-
 // Configure client to work, optionally specifying configuration attributes used
 func (c *DatabricksClient) Configure(attrsUsed ...string) error {
 	c.configAttributesUsed = attrsUsed
@@ -224,20 +206,24 @@ func (c *DatabricksClient) Authenticate(ctx context.Context) error {
 	if c.authVisitor != nil {
 		return nil
 	}
-	authorizers := []func(context.Context) (func(*http.Request) error, error){
-		c.configureWithDirectParams,
-		c.configureWithAzureClientSecret,
-		c.configureWithAzureManagedIdentity,
-		c.configureWithAzureCLI,
-		c.configureWithGoogleForAccountsAPI,
-		c.configureWithGoogleForWorkspace,
-		c.configureWithDatabricksCfg,
+	type auth struct {
+		configure func(context.Context) (func(*http.Request) error, 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"},
 	}
 	// try configuring authentication with different methods
-	for _, authProvider := range authorizers {
-		authorizer, err := authProvider(ctx)
+	for _, auth := range providers {
+		authorizer, err := auth.configure(ctx)
 		if err != nil {
-			return c.niceError(fmt.Sprintf("cannot configure auth: %s", err))
+			return c.niceAuthError(fmt.Sprintf("cannot configure %s auth: %s", auth.name, err))
 		}
 		if authorizer == nil {
 			continue
@@ -246,21 +232,51 @@ func (c *DatabricksClient) Authenticate(ctx context.Context) error {
 		c.fixHost()
 		return nil
 	}
-	return c.niceError("authentication is not configured for provider.")
+	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 " +
+			"`depends_on = [azurerm_databricks_workspace.this]` to every data resource. See " +
+			"https://www.terraform.io/docs/language/resources/behavior.html more info")
+	}
+	return c.niceAuthError("authentication is not configured for provider.")
 }
 
-func (c *DatabricksClient) niceError(message string) error {
+func (c *DatabricksClient) niceAuthError(message string) error {
 	info := ""
 	if len(c.configAttributesUsed) > 0 {
-		// TODO: first show env vars and filter out the attrs after
-		info = fmt.Sprintf(" Attributes used: %s", strings.Join(c.configAttributesUsed, ", "))
-		envVars := envVariablesUsed()
-		if envVars != "" {
-			info = fmt.Sprintf("%s. Environment variables used: %s", info, envVars)
+		envs := []string{}
+		attrs := []string{}
+		usedAsEnv := map[string]bool{}
+		for _, attr := range ClientAttributes() {
+			if len(attr.EnvVars) == 0 {
+				continue
+			}
+			for _, envVar := range attr.EnvVars {
+				value := os.Getenv(envVar)
+				if value == "" {
+					continue
+				}
+				usedAsEnv[attr.Name] = true
+				envs = append(envs, envVar)
+			}
+		}
+		for _, attr := range c.configAttributesUsed {
+			if usedAsEnv[attr] {
+				continue
+			}
+			attrs = append(attrs, attr)
+		}
+		infos := []string{}
+		if len(attrs) > 0 {
+			infos = append(infos, fmt.Sprintf("Attributes used: %s", strings.Join(attrs, ", ")))
+		}
+		if len(envs) > 0 {
+			infos = append(infos, fmt.Sprintf("Environment variables used: %s", strings.Join(envs, ", ")))
 		}
+		info = ". " + strings.Join(infos, ". ")
 	}
 	docUrl := "https://registry.terraform.io/providers/databrickslabs/databricks/latest/docs#authentication"
-	return fmt.Errorf("%s%s Please check %s for details", message, info, docUrl)
+	return fmt.Errorf("%s%s. Please check %s for details", message, info, docUrl)
 }
 
 func (c *DatabricksClient) fixHost() {

common/client_test.go

@@ -35,7 +35,7 @@ func TestDatabricksClientConfigure_BasicAuth_NoHost(t *testing.T) {
 		Password: "bar",
 	})
 
-	AssertErrorStartsWith(t, err, "cannot configure auth: host is empty, but is required by basic_auth")
+	AssertErrorStartsWith(t, err, "cannot configure direct auth: host is empty, but is required by basic_auth")
 	assert.Equal(t, "Zm9vOmJhcg==", dc.Token)
 }
 
@@ -66,7 +66,7 @@ func TestDatabricksClientConfigure_Token_NoHost(t *testing.T) {
 		Token: "dapi345678",
 	})
 
-	AssertErrorStartsWith(t, err, "cannot configure auth: host is empty, but is required by token")
+	AssertErrorStartsWith(t, err, "cannot configure direct auth: host is empty, but is required by token")
 	assert.Equal(t, "dapi345678", dc.Token)
 }
 
@@ -144,15 +144,6 @@ func TestDatabricksClientConfigure_InvalidConfigFilePath(t *testing.T) {
 	assert.Error(t, err)
 }
 
-// func TestDatabricksClientConfigure_InvalidHome(t *testing.T) {
-// 	defer CleanupEnvironment()()
-// 	os.Setenv("HOME", "whatever")
-// 	_, err := configureAndAuthenticate(&DatabricksClient{
-// 		Profile: "invalidhost",
-// 	})
-// 	assert.EqualError(t, err, ".")
-// }
-
 func TestDatabricksClient_FormatURL(t *testing.T) {
 	client := DatabricksClient{Host: "https://some.host"}
 	assert.Equal(t, "https://some.host/#job/123", client.FormatURL("#job/123"))
@@ -163,15 +154,31 @@ func TestClientAttributes(t *testing.T) {
 	assert.Len(t, ca, 25)
 }
 
-func TestEnvVarsUsed(t *testing.T) {
-	ResetCommonEnvironmentClient()
+func TestDatabricksClient_Authenticate(t *testing.T) {
+	defer CleanupEnvironment()()
+	dc := DatabricksClient{}
+	err := dc.Configure("account_id", "username", "password")
+	os.Setenv("DATABRICKS_PASSWORD", ".")
+	assert.NoError(t, err)
+	err = dc.Authenticate(context.WithValue(context.Background(), IsData, "yes"))
+	assert.EqualError(t, err, "workspace is most likely not created yet, because the `host` is empty. "+
+		"Please add `depends_on = [databricks_mws_workspaces.this]` or `depends_on = [azurerm_databricks"+
+		"_workspace.this]` to every data resource. See https://www.terraform.io/docs/language/resources/behavior.html more info. "+
+		"Attributes used: account_id, username. Environment variables used: DATABRICKS_PASSWORD. "+
+		"Please check https://registry.terraform.io/providers/databrickslabs/databricks/latest/docs#authentication for details")
+}
+
+func TestDatabricksClient_AuthenticateAzure(t *testing.T) {
 	defer CleanupEnvironment()()
-	os.Setenv("SUPER_SECRET", ".")
-	os.Setenv("DATABRICKS_HOST", ".")
-	os.Setenv("ARM_SUBSCRIPTION_ID", ".")
-	os.Setenv("DATABRICKS_ACCOUNT_ID", ".")
-	os.Setenv("DATABRICKS_GOOGLE_SERVICE_ACCOUNT", ".")
-
-	names := envVariablesUsed()
-	assert.Equal(t, "DATABRICKS_HOST, DATABRICKS_ACCOUNT_ID, DATABRICKS_GOOGLE_SERVICE_ACCOUNT, ARM_SUBSCRIPTION_ID", names)
+	os.Setenv("ARM_CLIENT_SECRET", ".")
+	os.Setenv("ARM_CLIENT_ID", ".")
+	dc := DatabricksClient{}
+	err := dc.Configure("azure_client_id", "azure_client_secret")
+	assert.NoError(t, err)
+	err = dc.Authenticate(context.WithValue(context.Background(), IsData, "yes"))
+	assert.EqualError(t, err, "workspace is most likely not created yet, because the `host` is empty. "+
+		"Please add `depends_on = [databricks_mws_workspaces.this]` or `depends_on = [azurerm_databricks"+
+		"_workspace.this]` to every data resource. See https://www.terraform.io/docs/language/resources/"+
+		"behavior.html more info. Environment variables used: ARM_CLIENT_SECRET, ARM_CLIENT_ID. "+
+		"Please check https://registry.terraform.io/providers/databrickslabs/databricks/latest/docs#authentication for details")
 }

common/context.go

@@ -11,35 +11,41 @@ import (
 // AddContextToAllResources ...
 func AddContextToAllResources(p *schema.Provider, prefix string) {
 	for k, r := range p.DataSourcesMap {
-		k = strings.ReplaceAll(k, prefix+"_", "")
-		addContextToResource(k, r)
+		name := strings.ReplaceAll(k, prefix+"_", "")
+		wrap := op(r.ReadContext).addContext(ResourceName, name).addContext(IsData, "yes")
+		r.ReadContext = schema.ReadContextFunc(wrap)
 	}
 	for k, r := range p.ResourcesMap {
 		k = strings.ReplaceAll(k, prefix+"_", "")
 		addContextToResource(k, r)
 	}
 }
 
+// any of TF resource CRUD operation, that may need context enhancement
+type op func(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics
+
+// wrap operation invokations with additional context
+func (f op) addContext(k contextKey, v string) op {
+	return func(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
+		ctx = context.WithValue(ctx, k, v)
+		return f(ctx, d, m)
+	}
+}
+
 func addContextToResource(name string, r *schema.Resource) {
+	addName := func(a op) func(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
+		return a.addContext(ResourceName, name)
+	}
 	if r.CreateContext != nil {
-		r.CreateContext = addContextToStage(name, r.CreateContext)
+		r.CreateContext = addName(op(r.CreateContext))
 	}
 	if r.ReadContext != nil {
-		r.ReadContext = addContextToStage(name, r.ReadContext)
+		r.ReadContext = addName(op(r.ReadContext))
 	}
 	if r.UpdateContext != nil {
-		r.UpdateContext = addContextToStage(name, r.UpdateContext)
+		r.UpdateContext = addName(op(r.UpdateContext))
 	}
 	if r.DeleteContext != nil {
-		r.DeleteContext = addContextToStage(name, r.DeleteContext)
-	}
-}
-
-func addContextToStage(name string,
-	f func(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics) func(
-	ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
-	return func(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {
-		ctx = context.WithValue(ctx, ResourceName, name)
-		return f(ctx, d, m)
+		r.DeleteContext = addName(op(r.DeleteContext))
 	}
 }

common/version.go

@@ -10,6 +10,8 @@ var (
 	Provider contextKey = 2
 	// Current is the current name of integration test
 	Current contextKey = 3
+	// If current resource is data
+	IsData contextKey = 4
 )
 
 type contextKey int

provider/provider_test.go

@@ -119,7 +119,7 @@ func TestConfig_TokenEnv(t *testing.T) {
 		env: map[string]string{
 			"DATABRICKS_TOKEN": "x",
 		},
-		assertError: "cannot configure auth: host is empty, but is required by token",
+		assertError: "cannot configure direct auth: host is empty, but is required by token. Environment variables used: DATABRICKS_TOKEN",
 	}.apply(t)
 }
 
@@ -151,7 +151,9 @@ func TestConfig_UserPasswordEnv(t *testing.T) {
 			"DATABRICKS_USERNAME": "x",
 			"DATABRICKS_PASSWORD": "x",
 		},
-		assertError: "cannot configure auth: host is empty, but is required by basic_auth",
+		assertError: "cannot configure direct auth: host is empty, but is required by basic_auth. " +
+			"Environment variables used: DATABRICKS_USERNAME, DATABRICKS_PASSWORD. " +
+			"Please check https://registry.terraform.io/providers/databrickslabs/databricks/latest/docs#authentication for details",
 		assertToken: "x",
 		assertHost:  "https://x",
 	}.apply(t)
@@ -254,7 +256,7 @@ func TestConfig_PatFromDatabricksCfg_NohostProfile(t *testing.T) {
 			"HOME":                      "../common/testdata",
 			"DATABRICKS_CONFIG_PROFILE": "nohost",
 		},
-		assertError: "cannot configure auth: config file ../common/testdata/.databrickscfg is corrupt: cannot find host in nohost profile",
+		assertError: "cannot configure Databricks CLI auth: config file ../common/testdata/.databrickscfg is corrupt: cannot find host in nohost profile",
 	}.apply(t)
 }
 
@@ -307,7 +309,7 @@ func TestConfig_AzureCliHost_Fail(t *testing.T) {
 			"HOME": "../common/testdata",
 			"FAIL": "yes",
 		},
-		assertError: "cannot configure auth: Invoking Azure CLI failed with the following error: This is just a failing script.",
+		assertError: "cannot configure Azure CLI auth: Invoking Azure CLI failed with the following error: This is just a failing script.",
 	}.apply(t)
 }
 
@@ -319,7 +321,7 @@ func TestConfig_AzureCliHost_AzNotInstalled(t *testing.T) {
 			"PATH": "whatever",
 			"HOME": "../common/testdata",
 		},
-		assertError: "cannot configure auth: most likely Azure CLI is not installed.",
+		assertError: "cannot configure Azure CLI auth: most likely Azure CLI is not installed.",
 	}.apply(t)
 }
 
@@ -407,7 +409,7 @@ func TestConfig_CorruptConfig(t *testing.T) {
 		env: map[string]string{
 			"HOME": "../common/testdata/corrupt",
 		},
-		assertError: "cannot configure auth: ../common/testdata/corrupt/.databrickscfg has no DEFAULT profile configured",
+		assertError: "cannot configure Databricks CLI auth: ../common/testdata/corrupt/.databrickscfg has no DEFAULT profile configured",
 	}.apply(t)
 }