fix(coderd/templatebuilder): include Terraform diagnostics in import errors (#26677)

`ClassifyProvisionerError` previously returned only the raw job error string (e.g. "terraform plan: exit status 1") for unrecognized failures, discarding the provisioner log lines it already had. This made template import errors in the builder unactionable. Now the function extracts Terraform diagnostic blocks (Error:/Warning: lines and their context) from the provisioner logs and appends them to the error detail. This surfaces the actual failure cause (missing credentials, invalid references, unsupported blocks) in the ErrorAlert banner. Changes: - `extractDiagnostics` parses Terraform diagnostic blocks from log output, capped at 20 lines with a truncation marker - Auth error classification for AWS, GCP, Azure credential failures with a targeted user-facing message - Case-insensitive pattern matching via `strings.ToLower` on the combined text - Auth branch guarded against empty diagnostics (no trailing `\n\n`) > [!NOTE] > This PR was authored by Coder Agents on behalf of @jeremyruppel.
2026-09-21 20:51:01 +08:00 · 2026-06-30 18:43:58 -04:00
parent c40b3b9bcb
commit a48ace5017
3 changed files with 320 additions and 61 deletions
@@ -99,6 +99,27 @@ func TestCompose(t *testing.T) {
 		require.Contains(t, result.ExtraFiles, "cloud-init/cloud-config.yaml.tftpl")
 	})

