caddy-revocation-validator icon indicating copy to clipboard operation
caddy-revocation-validator copied to clipboard

Support for Caddyfile

Open zachgalvin opened this issue 1 year ago • 9 comments

With version 2.8.0 of Caddy, the Caddyfile adapter will have the ability to load client_auth verifier modules with PR 6022, so I was hoping to add the below code to allow this module to work with the Caddyfile adapter as well.

Open issue on this here: #2

Not sure if there are other thoughts for how to do this, but the below works well!

zachgalvin avatar Jan 12 '24 19:01 zachgalvin

Oh that are great news i didnt know that this was added in caddy. I would have added this otherwise already but problem was that it required changes in caddy which could potentially cause backward incompatibility changes. I try to find some time in the next week to test and review this pr. Thanks

Gr33nbl00d avatar Jan 13 '24 21:01 Gr33nbl00d

No problem! That sounds great!

I did leave a debug line in the code, so it probably makes sense to remove that line and remove the log module import as well since that's the only time we use it: log.Printf("key: %v", key)

zachgalvin avatar Jan 16 '24 19:01 zachgalvin

No problem! That sounds great!

I did leave a debug line in the code, so it probably makes sense to remove that line and remove the log module import as well since that's the only time we use it: log.Printf("key: %v", key)

Ok no problem. Unfortunately i did not find the time this week to review the code. But i hope i can do it on monday. Currently a lot of project pressure ;)

Gr33nbl00d avatar Jan 19 '24 14:01 Gr33nbl00d

I did some quick review. But i need to do some further tests and refactorings (too many loops in one method) + additions + documentation. So i need some more days

Gr33nbl00d avatar Jan 26 '24 13:01 Gr33nbl00d

All good. That sounds good. Thanks!

zachgalvin avatar Jan 27 '24 13:01 zachgalvin

Funny it looks like they broke your pull request a few weeks after your merge somehow in caddy with a refactoring. https://github.com/caddyserver/caddy/pull/5784

I already commented and could repair it locally for implementation.

Gr33nbl00d avatar Feb 09 '24 11:02 Gr33nbl00d

Small nit: I'd call the new file caddyfile.go, not caddyUnmarshal.go.

francislavoie avatar Feb 09 '24 12:02 francislavoie

Small nit: I'd call the new file caddyfile.go, not caddyUnmarshal.go.

Thanks for the suggestion i will do so. Very welcome :)

Gr33nbl00d avatar Feb 09 '24 13:02 Gr33nbl00d

Hey @Gr33nbl00d, do you think you will have time to look at this again now that they re-added the code from my pull request? https://github.com/caddyserver/caddy/pull/6127

zachgalvin avatar Mar 06 '24 05:03 zachgalvin