Skip to content

feat: improve terraform template parsing errors #2331

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 15, 2022
Merged

feat: improve terraform template parsing errors #2331

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6dd067e
feat: improve terraform template parsing errors
deansheather Jun 14, 2022
ca01fcc
chore: add tests for parse failures
deansheather Jun 14, 2022
c1673d1
fixup! chore: add tests for parse failures
deansheather Jun 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ require (
github.com/kirsle/configdir v0.0.0-20170128060238-e45d2f54772f
github.com/lib/pq v1.10.6
github.com/mattn/go-isatty v0.0.14
github.com/mitchellh/go-wordwrap v1.0.1
github.com/mitchellh/mapstructure v1.5.0
github.com/moby/moby v20.10.17+incompatible
github.com/open-policy-agent/opa v0.41.0
Expand Down Expand Up @@ -190,7 +191,6 @@ require (
github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369 // indirect
github.com/mgutz/ansi v0.0.0-20170206155736-9520e82c474b // indirect
github.com/miekg/dns v1.1.45 // indirect
github.com/mitchellh/go-wordwrap v1.0.1 // indirect
github.com/moby/term v0.0.0-20210619224110-3f7ff695adc6 // indirect
github.com/muesli/ansi v0.0.0-20211031195517-c9f0611b6c70 // indirect
github.com/muesli/reflow v0.3.0 // indirect
Expand Down
56 changes: 51 additions & 5 deletions provisioner/terraform/parse.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,26 @@ package terraform

import (
"encoding/json"
"fmt"
"os"
"path/filepath"
"strings"

"github.com/hashicorp/terraform-config-inspect/tfconfig"
"github.com/mitchellh/go-wordwrap"
"golang.org/x/xerrors"

"github.com/coder/coder/provisionersdk/proto"
)

// Parse extracts Terraform variables from source-code.
func (*terraform) Parse(request *proto.Parse_Request, stream proto.DRPCProvisioner_ParseStream) error {
defer func() {
_ = stream.CloseSend()
}()

// Load the module and print any parse errors.
module, diags := tfconfig.LoadModule(request.Directory)
if diags.HasErrors() {
return xerrors.Errorf("load module: %w", diags.Err())
return xerrors.Errorf("load module: %s", formatDiagnostics(request.Directory, diags))
}

parameters := make([]*proto.ParameterSchema, 0, len(module.Variables))
for _, v := range module.Variables {
schema, err := convertVariableToParameter(v)
Expand Down Expand Up @@ -83,3 +85,47 @@ func convertVariableToParameter(variable *tfconfig.Variable) (*proto.ParameterSc

return schema, nil
}

// formatDiagnostics returns a nicely formatted string containing all of the
// error details within the tfconfig.Diagnostics. We need to use this because
// the default format doesn't provide much useful information.
func formatDiagnostics(baseDir string, diags tfconfig.Diagnostics) string {
var msgs strings.Builder
for _, d := range diags {
// Convert severity.
severity := "UNKNOWN SEVERITY"
switch {
case d.Severity == tfconfig.DiagError:
severity = "ERROR"
case d.Severity == tfconfig.DiagWarning:
severity = "WARN"
}

// Determine filepath and line
location := "unknown location"
if d.Pos != nil {
filename, err := filepath.Rel(baseDir, d.Pos.Filename)
if err != nil {
filename = d.Pos.Filename
}
location = fmt.Sprintf("%s:%d", filename, d.Pos.Line)
}

_, _ = msgs.WriteString(fmt.Sprintf("\n%s: %s (%s)\n", severity, d.Summary, location))

// Wrap the details to 80 characters and indent them.
if d.Detail != "" {
wrapped := wordwrap.WrapString(d.Detail, 78)
for _, line := range strings.Split(wrapped, "\n") {
_, _ = msgs.WriteString(fmt.Sprintf("> %s\n", line))
}
}
}

spacer := " "
if len(diags) > 1 {
spacer = "\n\n"
}

return spacer + strings.TrimSpace(msgs.String())
}
142 changes: 84 additions & 58 deletions provisioner/terraform/parse_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,83 +38,99 @@ func TestParse(t *testing.T) {
}()
api := proto.NewDRPCProvisionerClient(provisionersdk.Conn(client))

for _, testCase := range []struct {
testCases := []struct {
Name string
Files map[string]string
Response *proto.Parse_Response
}{{
Name: "single-variable",
Files: map[string]string{
"main.tf": `variable "A" {
// If ErrorContains is not empty, then response.Recv() should return an
// error containing this string before a Complete response is returned.
ErrorContains string
}{
{
Name: "single-variable",
Files: map[string]string{
"main.tf": `variable "A" {
description = "Testing!"
}`,
},
Response: &proto.Parse_Response{
Type: &proto.Parse_Response_Complete{
Complete: &proto.Parse_Complete{
ParameterSchemas: []*proto.ParameterSchema{{
Name: "A",
RedisplayValue: true,
AllowOverrideSource: true,
Description: "Testing!",
DefaultDestination: &proto.ParameterDestination{
Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE,
},
}},
},
Response: &proto.Parse_Response{
Type: &proto.Parse_Response_Complete{
Complete: &proto.Parse_Complete{
ParameterSchemas: []*proto.ParameterSchema{{
Name: "A",
RedisplayValue: true,
AllowOverrideSource: true,
Description: "Testing!",
DefaultDestination: &proto.ParameterDestination{
Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE,
},
}},
},
},
},
},
}, {
Name: "default-variable-value",
Files: map[string]string{
"main.tf": `variable "A" {
{
Name: "default-variable-value",
Files: map[string]string{
"main.tf": `variable "A" {
default = "wow"
}`,
},
Response: &proto.Parse_Response{
Type: &proto.Parse_Response_Complete{
Complete: &proto.Parse_Complete{
ParameterSchemas: []*proto.ParameterSchema{{
Name: "A",
RedisplayValue: true,
AllowOverrideSource: true,
DefaultSource: &proto.ParameterSource{
Scheme: proto.ParameterSource_DATA,
Value: "wow",
},
DefaultDestination: &proto.ParameterDestination{
Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE,
},
}},
},
Response: &proto.Parse_Response{
Type: &proto.Parse_Response_Complete{
Complete: &proto.Parse_Complete{
ParameterSchemas: []*proto.ParameterSchema{{
Name: "A",
RedisplayValue: true,
AllowOverrideSource: true,
DefaultSource: &proto.ParameterSource{
Scheme: proto.ParameterSource_DATA,
Value: "wow",
},
DefaultDestination: &proto.ParameterDestination{
Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE,
},
}},
},
},
},
},
}, {
Name: "variable-validation",
Files: map[string]string{
"main.tf": `variable "A" {
{
Name: "variable-validation",
Files: map[string]string{
"main.tf": `variable "A" {
validation {
condition = var.A == "value"
}
}`,
},
Response: &proto.Parse_Response{
Type: &proto.Parse_Response_Complete{
Complete: &proto.Parse_Complete{
ParameterSchemas: []*proto.ParameterSchema{{
Name: "A",
RedisplayValue: true,
ValidationCondition: `var.A == "value"`,
ValidationTypeSystem: proto.ParameterSchema_HCL,
AllowOverrideSource: true,
DefaultDestination: &proto.ParameterDestination{
Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE,
},
}},
},
Response: &proto.Parse_Response{
Type: &proto.Parse_Response_Complete{
Complete: &proto.Parse_Complete{
ParameterSchemas: []*proto.ParameterSchema{{
Name: "A",
RedisplayValue: true,
ValidationCondition: `var.A == "value"`,
ValidationTypeSystem: proto.ParameterSchema_HCL,
AllowOverrideSource: true,
DefaultDestination: &proto.ParameterDestination{
Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE,
},
}},
},
},
},
},
}} {
{
Name: "bad-syntax",
Files: map[string]string{
"main.tf": "a;sd;ajsd;lajsd;lasjdf;a",
},
ErrorContains: `The ";" character is not valid.`,
},
}

for _, testCase := range testCases {
testCase := testCase
t.Run(testCase.Name, func(t *testing.T) {
t.Parallel()
Expand All @@ -133,11 +149,21 @@ func TestParse(t *testing.T) {

for {
msg, err := response.Recv()
require.NoError(t, err)
if err != nil {
if testCase.ErrorContains != "" {
require.ErrorContains(t, err, testCase.ErrorContains)
break
}

require.NoError(t, err)
}

if msg.GetComplete() == nil {
continue
}
if testCase.ErrorContains != "" {
t.Fatal("expected error but job completed successfully")
}

// Ensure the want and got are equivalent!
want, err := json.Marshal(testCase.Response)
Expand Down
57 changes: 28 additions & 29 deletions provisioner/terraform/provision.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,30 +70,6 @@ func (t *terraform) Provision(stream proto.DRPCProvisioner_ProvisionStream) erro
return xerrors.Errorf("terraform version %q is too old. required >= %q", version.String(), minimumTerraformVersion.String())
}

statefilePath := filepath.Join(start.Directory, "terraform.tfstate")
if len(start.State) > 0 {
err := os.WriteFile(statefilePath, start.State, 0600)
if err != nil {
return xerrors.Errorf("write statefile %q: %w", statefilePath, err)
}
}

reader, writer := io.Pipe()
defer reader.Close()
defer writer.Close()
go func() {
scanner := bufio.NewScanner(reader)
for scanner.Scan() {
_ = stream.Send(&proto.Provision_Response{
Type: &proto.Provision_Response_Log{
Log: &proto.Log{
Level: proto.LogLevel_DEBUG,
Output: scanner.Text(),
},
},
})
}
}()
terraformEnv := map[string]string{}
// Required for "terraform init" to find "git" to
// clone Terraform modules.
Expand All @@ -113,15 +89,38 @@ func (t *terraform) Provision(stream proto.DRPCProvisioner_ProvisionStream) erro
if err != nil {
return xerrors.Errorf("set terraform env: %w", err)
}
terraform.SetStdout(writer)
t.logger.Debug(shutdown, "running initialization")

statefilePath := filepath.Join(start.Directory, "terraform.tfstate")
if len(start.State) > 0 {
err := os.WriteFile(statefilePath, start.State, 0600)
if err != nil {
return xerrors.Errorf("write statefile %q: %w", statefilePath, err)
}
}

reader, writer := io.Pipe()
go func(reader *io.PipeReader) {
scanner := bufio.NewScanner(reader)
for scanner.Scan() {
_ = stream.Send(&proto.Provision_Response{
Type: &proto.Provision_Response_Log{
Log: &proto.Log{
Level: proto.LogLevel_ERROR,
Output: scanner.Text(),
},
},
})
}
}(reader)

terraform.SetStderr(writer)
err = terraform.Init(shutdown)
_ = reader.Close()
_ = writer.Close()
if err != nil {
return xerrors.Errorf("initialize terraform: %w", err)
}
t.logger.Debug(shutdown, "ran initialization")
_ = reader.Close()
terraform.SetStdout(io.Discard)
terraform.SetStderr(io.Discard)

env := os.Environ()
env = append(env,
Expand Down
Loading