Skip to content

Conversation

jjiang-stripe
Copy link
Contributor

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 populating icp.pool since we're not calling Provision()

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 😅

Copy link
Member

@mholt mholt left a 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.)

@mholt mholt merged commit 49f9af9 into caddyserver:master Mar 10, 2025
20 checks passed
@mholt mholt added the bug 🐞 Something isn't working label Mar 10, 2025
@mholt mholt added this to the v2.9.2 milestone Mar 10, 2025
@jjiang-stripe
Copy link
Contributor Author

I'm working on getting us pointed to 2.9.1 master, so hopefully we can 🔪 the special branch once I'm done :')

@jjiang-stripe jjiang-stripe deleted the jjiang/patch-conn-policy branch March 10, 2025 18:58
@mholt
Copy link
Member

mholt commented Mar 11, 2025

Okay, awesome! What I might do instead, to ensure you get the exact tree of commits you want, is to change the stripe branch to 2.9.1 and then cherry-pick this commit. Would that work okay for your team?

@jjiang-stripe
Copy link
Contributor Author

yeah that'd work! I can point us back to master after we fix our config

mholt pushed a commit that referenced this pull request Mar 11, 2025
* add failing test

* fix ca pool provisioning

* remove unused param
@mholt
Copy link
Member

mholt commented Mar 11, 2025

@jjiang-stripe Perfect -- I just republished the stripe branch which is v2.9.1 with this commit cherry-picked.

@jjiang-stripe
Copy link
Contributor Author

amazing thank you!

mohammed90 pushed a commit to cedricziel/caddy that referenced this pull request Aug 29, 2025
* add failing test

* fix ca pool provisioning

* remove unused param
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐞 Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants