terraform-linters / tflint-plugin-sdk Goto Github PK

Hey,

I'm struggling with the sdk to get an Expr as a raw string without evaluating it.

Here's an example:
I want to make sure that all the vault providers are configured the right way, it should look like:

provider "vault" {
  address = var.vault_addr
}

or

provider "vault" {
  address = module.vault_finder.address
}

But I'm struggling to get var.vault_addr or module.vault_finder.address

I was able to get a splitted view of the Expr by iterating along the Traversers but it's clearly not convenient...

func AddressFieldDefinedInVaultProvider(attributes hclext.Attributes) bool {
	for _, v := range attributes {
		for _, v1 := range v.Expr.Variables() {
			for _, v2 := range v1 {
				switch v2.(type) {
				case hcl.TraverseAttr:
					logger.Debug(v2.(hcl.TraverseAttr).Name)
				case hcl.TraverseRoot:
					logger.Debug(v2.(hcl.TraverseRoot).Name)
				}
			}
		}
	}
	return false
}

Am I missing another obvious solution?

I'm looking to implement a rule for a custom provider but i need to be able to identify all the resources of a given type for one of my checks. I know the core plugins can directly iterate over the ManagedResources https://github.com/terraform-linters/tflint/blob/99035e1be1b0c5158999c3e9185ab419fe03878d/rules/terraformrules/terraform_deprecated_interpolation.go#L45 is this something that could be exposed in the sdk?

EDIT:
I have done tests using the runner.WalkResourceAttributes to iterate over all of the resources but all the errors get raised on the attribute i search for not the resource itself - which is mislead in my case.

A WalkResource would work I believe?

plugin "pingaccess" {
    enabled = true
    version = "0.0.1-alpha"
    source  = "github.com/iwarapter/tflint-ruleset-pingaccess"

    signing_key = <<-KEY
    -----BEGIN PGP PUBLIC KEY BLOCK-----

    mQINB...
    -----END PGP PUBLIC KEY BLOCK-----
    KEY
}

Version

$ tflint --version
TFLint version 0.30.0
$ tflint --init                                  
Installing `pingaccess` plugin...
Installed `pingaccess` (source: github.com/iwarapter/tflint-ruleset-pingaccess, version: 0.0.1-alpha)
$ tflint                                         
Failed to apply config to plugins. An error occurred:

Error: .tflint.hcl:59,8-8: Unterminated template string; No closing marker was found for the string.

line 59 is the final KEY marker for the heredoc on the signing_key attribute. Once installed I can remove the signing_key block and the plugin has been installed and works correctly, it just doesn't seem to be able to parse the signing_key heredoc.

This ruleset is currently private whilst I finish testing before OSS'ing it, and this is the last test 😂

Current protocol versioning is not stable enough. For example, imagine adding a new API to the server.

Plugin A, which uses the new API, wants to check if TFLint supports the new API when it starts, and stops with an error if it doesn't. On the other hand, Plugin B does not use the new API, so it will still work with older TFLints.

This behavior is ideal, but unfortunately, it cannot be supported with the current protocol versioning. The protocol version of go-plugin used in TFLint is just a number. If we bump the version for plugin A, plugin B will stop working with the old version, and if we don't bump the version for plugin B, plugin A doesn't know if it's supported until it actually calls the API.

To solve this, we need semantic versioning. The server has a single version in x.y.z format, and plugins can check the version by a constraint. In the above example, the server updates the protocol version from 1.0.0 to 1.1.0. Plugin A uses ~> 1.1 as a constraint and Plugin B uses ~> 1.0 as a constraint.

To achieve this, we need go-plugin support. Fortunately, go-plugin has semantic versioning on its roadmap, but it looks like it hasn't been updated in years.
https://github.com/hashicorp/go-plugin/tree/v1.4.5#roadmap

Currently, hcl.Body is transferred between the host and the plugin in the same way as hcl.Expression (serialization by source code and start position). However, this way has the problem that the merged body cannot be transferred because the range of the merged body spans multiple files.

The solution is to expose an interface like body.Content to the plugin side and get the body content based on the schema specified by the plugin in advance, instead of transferring the whole body. See hashicorp/hcl#332 (comment)

So we need to change the methods of the Runner interface:

• WalkResourceBlocks
• WalkResources
• WalkModuleCalls
• Backend
• Config
• RootProvider

This change plan is a draft. In fact, it may be affected by Terraform's package internalization.

Currently, the WalkResourceAttributes and WalkResourceBlocks API don't support JSON configuration syntax. To support this, the following features need to be merged in the HCL. This is a tracking issue.

