-
Notifications
You must be signed in to change notification settings - Fork 729
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
Fix handling of HTTP CA #1742
Fix handling of HTTP CA #1742
Conversation
dcd5f71
to
021cd92
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! Maybe add tests for the private key support?
return nil, fmt.Errorf("expected an RSA private key but got %t", key) | ||
} | ||
|
||
return rsaKey, nil |
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.
Am I mistaken or are we missing unit tests for this? Maybe an opportunity to add one (or two)?
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 reason I didn't add any unit tests is because these functions are just thin wrappers over standard library functions. Happy to add tests if you think differently.
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.
OK
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
132f4d4
to
0c52466
Compare
More details can be found in #1614.
Briefly, this PR does the following:
ca.crt
key in the secret as a signal to override the appropriate*.certificate_authorities
setting of the associated stack application.Fixes #1614