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

Add metrics logger to Config #231

Merged
merged 4 commits into from
Jun 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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