hashicorp/hcl#381
hashicorp/hcl#389

Hi,

I have a resource which has several sibling blocks, i'd like to be able to implement a rule that check if a block contains a attribute/value and if thats true then check another block.

E.g. if resource x block y attribute z equals some thing then check that resource's block abc attributes

resource "x" "example" {
  y {
    x = "some thing"
  }

  # we should then check this block
  abc {
    foo = "a conditionally bad thing todo depending on above block"
  }
}

I can do the first part with WalkResourceBlock but I then have no context for the current resource to check it's sibling

The SecureConfig is a mechanism for verifying checksums at runtime, provided by go-plugin.
https://pkg.go.dev/github.com/hashicorp/[email protected]#SecureConfig

To make use of this, we first need to save the checksum of plugins. It's probably a good idea to first introduce a dependency lockfile. See also terraform-linters/tflint#1486

Hey,

In order to enforce terraform folders location (for example .infra at the root of git repo), I'd like to be able to get the root module absolute path. I can't find another solution than getting the current workdir (which can be wrong if tflint is executed from another directory).
There's the GetFiles() function but it only give the filenames.

Is there another way?

See also terraform-linters/tflint#1477

The current inspection model is supposed to run per module. The advantage of this model is that rules don't have to know about whether they are inspecting the root module or child modules. This worked fine in early designs because the module context is irrelevant when checking for invalid instance types.

However, this design has an issue when implementing rules that depend on the child modules such as terraform-linters/tflint#1477. There are other issues as well. Currently, whether or not an issue emitted from a child module is propagated to the final result is determined by whether or not the module's input variables are included in the range, but this is not trivial.

To address these issues, I think we need to change our current inspection model to inspect a module tree instead of a module. However, we must consider the following issues:

• Incomplete module tree
  • The module tree is not always complete, unlike Terraform. Sometimes all modules are not included, sometimes only certain modules are ignored. An interface should be provided so that the rule can determine this and implement it appropriately.
• Consistency of checking against child modules
  • The check against a child module is consistent whether the range of the issue includes the module's input variables. However, allowing rules to control this behavior can lead to inconsistencies and confusion for end users. Also, parsing a module's call tree and emitting an issue to the caller may require a complex interface.
• Syntactic sugar for rules that don't require module context
  • Not all rules require a module tree, and simple rules should be implemented without concern. We'll need some syntactic sugar to accomplish that.

Introduction

We have a scenario where we need to check if a resource attribute value is unknown.

The current runner.EvaluateExpr method needs a go type, which makes this difficult to ascertain as the conversion from cty to go types loses this information.

Proposal

An additional method for the tflint.Runner interface that allows expression evaluation but returns a cty.Value.

The caller can then make its own assertions on this value.

Alternatively

A way to return a fully populated hcl.EvalContext from the runner for use in (hcl.Expression) Value()

References

I need to low-level iterate all files for style checking, but I can't see the list being accessible.
I read the intention of File(string) is such a use case, also there is an unexported list under runner.data.Files available, but I'm struggling to grasp their access.

Use case is to determine if a heredoc syntax has been used.

Thank you for help @wata727

Given the following providers.tf file:

provider "azuread" {
    tenant_id = "00000000-0000-0000-0000-000000000000"
}

provider "databricks" {
}

I'm working on a tflint plugin to validate the requirements in our declared providers. I can see the providers were parsed correctly but there are no public methods to expose them.

Is there any way to walk through providers in order to validate their attributes?

Introduction

I am writing a rule to check aws provider configuration. Specifically, I would like to assert that aws providers have certain default_tags values set.

The runner interface has a GetResourceContent to make it easier to get resources (without using GetModuleContent). I would appreciate a function that gets the providers within the module.

Proposal

type Runner interface {
    GetProviderContent(providerName string, schema *hclext.BodySchema, option *GetModuleContentOption) (*hclext.BodyContent, error)
...
}

References

We have created an internal tflint plugin which checks checks for our naming convention on all resources in a state. With the update to the new v10 plugin sdk I noticed that some resources are skipped when they include for example a count.

In the following example only the aws_eip.two is evaluated:

variable "test" {
  type = number
}

resource "aws_eip" "one" {
  count    = var.test
  vpc      = true
  instance = "i-123456789"
}

resource "aws_eip" "two" {
  vpc      = true
  instance = "i-123456789"
}

aws_eip.one is skipped with the following warning:

16:40:36 runner.go:317: [WARN] Skip walking `aws_eip.one` because it may not be created

I'm getting a list of resources via GetModuleContent, this list doesn't include the skipped resources:

