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

Feat/initial hooks support #315

Merged
merged 5 commits into from
Apr 17, 2024
Merged

Feat/initial hooks support #315

merged 5 commits into from
Apr 17, 2024

Conversation

dhutchison
Copy link
Collaborator

@dhutchison dhutchison commented Feb 11, 2024

Initial hook implementation for #302.

I don't think I'm finished with this quite yet, but this should give an idea of how I see a first pass going.

This doesn't include the plugin support you suggested quite yet, but aims to prove out the structure and usage patterns which are probably a good first step.

At the core of it there is two levels of hooks that will be supported - template & resource.

  • Template level hooks will be evaluated at (TODO) and will look like (TODO)
  • Resource level hooks will be evaluated at the point of the stack creation and are expected to take a single parameter of the type ResourceHookContext, which contains:
    • logical_id: str
    • resource_definition: Resource
    • stack: Stack
    • template: Template

Resource level hooks are configured on a template like this:

    template = Template.from_yaml(template_path)

    # Add in locally defined hooks
    template.hooks.resources = {
        "AWS::S3::Bucket": [my_s3_naming_hook, my_s3_encryption_hook]
    }

Hooks can be suppressed (in a similar fashion to other tools like cfn-lint and cfn-nag) at either the template or resource level by adding Metadata like the following, where the items in the list are the function names of the configured hooks.

Metadata:
      Cloud-Radar:
        ignore-hooks:
          - my_s3_encryption_hook

Copy link

codecov bot commented Feb 15, 2024

Codecov Report

Attention: Patch coverage is 93.50649% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 94.85%. Comparing base (4c51da0) to head (3f6ed3a).
Report is 75 commits behind head on develop.

Files Patch % Lines
src/cloud_radar/cf/unit/_hooks.py 92.64% 3 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #315      +/-   ##
===========================================
- Coverage    95.97%   94.85%   -1.13%     
===========================================
  Files           10       12       +2     
  Lines          695      835     +140     
  Branches       145      165      +20     
===========================================
+ Hits           667      792     +125     
- Misses          16       24       +8     
- Partials        12       19       +7     
Flag Coverage Δ
unit 94.85% <93.50%> (-1.13%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dhutchison dhutchison added enhancement New feature or request minor A new feature has been added. labels Mar 12, 2024
@dhutchison
Copy link
Collaborator Author

I think it is time for a review of this functionality with the aim to getting a release out, even as a stepping stone to the full vision. The documentation/explanation is contained in the examples/unit/hooks/README.md file in the PR.

This adds the support for hooks without the plugin scanning functionality. It's a starting point where you can rely on at least using a fixture in a conftest.py file for configuring the hooks from functions defined in external libraries. Not quite where you were hoping to get to, but a starting point. I've got a branch in this repo with code that should support scanning plugins, but I've not had time to test it.

Figured it was more worth while documenting this piece before going on to that side of testing.

@dhutchison dhutchison marked this pull request as ready for review April 16, 2024 22:19
Copy link
Member

@shadycuz shadycuz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dhutchison

My wedding was Jan 30th, I don't even remember seeing this PR opened! This is awesome stuff 🎉

@shadycuz shadycuz merged commit b463536 into develop Apr 17, 2024
7 checks passed
@shadycuz shadycuz deleted the feat/initial-hooks-support branch April 17, 2024 12:38
@dhutchison
Copy link
Collaborator Author

@shadycuz congratulations! Just a slightly more important thing to be thinking about 😂

shadycuz pushed a commit that referenced this pull request Apr 23, 2024
* feat(compliance): wip ramblings

* feat(hooks): continued development of hooks

* feat(hooks): added more tests and code docs for template hooks

* feat(hooks): documentation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request minor A new feature has been added.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants