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 new rule: terraform_map_duplicate_values #224

Closed
wants to merge 1 commit into from

Conversation

tiulpin
Copy link

@tiulpin tiulpin commented Nov 26, 2024

This PR introduces a new rule terraform_map_duplicate_values to the TFLint ruleset that disallows duplicate values in a map object.

The use-case is the following: I use maps for storing configuration data (e.g., SSM parameters), and I would like to make sure no duplicated values are added there.

  • This rule is not supposed to be enabled by default but can be helpful to be used on some files.
  • This rule ignores values "true" / "false"
  • Even in my case, there could be strings that I want to be repeated, so using # tflint-ignore: terraform_map_duplicate_values still works fine

Maybe it's a too niche case, happy to hear your thoughts and address any questions you might have!

New Rule Implementation:

  • rules/terraform_map_duplicate_values.go: Added the TerraformMapDuplicateValuesRule to check for duplicate values in map objects. This rule evaluates expressions to identify and report any duplicate values. Basically, mimics the similar rule that was already here for map keys.

Rule Registration:

  • rules/preset.go: Registered the new TerraformMapDuplicateValuesRule in the preset rules map.

Documentation:

Tests:

@bendrucker
Copy link
Member

I don't see this fitting in here.

Duplicate keys are strictly incorrect for all maps. The language could forbid them.

Duplicate values are indicative of an issue (a copy-paste error) but not conclusively. And so it's not enforceable at a language level, only in specific contexts—certain resources/attributes.

The goal of this ruleset is enforcement of documented best practices. Configurable style/policy rules is a non-goal. I don't see any way we can call this the former and not the latter.

@tiulpin
Copy link
Author

tiulpin commented Nov 26, 2024

@bendrucker that's why I wrote it's a niche use-case, that's why it's not enabled by default but could be useful for some users / cases.

So I'm totally fine if this PR does not get accepted into this ruleset, though I'm definitely keeping maintaining it for my own usage. At least this PR will stay in the repository history and could be looked up later if somebody may need such a rule.

@bendrucker
Copy link
Member

it's not enabled by default but could be useful for some users / cases

The bar in this ruleset is that every rule should be enabled if you wanted to strictly follow optional Terraform guidance. This doesn't meet that bar, so do go ahead and publish it in your own separate ruleset.

@bendrucker bendrucker closed this Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants