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

Shouldn't "Then it must contain something" step support case-sensitive comparisons? #268

Closed
victorfrancax1 opened this issue May 13, 2020 · 8 comments
Assignees
Labels
enhancement waiting for confirmation Workaround/Fix applied, waiting for confirmation

Comments

@victorfrancax1
Copy link

victorfrancax1 commented May 13, 2020

Question :

I was benchmarking the tool and noticed that the "Then it must contain something" step performs case-insensitive comparisons while searching for keys. My company's use case is mainly resource tagging enforcement on AWS, and since both tag key and tag value are case-sensitive, we were not able to properly check if our resources are compliant with the rule, which is pretty similar to the example provided by you guys.

The values are not a problem right now, as it's possible to work around them using regex through "its value must match the foo regex" step. Is there any workaround for the key?

This problem is exactly what we are trying to avoid by using terraform-compliance rules, as people often tag stuff with the "same" key but in different case types, making using the tags for cost and resource usage optimization very hard.

Thank you in advance!

Context :
AWS docs on resource tagging.

@eerkunt
Copy link
Member

eerkunt commented May 18, 2020

Thanks for the issue @victorfrancax1 !

Sounds like an easy and logical improvement. Will have a look on the next release!

@eerkunt
Copy link
Member

eerkunt commented May 19, 2020

Just released 1.2.1 that has this functionality. Can you have a try please ?

For more information, please have look on document about the new tag that is introduced.

@eerkunt eerkunt added the waiting for confirmation Workaround/Fix applied, waiting for confirmation label May 19, 2020
@victorfrancax1
Copy link
Author

Thank you for the release, @eerkunt! I was able to confirm that using case-sensitive bdd tag works for matching property values using regex. 🎉

However, matching property keys using "it must contain something" still makes case-insensitive comparisons. This happens because of this line at the helper function, AFAICS: https://github.com/eerkunt/terraform-compliance/blob/9d8c9908eff28d624c931add066bd40a608090e8/terraform_compliance/common/helper.py#L107

Could we use a similar approach for this statement?

@eerkunt
Copy link
Member

eerkunt commented May 27, 2020

Yep, that will be also added on next releases. This one was just for regex related one.

I will refactor those parts to make it in a better way.

@Kudbettin
Copy link
Member

Hi @victorfrancax1,

Starting from release 1.2.8, @case_sensitivity has been modified to capture all steps. Could you give it a try?

@victorfrancax1
Copy link
Author

victorfrancax1 commented Aug 11, 2020

Hi @Kudbettin ! I was able to confirm that now it's doing case-sensitive comparisons on both property key and value 🚀

Thank you for your help guys! @eerkunt , I think we can close this .

@Kudbettin
Copy link
Member

Awesome! Closing the issue.

@ghost
Copy link

ghost commented Aug 12, 2020

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

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

No branches or pull requests

3 participants