From 455d0fdee67a8b4608744b73fb5a53ffd8e9907b Mon Sep 17 00:00:00 2001 From: Daniel Flook Date: Sun, 24 Oct 2021 22:32:49 +0100 Subject: [PATCH 1/2] Refactor github_pr_comment.py --- .github/workflows/test.yaml | 2 +- image/tools/github_pr_comment.py | 511 +++++++++++++++---------------- tests/test_pr_comment.py | 460 ++++++++++++++++------------ 3 files changed, 517 insertions(+), 456 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index bc5f16d6..21b88f05 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -13,7 +13,7 @@ jobs: - name: Set up Python uses: actions/setup-python@v2 with: - python-version: 3.7 + python-version: 3.9 - name: Install dependencies run: | diff --git a/image/tools/github_pr_comment.py b/image/tools/github_pr_comment.py index 3f955435..93c2fd98 100755 --- a/image/tools/github_pr_comment.py +++ b/image/tools/github_pr_comment.py @@ -1,26 +1,141 @@ #!/usr/bin/python3 +import datetime +import hashlib import json import os import re import sys -import datetime -import hashlib -from typing import Optional, Dict, Iterable +from typing import Optional, Dict, Iterable, cast, NewType, TypedDict, Tuple, Any import requests -github = requests.Session() -github.headers['authorization'] = f'token {os.environ["GITHUB_TOKEN"]}' -github.headers['user-agent'] = 'terraform-github-actions' -github.headers['accept'] = 'application/vnd.github.v3+json' +GitHubUrl = NewType('GitHubUrl', str) +PrUrl = NewType('PrUrl', GitHubUrl) +IssueUrl = NewType('IssueUrl', GitHubUrl) +CommentUrl = NewType('CommentUrl', GitHubUrl) +Plan = NewType('Plan', str) +Status = NewType('Status', str) + + +class GitHubActionsEnv(TypedDict): + GITHUB_API_URL: str + GITHUB_TOKEN: str + GITHUB_EVENT_PATH: str + GITHUB_EVENT_NAME: str + GITHUB_REPOSITORY: str + GITHUB_SHA: str -github_url = os.environ.get('GITHUB_SERVER_URL', 'https://github.com') -github_api_url = os.environ.get('GITHUB_API_URL', 'https://api.github.com') job_tmp_dir = os.environ.get('JOB_TMP_DIR', '.') -def github_api_request(method, *args, **kwargs): +env = cast(GitHubActionsEnv, os.environ) + + +def github_session(github_env: GitHubActionsEnv) -> requests.Session: + session = requests.Session() + session.headers['authorization'] = f'token {github_env["GITHUB_TOKEN"]}' + session.headers['user-agent'] = 'terraform-github-actions' + session.headers['accept'] = 'application/vnd.github.v3+json' + return session + + +github = github_session(env) + + +class ActionInputs(TypedDict): + """ + Information used to identify a plan + """ + INPUT_BACKEND_CONFIG: str + INPUT_BACKEND_CONFIG_FILE: str + INPUT_VARIABLES: str + INPUT_VAR: str + INPUT_VAR_FILE: str + INPUT_PATH: str + INPUT_WORKSPACE: str + INPUT_LABEL: str + INPUT_ADD_GITHUB_COMMENT: str + + +def plan_identifier(action_inputs: ActionInputs) -> str: + def mask_backend_config() -> Optional[str]: + if action_inputs.get('INPUT_BACKEND_CONFIG') is None: + return None + + bad_words = [ + 'token', + 'password', + 'sas_token', + 'access_key', + 'secret_key', + 'client_secret', + 'access_token', + 'http_auth', + 'secret_id', + 'encryption_key', + 'key_material', + 'security_token', + 'conn_str', + 'sse_customer_key', + 'application_credential_secret' + ] + + def has_bad_word(s: str) -> bool: + for bad_word in bad_words: + if bad_word in s: + return True + return False + + clean = [] + + for field in action_inputs.get('INPUT_BACKEND_CONFIG', '').split(','): + if not field: + continue + + if not has_bad_word(field): + clean.append(field) + + return ','.join(clean) + + if action_inputs['INPUT_LABEL']: + return f'Terraform plan for __{action_inputs["INPUT_LABEL"]}__' + + label = f'Terraform plan in __{action_inputs["INPUT_PATH"]}__' + + if action_inputs["INPUT_WORKSPACE"] != 'default': + label += f' in the __{action_inputs["INPUT_WORKSPACE"]}__ workspace' + + backend_config = mask_backend_config() + if backend_config: + label += f'\nWith backend config: `{backend_config}`' + + if action_inputs["INPUT_BACKEND_CONFIG_FILE"]: + label += f'\nWith backend config files: `{action_inputs["INPUT_BACKEND_CONFIG_FILE"]}`' + + if action_inputs["INPUT_VAR"]: + label += f'\nWith vars: `{action_inputs["INPUT_VAR"]}`' + + if action_inputs["INPUT_VAR_FILE"]: + label += f'\nWith var files: `{action_inputs["INPUT_VAR_FILE"]}`' + + if action_inputs["INPUT_VARIABLES"]: + stripped_vars = action_inputs["INPUT_VARIABLES"].strip() + if '\n' in stripped_vars: + label += f'''
With variables + +```hcl +{stripped_vars} +``` +
+''' + else: + label += f'\nWith variables: `{stripped_vars}`' + + return label + + +def github_api_request(method: str, *args, **kwargs) -> requests.Response: response = github.request(method, *args, **kwargs) if 400 <= response.status_code < 500: @@ -47,12 +162,13 @@ def github_api_request(method, *args, **kwargs): return response + def debug(msg: str) -> None: sys.stderr.write(msg) sys.stderr.write('\n') -def paginate(url, *args, **kwargs) -> Iterable[Dict]: +def paginate(url: GitHubUrl, *args, **kwargs) -> Iterable[Dict[str, Any]]: while True: response = github_api_request('get', url, *args, **kwargs) response.raise_for_status() @@ -64,12 +180,8 @@ def paginate(url, *args, **kwargs) -> Iterable[Dict]: else: return -def prs(repo: str) -> Iterable[Dict]: - url = f'{github_api_url}/repos/{repo}/pulls' - yield from paginate(url, params={'state': 'all'}) - -def find_pr() -> str: +def find_pr(env: GitHubActionsEnv) -> PrUrl: """ Find the pull request this event is related to @@ -78,37 +190,41 @@ def find_pr() -> str: """ - with open(os.environ['GITHUB_EVENT_PATH']) as f: + with open(env['GITHUB_EVENT_PATH']) as f: event = json.load(f) - event_type = os.environ['GITHUB_EVENT_NAME'] + event_type = env['GITHUB_EVENT_NAME'] if event_type in ['pull_request', 'pull_request_review_comment', 'pull_request_target', 'pull_request_review']: - return event['pull_request']['url'] + return cast(PrUrl, event['pull_request']['url']) elif event_type == 'issue_comment': if 'pull_request' in event['issue']: - return event['issue']['pull_request']['url'] + return cast(PrUrl, event['issue']['pull_request']['url']) else: - raise Exception(f'This comment is not for a PR. Add a filter of `if: github.event.issue.pull_request`') + raise Exception('This comment is not for a PR. Add a filter of `if: github.event.issue.pull_request`') elif event_type == 'push': - repo = os.environ['GITHUB_REPOSITORY'] - commit = os.environ['GITHUB_SHA'] + repo = env['GITHUB_REPOSITORY'] + commit = env['GITHUB_SHA'] + + def prs() -> Iterable[Dict[str, Any]]: + url = f'{env.get("GITHUB_API_URL", "https://api.github.com")}/repos/{repo}/pulls' + yield from paginate(cast(PrUrl, url), params={'state': 'all'}) - for pr in prs(repo): + for pr in prs(): if pr['merge_commit_sha'] == commit: - return pr['url'] + return cast(PrUrl, pr['url']) raise Exception(f'No PR found in {repo} for commit {commit} (was it pushed directly to the target branch?)') else: raise Exception(f"The {event_type} event doesn\'t relate to a Pull Request.") -def current_user() -> str: - token_hash = hashlib.sha256(os.environ["GITHUB_TOKEN"].encode()).hexdigest() +def current_user(github_env: GitHubActionsEnv) -> str: + token_hash = hashlib.sha256(github_env['GITHUB_TOKEN'].encode()).hexdigest() try: with open(os.path.join(job_tmp_dir, 'token-cache', token_hash)) as f: @@ -118,7 +234,7 @@ def current_user() -> str: except Exception as e: debug(str(e)) - response = github_api_request('get', f'{github_api_url}/user') + response = github_api_request('get', f'{github_env["GITHUB_API_URL"]}/user') if response.status_code != 403: user = response.json() debug('GITHUB_TOKEN user:') @@ -139,295 +255,162 @@ def current_user() -> str: debug(f'GITHUB_TOKEN username: {username}') return username -class TerraformComment: - """ - The GitHub comment for this specific terraform plan - """ - - def __init__(self, pr_url: str=None): - self._plan = None - self._status = None - self._comment_url = None - - if pr_url is None: - return - response = github_api_request('get', pr_url) - response.raise_for_status() - - self._issue_url = response.json()['_links']['issue']['href'] + '/comments' - - username = current_user() +def create_summary(plan) -> Optional[str]: + summary = None - debug('Looking for an existing comment:') + for line in plan.splitlines(): + if line.startswith('No changes') or line.startswith('Error'): + return line - for comment in paginate(self._issue_url): - debug(json.dumps(comment)) - if comment['user']['login'] == username: - match = re.match(rf'{re.escape(self._comment_identifier)}.*```(?:hcl)?(.*?)```.*', comment['body'], re.DOTALL) + if line.startswith('Plan:'): + summary = line - if not match: - match = re.match(rf'{re.escape(self._old_comment_identifier)}\n```(.*?)```.*', comment['body'], re.DOTALL) - - if match: - self._comment_url = comment['url'] - self._plan = match.group(1).strip() - return + if line.startswith('Changes to Outputs'): + if summary: + return summary + ' Changes to Outputs.' + else: + return 'Changes to Outputs' - @property - def _comment_identifier(self): - if self.label: - return f'Terraform plan for __{self.label}__' + return summary - label = f'Terraform plan in __{self.path}__' - if self.workspace != 'default': - label += f' in the __{self.workspace}__ workspace' +def format_body(action_inputs: ActionInputs, plan: Plan, status: Status, collapse_threshold: int) -> str: - if self.backend_config: - label += f'\nWith backend config: `{self.backend_config}`' + details_open = '' + highlighting = '' - if self.backend_config_files: - label += f'\nWith backend config files: `{self.backend_config_files}`' + summary_line = create_summary(plan) - if self.vars: - label += f'\nWith vars: `{self.vars}`' + if plan.startswith('Error'): + details_open = ' open' + elif 'Plan:' in plan: + highlighting = 'hcl' + num_lines = len(plan.splitlines()) + if num_lines < collapse_threshold: + details_open = ' open' - if self.var_files: - label += f'\nWith var files: `{self.var_files}`' + if summary_line is None: + details_open = ' open' - if self.variables: - stripped_vars = self.variables.strip() - if '\n' in stripped_vars: - label += f'''
With variables + body = f'''{plan_identifier(action_inputs)} + +{ f'{summary_line}' if summary_line is not None else '' } -```hcl -{stripped_vars} +```{highlighting} +{plan} ```
''' - else: - label += f'\nWith variables: `{stripped_vars}`' - return label + if status: + body += '\n' + status - @property - def _old_comment_identifier(self): - if self.label: - return f'Terraform plan for __{self.label}__' + return body - label = f'Terraform plan in __{self.path}__' - if self.workspace != 'default': - label += f' in the __{self.workspace}__ workspace' +def update_comment(issue_url: IssueUrl, + comment_url: Optional[CommentUrl], + body: str, + only_if_exists: bool = False) -> Optional[CommentUrl]: + """ + Update (or create) a comment - if self.init_args: - label += f'\nUsing init args: `{self.init_args}`' - if self.plan_args: - label += f'\nUsing plan args: `{self.plan_args}`' + :param issue_url: The url of the issue to create or update the comment in + :param comment_url: The url of the comment to update + :param body: The new comment body + :param only_if_exists: Only update an existing comment - don't create it + """ - return label + debug(body) - @property - def backend_config(self) -> Optional[str]: - if os.environ.get('INPUT_BACKEND_CONFIG') is None: + if comment_url is None: + if only_if_exists: + debug('Comment doesn\'t already exist - not creating it') return None + # Create a new comment + debug('Creating comment') + response = github_api_request('post', issue_url, json={'body': body}) + else: + # Update existing comment + debug('Updating existing comment') + response = github_api_request('patch', comment_url, json={'body': body}) - bad_words = [ - 'token', - 'password', - 'sas_token', - 'access_key', - 'secret_key', - 'client_secret', - 'access_token', - 'http_auth', - 'secret_id', - 'encryption_key', - 'key_material', - 'security_token', - 'conn_str', - 'sse_customer_key', - 'application_credential_secret' - ] - - def has_bad_word(s: str) -> bool: - for bad_word in bad_words: - if bad_word in s: - return True - return False - - clean = [] - - for field in os.environ.get('INPUT_BACKEND_CONFIG', '').split(','): - if not field: - continue - - if not has_bad_word(field): - clean.append(field) - - return ','.join(clean) - - @property - def backend_config_files(self) -> str: - return os.environ.get('INPUT_BACKEND_CONFIG_FILE') - - @property - def variables(self) -> str: - return os.environ.get('INPUT_VARIABLES') - - @property - def vars(self) -> str: - return os.environ.get('INPUT_VAR') - - @property - def var_files(self) -> str: - return os.environ.get('INPUT_VAR_FILE') - - @property - def path(self) -> str: - return os.environ['INPUT_PATH'] - - @property - def workspace(self) -> str: - return os.environ.get('INPUT_WORKSPACE') - - @property - def label(self) -> str: - return os.environ.get('INPUT_LABEL') - - @property - def init_args(self) -> str: - return os.environ['INIT_ARGS'] - - @property - def plan_args(self) -> str: - return os.environ['PLAN_ARGS'] - - @property - def plan(self) -> Optional[str]: - return self._plan - - @plan.setter - def plan(self, plan: str) -> None: - self._plan = plan.strip() - - @property - def status(self) -> Optional[str]: - return self._status - - @status.setter - def status(self, status: str) -> None: - self._status = status.strip() - - def body(self) -> str: - body = f'{self._comment_identifier}\n```hcl\n{self.plan}\n```' - - if self.status: - body += '\n' + self.status - - return body - - def collapsable_body(self) -> str: - - try: - collapse_threshold = int(os.environ['TF_PLAN_COLLAPSE_LENGTH']) - except (ValueError, KeyError): - collapse_threshold = 10 - - open = '' - highlighting = '' - - if self.plan.startswith('Error'): - open = ' open' - elif 'Plan:' in self.plan: - highlighting = 'hcl' - num_lines = len(self.plan.splitlines()) - if num_lines < collapse_threshold: - open = ' open' - - body = f'''{self._comment_identifier} - - {self.summary()} - -```{highlighting} -{self.plan} -``` - -''' - - if self.status: - body += '\n' + self.status + debug(response.content.decode()) + response.raise_for_status() + return cast(CommentUrl, response.json()['url']) - return body - def summary(self) -> str: - summary = None +def find_issue_url(pr_url: str) -> IssueUrl: + response = github_api_request('get', pr_url) + response.raise_for_status() - for line in self.plan.splitlines(): - if line.startswith('No changes') or line.startswith('Error'): - return line + return cast(IssueUrl, response.json()['_links']['issue']['href'] + '/comments') - if line.startswith('Plan:'): - summary = line - if line.startswith('Changes to Outputs'): - if summary: - return summary + ' Changes to Outputs.' - else: - return 'Changes to Outputs' +def find_comment(issue_url: IssueUrl, username: str, action_inputs: ActionInputs) -> Tuple[ + Optional[CommentUrl], Optional[Plan]]: + debug('Looking for an existing comment:') - return summary + plan_id = plan_identifier(action_inputs) - def update_comment(self, only_if_exists=False): - body = self.collapsable_body() - debug(body) + for comment in paginate(issue_url): + debug(json.dumps(comment)) + if comment['user']['login'] == username: + match = re.match(rf'{re.escape(plan_id)}.*```(?:hcl)?(.*?)```.*', comment['body'], re.DOTALL) - if self._comment_url is None: - if only_if_exists: - debug('Comment doesn\'t already exist - not creating it') - return - # Create a new comment - debug('Creating comment') - response = github_api_request('post', self._issue_url, json={'body': body}) - else: - # Update existing comment - debug('Updating existing comment') - response = github_api_request('patch', self._comment_url, json={'body': body}) + if match: + return comment['url'], cast(Plan, match.group(1).strip()) - debug(response.content.decode()) - response.raise_for_status() - self._comment_url = response.json()['url'] + return None, None -if __name__ == '__main__': +def main() -> None: if len(sys.argv) < 2: print(f'''Usage: STATUS="" {sys.argv[0]} plan plan.txt''') - tf_comment = TerraformComment(find_pr()) + action_inputs = cast(ActionInputs, os.environ) + + try: + collapse_threshold = int(os.environ['TF_PLAN_COLLAPSE_LENGTH']) + except (ValueError, KeyError): + collapse_threshold = 10 + + pr_url = find_pr(env) + issue_url = find_issue_url(pr_url) + username = current_user(env) + comment_url, plan = find_comment(issue_url, username, action_inputs) + status = cast(Status, os.environ.get('STATUS', '')) + only_if_exists = False if sys.argv[1] == 'plan': - tf_comment.plan = sys.stdin.read().strip() - tf_comment.status = os.environ['STATUS'] + plan = cast(Plan, sys.stdin.read().strip()) - if os.environ['INPUT_ADD_GITHUB_COMMENT'] == 'changes-only' and os.environ.get('TF_CHANGES', 'true') == 'false': + if action_inputs['INPUT_ADD_GITHUB_COMMENT'] == 'changes-only' and os.environ.get('TF_CHANGES', + 'true') == 'false': only_if_exists = True + body = format_body(action_inputs, plan, status, collapse_threshold) + update_comment(issue_url, comment_url, body, only_if_exists) + elif sys.argv[1] == 'status': - if tf_comment.plan is None: + if plan is None: exit(1) else: - tf_comment.status = os.environ['STATUS'] + body = format_body(action_inputs, plan, status, collapse_threshold) + update_comment(issue_url, comment_url, body, only_if_exists) + elif sys.argv[1] == 'get': - if tf_comment.plan is None: + if plan is None: exit(1) with open(sys.argv[2], 'w') as f: - f.write(tf_comment.plan) - exit(0) + f.write(plan) - tf_comment.update_comment(only_if_exists) + +if __name__ == '__main__': + main() diff --git a/tests/test_pr_comment.py b/tests/test_pr_comment.py index a134fafc..215d3cb6 100644 --- a/tests/test_pr_comment.py +++ b/tests/test_pr_comment.py @@ -1,217 +1,323 @@ -import github_pr_comment -from github_pr_comment import TerraformComment - -import pytest - -def setup_comment(monkeypatch, *, - path ='/test/terraform', - workspace ='default', - backend_config ='', - backend_config_file ='', - var ='', - var_file ='', - label ='', - ): - - monkeypatch.setenv('INPUT_WORKSPACE', workspace) - monkeypatch.setenv('INPUT_PATH', path) - monkeypatch.setattr('github_pr_comment.current_user', lambda: 'github-actions[bot]') - monkeypatch.setenv('INPUT_BACKEND_CONFIG', backend_config) - monkeypatch.setenv('INPUT_BACKEND_CONFIG_FILE', backend_config_file) - monkeypatch.setenv('INPUT_VAR', var) - monkeypatch.setenv('INPUT_VAR_FILE', var_file) - monkeypatch.setenv('INPUT_LABEL', label) - monkeypatch.setenv('INIT_ARGS', '') - monkeypatch.setenv('PLAN_ARGS', '') - - -def test_path_only(monkeypatch): - - setup_comment(monkeypatch, +from github_pr_comment import format_body, ActionInputs, create_summary + +plan = '''An execution plan has been generated and is shown below. +... +Plan: 1 to add, 0 to change, 0 to destroy.''' + + +def action_inputs(*, path='/test/terraform', - ) - comment = TerraformComment() - comment.plan = 'Hello, this is my plan' - comment.status = 'Testing' + workspace='default', + backend_config='', + backend_config_file='', + variables='', + var='', + var_file='', + label='', ) -> ActionInputs: + return ActionInputs( + INPUT_WORKSPACE=workspace, + INPUT_PATH=path, + INPUT_BACKEND_CONFIG=backend_config, + INPUT_BACKEND_CONFIG_FILE=backend_config_file, + INPUT_VARIABLES=variables, + INPUT_VAR=var, + INPUT_VAR_FILE=var_file, + INPUT_LABEL=label, + INPUT_ADD_GITHUB_COMMENT='true' + ) + + +def test_path_only(): + inputs = action_inputs( + path='/test/terraform' + ) + + status = 'Testing' expected = '''Terraform plan in __/test/terraform__ +
+Plan: 1 to add, 0 to change, 0 to destroy. + ```hcl -Hello, this is my plan +An execution plan has been generated and is shown below. +... +Plan: 1 to add, 0 to change, 0 to destroy. ``` +
+ Testing''' - assert comment.body() == expected + assert format_body(inputs, plan, status, 10).splitlines() == expected.splitlines() -def test_nondefault_workspace(monkeypatch): - setup_comment(monkeypatch, - path='/test/terraform', - workspace='myworkspace' - ) - comment = TerraformComment() - comment.plan = 'Hello, this is my plan' - comment.status = 'Testing' +def test_nondefault_workspace(): + inputs = action_inputs( + path='/test/terraform', + workspace='myworkspace' + ) + + status = 'Testing' expected = '''Terraform plan in __/test/terraform__ in the __myworkspace__ workspace +
+Plan: 1 to add, 0 to change, 0 to destroy. + ```hcl -Hello, this is my plan +An execution plan has been generated and is shown below. +... +Plan: 1 to add, 0 to change, 0 to destroy. ``` +
+ Testing''' - assert comment.body() == expected + assert format_body(inputs, plan, status, 10).splitlines() == expected.splitlines() -def test_var(monkeypatch): - setup_comment(monkeypatch, - path='/test/terraform', - var='var1=value' - ) - comment = TerraformComment() - comment.plan = 'Hello, this is my plan' - comment.status = 'Testing' +def test_variables_single_line(): + inputs = action_inputs( + path='/test/terraform', + variables='var1="value"' + ) + + status = 'Testing' + + expected = '''Terraform plan in __/test/terraform__ +With variables: `var1="value"` +
+Plan: 1 to add, 0 to change, 0 to destroy. + +```hcl +An execution plan has been generated and is shown below. +... +Plan: 1 to add, 0 to change, 0 to destroy. +``` +
+ +Testing''' + + assert format_body(inputs, plan, status, 10).splitlines() == expected.splitlines() + + +def test_variables_multi_line(): + inputs = action_inputs( + path='/test/terraform', + variables='''var1="value" +var2="value2"''' + ) + + status = 'Testing' + + expected = '''Terraform plan in __/test/terraform__
With variables + +```hcl +var1="value" +var2="value2" +``` +
+ +
+Plan: 1 to add, 0 to change, 0 to destroy. + +```hcl +An execution plan has been generated and is shown below. +... +Plan: 1 to add, 0 to change, 0 to destroy. +``` +
+ +Testing''' + + assert format_body(inputs, plan, status, 10).splitlines() == expected.splitlines() + + +def test_var(): + inputs = action_inputs( + path='/test/terraform', + var='var1=value' + ) + + status = 'Testing' expected = '''Terraform plan in __/test/terraform__ With vars: `var1=value` +
+Plan: 1 to add, 0 to change, 0 to destroy. + ```hcl -Hello, this is my plan +An execution plan has been generated and is shown below. +... +Plan: 1 to add, 0 to change, 0 to destroy. ``` +
+ Testing''' - assert comment.body() == expected + assert format_body(inputs, plan, status, 10).splitlines() == expected.splitlines() -def test_var_file(monkeypatch): - setup_comment(monkeypatch, - path='/test/terraform', - var_file='vars.tf' - ) - comment = TerraformComment() - comment.plan = 'Hello, this is my plan' - comment.status = 'Testing' +def test_var_file(): + inputs = action_inputs( + path='/test/terraform', + var_file='vars.tf' + ) + + status = 'Testing' expected = '''Terraform plan in __/test/terraform__ With var files: `vars.tf` +
+Plan: 1 to add, 0 to change, 0 to destroy. + ```hcl -Hello, this is my plan +An execution plan has been generated and is shown below. +... +Plan: 1 to add, 0 to change, 0 to destroy. ``` +
+ Testing''' - assert comment.body() == expected + assert format_body(inputs, plan, status, 10).splitlines() == expected.splitlines() -def test_backend_config(monkeypatch): - setup_comment(monkeypatch, - path='/test/terraform', - backend_config='bucket=test,key=backend' - ) - comment = TerraformComment() - comment.plan = 'Hello, this is my plan' - comment.status = 'Testing' +def test_backend_config(): + inputs = action_inputs( + path='/test/terraform', + backend_config='bucket=test,key=backend' + ) + status = 'Testing' expected = '''Terraform plan in __/test/terraform__ With backend config: `bucket=test,key=backend` +
+Plan: 1 to add, 0 to change, 0 to destroy. + ```hcl -Hello, this is my plan +An execution plan has been generated and is shown below. +... +Plan: 1 to add, 0 to change, 0 to destroy. ``` +
+ Testing''' - assert comment.body() == expected + assert format_body(inputs, plan, status, 10).splitlines() == expected.splitlines() -def test_backend_config_bad_words(monkeypatch): - setup_comment(monkeypatch, - path='/test/terraform', - backend_config='bucket=test,password=secret,key=backend,token=secret' - ) - comment = TerraformComment() - comment.plan = 'Hello, this is my plan' - comment.status = 'Testing' +def test_backend_config_bad_words(): + inputs = action_inputs( + path='/test/terraform', + backend_config='bucket=test,password=secret,key=backend,token=secret' + ) + + status = 'Testing' expected = '''Terraform plan in __/test/terraform__ With backend config: `bucket=test,key=backend` +
+Plan: 1 to add, 0 to change, 0 to destroy. + ```hcl -Hello, this is my plan +An execution plan has been generated and is shown below. +... +Plan: 1 to add, 0 to change, 0 to destroy. ``` +
+ Testing''' - assert comment.body() == expected + assert format_body(inputs, plan, status, 10).splitlines() == expected.splitlines() -def test_backend_config_file(monkeypatch): +def test_backend_config_file(): + inputs = action_inputs( + path='/test/terraform', + backend_config_file='backend.tf' + ) - setup_comment(monkeypatch, - path='/test/terraform', - backend_config_file='backend.tf' - ) - comment = TerraformComment() - comment.plan = 'Hello, this is my plan' - comment.status = 'Testing' + status = 'Testing' expected = '''Terraform plan in __/test/terraform__ With backend config files: `backend.tf` +
+Plan: 1 to add, 0 to change, 0 to destroy. + ```hcl -Hello, this is my plan +An execution plan has been generated and is shown below. +... +Plan: 1 to add, 0 to change, 0 to destroy. ``` +
+ Testing''' - assert comment.body() == expected + assert format_body(inputs, plan, status, 10).splitlines() == expected.splitlines() -def test_all(monkeypatch): - setup_comment(monkeypatch, - path='/test/terraform', - workspace='test', - var='myvar=hello', - var_file='vars.tf', - backend_config='bucket=mybucket,password=secret', - backend_config_file='backend.tf' - ) - comment = TerraformComment() - comment.plan = 'Hello, this is my plan' - comment.status = 'Testing' +def test_all(): + inputs = action_inputs( + path='/test/terraform', + workspace='test', + var='myvar=hello', + var_file='vars.tf', + backend_config='bucket=mybucket,password=secret', + backend_config_file='backend.tf' + ) + + status = 'Testing' expected = '''Terraform plan in __/test/terraform__ in the __test__ workspace With backend config: `bucket=mybucket` With backend config files: `backend.tf` With vars: `myvar=hello` With var files: `vars.tf` +
+Plan: 1 to add, 0 to change, 0 to destroy. + ```hcl -Hello, this is my plan +An execution plan has been generated and is shown below. +... +Plan: 1 to add, 0 to change, 0 to destroy. ``` +
+ Testing''' - assert comment.body() == expected + assert format_body(inputs, plan, status, 10).splitlines() == expected.splitlines() -def test_label(monkeypatch): +def test_label(): + inputs = action_inputs( + path='/test/terraform', + workspace='test', + var='myvar=hello', + var_file='vars.tf', + backend_config='bucket=mybucket,password=secret', + backend_config_file='backend.tf', + label='test_label' + ) - setup_comment(monkeypatch, - path='/test/terraform', - workspace='test', - var='myvar=hello', - var_file='vars.tf', - backend_config='bucket=mybucket,password=secret', - backend_config_file='backend.tf', - label='test_label' - ) - comment = TerraformComment() - comment.plan = 'Hello, this is my plan' - comment.status = 'Testing' + status = 'Testing' expected = '''Terraform plan for __test_label__ +
+Plan: 1 to add, 0 to change, 0 to destroy. + ```hcl -Hello, this is my plan +An execution plan has been generated and is shown below. +... +Plan: 1 to add, 0 to change, 0 to destroy. ``` +
+ Testing''' - assert comment.body() == expected + assert format_body(inputs, plan, status, 10).splitlines() == expected.splitlines() -def test_summary_plan_11(monkeypatch): - setup_comment(monkeypatch, - path='/test/terraform', - ) - comment = TerraformComment() - comment.plan = '''An execution plan has been generated and is shown below. + +def test_summary_plan_11(): + plan = '''An execution plan has been generated and is shown below. Resource actions are indicated with the following symbols: + create @@ -233,14 +339,11 @@ def test_summary_plan_11(monkeypatch): ''' expected = 'Plan: 1 to add, 0 to change, 0 to destroy.' - assert comment.summary() == expected + assert create_summary(plan) == expected -def test_summary_plan_12(monkeypatch): - setup_comment(monkeypatch, - path='/test/terraform', - ) - comment = TerraformComment() - comment.plan = '''An execution plan has been generated and is shown below. + +def test_summary_plan_12(): + plan = '''An execution plan has been generated and is shown below. Resource actions are indicated with the following symbols: + create @@ -265,14 +368,11 @@ def test_summary_plan_12(monkeypatch): ''' expected = 'Plan: 1 to add, 0 to change, 0 to destroy.' - assert comment.summary() == expected + assert create_summary(plan) == expected -def test_summary_plan_14(monkeypatch): - setup_comment(monkeypatch, - path='/test/terraform', - ) - comment = TerraformComment() - comment.plan = '''An execution plan has been generated and is shown below. + +def test_summary_plan_14(): + plan = '''An execution plan has been generated and is shown below. Resource actions are indicated with the following symbols: + create @@ -300,27 +400,21 @@ def test_summary_plan_14(monkeypatch): ''' expected = 'Plan: 1 to add, 0 to change, 0 to destroy. Changes to Outputs.' - assert comment.summary() == expected + assert create_summary(plan) == expected -def test_summary_error_11(monkeypatch): - setup_comment(monkeypatch, - path='/test/terraform', - ) - comment = TerraformComment() - comment.plan = """ + +def test_summary_error_11(): + plan = """ Error: random_string.my_string: length: cannot parse '' as int: strconv.ParseInt: parsing "ten": invalid syntax """ expected = "Error: random_string.my_string: length: cannot parse '' as int: strconv.ParseInt: parsing \"ten\": invalid syntax" - assert comment.summary() == expected + assert create_summary(plan) == expected -def test_summary_error_12(monkeypatch): - setup_comment(monkeypatch, - path='/test/terraform', - ) - comment = TerraformComment() - comment.plan = """ + +def test_summary_error_12(): + plan = """ Error: Incorrect attribute value type on main.tf line 2, in resource "random_string" "my_string": @@ -330,15 +424,11 @@ def test_summary_error_12(monkeypatch): """ expected = "Error: Incorrect attribute value type" - assert comment.summary() == expected + assert create_summary(plan) == expected -def test_summary_no_change_11(monkeypatch): - setup_comment(monkeypatch, - path='/test/terraform', - ) - comment = TerraformComment() - comment.plan = """No changes. Infrastructure is up-to-date. +def test_summary_no_change_11(): + plan = """No changes. Infrastructure is up-to-date. This means that Terraform did not detect any differences between your configuration and real physical resources that exist. As a result, no @@ -346,14 +436,11 @@ def test_summary_no_change_11(monkeypatch): """ expected = "No changes. Infrastructure is up-to-date." - assert comment.summary() == expected + assert create_summary(plan) == expected -def test_summary_no_change_14(monkeypatch): - setup_comment(monkeypatch, - path='/test/terraform', - ) - comment = TerraformComment() - comment.plan = """No changes. Infrastructure is up-to-date. + +def test_summary_no_change_14(): + plan = """No changes. Infrastructure is up-to-date. This means that Terraform did not detect any differences between your configuration and real physical resources that exist. As a result, no @@ -361,14 +448,11 @@ def test_summary_no_change_14(monkeypatch): """ expected = "No changes. Infrastructure is up-to-date." - assert comment.summary() == expected + assert create_summary(plan) == expected -def test_summary_output_only_change_14(monkeypatch): - setup_comment(monkeypatch, - path='/test/terraform', - ) - comment = TerraformComment() - comment.plan = """An execution plan has been generated and is shown below. + +def test_summary_output_only_change_14(): + plan = """An execution plan has been generated and is shown below. Resource actions are indicated with the following symbols: Terraform will perform the following actions: @@ -381,17 +465,11 @@ def test_summary_output_only_change_14(monkeypatch): """ expected = "Plan: 0 to add, 0 to change, 0 to destroy. Changes to Outputs." - assert comment.summary() == expected + assert create_summary(plan) == expected -def test_summary_unknown(monkeypatch): - setup_comment(monkeypatch, - path='/test/terraform', - ) - comment = TerraformComment() - comment.plan = """ + +def test_summary_unknown(): + plan = """ This is not anything like terraform output we know. We don't want to generate a summary for this. """ - - expected = None - assert comment.summary() == expected - + assert create_summary(plan) is None From 2ea412e3dcfbab24a2581d2ad1aedc2b7f3467d5 Mon Sep 17 00:00:00 2001 From: Daniel Flook Date: Mon, 25 Oct 2021 09:12:34 +0100 Subject: [PATCH 2/2] Cache PR comment info This terraform-apply makes far fewer github API requests --- image/entrypoints/plan.sh | 1 - image/tools/github_pr_comment.py | 118 ++++++++++++++++++++++++------- 2 files changed, 91 insertions(+), 28 deletions(-) diff --git a/image/entrypoints/plan.sh b/image/entrypoints/plan.sh index 20afb668..17554735 100755 --- a/image/entrypoints/plan.sh +++ b/image/entrypoints/plan.sh @@ -26,7 +26,6 @@ if [[ $PLAN_EXIT -eq 1 ]]; then fi fi -cat "$STEP_TMP_DIR/plan.txt" cat "$STEP_TMP_DIR/terraform_plan.stderr" if [[ "$GITHUB_EVENT_NAME" == "pull_request" || "$GITHUB_EVENT_NAME" == "issue_comment" || "$GITHUB_EVENT_NAME" == "pull_request_review_comment" || "$GITHUB_EVENT_NAME" == "pull_request_target" || "$GITHUB_EVENT_NAME" == "pull_request_review" ]]; then diff --git a/image/tools/github_pr_comment.py b/image/tools/github_pr_comment.py index 93c2fd98..4bb508a4 100755 --- a/image/tools/github_pr_comment.py +++ b/image/tools/github_pr_comment.py @@ -19,6 +19,9 @@ class GitHubActionsEnv(TypedDict): + """ + Environment variables that are set by the actions runner + """ GITHUB_API_URL: str GITHUB_TOKEN: str GITHUB_EVENT_PATH: str @@ -28,11 +31,15 @@ class GitHubActionsEnv(TypedDict): job_tmp_dir = os.environ.get('JOB_TMP_DIR', '.') +step_tmp_dir = os.environ.get('STEP_TMP_DIR', '.') env = cast(GitHubActionsEnv, os.environ) def github_session(github_env: GitHubActionsEnv) -> requests.Session: + """ + A request session that is configured for the github API + """ session = requests.Session() session.headers['authorization'] = f'token {github_env["GITHUB_TOKEN"]}' session.headers['user-agent'] = 'terraform-github-actions' @@ -45,7 +52,7 @@ def github_session(github_env: GitHubActionsEnv) -> requests.Session: class ActionInputs(TypedDict): """ - Information used to identify a plan + Actions input environment variables that are set by the runner """ INPUT_BACKEND_CONFIG: str INPUT_BACKEND_CONFIG_FILE: str @@ -60,8 +67,6 @@ class ActionInputs(TypedDict): def plan_identifier(action_inputs: ActionInputs) -> str: def mask_backend_config() -> Optional[str]: - if action_inputs.get('INPUT_BACKEND_CONFIG') is None: - return None bad_words = [ 'token', @@ -181,7 +186,7 @@ def paginate(url: GitHubUrl, *args, **kwargs) -> Iterable[Dict[str, Any]]: return -def find_pr(env: GitHubActionsEnv) -> PrUrl: +def find_pr(actions_env: GitHubActionsEnv) -> PrUrl: """ Find the pull request this event is related to @@ -190,10 +195,10 @@ def find_pr(env: GitHubActionsEnv) -> PrUrl: """ - with open(env['GITHUB_EVENT_PATH']) as f: + with open(actions_env['GITHUB_EVENT_PATH']) as f: event = json.load(f) - event_type = env['GITHUB_EVENT_NAME'] + event_type = actions_env['GITHUB_EVENT_NAME'] if event_type in ['pull_request', 'pull_request_review_comment', 'pull_request_target', 'pull_request_review']: return cast(PrUrl, event['pull_request']['url']) @@ -206,11 +211,11 @@ def find_pr(env: GitHubActionsEnv) -> PrUrl: raise Exception('This comment is not for a PR. Add a filter of `if: github.event.issue.pull_request`') elif event_type == 'push': - repo = env['GITHUB_REPOSITORY'] - commit = env['GITHUB_SHA'] + repo = actions_env['GITHUB_REPOSITORY'] + commit = actions_env['GITHUB_SHA'] def prs() -> Iterable[Dict[str, Any]]: - url = f'{env.get("GITHUB_API_URL", "https://api.github.com")}/repos/{repo}/pulls' + url = f'{actions_env["GITHUB_API_URL"]}/repos/{repo}/pulls' yield from paginate(cast(PrUrl, url), params={'state': 'all'}) for pr in prs(): @@ -223,21 +228,20 @@ def prs() -> Iterable[Dict[str, Any]]: raise Exception(f"The {event_type} event doesn\'t relate to a Pull Request.") -def current_user(github_env: GitHubActionsEnv) -> str: - token_hash = hashlib.sha256(github_env['GITHUB_TOKEN'].encode()).hexdigest() +def current_user(actions_env: GitHubActionsEnv) -> str: + token_hash = hashlib.sha256(actions_env['GITHUB_TOKEN'].encode()).hexdigest() try: with open(os.path.join(job_tmp_dir, 'token-cache', token_hash)) as f: username = f.read() - debug(f'GITHUB_TOKEN username: {username}') + debug(f'GITHUB_TOKEN username from token-cache: {username}') return username except Exception as e: debug(str(e)) - response = github_api_request('get', f'{github_env["GITHUB_API_URL"]}/user') + response = github_api_request('get', f'{actions_env["GITHUB_API_URL"]}/user') if response.status_code != 403: user = response.json() - debug('GITHUB_TOKEN user:') debug(json.dumps(user)) username = user['login'] @@ -252,7 +256,7 @@ def current_user(github_env: GitHubActionsEnv) -> str: except Exception as e: debug(str(e)) - debug(f'GITHUB_TOKEN username: {username}') + debug(f'discovered GITHUB_TOKEN username: {username}') return username @@ -322,8 +326,6 @@ def update_comment(issue_url: IssueUrl, :param only_if_exists: Only update an existing comment - don't create it """ - debug(body) - if comment_url is None: if only_if_exists: debug('Comment doesn\'t already exist - not creating it') @@ -336,20 +338,40 @@ def update_comment(issue_url: IssueUrl, debug('Updating existing comment') response = github_api_request('patch', comment_url, json={'body': body}) + debug(body) debug(response.content.decode()) response.raise_for_status() return cast(CommentUrl, response.json()['url']) def find_issue_url(pr_url: str) -> IssueUrl: + pr_hash = hashlib.sha256(pr_url.encode()).hexdigest() + + try: + with open(os.path.join(job_tmp_dir, 'issue-href-cache', pr_hash)) as f: + issue_url = f.read() + debug(f'issue_url from issue-href-cache: {issue_url}') + return cast(IssueUrl, issue_url) + except Exception as e: + debug(str(e)) + response = github_api_request('get', pr_url) response.raise_for_status() - return cast(IssueUrl, response.json()['_links']['issue']['href'] + '/comments') + issue_url = cast(IssueUrl, response.json()['_links']['issue']['href'] + '/comments') + + try: + os.makedirs(os.path.join(job_tmp_dir, 'issue-href-cache'), exist_ok=True) + with open(os.path.join(job_tmp_dir, 'issue-href-cache', pr_hash), 'w') as f: + f.write(issue_url) + except Exception as e: + debug(str(e)) + + debug(f'discovered issue_url: {issue_url}') + return cast(IssueUrl, issue_url) -def find_comment(issue_url: IssueUrl, username: str, action_inputs: ActionInputs) -> Tuple[ - Optional[CommentUrl], Optional[Plan]]: +def find_comment(issue_url: IssueUrl, username: str, action_inputs: ActionInputs) -> Tuple[Optional[CommentUrl], Optional[Plan]]: debug('Looking for an existing comment:') plan_id = plan_identifier(action_inputs) @@ -364,6 +386,22 @@ def find_comment(issue_url: IssueUrl, username: str, action_inputs: ActionInputs return None, None +def read_step_cache() -> Dict[str, str]: + try: + with open(os.path.join(step_tmp_dir, 'github_pr_comment.cache')) as f: + debug('step cache loaded') + return json.load(f) + except Exception as e: + debug(str(e)) + return {} + +def save_step_cache(**kwargs) -> None: + try: + with open(os.path.join(step_tmp_dir, 'github_pr_comment.cache'), 'w') as f: + json.dump(kwargs, f) + debug('step cache saved') + except Exception as e: + debug(str(e)) def main() -> None: if len(sys.argv) < 2: @@ -372,6 +410,8 @@ def main() -> None: STATUS="" {sys.argv[0]} status {sys.argv[0]} get >plan.txt''') + debug(repr(sys.argv)) + action_inputs = cast(ActionInputs, os.environ) try: @@ -379,10 +419,34 @@ def main() -> None: except (ValueError, KeyError): collapse_threshold = 10 - pr_url = find_pr(env) - issue_url = find_issue_url(pr_url) + step_cache = read_step_cache() + + if step_cache.get('pr_url') is not None: + pr_url = step_cache['pr_url'] + debug(f'pr_url from step cache: {pr_url}') + else: + pr_url = find_pr(env) + debug(f'discovered pr_url: {pr_url}') + + if step_cache.get('pr_url') == pr_url and step_cache.get('issue_url') is not None: + issue_url = step_cache['issue_url'] + debug(f'issue_url from step cache: {issue_url}') + else: + issue_url = find_issue_url(pr_url) + + # Username is cached in the job tmp dir username = current_user(env) - comment_url, plan = find_comment(issue_url, username, action_inputs) + + if step_cache.get('comment_url') is not None and step_cache.get('plan') is not None: + comment_url = step_cache['comment_url'] + plan = step_cache['plan'] + debug(f'comment_url from step cache: {comment_url}') + debug(f'plan from step cache: {plan}') + else: + comment_url, plan = find_comment(issue_url, username, action_inputs) + debug(f'discovered comment_url: {comment_url}') + debug(f'discovered plan: {plan}') + status = cast(Status, os.environ.get('STATUS', '')) only_if_exists = False @@ -390,19 +454,18 @@ def main() -> None: if sys.argv[1] == 'plan': plan = cast(Plan, sys.stdin.read().strip()) - if action_inputs['INPUT_ADD_GITHUB_COMMENT'] == 'changes-only' and os.environ.get('TF_CHANGES', - 'true') == 'false': + if action_inputs['INPUT_ADD_GITHUB_COMMENT'] == 'changes-only' and os.environ.get('TF_CHANGES', 'true') == 'false': only_if_exists = True body = format_body(action_inputs, plan, status, collapse_threshold) - update_comment(issue_url, comment_url, body, only_if_exists) + comment_url = update_comment(issue_url, comment_url, body, only_if_exists) elif sys.argv[1] == 'status': if plan is None: exit(1) else: body = format_body(action_inputs, plan, status, collapse_threshold) - update_comment(issue_url, comment_url, body, only_if_exists) + comment_url = update_comment(issue_url, comment_url, body, only_if_exists) elif sys.argv[1] == 'get': if plan is None: @@ -411,6 +474,7 @@ def main() -> None: with open(sys.argv[2], 'w') as f: f.write(plan) + save_step_cache(pr_url=pr_url, issue_url=issue_url, comment_url=comment_url, plan=plan) if __name__ == '__main__': main()