-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
http-01: refactor provider for presenting token #76
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
Conversation
|
Hey there :) The change in itself looks sound. What I'm thinking about at the moment is though if it would make sense to have a general interface to all challenges along this implementation. |
|
@xenolf can you take another look. This incorporates feedback from #36 and exports a generic interface which (i think) will work for all challenges. type ChallengeProvider interface {
Present(domain, token, keyAuth string) error
CleanUp(domain string)
}Then there is a un-exported https and tls provider (ie: the http/tls servers that are the default providers for this package), and an exported function to set a custom providers, and exported helpers build the path for the I'm of the opinion that this package should not export I've tried to keep things named consistently, but if you have better naming suggestions on anything, just comment. I'm also happy to apply this refactoring to the dns provider when that lands in master |
735027e to
6dae9f2
Compare
|
@xenolf I see you've started landing support for |
acme/client.go
Outdated
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.
I think when going down this route we should export constants for these identifiers. Like acme.HTTP01.
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.
good idea; update forthcoming
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.
resolved
ed4fc6d to
141ff45
Compare
acme/challenges.go
Outdated
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.
I think those should be consts.
|
@xenolf let me know if you have any other feedback here, or if you'd like me to squash what's here for merge. |
|
Nah I think I'm happy now ;) |
* new ChallengeProvider with Present and CleanUp methods * new Challenge type describing `http-01`, `tls-sni-01`, `dns-01` * new client.SetChallengeProvider to support custom implementations
b9eff62 to
617dd4d
Compare
|
squashed |
http-01: refactor provider for presenting token
|
Changes Unknown when pulling 617dd4d on jehiah:custom_http_challenge_76 into ** on xenolf:master**. |
This implements the last suggestion from #32 by @janeczku to separate solving the
http-01challenge, from making thekeyAuthavailable at the appropriate path.This change creates a new interface
And a
httpChallengeServer(extracted fromhttpChallenge) which implements that interface and is used byClient.I am looking to use
legoas a library to generate certs and will need to have my own logic for making the token available, so the suggestion in #32 was quite useful.@xenolf If you have any thoughts on this change (or suggestions on impelemtnation) i'm all ears.