reverseproxy: Implement modular CA provider for TLS transport#6065
Conversation
armadi1809
force-pushed
the
new-ca-providers-for-reverse-proxy
branch
from
d605170 to
ab39dd4
Compare
|
Looks like the failed test is just a server timeout. |
armadi1809
force-pushed
the
new-ca-providers-for-reverse-proxy
branch
from
ab39dd4 to
c3a6efe
Compare
mohammed90
left a comment
mohammed90
left a comment
There was a problem hiding this comment.
GitHub does not let me comment on the unchanged lines, so excuse the non-inlined comments.
Mark the old way as deprecated and subject to removal in the future, and emit logs saying the same when used. You can get a logger from ctx.Logger().
armadi1809
force-pushed
the
new-ca-providers-for-reverse-proxy
branch
from
c3a6efe to
00b1449
Compare
|
I just added the "deprecated" comments to |
mohammed90
left a comment
mohammed90
left a comment
There was a problem hiding this comment.
Thanks for doing the rounds, Aziz! Sorry for not giving the review points in full at once. I'm squeezing the review time in tight time slots 😅
We're missing tests and Caddyfile support
|
No worries at all Mohammed! Thank you for your feedback. I implemented the caddy file support and added both caddy file integration tests as well as tests for the |
mholt
left a comment
mholt
left a comment
There was a problem hiding this comment.
Sorry for falling behind -- this is looking better now! I'd be willing to give this a try. Thank you!!
|
@mohammed90 Anything you can think of we need to change for this? |
mohammed90
left a comment
mohammed90
left a comment
There was a problem hiding this comment.
A few minor nitpicks and I believe I got everything
mohammed90
changed the title
armadi1809
force-pushed
the
new-ca-providers-for-reverse-proxy
branch
2 times, most recently
from
f8ba559 to
5e7a4e7
Compare
armadi1809
force-pushed
the
new-ca-providers-for-reverse-proxy
branch
from
5e7a4e7 to
391e352
Compare
|
@mohammed90 I think all your feedback items have been resolved Thanks for reviewing! |
mohammed90
left a comment
mohammed90
left a comment
There was a problem hiding this comment.
LGTM! Waiting on @mholt review then we're good to go.
francislavoie
changed the title
4 participants
Closes #6063