From 2efd734f7b25a97accc08fc9bd0d5f6cc7e82920 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 19 Oct 2023 13:03:30 +0200 Subject: [PATCH 1/3] Skip prompt on completion hook --- cmd/root/auth.go | 20 ++--------------- cmd/root/auth_options.go | 41 +++++++++++++++++++++++++++++++++++ cmd/root/auth_options_test.go | 16 ++++++++++++++ cmd/sync/sync.go | 3 +-- 4 files changed, 60 insertions(+), 20 deletions(-) create mode 100644 cmd/root/auth_options.go create mode 100644 cmd/root/auth_options_test.go diff --git a/cmd/root/auth.go b/cmd/root/auth.go index ed91090e38..1e051aef8c 100644 --- a/cmd/root/auth.go +++ b/cmd/root/auth.go @@ -92,8 +92,7 @@ func MustAccountClient(cmd *cobra.Command, args []string) error { } } - noPrompt, ok := cmd.Context().Value(noPromptKey).(bool) - allowPrompt := !hasProfileFlag && (!ok || !noPrompt) + allowPrompt := !hasProfileFlag && !shouldSkipPrompt(cmd.Context()) a, err := accountClientOrPrompt(cmd.Context(), cfg, allowPrompt) if err != nil { return err @@ -103,21 +102,6 @@ func MustAccountClient(cmd *cobra.Command, args []string) error { return nil } -type noPrompt int - -var noPromptKey noPrompt - -// NoPrompt allows to skip prompt for profile configuration in MustWorkspaceClient. -// -// When calling MustWorkspaceClient we want to be able to customise if to show prompt or not. -// Since we can't change function interface, in the code we only have an access to `cmd“ object. -// Command struct does not have any state flag which indicates that it's being called in completion mode and -// thus the Context object seems to be the only viable option for us to configure prompt behaviour based on -// the context it's executed from. -func NoPrompt(ctx context.Context) context.Context { - return context.WithValue(ctx, noPromptKey, true) -} - // Helper function to create a workspace client or prompt once if the given configuration is not valid. func workspaceClientOrPrompt(ctx context.Context, cfg *config.Config, allowPrompt bool) (*databricks.WorkspaceClient, error) { w, err := databricks.NewWorkspaceClient((*databricks.Config)(cfg)) @@ -174,7 +158,7 @@ func MustWorkspaceClient(cmd *cobra.Command, args []string) error { cfg = currentBundle.WorkspaceClient().Config } - allowPrompt := !hasProfileFlag + allowPrompt := !hasProfileFlag && !shouldSkipPrompt(cmd.Context()) w, err := workspaceClientOrPrompt(cmd.Context(), cfg, allowPrompt) if err != nil { return err diff --git a/cmd/root/auth_options.go b/cmd/root/auth_options.go new file mode 100644 index 0000000000..7c223e8884 --- /dev/null +++ b/cmd/root/auth_options.go @@ -0,0 +1,41 @@ +package root + +import ( + "context" +) + +type skipPrompt int + +var skipPromptKey skipPrompt + +// SkipPrompt allows to skip prompt for profile configuration in MustWorkspaceClient. +// +// When calling MustWorkspaceClient we want to be able to customise if to show prompt or not. +// Since we can't change function interface, in the code we only have an access to `cmd` object. +// Command struct does not have any state flag which indicates that it's being called in completion mode and +// thus the Context object seems to be the only viable option for us to configure prompt behaviour based on +// the context it's executed from. +func SkipPrompt(ctx context.Context) context.Context { + return context.WithValue(ctx, skipPromptKey, true) +} + +// shouldSkipPrompt returns whether or not [SkipPrompt] has been set on the specified context. +func shouldSkipPrompt(ctx context.Context) bool { + skipPrompt, ok := ctx.Value(skipPromptKey).(bool) + return ok && skipPrompt +} + +type skipLoadBundle int + +var skipLoadBundleKey skipLoadBundle + +// skipLoadBundle tells [MustWorkspaceClient to never try and load a bundle for configuration options. +// +// When calling MustWorkspaceClient we want to be able to customise if to show prompt or not. +// Since we can't change function interface, in the code we only have an access to `cmd“ object. +// Command struct does not have any state flag which indicates that it's being called in completion mode and +// thus the Context object seems to be the only viable option for us to configure prompt behaviour based on +// the context it's executed from. +func SkipLoadBundle(ctx context.Context) context.Context { + return context.WithValue(ctx, skipLoadBundleKey, true) +} diff --git a/cmd/root/auth_options_test.go b/cmd/root/auth_options_test.go new file mode 100644 index 0000000000..61c68acc9c --- /dev/null +++ b/cmd/root/auth_options_test.go @@ -0,0 +1,16 @@ +package root + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestSkipPrompt(t *testing.T) { + ctx := context.Background() + assert.False(t, shouldSkipPrompt(ctx)) + + ctx = SkipPrompt(ctx) + assert.True(t, shouldSkipPrompt(ctx)) +} diff --git a/cmd/sync/sync.go b/cmd/sync/sync.go index 5416b57359..dffa53456d 100644 --- a/cmd/sync/sync.go +++ b/cmd/sync/sync.go @@ -149,8 +149,7 @@ func New() *cobra.Command { } cmd.ValidArgsFunction = func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { - ctx := cmd.Context() - cmd.SetContext(root.NoPrompt(ctx)) + cmd.SetContext(root.SkipPrompt(cmd.Context())) err := root.MustWorkspaceClient(cmd, args) if err != nil { From 6eef04c1e1e29e5ff639a864ec8615b701426ef3 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 19 Oct 2023 13:07:06 +0200 Subject: [PATCH 2/3] Remove bundle related code --- cmd/root/auth_options.go | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/cmd/root/auth_options.go b/cmd/root/auth_options.go index 7c223e8884..3961f1aa54 100644 --- a/cmd/root/auth_options.go +++ b/cmd/root/auth_options.go @@ -24,18 +24,3 @@ func shouldSkipPrompt(ctx context.Context) bool { skipPrompt, ok := ctx.Value(skipPromptKey).(bool) return ok && skipPrompt } - -type skipLoadBundle int - -var skipLoadBundleKey skipLoadBundle - -// skipLoadBundle tells [MustWorkspaceClient to never try and load a bundle for configuration options. -// -// When calling MustWorkspaceClient we want to be able to customise if to show prompt or not. -// Since we can't change function interface, in the code we only have an access to `cmd“ object. -// Command struct does not have any state flag which indicates that it's being called in completion mode and -// thus the Context object seems to be the only viable option for us to configure prompt behaviour based on -// the context it's executed from. -func SkipLoadBundle(ctx context.Context) context.Context { - return context.WithValue(ctx, skipLoadBundleKey, true) -} From d22b61019674183f8220afbcdda84bda74b3ee7e Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 19 Oct 2023 13:19:09 +0200 Subject: [PATCH 3/3] Never load authentication configuration from bundle for sync command --- cmd/root/auth.go | 19 +++++++++---------- cmd/root/auth_options.go | 19 +++++++++++++++++++ cmd/root/auth_options_test.go | 8 ++++++++ cmd/sync/sync.go | 10 ++++++++-- 4 files changed, 44 insertions(+), 12 deletions(-) diff --git a/cmd/root/auth.go b/cmd/root/auth.go index 1e051aef8c..81c7147923 100644 --- a/cmd/root/auth.go +++ b/cmd/root/auth.go @@ -146,16 +146,15 @@ func MustWorkspaceClient(cmd *cobra.Command, args []string) error { cfg.Profile = profile } - // try configuring a bundle - err := TryConfigureBundle(cmd, args) - if err != nil { - return err - } - - // and load the config from there - currentBundle := bundle.GetOrNil(cmd.Context()) - if currentBundle != nil { - cfg = currentBundle.WorkspaceClient().Config + // 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) + if err != nil { + return err + } + if b := bundle.GetOrNil(cmd.Context()); b != nil { + cfg = b.WorkspaceClient().Config + } } allowPrompt := !hasProfileFlag && !shouldSkipPrompt(cmd.Context()) diff --git a/cmd/root/auth_options.go b/cmd/root/auth_options.go index 3961f1aa54..701900d4f6 100644 --- a/cmd/root/auth_options.go +++ b/cmd/root/auth_options.go @@ -24,3 +24,22 @@ func shouldSkipPrompt(ctx context.Context) bool { skipPrompt, ok := ctx.Value(skipPromptKey).(bool) return ok && skipPrompt } + +type skipLoadBundle int + +var skipLoadBundleKey skipLoadBundle + +// SkipLoadBundle instructs [MustWorkspaceClient] to never try and load a bundle for configuration options. +// +// This is used for the sync command, where we need to ensure that a bundle configuration never taints +// the authentication setup as prepared in the environment (by our VS Code extension). +// Once the VS Code extension fully builds on bundles, we can remove this check again. +func SkipLoadBundle(ctx context.Context) context.Context { + return context.WithValue(ctx, skipLoadBundleKey, true) +} + +// shouldSkipLoadBundle returns whether or not [SkipLoadBundle] has been set on the specified context. +func shouldSkipLoadBundle(ctx context.Context) bool { + skipLoadBundle, ok := ctx.Value(skipLoadBundleKey).(bool) + return ok && skipLoadBundle +} diff --git a/cmd/root/auth_options_test.go b/cmd/root/auth_options_test.go index 61c68acc9c..477c2029e9 100644 --- a/cmd/root/auth_options_test.go +++ b/cmd/root/auth_options_test.go @@ -14,3 +14,11 @@ func TestSkipPrompt(t *testing.T) { ctx = SkipPrompt(ctx) assert.True(t, shouldSkipPrompt(ctx)) } + +func TestSkipLoadBundle(t *testing.T) { + ctx := context.Background() + assert.False(t, shouldSkipLoadBundle(ctx)) + + ctx = SkipLoadBundle(ctx) + assert.True(t, shouldSkipLoadBundle(ctx)) +} diff --git a/cmd/sync/sync.go b/cmd/sync/sync.go index dffa53456d..2870e1e0f3 100644 --- a/cmd/sync/sync.go +++ b/cmd/sync/sync.go @@ -89,7 +89,13 @@ func New() *cobra.Command { cmd.Flags().BoolVar(&f.watch, "watch", false, "watch local file system for changes") cmd.Flags().Var(&f.output, "output", "type of output format") - cmd.PreRunE = root.MustWorkspaceClient + // Wrapper for [root.MustWorkspaceClient] that disables loading authentication configuration from a bundle. + mustWorkspaceClient := func(cmd *cobra.Command, args []string) error { + cmd.SetContext(root.SkipLoadBundle(cmd.Context())) + return root.MustWorkspaceClient(cmd, args) + } + + cmd.PreRunE = mustWorkspaceClient cmd.RunE = func(cmd *cobra.Command, args []string) error { var opts *sync.SyncOptions var err error @@ -151,7 +157,7 @@ func New() *cobra.Command { cmd.ValidArgsFunction = func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { cmd.SetContext(root.SkipPrompt(cmd.Context())) - err := root.MustWorkspaceClient(cmd, args) + err := mustWorkspaceClient(cmd, args) if err != nil { return nil, cobra.ShellCompDirectiveError }