From f46004eff9b203df929492b6f160ef23cb629246 Mon Sep 17 00:00:00 2001 From: Tom Limoncelli Date: Mon, 8 Jan 2024 16:01:08 -0500 Subject: [PATCH] DOCS: Update how to submit BYO credentials (#2767) Co-authored-by: Jeffrey Cafferata Co-authored-by: Pascal Mathis --- .github/workflows/pr_test.yml | 2 + documentation/byo-secrets.md | 120 ++++++++++++++++++------- documentation/styleguide-doc.md | 9 +- documentation/writing-providers.md | 137 +++++++++++++++++------------ 4 files changed, 180 insertions(+), 88 deletions(-) diff --git a/.github/workflows/pr_test.yml b/.github/workflows/pr_test.yml index be69881d24..226111f31a 100644 --- a/.github/workflows/pr_test.yml +++ b/.github/workflows/pr_test.yml @@ -102,6 +102,7 @@ jobs: TEST_RESULTS: "/tmp/test-results" GOTESTSUM_FORMAT: testname + # PROVIDER DOMAIN LIST # These providers will be tested if the env variable is set. # Set it to the domain name to use during the test. AZURE_DNS_DOMAIN: ${{ vars.AZURE_DNS_DOMAIN }} @@ -121,6 +122,7 @@ jobs: ROUTE53_DOMAIN: ${{ vars.ROUTE53_DOMAIN }} TRANSIP_DOMAIN: ${{ vars.TRANSIP_DOMAIN }} + # PROVIDER SECRET LIST # The above providers have additional env variables they # need for credentials and such. diff --git a/documentation/byo-secrets.md b/documentation/byo-secrets.md index d4d650bd18..ce7b9fae66 100644 --- a/documentation/byo-secrets.md +++ b/documentation/byo-secrets.md @@ -1,23 +1,17 @@ # Bring-Your-Own-Secrets for automated testing Goal: Enable automated integration testing without accidentally -leaking our API keys and other secrets; at the same time permit anyone -to automate their own tests without having to share their API keys and -secrets. +leaking credentials (API keys and other secrets); at the same time permit everyone +to automate their own tests without having to share their credentials. + +The instructions in this document will enable automated tests to run in these situations: * PR from a project member: - * Automated tests run for a long list of providers. All officially supported - providers have automated tests, plus a few others too. -* PR from an external person - * Automated tests run for a short list of providers. Any test that - requires secrets are skipped in the fork. They will run after the fact though - once the PR has been merged to into the `master` branch of StackExchange/dnscontrol. -* PR from an external person that wants automated tests for their - provider. - * They can set up secrets in their own GitHub account for any tests - they'd like to automate without sharing their secrets. - * Note: These tests can always be run outside of GitHub at the - command line. + * All officially supported providers plus many others too. +* PR from an external people: + * Automated tests run for providers that don't require secrets, which is currently only `BIND`. +* PR on a fork of DNSControl: + * The forker can set up secrets in their fork and only those providers with secrets will be tested. They can "set it and forget it" and all their future PRs will receive all the benefits of automated testing. # Background: How GitHub Actions protects secrets @@ -47,16 +41,16 @@ gets its secrets from TomOnTime's secrets. Our automated integration tests leverages this info to have tests only run if they have access to the secrets they will need. -# How it works +# Which providers are selected for testing? -Tests are executed if `*_DOMAIN` exists where `*` is the name of the provider. If the value is empty or +Tests are executed if the env variable`*_DOMAIN` exists where `*` is the name of the provider. If the value is empty or unset, the test is skipped. For example, if a provider is called `FANCYDNS`, there must -be a secret called `FANCYDNS_DOMAIN`. +be a variable called `FANCYDNS_DOMAIN`. # Bring your own secrets -This section describes how to add a provider to the testing system. +This section describes how to add a provider to the "Actions" part of GitHub. Step 1: Create a branch @@ -64,22 +58,27 @@ Create a branch as you normally would to submit a PR to the project. Step 2: Update `pr_test.yml` -In this branch, edit `.github/workflows/pr_test.yml`: +Edit `.github/workflows/pr_test.yml` + +1. Add the provider to the `PROVIDERS` list. -1. In the `integration-test-providers` section, the name of the provider. +* Add the name of the provider to the PROVIDERS list. +* Please keep this list sorted alphabetically. -Add your provider's name (alphabetically). The line looks something like: {% code title=".github/workflows/pr_test.yml" %} ``` - PROVIDERS: "['BIND','HEXONET','AZURE_DNS','CLOUDFLAREAPI','GCLOUD','NAMEDOTCOM','ROUTE53','CLOUDNS','DIGITALOCEAN','GANDI_V5','HEDNS','INWX','NS1','POWERDNS','TRANSIP']" + PROVIDERS: "['AZURE_DNS','BIND','CLOUDFLAREAPI','CLOUDNS','DIGITALOCEAN','GANDI_V5','GCLOUD','HEDNS','HEXONET','INWX','NAMEDOTCOM','NS1','POWERDNS','ROUTE53','TRANSIP']" ``` {% endcode %} 2. Add your providers `_DOMAIN` env variable: -Add it to the `env` section of `integration-tests`. +* Add it to the `env` section of `integration-tests`. +* Please keep this list sorted alphabetically. + +To find this section, search for `PROVIDER SECRET LIST`. For example, the entry for BIND looks like: @@ -91,7 +90,11 @@ For example, the entry for BIND looks like: 3. Add your providers other ENV variables: -If there are other env variables (for example, for an API key), add that as a "secret". +Every provider requires different variables set to perform the integration tests. The list of such variables is in `integrationTest/providers.json`. + +You've already added `*_DOMAIN` to `pr_test.yml`. Now we're going to add the remaining ones. + +To find this section, search for `PROVIDER SECRET LIST`. For example, the entry for CLOUDFLAREAPI looks like this: @@ -102,15 +105,66 @@ For example, the entry for CLOUDFLAREAPI looks like this: ``` {% endcode %} -Step 3. Submit this PR like any other. +Step 3. Add the secrets to the repo. + +The `*_DOMAIN` variable is stored as a "variable" while the others are stored as "secrets". + +1. Go to Settings -> Secrets and variables -> Actions. + +2. On the "Variables" tab, add `*_DOMAIN` with the name of a test domain. This domain must already exist in the account. The DNS records of the domain will be deleted, so please use a test domain or other disposable domain. + +{% hint style="info" %} +For the main project, **variables** are added here: [https://github.com/StackExchange/dnscontrol/settings/variables/actions](https://github.com/StackExchange/dnscontrol/settings/variables/actions) +{% endhint %} + +3. On the "Secrets" tab, add the other env variables. + +{% hint style="info" %} +For the main project, **secrets** are added here: [https://github.com/StackExchange/dnscontrol/settings/secrets/actions](https://github.com/StackExchange/dnscontrol/settings/secrets/actions) +{% endhint %} + +If you have forked the project, add these to the settings of that fork. + +Step 4. Submit this PR like any other. + +GitHub Actions should kick and and run the tests. + +The tests will fail if a secret is wrong or missing. It may take a few iterations to get everything working because... computers. + +# Donate secrets to the project + +The DNSControl project would like to have all providers automatically tested. +However, we can't fund purchasing domains or maintaining credentials at every +provider. Instead we depend on volunteers to maintain (and pay for) such +accounts. + +We recommend the domain be named `dnscontroltest-PROVIDER.com` (or similar) +where PROVIDER is replaced by the name of your provider or an abbreviation. For +example `dnscontroltest-r53.com` and `dnscontroltest-gcloud.com`. + +When possible, use an OTE or free domain. Don't spend money if you don't have +to. This isn't just to be thrifty! It avoids renewals and other hassles too. +You'd be surprised at how many providers (such as Google and Azure) permit DNS +zones to be created in your account without registering them. + +For actual DNS domains, please select the "private registration" option if it +is available. Otherwise you will get spam phones calls and emails. The phone +calls will make you wish you didn't own a phone. + +{% hint style="danger" %} +Some rules: + +* The account/credentials should only access the test domain. Don't send your company's actual credentials and trust us to only touch the test domain. (this hasn't happened yet, thankfully!) +* Renew the domain in a timely manner. This may be monitoring an email inbox you don't normally monitor. +* Don't do anything that will get you in trouble with your employer, like charging it to your employer without permission. (this hasn't happend yet either, thankfully!) +{% endhint %} + +Now that we've covered all that... +Create a new Github issue with a subject "Add PROVIDER to automated tests" where "PROVIDER" is the name of the provider. DO NOT SEND THE CREDENTIALS IN THE GITHUB ISSUE. Write that you understand the above rules and would like to volunteer to maintain the credentials and account. -# Caveats +To securely send the credentials to the project, use this link: [https://transfer.secretoverflow.com/u/tlimoncelli](https://transfer.secretoverflow.com/u/tlimoncelli) -Sadly there is no locking to prevent two PRs from running the same -test on the same domain at the same time. When that happens, both PRs -running the tests fail. In the future we hope to add some locking. +You'll hear back within a week. -Also, maintaining a fork requires keeping it up to date. That's a bit -more Git knowledge than I can describe here. (I'm not a Git expert by -any stretch of the imagination!) +Thank you for contributing credentials. The more providers we can test automatically with each PR, the better. It "shifts left" finding bugs and API changes and makes less work for everyone. diff --git a/documentation/styleguide-doc.md b/documentation/styleguide-doc.md index cf0c551d4d..80bf387118 100644 --- a/documentation/styleguide-doc.md +++ b/documentation/styleguide-doc.md @@ -70,7 +70,7 @@ Break lines every 80 chars. Include a blank line between paragraphs. -Leave one blank line before and after a heading. +Leave exactly one blank line before and after a heading. JavaScript code should use double quotes (`"`) for strings, not single quotes (`'`). They are equivalent but consistency is good. @@ -218,6 +218,13 @@ Blah blah blah blah blah. Blah blah blah [a search engine](https://www.google.com) blah blah. ``` +## Capitalization matters + +Please capitalize these terms as you see them here: + + * DNSControl + * GitHub + ## Proofreading Please spellcheck documents before submitting a PR. diff --git a/documentation/writing-providers.md b/documentation/writing-providers.md index a9f422ab7b..a4cd00a8b0 100644 --- a/documentation/writing-providers.md +++ b/documentation/writing-providers.md @@ -10,6 +10,8 @@ assigned bugs related to the provider in the future (unless you designate someone else as the maintainer). More details [here](providers.md). +Please follow the [DNSControl Code Style Guide](https://docs.dnscontrol.org/developer-info/styleguide-code) and the [DNSControl Documentation Style Guide](https://docs.dnscontrol.org/developer-info/styleguide-doc). + ## Overview I'll ignore all the small stuff and get to the point. @@ -62,10 +64,11 @@ you write the DnsProvider first, release it, and then write the Registrar if needed. If you have any questions, please discuss them in the GitHub issue -related to the request for this provider. Please let us know what +related to the request for this provider. + +This document is constantly being updated. Please let us know what was confusing so we can update this document with advice for future -authors (or even better, update [this document](https://github.com/StackExchange/dnscontrol/blob/master/documentation/writing-providers.md) -yourself.) +authors (or even better send a PR!). ## Step 2: Pick a base provider @@ -114,7 +117,7 @@ The main driver should be called `providers/name/nameProvider.go`. The API abstraction is usually in a separate file (often called `api.go`). -Directory names should be consitent. It should be all lowercase and match the ALLCAPS provider name. +Directory names should be consitent. It should be all lowercase and match the ALLCAPS provider name. Avoid `_`s. ## Step 4: Activate the driver @@ -151,39 +154,42 @@ Run the unit tests with this command: go test ./... - ## Step 7: Integration Test This is the most important kind of testing when adding a new provider. -Integration tests use a test account and a real domain. +Integration tests use a test account and a test domain. + +{% hint style="danger" %} +All records will be deleted from the test domain! Use a OTE domain or a real domain that isn't otherwise in use and can be destroyed. +{% endhint %} + +* Edit [integrationTest/providers.json](https://github.com/StackExchange/dnscontrol/blob/master/integrationTest/providers.json): + * Add the `creds.json` info required for this provider in the form of environment variables. -* Edit [integrationTest/providers.json](https://github.com/StackExchange/dnscontrol/blob/master/integrationTest/providers.json): Add the `creds.json` info required for this provider. +Now you can run the integration tests. -For example, this will run the tests using BIND: +For example, test BIND: ```shell -cd integrationTest # NOTE: Not needed if already in that subdirectory +cd integrationTest # NOTE: Not needed if already there +export BIND_DOMAIN='example.com' go test -v -verbose -provider BIND ``` -(BIND is a good place to start since it doesn't require any API keys.) +(BIND is a good place to start since it doesn't require API keys.) This will run the tests on Amazon AWS Route53: ```shell -export R53_DOMAIN=dnscontroltest-r53.com # Use a test domain. +export R53_DOMAIN='dnscontroltest-r53.com' # Use a test domain. export R53_KEY_ID='CHANGE_TO_THE_ID' export R53_KEY='CHANGE_TO_THE_KEY' -cd integrationTest # NOTE: Not needed if already in that subdirectory +cd integrationTest # NOTE: Not needed if already there go test -v -verbose -provider ROUTE53 ``` Some useful `go test` flags: -* Slow tests? Add `-timeout n` to increase the timeout for tests - * `go test` kills the tests after 10 minutes by default. Some providers need more time. - * This flag must be *before* the `-verbose` flag. Usually it is the first flag after `go test`. - * Example: `go test -timeout 20m -v -verbose -provider CLOUDFLAREAPI` * Run only certain tests using the `-start` and `-end` flags. * Rather than running all the tests, run just the tests you want. * These flags must be *after* the `-provider FOO` flag. @@ -191,34 +197,42 @@ Some useful `go test` flags: * Example: `go test -v -verbose -provider ROUTE53 -start 5 -end 5` runs only test 5. * Example: `go test -v -verbose -provider ROUTE53 -start 20` skip the first 19 tests. * Example: `go test -v -verbose -provider ROUTE53 -end 20` only run the first 20 tests. +* Slow tests? Add `-timeout n` to increase the timeout for tests + * `go test` kills the tests after 10 minutes by default. Some providers need more time. + * This flag must be *before* the `-verbose` flag. Usually it is the first flag after `go test`. + * Example: `go test -timeout 20m -v -verbose -provider CLOUDFLAREAPI` * If a test will always fail because the provider doesn't support the feature, you can opt out of the test. Look at `func makeTests()` in [integrationTest/integration_test.go](https://github.com/StackExchange/dnscontrol/blob/2f65533e1b92c2967229a92a304fff7c14f7f4b6/integrationTest/integration_test.go#L675) for more details. - ## Step 8: Manual tests +This is optional. + There is a potential bug in how TXT records are handled. Sadly we haven't found an automated way to test for this bug. The manual steps are here in [documentation/testing-txt-records.md](testing-txt-records.md) - -## Step 9: Update docs - -* Edit `README.md`: Add the provider to the bullet list. -* Edit `documentation/providers.md`: Add the provider to the provider list. -* Edit `documentation/SUMMARY.md`: Add the provider to the provider list. -* Create `documentation/providers/PROVIDERNAME.md`: Use one of the other files in that directory as a base. -* Edit `OWNERS`: Add the directory name and your GitHub username. - -## Step 10: Submit a PR - -At this point you can submit a PR. - -Actually you can submit the PR even earlier if you just want feedback, -input, or have questions. This is just a good stopping place to -submit a PR if you haven't already. - - -## Step 11: Capabilities +## Step 9: Update docs, CICD and other files + +* Edit `README.md`: + * Add the provider to the bullet list. +* Edit `.github/workflows/pr_test.yml` + * Add the name of the provider to the PROVIDERS list. +* Edit `documentation/providers.md`: + * Remove the provider from the `Requested providers` list (near the end of the doc) (if needed). + * Add the new provider to the [Providers with "contributor support"](providers.md#providers-with-contributor-support) section. +* Edit `documentation/SUMMARY.md`: + * Add the provider to the "Providers" list. +* Create `documentation/providers/PROVIDERNAME.md`: + * Use one of the other files in that directory as a base. +* Edit `OWNERS`: + * Add the directory name and your GitHub username. + +{% hint style="success" %} +**Need feedback?** Submit a draft PR! It's a great way to get early feedback, ask about fixing +a particular integration test, or request feedback. +{% endhint %} + +## Step 9: Capabilities Some DNS providers have features that others do not. For example some support the SRV record. A provider announces what it can do using @@ -251,10 +265,9 @@ you want to implement. FYI: If a provider's capabilities changes, run `go generate` to update the documentation. +## Step 10: Automated code tests -## Step 12: Clean up - -Run "go vet" and ["staticcheck"](https://staticcheck.io/) and clean up any errors found. +Run `go vet` and [`staticcheck`](https://staticcheck.io/) and clean up any errors found. ```shell go vet ./... @@ -276,30 +289,46 @@ go install golang.org/x/lint/golint golint ./... ``` - -## Step 13: Dependencies +## Step 11: Dependencies See [documentation/release-engineering.md](release-engineering.md) for tips about managing modules and checking for outdated dependencies. - -## Step 14: Modify the release regexp +## Step 12: Modify the release regexp In the repo root, open `.goreleaser.yml` and add the provider to `Provider-specific changes` regexp. +## Step 13: Check your work + +These are the things we'll be checking when you submit the PR. Please try to complete all or as many of these as possible. + +1. Run `go generate ./...` to make sure all generated files are fresh. +2. Make sure the following files were created and/or updated: + * `OWNERS` + * `README.md` + * `.github/workflows/pr_test.yml` (The PROVIDERS list) + * `.goreleaser.yml` (Search for `Provider-specific changes`) + * `documentation/SUMMARY.md` + * `documentation/providers.md` (the autogenerated table + the second one; make sure it is removed from the `requested` list) + * `documentation/providers/`PROVIDERNAME`.md` + * `integrationTest/providers.json` + * `providers/_all/all.go` +3. Review the code for style issues, remove debug statements, make sure all exported functions have a comment, and generally tighten up the code. +4. Verify you're using the most recent version of anything you import. (See [Step 13](#step-11-dependencies)) +5. Re-run the [integration test](#step-7-integration-test) one last time. + * Post the results as a comment to your PR. +6. Re-read the [maintainer's responsibilities](providers.md#providers-with-contributor-support) bullet list. By submitting a provider you agree to maintain it, respond to bugs, periodically re-run the integration test to verify nothing has broken, and if we don't hear from you for 2 months we may disable the provider. + +## Step 14: Submit a PR -## Step 15: Check your work - -Here are some last-minute things to check before you submit your PR. +At this point you can submit a PR. -1. Run `go generate` to make sure all generated files are fresh. -2. Make sure all appropriate documentation is current. (See [Step 8](#step-8-manual-tests)) -3. Check that dependencies are current (See [Step 13](#step-13-dependencies)) -4. Re-run the integration test one last time (See [Step 7](#step-7-integration-test)) -5. Re-read the [maintainer's responsibilities](providers.md) bullet list. By submitting a provider you agree to maintain it, respond to bugs, periodically re-run the integration test to verify nothing has broken, and if we don't hear from you for 2 months we may disable the provider. +Actually you can submit the PR even earlier if you just want feedback, +input, or have questions. This is just a good stopping place to +submit a PR if you haven't already. -## Step 16: After the PR is merged +## Step 15: After the PR is merged -1. Remove the "provider-request" label from the PR. -2. Verify that [documentation/providers.md](providers.md) no longer shows the provider as "requested" +1. Close any related GitHub issues. +3. Would you like your provider to be tested automatically as part of every PR? Sure you would! Follow the instructions in [Bring-Your-Own-Secrets for automated testing](byo-secrets.md)