Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

RELEASE: Move some BYOS steps to be standard #3107

Merged
merged 8 commits into from
Sep 17, 2024
Merged

RELEASE: Move some BYOS steps to be standard #3107

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3c5fd5c
RELEASE: Move some BYOS steps to be standard
tlimoncelli Sep 3, 2024
369df9b
Update documentation/writing-providers.md
tlimoncelli Sep 11, 2024
6994cd5
Update documentation/writing-providers.md
tlimoncelli Sep 11, 2024
632077c
Update documentation/writing-providers.md
tlimoncelli Sep 11, 2024
9b1f8bb
Merge branch 'main' into tlim_newprov
tlimoncelli Sep 12, 2024
127a3be
Update documentation/writing-providers.md
tlimoncelli Sep 12, 2024
52ae2af
responding to cafferata
tlimoncelli Sep 12, 2024
cf387bd
Merge branch 'main' into tlim_newprov
tlimoncelli Sep 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions documentation/byo-secrets.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ Create a branch as you normally would to submit a PR to the project.

Step 2: Update `pr_test.yml`

{% hint style="info" %}
Edits to `pr_test.yml` may have already been done for you.
{% endhint %}

Edit `.github/workflows/pr_test.yml`

1. Add the provider to the `PROVIDERS` list.
Expand Down
53 changes: 52 additions & 1 deletion documentation/writing-providers.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,57 @@ dependencies.

In the repo root, open `.goreleaser.yml` and add the provider to `Provider-specific changes` regexp.

## Step 14: Update `pr_test.yml`

This assures that in the future it will be easy to test this provider using Github Actions.
tlimoncelli marked this conversation as resolved.
Show resolved Hide resolved

Edit `.github/workflows/pr_test.yml`

1. Add the provider to the `PROVIDERS` list.

* Add the name of the provider to the PROVIDERS list.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: Is the idea to create a sublist? Then this content should get an extra indentation.

Suggested change
* Add the name of the provider to the PROVIDERS list.
* Add the name of the provider to the PROVIDERS list.

Screenshot 2024-09-11 at 13 05 47{width=750px}

https://docs.dnscontrol.org/~/revisions/uwTDnrDdAmna8AzbdoCc/developer-info/writing-providers#step-14-update-pr_test.yml

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two lines are a duplicate. i've removed one.

* Please keep this list sorted alphabetically.

The line looks something like:

{% code title=".github/workflows/pr_test.yml" %}
```
tlimoncelli marked this conversation as resolved.
Show resolved Hide resolved
PROVIDERS: "['AZURE_DNS','BIND','CLOUDFLAREAPI','CLOUDNS','DIGITALOCEAN','GANDI_V5','GCLOUD','HEDNS','HEXONET','INWX','NAMEDOTCOM','NS1','POWERDNS','ROUTE53','TRANSIP']"
tlimoncelli marked this conversation as resolved.
Show resolved Hide resolved
```
{% endcode %}

2. Add your providers `_DOMAIN` env variable:

* 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:

{% code title=".github/workflows/pr_test.yml" %}
```
BIND_DOMAIN: ${{ vars.BIND_DOMAIN }}
```
{% endcode %}

3. Add your providers other ENV variables:

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.
tlimoncelli marked this conversation as resolved.
Show resolved Hide resolved

To find this section, search for `PROVIDER SECRET LIST`.

For example, the entry for CLOUDFLAREAPI looks like this:

{% code title=".github/workflows/pr_test.yml" %}
```
CLOUDFLAREAPI_ACCOUNTID: ${{ secrets.CLOUDFLAREAPI_ACCOUNTID }}
CLOUDFLAREAPI_TOKEN: ${{ secrets.CLOUDFLAREAPI_TOKEN }}
```
{% endcode %}

## Step 14: Check your work
tlimoncelli marked this conversation as resolved.
Show resolved Hide resolved

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.
Expand All @@ -317,6 +368,7 @@ These are the things we'll be checking when you submit the PR. Please try to co
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.
7. [Create an issue (feature request)](https://github.com/StackExchange/dnscontrol/issues/new?title=Add%20label%20for%20PROVIDERNAME) with the text "Please create the GitHub label 'provider-PROVIDERNAME'".
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idea: Is it an idea to put this more as a checklist for us? Also in a review I feel the need to be able to tick off a checklist.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might make more sense to include the (create the GitHub label 'provider-PROVIDERNAME') request in the original GitHub pull request.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My philosophy is that if someone's work is going to be judged using a rubric, they should know what the rubric is ahead of time. This helps them get everything right on the first try.

I've moved the label request to the PR. Good idea!


## Step 15: Submit a PR

Expand All @@ -329,5 +381,4 @@ submit a PR if you haven't already.
## Step 16: After the PR is merged

1. Close any related GitHub issues.
2. [Create an issue (feature request)](https://github.com/StackExchange/dnscontrol/issues/new?title=Add%20label%20for%20PROVIDERNAME) with the text "Please create the GitHub label 'provider-PROVIDERNAME'".
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)
Loading