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

Remove file reading from bootstrap package #3249

Merged
merged 1 commit into from Nov 7, 2022
Merged

Conversation

phillebaba
Copy link
Member

This change moves all file reading for certificates and PGP from code located in pkg to the CLI command. The purpose for this is to make sharing the code easier where the content is not expected to be in files.

@@ -195,7 +196,7 @@ func (b *PlainGitBootstrapper) ReconcileSourceSecret(ctx context.Context, option
}

// Return early if exists and no custom config is passed
if ok && len(options.CAFilePath+options.PrivateKeyPath+options.Username+options.Password) == 0 {
Copy link
Member Author

Choose a reason for hiding this comment

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

Verify that this logic is the same as the previous.

@phillebaba phillebaba force-pushed the bootstrap/files branch 11 times, most recently from 7280d17 to e658b37 Compare November 1, 2022 11:02
@phillebaba phillebaba marked this pull request as ready for review November 1, 2022 12:33
@pjbgf pjbgf added the area/git Git related issues and pull requests label Nov 1, 2022
@pjbgf pjbgf added this to the Bootstrap GA milestone Nov 1, 2022
cmd/flux/bootstrap_gitlab.go Outdated Show resolved Hide resolved
@phillebaba phillebaba force-pushed the bootstrap/files branch 2 times, most recently from b757e53 to 8ce6863 Compare November 4, 2022 11:13
@phillebaba phillebaba requested a review from pjbgf November 4, 2022 11:39
Signed-off-by: Philip Laine <philip.laine@gmail.com>
// Bootstrap config

Copy link
Member

Choose a reason for hiding this comment

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

nit:

Suggested change

@pjbgf pjbgf mentioned this pull request Nov 4, 2022
22 tasks
Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @phillebaba 🏅

Copy link
Member

@pjbgf pjbgf left a comment

Choose a reason for hiding this comment

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

Apart from the nit, LGTM

Thanks @phillebaba! 🙇

@phillebaba phillebaba merged commit c3a44e8 into main Nov 7, 2022
@phillebaba phillebaba deleted the bootstrap/files branch November 7, 2022 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/git Git related issues and pull requests
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants