Order of argument definitions (variable, output, module)

[nitrocode]

It would be nice if we can create a linting rule to enforce the order of arguments for vars, outputs, and modules

terraform_module_argument_order

rule "terraform_module_argument_order" {
  enabled = true
  order   = ["source", "version", "count", "for_each"]
}

This would work

module "example" {
  source  = ""
  version = ""
}

This would fail

module "example" {
  version = ""
  source  = ""
}

terraform_variable_argument_order

rule "terraform_variable_argument_order" {
  enabled = true
  order   = ["type", "description", "default", "sensitive", "validation"]
}

This would work

variable "example" {
  type        = "string"
  description = ""
  default     = ""
  sensitive   = true
  validation {}
}

This would fail

variable "example" {
  description = ""
  type        = "string"
  default     = ""
  sensitive   = true
  validation {}
}

terraform_output_argument_order

rule "terraform_output_argument_order" {
  enabled = true
  order   = ["description", "value"]
}

This would work

output "example" {
  description = ""
  type        = ""
}

This would fail

output "example" {
  type        = ""
  description = ""
}

[nitrocode]

cc: @PatMyron could you add some comments?

[lonegunmanb]

Hi @wata727 , we've tried to implement such rule for resource and data block, but arguments' order in those two blocks depends on argument's required/optional property, which can only be fetched via provider's source code. We'd like to implement such rule in our own plugin so we can reference provider's source code there, but we found that Runner type in plugin sdk is different than Runner in tflint core. Is there any way for us to leverage powerful Runner.TFConfig information in plugin project to save effort? Many thanks!

[wata727]

arguments' order in those two blocks depends on argument's required/optional property,

You can see their order from hcl.Range. Is this not enough?

[lonegunmanb]

Hi folks, we're developing two new plugins for your Terraform Module repos:

• basci-ext
• azurerm-ext

For now we're still consider them as beta version and we're adding more rules to them. If we can have @wata727 's permission we'll be happy to contribute these rules to this plugin.

[wata727]

Interesting. If you open a pull request, I'd be happy to review it. Please open a pull request per rule.

[lonegunmanb]

Interesting. If you open a pull request, I'd be happy to review it. Please open a pull request per rule.

Great! Will do.

[bendrucker]

Spotted #27 and #29, I'm concerned about the logic and positioning of these rules.

Recommend proper order for variable blocks
Recommend proper order for variables in locals blocks.

These are personal opinions.

They are not official best practices (i.e. described/mentioned in the Terraform docs), which is the standard I'd recommend for this ruleset.

They are also not generally accepted community conventions (which, FWIW, I still would not recommend including). It is far more common in my experience to organize locals and variables according to topic/function, grouping related entries together.

I'm not disagreeing with these opinions, every module/project/organization is free to make these stylistic and organizational choices that Terraform leaves open. But for TFLint to have its own opinion on what is "proper" in an official/core ruleset, is decidedly wrong to me.

I don't consider enforcement of personal/org stylistic preferences a priority that merits inclusion in an official ruleset, but if they were included I'd suggest a rename:

• terraform_variables_alphabetical_order
• terraform_locals_alphabetical_order

Again, I don't think it's TFLint's job to offer tools to enforce personal style preferences in this ruleset, but if it's going to offer to enforce this it should be without bias.

[wata727]

Thank you for pointing it out. I think this concern is worth considering.

Personally, I'm not a fan of sorting definitions alphabetically. The order of definitions should be carefully designed to reflect the intent of the module designer. However, if alphabetical ordering is the best way to express the intent of the module designer, I think there is some value in enforcing this rule.

The issue is whether to add this rule to this ruleset. I haven't been very opposed to including personal rules that might be shared between several people, except the recommended preset. However, if there is a possibility that this rule is misunderstood by users as TFLint's recommended rule, I think it should be carefully designed to prevent this from happening.

I think there are several ways:

• Change rule name and documentation
• Extract these rules as another "opinionated" ruleset
• Provides a plugin that allows user-defined policies
  • See Custom rules without writing plugins tflint#1358

In my opinion, I believe the last option is the most flexible and may solve many problems.

[wata727]

I think we should make a new plugin tflint-ruleset-policy to solve this kind of issue.

This plugin focuses on enforcing organization-specific policies, such as definitions order, banned resources, etc. The tflint-ruleset-terraform will continue to only deal with official or widely agreed upon best practices and error-prone rules.

In the future, tflint-ruleset-policy will provide a policy definition in Rego, allowing users to enforce their own policies with less code. Until then, we'll provide some generic rules to give you the flexibility to define policies through configuration. The ordered rules should also be able to be included in this plugin.

@bendrucker @lonegunmanb What do you think?

[lonegunmanb]

@wata727 I have two questions.

