Skip to content
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

Allow domain overrides for challenge delegation #2

Merged
merged 5 commits into from
Dec 29, 2020
Merged

Allow domain overrides for challenge delegation #2

merged 5 commits into from
Dec 29, 2020

Conversation

jpeddicord
Copy link
Contributor

For more information, see libdns/duckdns#1

This commit adds Caddyfile parsing for an override_domain
directive.


Should be pretty straightforward -- this adds override_domain in the same manner as api_token was being parsed. I did not add it as an argument to the main directive as I don't think it'll be as commonly used.

I also changed the parsing for api_token -- unless I missed something, I think this wasn't working. That NextArg call has to happen to get the parser to move on from api_token to the provided value.

Currently writing up a description of this in the README; I'll un-mark draft status once I have that up.

For more information, see libdns/duckdns#1

This commit adds Caddyfile parsing for an `override_domain`
directive.
@francislavoie
Copy link
Collaborator

While you're at it, could you set the markdown code block language in the README to json? I forgot to do it, might as well do it now while you're updating the README 😄

@francislavoie francislavoie added the enhancement New feature or request label Dec 29, 2020
@jpeddicord jpeddicord marked this pull request as ready for review December 29, 2020 19:18
@jpeddicord
Copy link
Contributor Author

Done; changed that and fixed up a lingering cloudflare in an example too. :D

@francislavoie
Copy link
Collaborator

Could you add a quick section above ## Config examples with the actual Caddyfile syntax definition? i.e. the thing in the comment in the code.

Also please add the new option to the example JSON config as well.

@jpeddicord
Copy link
Contributor Author

Yep, something like that?

Copy link
Collaborator

@francislavoie francislavoie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect. Thank you!

README.md Outdated Show resolved Hide resolved
Co-authored-by: Francis Lavoie <[email protected]>
README.md Outdated Show resolved Hide resolved
Co-authored-by: Francis Lavoie <[email protected]>
Copy link
Collaborator

@francislavoie francislavoie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@jpeddicord
Copy link
Contributor Author

Done. I can rebase this all down to 1 commit if you prefer too, just let me know.

@francislavoie francislavoie merged commit c5d221e into caddy-dns:master Dec 29, 2020
@francislavoie
Copy link
Collaborator

Done. I can rebase this all down to 1 commit if you prefer too, just let me know.

No worry, I tend to squash merge. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants