-
Notifications
You must be signed in to change notification settings - Fork 93
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
support adding a CA cert to http collector #1624
Conversation
@adamancini |
Is |
yeah we use it here https://github.com/replicatedhq/troubleshoot/pull/1624/files#diff-23221d86b2950ee7b3125067dcade43eaecff05f4ea2654ff278d97bafdad2a7R138 |
293f4fa
to
b1395e0
Compare
added PR to embedded-cluster to add a spec to checklist - but this should be ready for review again @banjoh |
@banjoh I'll follow up with another PR to clean up the code at https://github.com/replicatedhq/troubleshoot/pull/1624/files#diff-23221d86b2950ee7b3125067dcade43eaecff05f4ea2654ff278d97bafdad2a7R113 |
disable: | ||
- errcheck |
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.
We intending to disable this?
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.
I added it intentionally because tests are failing on "you didn't check for a return error value" in the tests themselves
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.
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.
looks like because of this most recent change 402d111
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
Description, Motivation and Context
http
collector now supports acacert
field and aproxy
field, in order to support testing for man-in-the-middle proxies. This will be most useful in a HostPreflight scenario, checking whether the proxy CA cert provided byembedded-cluster install --private-ca
can be used to successfully traverse the forward proxy. The expectation is that the proxy address and CA cert will be templated into a preflight spec at runtime.These params are new and optional:
cacert
: a path to a file, directory, or a literal string containing the CA certificate in PEM formatproxy
: a URL to a proxy, including port and scheme:https://10.128.0.4:3130
E.g.:
Relates to #1302
Depends on replicatedhq/embedded-cluster#1228
Checklist
Does this PR introduce a breaking change?