manheim / manheim-c7n-tools

Manheim's Cloud Custodian (c7n) wrapper package, policy generator, runner, and supporting tools.
https://manheim-c7n-tools.readthedocs.io/
Apache License 2.0
45 stars 25 forks source link

dryrun_diff: add HTML breakdown #34

Closed JuubiSnake closed 4 years ago

JuubiSnake commented 4 years ago

Description

This PR adds a HTML rendered document that details a breakdown of affected policies between a dryrun and the last live run of manhiem.

The current Markdown document only describes the 'high-level' difference count between both the dryrun and the last live run. This is useful for comments on a PR, as the report is concise and outlines the overall effect changes to a given policy, however this is not enough information if you wish to know which resources have been affected by policy changes.

We feel that the inclusion of a detailed HTML report of changes will provide an extra layer of confidence during a review before any policy changes are merged. The mechanism of how to expose this document is entirely up to the user; they could upload it to S3 as a static HTML webpage, integrate it with Slack, email the report, etc.

Below is a brief snapshot of a report - with some information redacted:

Screenshot 2020-04-28 at 11 03 18

In order to generate a report, the user will need to have a jinja template report.j2 located within a folder named reporting-template in the same directory where the dryrun-diff step has ran.

Testing Done

Similar to the markdown generation, we log whenever the function is called - however no additional unit testing has been done.

We have modified the mock generation of the BaseStep Class to stub out self.config.account_id in order to supplant the mockers issues with __getattr__.

Important Notes

Contributor License Agreement

Required for external contributors.

By submitting this work for inclusion in manheim-c7n-tools, I agree to the following terms:

codecov-io commented 4 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request head (extend-dry-run-diff@9d0b085). Click here to learn what that means. The diff coverage is n/a.

jantman commented 4 years ago

@JuubiSnake We just released a new version, 0.10.0, which upgrades to the latest c7n release. Would you mind please rebasing this on master and re-testing? I haven't confirmed, but I think some of the API around ResourceManager and policy loading may have changed...

JuubiSnake commented 4 years ago

@JuubiSnake We just released a new version, 0.10.0, which upgrades to the latest c7n release. Would you mind please rebasing this on master and re-testing? I haven't confirmed, but I think some of the API around ResourceManager and policy loading may have changed...

Rebased and added the following https://github.com/manheim/manheim-c7n-tools/pull/34/files#diff-12aec96e72fb0486abb7d120ef81c3d7R35-R36 - turns out that the newer version of c7n makes it a bit eaiser to extract what we need :)

JuubiSnake commented 4 years ago

The right way to find the resource type for each policy would be, in the methods where we get the policy stats (_get_dryrun_results() for the dry-run, and _get_s3_results_for_region() which calls _get_latest_res_for_policy()) we should also retrieve the metadata files. We can then get the resource type from that.

@jantman Sorry for the delay - I've included some code for pulling the resource_type via metadata.json from both the dryrun and live results. I'll be updating the commit should it fail the tests on travis - but for now, I think it's close to finished. Any feedback you have I'll address 👍

robertstettner commented 4 years ago

Hi @jantman, just a friendly bump here. Could you please re-review this, as we are blocked. Thank you.

jantman commented 4 years ago

@robertstettner apologies, I'd been away from work for the past few days. I'll take another look at this now.

jantman commented 4 years ago

This looks great, thank you all so much! I'll get it merged now and released ASAP.

jantman commented 4 years ago

@JuubiSnake @robertstettner This has been released as 1.0.0. Thank you so much! The new version is now live on both PyPI and the Docker Hub.