-
Notifications
You must be signed in to change notification settings - Fork 620
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
Conversation
116f6de
to
30ed396
Compare
@@ -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 { |
There was a problem hiding this comment.
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.
7280d17
to
e658b37
Compare
b757e53
to
8ce6863
Compare
Signed-off-by: Philip Laine <philip.laine@gmail.com>
8ce6863
to
a4734d7
Compare
// Bootstrap config | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks @phillebaba 🏅
There was a problem hiding this 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! 🙇
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.