Skip to content

Commit

Permalink
Merge pull request #61 from manheim/dry_run_bugfix
Browse files Browse the repository at this point in the history
Fixing dryrun-diff step for inherited policies
jleopold28 authored Jun 18, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
2 parents 28c6e34 + d77daa6 commit 77a904e
Showing 4 changed files with 29 additions and 59 deletions.
7 changes: 7 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
Changelog
=========


1.3.1 (2021-06-14)
------------------

* 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)
------------------

4 changes: 3 additions & 1 deletion docs/source/dryrun-diff.rst
Original file line number Diff line number Diff line change
@@ -6,8 +6,10 @@ Dryrun-Diff

:py:mod:`Source code docs <manheim_c7n_tools.dryrun_diff>`.

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.
75 changes: 18 additions & 57 deletions manheim_c7n_tools/dryrun_diff.py
Original file line number Diff line number Diff line change
@@ -29,7 +29,6 @@
import itertools
import os
from zlib import decompress
import subprocess
from jinja2 import Environment, FileSystemLoader
from jinja2.exceptions import TemplateNotFound

@@ -59,34 +58,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)
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)
diff_md = self._make_diff_markdown(dryrun_results)
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' % (len(changed_policies) - 1)
)
else:
fh.write(
'PR found to contain changes to '
'%d policies\n\n' % len(changed_policies)
)
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)
@@ -95,27 +77,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
"""
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 _make_diff_report(self, dryrun):
"""
Return a HTML report breaking down the differences between the dryrun
@@ -232,6 +193,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())
))
@@ -246,15 +208,20 @@ 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"
for rname in self.config.regions:
a = len(self._live_results.get(pname, {}).get(rname, []))
b = len(dryrun.get(pname, {}).get(rname, []))
a_str = '--' if a == 0 else a
b_str = '--' if b == 0 else b
prefix = ' '
diff = ''
# No changes in resource counts
if a == b:
continue
res += '%s\n' % ('=' * linelen)
res += pname + "\n"
policy_diff_count += 1

if a == '--' and b != '--':
# in PR but not in master/live
prefix = '+'
@@ -275,9 +242,9 @@ def _make_diff_markdown(self, dryrun):
prefix, rname, a_str, b_str, diff
)
res += '```\n'
return res
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
@@ -297,9 +264,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:
@@ -336,7 +300,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``
@@ -350,9 +314,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]
2 changes: 1 addition & 1 deletion manheim_c7n_tools/version.py
Original file line number Diff line number Diff line change
@@ -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'

0 comments on commit 77a904e

Please sign in to comment.