From a9b64f068c851d96340d28954b8d4a354e1b1496 Mon Sep 17 00:00:00 2001 From: Daniel Flook Date: Thu, 23 Jan 2025 17:44:06 +0000 Subject: [PATCH 1/3] Add failing test for version constraint in a file we fail to parse --- .github/workflows/test-version.yaml | 28 +++++++++- .../workflows/test-version/hard-parse/main.tf | 54 +++++++++++++++++++ .../test-version/hard-parse/module/main.tf | 1 + 3 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 tests/workflows/test-version/hard-parse/main.tf create mode 100644 tests/workflows/test-version/hard-parse/module/main.tf diff --git a/.github/workflows/test-version.yaml b/.github/workflows/test-version.yaml index bf869c82..2981ae72 100644 --- a/.github/workflows/test-version.yaml +++ b/.github/workflows/test-version.yaml @@ -888,7 +888,7 @@ jobs: with: persist-credentials: false - - name: Test terraform-version + - name: Test tofu-version uses: ./tofu-version id: tofu-version env: @@ -907,3 +907,29 @@ jobs: echo "::error:: Terraform version not selected" exit 1 fi + + hard_parse: + runs-on: ubuntu-24.04 + name: Get version constraint from hard to parse file + steps: + - name: Checkout + uses: actions/checkout@v4 + with: + persist-credentials: false + + - name: Test terraform-version + uses: ./terraform-version + id: terraform-version + with: + path: tests/workflows/test-version/hard-parse + + - name: Check the version + env: + DETECTED_TERRAFORM_VERSION: ${{ steps.terraform-version.outputs.terraform }} + run: | + echo "The terraform version was $DETECTED_TERRAFORM_VERSION" + + if [[ "$DETECTED_TERRAFORM_VERSION" != "1.10.4" ]]; then + echo "::error:: Terraform constraint not parsed correctly" + exit 1 + fi diff --git a/tests/workflows/test-version/hard-parse/main.tf b/tests/workflows/test-version/hard-parse/main.tf new file mode 100644 index 00000000..2759b091 --- /dev/null +++ b/tests/workflows/test-version/hard-parse/main.tf @@ -0,0 +1,54 @@ +terraform { + +} + +terraform { + required_version = "1.10.4" +} + +locals { + cloud_run_services = [ + { + service_name = "service-1", + output_topics = [ + { + name = "topic-1", + version = "v1" + } + ] + } + ] +} + + +module "pubsub" { + for_each = { + for service in local.cloud_run_services : service.service_name => service + } + source = "./module" + topics = [ + for entity in each.value.output_topics : { + topic_name = entity.version != "" ? format("Topic-%s-%s", entity.name, entity.version) : format("Topic-%s", entity.name) + subscription_name = entity.version != "" ? format("Sub-%s-%s", entity.name, entity.version) : format("Sub-%s", entity.name) + } + ] +} + + +variable "not" {} + +variable "should-be-sensitive" { + sensitive=true +} + +variable "not-again" { + sensitive = false +} + +variable also_sensitive { + sensitive = "true" +} + +terraform { + backend "s3" {} +} diff --git a/tests/workflows/test-version/hard-parse/module/main.tf b/tests/workflows/test-version/hard-parse/module/main.tf new file mode 100644 index 00000000..141b3934 --- /dev/null +++ b/tests/workflows/test-version/hard-parse/module/main.tf @@ -0,0 +1 @@ +variable "topics" {} \ No newline at end of file From 90534104171fd07cfb1d42d46345fc8525206799 Mon Sep 17 00:00:00 2001 From: Daniel Flook Date: Thu, 23 Jan 2025 17:51:05 +0000 Subject: [PATCH 2/3] Do our own 'parsing' of files the real parser couldn't handle --- image/src/terraform/fallback_parser.py | 98 ++++++++++++++++++++++++++ image/src/terraform/hcl.py | 4 +- 2 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 image/src/terraform/fallback_parser.py diff --git a/image/src/terraform/fallback_parser.py b/image/src/terraform/fallback_parser.py new file mode 100644 index 00000000..0d0121b1 --- /dev/null +++ b/image/src/terraform/fallback_parser.py @@ -0,0 +1,98 @@ +""" +Fallback parsing for hcl files + +We only need limited information from Terraform Modules: +- The required_version constraint +- The backend type +- A list of sensitive variable names +- The backend configuration for remote backends and cloud blocks + +The easiest way to get this information is to parse the HCL files directly. +This doesn't always work if our parser fails, or the files are malformed. + +This fallback 'parser' does the stupidest thing that might work to get the information we need. + +TODO: The backend configuration is not yet implemented. +""" + +import re +from pathlib import Path +from typing import Optional + +from github_actions.debug import debug + + +def get_required_version(body: str) -> Optional[str]: + """Get the required_version constraint string from a tf file""" + + if version := re.search(r'required_version\s*=\s*"(.+)"', body): + return version.group(1) + +def get_backend_type(body: str) -> Optional[str]: + """Get the backend type from a tf file""" + + if backend := re.search(r'backend\s*"(.+)"', body): + return backend.group(1) + + if backend := re.search(r'backend\s+(.*)\s*{', body): + return backend.group(1).strip() + + if re.search(r'cloud\s+\{', body): + return 'cloud' + +def get_sensitive_variables(body: str) -> list[str]: + """Get the sensitive variable names from a tf file""" + + variables = [] + + found = False + + for line in reversed(body.splitlines()): + if re.search(r'sensitive\s*=\s*true', line, re.IGNORECASE) or re.search(r'sensitive\s*=\s*"true"', line, re.IGNORECASE): + found = True + continue + + if found and (variable := re.search(r'variable\s*"(.+)"', line)): + variables.append(variable.group(1)) + found = False + + if found and (variable := re.search(r'variable\s+(.+)\{', line)): + variables.append(variable.group(1)) + found = False + + return variables + +def parse(path: Path) -> dict: + debug(f'Attempting to parse {path} with fallback parser') + body = path.read_text() + + module = {} + + if constraint := get_required_version(body): + module['terraform'] = [{ + 'required_version': constraint + }] + + if backend_type := get_backend_type(body): + if 'terraform' not in module: + module['terraform'] = [] + + if backend_type == 'cloud': + module['terraform'].append({'cloud': [{}]}) + else: + module['terraform'].append({'backend': [{backend_type:{}}]}) + + if sensitive_variables := get_sensitive_variables(body): + module['variable'] = [] + for variable in sensitive_variables: + module['variable'].append({ + variable: { + 'sensitive': True + } + }) + + return module + +if __name__ == '__main__': + from pprint import pprint + pprint(parse(Path('tests/workflows/test-validate/hard-parse/main.tf'))) diff --git a/image/src/terraform/hcl.py b/image/src/terraform/hcl.py index 234ea882..85f51ee0 100644 --- a/image/src/terraform/hcl.py +++ b/image/src/terraform/hcl.py @@ -8,6 +8,7 @@ from pathlib import Path from github_actions.debug import debug +import terraform.fallback_parser def try_load(path: Path) -> dict: @@ -15,8 +16,9 @@ def try_load(path: Path) -> dict: with open(path) as f: return hcl2.load(f) except Exception as e: + debug(f'Failed to load {path}') debug(str(e)) - return {} + return terraform.fallback_parser.parse(Path(path)) def is_loadable(path: Path) -> bool: From 1f6b98ab5bc657cb0ec08263547b206f6aa117b0 Mon Sep 17 00:00:00 2001 From: Daniel Flook Date: Thu, 23 Jan 2025 19:11:34 +0000 Subject: [PATCH 3/3] Resolve actions command warning If line and endLine are different (like a range usually is), then it can't cope with col and endColumn being set: ``` Invalid error command value. 'col' and 'endColumn' cannot be set if 'line' and 'endLine' are different values. ``` --- image/tools/convert_validate_report.py | 7 +++++++ tests/test_validate.py | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/image/tools/convert_validate_report.py b/image/tools/convert_validate_report.py index fa4392bb..ebccd19e 100755 --- a/image/tools/convert_validate_report.py +++ b/image/tools/convert_validate_report.py @@ -25,6 +25,13 @@ def convert_to_github(report: Dict, base_path: str) -> Iterable[str]: params['endLine'] = diag['range']['end']['line'] params['endColumn'] = diag['range']['end']['column'] + if params.get('line') != params.get('endLine'): + # GitHub can't cope with 'col' and 'endColumn' if 'line' and 'endLine' are different values. + if 'col' in params: + del params['col'] + if 'endColumn' in params: + del params['endColumn'] + summary = diag['summary'].split('\n')[0] params = ','.join(f'{k}={v}' for k, v in params.items()) diff --git a/tests/test_validate.py b/tests/test_validate.py index 64647135..9d65b2ef 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -122,7 +122,7 @@ def test_invalid_paths(): expected_output = [ '::error file=tests/validate/invalid/main.tf,line=2,col=1,endLine=2,endColumn=33::Duplicate resource "null_resource" configuration', - '::error file=tests/validate/module/invalid.tf,line=2,col=1,endLine=5,endColumn=66::Duplicate resource "null_resource" configuration' + '::error file=tests/validate/module/invalid.tf,line=2,endLine=5::Duplicate resource "null_resource" configuration' ] output = list(convert_to_github(input, 'tests/validate/invalid'))