func (r *TerraformMyRule) Check(runner tflint.Runner) error {
	content, err := runner.GetModuleContent(&hclext.BodySchema{
		Blocks: []hclext.BlockSchema{
			{
				Type:       "resource",
				LabelNames: []string{"type", "name"},
			},
		},
	}, nil)

Is there any option to disable this skipping behaviour for specific plugins?

Similar issue in tflint: terraform-linters/tflint#1068

Some rulesets redefine Check() to support custom runners in rulesets. e.g.
https://github.com/terraform-linters/tflint-ruleset-aws/blob/v0.16.1/aws/ruleset.go#L46-L59
https://github.com/terraform-linters/tflint-ruleset-google/blob/v0.19.0/google/ruleset.go#L46-L59

What we want to do here is inject a custom runner, so it's nonsense to redefine checking.

Define a NewRunner function in RuleSet so that built-in rulesets always use that function. This allows custom rulesets to inject a custom runner by simply redefining the NewRunner(). No need to redefine Check().

Issue:
From what I've come across about this method, it should return the path of the child module from the terraform tree. However, it's returning nil when using it with the runner of any module.

Description:
path of childmodule is ./auth-module/main.tf
commands I ran is
terraform init only in root
terraform apply only in root
path/to/tflint --recursive --config path/to/.tflint.hcl --module --disable-rule=module_source in root
version of tflint 0.44.1
.tflint.hcl file is as follows
config {
disabled_by_default = true
}
plugin "developer"{
enabled = true
}
rule "flag_recommend"{
enabled=true
}

tflint ruleset plugin link:
https://github.com/trilogy-group/tflint-ruleset-template
The ruleset basically retrieves 2 files from env, and emits issues on resources blocks based on info in the 2 files.
I ran runner.GetModulePath as the first thing in check function, and it returned null for all tf file templates (including child).

Expected outcome:
when accessing module path using runner.GetModulePath inside the check function of my tflint-ruleset-plugin, it should return some information of the path to my childmodule, i.e., the terraform file inside ./auth-module/main.tf directory.

A few concerns regarding this issue to give a direction to issue elaboration :
Is it correct, that this method will provide path corresponding to the terraform project tree, and return a string containing ancestors?
How exactly is that information obtained by tflint, is there a special way of processing the terraform project for enabling tflint to generate that tree.
In code I saw, TFConfig.path to be containing this value, where is it getting that value from, tf file path specified in command?

I have this test and when I am trying to test for a type being specified in a variable I get this error:

--- FAIL: Test_TerraformDocumentedVariablesRule (0.00s)
panic: Failed to initialize runner: variables.tf:1,28-32: Unsupported argument; An argument named "type" is not expected here. [recovered]
        panic: Failed to initialize runner: variables.tf:1,28-32: Unsupported argument; An argument named "type" is not expected here.

It seems to do this with anything you should actually expect in a variable block like description and validate. I am not sure what I am doing wrong.

Here is my test (I've also tried formatting the Content on a new line and with proper terraform formating:

func Test_TerraformDocumentedVariablesRule(t *testing.T) {
	cases := []struct {
		Name     string
		Content  string
		Expected helper.Issues
	}{
		{
			Name:    "no validation",
			Content: `variable "no_validation" { type = string }`,
			Expected: helper.Issues{
				{
					Rule:    NewTerraformValidatedVariablesRule(),
					Message: "`no_validation` variable has no validations. Please include at least 1 validation for types that are not a bool.",
					Range: hcl.Range{
						Filename: "variables.tf",
						Start:    hcl.Pos{Line: 1, Column: 1},
						End:      hcl.Pos{Line: 1, Column: 25},
					},
				},
			},
		},
	}

	rule := NewTerraformValidatedVariablesRule()

	for _, tc := range cases {
		runner := helper.TestRunner(t, map[string]string{"variables.tf": tc.Content})

		if err := rule.Check(runner); err != nil {
			t.Fatalf("Unexpected error occurred: %s", err)
		}

		helper.AssertIssues(t, tc.Expected, runner.Issues)
	}
}

Can we get present modules information while parsing through the terraform code through tflint?
We need to find for each resource in which module that resource is made because same resource name can be present in different modules.

See terraform-linters/tflint#669

In order to achieve this goal, we need to be able to pass some meta-arguments to the plugin.

In TFLint, there are three patterns of expression evaluation results.

• Success. The inspection can be continued based on correct evaluation results.
• Failure. All inspections should be stopped because they are likely to give incorrect results.
• Known issues. You don't have to stop all inspections, but you should skip this rule.

It was a difficult problem to handle these 3 results appropriately. The interface had to be designed so that plugin developers didn't know the details of the known issues and only knew if succeeded or failed.

EnsureNoError was designed as a last resort. We made it a convention for developers to pass the error returned by EvaluateExpr to this function. This aims to hide this problem by only calling the callback if no known issues occur.

But this doesn't seem to work very well. By convention in Go, it is common to test the returned error by if err != nil, and EnsureNoError is not intuitive. Therefore, it confuses developers as they don't know where this function is to use.

We need a more intuitive interface. For example, with the type parameter added in Go 1.18, we can pass a callback function to be executed when the evaluation is successful to EvaluateExpr.

func EvaluateExpr[T any](expr hcl.Expression, callback func (ret T) error) error {
  val, err := getExprValueFromGRPCServer(expr)
  if errors.Is(err, knownErr) {
    // Skipped
    return nil
  }

  var out T
  gocty.FromCtyValue(val, &out)
  return callback(out)
}

func main() {
  EvaluateExpr(expr, func (ret string) error {
    fmt.Printf("evaluated value: %s", ret)
  })
}

I want to create a rule which iterates over all resources and checks if their style matches the convention.
While testing I found out that tflint runs into a segmentation violation when no terraform remote state backend is configured.

My test rule: https://github.com/janritter/tflint-ruleset-codestyle/blob/ec319421144054006fc888266fe88347e3e02f38/rules/terraform_this.go#L39

Without backend configuration

When I execute tflint with my test rule against this terraform definition:

resource "aws_s3_bucket" "wrong_name" {
  bucket = "my-tf-test-bucket"
  acl    = "private"
}

The following error is thrown:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x188f3da]

goroutine 860 [running]:
github.com/terraform-linters/tflint/plugin.(*Server).encodeBackend(0xc00040e380, 0x0, 0xc00098d7e0)
        /private/tmp/tflint-20210110-4829-12nifkt/tflint-0.23.1/plugin/encode.go:271 +0x3a
github.com/terraform-linters/tflint/plugin.(*Server).encodeModule(0xc00040e380, 0xc0004c13b0, 0x21fd0c0)
        /private/tmp/tflint-20210110-4829-12nifkt/tflint-0.23.1/plugin/encode.go:94 +0xea7
github.com/terraform-linters/tflint/plugin.(*Server).encodeConfig(0xc00040e380, 0xc0002bbba0, 0x0)
        /private/tmp/tflint-20210110-4829-12nifkt/tflint-0.23.1/plugin/encode.go:21 +0x5b
github.com/terraform-linters/tflint/plugin.(*Server).Config(0xc00040e380, 0x36ad8c0, 0xc00040e7e0, 0x0, 0x0)
        /private/tmp/tflint-20210110-4829-12nifkt/tflint-0.23.1/plugin/server.go:101 +0x36
reflect.Value.call(0xc0008da4e0, 0xc0000eaac0, 0x13, 0x270d83b, 0x4, 0xc00008ff08, 0x3, 0x3, 0xc0009523c0, 0x29dce00, ...)
        /usr/local/Cellar/go/1.15.6/libexec/src/reflect/value.go:476 +0x8c7
reflect.Value.Call(0xc0008da4e0, 0xc0000eaac0, 0x13, 0xc0008b4f08, 0x3, 0x3, 0xc0009523c0, 0x3577868, 0xc0009cd680)
        /usr/local/Cellar/go/1.15.6/libexec/src/reflect/value.go:337 +0xb9
net/rpc.(*service).call(0xc0005dfe00, 0xc0008c1720, 0xc0000e1578, 0xc0000e1590, 0xc00015f300, 0xc00040e420, 0x21fd080, 0x36ad8c0, 0x16, 0x21fd0c0, ...)
        /usr/local/Cellar/go/1.15.6/libexec/src/net/rpc/server.go:377 +0x189
created by net/rpc.(*Server).ServeCodec
        /usr/local/Cellar/go/1.15.6/libexec/src/net/rpc/server.go:474 +0x445

With backend configuration

When I execute tflint with my test rule against this terraform definition:

resource "aws_s3_bucket" "wrong_name" {
  bucket = "my-tf-test-bucket"
  acl    = "private"
}

terraform {
  backend "s3" {
      region = "eu-central-1"
      bucket = "my-bucket"
      key = "my-project/s3/terraform.tfstate"
      dynamodb_table = "terraform-state-lock"
  }
}

My rule is returning the test error as expected:

Failed to check ruleset. An error occurred:

Error: Failed to check `terraform_resource_name_this` rule: End of check

At $DAYJOB, I've started writing our own ruleset to enforce some very company-specific rules. One of the most valuable is validating that our backend configurations specify the correct attributes, so we can ensure that locks are always correctly taken, the tfstate is stored in S3 at the correct location, etc. Unfortunately, the SDK currently only permits walking over resources.

I've been looking into adding support for exposing more to the SDK, but I'm not sure what the preferred approach would be. Adding a Backend() func to the runners seemed straightforward enough, and I managed to put together a spike of it which more or less seems to work for a test rule:

1 issue(s) found:
Error: remote state backend must be S3 (backend_lint_hax)
  on test/backend.tf line 2:
   2:   backend "totally_wrong" {

But this implementation doesn't fit in terribly well with the WalkResourceAttributes style functions that are currently available.

Zooming out a bit, other rules I'd like to add will want access to ModuleCalls, ProviderConfigs, etc. Is there a more general approach to this issue, something like WalkBlocks(type string, ...) or such? That might be pretty unwieldy on the caller's side, though.

If this seems like a reasonable feature request, I'd be happy to help with implementation if I can.

Currently, we provide TestRunner for plugin developers to test its plugin. This Runner is not a real RPC client, but a dummy that mimics the behavior. The problem with this method is that we need to reimplement the Terraform's configuration loading.

To reduce the hassle of reimplementing, launch the actual TFLint plugin server process, and use it. To achieve this, it is necessary to add features on the TFLint side.

When defining a data source without any attribute, you receive a segmentation violation. Defining an empty data source is valid for "aws_caller_identity"

Code for the tflint rule

// Check checks whether ...
func (r *TerraformThisRule) Check(runner tflint.Runner) error {
	_, err := runner.Config()
	if err != nil {
		return err
	}
	return nil
}

Example failing with segmentation fault

resource "aws_s3_bucket" "this" {
  bucket = "my-tf-test-bucket"
  acl    = "private"
}

data "aws_caller_identity" "current" {}

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x18641ee]

goroutine 67 [running]:
github.com/terraform-linters/tflint/plugin.(*Server).encodeResource(0xc0001a1f08, 0xc0007b3180, 0xc00015d7d0)
        /private/tmp/tflint-20210425-9678-xk5bqe/tflint-0.28.0/plugin/encode.go:143 +0x22e
github.com/terraform-linters/tflint/plugin.(*Server).encodeModule(0xc0001a1f08, 0xc0009447e0, 0xc00056a320, 0xc0007f2be0, 0x100f0bb)
        /private/tmp/tflint-20210425-9678-xk5bqe/tflint-0.28.0/plugin/encode.go:95 +0xe2e
github.com/terraform-linters/tflint/plugin.(*Server).encodeConfig(0xc0001a1f08, 0xc0005055f0, 0x28, 0x3c185b8, 0x30)
        /private/tmp/tflint-20210425-9678-xk5bqe/tflint-0.28.0/plugin/encode.go:21 +0x5b
github.com/terraform-linters/tflint/plugin.(*Server).Config(0xc0001a1f08, 0x3712128, 0xc000150300, 0x0, 0x0)
        /private/tmp/tflint-20210425-9678-xk5bqe/tflint-0.28.0/plugin/server.go:111 +0x3a
reflect.Value.call(0xc000132d20, 0xc0003bc380, 0x13, 0x27439e9, 0x4, 0xc0007f2f08, 0x3, 0x3, 0x26dd240, 0xc00010e500, ...)
        /usr/local/Cellar/go/1.16.3/libexec/src/reflect/value.go:476 +0x8e7
reflect.Value.Call(0xc000132d20, 0xc0003bc380, 0x13, 0xc00077bf08, 0x3, 0x3, 0x2a40360, 0x100000001, 0xc0007b5290)
        /usr/local/Cellar/go/1.16.3/libexec/src/reflect/value.go:337 +0xb9
net/rpc.(*service).call(0xc00012f680, 0xc000131090, 0xc0006342c0, 0xc0006342d0, 0xc0001fe480, 0xc00056a040, 0x221c7e0, 0x3712128, 0x16, 0x221c820, ...)
        /usr/local/Cellar/go/1.16.3/libexec/src/net/rpc/server.go:377 +0x189
created by net/rpc.(*Server).ServeCodec
        /usr/local/Cellar/go/1.16.3/libexec/src/net/rpc/server.go:474 +0x44d

Working example

resource "aws_s3_bucket" "this" {
  bucket = "my-tf-test-bucket"
  acl    = "private"
}

data "aws_caller_identity" "current" {
  test = "test"
}