From b7389fb1e5bb4161430aa1215dbfe61443ba6e83 Mon Sep 17 00:00:00 2001 From: integration-testing-user <44485531+marshall7m@users.noreply.github.com> Date: Sat, 26 Nov 2022 19:04:33 -0800 Subject: [PATCH 1/6] add get_diff_block() --- docker/src/common/utils.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/docker/src/common/utils.py b/docker/src/common/utils.py index 9528bb5b..2dff083a 100644 --- a/docker/src/common/utils.py +++ b/docker/src/common/utils.py @@ -82,3 +82,37 @@ def send_commit_status(state: str, target_url: str): context=os.environ["STATUS_CHECK_NAME"], target_url=target_url, ) + + +def tf_to_diff(matchobj) -> str: + """Replaces Terraform plan syntax with GitHub markdown diff syntax""" + if matchobj["add"]: + return matchobj["add"].replace("+", " ").replace(" ", "+", 1) + + elif matchobj["minus"]: + return matchobj["minus"].replace("-", " ").replace(" ", "-", 1) + + elif matchobj["update"]: + # replace ~ with ! to highlight with orange + return matchobj["update"].replace("~", " ").replace(" ", "!", 1) + + +def get_diff_block(plan) -> str: + """ + Returns Terraform plan as a markdown diff code block + + Arguments: + plan: Terraform Plan stdout without color formatting (use -no-color flag for plan cmd) + """ + diff = re.sub( + r"((?P^\s*\+)|(?P^\s*\-)|(?P^\s*\~))", + tf_to_diff, + plan, + flags=re.MULTILINE, + ) + + return f""" +``` diff +{diff} +``` +""" From 710ba6022aa56d839a943722cf652aa29a46a66f Mon Sep 17 00:00:00 2001 From: integration-testing-user <44485531+marshall7m@users.noreply.github.com> Date: Sat, 26 Nov 2022 19:05:02 -0800 Subject: [PATCH 2/6] add comment PR plan feature --- docker/src/pr_plan/plan.py | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/docker/src/pr_plan/plan.py b/docker/src/pr_plan/plan.py index 0bb78e72..c9d27904 100644 --- a/docker/src/pr_plan/plan.py +++ b/docker/src/pr_plan/plan.py @@ -1,11 +1,14 @@ import os import logging +import json from pprint import pformat import subprocess import sys + import github -import json -from common.utils import get_task_log_url + +sys.path.append(os.path.dirname(__file__) + "/..") +from common.utils import get_task_log_url, get_diff_block log = logging.getLogger(__name__) stream = logging.StreamHandler(sys.stdout) @@ -14,18 +17,42 @@ log.setLevel(logging.DEBUG) +def comment_pr_plan(plan: str) -> str: + plan_block = get_diff_block(plan) + comment = f""" +## Open PR Infrastructure Changes +### Directory: {os.environ["CFG_PATH"]} +
+Plan +
+{plan_block} +
+""" + pr = ( + github.Github(os.environ["GITHUB_TOKEN"], retry=3) + .get_repo(os.environ["REPO_FULL_NAME"]) + .get_pull(int(os.environ["PR_ID"])) + ) + pr.create_issue_comment(comment) + + return comment + + def main() -> None: """ Runs Terragrunt plan command on Terragrunt directory that has been modified and send a commit status if enabled. """ - cmd = f'terragrunt plan --terragrunt-working-dir {os.environ["CFG_PATH"]} --terragrunt-iam-role {os.environ["ROLE_ARN"]}' + cmd = f'terragrunt plan --terragrunt-working-dir {os.environ["CFG_PATH"]} --terragrunt-iam-role {os.environ["ROLE_ARN"]} -no-color' log.debug(f"Command: {cmd}") try: run = subprocess.run(cmd.split(" "), capture_output=True, text=True, check=True) log.info(run.stdout) state = "success" + if os.environ.get("COMMENT_PLAN"): + comment_pr_plan(run.stdout) + except subprocess.CalledProcessError as e: log.info(e.stderr) log.info(e) From 66803b34ff926d7491d6115c6412713e802714ae Mon Sep 17 00:00:00 2001 From: integration-testing-user <44485531+marshall7m@users.noreply.github.com> Date: Sat, 26 Nov 2022 19:05:18 -0800 Subject: [PATCH 3/6] add comment deployment plan feature --- docker/src/terra_run/run.py | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/docker/src/terra_run/run.py b/docker/src/terra_run/run.py index feede25a..fa58513a 100644 --- a/docker/src/terra_run/run.py +++ b/docker/src/terra_run/run.py @@ -3,9 +3,11 @@ import subprocess import sys import json +import ast from typing import List + import aurora_data_api -import ast +import github import boto3 sys.path.append(os.path.dirname(__file__) + "/..") @@ -13,6 +15,7 @@ subprocess_run, send_commit_status, get_task_log_url, + get_diff_block, ) log = logging.getLogger(__name__) @@ -89,6 +92,29 @@ def update_new_resources() -> None: log.info("New provider resources were not created -- skipping") +def comment_terra_run_plan(plan) -> str: + """Sends a GitHub PR comment for the run's Terraform plan""" + plan_block = get_diff_block(plan) + comment = f""" +## Deployment Infrastructure Changes +### Directory: {os.environ["CFG_PATH"]} +### Execution ID: {os.environ["EXECUTION_ID"]} +
+Plan +
+{plan_block} +
+""" + pr = ( + github.Github(os.environ["GITHUB_TOKEN"], retry=3) + .get_repo(os.environ["REPO_FULL_NAME"]) + .get_pull(int(os.environ["PR_ID"])) + ) + pr.create_issue_comment(comment) + + return comment + + def main() -> None: """ Primarily this function prints the results of the Terragrunt command. If the @@ -106,11 +132,10 @@ def main() -> None: text=True, check=True, ) - print(run.stdout) + log.info(run.stdout) state = "success" except subprocess.CalledProcessError as e: - print(e.stderr) - print(e) + log.error(e) state = "failure" log_url = get_task_log_url() @@ -122,6 +147,9 @@ def main() -> None: ) # send ECS task log url with task token to allow Request Approval state to use log url # within approval email + if os.environ.get("COMMENT_PLAN"): + log.info("Commenting Terraform plan results") + comment_terra_run_plan(run.stdout) if state == "success": output = json.dumps({"LogsUrl": log_url}) sf.send_task_success(taskToken=os.environ["TASK_TOKEN"], output=output) From 21eeb8a9b7851b7afdc95f199d17b4d6dc905e83 Mon Sep 17 00:00:00 2001 From: integration-testing-user <44485531+marshall7m@users.noreply.github.com> Date: Sat, 26 Nov 2022 19:05:50 -0800 Subject: [PATCH 4/6] add COMMENT_PLAN env vars --- README.md | 4 +++- fargate.tf | 4 ++++ main.tf | 12 ++++++++++++ variables.tf | 27 ++++++++++++++++++++++----- 4 files changed, 41 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 4baa86ce..a42d24cb 100644 --- a/README.md +++ b/README.md @@ -167,12 +167,14 @@ Terraform module that provisions an AWS serverless CI/CD pipeline used for manag | [ecs\_task\_logs\_retention\_in\_days](#input\_ecs\_task\_logs\_retention\_in\_days) | Number of days the ECS task logs will be retained | `number` | `14` | no | | [ecs\_tasks\_common\_env\_vars](#input\_ecs\_tasks\_common\_env\_vars) | Common env vars defined within all ECS tasks. Useful for setting Terragrunt specific env vars required to run Terragrunt commands. | 
list(object({
    name  = string
    value = string
  }))