+	t.Run("GCPWindowsBase", func(t *testing.T) {
+		t.Parallel()
+		result, err := templatebuilder.Compose(templatebuilder.ComposeRequest{
+			BaseTemplateID: "gcp-windows",
+			RegistryURL:    "https://registry.coder.com",
+		})
+		require.NoError(t, err)
+		require.NotEmpty(t, result.MainTF)
+		require.Contains(t, string(result.MainTF), `resource "coder_agent" "main"`)
+	})
+
+	t.Run("AzureLinuxExtraFiles", func(t *testing.T) {
+		t.Parallel()
+		result, err := templatebuilder.Compose(templatebuilder.ComposeRequest{
+			BaseTemplateID: "azure-linux",
+			RegistryURL:    "https://registry.coder.com",
+		})
+		require.NoError(t, err)
+		require.Contains(t, result.ExtraFiles, "cloud-init/cloud-config.yaml.tftpl")
+	})
+
 	t.Run("SensitiveVariable", func(t *testing.T) {
 		t.Parallel()
 		result, err := templatebuilder.Compose(templatebuilder.ComposeRequest{
@@ -3,7 +3,9 @@ package templatebuilder
 import "strings"

 // networkErrorPatterns are substrings found in provisioner job output when
-// the Terraform registry or provider endpoints are unreachable.
+// the Terraform registry or provider endpoints are unreachable. These are
+// intentionally not augmented with diagnostic context because the canned
+// message is self-explanatory and the underlying logs rarely add value.
 var networkErrorPatterns = []string{
 	"no such host",
 	"connection refused",
@@ -11,14 +13,35 @@ var networkErrorPatterns = []string{
 	"dial tcp: lookup",
 	"network is unreachable",
 	"no route to host",
-	"TLS handshake timeout",
+	"tls handshake timeout",
 }

+// authErrorPatterns are substrings that indicate cloud provider
+// authentication or credential failures. Matching is case-insensitive
+// (the combined text is lowercased before comparison).
+var authErrorPatterns = []string{
+	"no valid credential sources found",
+	"could not find default credentials",
+	"unauthorizedaccess",
+	"authorizationfailed",
+	"authfailure",
+	"accessdenied",
+	"invalidclienttokenid",
+	"expiredtoken",
+	"signaturedoesnotmatch",
+	"not authorized to perform",
+}
+
+// maxLogContextLines caps how many relevant log lines are included in
+// the error detail to avoid returning excessive output.
+const maxLogContextLines = 20
+
 // ClassifyProvisionerError inspects a provisioner job error and its log
 // lines, returning a user-friendly message for known failure modes.
-// If the error is not recognized, the raw jobError is returned unchanged.
+// For unrecognized errors, the raw jobError is returned with relevant
+// log context appended so the user can diagnose the failure.
 func ClassifyProvisionerError(jobError string, logs []string) string {
-	combined := jobError + "\n" + strings.Join(logs, "\n")
+	combined := strings.ToLower(jobError + "\n" + strings.Join(logs, "\n"))

 	for _, pattern := range networkErrorPatterns {
 		if strings.Contains(combined, pattern) {
@@ -28,5 +51,87 @@ func ClassifyProvisionerError(jobError string, logs []string) string {
 		}
 	}

+	for _, pattern := range authErrorPatterns {
+		if strings.Contains(combined, pattern) {
+			msg := "Cloud provider authentication failed. " +
+				"Check that valid credentials are configured for the provisioner."
+			if diag := extractDiagnostics(logs); diag != "" {
+				msg += "\n\n" + diag
+			}
+			return msg
+		}
+	}
+
+	// For unrecognized errors, include relevant Terraform output so the
+	// user can diagnose the failure.
+	if diag := extractDiagnostics(logs); diag != "" {
+		return jobError + "\n\n" + diag
+	}
 	return jobError
 }
+
+// diagnosticPrefixes identify Terraform diagnostic output lines worth
+// surfacing. Terraform formats errors as blocks starting with "Error:"
+// or "Warning:" followed by indented detail lines.
+var diagnosticPrefixes = []string{
+	"Error:",
+	"Warning:",
+	"error:",
+	"warning:",
+}
+
+// isDiagnosticStart returns true if the line begins a Terraform
+// diagnostic block.
+func isDiagnosticStart(line string) bool {
+	for _, prefix := range diagnosticPrefixes {
+		if strings.HasPrefix(line, prefix) {
+			return true
+		}
+	}
+	return false
+}
+
+// extractDiagnostics pulls Terraform diagnostic blocks from provisioner
+// log output. It keeps lines that start a diagnostic (Error:/Warning:)
+// and subsequent indented or context lines, up to maxLogContextLines.
+func extractDiagnostics(logs []string) string {
+	var lines []string
+	inBlock := false
+
+	for _, line := range logs {
+		if len(lines) >= maxLogContextLines {
+			lines = append(lines, "... (truncated)")
+			break
+		}
+
+		trimmed := strings.TrimSpace(line)
+		if trimmed == "" {
+			if inBlock {
+				// Blank line inside a block: keep it to preserve
+				// readability, but end the block.
+				lines = append(lines, "")
+				inBlock = false
+			}
+			continue
+		}
+
+		if isDiagnosticStart(trimmed) {
+			inBlock = true
+			lines = append(lines, trimmed)
+			continue
+		}
+
+		if inBlock {
+			lines = append(lines, trimmed)
+			continue
+		}
+
+		// Capture "on <file> line <N>" references that appear
+		// outside the main diagnostic block in some Terraform versions.
+		if strings.HasPrefix(trimmed, "on ") && strings.Contains(trimmed, " line ") {
+			lines = append(lines, trimmed)
+		}
+	}
+
+	return strings.Join(lines, "\n")
+}
@@ -1,6 +1,7 @@
 package templatebuilder_test

 import (
+	"strings"
 	"testing"

 	"github.com/stretchr/testify/require"
@@ -11,62 +12,194 @@ import (
 func TestClassifyProvisionerError(t *testing.T) {
 	t.Parallel()

-	tests := []struct {
-		name     string
-		jobError string
-		logs     []string
-		contains string
-		exact    bool
-	}{
-		{
-			name:     "DNSFailure",
-			jobError: "init failed",
-			logs:     []string{"Error: Failed to query available provider packages", "dial tcp: lookup registry.terraform.io: no such host"},
-			contains: "unreachable from your provisioner",
-		},
-		{
-			name:     "ConnectionRefused",
-			jobError: "terraform init: connection refused",
-			logs:     nil,
-			contains: "unreachable from your provisioner",
-		},
-		{
-			name:     "IOTimeout",
-			jobError: "context deadline exceeded",
-			logs:     []string{"dial tcp 1.2.3.4:443: i/o timeout"},
-			contains: "unreachable from your provisioner",
-		},
-		{
-			name:     "TLSTimeout",
-			jobError: "init error",
-			logs:     []string{"net/http: TLS handshake timeout"},
-			contains: "unreachable from your provisioner",
-		},
-		{
-			name:     "UnknownError",
-			jobError: "Error: Unsupported block type",
-			logs:     []string{"on main.tf line 5"},
-			contains: "Unsupported block type",
-			exact:    true,
-		},
-		{
-			name:     "EmptyErrorPassthrough",
-			jobError: "",
-			logs:     nil,
-			contains: "",
-			exact:    true,
-		},
-	}
+	t.Run("NetworkErrors", func(t *testing.T) {
+		t.Parallel()

-	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
-			t.Parallel()
-			result := templatebuilder.ClassifyProvisionerError(tc.jobError, tc.logs)
-			if tc.exact {
-				require.Equal(t, tc.jobError, result)
-			} else {
-				require.Contains(t, result, tc.contains)
-			}
-		})
-	}
+		tests := []struct {
+			name     string
+			jobError string
+			logs     []string
+		}{
+			{
+				name:     "DNSFailure",
+				jobError: "init failed",
+				logs:     []string{"Error: Failed to query available provider packages", "dial tcp: lookup registry.terraform.io: no such host"},
+			},
+			{
+				name:     "ConnectionRefused",
+				jobError: "terraform init: connection refused",
+				logs:     nil,
+			},
+			{
+				name:     "IOTimeout",
+				jobError: "context deadline exceeded",
+				logs:     []string{"dial tcp 1.2.3.4:443: i/o timeout"},
+			},
+			{
+				name:     "TLSTimeout",
+				jobError: "init error",
+				logs:     []string{"net/http: TLS handshake timeout"},
+			},
+		}
+
+		for _, tc := range tests {
+			t.Run(tc.name, func(t *testing.T) {
+				t.Parallel()
+				result := templatebuilder.ClassifyProvisionerError(tc.jobError, tc.logs)
+				require.Contains(t, result, "unreachable from your provisioner")
+			})
+		}
+	})
+
+	t.Run("NetworkTakesPrecedenceOverAuth", func(t *testing.T) {
+		t.Parallel()
+		// When both network and auth patterns appear, network wins
+		// (checked first).
+		result := templatebuilder.ClassifyProvisionerError(
+			"connection refused",
+			[]string{"AccessDenied: check credentials"},
+		)
+		require.Contains(t, result, "unreachable from your provisioner")
+	})
+
+	t.Run("AuthErrors", func(t *testing.T) {
+		t.Parallel()
+
+		tests := []struct {
+			name     string
+			jobError string
+			logs     []string
+		}{
+			{
+				name:     "AWSNoCredentials",
+				jobError: "terraform plan: exit status 1",
+				logs: []string{
+					"Initializing the backend...",
+					"Initializing provider plugins...",
+					"Error: No valid credential sources found",
+					"",
+					"  on main.tf line 10, in provider \"aws\":",
+					"  10:   region = \"us-east-1\"",
+				},
+			},
+			{
+				name:     "GCPDefaultCredentials",
+				jobError: "terraform plan: exit status 1",
+				logs: []string{
+					"Error: could not find default credentials",
+				},
+			},
+			{
+				name:     "AzureAuthFailure",
+				jobError: "terraform plan: exit status 1",
+				logs:     []string{"Error: AuthorizationFailed for subscription"},
+			},
+			{
+				name:     "CaseInsensitive",
+				jobError: "terraform plan: exit status 1",
+				logs:     []string{"Error: ACCESSDENIED on resource"},
+			},
+		}
+
+		for _, tc := range tests {
+			t.Run(tc.name, func(t *testing.T) {
+				t.Parallel()
+				result := templatebuilder.ClassifyProvisionerError(tc.jobError, tc.logs)
+				require.Contains(t, result, "Cloud provider authentication failed")
+			})
+		}
+	})
+
+	t.Run("AuthErrorNoLogsNoDanglingNewlines", func(t *testing.T) {
+		t.Parallel()
+		// Auth pattern in jobError with nil logs should not produce
+		// trailing newlines.
+		result := templatebuilder.ClassifyProvisionerError(
+			"AccessDenied: you are not allowed",
+			nil,
+		)
+		require.Contains(t, result, "Cloud provider authentication failed")
+		require.False(t, strings.HasSuffix(result, "\n"), "should not end with newline")
+	})
+
+	t.Run("UnrecognizedErrorIncludesLogs", func(t *testing.T) {
+		t.Parallel()
+		result := templatebuilder.ClassifyProvisionerError(
+			"terraform plan: exit status 1",
+			[]string{
+				"Initializing the backend...",
+				"Initializing provider plugins...",
+				"",
+				"Error: Unsupported block type",
+				"",
+				"  on main.tf line 5, in resource \"null_resource\" \"test\":",
+				"  5:   invalid_block {",
+			},
+		)
+		require.Contains(t, result, "terraform plan: exit status 1")
+		require.Contains(t, result, "Unsupported block type")
+		require.Contains(t, result, "on main.tf line 5")
+	})
+
+	t.Run("UnrecognizedErrorNoLogs", func(t *testing.T) {
+		t.Parallel()
+		result := templatebuilder.ClassifyProvisionerError(
+			"terraform plan: exit status 1",
+			nil,
+		)
+		require.Equal(t, "terraform plan: exit status 1", result)
+	})
+
+	t.Run("EmptyErrorPassthrough", func(t *testing.T) {
+		t.Parallel()
+		result := templatebuilder.ClassifyProvisionerError("", nil)
+		require.Equal(t, "", result)
+	})
+
+	t.Run("MultipleDiagnosticBlocks", func(t *testing.T) {
+		t.Parallel()
+		result := templatebuilder.ClassifyProvisionerError(
+			"terraform plan: exit status 1",
+			[]string{
+				"Warning: Deprecated resource",
+				"  Use the new resource instead.",
+				"",
+				"Error: Invalid reference",
+				"  on main.tf line 12:",
+				"  12:   value = var.undefined",
+			},
+		)
+		require.Contains(t, result, "Deprecated resource")
+		require.Contains(t, result, "Invalid reference")
+		require.Contains(t, result, "on main.tf line 12")
+	})
+
+	t.Run("LogContextCappedAtMax", func(t *testing.T) {
+		t.Parallel()
+		var logs []string
+		for i := 0; i < 50; i++ {
+			logs = append(logs, "Error: problem number "+strings.Repeat("x", 10))
+		}
+		result := templatebuilder.ClassifyProvisionerError("exit status 1", logs)
+		parts := strings.SplitN(result, "\n\n", 2)
+		require.Len(t, parts, 2, "should have jobError and log context")
+		contextLines := strings.Split(strings.TrimSpace(parts[1]), "\n")
+		// 20 diagnostic lines + 1 truncation marker.
+		require.Equal(t, 21, len(contextLines))
+		require.Equal(t, "... (truncated)", contextLines[20])
+	})
+
+	t.Run("NonDiagnosticLogsFiltered", func(t *testing.T) {
+		t.Parallel()
+		result := templatebuilder.ClassifyProvisionerError(
+			"terraform plan: exit status 1",
+			[]string{
+				"Initializing the backend...",
+				"Initializing provider plugins...",
+				"- Finding latest version of hashicorp/aws...",
+				"- Installing hashicorp/aws v5.0.0...",
+			},
+		)
+		require.Equal(t, "terraform plan: exit status 1", result)
+	})
 }