bootstrapping does not ask for ssh password key if empty is introduce…

…d by the user (#3738) * not asking ssh if empty string * adding some documentation * fixed flag name Signed-off-by: Eneko Fernandez <[email protected]> --------- Signed-off-by: Eneko Fernandez <[email protected]>
weaveworks · Dec 19, 2023 · bfe1cc1 · bfe1cc1
1 parent ad7fe6b
commit bfe1cc1
Show file tree

Hide file tree

Showing 7 changed files with 125 additions and 65 deletions.
diff --git a/cmd/gitops/app/bootstrap/cmd.go b/cmd/gitops/app/bootstrap/cmd.go
@@ -123,6 +123,7 @@ func getBootstrapCmdRun(opts *config.Options) func(*cobra.Command, []string) err
 			).
 			WithGitAuthentication(flags.privateKeyPath,
 				flags.privateKeyPassword,
+				cmd.Flag("private-key-password").Changed,
 				flags.gitUsername,
 				flags.gitPassword,
 			).

diff --git a/cmd/gitops/app/bootstrap/cmd_auth.go b/cmd/gitops/app/bootstrap/cmd_auth.go
@@ -64,6 +64,7 @@ func getAuthCmdRun(opts *config.Options) func(*cobra.Command, []string) error {
 			).
 			WithGitAuthentication(flags.privateKeyPath,
 				flags.privateKeyPassword,
+				cmd.Flag("private-key-password").Changed,
 				flags.gitUsername,
 				flags.gitPassword,
 			).

diff --git a/docs/cli/bootstrap.md b/docs/cli/bootstrap.md
@@ -246,6 +246,7 @@ Note: If you find yourself adding common behaviour in this function think on whe
 **Inputs**
 
 - We usually prefix input names with `in` prefix (short for input) to distinguish these constants from everything else.
+- Check the usage of `PrivateKeyPassword` and `PrivateKeyPasswordChanged` as example of how to handle flags with empty strings as default values.
 
 ## How configuration works ?
 

