-
Notifications
You must be signed in to change notification settings - Fork 163
Add text output mode for auth token and auth env #4725
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 5 commits
ce79a10
aeae5b6
0a4a1c9
72ebfd9
0339754
0c68a51
a6000ec
23d833e
fcfc26d
1a3c536
f38dd6f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| package auth | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "testing" | ||
|
|
||
| "github.com/databricks/cli/libs/cmdio" | ||
| "github.com/databricks/cli/libs/flags" | ||
| "github.com/spf13/cobra" | ||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestQuoteEnvValue(t *testing.T) { | ||
| cases := []struct { | ||
| name string | ||
| in string | ||
| want string | ||
| }{ | ||
| {name: "simple value", in: "hello", want: "hello"}, | ||
| {name: "empty value", in: "", want: `''`}, | ||
| {name: "value with space", in: "hello world", want: "'hello world'"}, | ||
| {name: "value with tab", in: "hello\tworld", want: "'hello\tworld'"}, | ||
| {name: "value with double quote", in: `say "hi"`, want: "'say \"hi\"'"}, | ||
| {name: "value with backslash", in: `path\to`, want: "'path\\to'"}, | ||
| {name: "url value", in: "https://example.com", want: "https://example.com"}, | ||
| {name: "value with dollar", in: "price$5", want: "'price$5'"}, | ||
| {name: "value with backtick", in: "hello`world", want: "'hello`world'"}, | ||
| {name: "value with bang", in: "hello!world", want: "'hello!world'"}, | ||
| {name: "value with single quote", in: "it's", want: "'it'\\''s'"}, | ||
| } | ||
| for _, c := range cases { | ||
| t.Run(c.name, func(t *testing.T) { | ||
| got := quoteEnvValue(c.in) | ||
| assert.Equal(t, c.want, got) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestEnvCommand_TextOutput(t *testing.T) { | ||
| cases := []struct { | ||
| name string | ||
| args []string | ||
| wantJSON bool | ||
| }{ | ||
| { | ||
| name: "default output is JSON", | ||
| args: []string{"--host", "https://test.cloud.databricks.com"}, | ||
| wantJSON: true, | ||
| }, | ||
| { | ||
| name: "explicit --output text produces KEY=VALUE lines", | ||
| args: []string{"--host", "https://test.cloud.databricks.com", "--output", "text"}, | ||
| wantJSON: false, | ||
| }, | ||
| { | ||
| name: "explicit --output json produces JSON", | ||
| args: []string{"--host", "https://test.cloud.databricks.com", "--output", "json"}, | ||
| wantJSON: true, | ||
| }, | ||
| } | ||
|
|
||
| for _, c := range cases { | ||
| t.Run(c.name, func(t *testing.T) { | ||
| parent := &cobra.Command{Use: "databricks"} | ||
| outputFlag := flags.OutputText | ||
| parent.PersistentFlags().VarP(&outputFlag, "output", "o", "output type: text or json") | ||
|
|
||
| envCmd := newEnvCommand() | ||
| parent.AddCommand(envCmd) | ||
| parent.SetContext(cmdio.MockDiscard(t.Context())) | ||
|
|
||
| // Set DATABRICKS_TOKEN so the SDK's config.Authenticate succeeds | ||
| // without hitting a real endpoint. | ||
| t.Setenv("DATABRICKS_TOKEN", "test-token-value") | ||
|
|
||
| var buf bytes.Buffer | ||
| parent.SetOut(&buf) | ||
| parent.SetArgs(append([]string{"env"}, c.args...)) | ||
|
|
||
| err := parent.Execute() | ||
| require.NoError(t, err) | ||
|
|
||
| output := buf.String() | ||
| if c.wantJSON { | ||
| assert.Contains(t, output, "{") | ||
| assert.Contains(t, output, "DATABRICKS_HOST") | ||
| } else { | ||
| assert.NotContains(t, output, "{") | ||
| assert.Contains(t, output, "DATABRICKS_HOST=") | ||
| assert.Contains(t, output, "=") | ||
| // Verify KEY=VALUE format (no JSON structure) | ||
| assert.NotContains(t, output, `"env"`) | ||
| } | ||
| }) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,11 +8,13 @@ import ( | |
| "strings" | ||
| "time" | ||
|
|
||
| "github.com/databricks/cli/cmd/root" | ||
| "github.com/databricks/cli/libs/auth" | ||
| "github.com/databricks/cli/libs/cmdio" | ||
| "github.com/databricks/cli/libs/databrickscfg" | ||
| "github.com/databricks/cli/libs/databrickscfg/profile" | ||
| "github.com/databricks/cli/libs/env" | ||
| "github.com/databricks/cli/libs/flags" | ||
| "github.com/databricks/databricks-sdk-go/config" | ||
| "github.com/databricks/databricks-sdk-go/credentials/u2m" | ||
| "github.com/databricks/databricks-sdk-go/credentials/u2m/cache" | ||
|
|
@@ -83,17 +85,27 @@ using a client ID and secret is not supported.`, | |
| if err != nil { | ||
| return err | ||
| } | ||
| raw, err := json.MarshalIndent(t, "", " ") | ||
| if err != nil { | ||
| return err | ||
| } | ||
| _, _ = cmd.OutOrStdout().Write(raw) | ||
| return nil | ||
| return writeTokenOutput(cmd, t) | ||
| } | ||
|
|
||
| return cmd | ||
| } | ||
|
|
||
| func writeTokenOutput(cmd *cobra.Command, t *oauth2.Token) error { | ||
| // Output plain token when the user explicitly passes --output text. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This comment does not bring much compared to reading the code. Rather, I think we should briefly explain why we discard implicit — backward compatibily?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair point. I'll replace this with a comment explaining the actual reason: auth token defaults to JSON (unlike most CLI commands), so we only switch to text when the user explicitly passes --output text to avoid breaking scripts that parse the JSON output. |
||
| if cmd.Flag("output").Changed && root.OutputType(cmd) == flags.OutputText { | ||
| _, _ = fmt.Fprintln(cmd.OutOrStdout(), t.AccessToken) | ||
| return nil | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the rationale behind swallowing the error, here and in other places?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, I'll return the write errors instead. The pattern was carried over from the existing code but it's better to surface them so the CLI exits non-zero if stdout is broken. |
||
| } | ||
|
|
||
| raw, err := json.MarshalIndent(t, "", " ") | ||
| if err != nil { | ||
| return err | ||
| } | ||
| _, _ = cmd.OutOrStdout().Write(raw) | ||
| return nil | ||
| } | ||
|
|
||
| type loadTokenArgs struct { | ||
| // authArguments is the parsed auth arguments, including the host and optionally the account ID. | ||
| authArguments *auth.AuthArguments | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.