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.