Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #53 - mugc not respecting function_prefix #54

Merged
merged 4 commits into from
Jul 29, 2020

Conversation

jantman
Copy link
Contributor

@jantman jantman commented Jul 29, 2020

Description

c7n allows a configurable Lambda function name prefix, which defaults to custodian-. For the policies, this is configured in the actual policy YML. However, for mugc garbage collection, it needs to be explicitly passed in. In the current release, we have mugc hard-coded to the default of custodian-. The result of this, as described in #53, is that manheim-c7n-tools can provision policies with a non-default prefix but mugc keeps trying to clean up the default-prefixed policies.

This PR solves this issue with 3 pieces:

  1. A new function_prefix option in manheim-c7n-tools.yml, which defaults to the current and upstream default of custodian-.
  2. Modify MugcStep to use that configuration option instead of the hard-coded custodian-.
  3. Add a new policy sanity check which causes policygen to fail if any policies are configured with a function-prefix different from what's in the Config object (manheim-c7n-tools.yml). This should prevent issues like --step=mugc ignores mode.function-prefix #53 where the prefixes differ between function creation and garbage collection.

Testing Done

  • Complete unit test coverage
  • Manual tests of manheim-c7n-runner -A -s policygen dryrun accountname to verify behavior

@codecov-commenter
Copy link

codecov-commenter commented Jul 29, 2020

Codecov Report

Merging #54 into master will increase coverage by 0.24%.
The diff coverage is 83.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #54      +/-   ##
==========================================
+ Coverage   57.73%   57.98%   +0.24%     
==========================================
  Files           8        8              
  Lines        1389     1397       +8     
  Branches      262      264       +2     
==========================================
+ Hits          802      810       +8     
  Misses        586      586              
  Partials        1        1              
Impacted Files Coverage Δ
manheim_c7n_tools/errorscan.py 0.00% <0.00%> (ø)
manheim_c7n_tools/config.py 100.00% <100.00%> (ø)
manheim_c7n_tools/policygen.py 99.76% <100.00%> (+<0.01%) ⬆️
manheim_c7n_tools/runner.py 100.00% <100.00%> (ø)
manheim_c7n_tools/version.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0720bb7...9e8c914. Read the comment docs.

@jantman jantman merged commit f339e59 into manheim:master Jul 29, 2020
@jantman jantman deleted the issues/53 branch July 29, 2020 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants