-
Notifications
You must be signed in to change notification settings - Fork 156
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 9 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,102 @@ | ||
| 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'"}, | ||
| {name: "value with newline", in: "line1\nline2", want: "'line1\nline2'"}, | ||
| {name: "value with carriage return", in: "line1\rline2", want: "'line1\rline2'"}, | ||
| } | ||
| 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: nil, | ||
| wantJSON: true, | ||
| }, | ||
| { | ||
| name: "explicit --output text produces KEY=VALUE lines", | ||
| args: []string{"--output", "text"}, | ||
| wantJSON: false, | ||
| }, | ||
| { | ||
| name: "explicit --output json produces JSON", | ||
| args: []string{"--output", "json"}, | ||
| wantJSON: true, | ||
| }, | ||
| } | ||
|
|
||
| for _, c := range cases { | ||
| t.Run(c.name, func(t *testing.T) { | ||
| // Isolate from real config/token cache on the machine. | ||
| t.Setenv("DATABRICKS_CONFIG_FILE", t.TempDir()+"/.databrickscfg") | ||
| t.Setenv("HOME", t.TempDir()) | ||
| // Set env vars so MustAnyClient resolves auth via PAT. | ||
| t.Setenv("DATABRICKS_HOST", "https://test.cloud.databricks.com") | ||
| t.Setenv("DATABRICKS_TOKEN", "test-token-value") | ||
|
|
||
| parent := &cobra.Command{Use: "databricks"} | ||
| outputFlag := flags.OutputText | ||
| parent.PersistentFlags().VarP(&outputFlag, "output", "o", "output type: text or json") | ||
| parent.PersistentFlags().StringP("profile", "p", "", "~/.databrickscfg profile") | ||
|
|
||
| envCmd := newEnvCommand() | ||
| parent.AddCommand(envCmd) | ||
| parent.SetContext(cmdio.MockDiscard(t.Context())) | ||
|
|
||
| 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, "=") | ||
| 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. | ||
| if cmd.Flag("output").Changed && root.OutputType(cmd) == flags.OutputText { | ||
| _, _ = fmt.Fprintln(cmd.OutOrStdout(), t.AccessToken) | ||
| return nil | ||
|
||
| } | ||
|
|
||
| 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The 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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The 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.