-
Notifications
You must be signed in to change notification settings - Fork 1
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
NH-23785 Bundle public AO certificate for liboboe Reporter #70
NH-23785 Bundle public AO certificate for liboboe Reporter #70
Conversation
Tests failing on GH, boo! Fix coming. |
Fixed the GH test runs in 7c138aa. Ready for review! |
I just realized that AO staging might need a different cert than AO prod -- @cheempz is that correct? Also where might I get the staging cert? |
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.
LOL there must be something very confusing with our trustedpath/certificate handling logic, every implementation needed a tweak ;)
We actually want to keep supporting SW_APM_TRUSTEDPATH
handling even for SWO. So the logic should be:
If SW_APM_TRUSTEDPATH
is set, we should attempt to load the content of the given file and pass it to liboboe as the certificate. This should be regardless what the collector endpoint is.
If SW_APM_TRUSTEDPATH
is not set AND the collector endpoint is AO, we should default to the bundled AO cert.
Ah, the AO staging cert is thankfully signed by the same private CA that signs for AO prod, so using the same bundled AO CA cert is all good there -- and in fact I've been using your awesome fastapi books app to send test data to AO staging. |
Thank you @cheempz ! Addressed in 779821e and 3a2e979, with updated outcomes in confluence: https://swicloud.atlassian.net/wiki/spaces/NIT/pages/3309503060/2022-11-04+Reporter+init+with+certificate+string Please let me know if anything else! |
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 for the revisit and tests @tammy-baylis-swi!
This bumps Python APM library's liboboe version to 11.0.0 and inits the Reporter with a string for
certificates
. It no longer inits withtrusted_path
,is_grpc_clean_hack_enabled
, norw3c_trace_format
.As suggested at a standup a little while back, the public cert has been hard-coded as a constant in
solarwinds_apm
and is now part of the installed package. I'll be adjusting the integration tests to use this too if approved.I've tested this manually with the different behaviours recorded here: https://swicloud.atlassian.net/wiki/spaces/NIT/pages/3309503060/2022-11-04+Reporter+init+with+certificate+string. In two sentences:
pointing at NH collector is the same as before, whilePointing to AO prod/staging uses either the packaged cert by default OR uses what's atSW_APM_TRUSTEDPATH
if there's a file there. Pointing to any other collector (e.g. NH/SWO) does the same except the default is empty string.Please let me know as always if any suggestions! 🙂