Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Onboarding CLI | Flux bootstrapping and git authentication credentials #3578

Merged
merged 21 commits into from
Nov 9, 2023
Merged
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 37 additions & 7 deletions cmd/gitops/app/bootstrap/cmd.go
Original file line number Diff line number Diff line change
@@ -32,6 +32,12 @@ gitops bootstrap --password=hell0!

# Start WGE installation using OIDC
gitops bootstrap --client-id <client-id> --client-secret <client-secret> --discovery-url <discovery-url>

# Start WGE installation with OIDC and flux bootstrap with https
gitops bootstrap --version=<version> --password=<admin-password> --discovery-url=<oidc-discovery-url> --client-id=<oidc-client-id> --git-username=<git-username-https> -gitPassword=<gitPassword>--branch=<git-branch> --repo-path=<path-in-repo-for-management-cluster> --repo-url=https://<repo-url> --client-secret=<oidc-secret> -s

# Start WGE installation with OIDC and flux bootstrap with ssh
gitops bootstrap --version=<version> --password=<admin-password> --discovery-url=<oidc-discovery-url> --client-id=<oidc-client-id> --private-key-path=<private-key-path> --private-key-password=<private-key-password> --branch=<git-branch> --repo-path=<path-in-repo-for-management-cluster> --repo-url=ssh://<repo-url> --client-secret=<oidc-secret> -s
`
)

@@ -43,14 +49,26 @@ type bootstrapFlags struct {
domainType string
domain string

// private key flags
// ssh git auth flags
privateKeyPath string
privateKeyPassword string

// https git auth flags
gitUsername string
gitPassword string

// git repo flags
repoURL string
branch string
repoPath string

// oidc flags
discoveryURL string
clientID string
clientSecret string

// modes flags
silent bool
}

var flags bootstrapFlags
@@ -65,24 +83,27 @@ func Command(opts *config.Options) *cobra.Command {
}

cmd.Flags().StringVarP(&flags.domainType, "domain-type", "t", "", "dashboard domain type: could be 'localhost' or 'externaldns'")
cmd.Flags().StringVarP(&flags.domain, "domain", "d", "", "indicate the domain to use in case of using `externaldns`")
cmd.Flags().StringVarP(&flags.domain, "domain", "d", "", "the domain to access the dashboard in case of using externaldns")
cmd.Flags().StringVarP(&flags.version, "version", "v", "", "version of Weave GitOps Enterprise (should be from the latest 3 versions)")
cmd.PersistentFlags().BoolVarP(&flags.silent, "bootstrap-flux", "s", false, "always choose yes for interactive questions")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if yes as default is a safe choice. let review in the standup the list of questions and understand the side effects of it.

it also feel an important decision to record as ADR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we could add a hint that's this option is currently for testing only

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking into the questions of confirmyes we have:

  • admin creds yes means to reuse existing ones (which makes sense as default option)
  • oidc yes means to install it (where the default option makes sense to be no)
Screenshot 2023-11-09 at 09 29 47

suggestion would be to change semantics for this flag from always yes to default option if not introduced input

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

follow up: change the silent semantics to use default behaviour where the default behaviour for each step should be the most conservative one: dont do a state change or mutation

for now:

  • we ship it in this release
  • in the release notes we add a knowns saying that if cli silent is enabled it will install by default oidc.

cmd.PersistentFlags().StringVarP(&flags.gitUsername, "git-username", "", "", "git username used in https authentication type")
cmd.PersistentFlags().StringVarP(&flags.gitPassword, "git-password", "", "", "git password/token used in https authentication type")
cmd.PersistentFlags().StringVarP(&flags.branch, "branch", "b", "", "git branch for your flux repository (example: main)")
cmd.PersistentFlags().StringVarP(&flags.repoPath, "repo-path", "r", "", "git path for your flux repository (example: clusters/my-cluster)")
cmd.PersistentFlags().StringVarP(&flags.repoURL, "repo-url", "", "", "git repo url for your flux repository (example: ssh://[email protected]/my-org-name/my-repo-name or https://github.com/my-org-name/my-repo-name)")
cmd.PersistentFlags().StringVarP(&flags.privateKeyPath, "private-key", "k", "", "private key path. This key will be used to push the Weave GitOps Enterprise's resources to the default cluster repository")
cmd.PersistentFlags().StringVarP(&flags.privateKeyPassword, "private-key-password", "c", "", "private key password. If the private key is encrypted using password")
cmd.PersistentFlags().StringVarP(&flags.discoveryURL, "discovery-url", "", "", "OIDC discovery URL")
cmd.PersistentFlags().StringVarP(&flags.clientID, "client-id", "i", "", "OIDC client ID")
cmd.PersistentFlags().StringVarP(&flags.clientSecret, "client-secret", "s", "", "OIDC client secret")

cmd.PersistentFlags().StringVarP(&flags.clientSecret, "client-secret", "", "", "OIDC client secret")
cmd.AddCommand(AuthCommand(opts))

return cmd
}

func getBootstrapCmdRun(opts *config.Options) func(*cobra.Command, []string) error {
return func(cmd *cobra.Command, args []string) error {

cliLogger := logger.NewCLILogger(os.Stdout)

// create config from flags
c, err := steps.NewConfigBuilder().
WithLogWriter(cliLogger).
@@ -91,8 +112,17 @@ func getBootstrapCmdRun(opts *config.Options) func(*cobra.Command, []string) err
WithVersion(flags.version).
WithDomainType(flags.domainType).
WithDomain(flags.domain).
WithPrivateKey(flags.privateKeyPath, flags.privateKeyPassword).
WithGitRepository(flags.repoURL,
flags.branch,
flags.repoPath,
).
WithGitAuthentication(flags.privateKeyPath,
flags.privateKeyPassword,
flags.gitUsername,
flags.gitPassword,
).
WithOIDCConfig(flags.discoveryURL, flags.clientID, flags.clientSecret, true).
WithSilentFlag(flags.silent).
Build()

if err != nil {
48 changes: 46 additions & 2 deletions cmd/gitops/app/bootstrap/cmd_acceptance_test.go
Original file line number Diff line number Diff line change
@@ -74,9 +74,31 @@ func TestBootstrapCmd(t *testing.T) {

privateKeyFile := os.Getenv("GIT_PRIVATEKEY_PATH")
g.Expect(privateKeyFile).NotTo(BeEmpty())

repoURLSSH := os.Getenv("GIT_REPO_URL_SSH")
g.Expect(repoURLSSH).NotTo(BeEmpty())
repoURLHTTPS := os.Getenv("GIT_REPO_URL_HTTPS")
g.Expect(repoURLHTTPS).NotTo(BeEmpty())
gitUsername := os.Getenv("GIT_USERNAME")
g.Expect(gitUsername).NotTo(BeEmpty())
gitPassword := os.Getenv("GIT_PASSWORD")
g.Expect(gitPassword).NotTo(BeEmpty())
gitBranch := os.Getenv("GIT_BRANCH")
g.Expect(gitBranch).NotTo(BeEmpty())
gitRepoPath := os.Getenv("GIT_REPO_PATH")
g.Expect(gitRepoPath).NotTo(BeEmpty())

privateKeyFlag := fmt.Sprintf("--private-key=%s", privateKeyFile)
kubeconfigFlag := fmt.Sprintf("--kubeconfig=%s", kubeconfigPath)

repoHTTPSURLFlag := fmt.Sprintf("--repo-url=%s", repoURLHTTPS)

gitUsernameFlag := fmt.Sprintf("--git-username=%s", gitUsername)
gitPasswordFlag := fmt.Sprintf("--git-password=%s", gitPassword)

gitBranchFlag := fmt.Sprintf("--branch=%s", gitBranch)
gitRepoPathFlag := fmt.Sprintf("--repo-path=%s", gitRepoPath)

oidcClientSecret := os.Getenv("OIDC_CLIENT_SECRET")
g.Expect(oidcClientSecret).NotTo(BeEmpty())
oidcClientSecretFlag := fmt.Sprintf("--client-secret=%s", oidcClientSecret)
@@ -91,7 +113,7 @@ func TestBootstrapCmd(t *testing.T) {
reset func(t *testing.T)
}{
{
name: "should bootstrap non-interactive with valid arguments",
name: "journey flux exists: should bootstrap with valid arguments",
flags: []string{kubeconfigFlag,
"--version=0.35.0",
privateKeyFlag, "--private-key-password=\"\"",
@@ -112,6 +134,28 @@ func TestBootstrapCmd(t *testing.T) {
},
expectedErrorStr: "",
},
{
name: "journey flux does not exist: should bootstrap with valid arguments",
flags: []string{kubeconfigFlag,
"--version=0.35.0",
"--password=admin123",
"--domain-type=localhost",
"--discovery-url=https://dex-01.wge.dev.weave.works/.well-known/openid-configuration",
"--client-id=weave-gitops-enterprise",
gitUsernameFlag, gitPasswordFlag, gitBranchFlag, gitRepoPathFlag,
repoHTTPSURLFlag,
oidcClientSecretFlag, "-s",
},
setup: func(t *testing.T) {
createEntitlements(t, testLog)
},
reset: func(t *testing.T) {
deleteEntitlements(t, testLog)
deleteClusterUser(t, testLog)
uninstallFlux(g, kubeconfigFlag)
},
expectedErrorStr: "",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@@ -144,7 +188,7 @@ func TestBootstrapCmd(t *testing.T) {
func bootstrapFluxSsh(g *WithT, kubeconfigFlag string) {
var runner runner.CLIRunner

repoUrl := os.Getenv("GIT_URL_SSH")
repoUrl := os.Getenv("GIT_REPO_URL_SSH")
g.Expect(repoUrl).NotTo(BeEmpty())
fmt.Println(repoUrl)

11 changes: 10 additions & 1 deletion cmd/gitops/app/bootstrap/cmd_auth.go
Original file line number Diff line number Diff line change
@@ -58,8 +58,17 @@ func getAuthCmdRun(opts *config.Options) func(*cobra.Command, []string) error {
c, err := steps.NewConfigBuilder().
WithLogWriter(cliLogger).
WithKubeconfig(opts.Kubeconfig).
WithPrivateKey(flags.privateKeyPath, flags.privateKeyPassword).
WithGitRepository(flags.repoURL,
flags.branch,
flags.repoPath,
).
WithGitAuthentication(flags.privateKeyPath,
flags.privateKeyPassword,
flags.gitUsername,
flags.gitPassword,
).
WithOIDCConfig(flags.discoveryURL, flags.clientID, flags.clientSecret, false).
WithSilentFlag(flags.silent).
Build()

if err != nil {
57 changes: 56 additions & 1 deletion docs/cli/bootstrap.md
Original file line number Diff line number Diff line change
@@ -110,6 +110,13 @@ type Config struct {

```

