Skip to content

Commit

Permalink
Add metrics logger to Config (#231)
Browse files Browse the repository at this point in the history
* Add metrics_logger to be used for filters when used

* Update version and readme

* rewrite readme
  • Loading branch information
ignaciobolonio committed Jun 13, 2023
1 parent 75c0d7c commit 16993de
Show file tree
Hide file tree
Showing 6 changed files with 14 additions and 2 deletions.
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
# Changelog
All notable changes to this project will be documented in this file.

## [1.14.0]
### Additions
- `Config` includes a metrics logger and it is called to register when a filter is used

## [1.13.2]
### Fixes
- Fixes docs formatting with [#235](https://github.com/Skyscanner/cfripper/pull/235)
Expand All @@ -17,7 +21,6 @@ All notable changes to this project will be documented in this file.
### Fixes
- Update `GenericWildcardPrincipalRule`, `FullWildcardPrincipalRule`, `GenericResourceWildcardPrincipalRule` and `GenericResourceFullWildcardPrincipalRule` message, since sometimes it was bad-formatted in markdown.


## [1.12.0]
### Improvements
- Refactored the `KMSKeyWildcardPrincipalRule` rule
Expand Down
2 changes: 1 addition & 1 deletion cfripper/__version__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
VERSION = (1, 13, 2)
VERSION = (1, 14, 0)

__version__ = ".".join(map(str, VERSION))
2 changes: 2 additions & 0 deletions cfripper/config/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ def __init__(
aws_service_accounts=None,
rules_config=None,
rules_filters=None,
metrics_logger=None,
):
self.project_name = project_name
self.service_name = service_name
Expand All @@ -146,6 +147,7 @@ def __init__(
self.aws_account_name = aws_account_name
self.aws_account_id = aws_account_id
self.aws_user_agent = aws_user_agent
self.metrics_logger = metrics_logger
if aws_service_accounts is None:
self.aws_service_accounts = {
"cloudtrail_account_ids": AWS_CLOUDTRAIL_ACCOUNT_IDS,
Expand Down
2 changes: 2 additions & 0 deletions cfripper/rules/base_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ def add_failure_to_result(
if rule_filter(**context):
risk_value = rule_filter.risk_value or risk_value
rule_mode = rule_filter.rule_mode or rule_mode
if self._config.metrics_logger:
self._config.metrics_logger(rule=self.__class__.__name__, filter_reason=rule_filter.reason)
except Exception:
logger.exception(f"Exception raised while evaluating filter for `{rule_filter.reason}`", extra=context)

Expand Down
1 change: 1 addition & 0 deletions tests/config/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ def test_init_with_no_params():
assert config.rules is None
assert config.aws_account_id is None
assert config.aws_principals == []
assert config.metrics_logger is None


def test_init_with_nonexistent_params():
Expand Down
4 changes: 4 additions & 0 deletions tests/config/test_filter.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import logging
from typing import Callable
from unittest import mock

import pytest

Expand Down Expand Up @@ -276,6 +277,7 @@ def test_exist_function_and_property_does_not_exist(template_cross_account_role_


def test_exist_function_and_property_exists(template_cross_account_role_with_name):
mock_metrics_logger = mock.Mock()
mock_config = Config(
rules=["CrossAccountTrustRule"],
aws_account_id="123456789",
Expand All @@ -297,13 +299,15 @@ def test_exist_function_and_property_exists(template_cross_account_role_with_nam
rules={"CrossAccountTrustRule"},
),
],
metrics_logger=mock_metrics_logger,
)

rules = [DEFAULT_RULES.get(rule)(mock_config) for rule in mock_config.rules]
processor = RuleProcessor(*rules)
result = processor.process_cf_template(template_cross_account_role_with_name, mock_config)

assert result.valid
mock_metrics_logger.assert_called()
assert compare_lists_of_failures(result.failures, [])


Expand Down

0 comments on commit 16993de

Please sign in to comment.