From 8f0960834baac44e91e5c58bee06ea038911df89 Mon Sep 17 00:00:00 2001 From: Kazuma Watanabe Date: Sun, 26 Mar 2023 16:15:20 +0900 Subject: [PATCH] Add support for function callbacks as the target of EvaluateExpr (#246) --- helper/runner.go | 48 ++++- helper/runner_test.go | 11 ++ plugin/plugin2host/client.go | 49 ++++- plugin/plugin2host/plugin2host_test.go | 261 +++++++++++++++++++++++++ tflint/interface.go | 53 +++-- 5 files changed, 394 insertions(+), 28 deletions(-) diff --git a/helper/runner.go b/helper/runner.go index 300310f..3993758 100644 --- a/helper/runner.go +++ b/helper/runner.go @@ -1,8 +1,10 @@ package helper import ( + "errors" "fmt" "os" + "reflect" "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hclsyntax" @@ -196,9 +198,45 @@ func (r *Runner) DecodeRuleConfig(name string, ret interface{}) error { return nil } +var errRefTy = reflect.TypeOf((*error)(nil)).Elem() + // EvaluateExpr returns a value of the passed expression. // Note that some features are limited -func (r *Runner) EvaluateExpr(expr hcl.Expression, ret interface{}, opts *tflint.EvaluateExprOption) error { +func (r *Runner) EvaluateExpr(expr hcl.Expression, target interface{}, opts *tflint.EvaluateExprOption) error { + var callback bool + rval := reflect.ValueOf(target) + rty := rval.Type() + // Callback must meet the following requirements: + // - It must be a function + // - It must take an argument + // - It must return an error + if rty.Kind() == reflect.Func && rty.NumIn() == 1 && rty.NumOut() == 1 && rty.Out(0).Implements(errRefTy) { + callback = true + target = reflect.New(rty.In(0)).Interface() + } + + err := r.evaluateExpr(expr, target, opts) + if !callback { + // error should be handled in the caller + return err + } + + if err != nil { + // If it cannot be represented as a Go value, exit without invoking the callback rather than returning an error. + if errors.Is(err, tflint.ErrUnknownValue) || errors.Is(err, tflint.ErrNullValue) || errors.Is(err, tflint.ErrSensitive) || errors.Is(err, tflint.ErrUnevaluable) { + return nil + } + return err + } + + rerr := rval.Call([]reflect.Value{reflect.ValueOf(target).Elem()}) + if rerr[0].IsNil() { + return nil + } + return rerr[0].Interface().(error) +} + +func (r *Runner) evaluateExpr(expr hcl.Expression, target interface{}, opts *tflint.EvaluateExprOption) error { if opts == nil { opts = &tflint.EvaluateExprOption{} } @@ -207,7 +245,7 @@ func (r *Runner) EvaluateExpr(expr hcl.Expression, ret interface{}, opts *tflint if opts.WantType != nil { ty = *opts.WantType } else { - switch ret.(type) { + switch target.(type) { case *string, string: ty = cty.String case *int, int: @@ -223,7 +261,7 @@ func (r *Runner) EvaluateExpr(expr hcl.Expression, ret interface{}, opts *tflint case cty.Value, *cty.Value: ty = cty.DynamicPseudoType default: - return fmt.Errorf("unsupported result type: %T", ret) + return fmt.Errorf("unsupported target type: %T", target) } } @@ -251,7 +289,7 @@ func (r *Runner) EvaluateExpr(expr hcl.Expression, ret interface{}, opts *tflint return err } - return gocty.FromCtyValue(val, ret) + return gocty.FromCtyValue(val, target) } // EmitIssue adds an issue to the runner itself. @@ -266,7 +304,7 @@ func (r *Runner) EmitIssue(rule tflint.Rule, message string, location hcl.Range) // EnsureNoError is a method that simply runs a function if there is no error. // -// Deprecated: Use errors.Is() instead to determine which errors can be ignored. +// Deprecated: Use EvaluateExpr with a function callback. e.g. EvaluateExpr(expr, func (val T) error {}, ...) func (r *Runner) EnsureNoError(err error, proc func() error) error { if err == nil { return proc() diff --git a/helper/runner_test.go b/helper/runner_test.go index 4ad727f..a31bae7 100644 --- a/helper/runner_test.go +++ b/helper/runner_test.go @@ -576,6 +576,7 @@ resource "aws_instance" "foo" { } for _, resource := range resources.Blocks { + // raw value var instanceType string if err := runner.EvaluateExpr(resource.Body.Attributes["instance_type"].Expr, &instanceType, nil); err != nil { t.Fatal(err) @@ -584,6 +585,16 @@ resource "aws_instance" "foo" { if instanceType != test.Want { t.Fatalf(`"%s" is expected, but got "%s"`, test.Want, instanceType) } + + // callback + if err := runner.EvaluateExpr(resource.Body.Attributes["instance_type"].Expr, func(val string) error { + if instanceType != test.Want { + t.Fatalf(`"%s" is expected, but got "%s"`, test.Want, instanceType) + } + return nil + }, nil); err != nil { + t.Fatal(err) + } } }) } diff --git a/plugin/plugin2host/client.go b/plugin/plugin2host/client.go index dc1ab54..880e39c 100644 --- a/plugin/plugin2host/client.go +++ b/plugin/plugin2host/client.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "os" + "reflect" "strings" "github.com/hashicorp/hcl/v2" @@ -274,8 +275,46 @@ func (c *GRPCClient) DecodeRuleConfig(name string, ret interface{}) error { return nil } +var errRefTy = reflect.TypeOf((*error)(nil)).Elem() + // EvaluateExpr evals the passed expression based on the type. -func (c *GRPCClient) EvaluateExpr(expr hcl.Expression, ret interface{}, opts *tflint.EvaluateExprOption) error { +// Passing a callback function instead of a value as the target will invoke the callback, +// passing the evaluated value to the argument. +func (c *GRPCClient) EvaluateExpr(expr hcl.Expression, target interface{}, opts *tflint.EvaluateExprOption) error { + var callback bool + rval := reflect.ValueOf(target) + rty := rval.Type() + // Callback must meet the following requirements: + // - It must be a function + // - It must take an argument + // - It must return an error + if rty.Kind() == reflect.Func && rty.NumIn() == 1 && rty.NumOut() == 1 && rty.Out(0).Implements(errRefTy) { + callback = true + target = reflect.New(rty.In(0)).Interface() + } + + err := c.evaluateExpr(expr, target, opts) + if !callback { + // error should be handled in the caller + return err + } + + if err != nil { + // If it cannot be represented as a Go value, exit without invoking the callback rather than returning an error. + if errors.Is(err, tflint.ErrUnknownValue) || errors.Is(err, tflint.ErrNullValue) || errors.Is(err, tflint.ErrSensitive) || errors.Is(err, tflint.ErrUnevaluable) { + return nil + } + return err + } + + rerr := rval.Call([]reflect.Value{reflect.ValueOf(target).Elem()}) + if rerr[0].IsNil() { + return nil + } + return rerr[0].Interface().(error) +} + +func (c *GRPCClient) evaluateExpr(expr hcl.Expression, target interface{}, opts *tflint.EvaluateExprOption) error { if opts == nil { opts = &tflint.EvaluateExprOption{} } @@ -284,7 +323,7 @@ func (c *GRPCClient) EvaluateExpr(expr hcl.Expression, ret interface{}, opts *tf if opts.WantType != nil { ty = *opts.WantType } else { - switch ret.(type) { + switch target.(type) { case *string, string: ty = cty.String case *int, int: @@ -300,7 +339,7 @@ func (c *GRPCClient) EvaluateExpr(expr hcl.Expression, ret interface{}, opts *tf case cty.Value, *cty.Value: ty = cty.DynamicPseudoType default: - panic(fmt.Sprintf("unsupported result type: %T", ret)) + panic(fmt.Sprintf("unsupported target type: %T", target)) } } tyby, err := json.MarshalType(ty) @@ -332,7 +371,7 @@ func (c *GRPCClient) EvaluateExpr(expr hcl.Expression, ret interface{}, opts *tf } if ty == cty.DynamicPseudoType { - return gocty.FromCtyValue(val, ret) + return gocty.FromCtyValue(val, target) } // Returns an error if the value cannot be decoded to a Go value (e.g. unknown, null, sensitive). @@ -356,7 +395,7 @@ func (c *GRPCClient) EvaluateExpr(expr hcl.Expression, ret interface{}, opts *tf return err } - return gocty.FromCtyValue(val, ret) + return gocty.FromCtyValue(val, target) } // EmitIssue emits the issue with the passed rule, message, location diff --git a/plugin/plugin2host/plugin2host_test.go b/plugin/plugin2host/plugin2host_test.go index 92dfa0f..83129c1 100644 --- a/plugin/plugin2host/plugin2host_test.go +++ b/plugin/plugin2host/plugin2host_test.go @@ -1975,6 +1975,267 @@ func TestEvaluateExpr(t *testing.T) { } } +func TestEvaluateExpr_callback(t *testing.T) { + // default error check helper + neverHappend := func(err error) bool { return err != nil } + + // default getFileImpl function + fileIdx := 1 + files := map[string]*hcl.File{} + fileExists := func(filename string) (*hcl.File, error) { + return files[filename], nil + } + + // test util functions + hclExpr := func(expr string) hcl.Expression { + filename := fmt.Sprintf("test%d.tf", fileIdx) + file, diags := hclsyntax.ParseConfig([]byte(fmt.Sprintf(`expr = %s`, expr)), filename, hcl.InitialPos) + if diags.HasErrors() { + panic(diags) + } + attributes, diags := file.Body.JustAttributes() + if diags.HasErrors() { + panic(diags) + } + files[filename] = file + fileIdx = fileIdx + 1 + return attributes["expr"].Expr + } + + // test struct for decoding from cty.Value + type Object struct { + Name string `cty:"name"` + Enabled bool `cty:"enabled"` + } + objectTy := cty.Object(map[string]cty.Type{"name": cty.String, "enabled": cty.Bool}) + + tests := []struct { + name string + expr hcl.Expression + target any + option *tflint.EvaluateExprOption + serverImpl func(hcl.Expression, tflint.EvaluateExprOption) (cty.Value, error) + getFileImpl func(string) (*hcl.File, error) + errCheck func(err error) bool + }{ + { + name: "callback with string", + expr: hclExpr(`"foo"`), + target: func(val string) error { + return fmt.Errorf("value is %s", val) + }, + serverImpl: func(expr hcl.Expression, opts tflint.EvaluateExprOption) (cty.Value, error) { + return cty.StringVal("foo"), nil + }, + getFileImpl: fileExists, + errCheck: func(err error) bool { + return err == nil || err.Error() != "value is foo" + }, + }, + { + name: "callback with int", + expr: hclExpr(`1`), + target: func(val int) error { + return fmt.Errorf("value is %d", val) + }, + serverImpl: func(expr hcl.Expression, opts tflint.EvaluateExprOption) (cty.Value, error) { + return cty.NumberIntVal(1), nil + }, + getFileImpl: fileExists, + errCheck: func(err error) bool { + return err == nil || err.Error() != "value is 1" + }, + }, + { + name: "callback with []string", + expr: hclExpr(`["foo", "bar"]`), + target: func(val []string) error { + return fmt.Errorf("value is %#v", val) + }, + serverImpl: func(expr hcl.Expression, opts tflint.EvaluateExprOption) (cty.Value, error) { + return cty.ListVal([]cty.Value{cty.StringVal("foo"), cty.StringVal("bar")}), nil + }, + getFileImpl: fileExists, + errCheck: func(err error) bool { + return err == nil || err.Error() != `value is []string{"foo", "bar"}` + }, + }, + { + name: "callback with []int", + expr: hclExpr(`[1, 2]`), + target: func(val []int) error { + return fmt.Errorf("value is %#v", val) + }, + serverImpl: func(expr hcl.Expression, opts tflint.EvaluateExprOption) (cty.Value, error) { + return cty.ListVal([]cty.Value{cty.NumberIntVal(1), cty.NumberIntVal(2)}), nil + }, + getFileImpl: fileExists, + errCheck: func(err error) bool { + return err == nil || err.Error() != `value is []int{1, 2}` + }, + }, + { + name: "callback with map[string]string", + expr: hclExpr(`{ "foo" = "bar", "baz" = "qux" }`), + target: func(val map[string]string) error { + return fmt.Errorf("foo is %s, baz is %s", val["foo"], val["baz"]) + }, + serverImpl: func(expr hcl.Expression, opts tflint.EvaluateExprOption) (cty.Value, error) { + return cty.MapVal(map[string]cty.Value{ + "foo": cty.StringVal("bar"), + "baz": cty.StringVal("qux"), + }), nil + }, + getFileImpl: fileExists, + errCheck: func(err error) bool { + return err == nil || err.Error() != `foo is bar, baz is qux` + }, + }, + { + name: "callback with map[string]int", + expr: hclExpr(`{ "foo" = "bar", "baz" = "qux" }`), + target: func(val map[string]int) error { + return fmt.Errorf("foo is %d, baz is %d", val["foo"], val["baz"]) + }, + serverImpl: func(expr hcl.Expression, opts tflint.EvaluateExprOption) (cty.Value, error) { + return cty.MapVal(map[string]cty.Value{ + "foo": cty.NumberIntVal(1), + "baz": cty.NumberIntVal(2), + }), nil + }, + getFileImpl: fileExists, + errCheck: func(err error) bool { + return err == nil || err.Error() != `foo is 1, baz is 2` + }, + }, + { + name: "callback with cty.Value", + expr: hclExpr(`var.foo`), + target: func(val cty.Value) error { + return fmt.Errorf("value is %s", val.GoString()) + }, + serverImpl: func(expr hcl.Expression, opts tflint.EvaluateExprOption) (cty.Value, error) { + return cty.ObjectVal(map[string]cty.Value{ + "foo": cty.NumberIntVal(1), + "bar": cty.StringVal("baz"), + "qux": cty.UnknownVal(cty.String), + }), nil + }, + getFileImpl: fileExists, + errCheck: func(err error) bool { + return err == nil || err.Error() != `value is cty.ObjectVal(map[string]cty.Value{"bar":cty.StringVal("baz"), "foo":cty.NumberIntVal(1), "qux":cty.UnknownVal(cty.String)})` + }, + }, + { + name: "callback with struct", + expr: hclExpr(`var.foo`), + target: func(val Object) error { + return fmt.Errorf("value is %#v", val) + }, + option: &tflint.EvaluateExprOption{WantType: &objectTy}, + serverImpl: func(expr hcl.Expression, opts tflint.EvaluateExprOption) (cty.Value, error) { + return cty.ObjectVal(map[string]cty.Value{ + "name": cty.StringVal("bar"), + "enabled": cty.BoolVal(true), + }), nil + }, + getFileImpl: fileExists, + errCheck: func(err error) bool { + return err == nil || err.Error() != `value is plugin2host.Object{Name:"bar", Enabled:true}` + }, + }, + { + name: "callback with unknown value as Go value", + expr: hclExpr(`var.foo`), + target: func(val string) error { + return fmt.Errorf("value is %s", val) + }, + serverImpl: func(expr hcl.Expression, opts tflint.EvaluateExprOption) (cty.Value, error) { + return cty.UnknownVal(cty.String), nil + }, + getFileImpl: fileExists, + errCheck: neverHappend, + }, + { + name: "callback with unknown value as cty.Value", + expr: hclExpr(`var.foo`), + target: func(val cty.Value) error { + return fmt.Errorf("value is %s", val.GoString()) + }, + serverImpl: func(expr hcl.Expression, opts tflint.EvaluateExprOption) (cty.Value, error) { + return cty.UnknownVal(cty.String), nil + }, + getFileImpl: fileExists, + errCheck: func(err error) bool { + return err == nil || err.Error() != `value is cty.UnknownVal(cty.String)` + }, + }, + { + name: "callback with null as Go value", + expr: hclExpr(`var.foo`), + target: func(val string) error { + return fmt.Errorf("value is %s", val) + }, + serverImpl: func(expr hcl.Expression, opts tflint.EvaluateExprOption) (cty.Value, error) { + return cty.NullVal(cty.String), nil + }, + getFileImpl: fileExists, + errCheck: neverHappend, + }, + { + name: "callback with null as cty.Value", + expr: hclExpr(`var.foo`), + target: func(val cty.Value) error { + return fmt.Errorf("value is %s", val.GoString()) + }, + serverImpl: func(expr hcl.Expression, opts tflint.EvaluateExprOption) (cty.Value, error) { + return cty.NullVal(cty.String), nil + }, + getFileImpl: fileExists, + errCheck: func(err error) bool { + return err == nil || err.Error() != `value is cty.NullVal(cty.String)` + }, + }, + { + name: "callback with sensitive value as Go value", + expr: hclExpr(`var.foo`), + target: func(val string) error { + return fmt.Errorf("value is %s", val) + }, + serverImpl: func(expr hcl.Expression, opts tflint.EvaluateExprOption) (cty.Value, error) { + return cty.StringVal("foo").Mark(marks.Sensitive), nil + }, + getFileImpl: fileExists, + errCheck: neverHappend, + }, + { + name: "callback with sensitive value as cty.Value", + expr: hclExpr(`var.foo`), + target: func(val cty.Value) error { + return fmt.Errorf("value is %s", val.GoString()) + }, + serverImpl: func(expr hcl.Expression, opts tflint.EvaluateExprOption) (cty.Value, error) { + return cty.StringVal("foo").Mark(marks.Sensitive), nil + }, + getFileImpl: fileExists, + errCheck: func(err error) bool { + return err == nil || err.Error() != `value is cty.StringVal("foo").Mark(marks.Sensitive)` + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + client := startTestGRPCServer(t, newMockServer(mockServerImpl{evaluateExpr: test.serverImpl, getFile: test.getFileImpl})) + + err := client.EvaluateExpr(test.expr, test.target, test.option) + if test.errCheck(err) { + t.Fatalf("failed to call EvaluateExpr: %s", err) + } + }) + } +} + // test rule for TestEmitIssue type Rule struct { tflint.DefaultRule diff --git a/tflint/interface.go b/tflint/interface.go index b82a780..069c6d1 100644 --- a/tflint/interface.go +++ b/tflint/interface.go @@ -159,13 +159,18 @@ type Runner interface { // See the hclext.DecodeBody documentation and examples for more details. DecodeRuleConfig(ruleName string, ret interface{}) error - // EvaluateExpr evaluates the passed expression and reflects the result in the 2nd argument. - // In addition to the obvious errors, this function returns an error if: - // - The expression contains unknown values (e.g. variables without defaults, `aws_instance.foo.arn`) - // - The expression contains null values - // - The expression contains sensitive values (variables with `sensitive = true`) + // EvaluateExpr evaluates the given expression and assigns the value to the Go value target. + // The target must be a pointer. Otherwise, it will cause a panic. // - // You can use `errors.Is` to ignore these errors: + // If the value cannot be assigned to the target, it returns an error. + // There are particularly examples such as: + // + // 1. Unknown value (e.g. variables without defaults, `aws_instance.foo.arn`) + // 2. NULL + // 3. Sensitive value (variables with `sensitive = true`) + // + // However, if the target is cty.Value, these errors will not be returned. + // These errors can be handled with errors.Is(). // // ``` // var val string @@ -187,17 +192,29 @@ type Runner interface { // } // ``` // - // The types that can be passed to the 2nd argument are assumed to be as follows: - // - string - // - int - // - []string - // - []int - // - map[string]string - // - map[string]int - // - cty.Value + // The following are the types that can be passed as the target: + // + // 1. string + // 2. int + // 3. []string + // 4. []int + // 5. map[string]string + // 6. map[string]int + // 7. cty.Value + // 8. func (v T) error + // + // Passing any other type will cause a panic. If you pass a function, the assigned value + // will be used as an argument to execute the function. In this case, if a value cannot be + // assigned to the argument type, instead of returning an error, the execution is skipped. + // This is useful when it is always acceptable to ignore exceptional values. + // + // ``` + // runner.EvaluateExpr(expr, func (val string) error { + // // Test value + // }, nil) + // ``` // - // Besides this, you can pass a structure. In that case, you need to explicitly pass - // the type in the option of the 3rd argument: + // Besides this, you can pass a structure. In that case, you need to explicitly pass wantType. // // ``` // type complexVal struct { @@ -213,7 +230,7 @@ type Runner interface { // var complexVals []complexVal // runner.EvaluateExpr(expr, &compleVals, &tflint.EvaluateExprOption{WantType: &wantType}) // ``` - EvaluateExpr(expr hcl.Expression, ret interface{}, option *EvaluateExprOption) error + EvaluateExpr(expr hcl.Expression, target interface{}, option *EvaluateExprOption) error // EmitIssue sends an issue to TFLint. You need to pass the message of the issue and the range. EmitIssue(rule Rule, message string, issueRange hcl.Range) error @@ -221,7 +238,7 @@ type Runner interface { // EnsureNoError is a helper for error handling. Depending on the type of error generated by EvaluateExpr, // determine whether to exit, skip, or continue. If it is continued, the passed function will be executed. // - // Deprecated: Use errors.Is() instead to determine which errors can be ignored. + // Deprecated: Use EvaluateExpr with a function callback. e.g. EvaluateExpr(expr, func (val T) error {}, ...) EnsureNoError(error, func() error) error }