fix!: Change runner labels to list[string] and apply TFLint (#3410)

Change variables `runner_labels` and `runner_extra_labels` to type: `list[string]` to better represent the content Additionally: - [x] Automatic determine runner labels for multi runner based on label matchers - [x] Enable Terraform linting on all modules, including naming conventions - [x] Enable check annotations for linting errors - [x] Fix existing linting errors - [ ] Apply naming conventions within objects (not supported by [TFLint](terraform-linters/tflint#1310)) This PR is based on #3143 creaed by @koendelaat but now via a dedicate branch in this repository. --------- Co-authored-by: Koen de Laat <[email protected]> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
philips-labs · Oct 27, 2023 · 0b398f2 · 0b398f2
1 parent a948da3
commit 0b398f2
Show file tree

Hide file tree

Showing 44 changed files with 213 additions and 91 deletions.
diff --git a/.github/matchers.json b/.github/matchers.json
@@ -0,0 +1,18 @@
+{
+  "problemMatcher": [
+    {
+      "owner": "tflint-compact",
+      "pattern": [
+        {
+          "regexp": "^(.+):(\\d+):(\\d+):\\s(Error|Warning|Notice)\\s-\\s(.+)\\s\\((.+)\\)$",
+          "file": 1,
+          "line": 2,
+          "column": 3,
+          "severity": 4,
+          "message": 5,
+          "code": 6
+        }
+      ]
+    }
+  ]
+}
diff --git a/.github/workflows/terraform.yml b/.github/workflows/terraform.yml
@@ -41,26 +41,38 @@ jobs:
         continue-on-error: true
       - name: validate terraform
         run: terraform validate
+      - if: contains(matrix.terraform, '1.3.')
+        name: Fix for actions/cache on alpine
+        run: apk add --no-cache tar
+        continue-on-error: true
+      - if: contains(matrix.terraform, '1.3.')
+        uses: actions/[email protected]
+        name: Cache TFLint plugin dir
+        with:
+          path: ~/.tflint.d/plugins
+          key: tflint-${{ hashFiles('.tflint.hcl') }}
       - if: contains(matrix.terraform, '1.3.')
         name: Setup TFLint
         uses: terraform-linters/setup-tflint@v4
         with:
           github_token: ${{ secrets.GITHUB_TOKEN }}
       - if: contains(matrix.terraform, '1.3.')
         name: Run TFLint
-        run: tflint -f compact
+        run: |
+          tflint --init -c ${GITHUB_WORKSPACE}/.tflint.hcl
+          tflint -c ${GITHUB_WORKSPACE}/.tflint.hcl --var-file ${GITHUB_WORKSPACE}/.github/lint/tflint.tfvars
 
-  verify_examples:
-    name: Verify examples
+  verify_modules:
+    name: Verify modules
     strategy:
       fail-fast: false
       matrix:
-        terraform: [1.3.0, "latest"]
-        example:
-          ["default", "ubuntu", "prebuilt", "arm64", "ephemeral", "windows"]
+        terraform: [1.3.2, "latest"]
+        module:
+          ["download-lambda", "multi-runner", "runner-binaries-syncer", "runners", "setup-iam-permissions", "ssm", "webhook"]
     defaults:
       run:
-        working-directory: examples/${{ matrix.example }}
+        working-directory: modules/${{ matrix.module }}
     runs-on: ubuntu-latest
     container:
       image: hashicorp/terraform:${{ matrix.terraform }}
@@ -75,19 +87,41 @@ jobs:
         name: check terraform formatting
         run: terraform fmt -recursive -check=true -write=false
         continue-on-error: true
-      - name: validate terraform011
+      - name: validate terraform
         run: terraform validate
+      - if: contains(matrix.terraform, '1.3.')
+        name: Fix for actions/cache on alpine
+        run: apk add --no-cache tar
+        continue-on-error: true
+      - if: contains(matrix.terraform, '1.3.')
+        uses: actions/[email protected]
+        name: Cache TFLint plugin dir
+        with:
+          path: ~/.tflint.d/plugins
+          key: tflint-${{ hashFiles('.tflint.hcl') }}
+      - if: contains(matrix.terraform, '1.3.')
+        name: Setup TFLint
+        uses: terraform-linters/setup-tflint@v3
+        with:
+          github_token: ${{ secrets.GITHUB_TOKEN }}
+      - if: contains(matrix.terraform, '1.3.')
+        name: Run TFLint
+        working-directory: ${{ github.workspace }}
+        run: |
+          tflint --init -c ${GITHUB_WORKSPACE}/.tflint.hcl --chdir modules/${{ matrix.module }}
+          tflint -f compact -c ${GITHUB_WORKSPACE}/.tflint.hcl --var-file ${GITHUB_WORKSPACE}/.github/lint/tflint.tfvars --chdir modules/${{ matrix.module }}
 
-
-  verify_multi_runner_example:
-    name: Verify Multi-Runner examples
+  verify_examples:
+    name: Verify examples
     strategy:
       fail-fast: false
       matrix:
         terraform: [1.3.2, "latest"]
+        example:
+          ["default", "ubuntu", "prebuilt", "arm64", "ephemeral", "windows", "multi-runner"]
     defaults:
       run:
-        working-directory: examples/multi-runner
+        working-directory: examples/${{ matrix.example }}
     runs-on: ubuntu-latest
     container:
       image: hashicorp/terraform:${{ matrix.terraform }}
@@ -104,4 +138,25 @@ jobs:
         continue-on-error: true
       - name: validate terraform
         run: terraform validate
+      - if: contains(matrix.terraform, '1.3.')
+        name: Fix for actions/cache on alpine
+        run: apk add --no-cache tar
+        continue-on-error: true
+      - if: contains(matrix.terraform, '1.3.')
+        uses: actions/[email protected]
+        name: Cache TFLint plugin dir
+        with:
+          path: ~/.tflint.d/plugins
+          key: tflint-${{ hashFiles('.tflint.hcl') }}
+      - if: contains(matrix.terraform, '1.3.')
+        name: Setup TFLint
+        uses: terraform-linters/setup-tflint@v3
+        with:
+          github_token: ${{ secrets.GITHUB_TOKEN }}
+      - if: contains(matrix.terraform, '1.3.')
+        name: Run TFLint
+        working-directory: ${{ github.workspace }}
+        run: |
+          tflint --init -c ${GITHUB_WORKSPACE}/.tflint.hcl --chdir modules/${{ matrix.module }}
+          tflint -f compact -c ${GITHUB_WORKSPACE}/.tflint.hcl --var-file ${GITHUB_WORKSPACE}/.github/lint/tflint.tfvars --chdir examples/${{ matrix.example }}
 
diff --git a/.gitignore b/.gitignore
@@ -22,4 +22,3 @@ secrets.auto.tfvars
 **/coverage/*
 
 node_modules/
-
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -1,10 +1,12 @@
 repos:
   - repo: https://github.com/antonbabenko/pre-commit-terraform
-    rev: v1.77.1
+    rev: v1.83.5
     hooks:
       - id: terraform_fmt
       - id: terraform_tflint
+        args:
+          - --args=--config=__GIT_WORKING_DIR__/.tflint.hcl --var-file __GIT_WORKING_DIR__/.github/lint/tflint.tfvars
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.4.0
+    rev: v4.5.0
     hooks:
       - id: check-merge-conflict
diff --git a/.tflint.hcl b/.tflint.hcl
@@ -2,6 +2,23 @@ config {
   format = "compact"
   module = true
 
-  varfile = [".github/lint/tflint.tfvars"]
 
 }
+
+plugin "aws" {
+    enabled = true
+    version = "0.23.0"
+    source  = "github.com/terraform-linters/tflint-ruleset-aws"
+}
+
+rule "terraform_comment_syntax" {
+    enabled = true
+}
+
+rule "terraform_naming_convention" {
+  enabled = true
+}
+
+rule "terraform_documented_variables" {
+  enabled = true
+}
diff --git a/README.md b/README.md
@@ -502,7 +502,7 @@ We welcome any improvement to the standard module to make the default as secure
 | <a name="input_delay_webhook_event"></a> [delay\_webhook\_event](#input\_delay\_webhook\_event) | The number of seconds the event accepted by the webhook is invisible on the queue before the scale up lambda will receive the event. | `number` | `30` | no |
 | <a name="input_disable_runner_autoupdate"></a> [disable\_runner\_autoupdate](#input\_disable\_runner\_autoupdate) | Disable the auto update of the github runner agent. Be aware there is a grace period of 30 days, see also the [GitHub article](https://github.blog/changelog/2022-02-01-github-actions-self-hosted-runners-can-now-disable-automatic-updates/) | `bool` | `false` | no |
 | <a name="input_enable_cloudwatch_agent"></a> [enable\_cloudwatch\_agent](#input\_enable\_cloudwatch\_agent) | Enables the cloudwatch agent on the ec2 runner instances. The runner uses a default config that can be overridden via `cloudwatch_config`. | `bool` | `true` | no |
-| <a name="input_enable_enable_fifo_build_queue"></a> [enable\_enable\_fifo\_build\_queue](#input\_enable\_enable\_fifo\_build\_queue) | DEPCRECATED: Replaced by `enable_fifo_build_queue` / `fifo_build_queue`. | `string` | `null` | no |
+| <a name="input_enable_enable_fifo_build_queue"></a> [enable\_enable\_fifo\_build\_queue](#input\_enable\_enable\_fifo\_build\_queue) | DEPRECATED: Replaced by `enable_fifo_build_queue` / `fifo_build_queue`. | `string` | `null` | no |
 | <a name="input_enable_ephemeral_runners"></a> [enable\_ephemeral\_runners](#input\_enable\_ephemeral\_runners) | Enable ephemeral runners, runners will only be used once. | `bool` | `false` | no |
 | <a name="input_enable_event_rule_binaries_syncer"></a> [enable\_event\_rule\_binaries\_syncer](#input\_enable\_event\_rule\_binaries\_syncer) | Option to disable EventBridge Lambda trigger for the binary syncer, useful to stop automatic updates of binary distribution. | `bool` | `true` | no |
 | <a name="input_enable_fifo_build_queue"></a> [enable\_fifo\_build\_queue](#input\_enable\_fifo\_build\_queue) | Enable a FIFO queue to keep the order of events received by the webhook. Recommended for repo level runners. | `bool` | `false` | no |
@@ -517,9 +517,9 @@ We welcome any improvement to the standard module to make the default as secure
 | <a name="input_enable_user_data_debug_logging_runner"></a> [enable\_user\_data\_debug\_logging\_runner](#input\_enable\_user\_data\_debug\_logging\_runner) | Option to enable debug logging for user-data, this logs all secrets as well. | `bool` | `false` | no |
 | <a name="input_enable_userdata"></a> [enable\_userdata](#input\_enable\_userdata) | Should the userdata script be enabled for the runner. Set this to false if you are using your own prebuilt AMI. | `bool` | `true` | no |
 | <a name="input_enable_workflow_job_events_queue"></a> [enable\_workflow\_job\_events\_queue](#input\_enable\_workflow\_job\_events\_queue) | Enabling this experimental feature will create a secondory sqs queue to which a copy of the workflow\_job event will be delivered. | `bool` | `false` | no |
-| <a name="input_enabled_userdata"></a> [enabled\_userdata](#input\_enabled\_userdata) | DEPCRECATED: Replaced by `enable_userdata`. | `string` | `null` | no |
+| <a name="input_enabled_userdata"></a> [enabled\_userdata](#input\_enabled\_userdata) | DEPRECATED: Replaced by `enable_userdata`. | `string` | `null` | no |
 | <a name="input_environment"></a> [environment](#input\_environment) | DEPRECATED, no longer used. See `prefix` | `string` | `null` | no |
-| <a name="input_fifo_build_queue"></a> [fifo\_build\_queue](#input\_fifo\_build\_queue) | DEPCRECATED: Replaced by `enable_fifo_build_queue`. | `string` | `null` | no |
+| <a name="input_fifo_build_queue"></a> [fifo\_build\_queue](#input\_fifo\_build\_queue) | DEPRECATED: Replaced by `enable_fifo_build_queue`. | `string` | `null` | no |
 | <a name="input_ghes_ssl_verify"></a> [ghes\_ssl\_verify](#input\_ghes\_ssl\_verify) | GitHub Enterprise SSL verification. Set to 'false' when custom certificate (chains) is used for GitHub Enterprise Server (insecure). | `bool` | `true` | no |
 | <a name="input_ghes_url"></a> [ghes\_url](#input\_ghes\_url) | GitHub Enterprise Server URL. Example: https://github.internal.co - DO NOT SET IF USING PUBLIC GITHUB | `string` | `null` | no |
 | <a name="input_github_app"></a> [github\_app](#input\_github\_app) | GitHub app parameters, see your github app. Ensure the key is the base64-encoded `.pem` file (the output of `base64 app.private-key.pem`, not the content of `private-key.pem`). | <pre>object({<br>    key_base64     = string<br>    id             = string<br>    webhook_secret = string<br>  })</pre> | n/a | yes |
@@ -568,8 +568,8 @@ We welcome any improvement to the standard module to make the default as secure
 | <a name="input_runner_credit_specification"></a> [runner\_credit\_specification](#input\_runner\_credit\_specification) | The credit option for CPU usage of a T instance. Can be unset, "standard" or "unlimited". | `string` | `null` | no |
 | <a name="input_runner_ec2_tags"></a> [runner\_ec2\_tags](#input\_runner\_ec2\_tags) | Map of tags that will be added to the launch template instance tag specifications. | `map(string)` | `{}` | no |
 | <a name="input_runner_egress_rules"></a> [runner\_egress\_rules](#input\_runner\_egress\_rules) | List of egress rules for the GitHub runner instances. | <pre>list(object({<br>    cidr_blocks      = list(string)<br>    ipv6_cidr_blocks = list(string)<br>    prefix_list_ids  = list(string)<br>    from_port        = number<br>    protocol         = string<br>    security_groups  = list(string)<br>    self             = bool<br>    to_port          = number<br>    description      = string<br>  }))</pre> | <pre>[<br>  {<br>    "cidr_blocks": [<br>      "0.0.0.0/0"<br>    ],<br>    "description": null,<br>    "from_port": 0,<br>    "ipv6_cidr_blocks": [<br>      "::/0"<br>    ],<br>    "prefix_list_ids": null,<br>    "protocol": "-1",<br>    "security_groups": null,<br>    "self": null,<br>    "to_port": 0<br>  }<br>]</pre> | no |
-| <a name="input_runner_enable_workflow_job_labels_check_all"></a> [runner\_enable\_workflow\_job\_labels\_check\_all](#input\_runner\_enable\_workflow\_job\_labels\_check\_all) | DEPCRECATED: Replaced by `enable_runner_workflow_job_labels_check_all`. | `string` | `null` | no |
-| <a name="input_runner_extra_labels"></a> [runner\_extra\_labels](#input\_runner\_extra\_labels) | Extra (custom) labels for the runners (GitHub). Separate each label by a comma. Labels checks on the webhook can be enforced by setting `enable_workflow_job_labels_check`. GitHub read-only labels should not be provided. | `string` | `""` | no |
+| <a name="input_runner_enable_workflow_job_labels_check_all"></a> [runner\_enable\_workflow\_job\_labels\_check\_all](#input\_runner\_enable\_workflow\_job\_labels\_check\_all) | DEPRECATED: Replaced by `enable_runner_workflow_job_labels_check_all`. | `string` | `null` | no |
+| <a name="input_runner_extra_labels"></a> [runner\_extra\_labels](#input\_runner\_extra\_labels) | Extra (custom) labels for the runners (GitHub). Labels checks on the webhook can be enforced by setting `enable_workflow_job_labels_check`. GitHub read-only labels should not be provided. | `list(string)` | `[]` | no |
 | <a name="input_runner_group_name"></a> [runner\_group\_name](#input\_runner\_group\_name) | Name of the runner group. | `string` | `"Default"` | no |
 | <a name="input_runner_iam_role_managed_policy_arns"></a> [runner\_iam\_role\_managed\_policy\_arns](#input\_runner\_iam\_role\_managed\_policy\_arns) | Attach AWS or customer-managed IAM policies (by ARN) to the runner IAM role | `list(string)` | `[]` | no |
 | <a name="input_runner_log_files"></a> [runner\_log\_files](#input\_runner\_log\_files) | (optional) Replaces the module default cloudwatch log config. See https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/CloudWatch-Agent-Configuration-File-Details.html for details. | <pre>list(object({<br>    log_group_name   = string<br>    prefix_log_group = bool<br>    file_path        = string<br>    log_stream_name  = string<br>  }))</pre> | `null` | no |
@@ -594,7 +594,7 @@ We welcome any improvement to the standard module to make the default as secure
 | <a name="input_userdata_pre_install"></a> [userdata\_pre\_install](#input\_userdata\_pre\_install) | Script to be ran before the GitHub Actions runner is installed on the EC2 instances | `string` | `""` | no |
 | <a name="input_userdata_template"></a> [userdata\_template](#input\_userdata\_template) | Alternative user-data template, replacing the default template. By providing your own user\_data you have to take care of installing all required software, including the action runner. Variables userdata\_pre/post\_install are ignored. | `string` | `null` | no |
 | <a name="input_vpc_id"></a> [vpc\_id](#input\_vpc\_id) | The VPC for security groups of the action runners. | `string` | n/a | yes |
-| <a name="input_webhook_lambda_apigateway_access_log_settings"></a> [webhook\_lambda\_apigateway\_access\_log\_settings](#input\_webhook\_lambda\_apigateway\_access\_log\_settings) | n/a | <pre>object({<br>    destination_arn = string<br>    format          = string<br>  })</pre> | `null` | no |
+| <a name="input_webhook_lambda_apigateway_access_log_settings"></a> [webhook\_lambda\_apigateway\_access\_log\_settings](#input\_webhook\_lambda\_apigateway\_access\_log\_settings) | Access log settings for webhook API gateway. | <pre>object({<br>    destination_arn = string<br>    format          = string<br>  })</pre> | `null` | no |
 | <a name="input_webhook_lambda_s3_key"></a> [webhook\_lambda\_s3\_key](#input\_webhook\_lambda\_s3\_key) | S3 key for webhook lambda function. Required if using S3 bucket to specify lambdas. | `string` | `null` | no |
 | <a name="input_webhook_lambda_s3_object_version"></a> [webhook\_lambda\_s3\_object\_version](#input\_webhook\_lambda\_s3\_object\_version) | S3 object version for webhook lambda function. Useful if S3 versioning is enabled on source bucket. | `string` | `null` | no |
 | <a name="input_webhook_lambda_timeout"></a> [webhook\_lambda\_timeout](#input\_webhook\_lambda\_timeout) | Time out of the webhook lambda in seconds. | `number` | `10` | no |

diff --git a/examples/arm64/main.tf b/examples/arm64/main.tf
@@ -45,7 +45,7 @@ module "runners" {
 
   enable_organization_runners = false
   # Runners will automatically get the "arm64" label
-  runner_extra_labels = "default,example"
+  runner_extra_labels = ["default", "example"]
 
   # enable access to the runners via SSM
   enable_ssm_on_runners = true
@@ -87,7 +87,7 @@ module "runners" {
   scale_down_schedule_expression = "cron(* * * * ? *)"
 }
 
-module "webhook-github-app" {
+module "webhook_github_app" {
   source = "../../modules/webhook-github-app"
 
   github_app = {

diff --git a/examples/default/README.md b/examples/default/README.md
@@ -62,7 +62,7 @@ terraform output webhook_secret
 
 | Name | Description | Type | Default | Required |
 |------|-------------|------|---------|:--------:|
-| <a name="input_environment"></a> [environment](#input\_environment) | n/a | `string` | `null` | no |
+| <a name="input_environment"></a> [environment](#input\_environment) | Environment name, used as prefix | `string` | `null` | no |
 | <a name="input_github_app"></a> [github\_app](#input\_github\_app) | GitHub for API usages. | <pre>object({<br>    id         = string<br>    key_base64 = string<br>  })</pre> | n/a | yes |
 
 ## Outputs

diff --git a/examples/default/main.tf b/examples/default/main.tf
@@ -48,7 +48,7 @@ module "runners" {
   # runners_lambda_zip                = "../lambdas-download/runners.zip"
 
   enable_organization_runners = true
-  runner_extra_labels         = "default,example"
+  runner_extra_labels         = ["default", "example"]
 
   # enable access to the runners via SSM
   enable_ssm_on_runners = true
@@ -98,7 +98,7 @@ module "runners" {
   # log_level = "debug"
 }
 
-module "webhook-github-app" {
+module "webhook_github_app" {
   source     = "../../modules/webhook-github-app"
   depends_on = [module.runners]
 

diff --git a/examples/default/variables.tf b/examples/default/variables.tf
@@ -8,6 +8,8 @@ variable "github_app" {
 }
 
 variable "environment" {
+  description = "Environment name, used as prefix"
+
   type    = string
   default = null
 }
diff --git a/examples/ephemeral/main.tf b/examples/ephemeral/main.tf
@@ -38,7 +38,7 @@ module "runners" {
   runners_lambda_zip                = "../../lambda_output/runners.zip"
 
   enable_organization_runners = true
-  runner_extra_labels         = "default,example"
+  runner_extra_labels         = ["default", "example"]
 
   # enable access to the runners via SSM
   enable_ssm_on_runners = true
@@ -86,7 +86,7 @@ module "runners" {
   # }
 }
 
-module "webhook-github-app" {
+module "webhook_github_app" {
   source     = "../../modules/webhook-github-app"
   depends_on = [module.runners]
 

diff --git a/examples/multi-runner/README.md b/examples/multi-runner/README.md
@@ -66,7 +66,7 @@ terraform output webhook_secret
 | Name | Source | Version |
 |------|--------|---------|
 | <a name="module_base"></a> [base](#module\_base) | ../base | n/a |
-| <a name="module_multi-runner"></a> [multi-runner](#module\_multi-runner) | ../../modules/multi-runner | n/a |
+| <a name="module_multi_runner"></a> [multi\_runner](#module\_multi\_runner) | ../../modules/multi-runner | n/a |
 | <a name="module_webhook-github-app"></a> [webhook-github-app](#module\_webhook-github-app) | ../../modules/webhook-github-app | n/a |
 
 ## Resources
@@ -79,7 +79,7 @@ terraform output webhook_secret
 
 | Name | Description | Type | Default | Required |
 |------|-------------|------|---------|:--------:|
-| <a name="input_environment"></a> [environment](#input\_environment) | n/a | `string` | `null` | no |
+| <a name="input_environment"></a> [environment](#input\_environment) | Environment name, used as prefix | `string` | `null` | no |
 | <a name="input_github_app"></a> [github\_app](#input\_github\_app) | GitHub for API usages. | <pre>object({<br>    id         = string<br>    key_base64 = string<br>  })</pre> | n/a | yes |
 
 ## Outputs