Onboarding CLI | Flux bootstrapping and git authentication credentials #3578

waleedhammam · 2023-11-01T13:13:14Z

Closes #3295

What changed?

Add bootstrap flux
Support for using git authentication with ssh and https

Why was this change made?

As part of phase2 CLI to bootstrap flux if it's not available

How was this change implemented?

How did you validate the change?

Explain how a reviewer can verify the change themselves

for interactive gitops bootstrap and follow the wizard
for non-interactive 2 scenarios:

create namespace flux-system to apply entitlement in
apply entitlement
for each scenario, the following command

cluster with no flux, need to bootstrap ssh

gitops bootstrap --version=0.35.0 --private-key=/home/test/.ssh/id_rsa --private-key-password="" --password=admin123 --domain-type=localhost --discovery-url=https://dex-01.wge.dev.weave.works/.well-known/openid-configuration --client-id=weave-gitops-enterprise --repo-url=ssh://[email protected]/waleedhammam/wge-dev --branch=main --repo-path=clusters/my-cluster --client-secret=<secret> -s

cluster with no flux, need to bootstrap https

gitops bootstrap --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 --git-username=waleedhammam --git-token=<token>--branch=main --repo-path=clusters/my-cluster --repo-url=https://github.com/waleedhammam/wge-dev --client-secret=<secret> -s

Integration tests -- what is covered, what cannot be covered;
or, explain why there are no new tests
Unit tests -- what is covered, what cannot be covered; are
there tests that fail without the change?

Release notes

Documentation Changes

Other follow ups

enekofb

Reviewed the code with the common suggestion of adding tests for newly added functions or changed contracts.

I didn't test how it works as i could not find in the PR description indications on how to test it. Please add this information.

Last suggestion is to revisit adding integration and acceptance tests: it is an important journey with different integration points. It is likely that we would benefit from them.

cmd/gitops/app/bootstrap/cmd.go

enekofb · 2023-11-06T09:23:09Z

pkg/bootstrap/bootstrap.go

 		steps.VerifyFluxInstallation,
+		steps.NewAskBootstrapFluxStep(config),
+		steps.NewSelectGitAuthType(config),
+		steps.NewBootstrapFluxUsingSSH(config),
+		steps.NewBootstrapFluxUsingHTTPS(config),


Please share the design considerations used for selecting using these steps over other design like having a single step called fluxinstall. please shar

updated to be

steps.NewAskBootstrapFluxStep(config), steps.NewFluxGitRepositoryConfig(config), steps.NewBootstrapFlux(config),

the currently implemented logic, process all the inputs first, then execute the step function
meanwhile some inputs rely on other inputs from the previous step and could change accordingly

for example in case of passing a repo-url with ssh scheme, we need to only ask about private key and private key password

and in case of passing a repo-url with https scheme, then we need to ask about username and token

and since we handle input first, then process, then out. if we don't pass the value to this step it'll ask all of them anyway

pkg/bootstrap/steps/admin_password.go

pkg/bootstrap/steps/bootstrap_flux_https_test.go

pkg/bootstrap/steps/install_wge.go

pkg/bootstrap/steps/oidc.go

pkg/bootstrap/steps/select_git_auth_type.go

pkg/bootstrap/steps/step.go

pkg/bootstrap/utils/git.go

enekofb

it looks good!

I havent fully tested but here are until the point i got now! will continue tomorrow but currently this PR raised #3607

cmd/gitops/app/bootstrap/cmd.go

enekofb · 2023-11-08T09:23:12Z

cmd/gitops/app/bootstrap/cmd.go

 	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")
+	cmd.PersistentFlags().StringVarP(&flags.gitUsername, "git-username", "", "", "git username used in https authentication type")
+	cmd.PersistentFlags().StringVarP(&flags.gitToken, "git-token", "", "", "git token used in https authentication type")


should be token or password? i could see https://fluxcd.io/flux/cmd/flux_bootstrap_git/ is password

