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

Fix issue 249 #286

Merged
merged 10 commits into from
Jun 7, 2020
Merged

Fix issue 249 #286

merged 10 commits into from
Jun 7, 2020

Conversation

Kudbettin
Copy link
Member

On then/it_must_contain_something.py, if the value on "{something: value}" was an empty string/list, the step would evaluate the "something" key to be not existing inside the values.

@Kudbettin Kudbettin requested a review from eerkunt June 5, 2020 12:23
@coveralls
Copy link

coveralls commented Jun 5, 2020

Pull Request Test Coverage Report for Build 1024

  • 0 of 2 (0.0%) changed or added relevant lines in 1 file are covered.
  • 6 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.2%) to 61.635%

Changes Missing Coverage Covered Lines Changed/Added Lines %
terraform_compliance/steps/then/it_must_contain_something.py 0 2 0.0%
Files with Coverage Reduction New Missed Lines %
terraform_compliance/common/defaults.py 6 83.33%
Totals Coverage Status
Change from base Build 1021: 0.2%
Covered Lines: 1184
Relevant Lines: 1921

💛 - Coveralls

CHANGELOG.md Outdated
@@ -1,5 +1,8 @@
# CHANGELOG

## 1.2.5 (2020-05-25)
Copy link
Member

Choose a reason for hiding this comment

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

2020-06 :)

Btw, better not to change any CHANGELOG on a PR, since multiple PRs can happen simultaneously.

@@ -40,6 +40,7 @@ def it_must_contain_something(_step_obj, something, inherited_values=Null):

if isinstance(found_key, dict):
found_value = jsonify(found_key.get(something, found_key))
found_value = found_value if found_value != [] and found_value != '' else found_key
Copy link
Member

Choose a reason for hiding this comment

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

Lets change this conditions to ;

found value = found_value if found_value not in ([], '') else found_key

@@ -137,6 +138,7 @@ def it_must_not_contain_something(_step_obj, something, inherited_values=Null):

if isinstance(found_key, dict):
found_value = jsonify(found_key.get(something, found_key))
found_value = found_value if found_value != [] and found_value != '' else found_key
Copy link
Member

Choose a reason for hiding this comment

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

Same like above ^^

Comment on lines 9 to 13
Scenario: Ensure that waf_policy for ALB/CF tag is defined
Given I have aws_alb defined
When it has tags
Then it must contain waf_policy
And its value must match the "^(internal|external|custom)$" regex
Copy link
Member

Choose a reason for hiding this comment

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

When it has tags

will not drill down to the resource. waf_policy exists in tags parameter, thus the test must be changed like ;

  Scenario: Ensure that waf_policy for ALB/CF tag is defined
    Given I have aws_alb defined
    When it has tags
    Then it must contain tags
    And it must contain waf_policy
    And its value must match the "^(internal|external|custom)$" regex

Comment on lines 15 to 18
Scenario: Ensure that waf_custom for ALB/CF tag is defined
Given I have aws_alb defined
When it has tags
Then it must contain waf_custom
Copy link
Member

Choose a reason for hiding this comment

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

Same like above ^^

@eerkunt eerkunt merged commit 603a275 into terraform-compliance:master Jun 7, 2020
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