Add warning when variable interpolation is used for auth fields (#2399)

## Changes
This PR adds a warning which gives users clear guidance when they try to
use variable interpolation for an auth field.

## Tests
Modify existing acceptance test.
This commit is contained in:
shreyas-goenka 2025-02-27 21:28:48 +05:30 committed by GitHub
parent eb57dbd844
commit bc299cafb8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 151 additions and 1 deletions

View File

@ -17,6 +17,7 @@
10:07:59 Debug: Apply pid=12345 mutator=<func>
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

View File

@ -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

View File

@ -0,0 +1,77 @@
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 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
}

View File

@ -26,6 +26,7 @@ func Initialize(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
return bundle.ApplySeq(ctx, b,
validate.AllResourcesHaveValues(),
validate.NoInterpolationInAuthConfig(),
// Update all path fields in the sync block to be relative to the bundle root path.
mutator.RewriteSyncPaths(),

View File

@ -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
}

View File

@ -40,3 +40,42 @@ func TestAuthEnv(t *testing.T) {
out := Env(in)
assert.Equal(t, expected, out)
}
func TestGetEnvFor(t *testing.T) {
tcases := []struct {
name string
expected string
}{
// 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 {
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)
}

View File

@ -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) {