Skip to content
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

Added auth describe command #1244

Merged
merged 12 commits into from
Apr 3, 2024
21 changes: 17 additions & 4 deletions bundle/config/workspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ func (s User) MarshalJSON() ([]byte, error) {
return marshal.Marshal(s)
}

func (w *Workspace) Client() (*databricks.WorkspaceClient, error) {
cfg := config.Config{
func (w *Workspace) Config() *config.Config {
cfg := &config.Config{
andrewnester marked this conversation as resolved.
Show resolved Hide resolved
// Generic
Host: w.Host,
Profile: w.Profile,
Expand All @@ -101,6 +101,19 @@ func (w *Workspace) Client() (*databricks.WorkspaceClient, error) {
AzureLoginAppID: w.AzureLoginAppID,
}

for k := range config.ConfigAttributes {
attr := &config.ConfigAttributes[k]
if !attr.IsZero(cfg) {
cfg.SetAttrSource(attr, config.Source{Type: config.SourceType("bundle")})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could even include the file/line where the attr was defined.

}
andrewnester marked this conversation as resolved.
Show resolved Hide resolved
}

return cfg
}

func (w *Workspace) Client() (*databricks.WorkspaceClient, error) {
cfg := w.Config()

// If only the host is configured, we try and unambiguously match it to
// a profile in the user's databrickscfg file. Override the default loaders.
if w.Host != "" && w.Profile == "" {
Expand All @@ -124,13 +137,13 @@ func (w *Workspace) Client() (*databricks.WorkspaceClient, error) {
// Now that the configuration is resolved, we can verify that the host in the bundle configuration
// is identical to the host associated with the selected profile.
if w.Host != "" && w.Profile != "" {
err := databrickscfg.ValidateConfigAndProfileHost(&cfg, w.Profile)
err := databrickscfg.ValidateConfigAndProfileHost(cfg, w.Profile)
if err != nil {
return nil, err
}
}

return databricks.NewWorkspaceClient((*databricks.Config)(&cfg))
return databricks.NewWorkspaceClient((*databricks.Config)(cfg))
}

func init() {
Expand Down
1 change: 1 addition & 0 deletions cmd/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ func New() *cobra.Command {
cmd.AddCommand(newLoginCommand(&perisistentAuth))
cmd.AddCommand(newProfilesCommand())
cmd.AddCommand(newTokenCommand(&perisistentAuth))
cmd.AddCommand(newDescribeCommand())
return cmd
}

Expand Down
170 changes: 170 additions & 0 deletions cmd/auth/describe.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
package auth

import (
"context"
"encoding/json"
"fmt"

"github.com/databricks/cli/cmd/root"
"github.com/databricks/cli/libs/cmdio"
"github.com/databricks/cli/libs/flags"
"github.com/databricks/databricks-sdk-go/config"
"github.com/spf13/cobra"
)

var authTemplate = `{{"Host:" | bold}} {{.Details.Host}}
{{- if .Username}}
{{"User:" | bold}} {{.Username}}
{{- end}}
{{"Authenticated with:" | bold}} {{.Details.AuthType}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a new user, it took me a hot second to figure out what 'pat' was in the example output, and I also wondered what other types there were, and had to do a bit of searching. Low priority, but just a thought - could we output a friendlier string instead of just the exact field value?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do, we should do both (can be a follow-up to this, IMO). The literal value is what someone would use when hardcoding it in a profile or in the DATABRICKS_AUTH_TYPE environment variable.

{{- if .AccountID}}
{{"Account ID:" | bold}} {{.AccountID}}
{{- end}}
andrewnester marked this conversation as resolved.
Show resolved Hide resolved
-----
` + configurationTemplate

var errorTemplate = `Unable to authenticate: {{.Error}}
-----
` + configurationTemplate

const configurationTemplate = `Current configuration:
{{- $details := .Details}}
{{- range $k, $v := $details.Configuration }}
{{if $v.AuthTypeMismatch}}~{{else}}✓{{end}} {{$k | bold}}: {{$v.Value}}
{{- if not (eq $v.Source.String "dynamic configuration")}}
{{- " (from" | italic}} {{$v.Source.String | italic}}
{{- if $v.AuthTypeMismatch}}, {{ "not used for auth type " | red | italic }}{{$details.AuthType | red | italic}}{{end}})
{{- end}}
{{- end}}
`

func newDescribeCommand() *cobra.Command {
cmd := &cobra.Command{
Use: "describe",
Short: "Describes the credentials and the source of those credentials, being used by the CLI to authenticate",
}

var showSensitive bool
cmd.Flags().BoolVar(&showSensitive, "sensitive", false, "Include sensitive fields like passwords and tokens in the output")

cmd.RunE = func(cmd *cobra.Command, args []string) error {
ctx := cmd.Context()
var status *authStatus
var err error
status, err = getAuthStatus(cmd, args, showSensitive, func(cmd *cobra.Command, args []string) (*config.Config, bool, error) {
isAccount, err := root.MustAnyClient(cmd, args)
return root.ConfigUsed(cmd.Context()), isAccount, err
})

if err != nil {
return err
}

if status.Error != nil {
return render(ctx, cmd, status, errorTemplate)
}

return render(ctx, cmd, status, authTemplate)
}

return cmd
}

type tryAuth func(cmd *cobra.Command, args []string) (*config.Config, bool, error)

func getAuthStatus(cmd *cobra.Command, args []string, showSensitive bool, fn tryAuth) (*authStatus, error) {
cfg, isAccount, err := fn(cmd, args)
ctx := cmd.Context()
if err != nil {
return &authStatus{
Status: "error",
Error: err,
Details: getAuthDetails(cmd, cfg, showSensitive),
}, nil
}

if isAccount {
a := root.AccountClient(ctx)
status := authStatus{
Status: "success",
Details: getAuthDetails(cmd, a.Config, showSensitive),
AccountID: a.Config.AccountID,
Username: a.Config.Username,
}
andrewnester marked this conversation as resolved.
Show resolved Hide resolved

return &status, nil
}

w := root.WorkspaceClient(ctx)
me, err := w.CurrentUser.Me(ctx)
if err != nil {
return nil, err
}

status := authStatus{
Status: "success",
Details: getAuthDetails(cmd, w.Config, showSensitive),
Username: me.UserName,
}

return &status, nil
}

func render(ctx context.Context, cmd *cobra.Command, status *authStatus, template string) error {
switch root.OutputType(cmd) {
case flags.OutputText:
return cmdio.RenderWithTemplate(ctx, status, "", template)
case flags.OutputJSON:
buf, err := json.MarshalIndent(status, "", " ")
if err != nil {
return err
}
cmd.OutOrStdout().Write(buf)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed the "Source" struct doesn't include JSON tags in its definition, so it uses the uppercased field names in the JSON output. Should be fixed in a follow up SDK change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or it could be fixed here with a type copy.

Copy link
Contributor Author

@andrewnester andrewnester Mar 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pietern good catch, I'll do the follow up

default:
return fmt.Errorf("unknown output type %s", root.OutputType(cmd))
}

return nil
}

type authStatus struct {
Status string `json:"status"`
Error error `json:"error,omitempty"`
Username string `json:"username,omitempty"`
AccountID string `json:"account_id,omitempty"`
Details config.AuthDetails `json:"details"`
}

func getAuthDetails(cmd *cobra.Command, cfg *config.Config, showSensitive bool) config.AuthDetails {
var opts []config.AuthDetailsOptions
if showSensitive {
opts = append(opts, config.ShowSensitive)
}
details := cfg.GetAuthDetails(opts...)
andrewnester marked this conversation as resolved.
Show resolved Hide resolved

for k, v := range details.Configuration {
if k == "profile" && cmd.Flag("profile").Changed {
v.Source = config.Source{Type: config.SourceType("flag"), Name: "--profile"}
}

if k == "host" && cmd.Flag("host").Changed {
v.Source = config.Source{Type: config.SourceType("flag"), Name: "--host"}
}
}
andrewnester marked this conversation as resolved.
Show resolved Hide resolved

// If profile is not set explicitly, default to "default"
if _, ok := details.Configuration["profile"]; !ok {
profile := cfg.Profile
if profile == "" {
profile = "default"
}
details.Configuration["profile"] = &config.AttrConfig{Value: profile, Source: config.Source{Type: config.SourceDynamicConfig}}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we also change the source for DATABRICKS_CLI_PATH here?

Now every run outputs:

  ✓ databricks_cli_path: ./cli (from DATABRICKS_CLI_PATH environment variable)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And shouldn't it show up as ~ btw if the auth type is not OAuth btw?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And shouldn't it show up as ~ btw if the auth type is not OAuth btw?

yeah, we would need to mark it with correct auth_types in Go SDK. Since it's a minor thing, we'll do the follow up on this


// Unset source for databricks_cli_path because it can't be overridden anyway
if v, ok := details.Configuration["databricks_cli_path"]; ok {
v.Source = config.Source{Type: config.SourceDynamicConfig}
}

return details
}
Loading
Loading