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

Support Terraform 1.0 #937

Closed
wata727 opened this issue Oct 4, 2020 · 13 comments · Fixed by #1141
Closed

Support Terraform 1.0 #937

wata727 opened this issue Oct 4, 2020 · 13 comments · Fixed by #1141
Assignees

Comments

@wata727
Copy link
Member

wata727 commented Oct 4, 2020

Mentioned in hashicorp/terraform#26418, the Terraform team decided to internalize all of the Go packages in v0.15.0. This means that we can no longer import packages under github.com/hashicorp/terraform and can't build TFLint with Terraform v0.15.0.

TFLint depends on Terraform's internal API for loading configuration files and evaluating expressions, so we need to think of a strategy in response to this decision.

1. Drop some features of TFLint to avoid the dependency on Terraform

The supported interfaces for Terraform are CLI and HCL language, so it can be remade to depend only on these. In that case, it becomes difficult to offer provider rules (such as aws_instance_invalid_type). A workaround is to load the plan file instead of the configuration files.

These changes can be inconvenient for users because they can't do what they were able to do before, but they serve the goal of avoiding Terraform dependencies.

2. Create "libterraform" (Terraform as a library) and switch to this

We will develop a library that provides the same functionality as Terraform, which I was initially considering. This is the only way we can support v0.15.0 and above without changing any functionality. However, maintaining this library is probably not easy.

3. End of the maintenance 👋

I already knew that if package internalization was done, it would be difficult to continue the project.
We can take this opportunity to end the project. If this project is really needed by the community, you may negotiate with the Terraform team and come up with some good ideas.

@bendrucker
Copy link
Member

A workaround is to load the plan file instead of the configuration files.

This would be valuable but almost makes TFLint into an entirely new tool. Putting TFLint into the plan file pipeline makes things potentially quite complicated for users. If you use Terraform Cloud, plans are generated on each pull request commit and reported in GitHub checks. TFLint would need to run after those plans. A working CI pipeline would be pretty complex. It also brings TFLint quite close to Sentinel which is directly integrated with TF Cloud.

I'd want to fully flesh this concept out and have instructions, GitHub actions, etc. available to ease this integration if it's even possible.

There's always some need for configuration inspection too, to implement style features. I can see cutting out things like expression evaluation to focus on:

  • Generic HCL rules
  • Plan inspection

That would hopefully eliminate the need for a libterraform that implements Terraform semantics and just rely on the hcl packages.

@KyungUnChoi
Copy link

Hi,
It sounds like a big change, so I have a few questions.
What is the timeline of the change (like moth)?

It is hard to connect dots for me, so here is other question.
What else will be the challenges using a plan file other than working in CI pipeline?
I am thinking that using a plan file might be easier to implement rule like aws_instance_invalid_type cause all key values are ready evaluated.

@wata727
Copy link
Member Author

wata727 commented Oct 6, 2020

@bendrucker

This would be valuable but almost makes TFLint into an entirely new tool

Agreed. I've listed it as an idea, but I'm not positive about this one. In this regard, it may be better to create a new tool...
In my opinion, right now, 2 might be best. It's a big challenge, but I think it's still worthwhile to keep its current behavior.

@KyungUnChoi

What is the timeline of the change (like moth)?

Nothing has been decided yet. The purpose of this issue is to decide how to change it, when and what to do.

@bendrucker
Copy link
Member

There's also no public dates on HashiCorp, and this is planned for v0.15.0. They'll probably speed up the cadence a bit heading with smaller releases heading into 1.0, but there's some time.

@wata727
Copy link
Member Author

wata727 commented Dec 30, 2020

I heard that the terraform-ls team is looking at ways to achieve attribute access and expression evaluation without depending on the Terraform packages. I believe it's worth investigating the technologies in these repositories:
https://github.com/hashicorp/hcl-lang
https://github.com/hashicorp/terraform-schema

@circa10a
Copy link

circa10a commented Feb 2, 2021

A working CI pipeline would be pretty complex. It also brings TFLint quite close to Sentinel which is directly integrated with TF Cloud.

I am thinking that using a plan file might be easier to implement rule like aws_instance_invalid_type cause all key values are ready evaluated.

I saw these 2 comments and instantly thought of OPA's Terraform Integration which essentially does this. The terraform plan is loaded in as a map called input. Then you write your own rules using rego that traverse through the map and inspect each attribute and it's values. Example policy

Just wanted to put this out here so that the wheel isn't reinvented. Personally, I'd prefer a linter that would only be able to look at terraform source code and not a rendered plan since tools already exist for it.

@bendrucker
Copy link
Member

bendrucker commented Feb 2, 2021

Personally, I'd prefer a linter that would only be able to look at terraform source code and not a rendered plan since tools already exist for it.