yeah in flux is password but it's very confusing as adding the git https basic auth with password doesn't work anymore and only accept token

yes but i think that flux gets it from http where basic auth is username and password https://en.wikipedia.org/wiki/Basic_access_authentication

but auth with username and password is no longer accepted https://docs.github.com/en/rest/overview/authenticating-to-the-rest-api?apiVersion=2022-11-28#authenticating-with-username-and-password

ahh we add --token-auth to flux configuration so that it can handle it as a token

i guess flux is correct calling it password

https://fluxcd.io/flux/components/source/gitrepositories/#basic-access-authentication

as it is how is defined in the standard https://datatracker.ietf.org/doc/html/rfc7617#section-2

we could review this in the catchup too

cmd/gitops/app/bootstrap/cmd.go

pkg/bootstrap/steps/config.go

enekofb · 2023-11-08T17:58:58Z

pkg/bootstrap/steps/config.go

 		PrivateKeyPath:          cb.privateKeyPath,
 		PrivateKeyPassword:      cb.privateKeyPassword,
+		GitUsername:             cb.gitUsername,


we should work towards a more modular configuration approach ... like having just a single GitConfig struct for anything git configuration ...

pkg/bootstrap/steps/bootstrap_flux_repo_config.go

pkg/bootstrap/steps/bootstrap_flux.go

enekofb

Some more feedback after manually tested with

go run cmd/gitops/main.go bootstrap --version=0.35.0 --password=admin123 --domain-type=localhost --git-username=enekofb --git-token=<valid-token>--branch=main --repo-path=clusters/management --repo-url=https://github.com/enekofb/cli-dev.git

For the scenario I dont have flux bootstrapping:

► checking crds
✗ no crds found with the label selector 'app.kubernetes.io/part-of=flux'
✗ check failed
. please bootstrap Flux in 'flux-system' namespace: more info https://fluxcd.io/flux/installation
◎ bootstrap flux
? do you want to bootstrap flux using the generic way? [y/N] █

this message might be redundant? please bootstrap Flux in 'flux-system' namespace: more info https://fluxcd.io/flux/installation when you are offered to install it after. we could remove it.

When in the bootsrapping: may be there is an inconsistent usage of ◎ given that indicates begining and end of step or so

◎ bootstrap flux
do you want to bootstrap flux using the generic way: y
◎ flux repository configuration
► detected repo scheme: https
◎ git credentials
◎ bootstrapping flux ...

it feels better actions ►

install finishes but already containers have not yet been created -> it seems that we should wait for the release to happen

…an https. leaving https cause the bootstrapped scenario already have ssh

Co-authored-by: Eneko Fernández <[email protected]>

enekofb · 2023-11-09T09:09:57Z

cmd/gitops/app/bootstrap/cmd.go

 	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")


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

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

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)

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

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.

* unify repo url variables * update doc

pkg/bootstrap/steps/bootstrap_flux.go

Co-authored-by: Eneko Fernández <[email protected]>

enekofb · 2023-11-09T10:25:18Z

pkg/bootstrap/bootstrap.go

 		steps.VerifyFluxInstallation,
+		steps.NewAskBootstrapFluxStep(config),
+		steps.NewGitRepositoryConfig(config),
+		steps.NewBootstrapFlux(config),


git configuration is something that is discovered from flux.

when we dont have flux, we just need to ask for it.

enekofb

🚀

cmd/gitops/app/bootstrap/cmd.go

add flux bootstrapping and git auth using https and ssh

b8425e6

waleedhammam added the enhancement New feature or request label Nov 1, 2023

waleedhammam requested a review from enekofb November 1, 2023 13:13

waleedhammam linked an issue Nov 1, 2023 that may be closed by this pull request

Onboarding CLI | Flux bootstrapping git authentication credentials #3295

Closed

Merge branch 'main' into 3295-flux-bootstrap

7e4fd03

