Skip to content

feat: support provider aliases#342

Merged
wata727 merged 10 commits into
terraform-linters:masterfrom
suzuki-shunsuke:feat/support-provider-alias-2
May 7, 2022
Merged

feat: support provider aliases#342
wata727 merged 10 commits into
terraform-linters:masterfrom
suzuki-shunsuke:feat/support-provider-alias-2

Conversation

@suzuki-shunsuke

@suzuki-shunsuke suzuki-shunsuke commented May 5, 2022

Copy link
Copy Markdown
Contributor

#331

Follow up #332 (comment)

@suzuki-shunsuke

suzuki-shunsuke commented May 5, 2022

Copy link
Copy Markdown
Contributor Author

It failed to parse the attribute provider.

resource "aws_instance" "reverse_proxy_a01" {
  ami           = "ami-088da9557aae42f39"
  instance_type = "t3.micro"
  key_name      = "bootstrap"
  provider      = aws.us-east-1
}
func (r *Runner) AwsClient(attributes hclext.Attributes) (*Client, error) {
	provider := "aws"
	if attr, exists := attributes["provider"]; exists {
		if diags := gohcl.DecodeExpression(attr.Expr, nil, &provider); diags.HasErrors() {
			logger.Error("parse provider attribute")
			return nil, diags
		}
		logger.Info("provider: " + provider)
	}

	awsClient, ok := r.AwsClients[provider]
	if !ok {
		return nil, fmt.Errorf("aws provider %s isn't found", provider)
	}

	return awsClient, nil
}
23:10:22 [DEBUG] runtime/asm_arm64.s:1259: tflint-ruleset-aws: 23:10:22 [ERROR] aws/runner.go:50: parse provider attribute
23:10:22 [DEBUG] runtime/asm_arm64.s:1259: tflint-ruleset-aws: 23:10:22 [ERROR] interceptor/logging.go:18: failed to gRPC request: direction=host2plugin method=/proto.RuleSet/Check err="rpc error: code = Aborted desc = Failed to check `aws_instance_invalid_key_name` rule: main.tf:24,19-22: Variables not allowed; Variables may not be used here., and 1 other diagnostic(s)"
Failed to check ruleset; Failed to check `aws_instance_invalid_key_name` rule: main.tf:24,19-22: Variables not allowed; Variables may not be used here., and 1 other diagnostic(s)

		if diags := gohcl.DecodeExpression(attr.Expr, nil, &provider); diags.HasErrors() {
  provider      = aws.us-east-1
Variables not allowed; Variables may not be used here

https://www.terraform.io/language/providers/configuration#selecting-alternate-provider-configurations

$ terraform validate
Success! The configuration is valid.

@wata727

wata727 commented May 5, 2022

Copy link
Copy Markdown
Member

Umm... As you say, it seems that it is necessary to reimplement the decodeProviderConfigRef.
https://github.com/hashicorp/terraform/blob/3fbedf25430ead97eb42575d344427db3c32d524/internal/configs/resource.go#L498

I think we doesn't need to reproduce all implementations completely, for example, support for legacy configuration language can be ignored. The important thing is to get the list of traversals with hcl.AbsTraversalForExpr and get the root name and step name.

@suzuki-shunsuke

suzuki-shunsuke commented May 5, 2022

Copy link
Copy Markdown
Contributor Author

67a6433 Copied required code.
It seems to work well.
I guess we have to take the original code's license into account.

@suzuki-shunsuke

Copy link
Copy Markdown
Contributor Author

Mozilla Public License, version 2.0
https://github.com/hashicorp/terraform/blob/main/LICENSE

@suzuki-shunsuke suzuki-shunsuke changed the title [WIP] feat: support provider aliases feat: support provider aliases May 6, 2022
@suzuki-shunsuke suzuki-shunsuke marked this pull request as ready for review May 6, 2022 01:41
@suzuki-shunsuke suzuki-shunsuke requested a review from wata727 May 7, 2022 03:42
Comment thread aws/runner.go
if err != nil {
return nil, err
}
clients[k] = client

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that the client is not initialized as before if you don't declare any provider blocks. Previously, if you wrote a credential in .tflint.hcl, you should have been able to enable deep checking without a provider block. e.g.

plugin "aws" {
  enabled = true
  access_key = "AWS_ACCESS_KEY",
  secret_key = "AWS_SECRET_KEY",
}
resource "aws_instance" "foo" {
  ami = "invalid"
}

@suzuki-shunsuke suzuki-shunsuke May 7, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. f73855a

@suzuki-shunsuke suzuki-shunsuke May 7, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/terraform-linters/tflint-ruleset-aws/blob/master/docs/deep_checking.md#shared-credentials
https://github.com/terraform-linters/tflint-ruleset-aws/blob/master/docs/deep_checking.md#static-credentials

If these configurations are defined in the provider block, they will also be taken into account. But the priority is lower than the above way.
The priority is higher than the environment variable and lower than the above way.

Hmm... Maybe we have to fix f73855a according to the priority.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The priority of plugin setting is higher than provider.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. 313f0da

continue
}

awsClient, err := runner.AwsClient(resource.Body.Attributes)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to get the provider attribute explicitly like any other rule.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. 821423f

continue
}

awsClient, err := runner.AwsClient(resource.Body.Attributes)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as aws_instance_invalid_rule.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. 821423f

@wata727

wata727 commented May 7, 2022

Copy link
Copy Markdown
Member

I guess we have to take the original code's license into account.

Yes. We changed TFLint's license 4 years ago to follow this case. MPL 2.0 is a weak copyleft and the Large Work is allowed under the same license.
terraform-linters/tflint#245

@suzuki-shunsuke suzuki-shunsuke requested a review from wata727 May 7, 2022 06:55

@wata727 wata727 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@wata727 wata727 merged commit d1ec9c7 into terraform-linters:master May 7, 2022
@suzuki-shunsuke suzuki-shunsuke deleted the feat/support-provider-alias-2 branch May 7, 2022 07:46
@suzuki-shunsuke

Copy link
Copy Markdown
Contributor Author

Thank you for your support!

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants