-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Fix TrustedCACerts backwards compatibility #6889
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 TrustedCACerts backwards compatibility #6889
Conversation
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.
Oops, nice catch!! Thanks! I'll update your branch as well. (Just need a few minutes first.)
I'm working on getting us pointed to 2.9.1 master, so hopefully we can 🔪 the special branch once I'm done :') |
Okay, awesome! What I might do instead, to ensure you get the exact tree of commits you want, is to change the |
yeah that'd work! I can point us back to master after we fix our config |
* add failing test * fix ca pool provisioning * remove unused param
@jjiang-stripe Perfect -- I just republished the |
amazing thank you! |
* add failing test * fix ca pool provisioning * remove unused param
I'm very late to the party, but I was in the process of bumping our Caddy dependency and noticed #5784 has a slight bug with the backwards compatibility of the now-deprecated
TrustedCACerts
option.The Caddyfile bit works fine because it converts it to a
CARaw
, but if you specify the option directly via JSON, we don't end up populatingicp.pool
since we're not callingProvision()
I wrote up a quick test to validate the behaviour with the additional call to
Provision()
and also tested it on our instance of Caddy. It'd be nice to cherrypick this onto the 2.8.0+ releases (I'm specifically looking at 2.9.1) to make sure this option still works while it's deprecated! I plan to migrate us to the new config options, but our deploy process makes that a bit difficult to do simultaneously with the Caddy upgrade 😅