From ab206242060a05bd7a3b5073cc3a6869973446f3 Mon Sep 17 00:00:00 2001 From: shreyas-goenka <88374338+shreyas-goenka@users.noreply.github.com> Date: Mon, 14 Oct 2024 21:49:48 +0530 Subject: [PATCH 1/3] Assert SDK version is consistent in the CLI generation process (#1814) ## Changes Followup from https://github.com/databricks/cli/pull/1809#discussion_r1790045086. User will see the following error if the SDK version does not match during CLI code generation. ``` --- FAIL: TestConsistentDatabricksSdkVersion (1.34s) info_test.go:53: Error Trace: /Users/shreyas.goenka/cli/internal/build/info_test.go:53 Error: Not equal: expected: "0c86ea6dbd9a730c24ff0d4e509603e476955ac5" actual : "6f6b1371e640f2dfeba72d365ac566368656f6b6" Diff: --- Expected +++ Actual @@ -1 +1 @@ -0c86ea6dbd9a730c24ff0d4e509603e476955ac5 +6f6b1371e640f2dfeba72d365ac566368656f6b6 Test: TestConsistentDatabricksSdkVersion Messages: please update the SDK version before generating the CLI ``` ## Tests Manually asserted that: 1. Generating the CLI without the SDK bump causes an error. 2. Generating the CLI after the SDK bump works as expected. 3. The test works even when the SDK is pinned to a specific commit like `v0.47.1-0.20241002195128-6cecc224cbf7` --- .codegen.json | 1 + internal/build/sdk_consistency_test.go | 73 ++++++++++++++++++++++++++ 2 files changed, 74 insertions(+) create mode 100644 internal/build/sdk_consistency_test.go diff --git a/.codegen.json b/.codegen.json index 077d072b..4524ab55 100644 --- a/.codegen.json +++ b/.codegen.json @@ -11,6 +11,7 @@ "toolchain": { "required": ["go"], "post_generate": [ + "go test -timeout 240s -run TestConsistentDatabricksSdkVersion github.com/databricks/cli/internal/build", "go run ./bundle/internal/schema/*.go ./bundle/schema/jsonschema.json", "echo 'bundle/internal/tf/schema/\\*.go linguist-generated=true' >> ./.gitattributes", "echo 'go.sum linguist-generated=true' >> ./.gitattributes", diff --git a/internal/build/sdk_consistency_test.go b/internal/build/sdk_consistency_test.go new file mode 100644 index 00000000..37f60250 --- /dev/null +++ b/internal/build/sdk_consistency_test.go @@ -0,0 +1,73 @@ +package build + +import ( + "encoding/json" + "fmt" + "io" + "net/http" + "os" + "os/exec" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/mod/modfile" +) + +// This test ensures that the OpenAPI SHA the CLI is being generated from matches +// the OpenAPI SHA of the Go SDK version used in the CLI. We should always upgrade +// the Go SDK version before generating the CLI because downstream generated assets +// like the bundle schema depend on the Go SDK itself. +func TestConsistentDatabricksSdkVersion(t *testing.T) { + // Read the go.mod file + b, err := os.ReadFile("../../go.mod") + require.NoError(t, err) + + // Parse the go.mod file to get the databricks-sdk version + modFile, err := modfile.Parse("../../go.mod", b, nil) + require.NoError(t, err) + + modulePath := "github.com/databricks/databricks-sdk-go" + var version string + for _, r := range modFile.Require { + if r.Mod.Path == modulePath { + version = r.Mod.Version + } + } + require.NotEmpty(t, version) + + // Full path of the package. For example: github.com/databricks/databricks-sdk-go@v0.47.1-0.20241002195128-6cecc224cbf7 + fullPath := fmt.Sprintf("%s@%s", modulePath, version) + + type goListResponse struct { + Origin struct { + Hash string + } + } + + // Using the go CLI query for the git hash corresponding to the databricks-sdk-go version + cmd := exec.Command("go", "list", "-m", "-json", "-mod=readonly", fullPath) + out, err := cmd.Output() + require.NoError(t, err) + parsedOutput := new(goListResponse) + err = json.Unmarshal(out, parsedOutput) + require.NoError(t, err) + hash := parsedOutput.Origin.Hash + require.NotEmpty(t, hash) + + // Read the OpenAPI SHA from the Go SDK. + url := fmt.Sprintf("https://raw.githubusercontent.com/databricks/databricks-sdk-go/%s/.codegen/_openapi_sha", hash) + resp, err := http.Get(url) + require.NoError(t, err) + defer resp.Body.Close() + require.Equal(t, http.StatusOK, resp.StatusCode) + + sdkSha, err := io.ReadAll(resp.Body) + require.NoError(t, err) + + cliSha, err := os.ReadFile("../../.codegen/_openapi_sha") + require.NoError(t, err) + + assert.Equal(t, strings.TrimSpace(string(cliSha)), strings.TrimSpace(string(sdkSha)), "please update the SDK version before generating the CLI") +} From 0753dfe2f488c834724e2bfdf1618ebe8db271ab Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Tue, 15 Oct 2024 14:10:02 +0200 Subject: [PATCH 2/3] Fixed unmarshalling json input into `interface{}` type (#1832) ## Changes Fixed unmarshalling json input into `interface{}` type Commands like `api post` support free form request input, so it should be unmarshaled correctly ## Tests Added regression test + E2E test pass --- libs/flags/json_flag.go | 24 +++++++++++++++++++----- libs/flags/json_flag_test.go | 14 +++++--------- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/libs/flags/json_flag.go b/libs/flags/json_flag.go index 0ed8be74..0324f114 100644 --- a/libs/flags/json_flag.go +++ b/libs/flags/json_flag.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "os" + "reflect" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn/convert" @@ -63,11 +64,24 @@ func (j *JsonFlag) Unmarshal(v any) diag.Diagnostics { return diags.Extend(diag.FromErr(err)) } - // Finally unmarshal the normalized data to the output. - // It will fill in the ForceSendFields field if the struct contains it. - err = marshal.Unmarshal(data, v) - if err != nil { - return diags.Extend(diag.FromErr(err)) + kind := reflect.ValueOf(v).Kind() + if kind == reflect.Ptr { + kind = reflect.ValueOf(v).Elem().Kind() + } + + if kind == reflect.Struct { + // Finally unmarshal the normalized data to the output. + // It will fill in the ForceSendFields field if the struct contains it. + err = marshal.Unmarshal(data, v) + if err != nil { + return diags.Extend(diag.FromErr(err)) + } + } else { + // If the output is not a struct, just unmarshal the data to the output. + err = json.Unmarshal(data, v) + if err != nil { + return diags.Extend(diag.FromErr(err)) + } } return diags diff --git a/libs/flags/json_flag_test.go b/libs/flags/json_flag_test.go index 16769f4e..77530086 100644 --- a/libs/flags/json_flag_test.go +++ b/libs/flags/json_flag_test.go @@ -13,10 +13,6 @@ import ( "github.com/stretchr/testify/require" ) -type requestType struct { - Foo string `json:"foo"` -} - func TestJsonFlagEmpty(t *testing.T) { var body JsonFlag @@ -35,13 +31,13 @@ func TestJsonFlagInline(t *testing.T) { err := body.Set(`{"foo": "bar"}`) assert.NoError(t, err) - var request requestType + var request any diags := body.Unmarshal(&request) assert.NoError(t, diags.Error()) assert.Empty(t, diags) assert.Equal(t, "JSON (14 bytes)", body.String()) - assert.Equal(t, requestType{"bar"}, request) + assert.Equal(t, map[string]any{"foo": "bar"}, request) } func TestJsonFlagError(t *testing.T) { @@ -50,7 +46,7 @@ func TestJsonFlagError(t *testing.T) { err := body.Set(`{"foo":`) assert.NoError(t, err) - var request requestType + var request any diags := body.Unmarshal(&request) assert.EqualError(t, diags.Error(), "error decoding JSON at (inline):1:8: unexpected end of JSON input") assert.Equal(t, "JSON (7 bytes)", body.String()) @@ -58,7 +54,7 @@ func TestJsonFlagError(t *testing.T) { func TestJsonFlagFile(t *testing.T) { var body JsonFlag - var request requestType + var request any var fpath string var payload = []byte(`{"foo": "bar"}`) @@ -78,7 +74,7 @@ func TestJsonFlagFile(t *testing.T) { assert.NoError(t, diags.Error()) assert.Empty(t, diags) - assert.Equal(t, requestType{"bar"}, request) + assert.Equal(t, map[string]any{"foo": "bar"}, request) } const jsonData = ` From cc112961ce8aa3ced65635d5174aa38d5a27be8a Mon Sep 17 00:00:00 2001 From: shreyas-goenka <88374338+shreyas-goenka@users.noreply.github.com> Date: Wed, 16 Oct 2024 18:20:17 +0530 Subject: [PATCH 3/3] Fix `TestAccFsMkdirWhenFileExistsAtPath` in isolated Azure environments (#1833) ## Changes This test passes on normal `azure-prod` but started to fail on `azure-prod-is`, which is the isolated version of azure-prod. This PR patches the test to include the error returned from the cloud setup in `azure-prod-is`. ## Tests The test passes now on `azure-prod-is`. --- internal/fs_mkdir_test.go | 4 ++-- internal/helpers.go | 10 ++++------ 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/internal/fs_mkdir_test.go b/internal/fs_mkdir_test.go index dd75c7c3..9191f614 100644 --- a/internal/fs_mkdir_test.go +++ b/internal/fs_mkdir_test.go @@ -112,8 +112,8 @@ func TestAccFsMkdirWhenFileExistsAtPath(t *testing.T) { // assert mkdir fails _, _, err = RequireErrorRun(t, "fs", "mkdir", path.Join(tmpDir, "hello")) - // Different cloud providers return different errors. - regex := regexp.MustCompile(`(^|: )Path is a file: .*$|(^|: )Cannot create directory .* because .* is an existing file\.$|(^|: )mkdirs\(hadoopPath: .*, permission: rwxrwxrwx\): failed$`) + // Different cloud providers or cloud configurations return different errors. + regex := regexp.MustCompile(`(^|: )Path is a file: .*$|(^|: )Cannot create directory .* because .* is an existing file\.$|(^|: )mkdirs\(hadoopPath: .*, permission: rwxrwxrwx\): failed$|(^|: )"The specified path already exists.".*$`) assert.Regexp(t, regex, err.Error()) }) diff --git a/internal/helpers.go b/internal/helpers.go index 9387706b..3bf38775 100644 --- a/internal/helpers.go +++ b/internal/helpers.go @@ -20,6 +20,7 @@ import ( "time" "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/internal/acc" "github.com/databricks/cli/libs/flags" "github.com/databricks/cli/cmd" @@ -591,13 +592,10 @@ func setupWsfsExtensionsFiler(t *testing.T) (filer.Filer, string) { } func setupDbfsFiler(t *testing.T) (filer.Filer, string) { - t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) + _, wt := acc.WorkspaceTest(t) - w, err := databricks.NewWorkspaceClient() - require.NoError(t, err) - - tmpDir := TemporaryDbfsDir(t, w) - f, err := filer.NewDbfsClient(w, tmpDir) + tmpDir := TemporaryDbfsDir(t, wt.W) + f, err := filer.NewDbfsClient(wt.W, tmpDir) require.NoError(t, err) return f, path.Join("dbfs:/", tmpDir)