From 01d4926d0bb2a94742e0adc946482a02e258e490 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Thu, 29 Feb 2024 13:55:32 +0100 Subject: [PATCH 01/11] Added auth describe command --- bundle/config/workspace.go | 21 +++- cmd/auth/auth.go | 1 + cmd/auth/describe.go | 229 +++++++++++++++++++++++++++++++++++ cmd/auth/describe_test.go | 210 ++++++++++++++++++++++++++++++++ cmd/root/auth.go | 20 ++- go.mod | 16 +++ libs/cmdio/render.go | 6 + libs/databrickscfg/loader.go | 7 +- 8 files changed, 503 insertions(+), 7 deletions(-) create mode 100644 cmd/auth/describe.go create mode 100644 cmd/auth/describe_test.go diff --git a/bundle/config/workspace.go b/bundle/config/workspace.go index 5f8691babe..3223408d28 100644 --- a/bundle/config/workspace.go +++ b/bundle/config/workspace.go @@ -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{ // Generic Host: w.Host, Profile: w.Profile, @@ -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) { + attr.SetSource(&config.Source{Type: config.SourceType("bundle")}) + } + } + + 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 == "" { @@ -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() { diff --git a/cmd/auth/auth.go b/cmd/auth/auth.go index e0c7c7c5bb..59de76111d 100644 --- a/cmd/auth/auth.go +++ b/cmd/auth/auth.go @@ -22,6 +22,7 @@ func New() *cobra.Command { cmd.AddCommand(newLoginCommand(&perisistentAuth)) cmd.AddCommand(newProfilesCommand()) cmd.AddCommand(newTokenCommand(&perisistentAuth)) + cmd.AddCommand(newDescribeCommand()) return cmd } diff --git a/cmd/auth/describe.go b/cmd/auth/describe.go new file mode 100644 index 0000000000..c924298d31 --- /dev/null +++ b/cmd/auth/describe.go @@ -0,0 +1,229 @@ +package auth + +import ( + "context" + "encoding/json" + "fmt" + "slices" + + "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/env" + "github.com/databricks/cli/libs/flags" + "github.com/databricks/databricks-sdk-go/config" + "github.com/spf13/cobra" +) + +var authTemplate = fmt.Sprintf(`{{"Workspace:" | bold}} {{.Details.Host}}. +{{if .Details.Username }}{{"User:" | bold}} {{.Details.Username}}.{{end}} +{{"Authenticated with:" | bold}} {{.Details.AuthType}} +{{if .Details.AccountID }}{{"Account ID:" | bold}} {{.Details.AccountID}}.{{end}} +----- +%s +`, configurationTemplate) + +var errorTemplate = fmt.Sprintf(`Unable to authenticate: {{.Error}} +----- +%s +`, configurationTemplate) + +const configurationTemplate = `Configuration: + {{- $details := .Details}} + {{- range $k, $v := $details.Configuration }} + {{if $v.AuthTypeMismatch}}x{{else}}✓{{end}} {{$k | bold}}: {{$v.Value}} + {{- if $v.Source }} + {{- " (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 which auth credentials is being used by the CLI and identify their source", + } + + var showSensitive bool + var isAccount bool + cmd.Flags().BoolVar(&showSensitive, "sensitive", false, "Include sensitive fields like passwords and tokens in the output") + cmd.Flags().BoolVar(&isAccount, "account", false, "Describe the account client instead of the workspace client") + + cmd.RunE = func(cmd *cobra.Command, args []string) error { + ctx := cmd.Context() + var status *authStatus + var err error + if isAccount { + status, err = getAccountAuthStatus(cmd, args, showSensitive, func(cmd *cobra.Command, args []string) (*config.Config, error) { + err := root.MustAccountClient(cmd, args) + return root.ConfigUsed(cmd.Context()), err + }) + } else { + status, err = getWorkspaceAuthStatus(cmd, args, showSensitive, func(cmd *cobra.Command, args []string) (*config.Config, error) { + err := root.MustWorkspaceClient(cmd, args) + return root.ConfigUsed(cmd.Context()), 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, error) + +func getWorkspaceAuthStatus(cmd *cobra.Command, args []string, showSensitive bool, fn tryAuth) (*authStatus, error) { + cfg, err := fn(cmd, args) + ctx := cmd.Context() + if err != nil { + return &authStatus{ + Status: "error", + Error: err, + Details: getAuthDetails(ctx, cmd, cfg, showSensitive), + }, nil + } + + w := root.WorkspaceClient(ctx) + me, err := w.CurrentUser.Me(ctx) + if err != nil { + return nil, err + } + + status := authStatus{ + Status: "success", + Details: getAuthDetails(ctx, cmd, w.Config, showSensitive), + } + status.Details.Username = me.UserName + + return &status, nil +} + +func getAccountAuthStatus(cmd *cobra.Command, args []string, showSensitive bool, fn tryAuth) (*authStatus, error) { + cfg, err := fn(cmd, args) + ctx := cmd.Context() + if err != nil { + return &authStatus{ + Status: "error", + Error: err, + Details: getAuthDetails(ctx, cmd, cfg, showSensitive), + }, nil + } + + a := root.AccountClient(ctx) + status := authStatus{ + Status: "success", + Details: getAuthDetails(ctx, cmd, a.Config, showSensitive), + } + + status.Details.AccountID = a.Config.AccountID + status.Details.Username = a.Config.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) + 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"` + Details authDetails `json:"details"` +} + +type authDetails struct { + AuthType string `json:"auth_type"` + Username string `json:"username,omitempty"` + Host string `json:"host,omitempty"` + AccountID string `json:"account_id,omitempty"` + Configuration map[string]attrConfig `json:"configuration"` +} + +type attrConfig struct { + Value string `json:"value"` + Source *config.Source `json:"source"` + AuthTypeMismatch bool `json:"auth_type_mismatch"` +} + +func getAuthDetails(ctx context.Context, cmd *cobra.Command, cfg *config.Config, showSensitive bool) authDetails { + attrSet := make(map[string]attrConfig, 0) + for _, a := range config.ConfigAttributes { + if a.IsZero(cfg) { + continue + } + + attrSet[a.Name] = attrConfig{ + Value: getValue(cfg, a, showSensitive), + Source: getSource(ctx, cmd, cfg, &a), + AuthTypeMismatch: a.IsAuthAttribute() && !slices.Contains(a.Auth, cfg.AuthType), + } + } + + return authDetails{ + AuthType: cfg.AuthType, + Host: cfg.Host, + Configuration: attrSet, + } +} + +func getValue(cfg *config.Config, a config.ConfigAttribute, showSensitive bool) string { + if !showSensitive && a.Sensitive { + return "********" + } + + return a.GetString(cfg) +} + +func getSource(ctx context.Context, cmd *cobra.Command, cfg *config.Config, a *config.ConfigAttribute) *config.Source { + v := a.GetString(cfg) + + // Check if the value is from an environment variable + for _, envVar := range a.EnvVars { + if v == env.Get(ctx, envVar) { + return &config.Source{ + Type: config.SourceEnv, + Name: envVar, + } + } + } + + // Check if the value is from command flags + // Only profile and host can be set like this + // The rest of the values are set from the config file + if a.Name == "profile" && cmd.Flag("profile").Changed { + return &config.Source{ + Type: config.SourceType("flag"), + Name: "--profile", + } + } + + if a.Name == "host" && cmd.Flag("host").Changed { + return &config.Source{ + Type: config.SourceType("flag"), + Name: "--host", + } + } + + return a.Source +} diff --git a/cmd/auth/describe_test.go b/cmd/auth/describe_test.go new file mode 100644 index 0000000000..b3f44e2949 --- /dev/null +++ b/cmd/auth/describe_test.go @@ -0,0 +1,210 @@ +package auth + +import ( + "context" + "fmt" + "testing" + + "github.com/databricks/cli/cmd/root" + "github.com/databricks/databricks-sdk-go/config" + "github.com/databricks/databricks-sdk-go/experimental/mocks" + "github.com/databricks/databricks-sdk-go/service/iam" + "github.com/spf13/cobra" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func TestGetWorkspaceAuthStatus(t *testing.T) { + ctx := context.Background() + m := mocks.NewMockWorkspaceClient(t) + ctx = root.SetWorkspaceClient(ctx, m.WorkspaceClient) + + cmd := &cobra.Command{} + cmd.SetContext(ctx) + + showSensitive := false + + currentUserApi := m.GetMockCurrentUserAPI() + currentUserApi.EXPECT().Me(mock.Anything).Return(&iam.User{ + UserName: "test-user", + }, nil) + + cmd.Flags().String("host", "", "") + cmd.Flags().String("profile", "", "") + cmd.Flag("profile").Value.Set("my-profile") + cmd.Flag("profile").Changed = true + + cfg := &config.Config{ + Profile: "my-profile", + } + m.WorkspaceClient.Config = cfg + t.Setenv("DATABRICKS_AUTH_TYPE", "azure-cli") + + status, err := getWorkspaceAuthStatus(cmd, []string{}, showSensitive, func(cmd *cobra.Command, args []string) (*config.Config, error) { + config.ConfigAttributes.ResolveFromStringMap(cfg, map[string]string{ + "host": "https://test.com", + "token": "test-token", + "auth_type": "azure-cli", + }) + return cfg, nil + }) + require.NoError(t, err) + require.NotNil(t, status) + require.Equal(t, "success", status.Status) + require.Equal(t, "test-user", status.Details.Username) + require.Equal(t, "https://test.com", status.Details.Host) + require.Equal(t, "azure-cli", status.Details.AuthType) + + require.Equal(t, "azure-cli", status.Details.Configuration["auth_type"].Value) + require.Equal(t, "DATABRICKS_AUTH_TYPE environment variable", status.Details.Configuration["auth_type"].Source.String()) + require.False(t, status.Details.Configuration["auth_type"].AuthTypeMismatch) + + require.Equal(t, "********", status.Details.Configuration["token"].Value) + require.Equal(t, "dynamic configuration", status.Details.Configuration["token"].Source.String()) + require.True(t, status.Details.Configuration["token"].AuthTypeMismatch) + + require.Equal(t, "my-profile", status.Details.Configuration["profile"].Value) + require.Equal(t, "--profile flag", status.Details.Configuration["profile"].Source.String()) + require.False(t, status.Details.Configuration["profile"].AuthTypeMismatch) +} + +func TestGetWorkspaceAuthStatusError(t *testing.T) { + ctx := context.Background() + m := mocks.NewMockWorkspaceClient(t) + ctx = root.SetWorkspaceClient(ctx, m.WorkspaceClient) + + cmd := &cobra.Command{} + cmd.SetContext(ctx) + + showSensitive := false + + cmd.Flags().String("host", "", "") + cmd.Flags().String("profile", "", "") + cmd.Flag("profile").Value.Set("my-profile") + cmd.Flag("profile").Changed = true + + cfg := &config.Config{ + Profile: "my-profile", + } + m.WorkspaceClient.Config = cfg + t.Setenv("DATABRICKS_AUTH_TYPE", "azure-cli") + + status, err := getWorkspaceAuthStatus(cmd, []string{}, showSensitive, func(cmd *cobra.Command, args []string) (*config.Config, error) { + config.ConfigAttributes.ResolveFromStringMap(cfg, map[string]string{ + "host": "https://test.com", + "token": "test-token", + "auth_type": "azure-cli", + }) + return cfg, fmt.Errorf("auth error") + }) + require.NoError(t, err) + require.NotNil(t, status) + require.Equal(t, "error", status.Status) + + require.Equal(t, "azure-cli", status.Details.Configuration["auth_type"].Value) + require.Equal(t, "DATABRICKS_AUTH_TYPE environment variable", status.Details.Configuration["auth_type"].Source.String()) + require.False(t, status.Details.Configuration["auth_type"].AuthTypeMismatch) + + require.Equal(t, "********", status.Details.Configuration["token"].Value) + require.Equal(t, "dynamic configuration", status.Details.Configuration["token"].Source.String()) + require.True(t, status.Details.Configuration["token"].AuthTypeMismatch) + + require.Equal(t, "my-profile", status.Details.Configuration["profile"].Value) + require.Equal(t, "--profile flag", status.Details.Configuration["profile"].Source.String()) + require.False(t, status.Details.Configuration["profile"].AuthTypeMismatch) +} + +func TestGetWorkspaceAuthStatusSensitive(t *testing.T) { + ctx := context.Background() + m := mocks.NewMockWorkspaceClient(t) + ctx = root.SetWorkspaceClient(ctx, m.WorkspaceClient) + + cmd := &cobra.Command{} + cmd.SetContext(ctx) + + showSensitive := true + + cmd.Flags().String("host", "", "") + cmd.Flags().String("profile", "", "") + cmd.Flag("profile").Value.Set("my-profile") + cmd.Flag("profile").Changed = true + + cfg := &config.Config{ + Profile: "my-profile", + } + m.WorkspaceClient.Config = cfg + t.Setenv("DATABRICKS_AUTH_TYPE", "azure-cli") + + status, err := getWorkspaceAuthStatus(cmd, []string{}, showSensitive, func(cmd *cobra.Command, args []string) (*config.Config, error) { + config.ConfigAttributes.ResolveFromStringMap(cfg, map[string]string{ + "host": "https://test.com", + "token": "test-token", + "auth_type": "azure-cli", + }) + return cfg, fmt.Errorf("auth error") + }) + require.NoError(t, err) + require.NotNil(t, status) + require.Equal(t, "error", status.Status) + + require.Equal(t, "azure-cli", status.Details.Configuration["auth_type"].Value) + require.Equal(t, "DATABRICKS_AUTH_TYPE environment variable", status.Details.Configuration["auth_type"].Source.String()) + require.False(t, status.Details.Configuration["auth_type"].AuthTypeMismatch) + + require.Equal(t, "test-token", status.Details.Configuration["token"].Value) + require.Equal(t, "dynamic configuration", status.Details.Configuration["token"].Source.String()) + require.True(t, status.Details.Configuration["token"].AuthTypeMismatch) +} + +func TestGetAccountAuthStatus(t *testing.T) { + ctx := context.Background() + m := mocks.NewMockAccountClient(t) + ctx = root.SetAccountClient(ctx, m.AccountClient) + + cmd := &cobra.Command{} + cmd.SetContext(ctx) + + showSensitive := false + + cmd.Flags().String("host", "", "") + cmd.Flags().String("profile", "", "") + cmd.Flag("profile").Value.Set("my-profile") + cmd.Flag("profile").Changed = true + + cfg := &config.Config{ + Profile: "my-profile", + } + m.AccountClient.Config = cfg + t.Setenv("DATABRICKS_AUTH_TYPE", "azure-cli") + + status, err := getAccountAuthStatus(cmd, []string{}, showSensitive, func(cmd *cobra.Command, args []string) (*config.Config, error) { + config.ConfigAttributes.ResolveFromStringMap(cfg, map[string]string{ + "account_id": "test-account-id", + "username": "test-user", + "host": "https://test.com", + "token": "test-token", + "auth_type": "azure-cli", + }) + return cfg, nil + }) + require.NoError(t, err) + require.NotNil(t, status) + require.Equal(t, "success", status.Status) + + require.Equal(t, "test-user", status.Details.Username) + require.Equal(t, "https://test.com", status.Details.Host) + require.Equal(t, "azure-cli", status.Details.AuthType) + require.Equal(t, "test-account-id", status.Details.AccountID) + + require.Equal(t, "azure-cli", status.Details.Configuration["auth_type"].Value) + require.Equal(t, "DATABRICKS_AUTH_TYPE environment variable", status.Details.Configuration["auth_type"].Source.String()) + require.False(t, status.Details.Configuration["auth_type"].AuthTypeMismatch) + + require.Equal(t, "********", status.Details.Configuration["token"].Value) + require.Equal(t, "dynamic configuration", status.Details.Configuration["token"].Source.String()) + require.True(t, status.Details.Configuration["token"].AuthTypeMismatch) + + require.Equal(t, "my-profile", status.Details.Configuration["profile"].Value) + require.Equal(t, "--profile flag", status.Details.Configuration["profile"].Source.String()) + require.False(t, status.Details.Configuration["profile"].AuthTypeMismatch) +} diff --git a/cmd/root/auth.go b/cmd/root/auth.go index 89c7641c54..9f3d6c492f 100644 --- a/cmd/root/auth.go +++ b/cmd/root/auth.go @@ -18,6 +18,7 @@ import ( // Placeholders to use as unique keys in context.Context. var workspaceClient int var accountClient int +var configUsed int func initProfileFlag(cmd *cobra.Command) { cmd.PersistentFlags().StringP("profile", "p", "", "~/.databrickscfg profile") @@ -147,6 +148,10 @@ func MustWorkspaceClient(cmd *cobra.Command, args []string) error { cfg.Profile = profile } + ctx := cmd.Context() + ctx = context.WithValue(ctx, &configUsed, cfg) + cmd.SetContext(ctx) + // Try to load a bundle configuration if we're allowed to by the caller (see `./auth_options.go`). if !shouldSkipLoadBundle(cmd.Context()) { err := TryConfigureBundle(cmd, args) @@ -154,6 +159,8 @@ func MustWorkspaceClient(cmd *cobra.Command, args []string) error { return err } if b := bundle.GetOrNil(cmd.Context()); b != nil { + ctx = context.WithValue(ctx, &configUsed, b.Config.Workspace.Config()) + cmd.SetContext(ctx) client, err := b.InitializeWorkspaceClient() if err != nil { return err @@ -168,7 +175,6 @@ func MustWorkspaceClient(cmd *cobra.Command, args []string) error { return err } - ctx := cmd.Context() ctx = context.WithValue(ctx, &workspaceClient, w) cmd.SetContext(ctx) return nil @@ -178,6 +184,10 @@ func SetWorkspaceClient(ctx context.Context, w *databricks.WorkspaceClient) cont return context.WithValue(ctx, &workspaceClient, w) } +func SetAccountClient(ctx context.Context, a *databricks.AccountClient) context.Context { + return context.WithValue(ctx, &accountClient, a) +} + func AskForWorkspaceProfile(ctx context.Context) (string, error) { path, err := databrickscfg.GetPath(ctx) if err != nil { @@ -270,3 +280,11 @@ func AccountClient(ctx context.Context) *databricks.AccountClient { } return a } + +func ConfigUsed(ctx context.Context) *config.Config { + cfg, ok := ctx.Value(&configUsed).(*config.Config) + if !ok { + panic("cannot get *config.Config. Please report it as a bug") + } + return cfg +} diff --git a/go.mod b/go.mod index d9e6c24f08..c481bde5e4 100644 --- a/go.mod +++ b/go.mod @@ -54,6 +54,7 @@ require ( github.com/stretchr/objx v0.5.2 // indirect github.com/zclconf/go-cty v1.14.1 // indirect go.opencensus.io v0.24.0 // indirect +<<<<<<< HEAD go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.49.0 // indirect go.opentelemetry.io/otel v1.24.0 // indirect go.opentelemetry.io/otel/metric v1.24.0 // indirect @@ -67,5 +68,20 @@ require ( google.golang.org/genproto/googleapis/rpc v0.0.0-20240304161311-37d4d3c04a78 // indirect google.golang.org/grpc v1.62.0 // indirect google.golang.org/protobuf v1.33.0 // indirect +======= + go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.48.0 // indirect + go.opentelemetry.io/otel v1.23.0 // indirect + go.opentelemetry.io/otel/metric v1.23.0 // indirect + go.opentelemetry.io/otel/trace v1.23.0 // indirect + golang.org/x/crypto v0.19.0 // indirect + golang.org/x/net v0.21.0 // indirect + golang.org/x/sys v0.17.0 // indirect + golang.org/x/time v0.5.0 // indirect + google.golang.org/api v0.166.0 // indirect + google.golang.org/appengine v1.6.8 // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20240213162025-012b6fc9bca9 // indirect + google.golang.org/grpc v1.61.1 // indirect + google.golang.org/protobuf v1.32.0 // indirect +>>>>>>> d6010d7a (Added auth describe command) gopkg.in/yaml.v2 v2.4.0 // indirect ) diff --git a/libs/cmdio/render.go b/libs/cmdio/render.go index 40cdde354d..ec851b8fff 100644 --- a/libs/cmdio/render.go +++ b/libs/cmdio/render.go @@ -306,6 +306,12 @@ func renderUsingTemplate(ctx context.Context, r templateRenderer, w io.Writer, h "yellow": color.YellowString, "magenta": color.MagentaString, "cyan": color.CyanString, + "bold": func(format string, a ...interface{}) string { + return color.New(color.Bold).Sprintf(format, a...) + }, + "italic": func(format string, a ...interface{}) string { + return color.New(color.Italic).Sprintf(format, a...) + }, "replace": strings.ReplaceAll, "join": strings.Join, "bool": func(v bool) string { diff --git a/libs/databrickscfg/loader.go b/libs/databrickscfg/loader.go index 1dc2a94525..ab75b6433a 100644 --- a/libs/databrickscfg/loader.go +++ b/libs/databrickscfg/loader.go @@ -98,7 +98,10 @@ func (l profileFromHostLoader) Configure(cfg *config.Config) error { } log.Debugf(ctx, "Loading profile %s because of host match", match.Name()) - err = config.ConfigAttributes.ResolveFromStringMap(cfg, match.KeysHash()) + err = config.ConfigAttributes.ResolveFromStringMapWithSource(cfg, match.KeysHash(), &config.Source{ + Type: config.SourceFile, + Name: configFile.Path(), + }) if err != nil { return fmt.Errorf("%s %s profile: %w", configFile.Path(), match.Name(), err) } @@ -110,7 +113,7 @@ func (l profileFromHostLoader) Configure(cfg *config.Config) error { func (l profileFromHostLoader) isAnyAuthConfigured(cfg *config.Config) bool { // If any of the auth-specific attributes are set, we can skip profile resolution. for _, a := range config.ConfigAttributes { - if a.Auth == "" { + if !a.IsAuthAttribute() { continue } if !a.IsZero(cfg) { From 80927957b38ee77d656e438e3cad4557322fb466 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 4 Mar 2024 15:46:05 +0100 Subject: [PATCH 02/11] fix --- bundle/config/workspace.go | 2 +- cmd/auth/describe.go | 93 ++++++------------------------------ cmd/auth/describe_test.go | 4 ++ go.sum | 4 +- libs/databrickscfg/loader.go | 2 +- 5 files changed, 23 insertions(+), 82 deletions(-) diff --git a/bundle/config/workspace.go b/bundle/config/workspace.go index 3223408d28..a29d8e3cc2 100644 --- a/bundle/config/workspace.go +++ b/bundle/config/workspace.go @@ -104,7 +104,7 @@ func (w *Workspace) Config() *config.Config { for k := range config.ConfigAttributes { attr := &config.ConfigAttributes[k] if !attr.IsZero(cfg) { - attr.SetSource(&config.Source{Type: config.SourceType("bundle")}) + cfg.SetAttrSource(attr, &config.Source{Type: config.SourceType("bundle")}) } } diff --git a/cmd/auth/describe.go b/cmd/auth/describe.go index c924298d31..6ac9beafa3 100644 --- a/cmd/auth/describe.go +++ b/cmd/auth/describe.go @@ -4,11 +4,9 @@ import ( "context" "encoding/json" "fmt" - "slices" "github.com/databricks/cli/cmd/root" "github.com/databricks/cli/libs/cmdio" - "github.com/databricks/cli/libs/env" "github.com/databricks/cli/libs/flags" "github.com/databricks/databricks-sdk-go/config" "github.com/spf13/cobra" @@ -88,7 +86,7 @@ func getWorkspaceAuthStatus(cmd *cobra.Command, args []string, showSensitive boo return &authStatus{ Status: "error", Error: err, - Details: getAuthDetails(ctx, cmd, cfg, showSensitive), + Details: getAuthDetails(cmd, cfg, showSensitive), }, nil } @@ -100,7 +98,7 @@ func getWorkspaceAuthStatus(cmd *cobra.Command, args []string, showSensitive boo status := authStatus{ Status: "success", - Details: getAuthDetails(ctx, cmd, w.Config, showSensitive), + Details: getAuthDetails(cmd, w.Config, showSensitive), } status.Details.Username = me.UserName @@ -114,14 +112,14 @@ func getAccountAuthStatus(cmd *cobra.Command, args []string, showSensitive bool, return &authStatus{ Status: "error", Error: err, - Details: getAuthDetails(ctx, cmd, cfg, showSensitive), + Details: getAuthDetails(cmd, cfg, showSensitive), }, nil } a := root.AccountClient(ctx) status := authStatus{ Status: "success", - Details: getAuthDetails(ctx, cmd, a.Config, showSensitive), + Details: getAuthDetails(cmd, a.Config, showSensitive), } status.Details.AccountID = a.Config.AccountID @@ -147,83 +145,22 @@ func render(ctx context.Context, cmd *cobra.Command, status *authStatus, templat } type authStatus struct { - Status string `json:"status"` - Error error `json:"error,omitempty"` - Details authDetails `json:"details"` + Status string `json:"status"` + Error error `json:"error,omitempty"` + Details config.AuthDetails `json:"details"` } -type authDetails struct { - AuthType string `json:"auth_type"` - Username string `json:"username,omitempty"` - Host string `json:"host,omitempty"` - AccountID string `json:"account_id,omitempty"` - Configuration map[string]attrConfig `json:"configuration"` -} - -type attrConfig struct { - Value string `json:"value"` - Source *config.Source `json:"source"` - AuthTypeMismatch bool `json:"auth_type_mismatch"` -} - -func getAuthDetails(ctx context.Context, cmd *cobra.Command, cfg *config.Config, showSensitive bool) authDetails { - attrSet := make(map[string]attrConfig, 0) - for _, a := range config.ConfigAttributes { - if a.IsZero(cfg) { - continue - } - - attrSet[a.Name] = attrConfig{ - Value: getValue(cfg, a, showSensitive), - Source: getSource(ctx, cmd, cfg, &a), - AuthTypeMismatch: a.IsAuthAttribute() && !slices.Contains(a.Auth, cfg.AuthType), +func getAuthDetails(cmd *cobra.Command, cfg *config.Config, showSensitive bool) config.AuthDetails { + details := cfg.GetAuthDetails(showSensitive) + for k, v := range details.Configuration { + if k == "profile" && cmd.Flag("profile").Changed { + v.Source = &config.Source{Type: config.SourceType("flag"), Name: "--profile"} } - } - - return authDetails{ - AuthType: cfg.AuthType, - Host: cfg.Host, - Configuration: attrSet, - } -} - -func getValue(cfg *config.Config, a config.ConfigAttribute, showSensitive bool) string { - if !showSensitive && a.Sensitive { - return "********" - } - - return a.GetString(cfg) -} - -func getSource(ctx context.Context, cmd *cobra.Command, cfg *config.Config, a *config.ConfigAttribute) *config.Source { - v := a.GetString(cfg) - - // Check if the value is from an environment variable - for _, envVar := range a.EnvVars { - if v == env.Get(ctx, envVar) { - return &config.Source{ - Type: config.SourceEnv, - Name: envVar, - } - } - } - - // Check if the value is from command flags - // Only profile and host can be set like this - // The rest of the values are set from the config file - if a.Name == "profile" && cmd.Flag("profile").Changed { - return &config.Source{ - Type: config.SourceType("flag"), - Name: "--profile", - } - } - if a.Name == "host" && cmd.Flag("host").Changed { - return &config.Source{ - Type: config.SourceType("flag"), - Name: "--host", + if k == "host" && cmd.Flag("host").Changed { + v.Source = &config.Source{Type: config.SourceType("flag"), Name: "--host"} } } - return a.Source + return details } diff --git a/cmd/auth/describe_test.go b/cmd/auth/describe_test.go index b3f44e2949..e715ea7e80 100644 --- a/cmd/auth/describe_test.go +++ b/cmd/auth/describe_test.go @@ -39,6 +39,7 @@ func TestGetWorkspaceAuthStatus(t *testing.T) { } m.WorkspaceClient.Config = cfg t.Setenv("DATABRICKS_AUTH_TYPE", "azure-cli") + config.ConfigAttributes.Configure(cfg) status, err := getWorkspaceAuthStatus(cmd, []string{}, showSensitive, func(cmd *cobra.Command, args []string) (*config.Config, error) { config.ConfigAttributes.ResolveFromStringMap(cfg, map[string]string{ @@ -88,6 +89,7 @@ func TestGetWorkspaceAuthStatusError(t *testing.T) { } m.WorkspaceClient.Config = cfg t.Setenv("DATABRICKS_AUTH_TYPE", "azure-cli") + config.ConfigAttributes.Configure(cfg) status, err := getWorkspaceAuthStatus(cmd, []string{}, showSensitive, func(cmd *cobra.Command, args []string) (*config.Config, error) { config.ConfigAttributes.ResolveFromStringMap(cfg, map[string]string{ @@ -134,6 +136,7 @@ func TestGetWorkspaceAuthStatusSensitive(t *testing.T) { } m.WorkspaceClient.Config = cfg t.Setenv("DATABRICKS_AUTH_TYPE", "azure-cli") + config.ConfigAttributes.Configure(cfg) status, err := getWorkspaceAuthStatus(cmd, []string{}, showSensitive, func(cmd *cobra.Command, args []string) (*config.Config, error) { config.ConfigAttributes.ResolveFromStringMap(cfg, map[string]string{ @@ -176,6 +179,7 @@ func TestGetAccountAuthStatus(t *testing.T) { } m.AccountClient.Config = cfg t.Setenv("DATABRICKS_AUTH_TYPE", "azure-cli") + config.ConfigAttributes.Configure(cfg) status, err := getAccountAuthStatus(cmd, []string{}, showSensitive, func(cmd *cobra.Command, args []string) (*config.Config, error) { config.ConfigAttributes.ResolveFromStringMap(cfg, map[string]string{ diff --git a/go.sum b/go.sum index a4a6eb40b8..e338e31cd3 100644 --- a/go.sum +++ b/go.sum @@ -151,8 +151,8 @@ github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= -github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= -github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= +github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= github.com/xanzy/ssh-agent v0.3.3 h1:+/15pJfg/RsTxqYcX6fHqOXZwwMP+2VyYWJeWM2qQFM= github.com/xanzy/ssh-agent v0.3.3/go.mod h1:6dzNDKs0J9rVPHPhaGCukekBHKqfl+L3KghI1Bc68Uw= github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= diff --git a/libs/databrickscfg/loader.go b/libs/databrickscfg/loader.go index ab75b6433a..80c7dddf3c 100644 --- a/libs/databrickscfg/loader.go +++ b/libs/databrickscfg/loader.go @@ -113,7 +113,7 @@ func (l profileFromHostLoader) Configure(cfg *config.Config) error { func (l profileFromHostLoader) isAnyAuthConfigured(cfg *config.Config) bool { // If any of the auth-specific attributes are set, we can skip profile resolution. for _, a := range config.ConfigAttributes { - if !a.IsAuthAttribute() { + if !a.HasAuthAttribute() { continue } if !a.IsZero(cfg) { From 1cd5db3a226b967317a3ebed2e782ebbf8349689 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Fri, 8 Mar 2024 15:16:03 +0100 Subject: [PATCH 03/11] use updated sdk --- bundle/config/workspace.go | 2 +- cmd/auth/describe.go | 37 +++++++++++++++++++++--------------- go.mod | 16 ---------------- go.sum | 4 ++-- libs/databrickscfg/loader.go | 2 +- 5 files changed, 26 insertions(+), 35 deletions(-) diff --git a/bundle/config/workspace.go b/bundle/config/workspace.go index a29d8e3cc2..efc5caa663 100644 --- a/bundle/config/workspace.go +++ b/bundle/config/workspace.go @@ -104,7 +104,7 @@ func (w *Workspace) Config() *config.Config { for k := range config.ConfigAttributes { attr := &config.ConfigAttributes[k] if !attr.IsZero(cfg) { - cfg.SetAttrSource(attr, &config.Source{Type: config.SourceType("bundle")}) + cfg.SetAttrSource(attr, config.Source{Type: config.SourceType("bundle")}) } } diff --git a/cmd/auth/describe.go b/cmd/auth/describe.go index 6ac9beafa3..442b1541d2 100644 --- a/cmd/auth/describe.go +++ b/cmd/auth/describe.go @@ -13,9 +13,9 @@ import ( ) var authTemplate = fmt.Sprintf(`{{"Workspace:" | bold}} {{.Details.Host}}. -{{if .Details.Username }}{{"User:" | bold}} {{.Details.Username}}.{{end}} +{{if .Username }}{{"User:" | bold}} {{.Username}}.{{end}} {{"Authenticated with:" | bold}} {{.Details.AuthType}} -{{if .Details.AccountID }}{{"Account ID:" | bold}} {{.Details.AccountID}}.{{end}} +{{if .AccountID }}{{"Account ID:" | bold}} {{.AccountID}}.{{end}} ----- %s `, configurationTemplate) @@ -97,10 +97,10 @@ func getWorkspaceAuthStatus(cmd *cobra.Command, args []string, showSensitive boo } status := authStatus{ - Status: "success", - Details: getAuthDetails(cmd, w.Config, showSensitive), + Status: "success", + Details: getAuthDetails(cmd, w.Config, showSensitive), + Username: me.UserName, } - status.Details.Username = me.UserName return &status, nil } @@ -118,12 +118,12 @@ func getAccountAuthStatus(cmd *cobra.Command, args []string, showSensitive bool, a := root.AccountClient(ctx) status := authStatus{ - Status: "success", - Details: getAuthDetails(cmd, a.Config, showSensitive), + Status: "success", + Details: getAuthDetails(cmd, a.Config, showSensitive), + AccountID: a.Config.AccountID, + Username: a.Config.Username, } - status.Details.AccountID = a.Config.AccountID - status.Details.Username = a.Config.Username return &status, nil } @@ -145,20 +145,27 @@ func render(ctx context.Context, cmd *cobra.Command, status *authStatus, templat } type authStatus struct { - Status string `json:"status"` - Error error `json:"error,omitempty"` - Details config.AuthDetails `json:"details"` + 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 { - details := cfg.GetAuthDetails(showSensitive) + var opts []config.AuthDetailsOptions + if showSensitive { + opts = append(opts, config.ShowSensitive) + } + details := cfg.GetAuthDetails(opts...) + for k, v := range details.Configuration { if k == "profile" && cmd.Flag("profile").Changed { - v.Source = &config.Source{Type: config.SourceType("flag"), Name: "--profile"} + 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"} + v.Source = config.Source{Type: config.SourceType("flag"), Name: "--host"} } } diff --git a/go.mod b/go.mod index c481bde5e4..d9e6c24f08 100644 --- a/go.mod +++ b/go.mod @@ -54,7 +54,6 @@ require ( github.com/stretchr/objx v0.5.2 // indirect github.com/zclconf/go-cty v1.14.1 // indirect go.opencensus.io v0.24.0 // indirect -<<<<<<< HEAD go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.49.0 // indirect go.opentelemetry.io/otel v1.24.0 // indirect go.opentelemetry.io/otel/metric v1.24.0 // indirect @@ -68,20 +67,5 @@ require ( google.golang.org/genproto/googleapis/rpc v0.0.0-20240304161311-37d4d3c04a78 // indirect google.golang.org/grpc v1.62.0 // indirect google.golang.org/protobuf v1.33.0 // indirect -======= - go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.48.0 // indirect - go.opentelemetry.io/otel v1.23.0 // indirect - go.opentelemetry.io/otel/metric v1.23.0 // indirect - go.opentelemetry.io/otel/trace v1.23.0 // indirect - golang.org/x/crypto v0.19.0 // indirect - golang.org/x/net v0.21.0 // indirect - golang.org/x/sys v0.17.0 // indirect - golang.org/x/time v0.5.0 // indirect - google.golang.org/api v0.166.0 // indirect - google.golang.org/appengine v1.6.8 // indirect - google.golang.org/genproto/googleapis/rpc v0.0.0-20240213162025-012b6fc9bca9 // indirect - google.golang.org/grpc v1.61.1 // indirect - google.golang.org/protobuf v1.32.0 // indirect ->>>>>>> d6010d7a (Added auth describe command) gopkg.in/yaml.v2 v2.4.0 // indirect ) diff --git a/go.sum b/go.sum index e338e31cd3..a4a6eb40b8 100644 --- a/go.sum +++ b/go.sum @@ -151,8 +151,8 @@ github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= -github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= -github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= +github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= +github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/xanzy/ssh-agent v0.3.3 h1:+/15pJfg/RsTxqYcX6fHqOXZwwMP+2VyYWJeWM2qQFM= github.com/xanzy/ssh-agent v0.3.3/go.mod h1:6dzNDKs0J9rVPHPhaGCukekBHKqfl+L3KghI1Bc68Uw= github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= diff --git a/libs/databrickscfg/loader.go b/libs/databrickscfg/loader.go index 80c7dddf3c..2e22ee9502 100644 --- a/libs/databrickscfg/loader.go +++ b/libs/databrickscfg/loader.go @@ -98,7 +98,7 @@ func (l profileFromHostLoader) Configure(cfg *config.Config) error { } log.Debugf(ctx, "Loading profile %s because of host match", match.Name()) - err = config.ConfigAttributes.ResolveFromStringMapWithSource(cfg, match.KeysHash(), &config.Source{ + err = config.ConfigAttributes.ResolveFromStringMapWithSource(cfg, match.KeysHash(), config.Source{ Type: config.SourceFile, Name: configFile.Path(), }) From d560094d3a079f8fd13e2826e4baabaf26304389 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Fri, 8 Mar 2024 15:24:32 +0100 Subject: [PATCH 04/11] fixed tests --- cmd/auth/describe_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/auth/describe_test.go b/cmd/auth/describe_test.go index e715ea7e80..e46d171c77 100644 --- a/cmd/auth/describe_test.go +++ b/cmd/auth/describe_test.go @@ -52,7 +52,7 @@ func TestGetWorkspaceAuthStatus(t *testing.T) { require.NoError(t, err) require.NotNil(t, status) require.Equal(t, "success", status.Status) - require.Equal(t, "test-user", status.Details.Username) + require.Equal(t, "test-user", status.Username) require.Equal(t, "https://test.com", status.Details.Host) require.Equal(t, "azure-cli", status.Details.AuthType) @@ -195,10 +195,10 @@ func TestGetAccountAuthStatus(t *testing.T) { require.NotNil(t, status) require.Equal(t, "success", status.Status) - require.Equal(t, "test-user", status.Details.Username) + require.Equal(t, "test-user", status.Username) require.Equal(t, "https://test.com", status.Details.Host) require.Equal(t, "azure-cli", status.Details.AuthType) - require.Equal(t, "test-account-id", status.Details.AccountID) + require.Equal(t, "test-account-id", status.AccountID) require.Equal(t, "azure-cli", status.Details.Configuration["auth_type"].Value) require.Equal(t, "DATABRICKS_AUTH_TYPE environment variable", status.Details.Configuration["auth_type"].Source.String()) From c56cb6ee96145fb53a737fe6a5ed871cd73c79fc Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 25 Mar 2024 15:35:40 +0100 Subject: [PATCH 05/11] fixes --- cmd/auth/describe.go | 23 +++++++++++++++-------- cmd/root/auth.go | 7 ++++++- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/cmd/auth/describe.go b/cmd/auth/describe.go index 442b1541d2..ceceb55c5c 100644 --- a/cmd/auth/describe.go +++ b/cmd/auth/describe.go @@ -12,24 +12,22 @@ import ( "github.com/spf13/cobra" ) -var authTemplate = fmt.Sprintf(`{{"Workspace:" | bold}} {{.Details.Host}}. +var authTemplate = `{{"Workspace:" | bold}} {{.Details.Host}}. {{if .Username }}{{"User:" | bold}} {{.Username}}.{{end}} {{"Authenticated with:" | bold}} {{.Details.AuthType}} {{if .AccountID }}{{"Account ID:" | bold}} {{.AccountID}}.{{end}} ----- -%s -`, configurationTemplate) +` + configurationTemplate -var errorTemplate = fmt.Sprintf(`Unable to authenticate: {{.Error}} +var errorTemplate = `Unable to authenticate: {{.Error}} ----- -%s -`, configurationTemplate) +` + configurationTemplate const configurationTemplate = `Configuration: {{- $details := .Details}} {{- range $k, $v := $details.Configuration }} - {{if $v.AuthTypeMismatch}}x{{else}}✓{{end}} {{$k | bold}}: {{$v.Value}} - {{- if $v.Source }} + {{if $v.AuthTypeMismatch}}~{{else}}✓{{end}} {{$k | bold}}: {{$v.Value}} + {{- if and (not (eq $v.Source.String "dynamic configuration")) $v.Source }} {{- " (from" | italic}} {{$v.Source.String | italic}} {{- if $v.AuthTypeMismatch}}, {{ "not used for auth type " | red | italic }}{{$details.AuthType | red | italic}}{{end}}) {{- end}} @@ -169,5 +167,14 @@ func getAuthDetails(cmd *cobra.Command, cfg *config.Config, showSensitive bool) } } + // 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}} + } + return details } diff --git a/cmd/root/auth.go b/cmd/root/auth.go index 9f3d6c492f..0f07114139 100644 --- a/cmd/root/auth.go +++ b/cmd/root/auth.go @@ -78,6 +78,10 @@ func MustAccountClient(cmd *cobra.Command, args []string) error { cfg.Profile = profile } + ctx := cmd.Context() + ctx = context.WithValue(ctx, &configUsed, cfg) + cmd.SetContext(ctx) + if cfg.Profile == "" { // account-level CLI was not really done before, so here are the assumptions: // 1. only admins will have account configured @@ -100,7 +104,8 @@ func MustAccountClient(cmd *cobra.Command, args []string) error { return err } - cmd.SetContext(context.WithValue(cmd.Context(), &accountClient, a)) + ctx = context.WithValue(ctx, &accountClient, a) + cmd.SetContext(ctx) return nil } From 42e937429d2857f54a9cb97112fe17baacc3a75d Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Mon, 25 Mar 2024 15:39:03 +0100 Subject: [PATCH 06/11] no trailing dots --- cmd/auth/describe.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/auth/describe.go b/cmd/auth/describe.go index ceceb55c5c..cfcaa3265d 100644 --- a/cmd/auth/describe.go +++ b/cmd/auth/describe.go @@ -12,10 +12,10 @@ import ( "github.com/spf13/cobra" ) -var authTemplate = `{{"Workspace:" | bold}} {{.Details.Host}}. -{{if .Username }}{{"User:" | bold}} {{.Username}}.{{end}} +var authTemplate = `{{"Workspace:" | bold}} {{.Details.Host}} +{{if .Username }}{{"User:" | bold}} {{.Username}}{{end}} {{"Authenticated with:" | bold}} {{.Details.AuthType}} -{{if .AccountID }}{{"Account ID:" | bold}} {{.AccountID}}.{{end}} +{{if .AccountID }}{{"Account ID:" | bold}} {{.AccountID}}{{end}} ----- ` + configurationTemplate From 755b68397204a57c5d2f5afb47cf84955a6387b7 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Tue, 26 Mar 2024 10:35:11 +0100 Subject: [PATCH 07/11] Update cmd/auth/describe.go Co-authored-by: Julia Crawford (Databricks) --- cmd/auth/describe.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/auth/describe.go b/cmd/auth/describe.go index cfcaa3265d..57cd4b6918 100644 --- a/cmd/auth/describe.go +++ b/cmd/auth/describe.go @@ -37,7 +37,7 @@ const configurationTemplate = `Configuration: func newDescribeCommand() *cobra.Command { cmd := &cobra.Command{ Use: "describe", - Short: "Describes which auth credentials is being used by the CLI and identify their source", + Short: "Describes the credentials and the source of those credentials, being used by the CLI to authenticate", } var showSensitive bool From 8af0a4e46966cc5fc5be1641570197473910151f Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Thu, 28 Mar 2024 14:31:50 +0100 Subject: [PATCH 08/11] remove --account flag --- cmd/auth/describe.go | 76 +++++++++++++++++--------------------- cmd/auth/describe_test.go | 16 ++++---- cmd/root/auth.go | 43 +++++++++++++++++++-- cmd/root/auth_test.go | 78 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 159 insertions(+), 54 deletions(-) diff --git a/cmd/auth/describe.go b/cmd/auth/describe.go index 57cd4b6918..3e9711eec3 100644 --- a/cmd/auth/describe.go +++ b/cmd/auth/describe.go @@ -12,10 +12,14 @@ import ( "github.com/spf13/cobra" ) -var authTemplate = `{{"Workspace:" | bold}} {{.Details.Host}} -{{if .Username }}{{"User:" | bold}} {{.Username}}{{end}} +var authTemplate = `{{"Host:" | bold}} {{.Details.Host}} +{{- if .Username}} +{{"User:" | bold}} {{.Username}} +{{- end}} {{"Authenticated with:" | bold}} {{.Details.AuthType}} -{{if .AccountID }}{{"Account ID:" | bold}} {{.AccountID}}{{end}} +{{- if .AccountID}} +{{"Account ID:" | bold}} {{.AccountID}} +{{- end}} ----- ` + configurationTemplate @@ -23,11 +27,11 @@ var errorTemplate = `Unable to authenticate: {{.Error}} ----- ` + configurationTemplate -const configurationTemplate = `Configuration: +const configurationTemplate = `Current configuration: {{- $details := .Details}} {{- range $k, $v := $details.Configuration }} {{if $v.AuthTypeMismatch}}~{{else}}✓{{end}} {{$k | bold}}: {{$v.Value}} - {{- if and (not (eq $v.Source.String "dynamic configuration")) $v.Source }} + {{- 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}} @@ -41,25 +45,16 @@ func newDescribeCommand() *cobra.Command { } var showSensitive bool - var isAccount bool cmd.Flags().BoolVar(&showSensitive, "sensitive", false, "Include sensitive fields like passwords and tokens in the output") - cmd.Flags().BoolVar(&isAccount, "account", false, "Describe the account client instead of the workspace client") cmd.RunE = func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() var status *authStatus var err error - if isAccount { - status, err = getAccountAuthStatus(cmd, args, showSensitive, func(cmd *cobra.Command, args []string) (*config.Config, error) { - err := root.MustAccountClient(cmd, args) - return root.ConfigUsed(cmd.Context()), err - }) - } else { - status, err = getWorkspaceAuthStatus(cmd, args, showSensitive, func(cmd *cobra.Command, args []string) (*config.Config, error) { - err := root.MustWorkspaceClient(cmd, args) - return root.ConfigUsed(cmd.Context()), err - }) - } + 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 @@ -75,10 +70,10 @@ func newDescribeCommand() *cobra.Command { return cmd } -type tryAuth func(cmd *cobra.Command, args []string) (*config.Config, error) +type tryAuth func(cmd *cobra.Command, args []string) (*config.Config, bool, error) -func getWorkspaceAuthStatus(cmd *cobra.Command, args []string, showSensitive bool, fn tryAuth) (*authStatus, error) { - cfg, err := fn(cmd, args) +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{ @@ -88,6 +83,18 @@ func getWorkspaceAuthStatus(cmd *cobra.Command, args []string, showSensitive boo }, 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, + } + + return &status, nil + } + w := root.WorkspaceClient(ctx) me, err := w.CurrentUser.Me(ctx) if err != nil { @@ -103,28 +110,6 @@ func getWorkspaceAuthStatus(cmd *cobra.Command, args []string, showSensitive boo return &status, nil } -func getAccountAuthStatus(cmd *cobra.Command, args []string, showSensitive bool, fn tryAuth) (*authStatus, error) { - cfg, err := fn(cmd, args) - ctx := cmd.Context() - if err != nil { - return &authStatus{ - Status: "error", - Error: err, - Details: getAuthDetails(cmd, cfg, showSensitive), - }, nil - } - - a := root.AccountClient(ctx) - status := authStatus{ - Status: "success", - Details: getAuthDetails(cmd, a.Config, showSensitive), - AccountID: a.Config.AccountID, - Username: a.Config.Username, - } - - return &status, nil -} - func render(ctx context.Context, cmd *cobra.Command, status *authStatus, template string) error { switch root.OutputType(cmd) { case flags.OutputText: @@ -176,5 +161,10 @@ func getAuthDetails(cmd *cobra.Command, cfg *config.Config, showSensitive bool) details.Configuration["profile"] = &config.AttrConfig{Value: profile, Source: config.Source{Type: config.SourceDynamicConfig}} } + // 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 } diff --git a/cmd/auth/describe_test.go b/cmd/auth/describe_test.go index e46d171c77..e06120e6ec 100644 --- a/cmd/auth/describe_test.go +++ b/cmd/auth/describe_test.go @@ -41,13 +41,13 @@ func TestGetWorkspaceAuthStatus(t *testing.T) { t.Setenv("DATABRICKS_AUTH_TYPE", "azure-cli") config.ConfigAttributes.Configure(cfg) - status, err := getWorkspaceAuthStatus(cmd, []string{}, showSensitive, func(cmd *cobra.Command, args []string) (*config.Config, error) { + status, err := getAuthStatus(cmd, []string{}, showSensitive, func(cmd *cobra.Command, args []string) (*config.Config, bool, error) { config.ConfigAttributes.ResolveFromStringMap(cfg, map[string]string{ "host": "https://test.com", "token": "test-token", "auth_type": "azure-cli", }) - return cfg, nil + return cfg, false, nil }) require.NoError(t, err) require.NotNil(t, status) @@ -91,13 +91,13 @@ func TestGetWorkspaceAuthStatusError(t *testing.T) { t.Setenv("DATABRICKS_AUTH_TYPE", "azure-cli") config.ConfigAttributes.Configure(cfg) - status, err := getWorkspaceAuthStatus(cmd, []string{}, showSensitive, func(cmd *cobra.Command, args []string) (*config.Config, error) { + status, err := getAuthStatus(cmd, []string{}, showSensitive, func(cmd *cobra.Command, args []string) (*config.Config, bool, error) { config.ConfigAttributes.ResolveFromStringMap(cfg, map[string]string{ "host": "https://test.com", "token": "test-token", "auth_type": "azure-cli", }) - return cfg, fmt.Errorf("auth error") + return cfg, false, fmt.Errorf("auth error") }) require.NoError(t, err) require.NotNil(t, status) @@ -138,13 +138,13 @@ func TestGetWorkspaceAuthStatusSensitive(t *testing.T) { t.Setenv("DATABRICKS_AUTH_TYPE", "azure-cli") config.ConfigAttributes.Configure(cfg) - status, err := getWorkspaceAuthStatus(cmd, []string{}, showSensitive, func(cmd *cobra.Command, args []string) (*config.Config, error) { + status, err := getAuthStatus(cmd, []string{}, showSensitive, func(cmd *cobra.Command, args []string) (*config.Config, bool, error) { config.ConfigAttributes.ResolveFromStringMap(cfg, map[string]string{ "host": "https://test.com", "token": "test-token", "auth_type": "azure-cli", }) - return cfg, fmt.Errorf("auth error") + return cfg, false, fmt.Errorf("auth error") }) require.NoError(t, err) require.NotNil(t, status) @@ -181,7 +181,7 @@ func TestGetAccountAuthStatus(t *testing.T) { t.Setenv("DATABRICKS_AUTH_TYPE", "azure-cli") config.ConfigAttributes.Configure(cfg) - status, err := getAccountAuthStatus(cmd, []string{}, showSensitive, func(cmd *cobra.Command, args []string) (*config.Config, error) { + status, err := getAuthStatus(cmd, []string{}, showSensitive, func(cmd *cobra.Command, args []string) (*config.Config, bool, error) { config.ConfigAttributes.ResolveFromStringMap(cfg, map[string]string{ "account_id": "test-account-id", "username": "test-user", @@ -189,7 +189,7 @@ func TestGetAccountAuthStatus(t *testing.T) { "token": "test-token", "auth_type": "azure-cli", }) - return cfg, nil + return cfg, true, nil }) require.NoError(t, err) require.NotNil(t, status) diff --git a/cmd/root/auth.go b/cmd/root/auth.go index 62be0a3918..26275c7270 100644 --- a/cmd/root/auth.go +++ b/cmd/root/auth.go @@ -19,6 +19,22 @@ var workspaceClient int var accountClient int var configUsed int +type ErrNoWorkspaceProfiles struct { + path string +} + +func (e ErrNoWorkspaceProfiles) Error() string { + return fmt.Sprintf("%s does not contain workspace profiles; please create one by running 'databricks configure'", e.path) +} + +type ErrNoAccountProfiles struct { + path string +} + +func (e ErrNoAccountProfiles) Error() string { + return fmt.Sprintf("%s does not contain account profiles; please create one by running 'databricks configure'", e.path) +} + func initProfileFlag(cmd *cobra.Command) { cmd.PersistentFlags().StringP("profile", "p", "", "~/.databrickscfg profile") cmd.RegisterFlagCompletionFunc("profile", databrickscfg.ProfileCompletion) @@ -68,6 +84,27 @@ func accountClientOrPrompt(ctx context.Context, cfg *config.Config, allowPrompt return a, err } +func MustAnyClient(cmd *cobra.Command, args []string) (bool, error) { + // Try to create a workspace client + werr := MustWorkspaceClient(cmd, args) + if werr == nil { + return false, nil + } + + // If the error is not a workspace client error, return it because configuration is for workspace client + if !errors.Is(werr, databricks.ErrNotWorkspaceClient) && !errors.As(werr, &ErrNoWorkspaceProfiles{}) { + return false, werr + } + + // Otherwise, the config used is account client one, so try to create an account client + aerr := MustAccountClient(cmd, args) + if errors.As(aerr, &ErrNoAccountProfiles{}) { + return false, fmt.Errorf("%s does not contain any profiles; please create one by running 'databricks configure'", aerr.(ErrNoAccountProfiles).path) + } + + return true, aerr +} + func MustAccountClient(cmd *cobra.Command, args []string) error { cfg := &config.Config{} @@ -164,7 +201,7 @@ func MustWorkspaceClient(cmd *cobra.Command, args []string) error { } if b != nil { - ctx = context.WithValue(ctx, &configUsed, b.Config.Workspace.Config()) + ctx = context.WithValue(ctx, &configUsed, b.Config.Workspace.Config()) cmd.SetContext(ctx) client, err := b.InitializeWorkspaceClient() if err != nil { @@ -204,7 +241,7 @@ func AskForWorkspaceProfile(ctx context.Context) (string, error) { } switch len(profiles) { case 0: - return "", fmt.Errorf("%s does not contain workspace profiles; please create one by running 'databricks configure'", path) + return "", ErrNoWorkspaceProfiles{path: path} case 1: return profiles[0].Name, nil } @@ -237,7 +274,7 @@ func AskForAccountProfile(ctx context.Context) (string, error) { } switch len(profiles) { case 0: - return "", fmt.Errorf("%s does not contain account profiles; please create one by running 'databricks configure'", path) + return "", ErrNoAccountProfiles{path} case 1: return profiles[0].Name, nil } diff --git a/cmd/root/auth_test.go b/cmd/root/auth_test.go index 7864c254ee..8f8cb05a31 100644 --- a/cmd/root/auth_test.go +++ b/cmd/root/auth_test.go @@ -229,3 +229,81 @@ func TestMustAccountClientErrorsWithNoDatabricksCfg(t *testing.T) { err := MustAccountClient(cmd, []string{}) require.ErrorContains(t, err, "no configuration file found at") } + +func TestMustAnyClientCanCreateWorkspaceClient(t *testing.T) { + testutil.CleanupEnvironment(t) + + dir := t.TempDir() + configFile := filepath.Join(dir, ".databrickscfg") + err := os.WriteFile( + configFile, + []byte(` + [workspace-1111] + host = https://adb-1111.11.azuredatabricks.net/ + token = foobar + `), + 0755) + require.NoError(t, err) + + ctx, tt := cmdio.SetupTest(context.Background()) + t.Cleanup(tt.Done) + cmd := New(ctx) + + t.Setenv("DATABRICKS_CONFIG_FILE", configFile) + isAccount, err := MustAnyClient(cmd, []string{}) + require.False(t, isAccount) + require.NoError(t, err) + + w := WorkspaceClient(cmd.Context()) + require.NotNil(t, w) +} + +func TestMustAnyClientCanCreateAccountClient(t *testing.T) { + testutil.CleanupEnvironment(t) + + dir := t.TempDir() + configFile := filepath.Join(dir, ".databrickscfg") + err := os.WriteFile( + configFile, + []byte(` + [account-1111] + host = https://accounts.azuredatabricks.net/ + account_id = 1111 + token = foobar + `), + 0755) + require.NoError(t, err) + + ctx, tt := cmdio.SetupTest(context.Background()) + t.Cleanup(tt.Done) + cmd := New(ctx) + + t.Setenv("DATABRICKS_CONFIG_FILE", configFile) + isAccount, err := MustAnyClient(cmd, []string{}) + require.NoError(t, err) + require.True(t, isAccount) + + a := AccountClient(cmd.Context()) + require.NotNil(t, a) +} + +func TestMustAnyClientWithEmptyDatabricksCfg(t *testing.T) { + testutil.CleanupEnvironment(t) + + dir := t.TempDir() + configFile := filepath.Join(dir, ".databrickscfg") + err := os.WriteFile( + configFile, + []byte(""), // empty file + 0755) + require.NoError(t, err) + + ctx, tt := cmdio.SetupTest(context.Background()) + t.Cleanup(tt.Done) + cmd := New(ctx) + + t.Setenv("DATABRICKS_CONFIG_FILE", configFile) + + _, err = MustAnyClient(cmd, []string{}) + require.ErrorContains(t, err, "does not contain any profiles; please create one by running 'databricks configure'") +} From 566bc90a859dc8e144dd842efcd601f7531a8d4b Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Tue, 2 Apr 2024 16:45:36 +0200 Subject: [PATCH 09/11] fixes --- cmd/auth/describe.go | 36 +++++++++++++++++++++++++++++------- cmd/auth/describe_test.go | 3 +++ cmd/root/auth.go | 4 ++-- 3 files changed, 34 insertions(+), 9 deletions(-) diff --git a/cmd/auth/describe.go b/cmd/auth/describe.go index 3e9711eec3..125b0731a6 100644 --- a/cmd/auth/describe.go +++ b/cmd/auth/describe.go @@ -13,13 +13,13 @@ import ( ) var authTemplate = `{{"Host:" | bold}} {{.Details.Host}} +{{- if .AccountID}} +{{"Account ID:" | bold}} {{.AccountID}} +{{- end}} {{- if .Username}} {{"User:" | bold}} {{.Username}} {{- end}} {{"Authenticated with:" | bold}} {{.Details.AuthType}} -{{- if .AccountID}} -{{"Account ID:" | bold}} {{.AccountID}} -{{- end}} ----- ` + configurationTemplate @@ -28,14 +28,18 @@ var errorTemplate = `Unable to authenticate: {{.Error}} ` + configurationTemplate const configurationTemplate = `Current configuration: - {{- $details := .Details}} - {{- range $k, $v := $details.Configuration }} + {{- $details := .Status.Details}} + {{- range $a := .ConfigAttributes}} + {{- $k := $a.Name}} + {{- if index $details.Configuration $k}} + {{- $v := index $details.Configuration $k}} {{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}} + {{- end}} ` func newDescribeCommand() *cobra.Command { @@ -85,6 +89,17 @@ func getAuthStatus(cmd *cobra.Command, args []string, showSensitive bool, fn try if isAccount { a := root.AccountClient(ctx) + + // Doing a simple API call to check if the auth is valid + _, err := a.Workspaces.List(ctx) + if err != nil { + return &authStatus{ + Status: "error", + Error: err, + Details: getAuthDetails(cmd, cfg, showSensitive), + }, nil + } + status := authStatus{ Status: "success", Details: getAuthDetails(cmd, a.Config, showSensitive), @@ -98,7 +113,11 @@ func getAuthStatus(cmd *cobra.Command, args []string, showSensitive bool, fn try w := root.WorkspaceClient(ctx) me, err := w.CurrentUser.Me(ctx) if err != nil { - return nil, err + return &authStatus{ + Status: "error", + Error: err, + Details: getAuthDetails(cmd, cfg, showSensitive), + }, nil } status := authStatus{ @@ -113,7 +132,10 @@ func getAuthStatus(cmd *cobra.Command, args []string, showSensitive bool, fn try 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) + return cmdio.RenderWithTemplate(ctx, map[string]any{ + "Status": status, + "ConfigAttributes": config.ConfigAttributes, + }, "", template) case flags.OutputJSON: buf, err := json.MarshalIndent(status, "", " ") if err != nil { diff --git a/cmd/auth/describe_test.go b/cmd/auth/describe_test.go index e06120e6ec..d0260abc7b 100644 --- a/cmd/auth/describe_test.go +++ b/cmd/auth/describe_test.go @@ -181,6 +181,9 @@ func TestGetAccountAuthStatus(t *testing.T) { t.Setenv("DATABRICKS_AUTH_TYPE", "azure-cli") config.ConfigAttributes.Configure(cfg) + wsApi := m.GetMockWorkspacesAPI() + wsApi.EXPECT().List(mock.Anything).Return(nil, nil) + status, err := getAuthStatus(cmd, []string{}, showSensitive, func(cmd *cobra.Command, args []string) (*config.Config, bool, error) { config.ConfigAttributes.ResolveFromStringMap(cfg, map[string]string{ "account_id": "test-account-id", diff --git a/cmd/root/auth.go b/cmd/root/auth.go index 26275c7270..6e6225389a 100644 --- a/cmd/root/auth.go +++ b/cmd/root/auth.go @@ -32,7 +32,7 @@ type ErrNoAccountProfiles struct { } func (e ErrNoAccountProfiles) Error() string { - return fmt.Sprintf("%s does not contain account profiles; please create one by running 'databricks configure'", e.path) + return fmt.Sprintf("%s does not contain account profiles", e.path) } func initProfileFlag(cmd *cobra.Command) { @@ -99,7 +99,7 @@ func MustAnyClient(cmd *cobra.Command, args []string) (bool, error) { // Otherwise, the config used is account client one, so try to create an account client aerr := MustAccountClient(cmd, args) if errors.As(aerr, &ErrNoAccountProfiles{}) { - return false, fmt.Errorf("%s does not contain any profiles; please create one by running 'databricks configure'", aerr.(ErrNoAccountProfiles).path) + return false, aerr } return true, aerr From 6413e2414f567c34a8215da1d30b678b42188298 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Tue, 2 Apr 2024 16:49:47 +0200 Subject: [PATCH 10/11] fixed comment --- cmd/root/auth.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cmd/root/auth.go b/cmd/root/auth.go index 6e6225389a..387b67f0d7 100644 --- a/cmd/root/auth.go +++ b/cmd/root/auth.go @@ -91,7 +91,9 @@ func MustAnyClient(cmd *cobra.Command, args []string) (bool, error) { return false, nil } - // If the error is not a workspace client error, return it because configuration is for workspace client + // If the error is other than "not a workspace client error" or "no workspace profiles", + // return it because configuration is for workspace client + // and we don't want to try to create an account client. if !errors.Is(werr, databricks.ErrNotWorkspaceClient) && !errors.As(werr, &ErrNoWorkspaceProfiles{}) { return false, werr } From edcc7e592203e80ceb385d414a4fc8844664dd69 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Wed, 3 Apr 2024 10:08:31 +0200 Subject: [PATCH 11/11] fix test --- cmd/root/auth_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/root/auth_test.go b/cmd/root/auth_test.go index 8f8cb05a31..486f587ef3 100644 --- a/cmd/root/auth_test.go +++ b/cmd/root/auth_test.go @@ -305,5 +305,5 @@ func TestMustAnyClientWithEmptyDatabricksCfg(t *testing.T) { t.Setenv("DATABRICKS_CONFIG_FILE", configFile) _, err = MustAnyClient(cmd, []string{}) - require.ErrorContains(t, err, "does not contain any profiles; please create one by running 'databricks configure'") + require.ErrorContains(t, err, "does not contain account profiles") }