| `[]` | no | | [enable\_branch\_protection](#input\_enable\_branch\_protection) | Determines if the branch protection rule is created. If the repository is private (most likely), the GitHub account associated with
the GitHub provider must be registered as a GitHub Pro, GitHub Team, GitHub Enterprise Cloud, or GitHub Enterprise Server account. See here for details: https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/defining-the-mergeability-of-pull-requests/about-protected-branches | `bool` | `true` | no | +| [enable\_gh\_comment\_approval](#input\_enable\_gh\_comment\_approval) | Determines if execution approval votes can be sent via GitHub comments.
This will also enable Terraform plans to be commented within merged PR page | `bool` | `false` | no | +| [enable\_gh\_comment\_pr\_plan](#input\_enable\_gh\_comment\_pr\_plan) | Determines if Terraform plans will be commented within open PR page | `bool` | `false` | no | | [enforce\_admin\_branch\_protection](#input\_enforce\_admin\_branch\_protection) | Determines if the branch protection rule is enforced for the GitHub repository's admins.
This essentially gives admins permission to force push to the trunk branch and can allow their infrastructure-related commits to bypass the CI pipeline. | `bool` | `false` | no | | [file\_path\_pattern](#input\_file\_path\_pattern) | Regex pattern to match webhook modified/new files to. Defaults to any file with `.hcl` or `.tf` extension. | `string` | `".+\\.(hcl|tf)$\n"` | no | | [github\_token\_ssm\_description](#input\_github\_token\_ssm\_description) | Github token SSM parameter description | `string` | `"Github token used by Merge Lock Lambda Function"` | no | | [github\_token\_ssm\_key](#input\_github\_token\_ssm\_key) | AWS SSM Parameter Store key for sensitive Github personal token used by the Merge Lock Lambda Function | `string` | `null` | no | | [github\_token\_ssm\_tags](#input\_github\_token\_ssm\_tags) | Tags for Github token SSM parameter | `map(string)` | `{}` | no | -| [github\_token\_ssm\_value](#input\_github\_token\_ssm\_value) | Registered Github webhook token associated with the Github provider. The token will be used by the Merge Lock Lambda Function.
If not provided, module looks for pre-existing SSM parameter via `var.github_token_ssm_key`".
GitHub token needs the `repo` permission to send commit statuses for private repos. (see more about OAuth scopes here: https://docs.github.com/en/developers/apps/building-oauth-apps/scopes-for-oauth-apps) | `string` | `""` | no | +| [github\_token\_ssm\_value](#input\_github\_token\_ssm\_value) | Registered Github token associated with the Github provider. If not provided,
module looks for pre-existing SSM parameter via `var.github_token_ssm_key`".
GitHub token needs the `repo` permission to send commit statuses and write comments
for private repos (see more about OAuth scopes here:
https://docs.github.com/en/developers/apps/building-oauth-apps/scopes-for-oauth-apps) | `string` | `""` | no | | [lambda\_approval\_request\_vpc\_config](#input\_lambda\_approval\_request\_vpc\_config) | VPC configuration for Lambda approval request function.
Ensure that the configuration allows for outgoing HTTPS traffic. | 
object({
    subnet_ids         = list(string)
    security_group_ids = list(string)
  })
| `null` | no | | [lambda\_approval\_response\_vpc\_config](#input\_lambda\_approval\_response\_vpc\_config) | VPC configuration for Lambda approval response function.
Ensure that the configuration allows for outgoing HTTPS traffic. | 
object({
    subnet_ids         = list(string)
    security_group_ids = list(string)
  })
| `null` | no | | [lambda\_trigger\_sf\_vpc\_config](#input\_lambda\_trigger\_sf\_vpc\_config) | VPC configuration for Lambda trigger\_sf function.
Ensure that the configuration allows for outgoing HTTPS traffic. | 
object({
    subnet_ids         = list(string)
    security_group_ids = list(string)
  })
| `null` | no | diff --git a/fargate.tf b/fargate.tf index 891b4719..9eca159c 100644 --- a/fargate.tf +++ b/fargate.tf @@ -247,6 +247,10 @@ resource "aws_ecs_task_definition" "pr_plan" { { name = "LOG_STREAM_PREFIX" value = local.pr_plan_log_stream_prefix + }, + { + name = "COMMENT_PLAN" + value = var.enable_gh_comment_pr_plan ? "true" : "" } ], local.ecs_tasks_base_env_vars, diff --git a/main.tf b/main.tf index e48a48b9..6583c3c1 100644 --- a/main.tf +++ b/main.tf @@ -57,6 +57,18 @@ resource "aws_sfn_state_machine" "this" { "Name" = "TASK_TOKEN" "Value.$" = "$$.Task.Token" }, + { + "Name" = "CFG_PATH" + "Value.$" = "$.cfg_path" + }, + { + "Name" = "PR_ID" + "Value.$" = "States.Format('{}', $.pr_id)" + }, + { + "Name" = "COMMENT_PLAN" + "Value" = var.enable_gh_comment_approval ? "true" : "" + } ] ) } diff --git a/variables.tf b/variables.tf index 9c4beec3..d45861f9 100644 --- a/variables.tf +++ b/variables.tf @@ -302,9 +302,11 @@ variable "github_token_ssm_description" { variable "github_token_ssm_value" { description = < Date: Sat, 26 Nov 2022 19:06:23 -0800 Subject: [PATCH 5/6] add env vars needed for commenting plan --- functions/webhook_receiver/invoker.py | 2 ++ functions/webhook_receiver/lambda_function.py | 13 +++++++------ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/functions/webhook_receiver/invoker.py b/functions/webhook_receiver/invoker.py index 58517b96..2dcce0df 100644 --- a/functions/webhook_receiver/invoker.py +++ b/functions/webhook_receiver/invoker.py @@ -56,6 +56,7 @@ def trigger_pr_plan( base_ref: str, head_ref: str, head_sha: str, + pr_id: int, logs_url: str, send_commit_status: bool, ) -> None: @@ -140,6 +141,7 @@ def trigger_pr_plan( "name": "COMMIT_ID", "value": head_sha, }, + {"name": "PR_ID", "value": str(pr_id)}, {"name": "CFG_PATH", "value": path}, { "name": "ROLE_ARN", diff --git a/functions/webhook_receiver/lambda_function.py b/functions/webhook_receiver/lambda_function.py index 93815cd6..1981b66b 100644 --- a/functions/webhook_receiver/lambda_function.py +++ b/functions/webhook_receiver/lambda_function.py @@ -30,12 +30,13 @@ def open_pr(request: Request): ) trigger_pr_plan( - event.body.repository.full_name, - event.body.pull_request.base.ref, - event.body.pull_request.head.ref, - event.body.pull_request.head.sha, - context.logs_url, - event.body.commit_status_config.get("PrPlan"), + repo_full_name=event.body.repository.full_name, + base_ref=event.body.pull_request.base.ref, + head_ref=event.body.pull_request.head.ref, + head_sha=event.body.pull_request.head.sha, + pr_id=event.body.pull_request.number, + logs_url=context.logs_url, + send_commit_status=event.body.commit_status_config.get("PrPlan"), ) return JSONResponse( From e50cafa57a1cd6cf307f060ae0b3adc916508102 Mon Sep 17 00:00:00 2001 From: integration-testing-user <44485531+marshall7m@users.noreply.github.com> Date: Sat, 26 Nov 2022 19:07:23 -0800 Subject: [PATCH 6/6] add tests --- tests/e2e/base_e2e.py | 15 +++++ tests/fixtures/terraform/mut/basic/main.tf | 4 +- tests/unit/docker/test_pr_plan.py | 64 +++++++++++++++++++ tests/unit/docker/test_terra_run.py | 74 ++++++++++++++++++++-- 4 files changed, 151 insertions(+), 6 deletions(-) create mode 100644 tests/unit/docker/test_pr_plan.py diff --git a/tests/e2e/base_e2e.py b/tests/e2e/base_e2e.py index 45b22156..ac58e3f6 100644 --- a/tests/e2e/base_e2e.py +++ b/tests/e2e/base_e2e.py @@ -94,8 +94,14 @@ def pr_plan_pending_statuses(self, request, mut_output, pr, repo): ) log.debug(f"Expected count: {expected_count}") wait = 10 + attempts = 0 + max_attempts = 12 statuses = [] while len(statuses) != expected_count: + if attempts == max_attempts: + pytest.fail( + "Max attempt reached -- Lambda Function might have failed beforehand" + ) log.debug(f"Waiting {wait} seconds") time.sleep(wait) statuses = [ @@ -104,6 +110,8 @@ def pr_plan_pending_statuses(self, request, mut_output, pr, repo): if status.context != mut_output["merge_lock_status_check_name"] ] + attempts += 1 + return statuses @pytest.fixture(scope="class") @@ -111,8 +119,14 @@ def pr_plan_finished_statuses(self, pr_plan_pending_statuses, mut_output, pr, re """Returns list of PR plan tasks' finished commit statuses""" log.info("Waiting for all PR plan commit statuses to be updated") wait = 15 + attempts = 0 + max_attempts = 12 statuses = [] while len(statuses) != len(pr_plan_pending_statuses): + if attempts == max_attempts: + pytest.fail( + "Max attempt reached -- ECS task might have failed beforehand" + ) log.debug(f"Waiting {wait} seconds") time.sleep(wait) statuses = [ @@ -123,6 +137,7 @@ def pr_plan_finished_statuses(self, pr_plan_pending_statuses, mut_output, pr, re ] log.debug(f"Finished count: {len(statuses)}") + attempts += 1 return statuses diff --git a/tests/fixtures/terraform/mut/basic/main.tf b/tests/fixtures/terraform/mut/basic/main.tf index 432a1a1d..a17ec77e 100644 --- a/tests/fixtures/terraform/mut/basic/main.tf +++ b/tests/fixtures/terraform/mut/basic/main.tf @@ -22,7 +22,9 @@ module "mut_infrastructure_live_ci" { enforce_admin_branch_protection = var.enforce_admin_branch_protection - commit_status_config = var.commit_status_config + enable_gh_comment_pr_plan = true + enable_gh_comment_approval = true + commit_status_config = var.commit_status_config metadb_name = var.metadb_name metadb_username = var.metadb_username diff --git a/tests/unit/docker/test_pr_plan.py b/tests/unit/docker/test_pr_plan.py new file mode 100644 index 00000000..aa915797 --- /dev/null +++ b/tests/unit/docker/test_pr_plan.py @@ -0,0 +1,64 @@ +import os +import logging +from unittest.mock import patch + +from docker.src.pr_plan.plan import comment_pr_plan + +log = logging.getLogger(__name__) +log.setLevel(logging.DEBUG) + + +@patch("github.Github") +@patch.dict( + os.environ, + {"CFG_PATH": "terraform/cfg", "REPO_FULL_NAME": "user/repo", "PR_ID": "1"}, +) +def test_comment_pr_plan(mock_gh): + """Ensures comment_pr_plan() formats the comment's diff block properly and returns the expected comment""" + plan = """ + +Changes to Outputs: + - bar = "old" -> null + + baz = "new" + ~ foo = "old" -> "new" + +You can apply this plan to save these new output values to the Terraform +state, without changing any real infrastructure. + +───────────────────────────────────────────────────────────────────────────── + +Note: You didn't use the -out option to save this plan, so Terraform can't +guarantee to take exactly these actions if you run "terraform apply" now. + +""" + expected = """ +## Open PR Infrastructure Changes +### Directory: terraform/cfg +
+Plan +
+ +``` diff + + +Changes to Outputs: +- bar = "old" -> null ++ baz = "new" +! foo = "old" -> "new" + +You can apply this plan to save these new output values to the Terraform +state, without changing any real infrastructure. + +───────────────────────────────────────────────────────────────────────────── + +Note: You didn't use the -out option to save this plan, so Terraform can't +guarantee to take exactly these actions if you run "terraform apply" now. + + +``` + +
+""" + actual = comment_pr_plan(plan) + + assert actual == expected diff --git a/tests/unit/docker/test_terra_run.py b/tests/unit/docker/test_terra_run.py index 2519f35e..5516d790 100644 --- a/tests/unit/docker/test_terra_run.py +++ b/tests/unit/docker/test_terra_run.py @@ -1,18 +1,20 @@ -from docker.src.common.utils import ServerException -import pytest import os import logging from subprocess import CalledProcessError import json -import aurora_data_api from unittest.mock import patch, call -from tests.helpers.utils import null_provider_resource, insert_records, rds_data_client + +import aurora_data_api +import pytest + +from docker.src.common.utils import ServerException, subprocess_run from docker.src.terra_run.run import ( update_new_resources, get_new_provider_resources, main, + comment_terra_run_plan, ) -from docker.src.common.utils import subprocess_run +from tests.helpers.utils import null_provider_resource, insert_records, rds_data_client from tests.unit.docker.conftest import mock_subprocess_run log = logging.getLogger(__name__) @@ -183,3 +185,65 @@ def test_main( main() assert mock_send_commit_status.call_args_list == [call(expected_status, log_url)] + + +@patch("github.Github") +@patch.dict( + os.environ, + { + "CFG_PATH": "terraform/cfg", + "EXECUTION_ID": "run-123", + "REPO_FULL_NAME": "user/repo", + "PR_ID": "1", + }, +) +def test_comment_terra_run_plan(mock_gh): + """Ensures comment_terra_run_plan() formats the comment's diff block properly and returns the expected comment""" + plan = """ + +Changes to Outputs: + - bar = "old" -> null + + baz = "new" + ~ foo = "old" -> "new" + +You can apply this plan to save these new output values to the Terraform +state, without changing any real infrastructure. + +───────────────────────────────────────────────────────────────────────────── + +Note: You didn't use the -out option to save this plan, so Terraform can't +guarantee to take exactly these actions if you run "terraform apply" now. + +""" + expected = """ +## Deployment Infrastructure Changes +### Directory: terraform/cfg +### Execution ID: run-123 +
+Plan +
+ +``` diff + + +Changes to Outputs: +- bar = "old" -> null ++ baz = "new" +! foo = "old" -> "new" + +You can apply this plan to save these new output values to the Terraform +state, without changing any real infrastructure. + +───────────────────────────────────────────────────────────────────────────── + +Note: You didn't use the -out option to save this plan, so Terraform can't +guarantee to take exactly these actions if you run "terraform apply" now. + + +``` + +
+""" + actual = comment_terra_run_plan(plan) + + assert actual == expected