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

caddytls: Change clustering to be a plugin to the caddytls package #2459

Merged
merged 4 commits into from Feb 8, 2019

Conversation

mholt
Copy link
Member

@mholt mholt commented Feb 8, 2019

1. What does this change do, exactly?

Should resolve the failure in
coredns/coredns#2541.

This change is breaking to clustering plugin developers (not Caddy
users), but logical, since only the caddytls package uses CertMagic
directly (the httpserver package also uses it, but only because it also
uses the caddytls plugin); and it is early enough that no clustering
plugins really exist yet.

This will also require a change of devportal
so that it looks for a different registration function, which has moved
to the caddytls package.

2. Please link to the relevant issues.

coredns/coredns#2541

3. Which documentation changes (if any) need to be made because of this PR?

No user-facing changes, but update the wiki about how to write (and register) a clustering plugin. Devportal needs updating too, since the registration function moved.

4. Checklist

  • I have written tests and verified that they fail without my change
  • I have squashed any insignificant commits
  • This change has comments for package types, values, functions, and non-obvious lines of code
  • I am willing to help maintain this change if there are issues with it later

Should resolve the failure in
coredns/coredns#2541.

This change is breaking to clustering plugin developers (not Caddy
users), but logical, since only the caddytls package uses CertMagic
directly (the httpserver package also uses it, but only because it also
uses the caddytls plugin); and it is early enough that no clustering
plugins really exist yet.

This will also require a change of devportal
so that it looks for a different registration function, which has moved
to the caddytls package.
@mholt mholt requested a review from miekg February 8, 2019 19:27
@miekg
Copy link
Collaborator

miekg commented Feb 8, 2019

Compiled a coredns binary against this change and that starts (normally). So LGTM

Copy link
Collaborator

@miekg miekg left a comment

Choose a reason for hiding this comment

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

Mostly a move anyway.

@mholt
Copy link
Member Author

mholt commented Feb 8, 2019

Awesome, thanks. I'm just gonna make sure these tests pass now and then merge it.

somehow VS Code didn't fmt on save... weird.
@mholt mholt merged commit 1867ded into master Feb 8, 2019
@mholt mholt deleted the tlscluster branch February 8, 2019 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants