docs(acme): derive the provider table and --acme-dns-provider help from the registry - #107
docs(acme): derive the provider table and --acme-dns-provider help from the registry#107mhenrixon wants to merge 2 commits into
Conversation
…om the registry ## Summary The supported-provider list was hand-written in three places that had already drifted: the README credentials table was missing Cloudflare's CF_DNS_API_TOKEN alternative and misfiled gcloud's optional variables as required. Now the registry is the single source: - providers.Names() / ProviderListForHelp() feed the --acme-dns-provider usage string, so `run --help` always matches the registry - ProviderTableMarkdown() renders the README table (docs-linked names, the same OR-of-ANDs credential rule the boot check enforces, optional vars from the entry), rewritten between markers by `go generate ./internal/server/acme/providers` - TestREADMEProviderTable_MatchesRegistry fails naming the missing rows when the committed table does not match the registry — a generator with no drift check is a generator nobody reruns - the README flag-table row no longer enumerates providers; it points at the generated table ## Test Coverage - TestNames_CoversRegistryExactlySorted - TestProviderTableMarkdown_RendersEveryRegistryEntry - TestReplaceProviderTable_RewritesOnlyTheMarkedBlock / _FailsWithoutMarkers - TestREADMEProviderTable_MatchesRegistry (the drift gate) - TestProviderListForHelp_MatchesRegistry - TestRunCommand_DNSProviderHelpMatchesRegistry ## Verification - [x] gofmt/vet clean, make test green, make lint 0 issues, make build clean - [x] bin/kamal-proxy run -h shows the registry-derived list Closes #75
There was a problem hiding this comment.
All reported issues were addressed across 6 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
…d list Review findings on PR #107: - The generated table leads with the name the --acme-dns-provider flag actually accepts: "AWS Route53" is a display name, not a flag value, and the flag-table row tells readers to use names from this table - The drift test names stale rows, not just absent providers: a changed credentials or optional column now surfaces as the differing line instead of "missing rows: []" - acme.GetSupportedProviders() is removed: it had no non-test callers, was itself a hand-maintained provider list kept honest only by a cross-check test, and deriving it from the registry would cycle the acme <- providers import. ParseProviderName remains the validation
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
Confidence score: 5/5
- In
internal/server/acme/providers/docs_test.go, the docs registry/README consistency check can report a misleading "stale or missing row" error when a provider is removed but all rendered lines still exist, which mainly risks slower triage rather than runtime behavior — update the gate/message logic to distinguish true stale rows from registry-removal mismatches.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="internal/server/acme/providers/docs_test.go">
<violation number="1" location="internal/server/acme/providers/docs_test.go:84">
P3: When a provider is removed from the registry but its row remains in README, the equality gate fails while `stale` stays empty (every rendered line is still present in `data`), so the error prints "stale or missing rows:" with no rows and no clue which row is obsolete. The substring `Contains` check can also miss a genuinely removed row when a rendered line appears elsewhere in the document. Compare the committed table region (between the begin/end markers) against the rendered lines to also surface rows that exist in README but are no longer generated.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| // a brand-new provider AND a changed row (credentials, optional vars) | ||
| // both surface as an expected line the document does not contain. | ||
| stale := []string{} | ||
| for line := range strings.Lines(ProviderTableMarkdown()) { |
There was a problem hiding this comment.
P3: When a provider is removed from the registry but its row remains in README, the equality gate fails while stale stays empty (every rendered line is still present in data), so the error prints "stale or missing rows:" with no rows and no clue which row is obsolete. The substring Contains check can also miss a genuinely removed row when a rendered line appears elsewhere in the document. Compare the committed table region (between the begin/end markers) against the rendered lines to also surface rows that exist in README but are no longer generated.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/server/acme/providers/docs_test.go, line 84:
<comment>When a provider is removed from the registry but its row remains in README, the equality gate fails while `stale` stays empty (every rendered line is still present in `data`), so the error prints "stale or missing rows:" with no rows and no clue which row is obsolete. The substring `Contains` check can also miss a genuinely removed row when a rendered line appears elsewhere in the document. Compare the committed table region (between the begin/end markers) against the rendered lines to also surface rows that exist in README but are no longer generated.</comment>
<file context>
@@ -77,13 +77,18 @@ func TestREADMEProviderTable_MatchesRegistry(t *testing.T) {
+ // a brand-new provider AND a changed row (credentials, optional vars)
+ // both surface as an expected line the document does not contain.
+ stale := []string{}
+ for line := range strings.Lines(ProviderTableMarkdown()) {
+ line = strings.TrimSuffix(line, "\n")
+ if line != "" && !strings.Contains(string(data), line) {
</file context>
Summary
The supported-DNS-provider list was hand-maintained in three places — and the drift the issue predicted had already happened: the README credentials table was missing Cloudflare's
CF_DNS_API_TOKENalternative and misfiled gcloud's optional variables as required. The registry (registry.go, from the #77 refactor) is now the single source:internal/server/acme/providers/docs.go(new) —Names()(sorted),ProviderListForHelp()(registry +auto),ProviderTableMarkdown()(docs-linked display names, credentials rendered as the same OR-of-ANDs rule the boot check enforces viacredentialSets(), optionals from the entry), andReplaceProviderTable(), the marker-based block rewrite shared by the generator and the drift test.internal/server/acme/providers/gen/main.go(new) —go generate ./internal/server/acme/providersrewrites the README table in place between<!-- BEGIN/END GENERATED: dns-provider-table -->markers; it refuses to touch a document without markers.TestREADMEProviderTable_MatchesRegistry— the drift gate: a provider added to the registry without regeneration failsmake test, naming the missing rows and the generate command. A generator with no drift check is a generator nobody reruns.internal/cmd/run.go— the--acme-dns-providerusage string is built fromProviderListForHelp();TestRunCommand_DNSProviderHelpMatchesRegistrypins it. The README flag-table row no longer enumerates providers.With this in place, the two provider batches (#73, #74) become registry-entry-only changes: help text, README table, and the drift gate all follow automatically.
Closes #75
Test plan
TestREADMEProviderTable_MatchesRegistryfails (naming rows) when a registry entry is added without regenerating — verified during REDbin/kamal-proxy run -hshows the registry-derived list (cloudflare, digitalocean, gcloud, godaddy, hetzner, namecheap, route53, vultr, auto)ReplaceProviderTabletwice = identical) and deterministic (sorted names)make testgreen (323 tests across touched packages, full suite green),-raceclean on touched packages,make lint0 issues,gofmt/go vetclean,make buildcleanDeviations & judgment calls
kamal-proxy acme providersas an alternative if--helpbloats; at eight (soon twenty) comma-separated names, one usage line is still readable, and the subcommand can be added when it isn't. No RPC/CLI surface added.credentialSets()rather than the flattenedGetProviderInfofields, so the table shows the true OR-of-ANDs rule (Cloudflare's three alternatives) — the exact fidelity the hand-written table lost. Providers withNoBootCheck(route53) still render theirRequiredset: it is the documented happy path even though boot doesn't enforce it.AWS Route53sorts underroute53.Summary by cubic
Derives the
--acme-dns-providerhelp text and the README’s supported-provider table from the ACME provider registry to remove drift and fix incorrect docs. Old: hand-maintained lists with errors. New: registry is the single source; help and docs update automatically. Side effects: rungo generateafter registry changes; removedacme.GetSupportedProviders().name), links to lego docs, renders the exact OR-of-ANDs credential rule, shows optional vars from the registry, and is sorted by provider name. The--acme-dns-providerrow now references this table instead of enumerating names.internal/cmd/run.gobuilds the help list viaproviders.ProviderListForHelp().TestRunCommand_DNSProviderHelpMatchesRegistrypins the output to the registry.go generate ./internal/server/acme/providersrewrites the README block;ReplaceProviderTablerefuses to write without markers.TestREADMEProviderTable_MatchesRegistryfails with stale or missing rows named. Removed the deadacme.GetSupportedProviders()and its cross-check test.Developer workflow
go generate ./internal/server/acme/providers, commit the README change.Written for commit 9323e30. Summary will update on new commits.