diff --git a/go.mod b/go.mod index a6b5d55d154b..c91bde89a698 100644 --- a/go.mod +++ b/go.mod @@ -45,7 +45,7 @@ require ( github.com/hashicorp/go-uuid v1.0.2 github.com/hashicorp/go-version v1.3.0 github.com/hashicorp/hcl v0.0.0-20170504190234-a4b07c25de5f - github.com/hashicorp/hcl/v2 v2.12.0 + github.com/hashicorp/hcl/v2 v2.13.0 github.com/hashicorp/terraform-config-inspect v0.0.0-20210209133302-4fd17a0faac2 github.com/hashicorp/terraform-svchost v0.0.0-20200729002733-f050f53b9734 github.com/jmespath/go-jmespath v0.4.0 diff --git a/go.sum b/go.sum index 797c9050e1dd..f0386fa2c128 100644 --- a/go.sum +++ b/go.sum @@ -410,8 +410,8 @@ github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ github.com/hashicorp/hcl v0.0.0-20170504190234-a4b07c25de5f h1:UdxlrJz4JOnY8W+DbLISwf2B8WXEolNRA8BGCwI9jws= github.com/hashicorp/hcl v0.0.0-20170504190234-a4b07c25de5f/go.mod h1:oZtUIOe8dh44I2q6ScRibXws4Ajl+d+nod3AaR9vL5w= github.com/hashicorp/hcl/v2 v2.0.0/go.mod h1:oVVDG71tEinNGYCxinCYadcmKU9bglqW9pV3txagJ90= -github.com/hashicorp/hcl/v2 v2.12.0 h1:PsYxySWpMD4KPaoJLnsHwtK5Qptvj/4Q6s0t4sUxZf4= -github.com/hashicorp/hcl/v2 v2.12.0/go.mod h1:FwWsfWEjyV/CMj8s/gqAuiviY72rJ1/oayI9WftqcKg= +github.com/hashicorp/hcl/v2 v2.13.0 h1:0Apadu1w6M11dyGFxWnmhhcMjkbAiKCv7G1r/2QgCNc= +github.com/hashicorp/hcl/v2 v2.13.0/go.mod h1:e4z5nxYlWNPdDSNYX+ph14EvWYMFm3eP0zIUqPc2jr0= github.com/hashicorp/jsonapi v0.0.0-20210826224640-ee7dae0fb22d h1:9ARUJJ1VVynB176G1HCwleORqCaXm/Vx0uUi0dL26I0= github.com/hashicorp/jsonapi v0.0.0-20210826224640-ee7dae0fb22d/go.mod h1:Yog5+CPEM3c99L1CL2CFCYoSzgWm5vTU58idbRUaLik= github.com/hashicorp/logutils v1.0.0/go.mod h1:QIAnNjmIWmVIIkWDTG1z5v++HQmx9WQRO+LraFDTW64= @@ -682,7 +682,6 @@ github.com/yuin/goldmark v1.4.0/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1 github.com/zclconf/go-cty v1.0.0/go.mod h1:xnAOWiHeOqg2nWS62VtQ7pbOu17FtxJNW8RLEih+O3s= github.com/zclconf/go-cty v1.1.0/go.mod h1:xnAOWiHeOqg2nWS62VtQ7pbOu17FtxJNW8RLEih+O3s= github.com/zclconf/go-cty v1.2.0/go.mod h1:hOPWgoHbaTUnI5k4D2ld+GRpFJSCe6bCM7m1q/N4PQ8= -github.com/zclconf/go-cty v1.8.0/go.mod h1:vVKLxnk3puL4qRAv72AO+W99LUD4da90g3uUAzyuvAk= github.com/zclconf/go-cty v1.10.0 h1:mp9ZXQeIcN8kAwuqorjH+Q+njbJKjLrvB2yIh4q7U+0= github.com/zclconf/go-cty v1.10.0/go.mod h1:vVKLxnk3puL4qRAv72AO+W99LUD4da90g3uUAzyuvAk= github.com/zclconf/go-cty-debug v0.0.0-20191215020915-b22d67c1ba0b h1:FosyBZYxY34Wul7O/MSKey3txpPYyCqVO5ZyceuQJEI= diff --git a/internal/command/format/diagnostic.go b/internal/command/format/diagnostic.go index 6fbfe4113bf7..780592a08c51 100644 --- a/internal/command/format/diagnostic.go +++ b/internal/command/format/diagnostic.go @@ -278,7 +278,7 @@ func appendSourceSnippets(buf *bytes.Buffer, diag *viewsjson.Diagnostic, color * ) } - if len(snippet.Values) > 0 { + if len(snippet.Values) > 0 || (snippet.FunctionCall != nil && snippet.FunctionCall.Signature != nil) { // The diagnostic may also have information about the dynamic // values of relevant variables at the point of evaluation. // This is particularly useful for expressions that get evaluated @@ -291,6 +291,24 @@ func appendSourceSnippets(buf *bytes.Buffer, diag *viewsjson.Diagnostic, color * }) fmt.Fprint(buf, color.Color(" [dark_gray]├────────────────[reset]\n")) + if callInfo := snippet.FunctionCall; callInfo != nil && callInfo.Signature != nil { + + fmt.Fprintf(buf, color.Color(" [dark_gray]│[reset] while calling [bold]%s[reset]("), callInfo.CalledAs) + for i, param := range callInfo.Signature.Params { + if i > 0 { + buf.WriteString(", ") + } + buf.WriteString(param.Name) + } + if param := callInfo.Signature.VariadicParam; param != nil { + if len(callInfo.Signature.Params) > 0 { + buf.WriteString(", ") + } + buf.WriteString(param.Name) + buf.WriteString("...") + } + buf.WriteString(")\n") + } for _, value := range values { fmt.Fprintf(buf, color.Color(" [dark_gray]│[reset] [bold]%s[reset] %s\n"), value.Traversal, value.Statement) } diff --git a/internal/command/format/diagnostic_test.go b/internal/command/format/diagnostic_test.go index 184aa168071d..95f2ed6aa1ba 100644 --- a/internal/command/format/diagnostic_test.go +++ b/internal/command/format/diagnostic_test.go @@ -6,9 +6,11 @@ import ( "github.com/google/go-cmp/cmp" "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hclsyntax" "github.com/hashicorp/hcl/v2/hcltest" "github.com/mitchellh/colorstring" "github.com/zclconf/go-cty/cty" + "github.com/zclconf/go-cty/cty/function" viewsjson "github.com/hashicorp/terraform/internal/command/views/json" "github.com/hashicorp/terraform/internal/lang/marks" @@ -128,6 +130,7 @@ func TestDiagnostic(t *testing.T) { }), }, }, + Extra: diagnosticCausedBySensitive(true), }, `[red]╷[reset] [red]│[reset] [bold][red]Error: [reset][bold]Bad bad bad[reset] @@ -162,6 +165,7 @@ func TestDiagnostic(t *testing.T) { }), }, }, + Extra: diagnosticCausedByUnknown(true), }, `[red]╷[reset] [red]│[reset] [bold][red]Error: [reset][bold]Bad bad bad[reset] @@ -196,6 +200,7 @@ func TestDiagnostic(t *testing.T) { }), }, }, + Extra: diagnosticCausedByUnknown(true), }, `[red]╷[reset] [red]│[reset] [bold][red]Error: [reset][bold]Bad bad bad[reset] @@ -207,6 +212,54 @@ func TestDiagnostic(t *testing.T) { [red]│[reset] [red]│[reset] Whatever shall we do? [red]╵[reset] +`, + }, + "error with source code subject and function call annotation": { + &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Bad bad bad", + Detail: "Whatever shall we do?", + Subject: &hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{Line: 1, Column: 6, Byte: 5}, + End: hcl.Pos{Line: 1, Column: 12, Byte: 11}, + }, + Expression: hcltest.MockExprLiteral(cty.True), + EvalContext: &hcl.EvalContext{ + Functions: map[string]function.Function{ + "beep": function.New(&function.Spec{ + Params: []function.Parameter{ + { + Name: "pos_param_0", + Type: cty.String, + }, + { + Name: "pos_param_1", + Type: cty.Number, + }, + }, + VarParam: &function.Parameter{ + Name: "var_param", + Type: cty.Bool, + }, + }), + }, + }, + // This is simulating what the HCL function call expression + // type would generate on evaluation, by implementing the + // same interface it uses. + Extra: fakeDiagFunctionCallExtra("beep"), + }, + `[red]╷[reset] +[red]│[reset] [bold][red]Error: [reset][bold]Bad bad bad[reset] +[red]│[reset] +[red]│[reset] on test.tf line 1: +[red]│[reset] 1: test [underline]source[reset] code +[red]│[reset] [dark_gray]├────────────────[reset] +[red]│[reset] [dark_gray]│[reset] while calling [bold]beep[reset](pos_param_0, pos_param_1, var_param...) +[red]│[reset] +[red]│[reset] Whatever shall we do? +[red]╵[reset] `, }, } @@ -341,6 +394,7 @@ Whatever shall we do? }), }, }, + Extra: diagnosticCausedBySensitive(true), }, ` Error: Bad bad bad @@ -350,6 +404,37 @@ Error: Bad bad bad ├──────────────── │ boop.beep has a sensitive value +Whatever shall we do? +`, + }, + "error with source code subject and expression referring to sensitive value when not related to sensitivity": { + &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Bad bad bad", + Detail: "Whatever shall we do?", + Subject: &hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{Line: 1, Column: 6, Byte: 5}, + End: hcl.Pos{Line: 1, Column: 12, Byte: 11}, + }, + Expression: hcltest.MockExprTraversal(hcl.Traversal{ + hcl.TraverseRoot{Name: "boop"}, + hcl.TraverseAttr{Name: "beep"}, + }), + EvalContext: &hcl.EvalContext{ + Variables: map[string]cty.Value{ + "boop": cty.ObjectVal(map[string]cty.Value{ + "beep": cty.StringVal("blah").Mark(marks.Sensitive), + }), + }, + }, + }, + ` +Error: Bad bad bad + + on test.tf line 1: + 1: test source code + Whatever shall we do? `, }, @@ -374,6 +459,7 @@ Whatever shall we do? }), }, }, + Extra: diagnosticCausedByUnknown(true), }, ` Error: Bad bad bad @@ -383,6 +469,39 @@ Error: Bad bad bad ├──────────────── │ boop.beep is a string, known only after apply +Whatever shall we do? +`, + }, + "error with source code subject and unknown string expression when problem isn't unknown-related": { + &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Bad bad bad", + Detail: "Whatever shall we do?", + Subject: &hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{Line: 1, Column: 6, Byte: 5}, + End: hcl.Pos{Line: 1, Column: 12, Byte: 11}, + }, + Expression: hcltest.MockExprTraversal(hcl.Traversal{ + hcl.TraverseRoot{Name: "boop"}, + hcl.TraverseAttr{Name: "beep"}, + }), + EvalContext: &hcl.EvalContext{ + Variables: map[string]cty.Value{ + "boop": cty.ObjectVal(map[string]cty.Value{ + "beep": cty.UnknownVal(cty.String), + }), + }, + }, + }, + ` +Error: Bad bad bad + + on test.tf line 1: + 1: test source code + ├──────────────── + │ boop.beep is a string + Whatever shall we do? `, }, @@ -407,6 +526,7 @@ Whatever shall we do? }), }, }, + Extra: diagnosticCausedByUnknown(true), }, ` Error: Bad bad bad @@ -416,6 +536,37 @@ Error: Bad bad bad ├──────────────── │ boop.beep will be known only after apply +Whatever shall we do? +`, + }, + "error with source code subject and unknown expression of unknown type when problem isn't unknown-related": { + &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Bad bad bad", + Detail: "Whatever shall we do?", + Subject: &hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{Line: 1, Column: 6, Byte: 5}, + End: hcl.Pos{Line: 1, Column: 12, Byte: 11}, + }, + Expression: hcltest.MockExprTraversal(hcl.Traversal{ + hcl.TraverseRoot{Name: "boop"}, + hcl.TraverseAttr{Name: "beep"}, + }), + EvalContext: &hcl.EvalContext{ + Variables: map[string]cty.Value{ + "boop": cty.ObjectVal(map[string]cty.Value{ + "beep": cty.UnknownVal(cty.DynamicPseudoType), + }), + }, + }, + }, + ` +Error: Bad bad bad + + on test.tf line 1: + 1: test source code + Whatever shall we do? `, }, @@ -755,3 +906,40 @@ func TestDiagnosticFromJSON_invalid(t *testing.T) { }) } } + +// fakeDiagFunctionCallExtra is a fake implementation of the interface that +// HCL uses to provide "extra information" associated with diagnostics that +// describe errors during a function call. +type fakeDiagFunctionCallExtra string + +var _ hclsyntax.FunctionCallDiagExtra = fakeDiagFunctionCallExtra("") + +func (e fakeDiagFunctionCallExtra) CalledFunctionName() string { + return string(e) +} + +func (e fakeDiagFunctionCallExtra) FunctionCallError() error { + return nil +} + +// diagnosticCausedByUnknown is a testing helper for exercising our logic +// for selectively showing unknown values alongside our source snippets for +// diagnostics that are explicitly marked as being caused by unknown values. +type diagnosticCausedByUnknown bool + +var _ tfdiags.DiagnosticExtraBecauseUnknown = diagnosticCausedByUnknown(true) + +func (e diagnosticCausedByUnknown) DiagnosticCausedByUnknown() bool { + return bool(e) +} + +// diagnosticCausedBySensitive is a testing helper for exercising our logic +// for selectively showing sensitive values alongside our source snippets for +// diagnostics that are explicitly marked as being caused by sensitive values. +type diagnosticCausedBySensitive bool + +var _ tfdiags.DiagnosticExtraBecauseSensitive = diagnosticCausedBySensitive(true) + +func (e diagnosticCausedBySensitive) DiagnosticCausedBySensitive() bool { + return bool(e) +} diff --git a/internal/command/views/json/diagnostic.go b/internal/command/views/json/diagnostic.go index 6a4bba9b8ce9..1175792c72a2 100644 --- a/internal/command/views/json/diagnostic.go +++ b/internal/command/views/json/diagnostic.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hcled" "github.com/hashicorp/hcl/v2/hclparse" + "github.com/hashicorp/hcl/v2/hclsyntax" "github.com/hashicorp/terraform/internal/lang/marks" "github.com/hashicorp/terraform/internal/tfdiags" "github.com/zclconf/go-cty/cty" @@ -95,6 +96,10 @@ type DiagnosticSnippet struct { // Values is a sorted slice of expression values which may be useful in // understanding the source of an error in a complex expression. Values []DiagnosticExpressionValue `json:"values"` + + // FunctionCall is information about a function call whose failure is + // being reported by this diagnostic, if any. + FunctionCall *DiagnosticFunctionCall `json:"function_call,omitempty"` } // DiagnosticExpressionValue represents an HCL traversal string (e.g. @@ -107,6 +112,20 @@ type DiagnosticExpressionValue struct { Statement string `json:"statement"` } +// DiagnosticFunctionCall represents a function call whose information is +// being included as part of a diagnostic snippet. +type DiagnosticFunctionCall struct { + // CalledAs is the full name that was used to call this function, + // potentially including namespace prefixes if the function does not belong + // to the default function namespace. + CalledAs string `json:"called_as"` + + // Signature is a description of the signature of the function that was + // called, if any. Might be omitted if we're reporting that a call failed + // because the given function name isn't known, for example. + Signature *Function `json:"signature,omitempty"` +} + // NewDiagnostic takes a tfdiags.Diagnostic and a map of configuration sources, // and returns a Diagnostic struct. func NewDiagnostic(diag tfdiags.Diagnostic, sources map[string][]byte) *Diagnostic { @@ -252,6 +271,8 @@ func NewDiagnostic(diag tfdiags.Diagnostic, sources map[string][]byte) *Diagnost vars := expr.Variables() values := make([]DiagnosticExpressionValue, 0, len(vars)) seen := make(map[string]struct{}, len(vars)) + includeUnknown := tfdiags.DiagnosticCausedByUnknown(diag) + includeSensitive := tfdiags.DiagnosticCausedBySensitive(diag) Traversals: for _, traversal := range vars { for len(traversal) > 1 { @@ -273,14 +294,35 @@ func NewDiagnostic(diag tfdiags.Diagnostic, sources map[string][]byte) *Diagnost } switch { case val.HasMark(marks.Sensitive): - // We won't say anything at all about sensitive values, - // because we might give away something that was - // sensitive about them. + // We only mention a sensitive value if the diagnostic + // we're rendering is explicitly marked as being + // caused by sensitive values, because otherwise + // readers tend to be misled into thinking the error + // is caused by the sensitive value even when it isn't. + if !includeSensitive { + continue Traversals + } + // Even when we do mention one, we keep it vague + // in order to minimize the chance of giving away + // whatever was sensitive about it. value.Statement = "has a sensitive value" case !val.IsKnown(): + // We'll avoid saying anything about unknown or + // "known after apply" unless the diagnostic is + // explicitly marked as being caused by unknown + // values, because otherwise readers tend to be + // misled into thinking the error is caused by the + // unknown value even when it isn't. if ty := val.Type(); ty != cty.DynamicPseudoType { - value.Statement = fmt.Sprintf("is a %s, known only after apply", ty.FriendlyName()) + if includeUnknown { + value.Statement = fmt.Sprintf("is a %s, known only after apply", ty.FriendlyName()) + } else { + value.Statement = fmt.Sprintf("is a %s", ty.FriendlyName()) + } } else { + if !includeUnknown { + continue Traversals + } value.Statement = "will be known only after apply" } default: @@ -294,7 +336,24 @@ func NewDiagnostic(diag tfdiags.Diagnostic, sources map[string][]byte) *Diagnost return values[i].Traversal < values[j].Traversal }) diagnostic.Snippet.Values = values + + if callInfo := tfdiags.ExtraInfo[hclsyntax.FunctionCallDiagExtra](diag); callInfo != nil && callInfo.CalledFunctionName() != "" { + calledAs := callInfo.CalledFunctionName() + baseName := calledAs + if idx := strings.LastIndex(baseName, "::"); idx >= 0 { + baseName = baseName[idx+2:] + } + callInfo := &DiagnosticFunctionCall{ + CalledAs: calledAs, + } + if f, ok := ctx.Functions[calledAs]; ok { + callInfo.Signature = DescribeFunction(baseName, f) + } + diagnostic.Snippet.FunctionCall = callInfo + } + } + } } diff --git a/internal/command/views/json/diagnostic_test.go b/internal/command/views/json/diagnostic_test.go index 640b82bc60cb..422dade9b3cb 100644 --- a/internal/command/views/json/diagnostic_test.go +++ b/internal/command/views/json/diagnostic_test.go @@ -412,6 +412,7 @@ func TestNewDiagnostic(t *testing.T) { }), }, }, + Extra: diagnosticCausedBySensitive(true), }, &Diagnostic{ Severity: "error", @@ -445,6 +446,61 @@ func TestNewDiagnostic(t *testing.T) { }, }, }, + "error with source code subject and expression referring to sensitive value when not caused by sensitive values": { + &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Wrong noises", + Detail: "Biological sounds are not allowed", + Subject: &hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{Line: 2, Column: 9, Byte: 42}, + End: hcl.Pos{Line: 2, Column: 26, Byte: 59}, + }, + Expression: hcltest.MockExprTraversal(hcl.Traversal{ + hcl.TraverseRoot{Name: "var"}, + hcl.TraverseAttr{Name: "boop"}, + hcl.TraverseIndex{Key: cty.StringVal("hello!")}, + }), + EvalContext: &hcl.EvalContext{ + Variables: map[string]cty.Value{ + "var": cty.ObjectVal(map[string]cty.Value{ + "boop": cty.MapVal(map[string]cty.Value{ + "hello!": cty.StringVal("bleurgh").Mark(marks.Sensitive), + }), + }), + }, + }, + }, + &Diagnostic{ + Severity: "error", + Summary: "Wrong noises", + Detail: "Biological sounds are not allowed", + Range: &DiagnosticRange{ + Filename: "test.tf", + Start: Pos{ + Line: 2, + Column: 9, + Byte: 42, + }, + End: Pos{ + Line: 2, + Column: 26, + Byte: 59, + }, + }, + Snippet: &DiagnosticSnippet{ + Context: strPtr(`resource "test_resource" "test"`), + Code: (` foo = var.boop["hello!"]`), + StartLine: (2), + HighlightStartOffset: (8), + HighlightEndOffset: (25), + Values: []DiagnosticExpressionValue{ + // The sensitive value is filtered out because this is + // not a sensitive-value-related diagnostic message. + }, + }, + }, + }, "error with source code subject and expression referring to a collection containing a sensitive value": { &hcl.Diagnostic{ Severity: hcl.DiagError, @@ -525,6 +581,7 @@ func TestNewDiagnostic(t *testing.T) { }), }, }, + Extra: diagnosticCausedByUnknown(true), }, &Diagnostic{ Severity: "error", @@ -582,6 +639,7 @@ func TestNewDiagnostic(t *testing.T) { }), }, }, + Extra: diagnosticCausedByUnknown(true), }, &Diagnostic{ Severity: "error", @@ -615,6 +673,61 @@ func TestNewDiagnostic(t *testing.T) { }, }, }, + "error with source code subject and unknown expression of unknown type when not caused by unknown values": { + &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Wrong noises", + Detail: "Biological sounds are not allowed", + Subject: &hcl.Range{ + Filename: "test.tf", + Start: hcl.Pos{Line: 2, Column: 9, Byte: 42}, + End: hcl.Pos{Line: 2, Column: 26, Byte: 59}, + }, + Expression: hcltest.MockExprTraversal(hcl.Traversal{ + hcl.TraverseRoot{Name: "var"}, + hcl.TraverseAttr{Name: "boop"}, + hcl.TraverseIndex{Key: cty.StringVal("hello!")}, + }), + EvalContext: &hcl.EvalContext{ + Variables: map[string]cty.Value{ + "var": cty.ObjectVal(map[string]cty.Value{ + "boop": cty.MapVal(map[string]cty.Value{ + "hello!": cty.UnknownVal(cty.DynamicPseudoType), + }), + }), + }, + }, + }, + &Diagnostic{ + Severity: "error", + Summary: "Wrong noises", + Detail: "Biological sounds are not allowed", + Range: &DiagnosticRange{ + Filename: "test.tf", + Start: Pos{ + Line: 2, + Column: 9, + Byte: 42, + }, + End: Pos{ + Line: 2, + Column: 26, + Byte: 59, + }, + }, + Snippet: &DiagnosticSnippet{ + Context: strPtr(`resource "test_resource" "test"`), + Code: (` foo = var.boop["hello!"]`), + StartLine: (2), + HighlightStartOffset: (8), + HighlightEndOffset: (25), + Values: []DiagnosticExpressionValue{ + // The unknown value is filtered out because this is + // not an unknown-value-related diagnostic message. + }, + }, + }, + }, "error with source code subject with multiple expression values": { &hcl.Diagnostic{ Severity: hcl.DiagError, @@ -666,6 +779,7 @@ func TestNewDiagnostic(t *testing.T) { }), }, }, + Extra: diagnosticCausedBySensitive(true), }, &Diagnostic{ Severity: "error", @@ -813,3 +927,25 @@ func TestNewDiagnostic(t *testing.T) { // are fields which are pointer-to-string to ensure that the rendered JSON // results in `null` for an empty value, rather than `""`. func strPtr(s string) *string { return &s } + +// diagnosticCausedByUnknown is a testing helper for exercising our logic +// for selectively showing unknown values alongside our source snippets for +// diagnostics that are explicitly marked as being caused by unknown values. +type diagnosticCausedByUnknown bool + +var _ tfdiags.DiagnosticExtraBecauseUnknown = diagnosticCausedByUnknown(true) + +func (e diagnosticCausedByUnknown) DiagnosticCausedByUnknown() bool { + return bool(e) +} + +// diagnosticCausedBySensitive is a testing helper for exercising our logic +// for selectively showing sensitive values alongside our source snippets for +// diagnostics that are explicitly marked as being caused by sensitive values. +type diagnosticCausedBySensitive bool + +var _ tfdiags.DiagnosticExtraBecauseSensitive = diagnosticCausedBySensitive(true) + +func (e diagnosticCausedBySensitive) DiagnosticCausedBySensitive() bool { + return bool(e) +} diff --git a/internal/command/views/json/function.go b/internal/command/views/json/function.go new file mode 100644 index 000000000000..f36986dae078 --- /dev/null +++ b/internal/command/views/json/function.go @@ -0,0 +1,112 @@ +package json + +import ( + "encoding/json" + + "github.com/zclconf/go-cty/cty" + "github.com/zclconf/go-cty/cty/function" +) + +// Function is a description of the JSON representation of the signature of +// a function callable from the Terraform language. +type Function struct { + // Name is the leaf name of the function, without any namespace prefix. + Name string `json:"name"` + + Params []FunctionParam `json:"params"` + VariadicParam *FunctionParam `json:"variadic_param,omitempty"` + + // ReturnType is type constraint which is a static approximation of the + // possibly-dynamic return type of the function. + ReturnType json.RawMessage `json:"return_type"` + + Description string `json:"description,omitempty"` + DescriptionKind string `json:"description_kind,omitempty"` +} + +// FunctionParam represents a single parameter to a function, as represented +// by type Function. +type FunctionParam struct { + // Name is a name for the function which is used primarily for + // documentation purposes, because function arguments are positional + // and therefore don't appear directly in configuration source code. + Name string `json:"name"` + + // Type is a type constraint which is a static approximation of the + // possibly-dynamic type of the parameter. Particular functions may + // have additional requirements that a type constraint alone cannot + // represent. + Type json.RawMessage `json:"type"` + + // Maybe some of the other fields in function.Parameter would be + // interesting to describe here too, but we'll wait to see if there + // is a use-case first. + + Description string `json:"description,omitempty"` + DescriptionKind string `json:"description_kind,omitempty"` +} + +// DescribeFunction returns a description of the signature of the given cty +// function, as a pointer to this package's serializable type Function. +func DescribeFunction(name string, f function.Function) *Function { + ret := &Function{ + Name: name, + } + + params := f.Params() + ret.Params = make([]FunctionParam, len(params)) + typeCheckArgs := make([]cty.Type, len(params), len(params)+1) + for i, param := range params { + ret.Params[i] = describeFunctionParam(¶m) + typeCheckArgs[i] = param.Type + } + if varParam := f.VarParam(); varParam != nil { + descParam := describeFunctionParam(varParam) + ret.VariadicParam = &descParam + typeCheckArgs = append(typeCheckArgs, varParam.Type) + } + + retType, err := f.ReturnType(typeCheckArgs) + if err != nil { + // Getting an error when type-checking with exactly the type constraints + // the function called for is weird, so we'll just treat it as if it + // has a dynamic return type instead, for our purposes here. + // One reason this can happen is for a function which has a variadic + // parameter but has logic inside it which considers it invalid to + // specify exactly one argument for that parameter (since that's what + // we did in typeCheckArgs as an approximation of a valid call above.) + retType = cty.DynamicPseudoType + } + + if raw, err := retType.MarshalJSON(); err != nil { + // Again, we'll treat any errors as if the function is dynamically + // typed because it would be weird to get here. + ret.ReturnType = json.RawMessage(`"dynamic"`) + } else { + ret.ReturnType = json.RawMessage(raw) + } + + // We don't currently have any sense of descriptions for functions and + // their parameters, so we'll just leave those fields unpopulated for now. + + return ret +} + +func describeFunctionParam(p *function.Parameter) FunctionParam { + ret := FunctionParam{ + Name: p.Name, + } + + if raw, err := p.Type.MarshalJSON(); err != nil { + // We'll treat any errors as if the function is dynamically + // typed because it would be weird to get here. + ret.Type = json.RawMessage(`"dynamic"`) + } else { + ret.Type = json.RawMessage(raw) + } + + // We don't currently have any sense of descriptions for functions and + // their parameters, so we'll just leave those fields unpopulated for now. + + return ret +} diff --git a/internal/command/views/json/function_test.go b/internal/command/views/json/function_test.go new file mode 100644 index 000000000000..48d4a1323350 --- /dev/null +++ b/internal/command/views/json/function_test.go @@ -0,0 +1,92 @@ +package json + +import ( + "encoding/json" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/zclconf/go-cty/cty/function" + "github.com/zclconf/go-cty/cty/function/stdlib" +) + +func TestDescribeFunction(t *testing.T) { + // NOTE: This test case is referring to some real functions in other + // packages. and so if those functions change signature later it will + // probably make some cases here fail. If that is the cause of the failure, + // it's fine to update the test here to match rather than to revert the + // change to the function signature, as long as the change to the + // function signature is otherwise within the bounds of our compatibility + // promises. + + tests := map[string]struct { + Function function.Function + Want *Function + }{ + "upper": { + Function: stdlib.UpperFunc, + Want: &Function{ + Name: "upper", + Params: []FunctionParam{ + { + Name: "str", + Type: json.RawMessage(`"string"`), + }, + }, + ReturnType: json.RawMessage(`"string"`), + }, + }, + "coalesce": { + Function: stdlib.CoalesceFunc, + Want: &Function{ + Name: "coalesce", + Params: []FunctionParam{}, + VariadicParam: &FunctionParam{ + Name: "vals", + Type: json.RawMessage(`"dynamic"`), + }, + ReturnType: json.RawMessage(`"dynamic"`), + }, + }, + "join": { + Function: stdlib.JoinFunc, + Want: &Function{ + Name: "join", + Params: []FunctionParam{ + { + Name: "separator", + Type: json.RawMessage(`"string"`), + }, + }, + VariadicParam: &FunctionParam{ + Name: "lists", + Type: json.RawMessage(`["list","string"]`), + }, + ReturnType: json.RawMessage(`"string"`), + }, + }, + "jsonencode": { + Function: stdlib.JSONEncodeFunc, + Want: &Function{ + Name: "jsonencode", + Params: []FunctionParam{ + { + Name: "val", + Type: json.RawMessage(`"dynamic"`), + }, + }, + ReturnType: json.RawMessage(`"string"`), + }, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + got := DescribeFunction(name, test.Function) + want := test.Want + + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("wrong result\n%s", diff) + } + }) + } +} diff --git a/internal/command/views/json/testdata/diagnostic/error-with-source-code-subject-and-expression-referring-to-sensitive-value-when-not-caused-by-sensitive-values.json b/internal/command/views/json/testdata/diagnostic/error-with-source-code-subject-and-expression-referring-to-sensitive-value-when-not-caused-by-sensitive-values.json new file mode 100644 index 000000000000..8d3625dbe78d --- /dev/null +++ b/internal/command/views/json/testdata/diagnostic/error-with-source-code-subject-and-expression-referring-to-sensitive-value-when-not-caused-by-sensitive-values.json @@ -0,0 +1,26 @@ +{ + "severity": "error", + "summary": "Wrong noises", + "detail": "Biological sounds are not allowed", + "range": { + "filename": "test.tf", + "start": { + "line": 2, + "column": 9, + "byte": 42 + }, + "end": { + "line": 2, + "column": 26, + "byte": 59 + } + }, + "snippet": { + "context": "resource \"test_resource\" \"test\"", + "code": " foo = var.boop[\"hello!\"]", + "start_line": 2, + "highlight_start_offset": 8, + "highlight_end_offset": 25, + "values": [] + } +} diff --git a/internal/command/views/json/testdata/diagnostic/error-with-source-code-subject-and-unknown-expression-of-unknown-type-when-not-caused-by-unknown-values.json b/internal/command/views/json/testdata/diagnostic/error-with-source-code-subject-and-unknown-expression-of-unknown-type-when-not-caused-by-unknown-values.json new file mode 100644 index 000000000000..8d3625dbe78d --- /dev/null +++ b/internal/command/views/json/testdata/diagnostic/error-with-source-code-subject-and-unknown-expression-of-unknown-type-when-not-caused-by-unknown-values.json @@ -0,0 +1,26 @@ +{ + "severity": "error", + "summary": "Wrong noises", + "detail": "Biological sounds are not allowed", + "range": { + "filename": "test.tf", + "start": { + "line": 2, + "column": 9, + "byte": 42 + }, + "end": { + "line": 2, + "column": 26, + "byte": 59 + } + }, + "snippet": { + "context": "resource \"test_resource\" \"test\"", + "code": " foo = var.boop[\"hello!\"]", + "start_line": 2, + "highlight_start_offset": 8, + "highlight_end_offset": 25, + "values": [] + } +} diff --git a/internal/earlyconfig/diagnostics.go b/internal/earlyconfig/diagnostics.go index 9a6b2663636d..15adad56385a 100644 --- a/internal/earlyconfig/diagnostics.go +++ b/internal/earlyconfig/diagnostics.go @@ -76,3 +76,7 @@ func (d wrappedDiagnostic) Source() tfdiags.Source { func (d wrappedDiagnostic) FromExpr() *tfdiags.FromExpr { return nil } + +func (d wrappedDiagnostic) ExtraInfo() interface{} { + return nil +} diff --git a/internal/terraform/context_plan_test.go b/internal/terraform/context_plan_test.go index d05c2eb75c8d..2323dcc3b445 100644 --- a/internal/terraform/context_plan_test.go +++ b/internal/terraform/context_plan_test.go @@ -3093,6 +3093,15 @@ func TestContext2Plan_forEachUnknownValue(t *testing.T) { if !strings.Contains(gotErrStr, wantErrStr) { t.Fatalf("missing expected error\ngot: %s\n\nwant: error containing %q", gotErrStr, wantErrStr) } + + // We should have a diagnostic that is marked as being caused by unknown + // values. + for _, diag := range diags { + if tfdiags.DiagnosticCausedByUnknown(diag) { + return // don't fall through to the error below + } + } + t.Fatalf("no diagnostic is marked as being caused by unknown\n%s", diags.Err().Error()) } func TestContext2Plan_destroy(t *testing.T) { diff --git a/internal/terraform/diagnostics.go b/internal/terraform/diagnostics.go new file mode 100644 index 000000000000..26f22f06ce6c --- /dev/null +++ b/internal/terraform/diagnostics.go @@ -0,0 +1,42 @@ +package terraform + +import ( + "github.com/hashicorp/terraform/internal/tfdiags" +) + +// This file contains some package-local helpers for working with diagnostics. +// For the main diagnostics API, see the separate "tfdiags" package. + +// diagnosticCausedByUnknown is an implementation of +// tfdiags.DiagnosticExtraBecauseUnknown which we can use in the "Extra" field +// of a diagnostic to indicate that the problem was caused by unknown values +// being involved in an expression evaluation. +// +// When using this, set the Extra to diagnosticCausedByUnknown(true) and also +// populate the EvalContext and Expression fields of the diagnostic so that +// the diagnostic renderer can use all of that information together to assist +// the user in understanding what was unknown. +type diagnosticCausedByUnknown bool + +var _ tfdiags.DiagnosticExtraBecauseUnknown = diagnosticCausedByUnknown(true) + +func (e diagnosticCausedByUnknown) DiagnosticCausedByUnknown() bool { + return bool(e) +} + +// diagnosticCausedBySensitive is an implementation of +// tfdiags.DiagnosticExtraBecauseSensitive which we can use in the "Extra" field +// of a diagnostic to indicate that the problem was caused by sensitive values +// being involved in an expression evaluation. +// +// When using this, set the Extra to diagnosticCausedBySensitive(true) and also +// populate the EvalContext and Expression fields of the diagnostic so that +// the diagnostic renderer can use all of that information together to assist +// the user in understanding what was sensitive. +type diagnosticCausedBySensitive bool + +var _ tfdiags.DiagnosticExtraBecauseSensitive = diagnosticCausedBySensitive(true) + +func (e diagnosticCausedBySensitive) DiagnosticCausedBySensitive() bool { + return bool(e) +} diff --git a/internal/terraform/eval_count.go b/internal/terraform/eval_count.go index c74a051b301f..d4ab1a998f60 100644 --- a/internal/terraform/eval_count.go +++ b/internal/terraform/eval_count.go @@ -31,6 +31,12 @@ func evaluateCountExpression(expr hcl.Expression, ctx EvalContext) (int, tfdiags Summary: "Invalid count argument", Detail: `The "count" value depends on resource attributes that cannot be determined until apply, so Terraform cannot predict how many instances will be created. To work around this, use the -target argument to first apply only the resources that the count depends on.`, Subject: expr.Range().Ptr(), + + // TODO: Also populate Expression and EvalContext in here, but + // we can't easily do that right now because the hcl.EvalContext + // (which is not the same as the ctx we have in scope here) is + // hidden away inside evaluateCountExpressionValue. + Extra: diagnosticCausedByUnknown(true), }) } @@ -91,7 +97,7 @@ func evaluateCountExpressionValue(expr hcl.Expression, ctx EvalContext) (cty.Val diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Invalid count argument", - Detail: `The given "count" argument value is unsuitable: negative numbers are not supported.`, + Detail: `The given "count" argument value is unsuitable: must be greater than or equal to zero.`, Subject: expr.Range().Ptr(), }) return nullCount, diags diff --git a/internal/terraform/eval_for_each.go b/internal/terraform/eval_for_each.go index ba7a8c9bca6c..3c80ebff01d3 100644 --- a/internal/terraform/eval_for_each.go +++ b/internal/terraform/eval_for_each.go @@ -70,6 +70,7 @@ func evaluateForEachExpressionValue(expr hcl.Expression, ctx EvalContext, allowU Subject: expr.Range().Ptr(), Expression: expr, EvalContext: hclCtx, + Extra: diagnosticCausedBySensitive(true), }) } @@ -109,6 +110,7 @@ func evaluateForEachExpressionValue(expr hcl.Expression, ctx EvalContext, allowU Subject: expr.Range().Ptr(), Expression: expr, EvalContext: hclCtx, + Extra: diagnosticCausedByUnknown(true), }) } // ensure that we have a map, and not a DynamicValue @@ -144,6 +146,7 @@ func evaluateForEachExpressionValue(expr hcl.Expression, ctx EvalContext, allowU Subject: expr.Range().Ptr(), Expression: expr, EvalContext: hclCtx, + Extra: diagnosticCausedByUnknown(true), }) } return cty.UnknownVal(ty), diags diff --git a/internal/terraform/eval_for_each_test.go b/internal/terraform/eval_for_each_test.go index be4b551cf039..05dba9cabc46 100644 --- a/internal/terraform/eval_for_each_test.go +++ b/internal/terraform/eval_for_each_test.go @@ -88,38 +88,45 @@ func TestEvaluateForEachExpression_valid(t *testing.T) { func TestEvaluateForEachExpression_errors(t *testing.T) { tests := map[string]struct { - Expr hcl.Expression - Summary, DetailSubstring string + Expr hcl.Expression + Summary, DetailSubstring string + CausedByUnknown, CausedBySensitive bool }{ "null set": { hcltest.MockExprLiteral(cty.NullVal(cty.Set(cty.String))), "Invalid for_each argument", `the given "for_each" argument value is null`, + false, false, }, "string": { hcltest.MockExprLiteral(cty.StringVal("i am definitely a set")), "Invalid for_each argument", "must be a map, or set of strings, and you have provided a value of type string", + false, false, }, "list": { hcltest.MockExprLiteral(cty.ListVal([]cty.Value{cty.StringVal("a"), cty.StringVal("a")})), "Invalid for_each argument", "must be a map, or set of strings, and you have provided a value of type list", + false, false, }, "tuple": { hcltest.MockExprLiteral(cty.TupleVal([]cty.Value{cty.StringVal("a"), cty.StringVal("b")})), "Invalid for_each argument", "must be a map, or set of strings, and you have provided a value of type tuple", + false, false, }, "unknown string set": { hcltest.MockExprLiteral(cty.UnknownVal(cty.Set(cty.String))), "Invalid for_each argument", "set includes values derived from resource attributes that cannot be determined until apply", + true, false, }, "unknown map": { hcltest.MockExprLiteral(cty.UnknownVal(cty.Map(cty.Bool))), "Invalid for_each argument", "map includes keys derived from resource attributes that cannot be determined until apply", + true, false, }, "marked map": { hcltest.MockExprLiteral(cty.MapVal(map[string]cty.Value{ @@ -128,31 +135,37 @@ func TestEvaluateForEachExpression_errors(t *testing.T) { }).Mark(marks.Sensitive)), "Invalid for_each argument", "Sensitive values, or values derived from sensitive values, cannot be used as for_each arguments. If used, the sensitive value could be exposed as a resource instance key.", + false, true, }, "set containing booleans": { hcltest.MockExprLiteral(cty.SetVal([]cty.Value{cty.BoolVal(true)})), "Invalid for_each set argument", "supports maps and sets of strings, but you have provided a set containing type bool", + false, false, }, "set containing null": { hcltest.MockExprLiteral(cty.SetVal([]cty.Value{cty.NullVal(cty.String)})), "Invalid for_each set argument", "must not contain null values", + false, false, }, "set containing unknown value": { hcltest.MockExprLiteral(cty.SetVal([]cty.Value{cty.UnknownVal(cty.String)})), "Invalid for_each argument", "set includes values derived from resource attributes that cannot be determined until apply", + true, false, }, "set containing dynamic unknown value": { hcltest.MockExprLiteral(cty.SetVal([]cty.Value{cty.UnknownVal(cty.DynamicPseudoType)})), "Invalid for_each argument", "set includes values derived from resource attributes that cannot be determined until apply", + true, false, }, "set containing marked values": { hcltest.MockExprLiteral(cty.SetVal([]cty.Value{cty.StringVal("beep").Mark(marks.Sensitive), cty.StringVal("boop")})), "Invalid for_each argument", "Sensitive values, or values derived from sensitive values, cannot be used as for_each arguments. If used, the sensitive value could be exposed as a resource instance key.", + false, true, }, } @@ -184,6 +197,13 @@ func TestEvaluateForEachExpression_errors(t *testing.T) { } else { t.Errorf("diagnostic does not support FromExpr\ngot: %s", spew.Sdump(diags[0])) } + + if got, want := tfdiags.DiagnosticCausedByUnknown(diags[0]), test.CausedByUnknown; got != want { + t.Errorf("wrong result from tfdiags.DiagnosticCausedByUnknown\ngot: %#v\nwant: %#v", got, want) + } + if got, want := tfdiags.DiagnosticCausedBySensitive(diags[0]), test.CausedBySensitive; got != want { + t.Errorf("wrong result from tfdiags.DiagnosticCausedBySensitive\ngot: %#v\nwant: %#v", got, want) + } }) } } diff --git a/internal/tfdiags/consolidate_warnings.go b/internal/tfdiags/consolidate_warnings.go index 06f3d52cc022..08d36d60b6c8 100644 --- a/internal/tfdiags/consolidate_warnings.go +++ b/internal/tfdiags/consolidate_warnings.go @@ -119,6 +119,10 @@ func (wg *warningGroup) FromExpr() *FromExpr { return wg.Warnings[0].FromExpr() } +func (wg *warningGroup) ExtraInfo() interface{} { + return wg.Warnings[0].ExtraInfo() +} + func (wg *warningGroup) Append(diag Diagnostic) { if diag.Severity() != Warning { panic("can't append a non-warning diagnostic to a warningGroup") diff --git a/internal/tfdiags/diagnostic.go b/internal/tfdiags/diagnostic.go index c241ab4220c4..f988f398b6c9 100644 --- a/internal/tfdiags/diagnostic.go +++ b/internal/tfdiags/diagnostic.go @@ -15,6 +15,13 @@ type Diagnostic interface { // available. Returns nil if the diagnostic is not related to an // expression evaluation. FromExpr() *FromExpr + + // ExtraInfo returns the raw extra information value. This is a low-level + // API which requires some work on the part of the caller to properly + // access associated information, so in most cases it'll be more convienient + // to use the package-level ExtraInfo function to try to unpack a particular + // specialized interface from this value. + ExtraInfo() interface{} } type Severity rune diff --git a/internal/tfdiags/diagnostic_base.go b/internal/tfdiags/diagnostic_base.go index 04e56773deb2..88495290e7a8 100644 --- a/internal/tfdiags/diagnostic_base.go +++ b/internal/tfdiags/diagnostic_base.go @@ -31,3 +31,7 @@ func (d diagnosticBase) Source() Source { func (d diagnosticBase) FromExpr() *FromExpr { return nil } + +func (d diagnosticBase) ExtraInfo() interface{} { + return nil +} diff --git a/internal/tfdiags/diagnostic_extra.go b/internal/tfdiags/diagnostic_extra.go new file mode 100644 index 000000000000..97a14746c462 --- /dev/null +++ b/internal/tfdiags/diagnostic_extra.go @@ -0,0 +1,171 @@ +package tfdiags + +// This "Extra" idea is something we've inherited from HCL's diagnostic model, +// and so it's primarily to expose that functionality from wrapped HCL +// diagnostics but other diagnostic types could potentially implement this +// protocol too, if needed. + +// ExtraInfo tries to retrieve extra information of interface type T from +// the given diagnostic. +// +// "Extra information" is situation-specific additional contextual data which +// might allow for some special tailored reporting of particular +// diagnostics in the UI. Conventionally the extra information is provided +// as a hidden type that implements one or more interfaces which a caller +// can pass as type parameter T to retrieve a value of that type when the +// diagnostic has such an implementation. +// +// If the given diagnostic's extra value has an implementation of interface T +// then ExtraInfo returns a non-nil interface value. If there is no such +// implementation, ExtraInfo returns a nil T. +// +// Although the signature of this function does not constrain T to be an +// interface type, our convention is to only use interface types to access +// extra info in order to allow for alternative or wrapping implementations +// of the interface. +func ExtraInfo[T any](diag Diagnostic) T { + extra := diag.ExtraInfo() + if ret, ok := extra.(T); ok { + return ret + } + + // If "extra" doesn't implement T directly then we'll delegate to + // our ExtraInfoNext helper to try iteratively unwrapping it. + return ExtraInfoNext[T](extra) +} + +// ExtraInfoNext takes a value previously returned by ExtraInfo and attempts +// to find an implementation of interface T wrapped inside of it. The return +// value meaning is the same as for ExtraInfo. +// +// This is to help with the less common situation where a particular "extra" +// value might be wrapping another value implementing the same interface, +// and so callers can peel away one layer at a time until there are no more +// nested layers. +// +// Because this function is intended for searching for _nested_ implementations +// of T, ExtraInfoNext does not consider whether value "previous" directly +// implements interface T, on the assumption that the previous call to ExtraInfo +// with the same T caused "previous" to already be that result. +func ExtraInfoNext[T any](previous interface{}) T { + // As long as T is an interface type as documented, zero will always be + // a nil interface value for us to return in the non-matching case. + var zero T + + unwrapper, ok := previous.(DiagnosticExtraUnwrapper) + // If the given value isn't unwrappable then it can't possibly have + // any other info nested inside of it. + if !ok { + return zero + } + + extra := unwrapper.UnwrapDiagnosticExtra() + + // We'll keep unwrapping until we either find the interface we're + // looking for or we run out of layers of unwrapper. + for { + if ret, ok := extra.(T); ok { + return ret + } + + if unwrapper, ok := extra.(DiagnosticExtraUnwrapper); ok { + extra = unwrapper.UnwrapDiagnosticExtra() + } else { + return zero + } + } +} + +// DiagnosticExtraUnwrapper is an interface implemented by values in the +// Extra field of Diagnostic when they are wrapping another "Extra" value that +// was generated downstream. +// +// Diagnostic recipients which want to examine "Extra" values to sniff for +// particular types of extra data can either type-assert this interface +// directly and repeatedly unwrap until they recieve nil, or can use the +// helper function DiagnosticExtra. +// +// This interface intentionally matches hcl.DiagnosticExtraUnwrapper, so that +// wrapping extra values implemented using HCL's API will also work with the +// tfdiags API, but that non-HCL uses of this will not need to implement HCL +// just to get this interface. +type DiagnosticExtraUnwrapper interface { + // If the reciever is wrapping another "diagnostic extra" value, returns + // that value. Otherwise returns nil to indicate dynamically that nothing + // is wrapped. + // + // The "nothing is wrapped" condition can be signalled either by this + // method returning nil or by a type not implementing this interface at all. + // + // Implementers should never create unwrap "cycles" where a nested extra + // value returns a value that was also wrapping it. + UnwrapDiagnosticExtra() interface{} +} + +// DiagnosticExtraBecauseUnknown is an interface implemented by values in +// the Extra field of Diagnostic when the diagnostic is potentially caused by +// the presence of unknown values in an expression evaluation. +// +// Just implementing this interface is not sufficient signal, though. Callers +// must also call the DiagnosticCausedByUnknown method in order to confirm +// the result, or use the package-level function DiagnosticCausedByUnknown +// as a convenient wrapper. +type DiagnosticExtraBecauseUnknown interface { + // DiagnosticCausedByUnknown returns true if the associated diagnostic + // was caused by the presence of unknown values during an expression + // evaluation, or false otherwise. + // + // Callers might use this to tailor what contextual information they show + // alongside an error report in the UI, to avoid potential confusion + // caused by talking about the presence of unknown values if that was + // immaterial to the error. + DiagnosticCausedByUnknown() bool +} + +// DiagnosticCausedByUnknown returns true if the given diagnostic has an +// indication that it was caused by the presence of unknown values during +// an expression evaluation. +// +// This is a wrapper around checking if the diagnostic's extra info implements +// interface DiagnosticExtraBecauseUnknown and then calling its method if so. +func DiagnosticCausedByUnknown(diag Diagnostic) bool { + maybe := ExtraInfo[DiagnosticExtraBecauseUnknown](diag) + if maybe == nil { + return false + } + return maybe.DiagnosticCausedByUnknown() +} + +// DiagnosticExtraBecauseSensitive is an interface implemented by values in +// the Extra field of Diagnostic when the diagnostic is potentially caused by +// the presence of sensitive values in an expression evaluation. +// +// Just implementing this interface is not sufficient signal, though. Callers +// must also call the DiagnosticCausedBySensitive method in order to confirm +// the result, or use the package-level function DiagnosticCausedBySensitive +// as a convenient wrapper. +type DiagnosticExtraBecauseSensitive interface { + // DiagnosticCausedBySensitive returns true if the associated diagnostic + // was caused by the presence of sensitive values during an expression + // evaluation, or false otherwise. + // + // Callers might use this to tailor what contextual information they show + // alongside an error report in the UI, to avoid potential confusion + // caused by talking about the presence of sensitive values if that was + // immaterial to the error. + DiagnosticCausedBySensitive() bool +} + +// DiagnosticCausedBySensitive returns true if the given diagnostic has an +// indication that it was caused by the presence of sensitive values during +// an expression evaluation. +// +// This is a wrapper around checking if the diagnostic's extra info implements +// interface DiagnosticExtraBecauseSensitive and then calling its method if so. +func DiagnosticCausedBySensitive(diag Diagnostic) bool { + maybe := ExtraInfo[DiagnosticExtraBecauseSensitive](diag) + if maybe == nil { + return false + } + return maybe.DiagnosticCausedBySensitive() +} diff --git a/internal/tfdiags/error.go b/internal/tfdiags/error.go index 13f7a714f42a..1e26bf96805e 100644 --- a/internal/tfdiags/error.go +++ b/internal/tfdiags/error.go @@ -26,3 +26,8 @@ func (e nativeError) FromExpr() *FromExpr { // Native errors are not expression-related return nil } + +func (e nativeError) ExtraInfo() interface{} { + // Native errors don't carry any "extra information". + return nil +} diff --git a/internal/tfdiags/hcl.go b/internal/tfdiags/hcl.go index ad0d8220f996..edf16b5b4d73 100644 --- a/internal/tfdiags/hcl.go +++ b/internal/tfdiags/hcl.go @@ -50,6 +50,10 @@ func (d hclDiagnostic) FromExpr() *FromExpr { } } +func (d hclDiagnostic) ExtraInfo() interface{} { + return d.diag.Extra +} + // SourceRangeFromHCL constructs a SourceRange from the corresponding range // type within the HCL package. func SourceRangeFromHCL(hclRange hcl.Range) SourceRange { diff --git a/internal/tfdiags/rpc_friendly.go b/internal/tfdiags/rpc_friendly.go index 485063b0c0e4..4c627bf98aac 100644 --- a/internal/tfdiags/rpc_friendly.go +++ b/internal/tfdiags/rpc_friendly.go @@ -54,6 +54,11 @@ func (d rpcFriendlyDiag) FromExpr() *FromExpr { return nil } +func (d rpcFriendlyDiag) ExtraInfo() interface{} { + // RPC-friendly diagnostics always discard any "extra information". + return nil +} + func init() { gob.Register((*rpcFriendlyDiag)(nil)) } diff --git a/internal/tfdiags/simple_warning.go b/internal/tfdiags/simple_warning.go index b0f1ecd46c60..3c18f1924762 100644 --- a/internal/tfdiags/simple_warning.go +++ b/internal/tfdiags/simple_warning.go @@ -28,3 +28,8 @@ func (e simpleWarning) FromExpr() *FromExpr { // Simple warnings are not expression-related return nil } + +func (e simpleWarning) ExtraInfo() interface{} { + // Simple warnings cannot carry extra information. + return nil +}