From 879c018bb67cc37b43e4eaab84b7ccf8ba70073e Mon Sep 17 00:00:00 2001 From: James Leopold Date: Mon, 14 Jun 2021 09:39:48 -0400 Subject: [PATCH 01/33] wip --- CHANGES.rst | 6 ++++++ manheim_c7n_tools/runner.py | 2 +- manheim_c7n_tools/version.py | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index d12530c..9b0e7c9 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,6 +1,12 @@ Changelog ========= + +1.3.1 (2021-06-14) +------------------ + +* Fixing dryrun-diff bug to show changes to nested policies + 1.3.0 (2021-01-13) ------------------ diff --git a/manheim_c7n_tools/runner.py b/manheim_c7n_tools/runner.py index e326572..ff84fe0 100644 --- a/manheim_c7n_tools/runner.py +++ b/manheim_c7n_tools/runner.py @@ -360,7 +360,7 @@ def dryrun(self): @staticmethod def run_in_region(region_name, conf): - return region_name == conf.regions[-1] + return True class S3ArchiverStep(BaseStep): diff --git a/manheim_c7n_tools/version.py b/manheim_c7n_tools/version.py index 7bc071f..e0439a8 100644 --- a/manheim_c7n_tools/version.py +++ b/manheim_c7n_tools/version.py @@ -17,7 +17,7 @@ """ #: The semver-compliant version of the package. -VERSION = '1.3.0' +VERSION = '1.3.1' #: The URL for further information about the package. PROJECT_URL = 'https://github.com/manheim/manheim-c7n-tools' From dd7388ce448013128e316fef90dd4f333d581828 Mon Sep 17 00:00:00 2001 From: James Leopold Date: Mon, 14 Jun 2021 09:47:56 -0400 Subject: [PATCH 02/33] update tests --- manheim_c7n_tools/tests/test_runner.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/manheim_c7n_tools/tests/test_runner.py b/manheim_c7n_tools/tests/test_runner.py index bbdac93..00b5033 100644 --- a/manheim_c7n_tools/tests/test_runner.py +++ b/manheim_c7n_tools/tests/test_runner.py @@ -702,15 +702,7 @@ def test_run_in_region(self): return_value=ALL_REGIONS ) for rname in ALL_REGIONS: - if rname == 'us-west-2': - assert runner.DryRunDiffStep.run_in_region( - rname, self.m_conf - ) is True - else: - assert runner.DryRunDiffStep.run_in_region( - rname, self.m_conf - ) is False - + assert runner.DryRunDiffStep.run_in_region(rname, self.m_conf) is True class TestS3ArchiverStep(StepTester): From 42569fce36915cb40bc067c18cacf85a03b91ccd Mon Sep 17 00:00:00 2001 From: James Leopold Date: Mon, 14 Jun 2021 10:20:07 -0400 Subject: [PATCH 03/33] changelog --- CHANGES.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index 9b0e7c9..b2a2933 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -5,7 +5,7 @@ Changelog 1.3.1 (2021-06-14) ------------------ -* Fixing dryrun-diff bug to show changes to nested policies +* Fixing dryrun-diff bug to show changes to nested policies. 1.3.0 (2021-01-13) ------------------ From 962dfd03848e97bdd487f5182aac07cf958935d7 Mon Sep 17 00:00:00 2001 From: James Leopold Date: Mon, 14 Jun 2021 10:27:06 -0400 Subject: [PATCH 04/33] revert --- manheim_c7n_tools/runner.py | 2 +- manheim_c7n_tools/tests/test_runner.py | 10 +++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/manheim_c7n_tools/runner.py b/manheim_c7n_tools/runner.py index ff84fe0..e326572 100644 --- a/manheim_c7n_tools/runner.py +++ b/manheim_c7n_tools/runner.py @@ -360,7 +360,7 @@ def dryrun(self): @staticmethod def run_in_region(region_name, conf): - return True + return region_name == conf.regions[-1] class S3ArchiverStep(BaseStep): diff --git a/manheim_c7n_tools/tests/test_runner.py b/manheim_c7n_tools/tests/test_runner.py index 00b5033..bbdac93 100644 --- a/manheim_c7n_tools/tests/test_runner.py +++ b/manheim_c7n_tools/tests/test_runner.py @@ -702,7 +702,15 @@ def test_run_in_region(self): return_value=ALL_REGIONS ) for rname in ALL_REGIONS: - assert runner.DryRunDiffStep.run_in_region(rname, self.m_conf) is True + if rname == 'us-west-2': + assert runner.DryRunDiffStep.run_in_region( + rname, self.m_conf + ) is True + else: + assert runner.DryRunDiffStep.run_in_region( + rname, self.m_conf + ) is False + class TestS3ArchiverStep(StepTester): From 752dfe9d71444c157df723adc5a60b136187afbe Mon Sep 17 00:00:00 2001 From: James Leopold Date: Mon, 14 Jun 2021 11:10:04 -0400 Subject: [PATCH 05/33] wip --- manheim_c7n_tools/dryrun_diff.py | 22 ++++++++++++++++++++++ manheim_c7n_tools/version.py | 2 +- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/manheim_c7n_tools/dryrun_diff.py b/manheim_c7n_tools/dryrun_diff.py index 1d90f49..4ac93c0 100755 --- a/manheim_c7n_tools/dryrun_diff.py +++ b/manheim_c7n_tools/dryrun_diff.py @@ -100,10 +100,32 @@ def _find_changed_policies(self, git_dir=None, diff_against='master'): :return: list of policy names that differ from master :rtype: list """ + + copy_gitignore = subprocess.check_output( + ['mv', '.gitignore', '.gitignoreCOPY'], + cwd=git_dir + ).decode().split("\n") + + git_add_policies = subprocess.check_output( + ['git', 'add', '--all', '-N', 'policies'], + cwd=git_dir + ).decode().split("\n") + res = subprocess.check_output( ['git', 'diff', '--name-only', diff_against], cwd=git_dir ).decode().split("\n") + + revert_gitignore = subprocess.check_output( + ['mv', '.gitignoreCOPY', '.gitignore'], + cwd=git_dir + ).decode().split("\n") + + git_reset = subprocess.check_output( + ['git', 'reset'], + cwd=git_dir + ).decode().split("\n") + pnames = [] polname_re = re.compile(r'^policies.*/([a-zA-Z0-9_-]+)\.yml$') for x in res: diff --git a/manheim_c7n_tools/version.py b/manheim_c7n_tools/version.py index e0439a8..7bc071f 100644 --- a/manheim_c7n_tools/version.py +++ b/manheim_c7n_tools/version.py @@ -17,7 +17,7 @@ """ #: The semver-compliant version of the package. -VERSION = '1.3.1' +VERSION = '1.3.0' #: The URL for further information about the package. PROJECT_URL = 'https://github.com/manheim/manheim-c7n-tools' From b48a45641f0a6841931ff910add29ab9a66c0a42 Mon Sep 17 00:00:00 2001 From: James Leopold Date: Mon, 14 Jun 2021 11:15:06 -0400 Subject: [PATCH 06/33] fix spacing --- manheim_c7n_tools/dryrun_diff.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manheim_c7n_tools/dryrun_diff.py b/manheim_c7n_tools/dryrun_diff.py index 4ac93c0..134c534 100755 --- a/manheim_c7n_tools/dryrun_diff.py +++ b/manheim_c7n_tools/dryrun_diff.py @@ -107,7 +107,7 @@ def _find_changed_policies(self, git_dir=None, diff_against='master'): ).decode().split("\n") git_add_policies = subprocess.check_output( - ['git', 'add', '--all', '-N', 'policies'], + ['git', 'add', '--all', '-N', 'policies'], cwd=git_dir ).decode().split("\n") From 5eb0e75bff972cb6a43057890946b9c7b49adc77 Mon Sep 17 00:00:00 2001 From: James Leopold Date: Mon, 14 Jun 2021 11:19:24 -0400 Subject: [PATCH 07/33] use run --- manheim_c7n_tools/dryrun_diff.py | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/manheim_c7n_tools/dryrun_diff.py b/manheim_c7n_tools/dryrun_diff.py index 134c534..5bfe09d 100755 --- a/manheim_c7n_tools/dryrun_diff.py +++ b/manheim_c7n_tools/dryrun_diff.py @@ -101,30 +101,16 @@ def _find_changed_policies(self, git_dir=None, diff_against='master'): :rtype: list """ - copy_gitignore = subprocess.check_output( - ['mv', '.gitignore', '.gitignoreCOPY'], - cwd=git_dir - ).decode().split("\n") - - git_add_policies = subprocess.check_output( - ['git', 'add', '--all', '-N', 'policies'], - cwd=git_dir - ).decode().split("\n") + subprocess.run(['mv', '.gitignore', '.gitignoreCOPY'], cwd=git_dir) + subprocess.run(['git', 'add', '--all', '-N', 'policies'],cwd=git_dir) res = subprocess.check_output( ['git', 'diff', '--name-only', diff_against], cwd=git_dir ).decode().split("\n") - revert_gitignore = subprocess.check_output( - ['mv', '.gitignoreCOPY', '.gitignore'], - cwd=git_dir - ).decode().split("\n") - - git_reset = subprocess.check_output( - ['git', 'reset'], - cwd=git_dir - ).decode().split("\n") + subprocess.run(['mv', '.gitignoreCOPY', '.gitignore'], cwd=git_dir) + subprocess.run(['git', 'reset'], cwd=git_dir) pnames = [] polname_re = re.compile(r'^policies.*/([a-zA-Z0-9_-]+)\.yml$') From 6358f8f8e8237bd6fb197c8f27548c7aff9bbe0d Mon Sep 17 00:00:00 2001 From: James Leopold Date: Mon, 14 Jun 2021 11:20:03 -0400 Subject: [PATCH 08/33] fix spacing --- manheim_c7n_tools/dryrun_diff.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/manheim_c7n_tools/dryrun_diff.py b/manheim_c7n_tools/dryrun_diff.py index 5bfe09d..ed5c3b0 100755 --- a/manheim_c7n_tools/dryrun_diff.py +++ b/manheim_c7n_tools/dryrun_diff.py @@ -102,13 +102,11 @@ def _find_changed_policies(self, git_dir=None, diff_against='master'): """ subprocess.run(['mv', '.gitignore', '.gitignoreCOPY'], cwd=git_dir) - subprocess.run(['git', 'add', '--all', '-N', 'policies'],cwd=git_dir) - + subprocess.run(['git', 'add', '--all', '-N', 'policies'], cwd=git_dir) res = subprocess.check_output( ['git', 'diff', '--name-only', diff_against], cwd=git_dir ).decode().split("\n") - subprocess.run(['mv', '.gitignoreCOPY', '.gitignore'], cwd=git_dir) subprocess.run(['git', 'reset'], cwd=git_dir) From fc44b1b3628b8091a7758f6455880ac3f4523fd5 Mon Sep 17 00:00:00 2001 From: James Leopold Date: Mon, 14 Jun 2021 11:50:43 -0400 Subject: [PATCH 09/33] Adding debug --- manheim_c7n_tools/dryrun_diff.py | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/manheim_c7n_tools/dryrun_diff.py b/manheim_c7n_tools/dryrun_diff.py index ed5c3b0..a96e179 100755 --- a/manheim_c7n_tools/dryrun_diff.py +++ b/manheim_c7n_tools/dryrun_diff.py @@ -101,17 +101,20 @@ def _find_changed_policies(self, git_dir=None, diff_against='master'): :rtype: list """ + logger.info('Running `mv .gitignore .gitignoreCOPY`') subprocess.run(['mv', '.gitignore', '.gitignoreCOPY'], cwd=git_dir) + + logger.info('Running `git add --all -N policies`') subprocess.run(['git', 'add', '--all', '-N', 'policies'], cwd=git_dir) + + logger.info(f'Running git diff --name-only {diff_against}') res = subprocess.check_output( ['git', 'diff', '--name-only', diff_against], cwd=git_dir ).decode().split("\n") - subprocess.run(['mv', '.gitignoreCOPY', '.gitignore'], cwd=git_dir) - subprocess.run(['git', 'reset'], cwd=git_dir) pnames = [] - polname_re = re.compile(r'^policies.*/([a-zA-Z0-9_-]+)\.yml$') + polname_re = re.compile(r'^policies.*\/([a-zA-Z0-9_-]+)\.yml$') for x in res: x = x.strip() if x == '': @@ -120,6 +123,14 @@ def _find_changed_policies(self, git_dir=None, diff_against='master'): if not m: continue pnames.append(m.group(1)) + + + logger.info('Running `mv .gitignoreCOPY .gitignore`') + subprocess.run(['mv', '.gitignoreCOPY', '.gitignore'], cwd=git_dir) + + logger.info('Running `git reset`') + subprocess.run(['git', 'reset'], cwd=git_dir) + return pnames def _make_diff_report(self, dryrun): From fa4b588ac2c446ba12956eaa71459292d7995ed9 Mon Sep 17 00:00:00 2001 From: James Leopold Date: Mon, 14 Jun 2021 12:35:26 -0400 Subject: [PATCH 10/33] setting debug comments --- manheim_c7n_tools/dryrun_diff.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/manheim_c7n_tools/dryrun_diff.py b/manheim_c7n_tools/dryrun_diff.py index a96e179..96fea9b 100755 --- a/manheim_c7n_tools/dryrun_diff.py +++ b/manheim_c7n_tools/dryrun_diff.py @@ -101,13 +101,13 @@ def _find_changed_policies(self, git_dir=None, diff_against='master'): :rtype: list """ - logger.info('Running `mv .gitignore .gitignoreCOPY`') + logger.debug('Running `mv .gitignore .gitignoreCOPY` (allows us to see diffs for nested policies)') subprocess.run(['mv', '.gitignore', '.gitignoreCOPY'], cwd=git_dir) - logger.info('Running `git add --all -N policies`') + logger.debug('Running `git add --all -N policies`. Using (-N) intent-to-add to see diffs for untracked files.') subprocess.run(['git', 'add', '--all', '-N', 'policies'], cwd=git_dir) - logger.info(f'Running git diff --name-only {diff_against}') + logger.debug(f'Running `git diff --name-only {diff_against}`') res = subprocess.check_output( ['git', 'diff', '--name-only', diff_against], cwd=git_dir @@ -125,10 +125,10 @@ def _find_changed_policies(self, git_dir=None, diff_against='master'): pnames.append(m.group(1)) - logger.info('Running `mv .gitignoreCOPY .gitignore`') + logger.debug('Running `mv .gitignoreCOPY .gitignore` to reset the .gitignore file') subprocess.run(['mv', '.gitignoreCOPY', '.gitignore'], cwd=git_dir) - logger.info('Running `git reset`') + logger.debug('Running `git reset`') subprocess.run(['git', 'reset'], cwd=git_dir) return pnames From ef3300096130b01a95ccac70ef0ef0f6ebfd17ca Mon Sep 17 00:00:00 2001 From: James Leopold Date: Mon, 14 Jun 2021 13:30:51 -0400 Subject: [PATCH 11/33] refafctor logic --- manheim_c7n_tools/dryrun_diff.py | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/manheim_c7n_tools/dryrun_diff.py b/manheim_c7n_tools/dryrun_diff.py index 96fea9b..d3167cf 100755 --- a/manheim_c7n_tools/dryrun_diff.py +++ b/manheim_c7n_tools/dryrun_diff.py @@ -60,6 +60,17 @@ def __init__(self, config): def run(self, git_dir=None, diff_against='master'): changed_policies = self._find_changed_policies(git_dir, diff_against) + + # Loop over the 'parent' source paths (not the last element) + source_paths = self.config.policy_source_paths + parent_source_paths = source_paths[:-1] + for source_path in parent_source_paths: + if git_dir == None: + changed_policies.append(self._find_changed_policies(f'./policies/{source_path}', diff_against)) + else: + changed_policies.append(self._find_changed_policies(f'{git_dir}/policies/{source_path}', diff_against)) + + if len(changed_policies) == 0: logger.info( 'Git diff did not report any changed policies; skipping ' @@ -100,19 +111,10 @@ def _find_changed_policies(self, git_dir=None, diff_against='master'): :return: list of policy names that differ from master :rtype: list """ - - logger.debug('Running `mv .gitignore .gitignoreCOPY` (allows us to see diffs for nested policies)') - subprocess.run(['mv', '.gitignore', '.gitignoreCOPY'], cwd=git_dir) - - logger.debug('Running `git add --all -N policies`. Using (-N) intent-to-add to see diffs for untracked files.') - subprocess.run(['git', 'add', '--all', '-N', 'policies'], cwd=git_dir) - - logger.debug(f'Running `git diff --name-only {diff_against}`') res = subprocess.check_output( ['git', 'diff', '--name-only', diff_against], cwd=git_dir ).decode().split("\n") - pnames = [] polname_re = re.compile(r'^policies.*\/([a-zA-Z0-9_-]+)\.yml$') for x in res: @@ -123,14 +125,6 @@ def _find_changed_policies(self, git_dir=None, diff_against='master'): if not m: continue pnames.append(m.group(1)) - - - logger.debug('Running `mv .gitignoreCOPY .gitignore` to reset the .gitignore file') - subprocess.run(['mv', '.gitignoreCOPY', '.gitignore'], cwd=git_dir) - - logger.debug('Running `git reset`') - subprocess.run(['git', 'reset'], cwd=git_dir) - return pnames def _make_diff_report(self, dryrun): From af300bbe2164654d4ee0b94a908e442f0682003f Mon Sep 17 00:00:00 2001 From: James Leopold Date: Mon, 14 Jun 2021 13:33:13 -0400 Subject: [PATCH 12/33] style changesg --- manheim_c7n_tools/dryrun_diff.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/manheim_c7n_tools/dryrun_diff.py b/manheim_c7n_tools/dryrun_diff.py index d3167cf..ed36110 100755 --- a/manheim_c7n_tools/dryrun_diff.py +++ b/manheim_c7n_tools/dryrun_diff.py @@ -65,10 +65,12 @@ def run(self, git_dir=None, diff_against='master'): source_paths = self.config.policy_source_paths parent_source_paths = source_paths[:-1] for source_path in parent_source_paths: - if git_dir == None: - changed_policies.append(self._find_changed_policies(f'./policies/{source_path}', diff_against)) + if git_dir is None: + changed_policies.append( + self._find_changed_policies(f'./policies/{source_path}', diff_against)) else: - changed_policies.append(self._find_changed_policies(f'{git_dir}/policies/{source_path}', diff_against)) + changed_policies.append( + self._find_changed_policies(f'{git_dir}/policies/{source_path}', diff_against)) if len(changed_policies) == 0: From 5541fa692d972869df33343522566a4fc68ea8f5 Mon Sep 17 00:00:00 2001 From: James Leopold Date: Mon, 14 Jun 2021 13:40:58 -0400 Subject: [PATCH 13/33] cleanup lines too long --- manheim_c7n_tools/dryrun_diff.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/manheim_c7n_tools/dryrun_diff.py b/manheim_c7n_tools/dryrun_diff.py index ed36110..a46e91c 100755 --- a/manheim_c7n_tools/dryrun_diff.py +++ b/manheim_c7n_tools/dryrun_diff.py @@ -67,10 +67,18 @@ def run(self, git_dir=None, diff_against='master'): for source_path in parent_source_paths: if git_dir is None: changed_policies.append( - self._find_changed_policies(f'./policies/{source_path}', diff_against)) + self._find_changed_policies( + f'./policies/{source_path}', + diff_against + ) + ) else: changed_policies.append( - self._find_changed_policies(f'{git_dir}/policies/{source_path}', diff_against)) + self._find_changed_policies( + f'{git_dir}/policies/{source_path}', + diff_against + ) + ) if len(changed_policies) == 0: From 10b81bfe4f0f6952060c696a7ecaf277d55c484b Mon Sep 17 00:00:00 2001 From: James Leopold Date: Mon, 14 Jun 2021 13:47:03 -0400 Subject: [PATCH 14/33] remove blank lines --- manheim_c7n_tools/dryrun_diff.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/manheim_c7n_tools/dryrun_diff.py b/manheim_c7n_tools/dryrun_diff.py index a46e91c..787ebb0 100755 --- a/manheim_c7n_tools/dryrun_diff.py +++ b/manheim_c7n_tools/dryrun_diff.py @@ -79,8 +79,6 @@ def run(self, git_dir=None, diff_against='master'): diff_against ) ) - - if len(changed_policies) == 0: logger.info( 'Git diff did not report any changed policies; skipping ' From 1ca8706cf24399b6e821917abbfc4b63a68ff158 Mon Sep 17 00:00:00 2001 From: James Leopold Date: Mon, 14 Jun 2021 14:26:31 -0400 Subject: [PATCH 15/33] update logid --- manheim_c7n_tools/dryrun_diff.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/manheim_c7n_tools/dryrun_diff.py b/manheim_c7n_tools/dryrun_diff.py index 787ebb0..af680bb 100755 --- a/manheim_c7n_tools/dryrun_diff.py +++ b/manheim_c7n_tools/dryrun_diff.py @@ -60,24 +60,23 @@ def __init__(self, config): def run(self, git_dir=None, diff_against='master'): changed_policies = self._find_changed_policies(git_dir, diff_against) - - # Loop over the 'parent' source paths (not the last element) source_paths = self.config.policy_source_paths parent_source_paths = source_paths[:-1] - for source_path in parent_source_paths: - if git_dir is None: + for p in parent_source_paths: + # Assumes these policies are checked out into ./policies// + sub_policy_path = f'policies/{p}' + if os.path.isdir(sub_policy_path): changed_policies.append( self._find_changed_policies( - f'./policies/{source_path}', + sub_policy_path, diff_against ) ) else: - changed_policies.append( - self._find_changed_policies( - f'{git_dir}/policies/{source_path}', - diff_against - ) + logger.warning( + f'{sub_policy_path} is defined in `policy_source_paths` ' + 'but is not checked out into a seperate directory. ' + 'Dryrun-diff results will be incomplete until this is resolved!' ) if len(changed_policies) == 0: logger.info( From 22e71d351641e1d44d113cba67c96e490548e49f Mon Sep 17 00:00:00 2001 From: James Leopold Date: Mon, 14 Jun 2021 14:28:48 -0400 Subject: [PATCH 16/33] update line len --- manheim_c7n_tools/dryrun_diff.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manheim_c7n_tools/dryrun_diff.py b/manheim_c7n_tools/dryrun_diff.py index af680bb..109dec1 100755 --- a/manheim_c7n_tools/dryrun_diff.py +++ b/manheim_c7n_tools/dryrun_diff.py @@ -76,7 +76,7 @@ def run(self, git_dir=None, diff_against='master'): logger.warning( f'{sub_policy_path} is defined in `policy_source_paths` ' 'but is not checked out into a seperate directory. ' - 'Dryrun-diff results will be incomplete until this is resolved!' + 'dryrun-diff results will be incomplete!' ) if len(changed_policies) == 0: logger.info( From 5a710ca23f007cb48a4a5780a90a087d511b83ab Mon Sep 17 00:00:00 2001 From: James Leopold Date: Mon, 14 Jun 2021 14:29:31 -0400 Subject: [PATCH 17/33] fix --- manheim_c7n_tools/dryrun_diff.py | 1 - 1 file changed, 1 deletion(-) diff --git a/manheim_c7n_tools/dryrun_diff.py b/manheim_c7n_tools/dryrun_diff.py index 109dec1..b674c0f 100755 --- a/manheim_c7n_tools/dryrun_diff.py +++ b/manheim_c7n_tools/dryrun_diff.py @@ -63,7 +63,6 @@ def run(self, git_dir=None, diff_against='master'): source_paths = self.config.policy_source_paths parent_source_paths = source_paths[:-1] for p in parent_source_paths: - # Assumes these policies are checked out into ./policies// sub_policy_path = f'policies/{p}' if os.path.isdir(sub_policy_path): changed_policies.append( From 81316377015a50970cc2d1c212c54e652c7c2e39 Mon Sep 17 00:00:00 2001 From: James Leopold Date: Mon, 14 Jun 2021 15:27:34 -0400 Subject: [PATCH 18/33] improvements --- manheim_c7n_tools/dryrun_diff.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/manheim_c7n_tools/dryrun_diff.py b/manheim_c7n_tools/dryrun_diff.py index b674c0f..104c09c 100755 --- a/manheim_c7n_tools/dryrun_diff.py +++ b/manheim_c7n_tools/dryrun_diff.py @@ -65,10 +65,11 @@ def run(self, git_dir=None, diff_against='master'): for p in parent_source_paths: sub_policy_path = f'policies/{p}' if os.path.isdir(sub_policy_path): - changed_policies.append( + changed_policies.extend( self._find_changed_policies( sub_policy_path, - diff_against + diff_against, + parent_policy=True ) ) else: @@ -112,24 +113,30 @@ def run(self, git_dir=None, diff_against='master'): fh.write(diff_report) logger.info('PR report written to: pr_report.html') - def _find_changed_policies(self, git_dir=None, diff_against='master'): + def _find_changed_policies(self, git_dir=None, diff_against='master', parent_policy=False): """ :return: list of policy names that differ from master :rtype: list """ + logger.info(f'Running _find_changed_policies with {git_dir}, {diff_against}, {parent_policy}') res = subprocess.check_output( ['git', 'diff', '--name-only', diff_against], cwd=git_dir ).decode().split("\n") pnames = [] - polname_re = re.compile(r'^policies.*\/([a-zA-Z0-9_-]+)\.yml$') + if parent_policy: + polname_re = re.compile(r'^.*\/([a-zA-Z0-9_-]+)\.yml$') + else: + polname_re = re.compile(r'^policies.*\/([a-zA-Z0-9_-]+)\.yml$') for x in res: x = x.strip() if x == '': continue + logger.info(f'x: {x}') m = polname_re.match(x) if not m: continue + pnames.append(m.group(1)) return pnames From 0333a93d980f08b4d7e3c132378388e82607ce2a Mon Sep 17 00:00:00 2001 From: James Leopold Date: Wed, 16 Jun 2021 08:59:42 -0400 Subject: [PATCH 19/33] wip --- manheim_c7n_tools/dryrun_diff.py | 39 +++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/manheim_c7n_tools/dryrun_diff.py b/manheim_c7n_tools/dryrun_diff.py index 104c09c..5cd90c3 100755 --- a/manheim_c7n_tools/dryrun_diff.py +++ b/manheim_c7n_tools/dryrun_diff.py @@ -66,11 +66,7 @@ def run(self, git_dir=None, diff_against='master'): sub_policy_path = f'policies/{p}' if os.path.isdir(sub_policy_path): changed_policies.extend( - self._find_changed_policies( - sub_policy_path, - diff_against, - parent_policy=True - ) + self._get_inherited_policies(sub_policy_path) ) else: logger.warning( @@ -113,21 +109,42 @@ def run(self, git_dir=None, diff_against='master'): fh.write(diff_report) logger.info('PR report written to: pr_report.html') - def _find_changed_policies(self, git_dir=None, diff_against='master', parent_policy=False): + def _find_changed_policies(self, git_dir=None, diff_against='master'): """ :return: list of policy names that differ from master :rtype: list """ - logger.info(f'Running _find_changed_policies with {git_dir}, {diff_against}, {parent_policy}') + logger.info(f'Running _find_changed_policies with {git_dir}, {diff_against}') res = subprocess.check_output( ['git', 'diff', '--name-only', diff_against], cwd=git_dir ).decode().split("\n") pnames = [] - if parent_policy: - polname_re = re.compile(r'^.*\/([a-zA-Z0-9_-]+)\.yml$') - else: - polname_re = re.compile(r'^policies.*\/([a-zA-Z0-9_-]+)\.yml$') + polname_re = re.compile(r'^policies.*\/([a-zA-Z0-9_-]+)\.yml$') + for x in res: + x = x.strip() + if x == '': + continue + logger.info(f'x: {x}') + m = polname_re.match(x) + if not m: + continue + + pnames.append(m.group(1)) + return pnames + + def _get_inherited_policies(self, git_dir=None): + """ + :return: list of policy names from parent policies + :rtype: list + """ + logger.info(f'Running _get_inherited_policies with {git_dir}') + res = subprocess.check_output( + ['git', 'ls-files'], + cwd=git_dir + ).decode().split("\n") + pnames = [] + polname_re = re.compile(r'^.*\/([a-zA-Z0-9_-]+)\.yml$') for x in res: x = x.strip() if x == '': From 6b26b1a300da7eb3cdfb8b721c2dd6a56b177ed5 Mon Sep 17 00:00:00 2001 From: James Leopold Date: Wed, 16 Jun 2021 10:55:44 -0400 Subject: [PATCH 20/33] codestyle fix --- manheim_c7n_tools/dryrun_diff.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/manheim_c7n_tools/dryrun_diff.py b/manheim_c7n_tools/dryrun_diff.py index 5cd90c3..1a2a640 100755 --- a/manheim_c7n_tools/dryrun_diff.py +++ b/manheim_c7n_tools/dryrun_diff.py @@ -114,7 +114,7 @@ def _find_changed_policies(self, git_dir=None, diff_against='master'): :return: list of policy names that differ from master :rtype: list """ - logger.info(f'Running _find_changed_policies with {git_dir}, {diff_against}') + logger.debug(f'_find_changed_policies({git_dir}, {diff_against})') res = subprocess.check_output( ['git', 'diff', '--name-only', diff_against], cwd=git_dir @@ -138,7 +138,7 @@ def _get_inherited_policies(self, git_dir=None): :return: list of policy names from parent policies :rtype: list """ - logger.info(f'Running _get_inherited_policies with {git_dir}') + logger.debug(f'_get_inherited_policies({git_dir})') res = subprocess.check_output( ['git', 'ls-files'], cwd=git_dir From bb2534fba41141a9519c4aeb7c298a2768e62256 Mon Sep 17 00:00:00 2001 From: James Leopold Date: Wed, 16 Jun 2021 10:57:01 -0400 Subject: [PATCH 21/33] fix spacing --- manheim_c7n_tools/dryrun_diff.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/manheim_c7n_tools/dryrun_diff.py b/manheim_c7n_tools/dryrun_diff.py index 1a2a640..ef197dd 100755 --- a/manheim_c7n_tools/dryrun_diff.py +++ b/manheim_c7n_tools/dryrun_diff.py @@ -129,7 +129,6 @@ def _find_changed_policies(self, git_dir=None, diff_against='master'): m = polname_re.match(x) if not m: continue - pnames.append(m.group(1)) return pnames @@ -153,7 +152,6 @@ def _get_inherited_policies(self, git_dir=None): m = polname_re.match(x) if not m: continue - pnames.append(m.group(1)) return pnames From 6a2e3146a3a0b1712b7d217fe888a7da86bf021f Mon Sep 17 00:00:00 2001 From: James Leopold Date: Wed, 16 Jun 2021 10:57:45 -0400 Subject: [PATCH 22/33] remove debug lines --- manheim_c7n_tools/dryrun_diff.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/manheim_c7n_tools/dryrun_diff.py b/manheim_c7n_tools/dryrun_diff.py index ef197dd..83707dd 100755 --- a/manheim_c7n_tools/dryrun_diff.py +++ b/manheim_c7n_tools/dryrun_diff.py @@ -125,7 +125,6 @@ def _find_changed_policies(self, git_dir=None, diff_against='master'): x = x.strip() if x == '': continue - logger.info(f'x: {x}') m = polname_re.match(x) if not m: continue @@ -148,7 +147,6 @@ def _get_inherited_policies(self, git_dir=None): x = x.strip() if x == '': continue - logger.info(f'x: {x}') m = polname_re.match(x) if not m: continue From 1fc9150d3f951775ca6ea6373e1e4d63c9afd53d Mon Sep 17 00:00:00 2001 From: James Leopold Date: Wed, 16 Jun 2021 11:09:27 -0400 Subject: [PATCH 23/33] wip --- manheim_c7n_tools/dryrun_diff.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/manheim_c7n_tools/dryrun_diff.py b/manheim_c7n_tools/dryrun_diff.py index 83707dd..0acbab8 100755 --- a/manheim_c7n_tools/dryrun_diff.py +++ b/manheim_c7n_tools/dryrun_diff.py @@ -89,17 +89,17 @@ def run(self, git_dir=None, diff_against='master'): for rname in self.config.regions: logger.debug('Getting S3 results for region: %s', rname) self._get_s3_results_for_region(rname, changed_policies) - diff_md = self._make_diff_markdown(dryrun_results) + diff_md, diff_count = self._make_diff_markdown(dryrun_results) with open('pr_diff.md', 'w') as fh: if 'defaults' in changed_policies: fh.write( 'PR found to contain changes to defaults.yml and ' - '%d other policies\n\n' % (len(changed_policies) - 1) + '%d other policies\n\n' % (len(diff_count) - 1) ) else: fh.write( 'PR found to contain changes to ' - '%d policies\n\n' % len(changed_policies) + '%d policies\n\n' % len(diff_count) ) fh.write(diff_md) logger.info('PR diff written to: pr_diff.md') @@ -312,7 +312,7 @@ def _make_diff_markdown(self, dryrun): prefix, rname, a_str, b_str, diff ) res += '```\n' - return res + return res, len(res) def _get_dryrun_results(self, pol_names): """ From 2e54b4a0c8eef9b9f26cd529f330bb0213806980 Mon Sep 17 00:00:00 2001 From: James Leopold Date: Wed, 16 Jun 2021 11:20:56 -0400 Subject: [PATCH 24/33] fix count --- manheim_c7n_tools/dryrun_diff.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/manheim_c7n_tools/dryrun_diff.py b/manheim_c7n_tools/dryrun_diff.py index 0acbab8..9894313 100755 --- a/manheim_c7n_tools/dryrun_diff.py +++ b/manheim_c7n_tools/dryrun_diff.py @@ -94,12 +94,12 @@ def run(self, git_dir=None, diff_against='master'): if 'defaults' in changed_policies: fh.write( 'PR found to contain changes to defaults.yml and ' - '%d other policies\n\n' % (len(diff_count) - 1) + '%d other policies\n\n' % (diff_count - 1) ) else: fh.write( 'PR found to contain changes to ' - '%d policies\n\n' % len(diff_count) + '%d policies\n\n' % diff_count ) fh.write(diff_md) logger.info('PR diff written to: pr_diff.md') @@ -269,6 +269,7 @@ def _make_diff_markdown(self, dryrun): :return: markdown diff :rtype: str """ + policy_diff_count = 0 all_policies = list(set( list(dryrun.keys()) + list(self._live_results.keys()) )) @@ -285,6 +286,7 @@ def _make_diff_markdown(self, dryrun): for pname in sorted(all_policies): res += '%s\n' % ('=' * linelen) res += pname + "\n" + policy_diff_count += 1 for rname in self.config.regions: a = len(self._live_results.get(pname, {}).get(rname, [])) b = len(dryrun.get(pname, {}).get(rname, [])) @@ -312,7 +314,7 @@ def _make_diff_markdown(self, dryrun): prefix, rname, a_str, b_str, diff ) res += '```\n' - return res, len(res) + return res, policy_diff_count def _get_dryrun_results(self, pol_names): """ From 696cdb82d85918a2ce470ae9fc14a3f35a05c84a Mon Sep 17 00:00:00 2001 From: James Leopold Date: Wed, 16 Jun 2021 11:32:08 -0400 Subject: [PATCH 25/33] add comment --- manheim_c7n_tools/dryrun_diff.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/manheim_c7n_tools/dryrun_diff.py b/manheim_c7n_tools/dryrun_diff.py index 9894313..99070cd 100755 --- a/manheim_c7n_tools/dryrun_diff.py +++ b/manheim_c7n_tools/dryrun_diff.py @@ -65,6 +65,8 @@ def run(self, git_dir=None, diff_against='master'): for p in parent_source_paths: sub_policy_path = f'policies/{p}' if os.path.isdir(sub_policy_path): + # Add inherited policies to list of possible "changed_policies" + # We don't know if they differ from master until the comparison step changed_policies.extend( self._get_inherited_policies(sub_policy_path) ) From f61b98124e71014806bb1e611818ae908161545d Mon Sep 17 00:00:00 2001 From: James Leopold Date: Wed, 16 Jun 2021 11:39:18 -0400 Subject: [PATCH 26/33] line too long --- manheim_c7n_tools/dryrun_diff.py | 1 - 1 file changed, 1 deletion(-) diff --git a/manheim_c7n_tools/dryrun_diff.py b/manheim_c7n_tools/dryrun_diff.py index 99070cd..6303941 100755 --- a/manheim_c7n_tools/dryrun_diff.py +++ b/manheim_c7n_tools/dryrun_diff.py @@ -66,7 +66,6 @@ def run(self, git_dir=None, diff_against='master'): sub_policy_path = f'policies/{p}' if os.path.isdir(sub_policy_path): # Add inherited policies to list of possible "changed_policies" - # We don't know if they differ from master until the comparison step changed_policies.extend( self._get_inherited_policies(sub_policy_path) ) From 5e165656dcbf5ea4ccdbd009f49d33494654288a Mon Sep 17 00:00:00 2001 From: James Leopold Date: Wed, 16 Jun 2021 11:56:53 -0400 Subject: [PATCH 27/33] improve logging --- manheim_c7n_tools/dryrun_diff.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/manheim_c7n_tools/dryrun_diff.py b/manheim_c7n_tools/dryrun_diff.py index 6303941..ff50318 100755 --- a/manheim_c7n_tools/dryrun_diff.py +++ b/manheim_c7n_tools/dryrun_diff.py @@ -65,15 +65,22 @@ def run(self, git_dir=None, diff_against='master'): for p in parent_source_paths: sub_policy_path = f'policies/{p}' if os.path.isdir(sub_policy_path): - # Add inherited policies to list of possible "changed_policies" + logger.info( + f'Adding all inherited policies from {p} to list ' + 'of possible changed_policies' + ) changed_policies.extend( self._get_inherited_policies(sub_policy_path) ) else: logger.warning( - f'{sub_policy_path} is defined in `policy_source_paths` ' + f'\n\t{p} is defined in `policy_source_paths` ' 'but is not checked out into a seperate directory. ' - 'dryrun-diff results will be incomplete!' + 'Dryrun-diff results will be incomplete!\n' + f'\tTo resolve this run `git clone ' + f' {sub_policy_path}`.\n' + f'\tThis will checkout the inherited policy ({p}) ' + 'for dryrun-diff' ) if len(changed_policies) == 0: logger.info( From 630fbb5315b4ce7a3f7124be33ada51ebb35480f Mon Sep 17 00:00:00 2001 From: James Leopold Date: Wed, 16 Jun 2021 12:10:43 -0400 Subject: [PATCH 28/33] bump ver --- manheim_c7n_tools/version.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manheim_c7n_tools/version.py b/manheim_c7n_tools/version.py index 7bc071f..e0439a8 100644 --- a/manheim_c7n_tools/version.py +++ b/manheim_c7n_tools/version.py @@ -17,7 +17,7 @@ """ #: The semver-compliant version of the package. -VERSION = '1.3.0' +VERSION = '1.3.1' #: The URL for further information about the package. PROJECT_URL = 'https://github.com/manheim/manheim-c7n-tools' From 761dc14ed2430a5a83912b2e720bef61dd87e82e Mon Sep 17 00:00:00 2001 From: James Leopold Date: Wed, 16 Jun 2021 13:03:58 -0400 Subject: [PATCH 29/33] cleanup git diff --- manheim_c7n_tools/dryrun_diff.py | 105 +++---------------------------- 1 file changed, 8 insertions(+), 97 deletions(-) diff --git a/manheim_c7n_tools/dryrun_diff.py b/manheim_c7n_tools/dryrun_diff.py index ff50318..1988d01 100755 --- a/manheim_c7n_tools/dryrun_diff.py +++ b/manheim_c7n_tools/dryrun_diff.py @@ -59,56 +59,17 @@ def __init__(self, config): self.config = config def run(self, git_dir=None, diff_against='master'): - changed_policies = self._find_changed_policies(git_dir, diff_against) - source_paths = self.config.policy_source_paths - parent_source_paths = source_paths[:-1] - for p in parent_source_paths: - sub_policy_path = f'policies/{p}' - if os.path.isdir(sub_policy_path): - logger.info( - f'Adding all inherited policies from {p} to list ' - 'of possible changed_policies' - ) - changed_policies.extend( - self._get_inherited_policies(sub_policy_path) - ) - else: - logger.warning( - f'\n\t{p} is defined in `policy_source_paths` ' - 'but is not checked out into a seperate directory. ' - 'Dryrun-diff results will be incomplete!\n' - f'\tTo resolve this run `git clone ' - f' {sub_policy_path}`.\n' - f'\tThis will checkout the inherited policy ({p}) ' - 'for dryrun-diff' - ) - if len(changed_policies) == 0: - logger.info( - 'Git diff did not report any changed policies; skipping ' - 'resource count diff.' - ) - with open('pr_diff.md', 'w') as fh: - fh.write('Git diff did not report any changed policies; ' - 'skipping resource count diff.') - return - logger.info('Changed policies for diff: %s', changed_policies) - dryrun_results = self._get_dryrun_results(changed_policies) + dryrun_results = self._get_dryrun_results() logger.info('Reading results from last run from S3') for rname in self.config.regions: logger.debug('Getting S3 results for region: %s', rname) - self._get_s3_results_for_region(rname, changed_policies) + self._get_s3_results_for_region(rname) diff_md, diff_count = self._make_diff_markdown(dryrun_results) with open('pr_diff.md', 'w') as fh: - if 'defaults' in changed_policies: - fh.write( - 'PR found to contain changes to defaults.yml and ' - '%d other policies\n\n' % (diff_count - 1) - ) - else: - fh.write( - 'PR found to contain changes to ' - '%d policies\n\n' % diff_count - ) + fh.write( + 'PR found to contain changes to ' + '%d policies\n\n' % diff_count + ) fh.write(diff_md) logger.info('PR diff written to: pr_diff.md') diff_report = self._make_diff_report(dryrun_results) @@ -117,50 +78,6 @@ def run(self, git_dir=None, diff_against='master'): fh.write(diff_report) logger.info('PR report written to: pr_report.html') - def _find_changed_policies(self, git_dir=None, diff_against='master'): - """ - :return: list of policy names that differ from master - :rtype: list - """ - logger.debug(f'_find_changed_policies({git_dir}, {diff_against})') - res = subprocess.check_output( - ['git', 'diff', '--name-only', diff_against], - cwd=git_dir - ).decode().split("\n") - pnames = [] - polname_re = re.compile(r'^policies.*\/([a-zA-Z0-9_-]+)\.yml$') - for x in res: - x = x.strip() - if x == '': - continue - m = polname_re.match(x) - if not m: - continue - pnames.append(m.group(1)) - return pnames - - def _get_inherited_policies(self, git_dir=None): - """ - :return: list of policy names from parent policies - :rtype: list - """ - logger.debug(f'_get_inherited_policies({git_dir})') - res = subprocess.check_output( - ['git', 'ls-files'], - cwd=git_dir - ).decode().split("\n") - pnames = [] - polname_re = re.compile(r'^.*\/([a-zA-Z0-9_-]+)\.yml$') - for x in res: - x = x.strip() - if x == '': - continue - m = polname_re.match(x) - if not m: - continue - pnames.append(m.group(1)) - return pnames - def _make_diff_report(self, dryrun): """ Return a HTML report breaking down the differences between the dryrun @@ -324,7 +241,7 @@ def _make_diff_markdown(self, dryrun): res += '```\n' return res, policy_diff_count - def _get_dryrun_results(self, pol_names): + def _get_dryrun_results(self): """ Read the `resources.json` files from disk for the dryrun/ directory. Return a dictionary of string policy name to nested dictionaries, of @@ -344,9 +261,6 @@ def _get_dryrun_results(self, pol_names): continue region = m.group(1) policy = m.group(2) - if policy not in pol_names and 'defaults' not in pol_names: - # policy isn't changed - continue _dir = os.path.dirname(f) logger.debug('Reading files from directory: %s', _dir) try: @@ -383,7 +297,7 @@ def _read_dryrun_files(self, directory, pol, region, res): 'resource', self.UNKNOWN_RESOURCE_TYPE) res[pol][self.RESOURCE_TYPE_KEY] = _type - def _get_s3_results_for_region(self, region_name, changed_pols): + def _get_s3_results_for_region(self, region_name): """ Find the results files in S3 from the last live run of the deployed policies. Reads each file and maps resources to ``self._live_results`` @@ -397,9 +311,6 @@ def _get_s3_results_for_region(self, region_name, changed_pols): prefixes = self._get_s3_policy_prefixes(bkt) logger.debug('Found %d policy prefixes in %s', len(prefixes), bktname) for p in prefixes: - if p not in changed_pols and 'defaults' not in changed_pols: - # policy was not changed, skip it - continue if p not in self._live_results: self._live_results[p] = {} fetch_type = self.RESOURCE_TYPE_KEY not in self._live_results[p] From 1ac466e92a60a27a6bc340cefdcd512fa6708f9b Mon Sep 17 00:00:00 2001 From: James Leopold Date: Thu, 17 Jun 2021 09:48:57 -0400 Subject: [PATCH 30/33] fix output pr_md --- manheim_c7n_tools/dryrun_diff.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/manheim_c7n_tools/dryrun_diff.py b/manheim_c7n_tools/dryrun_diff.py index 1988d01..4098e78 100755 --- a/manheim_c7n_tools/dryrun_diff.py +++ b/manheim_c7n_tools/dryrun_diff.py @@ -29,7 +29,6 @@ import itertools import os from zlib import decompress -import subprocess from jinja2 import Environment, FileSystemLoader from jinja2.exceptions import TemplateNotFound @@ -209,9 +208,6 @@ def _make_diff_markdown(self, dryrun): res += '##%s live ##\n' % (' ' * maxlen) res += '##%s run PR diff ##\n' % (' ' * maxlen) for pname in sorted(all_policies): - res += '%s\n' % ('=' * linelen) - res += pname + "\n" - policy_diff_count += 1 for rname in self.config.regions: a = len(self._live_results.get(pname, {}).get(rname, [])) b = len(dryrun.get(pname, {}).get(rname, [])) @@ -219,6 +215,13 @@ def _make_diff_markdown(self, dryrun): b_str = '--' if b == 0 else b prefix = ' ' diff = '' + if a == b: + # no changes + continue + res += '%s\n' % ('=' * linelen) + res += pname + "\n" + policy_diff_count += 1 + if a == '--' and b != '--': # in PR but not in master/live prefix = '+' From 300d0d28b447468d6048b6b7fe17c7706f2c9978 Mon Sep 17 00:00:00 2001 From: James Leopold Date: Thu, 17 Jun 2021 12:22:27 -0400 Subject: [PATCH 31/33] comment update --- manheim_c7n_tools/dryrun_diff.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manheim_c7n_tools/dryrun_diff.py b/manheim_c7n_tools/dryrun_diff.py index 4098e78..8eb27a6 100755 --- a/manheim_c7n_tools/dryrun_diff.py +++ b/manheim_c7n_tools/dryrun_diff.py @@ -215,8 +215,8 @@ def _make_diff_markdown(self, dryrun): b_str = '--' if b == 0 else b prefix = ' ' diff = '' + # No changes in resource counts if a == b: - # no changes continue res += '%s\n' % ('=' * linelen) res += pname + "\n" From 73677e0234041fae8eb3b9fa0f61b8be2c2c9354 Mon Sep 17 00:00:00 2001 From: James Leopold Date: Thu, 17 Jun 2021 12:28:21 -0400 Subject: [PATCH 32/33] update changelog --- CHANGES.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index b2a2933..d6666c2 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -5,7 +5,8 @@ Changelog 1.3.1 (2021-06-14) ------------------ -* Fixing dryrun-diff bug to show changes to nested policies. +* Fixing dryrun-diff bug to show changes to inherited policies. + * Remove `git diff` comparison; Now we compare results of full dryrun to last live-run 1.3.0 (2021-01-13) ------------------ From d77daa63fb863ade543d7612e5017dad53ed2905 Mon Sep 17 00:00:00 2001 From: James Leopold Date: Thu, 17 Jun 2021 14:13:06 -0400 Subject: [PATCH 33/33] update docs --- docs/source/dryrun-diff.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/source/dryrun-diff.rst b/docs/source/dryrun-diff.rst index 339ad66..abd5f4a 100644 --- a/docs/source/dryrun-diff.rst +++ b/docs/source/dryrun-diff.rst @@ -6,8 +6,10 @@ Dryrun-Diff :py:mod:`Source code docs `. -The ``dryrun-diff`` entrypoint (and corresponding ``manheim-c7n-tools`` step) must be run in a directory containing the ``dryrun/`` output directory from a custodian dry run. It parses the resource counts for each policy executed in each region during the dry run, then retrieves the logs from the last actual custodian run from S3. The matched resource counts are compared, and a markdown file is generated for use as a GitHub PR comment. This allows us to compare the impact of policy change pull requests. +The ``dryrun-diff`` entrypoint (and corresponding ``manheim-c7n-tools`` step) must be run after the custodian dry run step is completed in all regions. It parses the resource counts for each policy executed in each region during the dry run (from the ``dryrun/`` output directory), then retrieves the logs from the last actual custodian run from S3. The matched resource counts are compared, and a markdown file is generated for use as a GitHub PR comment. This allows us to compare the impact of policy change pull requests. The generated markdown file will be written to ``./pr_diff.md`` in the current directory. +Note: ``dryrun-diff`` will ONLY show changes to resource counts. Updated policies (comment updates, etc.) will not show up in the ``dryrun-diff`` unless resource counts are changing. + If the ``dryrun-diff`` entrypoint has been run in a directory containing a jinja template located at ``./reporting-template/report.j2``, this template will be used to generate a detailed HTML report of which resources have been affected by policy changes. An example of a reporting jinja template can be found within the ``./example_config_repo`` folder at the root of the Manheim repository. The report will written to ``./pr_report.html`` in the current directory. \ No newline at end of file