From c056ce0c09e7526d8d0a894e7d4da80e69302d22 Mon Sep 17 00:00:00 2001 From: Steven Crespo Date: Thu, 29 May 2025 09:09:26 -0700 Subject: [PATCH 1/2] feat: add prompt to proceed if http proxy is set and https proxy unset --- cmd/installer/cli/install.go | 27 +++++++- cmd/installer/cli/install_runpreflights.go | 3 +- cmd/installer/cli/install_test.go | 71 ++++++++++++++++++++++ 3 files changed, 97 insertions(+), 4 deletions(-) diff --git a/cmd/installer/cli/install.go b/cmd/installer/cli/install.go index d18f75016..72afe623e 100644 --- a/cmd/installer/cli/install.go +++ b/cmd/installer/cli/install.go @@ -513,7 +513,7 @@ func waitForInstallAPI(ctx context.Context, addr string) error { } func runInstall(ctx context.Context, name string, flags InstallCmdFlags, metricsReporter preflights.MetricsReporter) error { - if err := runInstallVerifyAndPrompt(ctx, name, &flags); err != nil { + if err := runInstallVerifyAndPrompt(ctx, name, &flags, prompts.New()); err != nil { return err } @@ -678,7 +678,7 @@ func markUIInstallComplete(password string, managerPort int) error { return nil } -func runInstallVerifyAndPrompt(ctx context.Context, name string, flags *InstallCmdFlags) error { +func runInstallVerifyAndPrompt(ctx context.Context, name string, flags *InstallCmdFlags, prompt prompts.Prompt) error { logrus.Debugf("checking if k0s is already installed") err := verifyNoInstallation(name, "reinstall") if err != nil { @@ -703,7 +703,7 @@ func runInstallVerifyAndPrompt(ctx context.Context, name string, flags *InstallC } if !flags.isAirgap { - if err := maybePromptForAppUpdate(ctx, prompts.New(), license, flags.assumeYes); err != nil { + if err := maybePromptForAppUpdate(ctx, prompt, license, flags.assumeYes); err != nil { if errors.As(err, &ErrorNothingElseToAdd{}) { return err } @@ -713,6 +713,11 @@ func runInstallVerifyAndPrompt(ctx context.Context, name string, flags *InstallC } } + if err := promptProceedWithPartialProxyConfig(flags.proxy, prompt); err != nil { + return err + } + logrus.Debug("User confirmed prompt to proceed installing with `http_proxy` set and `https_proxy` unset") + if err := preflights.ValidateApp(); err != nil { return err } @@ -1098,6 +1103,22 @@ func maybePromptForAppUpdate(ctx context.Context, prompt prompts.Prompt, license return nil } +// promptProceedWithPartialProxyConfig prompts for confirmation when HTTP proxy is set without HTTPS proxy, +// returning an error if the user declines to proceed. +func promptProceedWithPartialProxyConfig(proxy *ecv1beta1.ProxySpec, prompt prompts.Prompt) error { + if proxy != nil && proxy.HTTPProxy != "" && proxy.HTTPSProxy == "" { + message := "Typically --https-proxy should be set if --http-proxy is set. Installation failures are likely otherwise. Do you want to continue anyway?" + confirmed, err := prompt.Confirm(message, false) + if err != nil { + return fmt.Errorf("failed to confirm proxy settings: %w", err) + } + if !confirmed { + return NewErrorNothingElseToAdd(errors.New("user aborted: HTTP proxy configured without HTTPS proxy")) + } + } + return nil +} + // Minimum character length for the Admin Console password const minAdminPasswordLength = 6 diff --git a/cmd/installer/cli/install_runpreflights.go b/cmd/installer/cli/install_runpreflights.go index 21ffae70b..dad9f9583 100644 --- a/cmd/installer/cli/install_runpreflights.go +++ b/cmd/installer/cli/install_runpreflights.go @@ -8,6 +8,7 @@ import ( "github.com/replicatedhq/embedded-cluster/pkg/configutils" "github.com/replicatedhq/embedded-cluster/pkg/netutils" "github.com/replicatedhq/embedded-cluster/pkg/preflights" + "github.com/replicatedhq/embedded-cluster/pkg/prompts" "github.com/replicatedhq/embedded-cluster/pkg/runtimeconfig" "github.com/sirupsen/logrus" "github.com/spf13/cobra" @@ -50,7 +51,7 @@ func InstallRunPreflightsCmd(ctx context.Context, name string) *cobra.Command { } func runInstallRunPreflights(ctx context.Context, name string, flags InstallCmdFlags) error { - if err := runInstallVerifyAndPrompt(ctx, name, &flags); err != nil { + if err := runInstallVerifyAndPrompt(ctx, name, &flags, prompts.New()); err != nil { return err } diff --git a/cmd/installer/cli/install_test.go b/cmd/installer/cli/install_test.go index 908d52e56..db93e2cbf 100644 --- a/cmd/installer/cli/install_test.go +++ b/cmd/installer/cli/install_test.go @@ -15,6 +15,7 @@ import ( "time" "github.com/replicatedhq/embedded-cluster/api" + ecv1beta1 "github.com/replicatedhq/embedded-cluster/kinds/apis/v1beta1" "github.com/replicatedhq/embedded-cluster/pkg-new/tlsutils" "github.com/replicatedhq/embedded-cluster/pkg/prompts" "github.com/replicatedhq/embedded-cluster/pkg/prompts/plain" @@ -592,3 +593,73 @@ func Test_runInstallAPI(t *testing.T) { assert.ErrorIs(t, <-errCh, http.ErrServerClosed) t.Logf("Install API exited") } + +func Test_promptIncompleteProxyConfig(t *testing.T) { + tests := []struct { + name string + proxy *ecv1beta1.ProxySpec + confirm bool + wantErr bool + isErrNothingElseToAdd bool + }{ + { + name: "no proxy set", + proxy: nil, + wantErr: false, + }, + { + name: "http proxy set without https proxy and user confirms", + proxy: &ecv1beta1.ProxySpec{ + HTTPProxy: "http://proxy:8080", + }, + confirm: true, + wantErr: false, + }, + { + name: "http proxy set without https proxy and user declines", + proxy: &ecv1beta1.ProxySpec{ + HTTPProxy: "http://proxy:8080", + }, + confirm: false, + wantErr: true, + isErrNothingElseToAdd: true, + }, + { + name: "both proxies set", + proxy: &ecv1beta1.ProxySpec{ + HTTPProxy: "http://proxy:8080", + HTTPSProxy: "https://proxy:8080", + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var in *bytes.Buffer + if tt.confirm { + in = bytes.NewBuffer([]byte("y\n")) + } else { + in = bytes.NewBuffer([]byte("n\n")) + } + out := bytes.NewBuffer([]byte{}) + mockPrompt := plain.New(plain.WithIn(in), plain.WithOut(out)) + + prompts.SetTerminal(true) + t.Cleanup(func() { prompts.SetTerminal(false) }) + + err := promptProceedWithPartialProxyConfig(tt.proxy, mockPrompt) + if tt.wantErr { + require.Error(t, err) + if tt.isErrNothingElseToAdd { + assert.ErrorAs(t, err, &ErrorNothingElseToAdd{}) + } + if tt.proxy != nil && tt.proxy.HTTPProxy != "" && tt.proxy.HTTPSProxy == "" { + assert.Contains(t, out.String(), "Typically --https-proxy should be set if --http-proxy is set") + } + } else { + assert.NoError(t, err) + } + }) + } +} From a2c0a704a6b0edd75ff7a11b4808f0cbf206b304 Mon Sep 17 00:00:00 2001 From: Steven Crespo Date: Thu, 29 May 2025 14:15:07 -0700 Subject: [PATCH 2/2] rename function and use assumeYes to bypass prompt --- cmd/installer/cli/install.go | 8 ++++---- cmd/installer/cli/install_test.go | 15 ++++++++++++--- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/cmd/installer/cli/install.go b/cmd/installer/cli/install.go index 6358c59aa..afe65160f 100644 --- a/cmd/installer/cli/install.go +++ b/cmd/installer/cli/install.go @@ -722,7 +722,7 @@ func runInstallVerifyAndPrompt(ctx context.Context, name string, flags *InstallC } } - if err := promptProceedWithPartialProxyConfig(flags.proxy, prompt); err != nil { + if err := verifyProxyConfig(flags.proxy, prompt, flags.assumeYes); err != nil { return err } logrus.Debug("User confirmed prompt to proceed installing with `http_proxy` set and `https_proxy` unset") @@ -1112,10 +1112,10 @@ func maybePromptForAppUpdate(ctx context.Context, prompt prompts.Prompt, license return nil } -// promptProceedWithPartialProxyConfig prompts for confirmation when HTTP proxy is set without HTTPS proxy, +// verifyProxyConfig prompts for confirmation when HTTP proxy is set without HTTPS proxy, // returning an error if the user declines to proceed. -func promptProceedWithPartialProxyConfig(proxy *ecv1beta1.ProxySpec, prompt prompts.Prompt) error { - if proxy != nil && proxy.HTTPProxy != "" && proxy.HTTPSProxy == "" { +func verifyProxyConfig(proxy *ecv1beta1.ProxySpec, prompt prompts.Prompt, assumeYes bool) error { + if proxy != nil && proxy.HTTPProxy != "" && proxy.HTTPSProxy == "" && !assumeYes { message := "Typically --https-proxy should be set if --http-proxy is set. Installation failures are likely otherwise. Do you want to continue anyway?" confirmed, err := prompt.Confirm(message, false) if err != nil { diff --git a/cmd/installer/cli/install_test.go b/cmd/installer/cli/install_test.go index 531f437bf..cd1560f03 100644 --- a/cmd/installer/cli/install_test.go +++ b/cmd/installer/cli/install_test.go @@ -618,11 +618,12 @@ kind: Application t.Logf("Install API exited") } -func Test_promptIncompleteProxyConfig(t *testing.T) { +func Test_verifyProxyConfig(t *testing.T) { tests := []struct { name string proxy *ecv1beta1.ProxySpec confirm bool + assumeYes bool wantErr bool isErrNothingElseToAdd bool }{ @@ -648,6 +649,14 @@ func Test_promptIncompleteProxyConfig(t *testing.T) { wantErr: true, isErrNothingElseToAdd: true, }, + { + name: "http proxy set without https proxy and assumeYes is true", + proxy: &ecv1beta1.ProxySpec{ + HTTPProxy: "http://proxy:8080", + }, + assumeYes: true, + wantErr: false, + }, { name: "both proxies set", proxy: &ecv1beta1.ProxySpec{ @@ -672,13 +681,13 @@ func Test_promptIncompleteProxyConfig(t *testing.T) { prompts.SetTerminal(true) t.Cleanup(func() { prompts.SetTerminal(false) }) - err := promptProceedWithPartialProxyConfig(tt.proxy, mockPrompt) + err := verifyProxyConfig(tt.proxy, mockPrompt, tt.assumeYes) if tt.wantErr { require.Error(t, err) if tt.isErrNothingElseToAdd { assert.ErrorAs(t, err, &ErrorNothingElseToAdd{}) } - if tt.proxy != nil && tt.proxy.HTTPProxy != "" && tt.proxy.HTTPSProxy == "" { + if tt.proxy != nil && tt.proxy.HTTPProxy != "" && tt.proxy.HTTPSProxy == "" && !tt.assumeYes { assert.Contains(t, out.String(), "Typically --https-proxy should be set if --http-proxy is set") } } else {