From d9272d016a8cf3c7c5b5f5c5041e2e4b933b6d39 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 27 Feb 2025 15:33:54 +0100 Subject: [PATCH 1/6] Add warning when variable interpolation is used for auth fields --- acceptance/bundle/debug/out.stderr.txt | 3 +- .../databricks.yml | 10 +++ .../interpolation-in-auth-field/output.txt | 22 ++++++ .../interpolation-in-auth-field/script | 3 + acceptance/bundle/variables/host/output.txt | 16 +++- .../validate/interpolation_in_auth_config.go | 78 +++++++++++++++++++ bundle/phases/initialize.go | 1 + libs/auth/env.go | 14 ++++ libs/auth/env_test.go | 68 ++++++++++++++++ libs/dyn/dynvar/ref.go | 4 + 10 files changed, 217 insertions(+), 2 deletions(-) create mode 100644 acceptance/bundle/validate/interpolation-in-auth-field/databricks.yml create mode 100644 acceptance/bundle/validate/interpolation-in-auth-field/output.txt create mode 100644 acceptance/bundle/validate/interpolation-in-auth-field/script create mode 100644 bundle/config/validate/interpolation_in_auth_config.go diff --git a/acceptance/bundle/debug/out.stderr.txt b/acceptance/bundle/debug/out.stderr.txt index e5867e00883..f141f563150 100644 --- a/acceptance/bundle/debug/out.stderr.txt +++ b/acceptance/bundle/debug/out.stderr.txt @@ -22,6 +22,7 @@ 10:07:59 Info: Phase: initialize pid=12345 mutator=initialize 10:07:59 Debug: Apply pid=12345 mutator=initialize mutator=seq 10:07:59 Debug: Apply pid=12345 mutator=initialize mutator=seq mutator=validate:AllResourcesHaveValues +10:07:59 Debug: Apply pid=12345 mutator=initialize mutator=seq mutator=validate:interpolation_in_auth_config 10:07:59 Debug: Apply pid=12345 mutator=initialize mutator=seq mutator=RewriteSyncPaths 10:07:59 Debug: Apply pid=12345 mutator=initialize mutator=seq mutator=SyncDefaultPath 10:07:59 Debug: Apply pid=12345 mutator=initialize mutator=seq mutator=SyncInferRoot @@ -72,7 +73,7 @@ 10:07:59 Debug: Apply pid=12345 mutator=initialize mutator=seq mutator=metadata.AnnotatePipelines 10:07:59 Debug: Apply pid=12345 mutator=initialize mutator=seq mutator=terraform.Initialize 10:07:59 Debug: Using Terraform from DATABRICKS_TF_EXEC_PATH at [TERRAFORM] pid=12345 mutator=initialize mutator=seq mutator=terraform.Initialize -10:07:59 Debug: Using Terraform CLI config from DATABRICKS_TF_CLI_CONFIG_FILE at [DATABRICKS_TF_CLI_CONFIG_FILE] pid=12345 mutator=initialize mutator=seq mutator=terraform.Initialize +10:07:59 Debug: DATABRICKS_TF_PROVIDER_VERSION as 1.62.0 does not match the current version 1.65.1, ignoring DATABRICKS_TF_CLI_CONFIG_FILE pid=12345 mutator=initialize mutator=seq mutator=terraform.Initialize 10:07:59 Debug: Environment variables for Terraform: ...redacted... pid=12345 mutator=initialize mutator=seq mutator=terraform.Initialize 10:07:59 Debug: Apply pid=12345 mutator=initialize mutator=seq mutator=scripts.postinit 10:07:59 Debug: No script defined for postinit, skipping pid=12345 mutator=initialize mutator=seq mutator=scripts.postinit diff --git a/acceptance/bundle/validate/interpolation-in-auth-field/databricks.yml b/acceptance/bundle/validate/interpolation-in-auth-field/databricks.yml new file mode 100644 index 00000000000..32a693996f0 --- /dev/null +++ b/acceptance/bundle/validate/interpolation-in-auth-field/databricks.yml @@ -0,0 +1,10 @@ +bundle: + name: interpolation-in-auth-field + +variables: + bar: + default: "12345" + +workspace: + client_id: ${var.bar} + auth_type: ${bundle.name} diff --git a/acceptance/bundle/validate/interpolation-in-auth-field/output.txt b/acceptance/bundle/validate/interpolation-in-auth-field/output.txt new file mode 100644 index 00000000000..aa53adbea81 --- /dev/null +++ b/acceptance/bundle/validate/interpolation-in-auth-field/output.txt @@ -0,0 +1,22 @@ +Warning: Variable interpolation is not supported for fields that configure authentication + at workspace.auth_type + in databricks.yml:10:14 + +Interpolation is not supported for the field workspace.auth_type. Please set +the DATABRICKS_AUTH_TYPE environment variable if you wish to configure this field at runtime. + +Warning: Variable interpolation is not supported for fields that configure authentication + at workspace.client_id + in databricks.yml:9:14 + +Interpolation is not supported for the field workspace.client_id. Please set +the DATABRICKS_CLIENT_ID environment variable if you wish to configure this field at runtime. + +Error: failed during request visitor: default auth: cannot configure default credentials, please check https://docs.databricks.com/en/dev-tools/auth.html#databricks-client-unified-authentication to configure credentials for your preferred authentication method. Config: host=[DATABRICKS_URL], client_id=${var.bar}, databricks_cli_path=[CLI]. Env: DATABRICKS_HOST, DATABRICKS_CLI_PATH + +Name: interpolation-in-auth-field +Target: default + +Found 1 error and 2 warnings + +Exit code: 1 diff --git a/acceptance/bundle/validate/interpolation-in-auth-field/script b/acceptance/bundle/validate/interpolation-in-auth-field/script new file mode 100644 index 00000000000..dfa6994ee0f --- /dev/null +++ b/acceptance/bundle/validate/interpolation-in-auth-field/script @@ -0,0 +1,3 @@ +export DATABRICKS_TOKEN="" + +$CLI bundle validate diff --git a/acceptance/bundle/variables/host/output.txt b/acceptance/bundle/variables/host/output.txt index df0a4527a58..eef4a08bf8f 100644 --- a/acceptance/bundle/variables/host/output.txt +++ b/acceptance/bundle/variables/host/output.txt @@ -1,5 +1,12 @@ >>> errcode [CLI] bundle validate -o json +Warning: Variable interpolation is not supported for fields that configure authentication + at workspace.host + in databricks.yml:10:9 + +Interpolation is not supported for the field workspace.host. Please set +the DATABRICKS_HOST environment variable if you wish to configure this field at runtime. + Error: failed during request visitor: parse "https://${var.host}": invalid character "{" in host name { @@ -27,6 +34,13 @@ Error: failed during request visitor: parse "https://${var.host}": invalid chara Exit code: 1 >>> errcode [CLI] bundle validate +Warning: Variable interpolation is not supported for fields that configure authentication + at workspace.host + in databricks.yml:10:9 + +Interpolation is not supported for the field workspace.host. Please set +the DATABRICKS_HOST environment variable if you wish to configure this field at runtime. + Error: failed during request visitor: parse "https://${var.host}": invalid character "{" in host name Name: host @@ -34,6 +48,6 @@ Target: default Workspace: Host: ${var.host} -Found 1 error +Found 1 error and 1 warning Exit code: 1 diff --git a/bundle/config/validate/interpolation_in_auth_config.go b/bundle/config/validate/interpolation_in_auth_config.go new file mode 100644 index 00000000000..db47940faf8 --- /dev/null +++ b/bundle/config/validate/interpolation_in_auth_config.go @@ -0,0 +1,78 @@ +package validate + +import ( + "context" + "fmt" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/auth" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/dynvar" +) + +type noInterpolationInAuthConfig struct{} + +func NoInterpolationInAuthConfig() bundle.Mutator { + return &noInterpolationInAuthConfig{} +} + +func (f *noInterpolationInAuthConfig) Name() string { + return "validate:interpolation_in_auth_config" +} + +func (f *noInterpolationInAuthConfig) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + authFields := []string{ + // Generic attributes. + "host", "profile", "auth_type", "metadata_service_url", + + // OAuth specific attributes. + "client_id", + + // Google specific attributes. + "google_service_account", + + // Azure specific attributes. + "azure_resource_id", "azure_use_msi", "azure_client_id", "azure_tenant_id", + "azure_environment", "azure_login_app_id", + } + + diags := diag.Diagnostics{} + + for _, fieldName := range authFields { + p := dyn.NewPath(dyn.Key("workspace"), dyn.Key(fieldName)) + v, err := dyn.GetByPath(b.Config.Value(), p) + if dyn.IsNoSuchKeyError(err) { + continue + } + if err != nil { + return diag.FromErr(err) + } + + // If the field is not defined or empty, skip it. + if v.Kind() == dyn.KindInvalid || v.Kind() == dyn.KindNil { + continue + } + + vv := v.MustString() + + // Check if the field contains interpolation. + if dynvar.ContainsVariableReference(vv) { + envVar, ok := auth.GetEnvFor(fieldName) + if !ok { + continue + } + + diags = append(diags, diag.Diagnostic{ + Severity: diag.Warning, + Summary: "Variable interpolation is not supported for fields that configure authentication", + Detail: fmt.Sprintf(`Interpolation is not supported for the field %s. Please set +the %s environment variable if you wish to configure this field at runtime.`, p.String(), envVar), + Locations: v.Locations(), + Paths: []dyn.Path{p}, + }) + } + } + + return diags +} diff --git a/bundle/phases/initialize.go b/bundle/phases/initialize.go index afd6def3f55..086ebaa2128 100644 --- a/bundle/phases/initialize.go +++ b/bundle/phases/initialize.go @@ -22,6 +22,7 @@ func Initialize() bundle.Mutator { "initialize", []bundle.Mutator{ validate.AllResourcesHaveValues(), + validate.NoInterpolationInAuthConfig(), // Update all path fields in the sync block to be relative to the bundle root path. mutator.RewriteSyncPaths(), diff --git a/libs/auth/env.go b/libs/auth/env.go index c58cc53e3f6..5c0d2129297 100644 --- a/libs/auth/env.go +++ b/libs/auth/env.go @@ -24,3 +24,17 @@ func Env(cfg *config.Config) map[string]string { return out } + +func GetEnvFor(name string) (string, bool) { + for _, attr := range config.ConfigAttributes { + if attr.Name != name { + continue + } + if len(attr.EnvVars) == 0 { + return "", false + } + return attr.EnvVars[0], true + } + + return "", false +} diff --git a/libs/auth/env_test.go b/libs/auth/env_test.go index be1cfc7acac..e682bfcc47d 100644 --- a/libs/auth/env_test.go +++ b/libs/auth/env_test.go @@ -40,3 +40,71 @@ func TestAuthEnv(t *testing.T) { out := Env(in) assert.Equal(t, expected, out) } + +func TestGetEnvFor(t *testing.T) { + tcases := []struct { + name string + expected string + }{ + { + name: "host", + expected: "DATABRICKS_HOST", + }, + { + name: "profile", + expected: "DATABRICKS_CONFIG_PROFILE", + }, + { + name: "auth_type", + expected: "DATABRICKS_AUTH_TYPE", + }, + { + name: "metadata_service_url", + expected: "DATABRICKS_METADATA_SERVICE_URL", + }, + { + name: "client_id", + expected: "DATABRICKS_CLIENT_ID", + }, + { + name: "google_service_account", + expected: "DATABRICKS_GOOGLE_SERVICE_ACCOUNT", + }, + { + name: "azure_workspace_resource_id", + expected: "DATABRICKS_AZURE_RESOURCE_ID", + }, + { + name: "azure_use_msi", + expected: "ARM_USE_MSI", + }, + { + name: "azure_client_id", + expected: "ARM_CLIENT_ID", + }, + { + name: "azure_tenant_id", + expected: "ARM_TENANT_ID", + }, + { + name: "azure_environment", + expected: "ARM_ENVIRONMENT", + }, + { + name: "azure_login_app_id", + expected: "DATABRICKS_AZURE_LOGIN_APP_ID", + }, + } + + for _, tcase := range tcases { + t.Run(tcase.name, func(t *testing.T) { + out, ok := GetEnvFor(tcase.name) + assert.True(t, ok) + assert.Equal(t, tcase.expected, out) + }) + } + + out, ok := GetEnvFor("notfound") + assert.False(t, ok) + assert.Empty(t, out) +} diff --git a/libs/dyn/dynvar/ref.go b/libs/dyn/dynvar/ref.go index ba397267ab1..21ec00fdaa0 100644 --- a/libs/dyn/dynvar/ref.go +++ b/libs/dyn/dynvar/ref.go @@ -76,6 +76,10 @@ func IsPureVariableReference(s string) bool { return len(s) > 0 && re.FindString(s) == s } +func ContainsVariableReference(s string) bool { + return re.MatchString(s) +} + // If s is a pure variable reference, this function returns the corresponding // dyn.Path. Otherwise, it returns false. func PureReferenceToPath(s string) (dyn.Path, bool) { From 79a0980b646325c7f49ff023991523f4a22d8868 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 27 Feb 2025 15:35:10 +0100 Subject: [PATCH 2/6] - --- acceptance/bundle/debug/out.stderr.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acceptance/bundle/debug/out.stderr.txt b/acceptance/bundle/debug/out.stderr.txt index f141f563150..96cee55991f 100644 --- a/acceptance/bundle/debug/out.stderr.txt +++ b/acceptance/bundle/debug/out.stderr.txt @@ -73,7 +73,7 @@ 10:07:59 Debug: Apply pid=12345 mutator=initialize mutator=seq mutator=metadata.AnnotatePipelines 10:07:59 Debug: Apply pid=12345 mutator=initialize mutator=seq mutator=terraform.Initialize 10:07:59 Debug: Using Terraform from DATABRICKS_TF_EXEC_PATH at [TERRAFORM] pid=12345 mutator=initialize mutator=seq mutator=terraform.Initialize -10:07:59 Debug: DATABRICKS_TF_PROVIDER_VERSION as 1.62.0 does not match the current version 1.65.1, ignoring DATABRICKS_TF_CLI_CONFIG_FILE pid=12345 mutator=initialize mutator=seq mutator=terraform.Initialize +10:07:59 Debug: Using Terraform CLI config from DATABRICKS_TF_CLI_CONFIG_FILE at [DATABRICKS_TF_CLI_CONFIG_FILE] pid=12345 mutator=initialize mutator=seq mutator=terraform.Initialize 10:07:59 Debug: Environment variables for Terraform: ...redacted... pid=12345 mutator=initialize mutator=seq mutator=terraform.Initialize 10:07:59 Debug: Apply pid=12345 mutator=initialize mutator=seq mutator=scripts.postinit 10:07:59 Debug: No script defined for postinit, skipping pid=12345 mutator=initialize mutator=seq mutator=scripts.postinit From a8bb9e511e7d8763aef6a822e12135eae0ad38de Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 27 Feb 2025 15:37:15 +0100 Subject: [PATCH 3/6] - --- bundle/config/validate/interpolation_in_auth_config.go | 1 - 1 file changed, 1 deletion(-) diff --git a/bundle/config/validate/interpolation_in_auth_config.go b/bundle/config/validate/interpolation_in_auth_config.go index db47940faf8..83d6c6ae146 100644 --- a/bundle/config/validate/interpolation_in_auth_config.go +++ b/bundle/config/validate/interpolation_in_auth_config.go @@ -49,7 +49,6 @@ func (f *noInterpolationInAuthConfig) Apply(ctx context.Context, b *bundle.Bundl return diag.FromErr(err) } - // If the field is not defined or empty, skip it. if v.Kind() == dyn.KindInvalid || v.Kind() == dyn.KindNil { continue } From a4d36b8ef182a83830e1a0b063614c9dd6d182c4 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 27 Feb 2025 15:39:21 +0100 Subject: [PATCH 4/6] better formatting --- libs/auth/env_test.go | 67 ++++++++++++------------------------------- 1 file changed, 19 insertions(+), 48 deletions(-) diff --git a/libs/auth/env_test.go b/libs/auth/env_test.go index e682bfcc47d..850110b6029 100644 --- a/libs/auth/env_test.go +++ b/libs/auth/env_test.go @@ -46,54 +46,25 @@ func TestGetEnvFor(t *testing.T) { name string expected string }{ - { - name: "host", - expected: "DATABRICKS_HOST", - }, - { - name: "profile", - expected: "DATABRICKS_CONFIG_PROFILE", - }, - { - name: "auth_type", - expected: "DATABRICKS_AUTH_TYPE", - }, - { - name: "metadata_service_url", - expected: "DATABRICKS_METADATA_SERVICE_URL", - }, - { - name: "client_id", - expected: "DATABRICKS_CLIENT_ID", - }, - { - name: "google_service_account", - expected: "DATABRICKS_GOOGLE_SERVICE_ACCOUNT", - }, - { - name: "azure_workspace_resource_id", - expected: "DATABRICKS_AZURE_RESOURCE_ID", - }, - { - name: "azure_use_msi", - expected: "ARM_USE_MSI", - }, - { - name: "azure_client_id", - expected: "ARM_CLIENT_ID", - }, - { - name: "azure_tenant_id", - expected: "ARM_TENANT_ID", - }, - { - name: "azure_environment", - expected: "ARM_ENVIRONMENT", - }, - { - name: "azure_login_app_id", - expected: "DATABRICKS_AZURE_LOGIN_APP_ID", - }, + // Generic attributes. + {"host", "DATABRICKS_HOST"}, + {"profile", "DATABRICKS_CONFIG_PROFILE"}, + {"auth_type", "DATABRICKS_AUTH_TYPE"}, + {"metadata_service_url", "DATABRICKS_METADATA_SERVICE_URL"}, + + // OAuth specific attributes. + {"client_id", "DATABRICKS_CLIENT_ID"}, + + // Google specific attributes. + {"google_service_account", "DATABRICKS_GOOGLE_SERVICE_ACCOUNT"}, + + // Azure specific attributes. + {"azure_workspace_resource_id", "DATABRICKS_AZURE_RESOURCE_ID"}, + {"azure_use_msi", "ARM_USE_MSI"}, + {"azure_client_id", "ARM_CLIENT_ID"}, + {"azure_tenant_id", "ARM_TENANT_ID"}, + {"azure_environment", "ARM_ENVIRONMENT"}, + {"azure_login_app_id", "DATABRICKS_AZURE_LOGIN_APP_ID"}, } for _, tcase := range tcases { From 8c513713c849b3fc882aab032e48e66cabbcb209 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 27 Feb 2025 15:51:29 +0100 Subject: [PATCH 5/6] remove new acc test --- .../databricks.yml | 10 --------- .../interpolation-in-auth-field/output.txt | 22 ------------------- .../interpolation-in-auth-field/script | 3 --- 3 files changed, 35 deletions(-) delete mode 100644 acceptance/bundle/validate/interpolation-in-auth-field/databricks.yml delete mode 100644 acceptance/bundle/validate/interpolation-in-auth-field/output.txt delete mode 100644 acceptance/bundle/validate/interpolation-in-auth-field/script diff --git a/acceptance/bundle/validate/interpolation-in-auth-field/databricks.yml b/acceptance/bundle/validate/interpolation-in-auth-field/databricks.yml deleted file mode 100644 index 32a693996f0..00000000000 --- a/acceptance/bundle/validate/interpolation-in-auth-field/databricks.yml +++ /dev/null @@ -1,10 +0,0 @@ -bundle: - name: interpolation-in-auth-field - -variables: - bar: - default: "12345" - -workspace: - client_id: ${var.bar} - auth_type: ${bundle.name} diff --git a/acceptance/bundle/validate/interpolation-in-auth-field/output.txt b/acceptance/bundle/validate/interpolation-in-auth-field/output.txt deleted file mode 100644 index aa53adbea81..00000000000 --- a/acceptance/bundle/validate/interpolation-in-auth-field/output.txt +++ /dev/null @@ -1,22 +0,0 @@ -Warning: Variable interpolation is not supported for fields that configure authentication - at workspace.auth_type - in databricks.yml:10:14 - -Interpolation is not supported for the field workspace.auth_type. Please set -the DATABRICKS_AUTH_TYPE environment variable if you wish to configure this field at runtime. - -Warning: Variable interpolation is not supported for fields that configure authentication - at workspace.client_id - in databricks.yml:9:14 - -Interpolation is not supported for the field workspace.client_id. Please set -the DATABRICKS_CLIENT_ID environment variable if you wish to configure this field at runtime. - -Error: failed during request visitor: default auth: cannot configure default credentials, please check https://docs.databricks.com/en/dev-tools/auth.html#databricks-client-unified-authentication to configure credentials for your preferred authentication method. Config: host=[DATABRICKS_URL], client_id=${var.bar}, databricks_cli_path=[CLI]. Env: DATABRICKS_HOST, DATABRICKS_CLI_PATH - -Name: interpolation-in-auth-field -Target: default - -Found 1 error and 2 warnings - -Exit code: 1 diff --git a/acceptance/bundle/validate/interpolation-in-auth-field/script b/acceptance/bundle/validate/interpolation-in-auth-field/script deleted file mode 100644 index dfa6994ee0f..00000000000 --- a/acceptance/bundle/validate/interpolation-in-auth-field/script +++ /dev/null @@ -1,3 +0,0 @@ -export DATABRICKS_TOKEN="" - -$CLI bundle validate From 5cc180b076c8dece5b2c30fa2ed938566589e555 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Thu, 27 Feb 2025 15:55:57 +0100 Subject: [PATCH 6/6] - --- acceptance/bundle/debug/out.stderr.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/acceptance/bundle/debug/out.stderr.txt b/acceptance/bundle/debug/out.stderr.txt index e763cb6ffa3..147b33a454e 100644 --- a/acceptance/bundle/debug/out.stderr.txt +++ b/acceptance/bundle/debug/out.stderr.txt @@ -17,6 +17,7 @@ 10:07:59 Debug: Apply pid=12345 mutator= 10:07:59 Info: Phase: initialize pid=12345 10:07:59 Debug: Apply pid=12345 mutator=validate:AllResourcesHaveValues +10:07:59 Debug: Apply pid=12345 mutator=validate:interpolation_in_auth_config 10:07:59 Debug: Apply pid=12345 mutator=RewriteSyncPaths 10:07:59 Debug: Apply pid=12345 mutator=SyncDefaultPath 10:07:59 Debug: Apply pid=12345 mutator=SyncInferRoot