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

Git config like .tflint.hcl files combination #1536

Open
pjroth opened this issue Oct 5, 2022 · 5 comments
Open

Git config like .tflint.hcl files combination #1536

pjroth opened this issue Oct 5, 2022 · 5 comments
Labels
enhancement needs-design Detailed design is required for implementation

Comments

@pjroth
Copy link

pjroth commented Oct 5, 2022

Introduction

I have a git repo with lots of directories that represent my deployed infrastructure. I'm using the Gruntwork infrastructure-live setup as can be seen in their example.

In my directories that start with /dev which represent my dev infrastructure. I want to allow main as a target for terraform module source lines so that I can deploy to DEV work that is not yet tagged/released such as:

module "my-app" {
  source                     = "git::ssh://[email protected]/my-org/my-repo.git//terraform?ref=main"

  // etc
}

In my qa and prod directories, I want to allow only released version tags and NOT allow main such as:

module "my-app" {
  source                     = "git::ssh://[email protected]/my-org/my-repo.git//terraform?ref=1.2.3"

  // etc
}

This means I need to configure tflint differently depending on which directory tree I'm under. Specifically I want to disable the rule terraform_module_pinned_source for my dev environment but enable it for my qa and prod environments.

Proposal

Allow .tflint.hcl files to "combine" if multiple files exist in defined locations. This would happen by looking "up" directories until the root is reached combining them into one where the more specific directory takes precedence. This would allow me to put "common" config into my home directory the root of /dev to disable the rules I need to disable for my dev files

A slightly different solution is to not look in all directories, but define some locations, perhaps GLOBAL -> HOME DIR -> current directory. This solution is inspired withs how git config files work. Copying the git config solution would be very nice and consistent since many users are likely familiar with git config.

Thank you for this great tool!

References

  1. https://git-scm.com/docs/git-config#FILES
@bendrucker
Copy link
Member

I've mentioned this in the past:

#841 (comment)

Walking upward is simple/consistent.

I played with it on a branch a while back and just didn't finish. The facilities to merge configs are there.

@wata727
Copy link
Member

wata727 commented Oct 6, 2022

I believe it should be solved by #1355.

As mentioned in #1355 (comment), walking upward may make new confusion. (See ESLint's blog post)
I think the "flat config" idea is a good solution to this issue.

@bendrucker
Copy link
Member

In a large repo, putting the config at the top level leads to bloat and divorces the config from the relevant context (module). In other words, if you want to customize something for one module and then later decide to delete that module, you're likely to orphan the config at the top level.

Hierarchical configuration is still encouraged by ESLint:

https://eslint.org/docs/latest/user-guide/configuring/configuration-files#cascading-and-hierarchy

Globs are part of overrides and are pitched more as an escape hatch for things not readily accomplished by hierarchy:

Sometimes a more fine-controlled configuration is necessary, for example, if the configuration for files within the same directory has to be different.

The blog post takes a very different tone and I can imagine the support burden coming from users who don't understand their own setup. But it's not in production (in the CLI) so this is more vision than reality at the moment.

The posts also don't really address monorepos at all. The given globbing examples target directory patterns (e.g., test/), extensions, etc. A JavaScript monorepo would have multiple packages (with package.json). Here's the top Google hit for eslint javascript monorepo`:

https://turbo.build/repo/docs/handbook/linting/eslint

It calls for a shared config package to be published to npm and then installed into each package in the monorepo. The Terraform take on that might be:

# ./my/module/.tflint.hcl
extends = [
  "../../.tflint.hcl",
]

Meaning, rather than navigating upwards, you'd have to explicitly reference a parent config with extends. Inevitably people complain about ../.. and want git-awareness, which I'd argue is inappropriate.

If you assume the config file will exist at an ancestor directory and apply to modules via recursion, you lose the ability to cd ./my/module && tflint.

@wata727
Copy link
Member

wata727 commented Nov 4, 2022

Agreed with the concerns about the divorce of the config from the module context, but this is the same for the Git config. In a deeply nested directory structure, it can be very difficult to keep track of which config is ultimately applied to the module.

├── .tflint.hcl
├── aws
│   ├── .tflint.hcl
│   ├── modules
│   ├── product1
│   │   ├── dev
│   │   ├── modules
│   │   ├── prod
│   │   │   └── .tflint.hcl <-- /.tflint.hcl and /aws/.tflint.hcl actually apply, but you should also check /aws/product1/.tflint.hcl
│   │   ├── qa
│   │   └── staging
│   ├── product2
│   └── product3
├── azure
└── google

Explicitly putting the config in one place relieves this complexity.

├── .tflint.hcl
├── aws
│   ├── modules
│   ├── product1
│   │   ├── dev
│   │   ├── modules
│   │   ├── prod
│   │   ├── qa
│   │   └── staging
│   ├── product2
│   └── product3
├── azure
├── google
└── tflint
    ├── aws.hcl
    └── product1_prod.hcl

However, I understand that this design is bad if almost every module needs another config. In that case, placing .tflint.hcl under each module would be appropriate.

The reason I support this design is that I believe it's rare that every module needs another config. The company I work for has a directory structure like the one above, and the reason I want to apply a different config is to change the rules that apply in the root module and child modules.

├── aws
│   ├── modules
│   ├── product1
│   │   ├── dev
│   │   ├── modules  <-- child module rules
│   │   ├── prod     <-- root module rules
│   │   ├── qa
│   │   └── staging
│   ├── product2
│   │   ├── dev
│   │   ├── modules  <-- child module rules
│   │   ├── prod     <-- root module rules
│   │   ├── qa
│   │   └── staging
│   └── product3
├── azure
└── google

The Git-like config doesn't work well for this example. With the flat config, you only have to manage two types of config (for root and child modules) and apply them appropriately by globs.

ESLint introduces root: true and overrides to support git-awareness, which is very complex and should be avoided if possible. I know the flat config is untested in production, but I think it's worth looking into.

For monorepo and shared config issues, this is a tough one. I agree that ../../.tflint.hcl is redundant, but I prefer explicit over implicit. I also don't want to lose the ability of cd ./my/module && tflint. Maybe we need find_in_parent_folders like Terragrunt?

@wata727 wata727 added the needs-design Detailed design is required for implementation label Dec 28, 2022
@wata727
Copy link
Member

wata727 commented Dec 30, 2022

I take a slightly different position on the subject now, since we didn't adopt a flat config for recursive inspection.

Basically, I support the policy of walking upward. Instead of introducing root: true, it might be better to adopt the original working directory in --chdir/--recursive as the root.

However, I may change my policy if the recursive inspection is implemented differently in the future. Now I would like to see how the recursive inspection added in v0.44 are used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement needs-design Detailed design is required for implementation
Development

No branches or pull requests

3 participants