plugin: Handle sensitive values on the plugin side (#1722)

* Handle sensitive values on the plugin side * Use function callback instead of EnsureNoError
terraform-linters · Mar 26, 2023 · be21823 · be21823
1 parent cd99b1f
commit be21823
Show file tree

Hide file tree

Showing 30 changed files with 66 additions and 120 deletions.
diff --git a/docs/developer-guide/api_compatibility.md b/docs/developer-guide/api_compatibility.md
@@ -23,3 +23,8 @@ TFLint version: v0.40.0+
 - Expand mode is only supported by SDK v0.14.0+ and TFLint v0.42.0+.
   - https://github.com/terraform-linters/tflint/pull/1537
   - https://github.com/terraform-linters/tflint-plugin-sdk/pull/208
+- Client-side value handling is introduced in SDK v0.16.0 and TFLint v0.46.0. TFLint v0.45.0 returns an error instead of a value.
+  - https://github.com/terraform-linters/tflint/pull/1700
+  - https://github.com/terraform-linters/tflint/pull/1722
+  - https://github.com/terraform-linters/tflint-plugin-sdk/pull/235
+  - https://github.com/terraform-linters/tflint-plugin-sdk/pull/239
diff --git a/go.mod b/go.mod
@@ -24,7 +24,7 @@ require (
 	github.com/sourcegraph/go-lsp v0.0.0-20200429204803-219e11d77f5d
 	github.com/sourcegraph/jsonrpc2 v0.1.0
 	github.com/spf13/afero v1.9.5
-	github.com/terraform-linters/tflint-plugin-sdk v0.15.1-0.20230225141907-dd804b3671af
+	github.com/terraform-linters/tflint-plugin-sdk v0.15.1-0.20230326071520-8f0960834baa
 	github.com/terraform-linters/tflint-ruleset-terraform v0.2.2
 	github.com/xeipuuv/gojsonschema v1.2.0
 	github.com/zclconf/go-cty v1.12.1

diff --git a/go.sum b/go.sum
@@ -457,8 +457,8 @@ github.com/stretchr/testify v1.7.2/go.mod h1:R6va5+xMeoiuVRoj+gSkQ7d3FALtqAAGI1F
 github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
 github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk=
 github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
-github.com/terraform-linters/tflint-plugin-sdk v0.15.1-0.20230225141907-dd804b3671af h1:TAsqOUKu3DXg6ZmV3igB8ksKkHkaQrdSdZfCE3Ff7nc=
-github.com/terraform-linters/tflint-plugin-sdk v0.15.1-0.20230225141907-dd804b3671af/go.mod h1:g5UIXcskejxp38JWqvYqEb/HkvIX6X6luEdS60yimTw=
+github.com/terraform-linters/tflint-plugin-sdk v0.15.1-0.20230326071520-8f0960834baa h1:fjuUEYYH8dV7WW5ZvJHYF/nF4eEyBGft+Js3ydw46mk=
+github.com/terraform-linters/tflint-plugin-sdk v0.15.1-0.20230326071520-8f0960834baa/go.mod h1:g5UIXcskejxp38JWqvYqEb/HkvIX6X6luEdS60yimTw=
 github.com/terraform-linters/tflint-ruleset-terraform v0.2.2 h1:iTE09KkaZ0DE29xvp6IIM1/gmas9V0h8CER28SyBmQ8=
 github.com/terraform-linters/tflint-ruleset-terraform v0.2.2/go.mod h1:bCkvH8Vqzr16bWEE3e6Q3hvdZlmSAOR8i6G3M5y+M+k=
 github.com/ulikunitz/xz v0.5.10 h1:t92gobL9l3HE202wg3rlk19F6X+JOxl9BBrCCMYEYd8=

diff --git a/plugin/server.go b/plugin/server.go
@@ -127,6 +127,11 @@ func (s *GRPCServer) EvaluateExpr(expr hcl.Expression, opts sdk.EvaluateExprOpti
 		return val, diags
 	}
 
+	// SDK v0.16+ introduces client-side handling of unknown/NULL/sensitive values.
+	if s.clientSDKVersion != nil && s.clientSDKVersion.GreaterThanOrEqual(version.Must(version.NewVersion("0.16.0"))) {
+		return val, nil
+	}
+
 	if val.ContainsMarked() {
 		err := fmt.Errorf(
 			"sensitive value found in %s:%d%w",
@@ -138,11 +143,6 @@ func (s *GRPCServer) EvaluateExpr(expr hcl.Expression, opts sdk.EvaluateExprOpti
 		return cty.NullVal(cty.NilType), err
 	}
 
-	// SDK v0.16+ introduces client-side handling of unknown and NULL values.
-	if s.clientSDKVersion != nil && s.clientSDKVersion.GreaterThanOrEqual(version.Must(version.NewVersion("0.16.0"))) {
-		return val, nil
-	}
-
 	if *opts.WantType == cty.DynamicPseudoType {
 		return val, nil
 	}

diff --git a/plugin/server_test.go b/plugin/server_test.go
@@ -14,6 +14,7 @@ import (
 	"github.com/spf13/afero"
 	"github.com/terraform-linters/tflint-plugin-sdk/hclext"
 	"github.com/terraform-linters/tflint-plugin-sdk/plugin/host2plugin"
+	"github.com/terraform-linters/tflint-plugin-sdk/terraform/lang/marks"
 	sdk "github.com/terraform-linters/tflint-plugin-sdk/tflint"
 	"github.com/terraform-linters/tflint/tflint"
 	"github.com/zclconf/go-cty/cty"
@@ -543,18 +544,28 @@ variable "foo" {
 			Args: func() (hcl.Expression, sdk.EvaluateExprOption) {
 				return hclExpr(`var.sensitive`), sdk.EvaluateExprOption{WantType: &cty.String, ModuleCtx: sdk.SelfModuleCtxType}
 			},
-			Want: cty.NullVal(cty.NilType),
+			Want:     cty.StringVal("foo").Mark(marks.Sensitive),
+			ErrCheck: neverHappend,
+		},
+		{
+			Name: "sensitive value (SDK v0.15)",
+			Args: func() (hcl.Expression, sdk.EvaluateExprOption) {
+				return hclExpr(`var.sensitive`), sdk.EvaluateExprOption{WantType: &cty.String, ModuleCtx: sdk.SelfModuleCtxType}
+			},
+			Want:       cty.NullVal(cty.NilType),
+			SDKVersion: sdkv15,
 			ErrCheck: func(err error) bool {
 				return err == nil || !errors.Is(err, sdk.ErrSensitive)
 			},
 		},
 		{
-			Name: "sensitive value in object",
+			Name: "sensitive value in object (SDK v0.15)",
 			Args: func() (hcl.Expression, sdk.EvaluateExprOption) {
 				ty := cty.Object(map[string]cty.Type{"value": cty.String})
 				return hclExpr(`{ value = var.sensitive }`), sdk.EvaluateExprOption{WantType: &ty, ModuleCtx: sdk.SelfModuleCtxType}
 			},
-			Want: cty.NullVal(cty.NilType),
+			Want:       cty.NullVal(cty.NilType),
+			SDKVersion: sdkv15,
 			ErrCheck: func(err error) bool {
 				return err == nil || !errors.Is(err, sdk.ErrSensitive)
 			},

diff --git a/plugin/stub-generator/sources/customrulesettesting/custom/runner.go b/plugin/stub-generator/sources/customrulesettesting/custom/runner.go
@@ -49,25 +49,21 @@ func NewRunner(runner tflint.Runner, config *Config) (*Runner, error) {
 		opts := &tflint.EvaluateExprOption{ModuleCtx: tflint.RootModuleCtxType}
 
 		if attr, exists := provider.Body.Attributes["zone"]; exists {
-			var zone string
-			err := runner.EvaluateExpr(attr.Expr, &zone, opts)
-			err = runner.EnsureNoError(err, func() error {
+			err := runner.EvaluateExpr(attr.Expr, func(zone string) error {
 				config.Zone = zone
 				return nil
-			})
+			}, opts)
 			if err != nil {
 				return nil, err
 			}
 		}
 
 		for _, annotation := range provider.Body.Blocks {
 			if attr, exists := annotation.Body.Attributes["value"]; exists {
-				var val string
-				err := runner.EvaluateExpr(attr.Expr, &val, opts)
-				err = runner.EnsureNoError(err, func() error {
+				err := runner.EvaluateExpr(attr.Expr, func(val string) error {
 					config.Annotation = val
 					return nil
-				})
+				}, opts)
 				if err != nil {
 					return nil, err
 				}

diff --git a/plugin/stub-generator/sources/example/main.go b/plugin/stub-generator/sources/example/main.go
@@ -65,16 +65,13 @@ func (r *AwsInstanceExampleTypeRule) Check(runner tflint.Runner) error {
 			continue
 		}
 
-		var instanceType string
-		err := runner.EvaluateExpr(attribute.Expr, &instanceType, nil)
-
-		err = runner.EnsureNoError(err, func() error {
+		err := runner.EvaluateExpr(attribute.Expr, func(instanceType string) error {
 			return runner.EmitIssue(
 				r,
 				fmt.Sprintf("instance type is %s", instanceType),
 				attribute.Expr.Range(),
 			)
-		})
+		}, nil)
 		if err != nil {
 			return err
 		}

diff --git a/plugin/stub-generator/sources/testing/rules/aws_autoscaling_group_cty_eval_example.go b/plugin/stub-generator/sources/testing/rules/aws_autoscaling_group_cty_eval_example.go
@@ -64,16 +64,13 @@ func (r *AwsAutoscalingGroupCtyEvalExampleRule) Check(runner tflint.Runner) erro
 			"value":               cty.String,
 			"propagate_at_launch": cty.Bool,
 		}))
-		var tags []tag
-		err := runner.EvaluateExpr(attribute.Expr, &tags, &tflint.EvaluateExprOption{WantType: &wantType})
-
-		err = runner.EnsureNoError(err, func() error {
+		err := runner.EvaluateExpr(attribute.Expr, func(tags []tag) error {
 			return runner.EmitIssue(
 				r,
 				fmt.Sprintf("autoscaling tags: %#v", tags),
 				attribute.Expr.Range(),
 			)
-		})
+		}, &tflint.EvaluateExprOption{WantType: &wantType})
 		if err != nil {
 			return err
 		}

diff --git a/plugin/stub-generator/sources/testing/rules/aws_db_instance_with_default_config_example.go b/plugin/stub-generator/sources/testing/rules/aws_db_instance_with_default_config_example.go
@@ -61,16 +61,13 @@ func (r *AwsDBInstanceWithDefaultConfigExampleRule) Check(runner tflint.Runner)
 			continue
 		}
 
-		var name string
-		err := runner.EvaluateExpr(attribute.Expr, &name, nil)
-
-		err = runner.EnsureNoError(err, func() error {
+		err := runner.EvaluateExpr(attribute.Expr, func(name string) error {
 			return runner.EmitIssue(
 				r,
 				fmt.Sprintf("DB name is %s, config=%s", name, config.Name),
 				attribute.Expr.Range(),
 			)
-		})
+		}, nil)
 		if err != nil {
 			return err
 		}

diff --git a/plugin/stub-generator/sources/testing/rules/aws_iam_policy_example.go b/plugin/stub-generator/sources/testing/rules/aws_iam_policy_example.go
@@ -55,16 +55,13 @@ func (r *AwsIAMPolicyExampleRule) Check(runner tflint.Runner) error {
 			continue
 		}
 
-		var name string
-		err := runner.EvaluateExpr(attribute.Expr, &name, nil)
-
-		err = runner.EnsureNoError(err, func() error {
+		err := runner.EvaluateExpr(attribute.Expr, func(name string) error {
 			return runner.EmitIssue(
 				r,
 				fmt.Sprintf("name is %s", name),
 				attribute.Expr.Range(),
 			)
-		})
+		}, nil)
 		if err != nil {
 			return err
 		}

diff --git a/plugin/stub-generator/sources/testing/rules/aws_iam_role_example.go b/plugin/stub-generator/sources/testing/rules/aws_iam_role_example.go
@@ -67,16 +67,13 @@ func (r *AwsIAMRoleExampleRule) Check(runner tflint.Runner) error {
 				continue
 			}
 
-			var name string
-			err := runner.EvaluateExpr(attribute.Expr, &name, nil)
-
-			err = runner.EnsureNoError(err, func() error {
+			err := runner.EvaluateExpr(attribute.Expr, func(name string) error {
 				return runner.EmitIssue(
 					r,
 					fmt.Sprintf("name is %s", name),
 					attribute.Expr.Range(),
 				)
-			})
+			}, nil)
 			if err != nil {
 				return err
 			}

diff --git a/plugin/stub-generator/sources/testing/rules/aws_instance_example_type.go b/plugin/stub-generator/sources/testing/rules/aws_instance_example_type.go
@@ -52,16 +52,13 @@ func (r *AwsInstanceExampleTypeRule) Check(runner tflint.Runner) error {
 			continue
 		}
 
-		var instanceType string
-		err := runner.EvaluateExpr(attribute.Expr, &instanceType, nil)
-
-		err = runner.EnsureNoError(err, func() error {
+		err := runner.EvaluateExpr(attribute.Expr, func(instanceType string) error {
 			return runner.EmitIssue(
 				r,
 				fmt.Sprintf("instance type is %s", instanceType),
 				attribute.Expr.Range(),
 			)
-		})
+		}, nil)
 		if err != nil {
 			return err
 		}

diff --git a/plugin/stub-generator/sources/testing/rules/aws_instance_map_eval_example.go b/plugin/stub-generator/sources/testing/rules/aws_instance_map_eval_example.go
@@ -54,16 +54,13 @@ func (r *AwsInstanceMapEvalExampleRule) Check(runner tflint.Runner) error {
 		}
 
 		wantType := cty.Map(cty.String)
-		tags := map[string]string{}
-		err := runner.EvaluateExpr(attribute.Expr, &tags, &tflint.EvaluateExprOption{WantType: &wantType})
-
-		err = runner.EnsureNoError(err, func() error {
+		err := runner.EvaluateExpr(attribute.Expr, func(tags map[string]string) error {
 			return runner.EmitIssue(
 				r,
 				fmt.Sprintf("instance tags: %#v", tags),
 				attribute.Expr.Range(),
 			)
-		})
+		}, &tflint.EvaluateExprOption{WantType: &wantType})
 		if err != nil {
 			return err
 		}

diff --git a/plugin/stub-generator/sources/testing/rules/aws_route53_record_eval_on_root_ctx_example.go b/plugin/stub-generator/sources/testing/rules/aws_route53_record_eval_on_root_ctx_example.go
@@ -52,16 +52,13 @@ func (r *AwsRoute53RecordEvalOnRootCtxExampleRule) Check(runner tflint.Runner) e
 			continue
 		}
 
-		var name string
-		err := runner.EvaluateExpr(attribute.Expr, &name, &tflint.EvaluateExprOption{ModuleCtx: tflint.RootModuleCtxType})
-
-		err = runner.EnsureNoError(err, func() error {
+		err := runner.EvaluateExpr(attribute.Expr, func(name string) error {
 			return runner.EmitIssue(
 				r,
 				fmt.Sprintf("record name (root): %#v", name),
 				attribute.Expr.Range(),
 			)
-		})
+		}, &tflint.EvaluateExprOption{ModuleCtx: tflint.RootModuleCtxType})
 		if err != nil {
 			return err
 		}

diff --git a/plugin/stub-generator/sources/testing/rules/aws_s3_bucket_example_lifecycle_rule.go b/plugin/stub-generator/sources/testing/rules/aws_s3_bucket_example_lifecycle_rule.go
@@ -72,10 +72,9 @@ func (r *AwsS3BucketExampleLifecycleRuleRule) Check(runner tflint.Runner) error
 			}
 
 			if attr, exists := lifecycle.Body.Attributes["enabled"]; exists {
-				var enabled string
-				err := runner.EnsureNoError(runner.EvaluateExpr(attr.Expr, &enabled, nil), func() error {
+				err := runner.EvaluateExpr(attr.Expr, func(enabled string) error {
 					return runner.EmitIssue(r, fmt.Sprintf("`enabled` attribute found: %s", enabled), attr.Expr.Range())
-				})
+				}, nil)
 				if err != nil {
 					return err
 				}
@@ -87,10 +86,9 @@ func (r *AwsS3BucketExampleLifecycleRuleRule) Check(runner tflint.Runner) error
 				}
 
 				if attr, exists := transition.Body.Attributes["days"]; exists {
-					var days int
-					err := runner.EnsureNoError(runner.EvaluateExpr(attr.Expr, &days, nil), func() error {
+					err := runner.EvaluateExpr(attr.Expr, func(days int) error {
 						return runner.EmitIssue(r, fmt.Sprintf("`days` attribute found: %d", days), attr.Expr.Range())
-					})
+					}, nil)
 					if err != nil {
 						return err
 					}

diff --git a/plugin/stub-generator/sources/testing/rules/aws_s3_bucket_with_config_example.go b/plugin/stub-generator/sources/testing/rules/aws_s3_bucket_with_config_example.go
@@ -61,16 +61,13 @@ func (r *AwsS3BucketWithConfigExampleRule) Check(runner tflint.Runner) error {
 			continue
 		}
 
-		var bucket string
-		err := runner.EvaluateExpr(attribute.Expr, &bucket, nil)
-
-		err = runner.EnsureNoError(err, func() error {
+		err := runner.EvaluateExpr(attribute.Expr, func(bucket string) error {
 			return runner.EmitIssue(
 				r,
 				fmt.Sprintf("bucket name is %s, config=%s", bucket, config.Name),
 				attribute.Expr.Range(),
 			)
-		})
+		}, nil)
 		if err != nil {
 			return err
 		}

diff --git a/terraform/evaluator.go b/terraform/evaluator.go
@@ -9,9 +9,9 @@ import (
 	"github.com/agext/levenshtein"
 	"github.com/hashicorp/hcl/v2"
 	"github.com/terraform-linters/tflint-plugin-sdk/hclext"
+	"github.com/terraform-linters/tflint-plugin-sdk/terraform/lang/marks"
 	"github.com/terraform-linters/tflint/terraform/addrs"
 	"github.com/terraform-linters/tflint/terraform/lang"
-	"github.com/terraform-linters/tflint/terraform/lang/marks"
 	"github.com/zclconf/go-cty/cty"
 	"github.com/zclconf/go-cty/cty/convert"
 )

diff --git a/terraform/evaluator_test.go b/terraform/evaluator_test.go
@@ -13,7 +13,7 @@ import (
 	"github.com/hashicorp/hcl/v2/hclsyntax"
 	"github.com/spf13/afero"
 	"github.com/terraform-linters/tflint-plugin-sdk/hclext"
-	"github.com/terraform-linters/tflint/terraform/lang/marks"
+	"github.com/terraform-linters/tflint-plugin-sdk/terraform/lang/marks"
 	"github.com/zclconf/go-cty/cty"
 )
 

diff --git a/terraform/lang/funcs/collection_test.go b/terraform/lang/funcs/collection_test.go
@@ -5,7 +5,7 @@ import (
 	"math"
 	"testing"
 
-	"github.com/terraform-linters/tflint/terraform/lang/marks"
+	"github.com/terraform-linters/tflint-plugin-sdk/terraform/lang/marks"
 	"github.com/zclconf/go-cty/cty"
 )
 

diff --git a/terraform/lang/funcs/conversion.go b/terraform/lang/funcs/conversion.go
@@ -3,7 +3,7 @@ package funcs
 import (
 	"strconv"
 
-	"github.com/terraform-linters/tflint/terraform/lang/marks"
+	"github.com/terraform-linters/tflint-plugin-sdk/terraform/lang/marks"
 	"github.com/zclconf/go-cty/cty"
 	"github.com/zclconf/go-cty/cty/convert"
 	"github.com/zclconf/go-cty/cty/function"

diff --git a/terraform/lang/funcs/conversion_test.go b/terraform/lang/funcs/conversion_test.go
@@ -4,7 +4,7 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/terraform-linters/tflint/terraform/lang/marks"
+	"github.com/terraform-linters/tflint-plugin-sdk/terraform/lang/marks"
 	"github.com/zclconf/go-cty/cty"
 )
 

diff --git a/terraform/lang/funcs/encoding_test.go b/terraform/lang/funcs/encoding_test.go
@@ -4,7 +4,7 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/terraform-linters/tflint/terraform/lang/marks"
+	"github.com/terraform-linters/tflint-plugin-sdk/terraform/lang/marks"
 	"github.com/zclconf/go-cty/cty"
 )
 

diff --git a/terraform/lang/funcs/filesystem_test.go b/terraform/lang/funcs/filesystem_test.go
@@ -8,7 +8,7 @@ import (
 	"testing"
 
 	homedir "github.com/mitchellh/go-homedir"
-	"github.com/terraform-linters/tflint/terraform/lang/marks"
+	"github.com/terraform-linters/tflint-plugin-sdk/terraform/lang/marks"
 	"github.com/zclconf/go-cty/cty"
 	"github.com/zclconf/go-cty/cty/function"
 	"github.com/zclconf/go-cty/cty/function/stdlib"

diff --git a/terraform/lang/funcs/number_test.go b/terraform/lang/funcs/number_test.go
@@ -4,7 +4,7 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/terraform-linters/tflint/terraform/lang/marks"
+	"github.com/terraform-linters/tflint-plugin-sdk/terraform/lang/marks"
 	"github.com/zclconf/go-cty/cty"
 )