diff --git a/pkg/bootstrap/steps/bootstrap_flux.go b/pkg/bootstrap/steps/bootstrap_flux.go
@@ -68,7 +68,9 @@ func NewBootstrapFlux(config Config) BootstrapStep {
 	if config.PrivateKeyPath == "" {
 		inputs = append(inputs, getKeyPath)
 	}
-	if config.PrivateKeyPassword == "" {
+
+	// we need to ask if empty password comes by default
+	if config.PrivateKeyPassword == "" && !config.PrivateKeyPasswordChanged {
 		inputs = append(inputs, getKeyPassword)
 	}
 

diff --git a/pkg/bootstrap/steps/bootstrap_flux_test.go b/pkg/bootstrap/steps/bootstrap_flux_test.go
@@ -3,9 +3,54 @@ package steps
 import (
 	"testing"
 
-	"github.com/alecthomas/assert"
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+	"github.com/stretchr/testify/assert"
 )
 
+func TestNewBootstrapFlux(t *testing.T) {
+	tests := []struct {
+		name      string
+		config    Config
+		wantInput []StepInput
+	}{
+		{
+			name: "should not ask for key password if user introduced empty flag",
+			config: MakeTestConfig(t, Config{
+				PrivateKeyPassword:        "",
+				PrivateKeyPasswordChanged: true,
+			}),
+			wantInput: []StepInput{
+				getKeyPath,
+				getGitUsername,
+				getGitPassword,
+			},
+		},
+		{
+			name: "should ask for key password if user not introduced",
+			config: MakeTestConfig(t, Config{
+				PrivateKeyPassword:        "",
+				PrivateKeyPasswordChanged: false,
+			}),
+			wantInput: []StepInput{
+				getKeyPath,
+				getKeyPassword,
+				getGitUsername,
+				getGitPassword,
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := NewBootstrapFlux(tt.config)
+			opts := cmpopts.IgnoreFields(StepInput{}, "Enabled")
+			if diff := cmp.Diff(tt.wantInput, got.Input, opts); diff != "" {
+				t.Fatalf("unpected step inputs:\n%s", diff)
+			}
+		})
+	}
+}
+
 func TestConfigureFluxCreds(t *testing.T) {
 
 	tests := []struct {

diff --git a/pkg/bootstrap/steps/common_tests.go b/pkg/bootstrap/steps/common_tests.go
@@ -42,30 +42,31 @@ func MakeTestConfig(t *testing.T, config Config, objects ...runtime.Object) Conf
 	cliLogger := utils.CreateLogger()
 
 	return Config{
-		KubernetesClient:        fakeClient,
-		GitClient:               fakeGitClient{},
-		FluxClient:              fakeFluxClient{},
-		InReader:                config.InReader,
-		OutWriter:               config.OutWriter,
-		Logger:                  cliLogger,
-		WgeConfig:               config.WgeConfig,
-		ModesConfig:             config.ModesConfig,
-		ClusterUserAuth:         config.ClusterUserAuth,
-		GitRepository:           config.GitRepository,
-		FluxConfig:              config.FluxConfig,
-		PrivateKeyPath:          config.PrivateKeyPath,
-		PrivateKeyPassword:      config.PrivateKeyPassword,
-		GitUsername:             config.GitUsername,
-		GitToken:                config.GitToken,
-		AuthType:                config.AuthType,
-		InstallOIDC:             config.InstallOIDC,
-		DiscoveryURL:            config.DiscoveryURL,
-		IssuerURL:               config.IssuerURL,
-		ClientID:                config.ClientID,
-		ClientSecret:            config.ClientSecret,
-		RedirectURL:             config.RedirectURL,
-		PromptedForDiscoveryURL: config.PromptedForDiscoveryURL,
-		ComponentsExtra:         config.ComponentsExtra,
+		KubernetesClient:          fakeClient,
+		GitClient:                 fakeGitClient{},
+		FluxClient:                fakeFluxClient{},
+		InReader:                  config.InReader,
+		OutWriter:                 config.OutWriter,
+		Logger:                    cliLogger,
+		WgeConfig:                 config.WgeConfig,
+		ModesConfig:               config.ModesConfig,
+		ClusterUserAuth:           config.ClusterUserAuth,
+		GitRepository:             config.GitRepository,
+		FluxConfig:                config.FluxConfig,
+		PrivateKeyPath:            config.PrivateKeyPath,
+		PrivateKeyPassword:        config.PrivateKeyPassword,
+		PrivateKeyPasswordChanged: config.PrivateKeyPasswordChanged,
+		GitUsername:               config.GitUsername,
+		GitToken:                  config.GitToken,
+		AuthType:                  config.AuthType,
+		InstallOIDC:               config.InstallOIDC,
+		DiscoveryURL:              config.DiscoveryURL,
+		IssuerURL:                 config.IssuerURL,
+		ClientID:                  config.ClientID,
+		ClientSecret:              config.ClientSecret,
+		RedirectURL:               config.RedirectURL,
+		PromptedForDiscoveryURL:   config.PromptedForDiscoveryURL,
+		ComponentsExtra:           config.ComponentsExtra,
 	}
 }
 

diff --git a/pkg/bootstrap/steps/config.go b/pkg/bootstrap/steps/config.go
@@ -59,29 +59,32 @@ const (
 
 // ConfigBuilder contains all the different configuration options that a user can introduce
 type ConfigBuilder struct {
-	logger                  logger.Logger
-	kubeconfig              string
-	password                string
-	wgeVersion              string
-	privateKeyPath          string
-	privateKeyPassword      string
-	silent                  bool
-	export                  bool
-	gitUsername             string
-	gitToken                string
-	repoURL                 string
-	repoBranch              string
-	repoPath                string
-	authType                string
-	installOIDC             string
-	discoveryURL            string
-	clientID                string
-	clientSecret            string
-	PromptedForDiscoveryURL bool
-	bootstrapFlux           bool
-	componentsExtra         []string
-	outWriter               io.Writer
-	inReader                io.Reader
+	logger             logger.Logger
+	kubeconfig         string
+	password           string
+	wgeVersion         string
+	privateKeyPath     string
+	privateKeyPassword string
+	// privateKeyPasswordChanged indicates true when the value privateKeyPassword
+	// comes from the user input. false otherwise.
+	privateKeyPasswordChanged bool
+	silent                    bool
+	export                    bool
+	gitUsername               string
+	gitToken                  string
+	repoURL                   string
+	repoBranch                string
+	repoPath                  string
+	authType                  string
+	installOIDC               string
+	discoveryURL              string
+	clientID                  string
+	clientSecret              string
+	PromptedForDiscoveryURL   bool
+	bootstrapFlux             bool
+	componentsExtra           []string
+	outWriter                 io.Writer
+	inReader                  io.Reader
 }
 
 func NewConfigBuilder() *ConfigBuilder {
@@ -108,9 +111,11 @@ func (c *ConfigBuilder) WithVersion(version string) *ConfigBuilder {
 	return c
 }
 
-func (c *ConfigBuilder) WithGitAuthentication(privateKeyPath, privateKeyPassword, gitUsername, gitToken string) *ConfigBuilder {
+func (c *ConfigBuilder) WithGitAuthentication(privateKeyPath, privateKeyPassword string, privateKeyPasswordChanged bool,
+	gitUsername, gitToken string) *ConfigBuilder {
 	c.privateKeyPath = privateKeyPath
 	c.privateKeyPassword = privateKeyPassword
+	c.privateKeyPasswordChanged = privateKeyPasswordChanged
 	c.gitUsername = gitUsername
 	c.gitToken = gitToken
 
@@ -186,9 +191,12 @@ type Config struct {
 	ClusterUserAuth ClusterUserAuthConfig
 	ModesConfig     ModesConfig
 
-	PrivateKeyPath     string
-	PrivateKeyPassword string
+	// TODO refactor me to git ssh auth config type
+	PrivateKeyPath            string
+	PrivateKeyPassword        string
+	PrivateKeyPasswordChanged bool
 
+	// TODO refactor me to git https auth config type
 	GitUsername string
 	GitToken    string
 
@@ -272,19 +280,20 @@ func (cb *ConfigBuilder) Build() (Config, error) {
 			Silent: cb.silent,
 			Export: cb.export,
 		},
-		PrivateKeyPath:          cb.privateKeyPath,
-		PrivateKeyPassword:      cb.privateKeyPassword,
-		GitUsername:             cb.gitUsername,
-		GitToken:                cb.gitToken,
-		AuthType:                cb.authType,
-		InstallOIDC:             cb.installOIDC,
-		DiscoveryURL:            cb.discoveryURL,
-		ClientID:                cb.clientID,
-		ClientSecret:            cb.clientSecret,
-		PromptedForDiscoveryURL: cb.PromptedForDiscoveryURL,
-		ComponentsExtra:         componentsExtraConfig,
-		FluxConfig:              fluxConfig,
-		BootstrapFlux:           cb.bootstrapFlux,
+		PrivateKeyPath:            cb.privateKeyPath,
+		PrivateKeyPassword:        cb.privateKeyPassword,
+		PrivateKeyPasswordChanged: cb.privateKeyPasswordChanged,
+		GitUsername:               cb.gitUsername,
+		GitToken:                  cb.gitToken,
+		AuthType:                  cb.authType,
+		InstallOIDC:               cb.installOIDC,
+		DiscoveryURL:              cb.discoveryURL,
+		ClientID:                  cb.clientID,
+		ClientSecret:              cb.clientSecret,
+		PromptedForDiscoveryURL:   cb.PromptedForDiscoveryURL,
+		ComponentsExtra:           componentsExtraConfig,
+		FluxConfig:                fluxConfig,
+		BootstrapFlux:             cb.bootstrapFlux,
 	}, nil
 
 }