From bde2ea55950d321d6f900d4bc78ca0853c25ff1c Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Fri, 17 Mar 2023 12:44:18 +0100 Subject: [PATCH 1/2] feat: show Terraform error details --- provisioner/terraform/diagnostic.go | 68 ++++++++++++++++++++++++ provisioner/terraform/diagnostic_test.go | 62 +++++++++++++++++++++ provisioner/terraform/executor.go | 17 +++--- 3 files changed, 137 insertions(+), 10 deletions(-) create mode 100644 provisioner/terraform/diagnostic.go create mode 100644 provisioner/terraform/diagnostic_test.go diff --git a/provisioner/terraform/diagnostic.go b/provisioner/terraform/diagnostic.go new file mode 100644 index 0000000000000..8266cad9725aa --- /dev/null +++ b/provisioner/terraform/diagnostic.go @@ -0,0 +1,68 @@ +package terraform + +import ( + "bytes" + "fmt" + "sort" + "strings" + + tfjson "github.com/hashicorp/terraform-json" +) + +// This implementation bases on the original Terraform formatter, which unfortunately is internal: +// https://github.com/hashicorp/terraform/blob/6b35927cf0988262739a5f0acea4790ae58a16d3/internal/command/format/diagnostic.go#L125 + +func FormatDiagnostic(diag *tfjson.Diagnostic) []string { + var buf bytes.Buffer + appendSourceSnippets(&buf, diag) + _, _ = buf.WriteString(diag.Detail) + return strings.Split(buf.String(), "\n") +} + +func appendSourceSnippets(buf *bytes.Buffer, diag *tfjson.Diagnostic) { + if diag.Range == nil { + return + } + + if diag.Snippet == nil { + // This should generally not happen, as long as sources are always + // loaded through the main loader. We may load things in other + // ways in weird cases, so we'll tolerate it at the expense of + // a not-so-helpful error message. + _, _ = fmt.Fprintf(buf, "on %s line %d:\n (source code not available)\n", diag.Range.Filename, diag.Range.Start.Line) + } else { + snippet := diag.Snippet + code := snippet.Code + + var contextStr string + if snippet.Context != nil { + contextStr = fmt.Sprintf(", in %s", *snippet.Context) + } + _, _ = fmt.Fprintf(buf, "on %s line %d%s:\n", diag.Range.Filename, diag.Range.Start.Line, contextStr) + + // Split the snippet into lines and render one at a time + lines := strings.Split(code, "\n") + for i, line := range lines { + _, _ = fmt.Fprintf(buf, " %d: %s\n", snippet.StartLine+i, line) + } + + if len(snippet.Values) > 0 { + // 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 + // multiple times with different values, such as blocks using + // "count" and "for_each", or within "for" expressions. + values := make([]tfjson.DiagnosticExpressionValue, len(snippet.Values)) + copy(values, snippet.Values) + sort.Slice(values, func(i, j int) bool { + return values[i].Traversal < values[j].Traversal + }) + + _, _ = buf.WriteString(" ├────────────────\n") + for _, value := range values { + _, _ = fmt.Fprintf(buf, " │ %s %s\n", value.Traversal, value.Statement) + } + } + } + _ = buf.WriteByte('\n') +} diff --git a/provisioner/terraform/diagnostic_test.go b/provisioner/terraform/diagnostic_test.go new file mode 100644 index 0000000000000..8904facc0e094 --- /dev/null +++ b/provisioner/terraform/diagnostic_test.go @@ -0,0 +1,62 @@ +package terraform_test + +import ( + "encoding/json" + "testing" + + tfjson "github.com/hashicorp/terraform-json" + "github.com/stretchr/testify/require" + + "github.com/coder/coder/provisioner/terraform" +) + +type hasDiagnostic struct { + Diagnostic *tfjson.Diagnostic `json:"diagnostic"` +} + +func TestFormatDiagnostic(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + input string + expected []string + }{ + "Expression": { + input: `{"@level":"error","@message":"Error: Unsupported attribute","@module":"terraform.ui","@timestamp":"2023-03-17T10:33:38.761493+01:00","diagnostic":{"severity":"error","summary":"Unsupported attribute","detail":"This object has no argument, nested block, or exported attribute named \"foobar\".","range":{"filename":"main.tf","start":{"line":230,"column":81,"byte":5648},"end":{"line":230,"column":88,"byte":5655}},"snippet":{"context":"resource \"docker_container\" \"workspace\"","code":" name = \"coder-${data.coder_workspace.me.owner}-${lower(data.coder_workspace.me.foobar)}\"","start_line":230,"highlight_start_offset":80,"highlight_end_offset":87,"values":[]}},"type":"diagnostic"}`, + expected: []string{ + "on main.tf line 230, in resource \"docker_container\" \"workspace\":", + " 230: name = \"coder-${data.coder_workspace.me.owner}-${lower(data.coder_workspace.me.foobar)}\"", + "", + "This object has no argument, nested block, or exported attribute named \"foobar\".", + }, + }, + "DynamicValues": { + input: `{"@level":"error","@message":"Error: Invalid value for variable","@module":"terraform.ui","@timestamp":"2023-03-17T12:25:37.864793+01:00","diagnostic":{"severity":"error","summary":"Invalid value for variable","detail":"Invalid Digital Ocean Project ID.\n\nThis was checked by the validation rule at main.tf:27,3-13.","range":{"filename":"main.tf","start":{"line":18,"column":1,"byte":277},"end":{"line":18,"column":31,"byte":307}},"snippet":{"context":null,"code":"variable \"step1_do_project_id\" {","start_line":18,"highlight_start_offset":0,"highlight_end_offset":30,"values":[{"traversal":"var.step1_do_project_id","statement":"is \"magic-project-id\""}]}},"type":"diagnostic"}`, + expected: []string{ + "on main.tf line 18:", + " 18: variable \"step1_do_project_id\" {", + " ├────────────────", + " │ var.step1_do_project_id is \"magic-project-id\"", + "", + "Invalid Digital Ocean Project ID.", + "", + "This was checked by the validation rule at main.tf:27,3-13.", + }, + }, + } + + for name, tc := range tests { + tc := tc + + t.Run(name, func(t *testing.T) { + t.Parallel() + + var d hasDiagnostic + err := json.Unmarshal([]byte(tc.input), &d) + require.NoError(t, err) + + output := terraform.FormatDiagnostic(d.Diagnostic) + require.Equal(t, tc.expected, output) + }) + } +} diff --git a/provisioner/terraform/executor.go b/provisioner/terraform/executor.go index f273e5245df4f..6c516da4ac122 100644 --- a/provisioner/terraform/executor.go +++ b/provisioner/terraform/executor.go @@ -496,8 +496,11 @@ func provisionReadAndLog(sink logSink, r io.Reader, done chan<- any) { if log.Diagnostic == nil { continue } - logLevel = convertTerraformLogLevel(log.Diagnostic.Severity, sink) - sink.Log(&proto.Log{Level: logLevel, Output: log.Diagnostic.Detail}) + logLevel = convertTerraformLogLevel(string(log.Diagnostic.Severity), sink) + + for _, diagLine := range FormatDiagnostic(log.Diagnostic) { + sink.Log(&proto.Log{Level: logLevel, Output: diagLine}) + } } } @@ -509,7 +512,7 @@ func convertTerraformLogLevel(logLevel string, sink logSink) proto.LogLevel { return proto.LogLevel_DEBUG case "info": return proto.LogLevel_INFO - case "warn": + case "warn", "warning": return proto.LogLevel_WARN case "error": return proto.LogLevel_ERROR @@ -526,13 +529,7 @@ type terraformProvisionLog struct { Level string `json:"@level"` Message string `json:"@message"` - Diagnostic *terraformProvisionLogDiagnostic `json:"diagnostic"` -} - -type terraformProvisionLogDiagnostic struct { - Severity string `json:"severity"` - Summary string `json:"summary"` - Detail string `json:"detail"` + Diagnostic *tfjson.Diagnostic `json:"diagnostic,omitempty"` } // syncWriter wraps an io.Writer in a sync.Mutex. From 50c0766230014728502a5c4846ddd426d16fe9cd Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Mon, 20 Mar 2023 11:15:13 +0100 Subject: [PATCH 2/2] Address PR comments --- provisioner/terraform/diagnostic.go | 4 ++-- provisioner/terraform/diagnostic_test.go | 3 ++- provisioner/terraform/executor.go | 3 +-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/provisioner/terraform/diagnostic.go b/provisioner/terraform/diagnostic.go index 8266cad9725aa..1718be9f85310 100644 --- a/provisioner/terraform/diagnostic.go +++ b/provisioner/terraform/diagnostic.go @@ -12,11 +12,11 @@ import ( // This implementation bases on the original Terraform formatter, which unfortunately is internal: // https://github.com/hashicorp/terraform/blob/6b35927cf0988262739a5f0acea4790ae58a16d3/internal/command/format/diagnostic.go#L125 -func FormatDiagnostic(diag *tfjson.Diagnostic) []string { +func FormatDiagnostic(diag *tfjson.Diagnostic) string { var buf bytes.Buffer appendSourceSnippets(&buf, diag) _, _ = buf.WriteString(diag.Detail) - return strings.Split(buf.String(), "\n") + return buf.String() } func appendSourceSnippets(buf *bytes.Buffer, diag *tfjson.Diagnostic) { diff --git a/provisioner/terraform/diagnostic_test.go b/provisioner/terraform/diagnostic_test.go index 8904facc0e094..836ae85e4915c 100644 --- a/provisioner/terraform/diagnostic_test.go +++ b/provisioner/terraform/diagnostic_test.go @@ -2,6 +2,7 @@ package terraform_test import ( "encoding/json" + "strings" "testing" tfjson "github.com/hashicorp/terraform-json" @@ -56,7 +57,7 @@ func TestFormatDiagnostic(t *testing.T) { require.NoError(t, err) output := terraform.FormatDiagnostic(d.Diagnostic) - require.Equal(t, tc.expected, output) + require.Equal(t, tc.expected, strings.Split(output, "\n")) }) } } diff --git a/provisioner/terraform/executor.go b/provisioner/terraform/executor.go index 6c516da4ac122..264647643f416 100644 --- a/provisioner/terraform/executor.go +++ b/provisioner/terraform/executor.go @@ -497,8 +497,7 @@ func provisionReadAndLog(sink logSink, r io.Reader, done chan<- any) { continue } logLevel = convertTerraformLogLevel(string(log.Diagnostic.Severity), sink) - - for _, diagLine := range FormatDiagnostic(log.Diagnostic) { + for _, diagLine := range strings.Split(FormatDiagnostic(log.Diagnostic), "\n") { sink.Log(&proto.Log{Level: logLevel, Output: diagLine}) } }