Concern around Terraform code formatting and variable validation

[taliesins]

We would like to make use of the modules inside this repository but on the surface I am concered by the code formatting and non standard validation of inputs.

This repo probably needs formatting, linting, document generation and validation applied to it. Then a bit of general code cleanup.

Potential IaC pipeline

Could make use something simple like pre-commit to format, lint, generate documentation and validation. You can run it client side or server side with pre-commit run --all-files.

Example .pre-commit-config.yaml file:

fail_fast: true
repos:
  - repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.4.0
    hooks:
      # Git style
      - id: check-added-large-files
      - id: check-merge-conflict
      - id: check-vcs-permalinks
      - id: forbid-new-submodules
      - id: no-commit-to-branch

      # Common errors
      - id: end-of-file-fixer
      - id: trailing-whitespace
        args: [--markdown-linebreak-ext=md]
      - id: check-merge-conflict

      # Cross platform
      - id: check-case-conflict
      - id: mixed-line-ending
        args: [--fix=lf]

      # Security
      - id: detect-aws-credentials
        args: ["--allow-missing-credentials"]

      - id: detect-private-key

  - repo: https://github.com/adrienverge/yamllint.git
    rev: v1.29.0
    hooks:
      - id: yamllint

  - repo: https://github.com/jumanjihouse/pre-commit-hooks
    rev: 3.0.0
    hooks:
      - id: shfmt
        args: ["-l", "-i", "2", "-ci", "-sr", "-w"]
      - id: shellcheck

  # Dockerfile linter
  - repo: https://github.com/hadolint/hadolint
    rev: v2.12.1-beta
    hooks:
      - id: hadolint
        args: [
            "--ignore",
            "SC2086" # Double quote to prevent globbing and word splitting
          ]

  - repo: https://github.com/antonbabenko/pre-commit-terraform
    rev: "v1.77.0" # Get the latest from: https://github.com/antonbabenko/pre-commit-terraform/releases
    hooks:
      - id: terraform_fmt
      - id: terragrunt_fmt
      - id: terraform_docs
        args:
          - --hook-config=.terraform-doc.yml # Valid UNIX path. I.e. ../TFDOC.md or docs/README.md etc.
          - --hook-config=--add-to-existing-file=false # Boolean. true or false
          - --hook-config=--create-file-if-not-exist=true # Boolean. true or false
          - --hook-config=--output-file=README.md"
          - --hook-config=--recursive
      - id: terraform_tflint
      - id: terraform_checkov
        args:
          - --args=--quiet
          - --args=--skip-check CKV_SECRET_6
          - --args=--skip-path .terragrunt-cache
          - --args=--skip-path .terraform
          - --args=--skip-path .git
          - --args=--skip-path .profile
          - --args=--skip-path test
      - id: terraform_tfsec
        args:
          - >
            --args=--exclude-downloaded-modules --no-module-downloads
            --concise-output
            -e aws-ecs-enable-container-insight

.tflint.hcl

plugin "aws" {
  enabled = true
  version = "0.30.0"
  source  = "github.com/terraform-linters/tflint-ruleset-aws"
}

.yamllint

---
# Based on ansible-lint config
extends: default

ignore: |
  **/templates/*.yaml

rules:
  braces:
    max-spaces-inside: 1
    level: error
  brackets:
    max-spaces-inside: 1
    level: error
  colons:
    max-spaces-after: -1
    level: error
  commas:
    max-spaces-after: -1
    level: error
  comments: disable
  comments-indentation: disable
  document-start: disable
  empty-lines:
    max: 3
    level: error
  hyphens:
    level: error
  indentation: disable
  key-duplicates: enable
  line-length: disable
  new-line-at-end-of-file: disable
  new-lines:
    type: unix
  trailing-spaces: disable
  truthy: disable

Variable Validation

Variable validation has been part of Terraform since 0.13 - https://developer.hashicorp.com/terraform/language/values/variables#custom-validation-rules

There seems to be a lot of examples like the following:

We have regex validation occuring here: https://github.com/CheckPointSW/CloudGuardIaaS/blob/master/terraform/aws/autoscale-gwlb/locals.tf#L23

Which should probably be applied here: https://github.com/CheckPointSW/CloudGuardIaaS/blob/master/terraform/aws/autoscale-gwlb/variables.tf#L89

Code formatting

These types of issues are throughout the code base.

Poor formatting

Random indentation:
https://github.com/CheckPointSW/CloudGuardIaaS/blob/master/terraform/aws/cme-iam-role-gwlb/main.tf#L94

Should probably consistenly add spaces between resources

With space
https://github.com/CheckPointSW/CloudGuardIaaS/blob/master/terraform/aws/autoscale-gwlb/main.tf#L134

With no space
https://github.com/CheckPointSW/CloudGuardIaaS/blob/master/terraform/aws/autoscale-gwlb/main.tf#L141

[chkp-romanka]

Hello @taliesins
Very appreciate your valuable and very detailed feedback!
We are working these days on templates improvements and definitely implement your suggestions.
Thanks

[taliesins]

@chkp-natanelm is this fixed in a branch or is this a case of this will not implemented? Hopefully quality of the code is considered important by your team.

[chkp-natanelm]

Hi @taliesins,
Thank you for reaching out and for your concern regarding code quality.
I want to assure you that our team prioritizes code quality at the highest level.
We have established internal processes, not visible to the public, which integrating some of your suggestions and more, including internal tools to enhance our code quality, review, and testing procedures.
Thanks