feat(wildcard-certs): per-zone DNS-01 provider selection - #94
Merged
Conversation
…table Every DNS provider used to be spread across five edit sites: the lego import, a ProviderInfo entry, a switch case, a bespoke constructor with a hand-written credential check, and two ordered lists that had to agree by convention. One registry table now carries all of it, and a test pins the detection order to the registry so a new provider cannot be forgotten. Behavior is unchanged for every existing credential combination: all three Cloudflare paths, route53's no-boot-check contract, and credential errors that still name the missing variables. Closes #72
--acme-dns-provider selected one provider process-wide, so a fleet whose wildcard zones live at different DNS hosts could never issue the second zone's wildcard. The flag now maps zones to providers, keeping the current forms working: zone=provider entries pin a zone to its DNS host, one bare entry stays the default for unmatched zones. Selection is by longest matching zone suffix at issuance time, and one ACME order never spans providers: the handshake batch narrows to the requested domain's partition, the dynamic issuer assembles provider-pure batches, and renewal of a pre-mapping mixed-zone certificate splits along provider boundaries. Every mapped provider validates its credentials at startup, on the shared ACME account. ## Test Coverage - ParseProviderEntries: bare/mapped grammar, auto refused in mappings, duplicate zones and defaults refused - ProviderFor: longest-suffix matching on label boundaries, wildcards - obtainCertificate: routes by zone, refuses spanning orders, keeps the wildcard-needs-DNS refusal and HTTP-01 fallback - provisionCertificate: narrows a spanning handshake batch, defers the rest - domainIssuer.nextBatch: never spans provider partitions - certRenewer: splits mixed certs, keeps the old cert when a partition fails - run.go: flag/env parsing lands in config, invalid names fail preRun ## Verification - [x] gofmt -l internal/ cmd/ clean - [x] make test + go test -race ./internal/server/ pass - [x] make lint (golangci-lint) clean Closes #89
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
internal/server/acme/providers/factory.go's five per-provider edit sites into one registry table (registry.go) plus a singledetectionOrderslice, with a test that fails by name if they drift apart. Pure refactor — all three Cloudflare credential paths, route53's no-boot-check contract, and error messages naming the missing variables behave identically.--acme-dns-providerbecomes repeatable and acceptszone=providermappings alongside the existing bare form (internal/server/acme/mapping.go). At issuance time the provider is selected by longest-suffix zone match; one lego client per distinct provider, all on the same ACME account (internal/server/san_cert_zones.go).obtainCertificaterefuses a spanning order as an invariant violation, and every batch assembler keeps orders provider-pure — the handshake path narrows to the requested domain's partition (san_cert_manager.go), the dynamic issuer groups batches by partition (domain_issuer.go), and renewal splits a pre-mapping mixed-zone certificate into per-partition orders (domain_renewal.go).Closes #72
Closes #89
Test plan
make test(full suite, 1900+ tests) andgo test -race ./internal/server/gofmt -l internal/ cmd/empty,go vet ./...,make lintcleankamal-proxy run --acme-email … --acme-dns-provider platform.example=cloudflare --acme-dns-provider legacy.example=hetzneragainst staging, one wildcard per zone issued via its own providerDeviations & judgment calls
Required [][]string, but three distinct notions exist today: display metadata, the boot credential check (route53 has none — the AWS SDK resolves credentials itself, including IAM roles), and detection (route53 detects onAWS_ACCESS_KEY_IDorAWS_PROFILE). Collapsing them would change behavior; the registry keepsRequired/Alternatives/NoBootCheck/Detectso every existing credential combination behaves identically.--acme-dns-provider clodflarelogged a warning and silently continued on HTTP-01 only. With mappings validated strictly at startup, keeping a lenient path for the bare form alone would be inconsistent — and the old behavior silently disabled DNS-01.replacesrides the first partition only — a certificate can be "replaced" once.autohides which provider it resolved to.autoDetectProvidertoday effectively always "detects" route53 if reached, because its constructor has no credential check and the AWS SDK constructs without credentials. Pre-existing behavior, preserved exactly; worth its own issue.