Merge remote-tracking branch 'origin' into set-config-used-bundle

acceptance/acceptance_test.go

@@ -3,6 +3,7 @@ package acceptance_test
 import (
 	"context"
 	"errors"
+	"flag"
 	"fmt"
 	"io"
 	"os"
@@ -23,7 +24,22 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-var KeepTmp = os.Getenv("KEEP_TMP") != ""
+var KeepTmp bool
+
+// In order to debug CLI running under acceptance test, set this to full subtest name, e.g. "bundle/variables/empty"
+// Then install your breakpoints and click "debug test" near TestAccept in VSCODE.
+// example: var singleTest = "bundle/variables/empty"
+var singleTest = ""
+
+// If enabled, instead of compiling and running CLI externally, we'll start in-process server that accepts and runs
+// CLI commands. The $CLI in test scripts is a helper that just forwards command-line arguments to this server (see bin/callserver.py).
+// Also disables parallelism in tests.
+var InprocessMode bool
+
+func init() {
+	flag.BoolVar(&InprocessMode, "inprocess", singleTest != "", "Run CLI in the same process as test (for debugging)")
+	flag.BoolVar(&KeepTmp, "keeptmp", false, "Do not delete TMP directory after run")
+}
 
 const (
 	EntryPointScript = "script"
@@ -38,6 +54,23 @@ var Scripts = map[string]bool{
 }
 
 func TestAccept(t *testing.T) {
+	testAccept(t, InprocessMode, "")
+}
+
+func TestInprocessMode(t *testing.T) {
+	if InprocessMode {
+		t.Skip("Already tested by TestAccept")
+	}
+	if runtime.GOOS == "windows" {
+		// -  catalogs                               A catalog is the first layer of Unity Catalog’s three-level namespace.
+		// +  catalogs                               A catalog is the first layer of Unity Catalog<6F>s three-level namespace.
+		t.Skip("Fails on CI on unicode characters")
+	}
+	require.NotZero(t, testAccept(t, true, "help"))
+}
+
+func testAccept(t *testing.T, InprocessMode bool, singleTest string) int {
+	repls := testdiff.ReplacementsContext{}
 	cwd, err := os.Getwd()
 	require.NoError(t, err)
 
@@ -50,16 +83,22 @@ func TestAccept(t *testing.T) {
 		t.Logf("Writing coverage to %s", coverDir)
 	}
 
-	execPath := BuildCLI(t, cwd, coverDir)
-	// $CLI is what test scripts are using
+	execPath := ""
+
+	if InprocessMode {
+		cmdServer := StartCmdServer(t)
+		t.Setenv("CMD_SERVER_URL", cmdServer.URL)
+		execPath = filepath.Join(cwd, "bin", "callserver.py")
+	} else {
+		execPath = BuildCLI(t, cwd, coverDir)
+	}
+
 	t.Setenv("CLI", execPath)
+	repls.Set(execPath, "$CLI")
 
 	// Make helper scripts available
 	t.Setenv("PATH", fmt.Sprintf("%s%c%s", filepath.Join(cwd, "bin"), os.PathListSeparator, os.Getenv("PATH")))
 
-	repls := testdiff.ReplacementsContext{}
-	repls.Set(execPath, "$CLI")
-
 	tempHomeDir := t.TempDir()
 	repls.Set(tempHomeDir, "$TMPHOME")
 	t.Logf("$TMPHOME=%v", tempHomeDir)
@@ -95,13 +134,25 @@ func TestAccept(t *testing.T) {
 	testDirs := getTests(t)
 	require.NotEmpty(t, testDirs)
 
+	if singleTest != "" {
+		testDirs = slices.DeleteFunc(testDirs, func(n string) bool {
+			return n != singleTest
+		})
+		require.NotEmpty(t, testDirs, "singleTest=%#v did not match any tests\n%#v", singleTest, testDirs)
+	}
+
 	for _, dir := range testDirs {
 		testName := strings.ReplaceAll(dir, "\\", "/")
 		t.Run(testName, func(t *testing.T) {
-			t.Parallel()
+			if !InprocessMode {
+				t.Parallel()
+			}
+
 			runTest(t, dir, coverDir, repls.Clone())
 		})
 	}
+
+	return len(testDirs)
 }
 
 func getTests(t *testing.T) []string {
@@ -137,12 +188,12 @@ func runTest(t *testing.T, dir, coverDir string, repls testdiff.ReplacementsCont
 		tmpDir = t.TempDir()
 	}
 
-	repls.Set("/private"+tmpDir, "$TMPDIR")
-	repls.Set("/private"+filepath.Dir(tmpDir), "$TMPPARENT")
-	repls.Set("/private"+filepath.Dir(filepath.Dir(tmpDir)), "$TMPGPARENT")
-	repls.Set(tmpDir, "$TMPDIR")
-	repls.Set(filepath.Dir(tmpDir), "$TMPPARENT")
-	repls.Set(filepath.Dir(filepath.Dir(tmpDir)), "$TMPGPARENT")
+	// Converts C:\Users\DENIS~1.BIL -> C:\Users\denis.bilenko
+	tmpDirEvalled, err1 := filepath.EvalSymlinks(tmpDir)
+	if err1 == nil && tmpDirEvalled != tmpDir {
+		repls.SetPathWithParents(tmpDirEvalled, "$TMPDIR")
+	}
+	repls.SetPathWithParents(tmpDir, "$TMPDIR")
 
 	scriptContents := readMergedScriptContents(t, dir)
 	testutil.WriteFile(t, filepath.Join(tmpDir, EntryPointScript), scriptContents)

acceptance/bin/callserver.py

@@ -0,0 +1,31 @@
+#!/usr/bin/env python3
+import sys
+import os
+import json
+import urllib.request
+from urllib.parse import urlencode
+
+env = {}
+for key, value in os.environ.items():
+    if len(value) > 10_000:
+        sys.stderr.write(f"Dropping key={key} value len={len(value)}\n")
+        continue
+    env[key] = value
+
+q = {
+    "args": " ".join(sys.argv[1:]),
+    "cwd": os.getcwd(),
+    "env": json.dumps(env),
+}
+
+url = os.environ["CMD_SERVER_URL"] + "/?" + urlencode(q)
+if len(url) > 100_000:
+    sys.exit("url too large")
+
+resp = urllib.request.urlopen(url)
+assert resp.status == 200, (resp.status, resp.url, resp.headers)
+result = json.load(resp)
+sys.stderr.write(result["stderr"])
+sys.stdout.write(result["stdout"])
+exitcode = int(result["exitcode"])
+sys.exit(exitcode)

acceptance/bundle/sync-paths-dotdot/script

@@ -1 +0,0 @@
-$CLI bundle validate

acceptance/bundle/syncroot/dotdot-git/output.txt

@@ -1,4 +1,4 @@
-Error: path "$TMPPARENT" is not within repository root "$TMPDIR"
+Error: path "$TMPDIR" is not within repository root "$TMPDIR/myrepo"
 
 Name: test-bundle
 Target: default

acceptance/bundle/syncroot/dotdot-git/script

@@ -0,0 +1,6 @@
+# This should error, we do not allow syncroot outside of git repo.
+mkdir myrepo
+cd myrepo
+cp ../databricks.yml .
+git-repo-init
+$CLI bundle validate | sed 's/\\\\/\//g'

acceptance/bundle/syncroot/dotdot-nogit/databricks.yml

@@ -0,0 +1,5 @@
+bundle:
+  name: test-bundle
+sync:
+  paths:
+    - ..

acceptance/bundle/syncroot/dotdot-nogit/output.txt

@@ -0,0 +1,7 @@
+Name: test-bundle
+Target: default
+Workspace:
+  User: $USERNAME
+  Path: /Workspace/Users/$USERNAME/.bundle/test-bundle/default
+
+Validation OK!

acceptance/bundle/syncroot/dotdot-nogit/script

@@ -0,0 +1,2 @@
+# This should not error, syncroot can be outside bundle root.
+$CLI bundle validate

acceptance/cmd_server_test.go

@@ -0,0 +1,73 @@
+package acceptance_test
+
+import (
+	"encoding/json"
+	"net/http"
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/databricks/cli/internal/testcli"
+	"github.com/stretchr/testify/require"
+)
+
+func StartCmdServer(t *testing.T) *TestServer {
+	server := StartServer(t)
+	server.Handle("/", func(r *http.Request) (any, error) {
+		q := r.URL.Query()
+		args := strings.Split(q.Get("args"), " ")
+
+		var env map[string]string
+		require.NoError(t, json.Unmarshal([]byte(q.Get("env")), &env))
+
+		for key, val := range env {
+			defer Setenv(t, key, val)()
+		}
+
+		defer Chdir(t, q.Get("cwd"))()
+
+		c := testcli.NewRunner(t, r.Context(), args...)
+		c.Verbose = false
+		stdout, stderr, err := c.Run()
+		result := map[string]any{
+			"stdout": stdout.String(),
+			"stderr": stderr.String(),
+		}
+		exitcode := 0
+		if err != nil {
+			exitcode = 1
+		}
+		result["exitcode"] = exitcode
+		return result, nil
+	})
+	return server
+}
+
+// Chdir variant that is intended to be used with defer so that it can switch back before function ends.
+// This is unlike testutil.Chdir which switches back only when tests end.
+func Chdir(t *testing.T, cwd string) func() {
+	require.NotEmpty(t, cwd)
+	prevDir, err := os.Getwd()
+	require.NoError(t, err)
+	err = os.Chdir(cwd)
+	require.NoError(t, err)
+	return func() {
+		_ = os.Chdir(prevDir)
+	}
+}
+
+// Setenv variant that is intended to be used with defer so that it can switch back before function ends.
+// This is unlike t.Setenv which switches back only when tests end.
+func Setenv(t *testing.T, key, value string) func() {
+	prevVal, exists := os.LookupEnv(key)
+
+	require.NoError(t, os.Setenv(key, value))
+
+	return func() {
+		if exists {
+			_ = os.Setenv(key, prevVal)
+		} else {
+			_ = os.Unsetenv(key)
+		}
+	}
+}

acceptance/script.prepare

@@ -34,7 +34,7 @@ trace() {
 
 git-repo-init() {
     git init -qb main
-    git config --global core.autocrlf false
+    git config core.autocrlf false
     git config user.name "Tester"
     git config user.email "tester@databricks.com"
     git add databricks.yml

bundle/bundle.go

@@ -17,6 +17,7 @@ import (
 	"github.com/databricks/cli/bundle/config"
 	"github.com/databricks/cli/bundle/env"
 	"github.com/databricks/cli/bundle/metadata"
+	"github.com/databricks/cli/libs/auth"
 	"github.com/databricks/cli/libs/fileset"
 	"github.com/databricks/cli/libs/locker"
 	"github.com/databricks/cli/libs/log"
@@ -24,7 +25,6 @@ import (
 	"github.com/databricks/cli/libs/terraform"
 	"github.com/databricks/cli/libs/vfs"
 	"github.com/databricks/databricks-sdk-go"
-	sdkconfig "github.com/databricks/databricks-sdk-go/config"
 	"github.com/hashicorp/terraform-exec/tfexec"
 )
 
@@ -245,21 +245,5 @@ func (b *Bundle) AuthEnv() (map[string]string, error) {
 	}
 
 	cfg := b.client.Config
-	out := make(map[string]string)
-	for _, attr := range sdkconfig.ConfigAttributes {
-		// Ignore profile so that downstream tools don't try and reload
-		// the profile even though we know the current configuration is valid.
-		if attr.Name == "profile" {
-			continue
-		}
-		if len(attr.EnvVars) == 0 {
-			continue
-		}
-		if attr.IsZero(cfg) {
-			continue
-		}
-		out[attr.EnvVars[0]] = attr.GetString(cfg)
-	}
-
-	return out, nil
+	return auth.Env(cfg), nil
 }

bundle/config/mutator/load_git_details.go

@@ -32,7 +32,7 @@ func (m *loadGitDetails) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagn
 	}
 
 	if info.WorktreeRoot == "" {
-		b.WorktreeRoot = b.BundleRoot
+		b.WorktreeRoot = b.SyncRoot
 	} else {
 		b.WorktreeRoot = vfs.MustNew(info.WorktreeRoot)
 	}

bundle/internal/schema/main.go

@@ -40,6 +40,19 @@ func addInterpolationPatterns(typ reflect.Type, s jsonschema.Schema) jsonschema.
 		}
 	}
 
+	// Allows using variables in enum fields
+	if s.Type == jsonschema.StringType && s.Enum != nil {
+		return jsonschema.Schema{
+			OneOf: []jsonschema.Schema{
+				s,
+				{
+					Type:    jsonschema.StringType,
+					Pattern: interpolationPattern("var"),
+				},
+			},
+		}
+	}
+
 	switch s.Type {
 	case jsonschema.ArrayType, jsonschema.ObjectType:
 		// arrays and objects can have complex variable values specified.

bundle/internal/schema/testdata/pass/job.yml

@@ -13,6 +13,8 @@ variables:
   simplevar:
     default: true
     description: "simplevar description"
+  schedule_status:
+    default: "PAUSED"
 
   complexvar:
     default:
@@ -42,6 +44,8 @@ resources:
             dependencies:
               - python=3.7
             client: "myclient"
+      trigger:
+        pause_status: ${var.schedule_status}
       tags:
         foo: bar
         bar: baz

bundle/schema/embed_test.go

@@ -59,8 +59,8 @@ func TestJsonSchema(t *testing.T) {
 	}
 
 	providers := walk(s.Definitions, "github.com", "databricks", "databricks-sdk-go", "service", "jobs.GitProvider")
-	assert.Contains(t, providers.Enum, "gitHub")
-	assert.Contains(t, providers.Enum, "bitbucketCloud")
-	assert.Contains(t, providers.Enum, "gitHubEnterprise")
-	assert.Contains(t, providers.Enum, "bitbucketServer")
+	assert.Contains(t, providers.OneOf[0].Enum, "gitHub")
+	assert.Contains(t, providers.OneOf[0].Enum, "bitbucketCloud")
+	assert.Contains(t, providers.OneOf[0].Enum, "gitHubEnterprise")
+	assert.Contains(t, providers.OneOf[0].Enum, "bitbucketServer")
 }

libs/auth/env.go

@@ -0,0 +1,26 @@
+package auth
+
+import "github.com/databricks/databricks-sdk-go/config"
+
+// Env generates the authentication environment variables we need to set for
+// downstream applications from the CLI to work correctly.
+func Env(cfg *config.Config) map[string]string {
+	out := make(map[string]string)
+	for _, attr := range config.ConfigAttributes {
+		// Ignore profile so that downstream tools don't try and reload
+		// the profile. We know the current configuration is already valid since
+		// otherwise the CLI would have thrown an error when loading it.
+		if attr.Name == "profile" {
+			continue
+		}
+		if len(attr.EnvVars) == 0 {
+			continue
+		}
+		if attr.IsZero(cfg) {
+			continue
+		}
+		out[attr.EnvVars[0]] = attr.GetString(cfg)
+	}
+
+	return out
+}

libs/auth/env_test.go

@@ -0,0 +1,42 @@
+package auth
+
+import (
+	"testing"
+
+	"github.com/databricks/databricks-sdk-go/config"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestAuthEnv(t *testing.T) {
+	in := &config.Config{
+		Profile:            "thisshouldbeignored",
+		Host:               "https://test.com",
+		Token:              "test-token",
+		Password:           "test-password",
+		MetadataServiceURL: "http://somurl.com",
+
+		AzureUseMSI:       true,
+		AzureTenantID:     "test-tenant-id",
+		AzureClientID:     "test-client-id",
+		AzureClientSecret: "test-client-secret",
+
+		ActionsIDTokenRequestToken: "test-actions-id-token-request-token",
+	}
+
+	expected := map[string]string{
+		"DATABRICKS_HOST":                 "https://test.com",
+		"DATABRICKS_TOKEN":                "test-token",
+		"DATABRICKS_PASSWORD":             "test-password",
+		"DATABRICKS_METADATA_SERVICE_URL": "http://somurl.com",
+
+		"ARM_USE_MSI":       "true",
+		"ARM_TENANT_ID":     "test-tenant-id",
+		"ARM_CLIENT_ID":     "test-client-id",
+		"ARM_CLIENT_SECRET": "test-client-secret",
+
+		"ACTIONS_ID_TOKEN_REQUEST_TOKEN": "test-actions-id-token-request-token",
+	}
+
+	out := Env(in)
+	assert.Equal(t, expected, out)
+}

libs/testdiff/replacement.go

@@ -3,7 +3,9 @@ package testdiff
 import (
 	"encoding/json"
 	"fmt"
+	"path/filepath"
 	"regexp"
+	"runtime"
 	"slices"
 	"strings"
 
@@ -74,13 +76,48 @@ func (r *ReplacementsContext) Set(old, new string) {
 	if err == nil {
 		encodedOld, err := json.Marshal(old)
 		if err == nil {
-			r.appendLiteral(string(encodedOld), string(encodedNew))
+			r.appendLiteral(trimQuotes(string(encodedOld)), trimQuotes(string(encodedNew)))
 		}
 	}
 
 	r.appendLiteral(old, new)
 }
 
+func trimQuotes(s string) string {
+	if len(s) > 0 && s[0] == '"' {
+		s = s[1:]
+	}
+	if len(s) > 0 && s[len(s)-1] == '"' {
+		s = s[:len(s)-1]
+	}
+	return s
+}
+
+func (r *ReplacementsContext) SetPath(old, new string) {
+	r.Set(old, new)
+
+	if runtime.GOOS != "windows" {
+		return
+	}
+
+	// Support both forward and backward slashes
+	m1 := strings.ReplaceAll(old, "\\", "/")
+	if m1 != old {
+		r.Set(m1, new)
+	}
+
+	m2 := strings.ReplaceAll(old, "/", "\\")
+	if m2 != old && m2 != m1 {
+		r.Set(m2, new)
+	}
+}
+
+func (r *ReplacementsContext) SetPathWithParents(old, new string) {
+	r.SetPath(old, new)
+	r.SetPath(filepath.Dir(old), new+"_PARENT")
+	r.SetPath(filepath.Dir(filepath.Dir(old)), new+"_GPARENT")
+}
+
 func PrepareReplacementsWorkspaceClient(t testutil.TestingT, r *ReplacementsContext, w *databricks.WorkspaceClient) {
 	t.Helper()
 	// in some clouds (gcp) w.Config.Host includes "https://" prefix in others it's really just a host (azure)