This is where my head's at as well. TFLint seems best suited to enforcing style and usage conventions in a user-friendly way. If users want absolute assurances that a policy is followed, whether that's a security rule, inclusion of cost-tracking tags, etc., one of OPA, Sentinel, or other plan analysis tools out there can handle that.

Checking a plan starts to step beyond the boundaries of "linting" as far as I'm concerned. A plan has two inputs: state and configuration. Configuration is user input that should be validated (linted), while the state is external data where "linting" would not naturally apply.

I haven't had a chance to deeply explore the new HashiCorp repos as they spin off internal bits of HCL and the Terraform language as official packages, but I definitely expect that's the direction we'll go.

@bendrucker
Copy link
Member

https://github.com/hashicorp/terraform-schema is doing a good job of offering the Terraform schema for full decoding, but we won't be able to evaluate expressions that use Terraform's internal functions if those packages are not exposed. Evaluating expressions is already challenging and incomplete, see #571.

It seems like it should be possible to at least provide static evaluation in rules like aws_instance_invalid_type, or perhaps even variable defaults as a special case. If so it seems like we could move forward with this transition. This would mainly consist of rewrites of TFLint and the plugin SDK to reference Terraform types defined in TFLint, populated by our own HCL decoding of the config.

Thoughts @wata727? Happy to start looking at this and spending some time building with the new TF/HCL packages.

@wata727
Copy link
Member Author

wata727 commented Apr 30, 2021

Agreed. Compatibility of expression evaluation is a hard problem, but I think that it can be almost reproduced by reimplementing scope.EvalContext and using go-cty's stdlib as much as possible.

By the way, how the terraform-ls team plans to reproduce Terraform's semantics, including expression evaluation, is an interesting question. If it is a reusable way, it may also help our implementation. However, at this point, reimplementing it using terraform-schema etc. is probably the best way to do it.

@bendrucker
Copy link
Member

Oh nice, great to see more functions heading into HCL's stdlib. There are still many in TF:

https://github.com/hashicorp/terraform/tree/main/lang/funcs

Reimplementing EvalContext shouldn't be too bad.

I spent a few minutes reading through terraform-ls and I'm not sure it actually does any expression evaluation. The language server has to be able to understand the structure of a document/expression but it doesn't seem to understand the relationship between different blocks or attributes or really anything about values beyond their syntax (for highlighting). I have resource attributes auto-completed via the provider schema but if I try to reference one resource from another, there's no completion. Same thing if I start typing local..

@wata727
Copy link
Member Author

wata727 commented May 24, 2021

Terraform v0.15.4 has been released. This release includes a patch about Go package internalization. We probably need to move this issue forward to support the next major version.

@bendrucker Any progress on this? I'm thinking of working on this in the near future.

@bendrucker
Copy link
Member

Haven't had a chance to work on this at all yet

@wata727
Copy link
Member Author

wata727 commented Jun 23, 2021

I was investigating terraform-schema and found it difficult to use the schema in TFLint as it is only supposed to behave as part of hcl-lang now.

Currently, I think it's a good idea to partially reimplement Terraform's internal packages to solve this issue. However, instead of reimplementing everything like a fork, I'm also considering focusing on the bare minimum of linter's functionality and, in some cases, rewriting it to behave as desired for TFLint.

For example, TFLint does not need to know the full schema because each rule knows the structure of the schema for the resources and configurations that want to check. This means that we only have to reimplement some of these schema, which results in better compatibility and maintainability.

With this in mind, the SDK's API will be closer to hcl's Content. Imagine an interface like this:

runner.ResourceContent("aws_instance", &sdk.schema{
  Blocks: []sdk.BlockSchema{
    Type: "ebs_block_device",
    Schema: &sdk.schema{Attributes: []sdk.AttributeSchema{Name: "volume_type"}}
  },
  Attributes: []sdk.AttributeSchema{Name: "instance_type"},
})

This also resolves the known issue that hcl.Body is difficult to transfer over RPC. This is mentioned in terraform-linters/tflint-plugin-sdk#89

The tricky part is dealing with analysis that takes Terraform Language semantics into account, such as merging configs, dynamic blocks, and skipping conditional resources. However, this is an API design issue, for example, there is a way to be able to control whether to retrieve the merged or the raw content as a option of the ResourceContent.

Ultimately, TFLint should only need to decode variables and module calls, and treat the rest as hcl.File. If only this, the cost of maintaining language semantics may be acceptable.

I'm thinking of starting by importing a simple copy of the Terraform internal packages used by TFLint. This includes redundant implementations, but ignore them to keep the interface. After that, while changing the interface, I will cut down the copied implementation as much as possible and refactor it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

4 participants