Enable Bring-Your-Own-Secrets (#982)

* Move env from steps to job
* Rename env to be predictable.
* fix conditional
* document B.O.Y.S.
* Update docs/byo-secrets.md

Co-authored-by: Max Horstmann <MaxHorstmann@users.noreply.github.com>

* respond to reviews
* fixup!

Co-authored-by: Max Horstmann <MaxHorstmann@users.noreply.github.com>

.github/workflows/build.yml

@@ -1,3 +1,4 @@
+---
 name: build
 
 on:
@@ -44,6 +45,69 @@ jobs:
         - DIGITALOCEAN
         - GANDI_V5
         - INWX
+# Bring-Your-Own-Secrets:
+# To reduce the risk of secrets being logged by third-parties, secrets
+# come from the account of the fork.  For example, the PR submitted by
+# a member of the project has access to the secrets in
+# github.com/StackExchange/dnscontrol. However a PR submitted by a
+# third-party receives secrets from the account of their fork.
+#
+# If a test requires no secrets: List any parameters here in
+# plaintext. (see BIND and HEXONET as examples).
+# However secrets are needed for most tests. In that case, create a secret called
+# ${PROVIDER}_DOMAIN and other env variables listed in
+# integrationTest/providers.json for that provider.  the test will only run on systems
+# with access to those secrets (specifically, the ${PROVIDER}_DOMAIN secret).
+# This way the main project can maintain its tests and secrets
+# securely, plus forks can run their own tests.
+#
+# See https://stackexchange.github.io/dnscontrol/byo-secrets
+#
+# (Sort order: groups in the same order as the matrix; _DOMAIN first; sort the others alphabetically.)
+    env:
+      BIND_DOMAIN: example.com
+#
+      HEXONET_DOMAIN: a-b-c-movies.com
+      HEXONET_ENTITY: OTE
+      HEXONET_PW: test.passw0rd
+      HEXONET_UID: test.user
+#
+      AZURE_DNS_DOMAIN: ${{ secrets.AZURE_DNS_DOMAIN }}
+      AZURE_DNS_CLIENT_ID: ${{ secrets.AZURE_DNS_CLIENT_ID }}
+      AZURE_DNS_CLIENT_SECRET: ${{ secrets.AZURE_DNS_CLIENT_SECRET }}
+      AZURE_DNS_RESOURCE_GROUP: DNSControl
+      AZURE_DNS_SUBSCRIPTION_ID: ${{ secrets.AZURE_DNS_SUBSCRIPTION_ID }}
+      AZURE_DNS_TENANT_ID: ${{ secrets.AZURE_DNS_TENANT_ID }}
+#
+      CLOUDFLAREAPI_DOMAIN: ${{ secrets.CLOUDFLAREAPI_DOMAIN }}
+      CLOUDFLAREAPI_KEY: ${{ secrets.CLOUDFLAREAPI_KEY }}
+      CLOUDFLAREAPI_TOKEN: ${{ secrets.CLOUDFLAREAPI_TOKEN }}
+      CLOUDFLAREAPI_USER: ${{ secrets.CLOUDFLAREAPI_USER }}
+#
+      GCLOUD_DOMAIN: ${{ secrets.GCLOUD_DOMAIN }}
+      GCLOUD_EMAIL: dnscontrol@dnscontrol-dev.iam.gserviceaccount.com
+      GCLOUD_PRIVATEKEY: ${{ secrets.GCLOUD_PRIVATEKEY }}
+      GCLOUD_PROJECT: dnscontrol-dev
+      GCLOUD_TYPE: service_account
+#
+      NAMEDOTCOM_DOMAIN: ${{ secrets.NAMEDOTCOM_DOMAIN }}
+      NAMEDOTCOM_KEY: ${{ secrets.NAMEDOTCOM_KEY }}
+      NAMEDOTCOM_URL: api.name.com
+      NAMEDOTCOM_USER: dnscontroltest
+#
+      ROUTE53_DOMAIN: ${{ secrets.ROUTE53_DOMAIN }}
+      ROUTE53_KEY: ${{ secrets.ROUTE53_KEY }}
+      ROUTE53_KEY_ID: ${{ secrets.ROUTE53_KEY_ID }}
+#
+      DIGITALOCEAN_DOMAIN: ${{ secrets.DIGITALOCEAN_DOMAIN }}
+      DIGITALOCEAN_TOKEN: ${{ secrets.DIGITALOCEAN_TOKEN }}
+#
+      GANDI_V5_DOMAIN: ${{ secrets.GANDI_V5_DOMAIN }}
+      GANDI_V5_APIKEY: ${{ secrets.GANDI_V5_APIKEY }}
+#
+      INWX_DOMAIN: ${{ secrets.INWX_DOMAIN }}
+      INWX_PASSWORD: ${{ secrets.INWX_PASSWORD }}
+      INWX_USER: ${{ secrets.INWX_USER }}
     steps:
     - name: Checkout repo
       uses: actions/checkout@v2
@@ -54,60 +118,9 @@ jobs:
       with:
         go-version: ^1.15
     - name: Determining test viability for ${{ matrix.provider }} provider
-      run: if [ "$${{ matrix.provider }}__CAN_SECRET" = "true" ] ; then echo "CAN_CONTINUE=yes" >> "$GITHUB_ENV" ; fi
+      run: if [ -n "$${{ matrix.provider }}_DOMAIN" ] ; then echo "CAN_CONTINUE=yes" >> "$GITHUB_ENV" ; fi
-# Does the provider's tests require secrets?
-# Yes?  Set a secret called ${PROVIDER}__CAN_SECRET with value "true" (no quotes).
-# No?   Set it to "true" like you see for BIND__CAN_SECRET.
-# This way tests only run if the secrets are available to the runner.
-# A fork can "bring your own secrets" for locally-defined tests.
-# Please keep the list sorted.
-      env:
-        AZURE_DNS__CAN_SECRET: ${{ secrets.AZURE_DNS__CAN_SECRET }}
-        BIND__CAN_SECRET: true
-        CLOUDFLAREAPI__CAN_SECRET: ${{ secrets.CLOUDFLAREAPI__CAN_SECRET }}
-        DIGITALOCEAN__CAN_SECRET: ${{ secrets.DIGITALOCEAN__CAN_SECRET }}
-        GANDI_V5__CAN_SECRET: ${{ secrets.GANDI_V5__CAN_SECRET }}
-        GCLOUD__CAN_SECRET: ${{ secrets.GCLOUD__CAN_SECRET }}
-        HEXONET__CAN_SECRET: true
-        INWX__CAN_SECRET: ${{ secrets.INWX__CAN_SECRET }}
-        NAMEDOTCOM__CAN_SECRET: ${{ secrets.NAMEDOTCOM__CAN_SECRET }}
-        ROUTE53__CAN_SECRET: ${{ secrets.ROUTE53__CAN_SECRET }}
     - name: Run integration tests for ${{ matrix.provider }} provider
+      if: env.CAN_CONTINUE == 'yes'
       working-directory: integrationTest
       run: go test -v -verbose -provider ${{ matrix.provider }}
-      if: ${{ env.CAN_CONTINUE == 'yes' }}
+...
-# Extract the secrets that are used by the tests. (Please keep this list sorted)
-      env:
-        AZURE_CLIENT_ID: ${{ secrets.AZURE_CLIENT_ID }}
-        AZURE_CLIENT_SECRET: ${{ secrets.AZURE_CLIENT_SECRET }}
-        AZURE_DOMAIN: dnscontrol-azure.com
-        AZURE_RESOURCE_GROUP: DNSControl
-        AZURE_SUBSCRIPTION_ID: ${{ secrets.AZURE_SUBSCRIPTION_ID }}
-        AZURE_TENANT_ID: ${{ secrets.AZURE_TENANT_ID }}
-        CF_DOMAIN: ${{ secrets.CF_DOMAIN }}
-        CF_KEY: ${{ secrets.CF_KEY }}
-        CF_TOKEN: ${{ secrets.CF_TOKEN }}
-        CF_USER: ${{ secrets.CF_USER }}
-        DO_DOMAIN: dnscontrol-do.com
-        DO_TOKEN: ${{ secrets.DO_TOKEN }}
-        GANDI_V5_APIKEY: ${{ secrets.GANDI_V5_APIKEY }}
-        GANDI_V5_DOMAIN: dnscontroltest-gandilivedns.com
-        GCLOUD_DOMAIN: dnscontroltest-gcloud.com
-        GCLOUD_EMAIL: dnscontrol@dnscontrol-dev.iam.gserviceaccount.com
-        GCLOUD_PRIVATEKEY: ${{ secrets.GCLOUD_PRIVATEKEY }}
-        GCLOUD_PROJECT: dnscontrol-dev
-        GCLOUD_TYPE: service_account
-        HEXONET_DOMAIN: a-b-c-movies.com
-        HEXONET_ENTITY: OTE
-        HEXONET_PW: test.passw0rd
-        HEXONET_UID: test.user
-        INWX_DOMAIN: ${{ secrets.INWX_DOMAIN }}
-        INWX_PASSWORD: ${{ secrets.INWX_PASSWORD }}
-        INWX_USER: ${{ secrets.INWX_USER }}
-        NAMEDOTCOM_DOMAIN: dnscontrol-ndc.com
-        NAMEDOTCOM_KEY: ${{ secrets.NAMEDOTCOM_KEY }}
-        NAMEDOTCOM_URL: api.name.com
-        NAMEDOTCOM_USER: dnscontroltest
-        R53_DOMAIN: dnscontroltest-r53.com
-        R53_KEY: ${{ secrets.R53_KEY }}
-        R53_KEY_ID: ${{ secrets.R53_KEY_ID }}

docs/byo-secrets.md

@@ -0,0 +1,180 @@
+---
+layout: default
+title: Bring-Your-Own-Secrets for automated testing
+---
+
+# 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.
+
+* 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.
+
+# Background: How GitHub Actions protects secrets
+
+Github Actions has a secure
+[secrets storage system](https://docs.github.com/en/free-pro-team@latest/actions/reference/encrypted-secrets).
+Those secrets are available to Github Actions and are required for the
+integration tests to communicate with the various DNS providers that
+DNSControl supports.
+
+For security reasons, those secrets are unavailable if the PR comes
+from outside the project (a forked repo).  This is a good thing.  If
+it didn't work that way, a third-party could write a PR that leaks the
+secrets without the owners of the project knowing.
+
+The docs (and many blog posts) describe this as forked repos don't
+have access to secrets, and instead receive null strings. That's not
+actually what's happening.
+
+Actually what happens is the secrets come from the forked repo.  Or,
+more precisely, the secrets offered to a PR come from the repo that the
+PR came from.  A PR from DNSControl's owners gets secrets from
+[github.com/StackExchange/dnscontrol's secret store](https://github.com/StackExchange/dnscontrol/settings/secrets/actions)
+but a PR from a fork, such as
+[https://github.com/TomOnTime/dnscontrol](https://github.com/TomOnTime/dnscontrol)
+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:
+
+Tests are executed if `*_DOMAIN` exists.  If the value is empty or
+unset, the test is skipped.  If a test doesn't require secrets, the
+`*_DOMAIN` variable is hardcoded.  Otherwise, it is set by looking up
+the secret. For example, if a provider is called `FANCYDNS`, there must
+be a secret called `FANCYDNS_DOMAIN`.
+
+# Bring your own secrets
+
+This section describes how to add a provider to the testing system.
+
+In this example, we will use a fictional DNS provider named
+"FANCYDNS".
+
+Step 1: Create a branch
+
+Create a branch as you normally would to submit a PR to the project.
+
+Step 2: Update `build.yml`
+
+In this branch, edit `.github/workflows/build.yml`:
+
+1. In the `integration-tests` section, add the name of your provider
+   to the matrix of providers.  Technically you are adding to the list
+   at `jobs.integration-tests.strategy.matrix.provider`.
+
+```
+      matrix:
+        provider:
+...
+        - DIGITALOCEAN
+        - GANDI_V5
+        - FANCYDNS          <<< NEW ITEM ADDED HERE
+        - INWX
+```
+
+2. Add your test's env:
+
+Locate the env section (technically this is `jobs.integration-tests.env`) and
+add all the env names that your provider sets in
+`integrationTest/providers.json`.
+
+Please replicate the formatting of the existing entries:
+
+* A blank comment separates each provider's section.
+* The providers are listed in the same order as the matrix.provider list.
+* The `*_DOMAIN` variable is first.
+* The remaining variables are sorted lexicographically (what nerds call alphabetical order).
+
+```
+      FANCYDNS_DOMAIN: ${{ secrets.FANCYDNS_DOMAIN }}
+      FANCYDNS_KEY: ${{ secrets.FANCYDNS_KEY }}
+      FANCYDNS_USER: ${{ secrets.FANCYDNS_USER }}
+```
+
+# Examples
+
+Let's look at three examples:
+
+## Example 1:
+
+The `BIND` integration tests do not require any secrets because it
+simply generates files locally.
+
+```
+      BIND_DOMAIN: example.com
+```
+
+The existence of `BIND_DOMAIN`, and the fact that the value is
+available to all, means these tests will run for everyone.
+
+## Example 2:
+
+The `AZURE_DNS` provider requires many settings. Since
+`AZURE_DNS_DOMAIN` comes from GHA's secrets storage, we can be assured
+that the tests will skip if the PR does not have access to the
+secrets.
+
+If you have a fork and want to automate the testing of `AZURE_DNS`,
+simply set the secrets named in `build.yml` and the tests will
+activate for your PRs.
+
+Note that `AZURE_DNS_RESOURCE_GROUP` is hardcoded to `DNSControl`. If
+this is not true for you, please feel free to submit a PR that turns
+it into a secret.
+
+```
+      AZURE_DNS_DOMAIN: ${{ secrets.AZURE_DNS_DOMAIN }}
+      AZURE_DNS_CLIENT_ID: ${{ secrets.AZURE_DNS_CLIENT_ID }}
+      AZURE_DNS_CLIENT_SECRET: ${{ secrets.AZURE_DNS_CLIENT_SECRET }}
+      AZURE_DNS_RESOURCE_GROUP: DNSControl
+      AZURE_DNS_SUBSCRIPTION_ID: ${{ secrets.AZURE_DNS_SUBSCRIPTION_ID }}
+      AZURE_DNS_TENANT_ID: ${{ secrets.AZURE_DNS_TENANT_ID }}
+```
+
+## Example 3:
+
+The HEXONET integration tests require secrets, but HEXONET provides an
+Operational Test and Evaluation (OT&E) environment with some "fake"
+credentials which are known publicly.
+
+Therefore, since there's nothing secret about these particular
+secrets, we hard-code them into the `build.yml` file. Since
+`HEXONET_DOMAIN` does not come from secret storage, everyone can run
+these tests. (We are grateful to HEXONET for this public service!)
+
+```
+      HEXONET_DOMAIN: a-b-c-movies.com
+      HEXONET_ENTITY: OTE
+      HEXONET_PW: test.passw0rd
+      HEXONET_UID: test.user
+```
+
+NOTE: The above credentials are [known to the public]({{site.github.url}}/providers/hexonet).
+
+
+# Caveats
+
+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.
+
+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!)

docs/index.md

@@ -164,16 +164,19 @@ title: DNSControl
 					Mailing list: <a href="https://groups.google.com/forum/#!forum/dnscontrol-discuss">dnscontrol-discuss</a>: The friendly best place to ask questions and propose new features
 				</li>
 				<li>
-					<a href="{{site.github.url}}/writing-providers">Step-by-Step Guide: Writing Providers</a>: How to write a DNS or Registrar Provider
+					<a href="{{site.github.url}}/bug-triage">Bug Triage</a>: How bugs are triaged
-				</li>
-				<li>
-					<a href="{{site.github.url}}/adding-new-rtypes">Step-by-Step Guide: Adding new DNS rtypes</a>: How to add a new DNS record type
 				</li>
 				<li>
 					<a href="{{site.github.url}}/release-engineering">Release Engineering</a>: How to build and ship a release
 				</li>
 				<li>
-					<a href="{{site.github.url}}/bug-triage">Bug Triage</a>: How bugs are triaged
+					<a href="{{site.github.url}}/byo-secrets">Bring-Your-Own-Secrets</a>: Automate tests
+				</li>
+				<li>
+					<a href="{{site.github.url}}/writing-providers">Step-by-Step Guide: Writing Providers</a>: How to write a DNS or Registrar Provider
+				</li>
+				<li>
+					<a href="{{site.github.url}}/adding-new-rtypes">Step-by-Step Guide: Adding new DNS rtypes</a>: How to add a new DNS record type
 				</li>
 			</ul>
 		</div>

integrationTest/providers.json

@@ -11,21 +11,21 @@
     "update-key": "$AXFRDDNS_UPDATE_KEY"
   },
   "AZURE_DNS": {
-    "ClientID": "$AZURE_CLIENT_ID",
+    "ClientID": "$AZURE_DNS_CLIENT_ID",
-    "ClientSecret": "$AZURE_CLIENT_SECRET",
+    "ClientSecret": "$AZURE_DNS_CLIENT_SECRET",
-    "ResourceGroup": "$AZURE_RESOURCE_GROUP",
+    "ResourceGroup": "$AZURE_DNS_RESOURCE_GROUP",
-    "SubscriptionID": "$AZURE_SUBSCRIPTION_ID",
+    "SubscriptionID": "$AZURE_DNS_SUBSCRIPTION_ID",
-    "TenantID": "$AZURE_TENANT_ID",
+    "TenantID": "$AZURE_DNS_TENANT_ID",
-    "domain": "$AZURE_DOMAIN"
+    "domain": "$AZURE_DNS_DOMAIN"
   },
   "BIND": {
-    "domain": "example.com"
+    "domain": "$BIND_DOMAIN"
   },
   "CLOUDFLAREAPI": {
-    "apikey": "$CF_KEY",
+    "apikey": "$CLOUDFLAREAPI_KEY",
-    "apitoken": "$CF_TOKEN",
+    "apitoken": "$CLOUDFLAREAPI_TOKEN",
-    "apiuser": "$CF_USER",
+    "apiuser": "$CLOUDFLAREAPI_USER",
-    "domain": "$CF_DOMAIN"
+    "domain": "$CLOUDFLAREAPI_DOMAIN"
   },
   "CLOUDFLAREAPI_OLD": {
     "apikey": "$CF_KEY",
@@ -39,8 +39,8 @@
     "domain": "$CLOUDNS_DOMAIN"
   },
   "DIGITALOCEAN": {
-    "domain": "$DO_DOMAIN",
+    "domain": "$DIGITALOCEAN_DOMAIN",
-    "token": "$DO_TOKEN"
+    "token": "$DIGITALOCEAN_TOKEN"
   },
   "DNSIMPLE": {
     "baseurl": "https://api.sandbox.dnsimple.com",
@@ -74,8 +74,8 @@
   "HETZNER": {
     "api_key": "$HETZNER_API_KEY",
     "domain": "$HETZNER_DOMAIN",
-    "start_with_default_rate_limit": "true",
+    "optimize_for_rate_limit_quota": "Hour",
-    "optimize_for_rate_limit_quota": "Hour"
+    "start_with_default_rate_limit": "true"
   },
   "HEXONET": {
     "apientity": "$HEXONET_ENTITY",
@@ -132,9 +132,9 @@
     "servername": "$POWERDNS_SERVERNAME"
   },
   "ROUTE53": {
-    "KeyId": "$R53_KEY_ID",
+    "KeyId": "$ROUTE53_KEY_ID",
-    "SecretKey": "$R53_KEY",
+    "SecretKey": "$ROUTE53_KEY",
-    "domain": "$R53_DOMAIN"
+    "domain": "$ROUTE53_DOMAIN"
   },
   "SOFTLAYER": {
     "api_key": "$SL_API_KEY",