enekofb reviewed Nov 6, 2023

View reviewed changes

waleedhammam added 9 commits November 7, 2023 11:17

Merge branch 'main' into 3295-flux-bootstrap

118c73e

Merge branch 'main' into 3295-flux-bootstrap

006f45a

fix merge conflicts

da28aa8

refactor input names and update flux installation

80667a4

Merge branch 'main' into 3295-flux-bootstrap

65dfcf2

fix lint

0b27797

update internal docs

5e4d52b

add acceptance test

7faee99

cleanup

e3cc8a9

waleedhammam requested a review from enekofb November 7, 2023 18:07

waleedhammam added 2 commits November 7, 2023 20:07

Merge branch 'main' into 3295-flux-bootstrap

f58abae

add unit test for helper methods

b6f1064

enekofb reviewed Nov 8, 2023

View reviewed changes

enekofb self-requested a review November 8, 2023 18:25

enekofb reviewed Nov 8, 2023

View reviewed changes

enekofb and others added 2 commits November 9, 2023 08:33

removed ssh bootstrap scenario as represents the same user journey th…

51355a3

…an https. leaving https cause the bootstrapped scenario already have ssh

Update cmd/gitops/app/bootstrap/cmd.go

49d284a

Co-authored-by: Eneko Fernández <[email protected]>

enekofb reviewed Nov 9, 2023

View reviewed changes

waleedhammam and others added 2 commits November 9, 2023 11:38

rename flux git repository to git repository

03879c0

adding just small docs update (#3610)

1395ede

* unify repo url variables * update doc

enekofb reviewed Nov 9, 2023

View reviewed changes

pkg/bootstrap/steps/bootstrap_flux.go Outdated Show resolved Hide resolved

Update pkg/bootstrap/steps/bootstrap_flux.go

c9b4477

Co-authored-by: Eneko Fernández <[email protected]>

enekofb reviewed Nov 9, 2023

View reviewed changes

rename gitToken to gitPassword

c3dfb5c

waleedhammam added 2 commits November 9, 2023 12:31

update flux msg

0059abb

update flux msg

0ed19ee

enekofb mentioned this pull request Nov 9, 2023

WGE onboarding: cli bootstrapping #3026

Open

44 tasks

enekofb self-requested a review November 9, 2023 10:42

enekofb approved these changes Nov 9, 2023

View reviewed changes

waleedhammam merged commit 221d874 into main Nov 9, 2023
10 checks passed

waleedhammam deleted the 3295-flux-bootstrap branch November 9, 2023 10:55

lasomethingsomething reviewed Nov 13, 2023

View reviewed changes

cmd/gitops/app/bootstrap/cmd.go Show resolved Hide resolved

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Onboarding CLI | Flux bootstrapping and git authentication credentials #3578

Onboarding CLI | Flux bootstrapping and git authentication credentials #3578

waleedhammam commented Nov 1, 2023 •

edited

Loading

enekofb left a comment

enekofb Nov 6, 2023

waleedhammam Nov 7, 2023

enekofb left a comment

enekofb Nov 8, 2023

waleedhammam Nov 9, 2023

enekofb Nov 9, 2023

waleedhammam Nov 9, 2023

waleedhammam Nov 9, 2023

enekofb Nov 9, 2023

enekofb Nov 8, 2023

enekofb left a comment

enekofb Nov 9, 2023

waleedhammam Nov 9, 2023

enekofb Nov 9, 2023

enekofb Nov 9, 2023

enekofb Nov 9, 2023

enekofb left a comment

		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")

Onboarding CLI | Flux bootstrapping and git authentication credentials #3578

Onboarding CLI | Flux bootstrapping and git authentication credentials #3578

Conversation

waleedhammam commented Nov 1, 2023 • edited Loading

enekofb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enekofb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enekofb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enekofb left a comment

Choose a reason for hiding this comment

waleedhammam commented Nov 1, 2023 •

edited

Loading