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

Introduce "preset" config #3

Merged
merged 1 commit into from
Aug 20, 2022
Merged

Introduce "preset" config #3

merged 1 commit into from
Aug 20, 2022

Conversation

wata727
Copy link
Member

@wata727 wata727 commented Aug 17, 2022

See also terraform-linters/tflint#1056

This PR introduces the concept of "preset" to the ruleset config. The preset is for naming a group of rules so that you can use the rules you need without having to configure each rule individually. The idea was inspired by ESLint's "extends": "eslint:recommended".
https://eslint.org/docs/latest/user-guide/configuring/configuration-files#extending-configuration-files

This enables all rules that were previously disabled by default, and makes all rules available if not set. If you want to enable only some rules as before, set "recommended" to preset.

plugin "terraform" {
  preset = "recommended"
}

For compatibility, default to adopt preset="recommended" if no plugin "terraform" is declared. In other words, the rules that are enabled by default aren't much different than they used to be. However, at this time, we will review the rules that correspond to "recommended", so there may be differences in behavior.

rules.NewTerraformTypedVariablesRule(),
rules.NewTerraformUnusedDeclarationsRule(),
rules.NewTerraformUnusedRequiredProvidersRule(),
rules.NewTerraformWorkspaceRemoteRule(),
Copy link
Member Author

Choose a reason for hiding this comment

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

Unlike the current settings, the following are enabled by default:

  • terraform_deprecated_index
  • terraform_required_providers
  • terraform_required_version
  • terraform_typed_variables
  • terraform_unused_declarations
  • terraform_unused_required_providers

These are the rules that I consider to be recommended even for senior engineers by my standards.

However, there are issues with https://github.com/terraform-linters/tflint/issues/1328 and https://github.com/terraform-linters/tflint/blob/v0.39.3/docs/rules/terraform_unused_declarations.md#how-to-fix, and the checks aren't perfect, so I'm not sure if these should be the default.

@bendrucker What do you think? If you have any other thoughts on the default rules, please let me know.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I'd say terraform_unused_required_providers needs to have the issue of implicit provider passing into modules addressed before it comes on by default. The others seem like good defaults for a "recommended" config.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, we will not include terraform_unused_required_providers in the default for now.

@wata727 wata727 requested a review from bendrucker August 17, 2022 17:47
Copy link
Member

@bendrucker bendrucker left a comment

Choose a reason for hiding this comment

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

Nice! I think the concept of ruleset presets works well.

@wata727 wata727 marked this pull request as ready for review August 20, 2022 13:13
@wata727 wata727 merged commit 8dbed4f into main Aug 20, 2022
@wata727 wata727 deleted the preset_config branch August 20, 2022 13:18
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