FEATURE: Init command#4208
Conversation
chicks-net
left a comment
There was a problem hiding this comment.
Very clean and useful.
| provider reduces to a few lines: | ||
|
|
||
| ```go | ||
| providers.RegisterCredsMetadata("MYPROVIDER", providers.CredsMetadata{ |
There was a problem hiding this comment.
This example is good and I wouldn't replace it, but it would be nice if the longer example from the github issue were also included. Another way to accomplish this without writing so much in this doc would be pointing at the other working examples you've done.
There was a problem hiding this comment.
Good call. Went with your second suggestion (point at the working examples) so the doc stays compact and does not drift when the providers evolve. The bind and transip references are now clickable links plus a one line description of what each example demonstrates (16e5e1f).
TomOnTime
left a comment
There was a problem hiding this comment.
Wow! This is really going to make onboarding a lot easier!
8f7f6bc to
fabc46c
Compare
|
Heads up: the lint job is failing on three |
|
@TomOnTime thanks for the wording suggestions on the documentation. I've processed your textual feedback in 0d23b6b, so the onboarding docs and the On top of that, I fixed the failing lint job in 371f05e by migrating from From my side this is ready for another review pass. Let me know if you'd like any further tweaks! |
|
Ah! We were both working on the golangci-lint issue at the same time! I will revert #4215 and use your solution. |
|
@TomOnTime I've re-resolved the merge conflict with #4222 in |
947a40b to
f25ce33
Compare
|
@TomOnTime I've added an additional commit f25ce33 to apply the renamed import paths from |
I ran into this same issue while working on #4208 and can confirm the behavior @stefanwascoding describes. The only form that works requires the provider name explicitly: ```shell dnscontrol get-zones transip TRANSIP all ``` ```text WARNING: To retain compatibility in future versions, please change "TRANSIP" to "-". See "https://docs.dnscontrol.org/commands/get-zones" ``` But following that suggestion also fails: ```shell dnscontrol get-zones transip - all ``` ```text Arguments should be: credskey providername zone(s) (Ex: r53 ROUTE53 example.com) ``` So neither the documented 2 argument form (`transip all`) nor the suggested dash form (`transip - all`) works. This pull request fixes the argument parsing so that `get-zones` accepts 2 arguments (`credkey zone`), resolving the provider type from the `TYPE` field in `creds.json`. The old 3 argument form still works but now shows a deprecation warning with the correct command: ```shell dnscontrol get-zones transip TRANSIP all ``` ```text WARNING: The provider name argument is deprecated. Please use "dnscontrol get-zones transip all" instead. See "https://docs.dnscontrol.org/commands/get-zones" ``` With 3+ arguments, the code now detects whether the second argument is a registered provider type to distinguish the deprecated form (`credkey provider zone...`) from the new multi-zone form (`credkey zone1 zone2...`): ```shell dnscontrol get-zones --format=nameonly transip dnscontrol.org dnscontrol.nl ``` ```text dnscontrol.org dnscontrol.nl ``` Additionally: - CLI help is shortened and links to online documentation - Examples use descriptive `credkey` names (`my_route53` instead of `myr53`) so they are self-explanatory without the provider name - `Documentation:` links are added to `get-zones` and `check-creds` - `documentation/commands/get-zones.md` is updated to match Tested against the TransIP provider with the `dnscontrol.org` zone from #4204. - [x] `go test ./commands/` - [x] `dnscontrol get-zones transip all` (2 args, new style) - [x] `dnscontrol get-zones transip dnscontrol.org` (specific zone) - [x] `dnscontrol get-zones transip dnscontrol.org dnscontrol.nl` (multiple zones, new style) - [x] `dnscontrol get-zones transip TRANSIP all` (3 args, deprecation warning) - [x] `dnscontrol get-zones transip` (too few args, error message) - [x] `dnscontrol get-zones --format=zone transip dnscontrol.org` - [x] `dnscontrol get-zones --format=tsv transip dnscontrol.org` - [x] `dnscontrol get-zones --format=nameonly transip all` <details> <summary>CLI diff</summary> ```diff --- before (v4.37.1) +++ after @@ -2,7 +2,7 @@ dnscontrol get-zones - gets a zone from a provider (stand-alone) USAGE: - dnscontrol get-zones [command options] credkey provider zone [...] + dnscontrol get-zones [command options] credkey zone [...] CATEGORY: utility @@ -11,34 +11,18 @@ Download a zone from a provider. This is a stand-alone utility. ARGUMENTS: - credkey: The name used in creds.json (first parameter to NewDnsProvider() in dnsconfig.js) + credkey: The name used in creds.json - provider: The name of the provider (second parameter to NewDnsProvider() in dnsconfig.js) zone: One or more zones (domains) to download; or "all". - FORMATS: - --format=js dnsconfig.js format (not perfect, just a decent first draft) - --format=djs js with disco commas (leading commas) - --format=zone BIND zonefile format - --format=tsv TAB separated value (useful for AWK) - --format=nameonly Just print the zone names - - The columns in --format=tsv are: - FQDN (the label with the domain) - ShortName (just the label, "@" if it is the naked domain) - TTL - Record Type (A, AAAA, CNAME, etc.) - Target and arguments (quoted like in a zonefile) - Either empty or a comma-separated list of properties like "cloudflare_proxy=true" - - The --ttl flag only applies to zone/js/djs formats. - EXAMPLES: - dnscontrol get-zones myr53 ROUTE53 example.com - dnscontrol get-zones gmain GANDI_V5 example.com other.com - dnscontrol get-zones cfmain CLOUDFLAREAPI all - dnscontrol get-zones --format=tsv bind BIND example.com - dnscontrol get-zones --format=djs --out=draft.js gcloud GCLOUD example.com + dnscontrol get-zones my_route53 example.com + dnscontrol get-zones my_gandi example.com other.com + dnscontrol get-zones my_cloudflare all + dnscontrol get-zones --format=tsv my_bind example.com + dnscontrol get-zones --format=djs --out=draft.js my_gcloud example.com + Documentation: https://docs.dnscontrol.org/commands/get-zones + OPTIONS: --creds string Provider credentials JSON file (or !program to execute program that outputs json) (default: "creds.json") --format string Output format: js djs zone tsv nameonly (default: "zone") ``` </details> Fixes #4235
|
@TomOnTime I've resolved the merge conflicts with #4242 ( All previous review feedback has been addressed, so from my side this is ready for a final review pass. Let me know if you'd like any further changes! |
52794a7 to
d40c5cb
Compare
|
I'm going to merge this but there are some minor nits that I'd like you to consider for future releases: (1) When the default BIND values are used, don't store the value in the JSON: These lines are unnecessary in creds.json: (2) I think it is confusing if the credkey is the same as the provider name. How about defaulting to It also future-proofs the The name scheme that I recommend is |
|
|
||
| A DNS provider hosts the records (A, MX, TXT, CNAME, and so on) for your zones. | ||
| Pick NONE if you want to defer this choice. | ||
| ? Which DNS service provider do you want to configure? CLOUDFLAREAPI |
There was a problem hiding this comment.
When I run this I don't get CLOUDFLAREAPI as an option. Did a file not get checked in?
Co-authored-by: Tom Limoncelli <tal@whatexit.org>
- When the user accepts the default for an optional `CredsField`, the value is no longer written to `creds.json`. This avoids redundant entries like `"directory": "zones"` and `"filenameformat": "%c.zone"` for the BIND provider.
…nscontrol init`. - Change `defaultEntryName` from `cloudflareapi` to `cloudflareapi_primary` so the credkey is visually distinct from the provider TYPE and future-proofs for multiple accounts. - Update flow test and documentation example to match.
|
The Cloudflare example in the documentation predates the change to only list providers that have registered onboarding metadata. That change was made after feedback from @tomfitzhenry suggesting there should be no generic fallback, and I forgot to update the Cloudflare example afterwards. The two nits from your other comment are addressed in separate commits on this branch: |
|
This is such an exciting new feature! I can't wait for more providers to support it! (Maybe @fm can help communicate to all the providers) |
Add interactive
dnscontrol initonboarding wizardCloses #4205
Summary
A new user can run
dnscontrol initand have a workingcreds.jsonplus a starterdnsconfig.jswithin a minute. The wizard asks the right questions for the chosen provider and writes both files for them. Existing workflows are unaffected; users who prefer to hand editcreds.jsoncan keep doing exactly that.Why
DNSControl's biggest entry barrier is the very first ten minutes. Newcomers have to discover, on their own, what fields belong in
creds.jsonfor their provider, where to create the API token, whether a value should be a secret, whether it spans multiple lines. That information lives in three places (the provider documentation, the provider's source, and the integration tests workflow) and rarely all in the same form. After helping multiple developers through that ramp myself, the friction is recognisable every time.The wizard collapses that into a single guided session. A maintainer describes their provider's credential fields once via
RegisterCredsMetadata, and from then on every newcomer reuses that knowledge.What this gives users
$EDITORso multiline values stay intact.dnsconfig.jswith the live zones at the provider, and to rundnscontrol previewas a sanity check.What this gives provider maintainers
A small registration block next to the existing
RegisterMaintainercall:That is enough for the provider to appear in the wizard. Providers with multiple auth methods (TransIP access token versus account name plus private key, or Route53 static keys versus assume role) can branch with
InternalandShowIf.Three providers ship with metadata in this PR as a starting point and a worked example: NONE, BIND, and TransIP. The remaining providers can land incrementally in follow up PRs, one per maintainer.
Scope and follow ups
Try it
Check out the branch:
Build and run the wizard against an empty directory:
Try one of the included providers (NONE, BIND, TransIP) to see the full flow including the optional zone comparison and
dnscontrol previewstep.Test plan
go build ./...go test ./...$EDITOR=nano).Documentation preview (GitBook)
The new and updated pages render through the GitBook Git Sync. Preview URLs:
index.md: https://docs.dnscontrol.org/~/revisions/qjuEAKjgO6xGzbKshhTH#try-itcommands/init.md: https://docs.dnscontrol.org/~/revisions/qjuEAKjgO6xGzbKshhTH/commands/initgetting-started/getting-started.md: https://docs.dnscontrol.org/~/revisions/qjuEAKjgO6xGzbKshhTH/getting-started/getting-started#id-3.-create-the-initial-dnsconfig.jsgetting-started/migrating.md: https://docs.dnscontrol.org/~/revisions/qjuEAKjgO6xGzbKshhTH/getting-started/migratingcommands/creds-json.md: https://docs.dnscontrol.org/~/revisions/qjuEAKjgO6xGzbKshhTH/commands/creds-jsonLinked discussion
Initial proposal and feedback live in #4205. Earlier consensus there: drop
DomainsURL, keep the auth method picker, drop the generic fallback for the first version of the PR, and keep the per provider documentation pages intact.