-
Notifications
You must be signed in to change notification settings - Fork 105
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
adding option for skipping SSL verification when using Git #1419
Conversation
0b43f4a
to
a438f0d
Compare
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.
The changes in the PR looks good to me. However, I think the idea was to use the existing dangerousSkipTLSVerify
flag available in kapp config, so we need to extend the functionality here
e1470ed
to
3bd38c2
Compare
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.
Is it possible to add a test for this change?
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.
Adding an e2e test with a local gitlab server might be a bit much, but it would be great if we can specify what sort of manual testing has been done. (We follow these steps and create an App which uses local gitlab server)
7f3c14b
to
cb8f2dd
Compare
97bc262
to
be04933
Compare
57c820a
to
5c7ebfe
Compare
Signed-off-by: sethiyash <[email protected]>
Signed-off-by: sethiyash <[email protected]>
Signed-off-by: sethiyash <[email protected]>
Signed-off-by: sethiyash <[email protected]>
Signed-off-by: sethiyash <[email protected]>
5c7ebfe
to
0708c18
Compare
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! (Apart from the minor comment)
Signed-off-by: Yash Sethiya <[email protected]>
0708c18
to
2592666
Compare
What this PR does / why we need it:
#1286
Which issue(s) this PR fixes:
#1286
Fixes #
Does this PR introduce a user-facing change?
Additional Notes for your reviewer:
Review Checklist:
a link to that PR
change
Additional documentation e.g., Proposal, usage docs, etc.: