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

Tags property is considered null if any tag has a value "none" or "null" #343

Closed
askainet opened this issue Aug 4, 2020 · 7 comments
Closed
Assignees
Labels
bug waiting for confirmation Workaround/Fix applied, waiting for confirmation

Comments

@askainet
Copy link

askainet commented Aug 4, 2020

Description :
When the value of any tag in a resource matches /^(none|null)$/i, the resource is considered as not having tags.

To Reproduce

  1. terraform code
resource "aws_s3_bucket" "a" {
  bucket = "my-tf-test-bucket"
  tags = {
    MyBuggyTag = "none"
    MyOkTag    = "ok"
  }
}
  1. terraform-compliance -p ./plan.json -f compliance

  2. python3 package

  3. Unexpected failed scenario

Feature: Test tags  # /private/tmp/test/compliance/tags.feature

    Scenario: Ensure all resources have tags
        Given I have resource that supports tags defined
        Then it must have tags
		Failure: tags property in aws_s3_bucket.a resource matches with (|^$|^null|^None)$ case insensitive regex. It is set to none.
        And its value must not be null
          Failure:

1 features (0 passed, 1 failed)
1 scenarios (0 passed, 1 failed)
3 steps (2 passed, 1 failed)
  1. <Your feature/scenario/steps>
Given I have resource that supports tags defined
Then it must have tags
And its value must not be null

Expected behavior :
Tags property should not be considered null if any tag has a string value "none" or "null". A smarter better way to check if the tags property is really empty should be implemented to allow setting "none" and "null" as valid values for a tag.

Tested versions :

  • terraform-compliance version 1.3.0
  • terraform version 0.12.29
  • python runtime version 3.8.5
@Kudbettin
Copy link
Member

Hi @askainet,

This is actually intended. Then its value condition be null is simply a specialized version of Then its value condition match the search regex, checking for the general "null like" values such as \x00, null, None.

As a result, "none" ends up matching the case insensitive regex under the hood.

If this is not what you're looking for, I would reccommend using the following scenario with a regex appropriate for you:

Scenario: Ensure all resources have tags
    Given I have resource that supports tags defined
    Then it must have tags
    And its value must not match the "^null$" regex  # Matches null only

Alternatively, you could use the @case_sensitive tag to prevent none matching the None in the regex during comparison.

@askainet
Copy link
Author

Hi @Kudbettin, thanks for your reply.

This is actually intended. Then its value condition be null is simply a specialized version of Then its value condition match the search regex, checking for the general "null like" values such as \x00, null, None.

Then this example in the documentation is wrong, as it leads to false positives.

Is there an actual way to properly test for empty sets of tags in resources?

@Kudbettin
Copy link
Member

@askainet

You're absolutely right. I looked a bit into this issue today and we definitely need improvements on both how the step works and is documented. #352 addresses some of the issues. I will update the documentation depending on the final version of the step.

Thanks for bringing this up! I can ping the issue once the pr makes into a release, but please feel free to let us know if something comes up.

@Kudbettin
Copy link
Member

Hi @askainet,

Could you give 1.3.2 a try? (The documentation is yet to be updated)

@Kudbettin Kudbettin added the waiting for confirmation Workaround/Fix applied, waiting for confirmation label Aug 18, 2020
@askainet
Copy link
Author

I can confirm the scenario works as expected in 1.3.2, many thanks @Kudbettin !

@eerkunt
Copy link
Member

eerkunt commented Aug 19, 2020

Thanks for trying again! :) Closing the issue since its confirmed as solved.

@eerkunt eerkunt closed this as completed Aug 19, 2020
@ghost
Copy link

ghost commented Aug 19, 2020

This issue's conversation is now locked. If you want to continue this discussion please open a new issue.

@ghost ghost locked and limited conversation to collaborators Aug 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug waiting for confirmation Workaround/Fix applied, waiting for confirmation
Projects
None yet
Development

No branches or pull requests

3 participants