From f145d2a07da722f988ba6845737c3f2c1d55e476 Mon Sep 17 00:00:00 2001 From: Nuru Date: Mon, 21 Dec 2020 17:51:49 -0800 Subject: [PATCH] Add var.attributes to end of context.attributes, not vice versa (#114) --- .github/auto-release.yml | 47 ++++++++----- .github/workflows/auto-format.yml | 86 +++++++++++++++++++++++ .github/workflows/auto-release.yml | 2 +- .github/workflows/chatops.yml | 4 +- .github/workflows/validate-codeowners.yml | 25 +++++++ README.md | 21 ++++-- README.yaml | 10 ++- examples/complete/label7.tf | 44 ++++++++++++ exports/context.tf | 3 +- main.tf | 12 ++-- test/src/examples_complete_test.go | 3 + 11 files changed, 221 insertions(+), 36 deletions(-) create mode 100644 .github/workflows/auto-format.yml create mode 100644 .github/workflows/validate-codeowners.yml create mode 100644 examples/complete/label7.tf diff --git a/.github/auto-release.yml b/.github/auto-release.yml index 2836185..c78a4d8 100644 --- a/.github/auto-release.yml +++ b/.github/auto-release.yml @@ -4,30 +4,35 @@ version-template: '$MAJOR.$MINOR.$PATCH' version-resolver: major: labels: - - 'major' + - 'major' minor: labels: - - 'minor' - - 'enhancement' + - 'minor' + - 'enhancement' patch: labels: - - 'patch' - - 'fix' - - 'bugfix' - - 'bug' - - 'hotfix' + - 'auto-update' + - 'patch' + - 'fix' + - 'bugfix' + - 'bug' + - 'hotfix' default: 'minor' categories: - - title: '🚀 Enhancements' - labels: - - 'enhancement' - - title: '🐛 Bug Fixes' - labels: - - 'fix' - - 'bugfix' - - 'bug' - - 'hotfix' +- title: '🚀 Enhancements' + labels: + - 'enhancement' + - 'patch' +- title: '🐛 Bug Fixes' + labels: + - 'fix' + - 'bugfix' + - 'bug' + - 'hotfix' +- title: '🤖 Automatic Updates' + labels: + - 'auto-update' change-template: |
@@ -38,3 +43,11 @@ change-template: | template: | $CHANGES + +replacers: +# Remove irrelevant information from Renovate bot +- search: '/---\s+^#.*Renovate configuration(?:.|\n)*?This PR has been generated .*/gm' + replace: '' +# Remove Renovate bot banner image +- search: '/\[!\[[^\]]*Renovate\][^\]]*\](\([^)]*\))?\s*\n+/gm' + replace: '' diff --git a/.github/workflows/auto-format.yml b/.github/workflows/auto-format.yml new file mode 100644 index 0000000..990abed --- /dev/null +++ b/.github/workflows/auto-format.yml @@ -0,0 +1,86 @@ +name: Auto Format +on: + pull_request_target: + types: [opened, synchronize] + +jobs: + auto-format: + runs-on: ubuntu-latest + container: cloudposse/build-harness:slim-latest + steps: + # Checkout the pull request branch + # "An action in a workflow run can’t trigger a new workflow run. For example, if an action pushes code using + # the repository’s GITHUB_TOKEN, a new workflow will not run even when the repository contains + # a workflow configured to run when push events occur." + # However, using a personal access token will cause events to be triggered. + # We need that to ensure a status gets posted after the auto-format commit. + # We also want to trigger tests if the auto-format made no changes. + - uses: actions/checkout@v2 + if: github.event.pull_request.state == 'open' + name: Privileged Checkout + with: + token: ${{ secrets.PUBLIC_REPO_ACCESS_TOKEN }} + repository: ${{ github.event.pull_request.head.repo.full_name }} + # Check out the PR commit, not the merge commit + # Use `ref` instead of `sha` to enable pushing back to `ref` + ref: ${{ github.event.pull_request.head.ref }} + + # Do all the formatting stuff + - name: Auto Format + if: github.event.pull_request.state == 'open' + shell: bash + run: make BUILD_HARNESS_PATH=/build-harness PACKAGES_PREFER_HOST=true -f /build-harness/templates/Makefile.build-harness pr/auto-format/host + + # Commit changes (if any) to the PR branch + - name: Commit changes to the PR branch + if: github.event.pull_request.state == 'open' + shell: bash + id: commit + env: + SENDER: ${{ github.event.sender.login }} + run: | + set -x + output=$(git diff --name-only) + + if [ -n "$output" ]; then + echo "Changes detected. Pushing to the PR branch" + git config --global user.name 'cloudpossebot' + git config --global user.email '11232728+cloudpossebot@users.noreply.github.com' + git add -A + git commit -m "Auto Format" + # Prevent looping by not pushing changes in response to changes from cloudpossebot + [[ $SENDER == "cloudpossebot" ]] || git push + # Set status to fail, because the push should trigger another status check, + # and we use success to indicate the checks are finished. + printf "::set-output name=%s::%s\n" "changed" "true" + exit 1 + else + printf "::set-output name=%s::%s\n" "changed" "false" + echo "No changes detected" + fi + + - name: Auto Test + uses: cloudposse/actions/github/repository-dispatch@0.22.0 + # match users by ID because logins (user names) are inconsistent, + # for example in the REST API Renovate Bot is `renovate[bot]` but + # in GraphQL it is just `renovate`, plus there is a non-bot + # user `renovate` with ID 1832810. + # Mergify bot: 37929162 + # Renovate bot: 29139614 + # Cloudpossebot: 11232728 + # Need to use space separators to prevent "21" from matching "112144" + if: > + contains(' 37929162 29139614 11232728 ', format(' {0} ', github.event.pull_request.user.id)) + && steps.commit.outputs.changed == 'false' && github.event.pull_request.state == 'open' + with: + token: ${{ secrets.PUBLIC_REPO_ACCESS_TOKEN }} + repository: cloudposse/actions + event-type: test-command + client-payload: |- + { "slash_command":{"args": {"unnamed": {"all": "all", "arg1": "all"}}}, + "pull_request": ${{ toJSON(github.event.pull_request) }}, + "github":{"payload":{"repository": ${{ toJSON(github.event.repository) }}, + "comment": {"id": ""} + } + } + } diff --git a/.github/workflows/auto-release.yml b/.github/workflows/auto-release.yml index e69e53b..3e2b202 100644 --- a/.github/workflows/auto-release.yml +++ b/.github/workflows/auto-release.yml @@ -6,7 +6,7 @@ on: - master jobs: - semver: + publish: runs-on: ubuntu-latest steps: # Drafts your next Release notes as Pull Requests are merged into "master" diff --git a/.github/workflows/chatops.yml b/.github/workflows/chatops.yml index 0d94310..4ddc067 100644 --- a/.github/workflows/chatops.yml +++ b/.github/workflows/chatops.yml @@ -9,7 +9,7 @@ jobs: steps: - uses: actions/checkout@v2 - name: "Handle common commands" - uses: cloudposse/actions/github/slash-command-dispatch@0.16.0 + uses: cloudposse/actions/github/slash-command-dispatch@0.22.0 with: token: ${{ secrets.PUBLIC_REPO_ACCESS_TOKEN }} reaction-token: ${{ secrets.GITHUB_TOKEN }} @@ -24,7 +24,7 @@ jobs: - name: "Checkout commit" uses: actions/checkout@v2 - name: "Run tests" - uses: cloudposse/actions/github/slash-command-dispatch@0.16.0 + uses: cloudposse/actions/github/slash-command-dispatch@0.22.0 with: token: ${{ secrets.PUBLIC_REPO_ACCESS_TOKEN }} reaction-token: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/validate-codeowners.yml b/.github/workflows/validate-codeowners.yml new file mode 100644 index 0000000..386eb28 --- /dev/null +++ b/.github/workflows/validate-codeowners.yml @@ -0,0 +1,25 @@ +name: Validate Codeowners +on: + pull_request: + +jobs: + validate-codeowners: + runs-on: ubuntu-latest + steps: + - name: "Checkout source code at current commit" + uses: actions/checkout@v2 + - uses: mszostok/codeowners-validator@v0.5.0 + if: github.event.pull_request.head.repo.full_name == github.repository + name: "Full check of CODEOWNERS" + with: + # For now, remove "files" check to allow CODEOWNERS to specify non-existent + # files so we can use the same CODEOWNERS file for Terraform and non-Terraform repos + # checks: "files,syntax,owners,duppatterns" + checks: "syntax,owners,duppatterns" + # GitHub access token is required only if the `owners` check is enabled + github_access_token: "${{ secrets.PUBLIC_REPO_ACCESS_TOKEN }}" + - uses: mszostok/codeowners-validator@v0.5.0 + if: github.event.pull_request.head.repo.full_name != github.repository + name: "Syntax check of CODEOWNERS" + with: + checks: "syntax,duppatterns" diff --git a/README.md b/README.md index 33a4dbf..fea3fd9 100644 --- a/README.md +++ b/README.md @@ -29,9 +29,13 @@ Terraform module designed to generate consistent names and tags for resources. Use `terraform-null-label` to implement a strict naming convention. -A label follows the following convention: `{namespace}-{environment}-{stage}-{name}-{attributes}`. The delimiter (e.g. `-`) is configurable. -The label items are all optional. So if you prefer the term `stage` to `environment` you can exclude environment and the label `id` will look like `{namespace}-{stage}-{name}-{attributes}`. -If attributes are excluded but `stage` and `environment` are included, `id` will look like `{namespace}-{environment}-{stage}-{name}` +This module generates names using the following convention by default: `{namespace}-{environment}-{stage}-{name}-{attributes}`. +However, it is highly configurable. The delimiter (e.g. `-`) is configurable. Each label item is optional (although you must provide at least one). +So if you prefer the term `stage` to `environment` +you can exclude environment and the label `id` will look like `{namespace}-{stage}-{name}-{attributes}`. +If attributes are excluded but `stage` and `environment` are included, `id` will look like `{namespace}-{environment}-{stage}-{name}`. +If you want the attributes in a different order, you can specify that, too, with the `label_order` list. +You can set a maximum length for the name, and the module will create a unique name that fits within that length. It's recommended to use one `terraform-null-label` module for every unique resource of a given resource type. For example, if you have 10 instances, there should be 10 different labels. @@ -78,8 +82,15 @@ We literally have [*hundreds of terraform modules*][terraform_modules] that are ## Usage -**IMPORTANT:** The `master` branch is used in `source` just as an example. In your code, do not pin to `master` because there may be breaking changes between releases. -Instead pin to the release tag (e.g. `?ref=tags/x.y.z`) of one of our [latest releases](https://github.com/cloudposse/terraform-null-label/releases). +**IMPORTANT:** We do not pin modules to versions in our examples because of the +difficulty of keeping the versions in the documentation in sync with the latest released versions. +We highly recommend that in your code you pin the version to the exact version you are +using so that your infrastructure remains stable, and update versions in a +systematic way so that they do not catch you by surprise. + +Also, because of a bug in the Terraform registry ([hashicorp/terraform#21417](https://github.com/hashicorp/terraform/issues/21417)), +the registry shows many of our inputs as required when in fact they are optional. +The table below correctly indicates which inputs are required. ### Defaults diff --git a/README.yaml b/README.yaml index b90bd86..940d6b1 100644 --- a/README.yaml +++ b/README.yaml @@ -26,9 +26,13 @@ related: description: |- Terraform module designed to generate consistent names and tags for resources. Use `terraform-null-label` to implement a strict naming convention. - A label follows the following convention: `{namespace}-{environment}-{stage}-{name}-{attributes}`. The delimiter (e.g. `-`) is configurable. - The label items are all optional. So if you prefer the term `stage` to `environment` you can exclude environment and the label `id` will look like `{namespace}-{stage}-{name}-{attributes}`. - If attributes are excluded but `stage` and `environment` are included, `id` will look like `{namespace}-{environment}-{stage}-{name}` + This module generates names using the following convention by default: `{namespace}-{environment}-{stage}-{name}-{attributes}`. + However, it is highly configurable. The delimiter (e.g. `-`) is configurable. Each label item is optional (although you must provide at least one). + So if you prefer the term `stage` to `environment` + you can exclude environment and the label `id` will look like `{namespace}-{stage}-{name}-{attributes}`. + If attributes are excluded but `stage` and `environment` are included, `id` will look like `{namespace}-{environment}-{stage}-{name}`. + If you want the attributes in a different order, you can specify that, too, with the `label_order` list. + You can set a maximum length for the name, and the module will create a unique name that fits within that length. It's recommended to use one `terraform-null-label` module for every unique resource of a given resource type. For example, if you have 10 instances, there should be 10 different labels. diff --git a/examples/complete/label7.tf b/examples/complete/label7.tf new file mode 100644 index 0000000..bb365d4 --- /dev/null +++ b/examples/complete/label7.tf @@ -0,0 +1,44 @@ +module "label7a" { + source = "../../" + enabled = true + namespace = "eg" + environment = "demo" + name = "blue" + attributes = ["cluster"] + delimiter = "-" + + tags = { + } +} + +module "label7" { + source = "../../" + + attributes = ["nodegroup"] + + context = module.label7a.context +} + + +output "label7" { + value = { + id = module.label7.id + name = module.label7.name + namespace = module.label7.namespace + stage = module.label7.stage + attributes = module.label7.attributes + delimiter = module.label7.delimiter + } +} + +output "label7_id" { + value = module.label7.id +} + +output "label7_attributes" { + value = module.label7.attributes +} + +output "label7_context" { + value = module.label7.context +} diff --git a/exports/context.tf b/exports/context.tf index e5734b7..f5f2797 100644 --- a/exports/context.tf +++ b/exports/context.tf @@ -18,10 +18,9 @@ # will be null, and `module.this.delimiter` will be `-` (hyphen). # - module "this" { source = "cloudposse/label/null" - version = "0.22.0" // requires Terraform >= 0.12.26 + version = "0.22.1" // requires Terraform >= 0.12.26 enabled = var.enabled namespace = var.namespace diff --git a/main.tf b/main.tf index 064c8b1..c9c73b8 100644 --- a/main.tf +++ b/main.tf @@ -7,7 +7,6 @@ locals { replacement = "" # The `sentinel` should match the `regex_replace_chars`, so it will be replaced with the `replacement` value sentinel = "\t" - attributes = [] id_length_limit = 0 id_hash_length = 5 } @@ -17,7 +16,8 @@ locals { sentinel = local.defaults.sentinel id_hash_length = local.defaults.id_hash_length - # The values provided by variables supersede the values inherited from the context object + # The values provided by variables supersede the values inherited from the context object, + # except for tags and attributes which are merged. input = { # It would be nice to use coalesce here, but we cannot, because it # is an error for all the arguments to coalesce to be empty. @@ -27,8 +27,9 @@ locals { stage = var.stage == null ? var.context.stage : var.stage name = var.name == null ? var.context.name : var.name delimiter = var.delimiter == null ? var.context.delimiter : var.delimiter - attributes = compact(distinct(concat(var.attributes, var.context.attributes))) - tags = merge(var.context.tags, var.tags) + # modules tack on attributes (passed by var) to the end of the list (passed by context) + attributes = compact(distinct(concat(coalesce(var.context.attributes, []), coalesce(var.attributes, [])))) + tags = merge(var.context.tags, var.tags) additional_tag_map = merge(var.context.additional_tag_map, var.additional_tag_map) label_order = var.label_order == null ? var.context.label_order : var.label_order @@ -51,8 +52,7 @@ locals { additional_tag_map = merge(var.context.additional_tag_map, var.additional_tag_map) - # Merge attributes - attributes = compact(distinct(concat(local.input.attributes, local.defaults.attributes))) + attributes = local.input.attributes tags = merge(local.generated_tags, local.input.tags) diff --git a/test/src/examples_complete_test.go b/test/src/examples_complete_test.go index 99b46a3..5fc9341 100644 --- a/test/src/examples_complete_test.go +++ b/test/src/examples_complete_test.go @@ -174,4 +174,7 @@ func TestExamplesComplete(t *testing.T) { assert.Equal(t, label6t["id_length_limit"], fmt.Sprintf("%d", len(label6t["id"])), "Truncated ID length should equal length limit") + label7 := terraform.OutputMap(t, terraformOptions, "label7") + assert.Equal(t, "eg-demo-blue-cluster-nodegroup", label7["id"], "var.attributes should be appended after context.attributes") + }