### Style sugestions for steps

**Inputs**

- We usually prefix input names with `in` prefix (short for input) to distinguish these constants from everything else.


## Error management

A bootstrapping error received by the platform engineer shoudl allow:
@@ -141,6 +148,20 @@ These messages will provide extra information that's not provided by errors like

Use custom errors when required for better handling like [this](https://github.com/weaveworks/weave-gitops-enterprise/blob/6b1c1db9dc0512a9a5c8dd03ddb2811a897849e6/pkg/bootstrap/steps/entitlement.go#L65)

3) Special case for cases where we could recover from the error and don't need to terminate

for example [here](https://github.com/weaveworks/weave-gitops-enterprise/blob/80667a419c286ee7d45178b639e36a2015533cb6/pkg/bootstrap/steps/flux.go#L39)

flux is not bootstrapped, but in the process we can bootstrap flux. in this case we could log the failure and continue the execution
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this is failure cause, if we dont have as a requirement that flux is bootstrapped, the fact that flux is not bootstrapped is just an expected scenario that we handle instead of an error.


```go
out, err := runner.Run("flux", "check")
if err != nil {
c.Logger.Failuref("flux installed error: %v. %s", string(out), fluxRecoverMsg)
return []StepOutput{}, nil
}
```

## Logging Actions

For sharing progress with the user, the following levels are used:
@@ -182,8 +203,15 @@ Entitlement stage
- `WGE_ENTITLEMENT_USERNAME`: entitlements username to use for creating the entitlement before running the test.
- `WGE_ENTITLEMENT_PASSWORD`: entitlements password to use for creating the entitlement before running the test.
- `WGE_ENTITLEMENT_ENTITLEMENT`: valid entitlements token to use for creating the entitlement before running the test.
- `OIDC_CLIENT_SECRET`: client secret for oidc flag
- `GIT_PRIVATEKEY_PATH`: path to the private key to do the git operations.
- `GIT_URL_SSH`: git ssh url for the repo wge configuration repo.
- `GIT_REPO_URL_SSH`: git ssh url for the repo wge configuration repo.
- `GIT_REPO_URL_HTTPS`: git https url for the repo wge configuration repo.
- `GIT_USERNAME`: git username for testing https auth
- `GIT_PASSWORD`: git password for testing https auth
- `GIT_BRANCH`: git branch for testing with flux bootstrap
- `GIT_REPO_PATH`: git repo path for default cluster for testing with flux bootstrap


Run it via `make cli-acceptance-tests`

@@ -204,4 +232,31 @@ See the following examples:

This will be addressed in the following [ticket](https://github.com/weaveworks/weave-gitops-enterprise/issues/3405)

## Enable/Disable one or more input from step inputs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eneko to test out


Field [`Enabled`](https://github.com/weaveworks/weave-gitops-enterprise/blob/80667a419c286ee7d45178b639e36a2015533cb6/pkg/bootstrap/steps/ask_bootstrap_flux.go#L14) is added to the step input to allow/disallow this input from being processd

This field should receive a function that takes the step input, config object and returns boolean value

example:

- step input

```go
var bootstrapFLuxQuestion = StepInput{
Name: inBootstrapFlux,
Type: confirmInput,
Msg: bootstrapFluxMsg,
Enabled: canAskForFluxBootstrap,
}
```

- function

```go
func canAskForFluxBootstrap(input []StepInput, c *Config) bool {
return !c.FluxInstallated
}
```

This input will be processed only if `Enabled` field is equal to `true`
4 changes: 3 additions & 1 deletion pkg/bootstrap/bootstrap.go
Original file line number Diff line number Diff line change
@@ -9,8 +9,10 @@ func Bootstrap(config steps.Config) error {
// TODO have a single workflow source of truth and documented in https://docs.gitops.weave.works/docs/0.33.0/enterprise/getting-started/install-enterprise/
var steps = []steps.BootstrapStep{
steps.VerifyFluxInstallation,
steps.NewAskBootstrapFluxStep(config),
steps.NewGitRepositoryConfig(config),
steps.NewBootstrapFlux(config),
Comment on lines 11 to +14
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • git configuration is something that is discovered from flux.
  • when we dont have flux, we just need to ask for it.

steps.CheckEntitlementSecret,
steps.NewAskPrivateKeyStep(config),
steps.NewSelectWgeVersionStep(config),
steps.NewAskAdminCredsSecretStep(config),
steps.NewSelectDomainType(config),
2 changes: 1 addition & 1 deletion pkg/bootstrap/bootstrap_auth.go
Original file line number Diff line number Diff line change
@@ -26,7 +26,7 @@ func bootstrapOIDC(config steps.Config) error {
var steps = []steps.BootstrapStep{
steps.VerifyFluxInstallation,
steps.CheckEntitlementSecret,
steps.NewAskPrivateKeyStep(config),
steps.NewBootstrapFlux(config),
steps.NewInstallOIDCStep(config),
steps.NewOIDCConfigStep(config),
}
28 changes: 11 additions & 17 deletions pkg/bootstrap/steps/admin_password.go
Original file line number Diff line number Diff line change
@@ -24,11 +24,11 @@ const (
)

var getPasswordInput = StepInput{
Name: Password,
Name: inPassword,
Type: passwordInput,
Msg: adminPasswordMsg,
DefaultValue: defaultAdminPassword,
Valuesfn: canAskForCreds,
Enabled: canAskForCreds,
Required: true,
}

@@ -41,11 +41,11 @@ var getPasswordInput = StepInput{
func NewAskAdminCredsSecretStep(config Config) BootstrapStep {
inputs := []StepInput{
{
Name: existingCreds,
Name: inExistingCreds,
Type: confirmInput,
Msg: existingCredsMsg,
DefaultValue: "",
Valuesfn: isExistingAdminSecret,
Enabled: isExistingAdminSecret,
StepInformation: fmt.Sprintf(adminSecretExistsMsgFormat, adminSecretName, WGEDefaultNamespace),
},
}
@@ -65,21 +65,21 @@ func createCredentials(input []StepInput, c *Config) ([]StepOutput, error) {
// search for existing admin credentials in secret cluster-user-auth
continueWithExistingCreds := confirmYes
for _, param := range input {
if param.Name == Password {
if param.Name == inPassword {
password, ok := param.Value.(string)
if ok {
c.Password = password
}
}
if param.Name == existingCreds {
if param.Name == inExistingCreds {
existing, ok := param.Value.(string)
if ok {
continueWithExistingCreds = existing
}
}
}

if existing, _ := isExistingAdminSecret(input, c); existing.(bool) {
if existing := isExistingAdminSecret(input, c); existing {
if continueWithExistingCreds != confirmYes {
return []StepOutput{}, fmt.Errorf(existingCredsExitMsg, adminSecretName, WGEDefaultNamespace)
} else {
@@ -120,17 +120,11 @@ func createCredentials(input []StepInput, c *Config) ([]StepOutput, error) {
// isExistingAdminSecret checks for admin secret on management cluster
// returns true if admin secret is already on the cluster
// returns false if no admin secret on the cluster
func isExistingAdminSecret(input []StepInput, c *Config) (interface{}, error) {
func isExistingAdminSecret(input []StepInput, c *Config) bool {
_, err := utils.GetSecret(c.KubernetesClient, adminSecretName, WGEDefaultNamespace)
if err != nil {
return false, nil
}
return true, nil
return err == nil
}

func canAskForCreds(input []StepInput, c *Config) (interface{}, error) {
if ask, _ := isExistingAdminSecret(input, c); ask.(bool) {
return false, nil
}
return true, nil
func canAskForCreds(input []StepInput, c *Config) bool {
return !isExistingAdminSecret(input, c)
}
Loading