diff --git a/cmd/gitops/app/bootstrap/cmd.go b/cmd/gitops/app/bootstrap/cmd.go index a873d7340e..7d0827fcdb 100644 --- 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 index 7ec1218080..20fc2de487 100644 --- 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 index fcfefa28f6..5592c4bea9 100644 --- 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 index cd6d7f24c9..72ab62a7b7 100644 --- 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 index f59f6a5d2e..2c48fb2f42 100644 --- 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 index ffeee5a123..6fc519ad3f 100644 --- 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 index c695c26cf3..7f1eebeeeb 100644 --- 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 }