As different organization may has different poilcies, how do we determine which rule should be added into this new plugin? We can add them all and make them turn off by default, let the users choose the rules they want, then why don't we do the same thing in this plugin? If we decide to create this new plugin, then we need a triage procedure, any new rule should descibe their intension, let the community to decide whether we'll accept it, or this rule should be committed to this new plugin before someone start the coding work, or his contribution might be rejected.

For the rules I've contributed to this repo, I'm ok if we decide to move them into this new plugin because I've already implemented them in our own plugins and we're using them now.

As for the Rego policy, the community now have some mature tools based on Rego, such as ConfTest、Opa Terraform or even TfSec. I choose TFLint because it is capable to analyze my code in AST level, or even in token stream level, I can write policy that is hard to express in Rego. Why don't we leave these Rego policies to these tools? Could TFLint with Rego policy provide any extra values that these tools cannot provide? Just a personal thought, anyway the TFLint is the best choice I've made for my module projects.

One more thing, I know that there's a code style guidance drafted by HashiCorp, but they havn't pulished it yet. We have implemented some of them in our own plugins. We'll be happy to contribute them to this plugin once the official guidance is published.

[wata727]

how do we determine which rule should be added into this new plugin?

Community members can comment on which plugins to add rules to, but the final decision rests with the maintainers.

We can add them all and make them turn off by default, let the users choose the rules they want, then why don't we do the same thing in this plugin?

I used to have this same opinion. But lately I've been leaning towards the idea that tflint-ruleset-terraform should only contain minimal best practices and error-prune.

The tflint-ruleset-terraform is used by almost all TFLint users, and I know from experience that some of them try to mechanically follow all the rules as much as possible. In that case, I believe it makes some sense to clearly separate policies that the maintainers cannot agree with into another plugin.

If we decide to create this new plugin, then we need a triage procedure, any new rule should descibe their intension, let the community to decide whether we'll accept it, or this rule should be committed to this new plugin before someone start the coding work, or his contribution might be rejected.

This is still the case today. We cannot accept all rules in tflint-ruleset-terraform.

Why don't we leave these Rego policies to these tools? Could TFLint with Rego policy provide any extra values that these tools cannot provide?

Unlike Conftest and OPA, TFLint does not require to runterraform plan, which is a big difference in the experience. There is other AST-level information that can be passed directly to the input, so there may be more information available for the policy than these.

One more thing, I know that there's a code style guidance drafted by HashiCorp, but they havn't pulished it yet. We have implemented some of them in our own plugins. We'll be happy to contribute them to this plugin once the official guidance is published.

Interesting. I'm personally not that interested in enforcing code style, but if it's widely accepted it might be worth including in tflint-ruleset-terraform.

Finally, thank you for participating in this discussion. I believe that this discussion was born from your contributions and that it has inspired the project to move forward.

[lonegunmanb]

I fully understand the challenges that TFLint meets now so I'm ok no matter how things go. It's me who owes you and the TFLint community a big thanks, without your magnificent work it'll be impossible for us to manage hundreds of Terraform modules in the future.

I found one big difference between the general programming language's function and Terraform module, that is most of Terraform modules are scenario specified. For example, an organzation may have multiple modules which provision vpc and subnets, it's very common for this organzation to maintain hunndreds of modules, in that case it'll be very hard for teams to maintain them while preserving personal style. In that case maybe it'll be better to enforce a strict code style policy to keep consistence among these modules. Anyway I agree with you that we might leave this choice to our users.

[bendrucker]

Having a policy ruleset isolates style opinions, which is better than having them here. I still think it'll face a similar problem, which is lack of consistency. Even if rules are fairly generic, the things they enforce will be a reflection of the contributors personal/team preferences.

I certainly agree that style enforcement in increasing detail is valuable in an organization trying to maintain lots of modules. I just think the natural starting solution is for each organization to maintain their own ruleset plugin with their organization-specific styles. If a few organizations had these and published them publicly, either:

• Users could just use these plugins directly, including in combination, instead of writing their own
• We'd have some actively tested rulesets to examine to consider whether merging them into a community policy or style ruleset would make sense

[wata727]

Thank you for your comments. I think we need some more time to think about this issue.

For now, revert the terraform_ordered_locals rule. If you need these rules, https://github.com/Azure/tflint-ruleset-basic-ext is available. Thank you for @lonegunmanb's work.

I'm still not sure if such a rule should be in the terraform ruleset or a policy ruleset. However, since TFLint has a plugin system, even if this decision is postponed, you can create a plugin and use any rules.

First of all, I'm thinking of trying the possibility of Policy as Code by Rego.

[bendrucker]

Thanks, I think it's the right move for now to not focus energy on style guidelines. The idea of generic rules based on something like Rego is interesting. But even that is focused on data and not syntax. Enforcing non-behavioral style rules will require HCL-level logic.