fix: parse error when only outputs is changed (#896)

* fix: parse error when only outputs is changed * If U+2500 is used as a rule, `ChangedResult` cannot be extracted correctly. * fix: `Change Result` is not output when only outputs is changed * chore: improve plan results message when only Outputs will be changed
suzuki-shunsuke · Sep 9, 2023 · c4a74db · c4a74db
1 parent 7cb1a89
commit c4a74db
Show file tree

Hide file tree

Showing 2 changed files with 144 additions and 6 deletions.
diff --git a/pkg/terraform/parser.go b/pkg/terraform/parser.go
@@ -64,7 +64,7 @@ func NewDefaultParser() *DefaultParser {
 // NewPlanParser is PlanParser initialized with its Regexp
 func NewPlanParser() *PlanParser {
 	return &PlanParser{
-		Pass: regexp.MustCompile(`(?m)^(Plan: \d|No changes.)`),
+		Pass: regexp.MustCompile(`(?m)^(Plan: \d|No changes.|Changes to Outputs:)`),
 		Fail: regexp.MustCompile(`(?m)^(Error: )`),
 		// "0 to destroy" should be treated as "no destroy"
 		HasDestroy:    regexp.MustCompile(`(?m)([1-9][0-9]* to destroy.)`),
@@ -150,14 +150,22 @@ func (p *PlanParser) Parse(body string) ParseResult { //nolint:cyclop
 		if line == "Terraform will perform the following actions:" { // https://github.com/hashicorp/terraform/blob/332045a4e4b1d256c45f98aac74e31102ace7af7/internal/command/views/plan.go#L252
 			startChangeOutput = i + 1
 		}
-		if startChangeOutput != -1 && endChangeOutput == -1 && strings.HasPrefix(line, "Plan: ") { // https://github.com/hashicorp/terraform/blob/dfc12a6a9e1cff323829026d51873c1b80200757/internal/command/views/plan.go#L306
-			endChangeOutput = i + 1
+		// If we have output changes but not resource changes, Terraform
+		// does not output `Terraform will perform the following actions:`.
+		if line == "Changes to Outputs:" && startChangeOutput == -1 {
+			startChangeOutput = i
 		}
 		if strings.HasPrefix(line, "Warning:") && startWarning == -1 {
 			startWarning = i
 		}
-		if strings.HasPrefix(line, "─────") && startWarning != -1 && endWarning == -1 {
-			endWarning = i
+		// Terraform uses two types of rules.
+		if strings.HasPrefix(line, "─────") || strings.HasPrefix(line, "-----") {
+			if startWarning != -1 && endWarning == -1 {
+				endWarning = i
+			}
+			if startChangeOutput != -1 && endChangeOutput == -1 {
+				endChangeOutput = i - 1
+			}
 		}
 		if firstMatchLineIndex == -1 {
 			if p.Pass.MatchString(line) || p.Fail.MatchString(line) {
@@ -201,6 +209,10 @@ func (p *PlanParser) Parse(body string) ParseResult { //nolint:cyclop
 
 	changeResult := ""
 	if startChangeOutput != -1 {
+		// if we get here before finding a horizontal rule, output all remaining.
+		if endChangeOutput == -1 {
+			endChangeOutput = len(lines) - 1
+		}
 		changeResult = strings.Join(lines[startChangeOutput:endChangeOutput], "\n")
 	}
 
@@ -214,7 +226,7 @@ func (p *PlanParser) Parse(body string) ParseResult { //nolint:cyclop
 	}
 
 	return ParseResult{
-		Result:             result,
+		Result:             refinePlanResult(result),
 		ChangedResult:      changeResult,
 		OutsideTerraform:   outsideTerraform,
 		Warning:            warnings,
@@ -233,6 +245,15 @@ func (p *PlanParser) Parse(body string) ParseResult { //nolint:cyclop
 	}
 }
 
+// It can be difficult to understand if we just cut out a part of
+// Terraform's output, so rewrite the text in a way that users can understand.
+func refinePlanResult(s string) string {
+	if s == "Changes to Outputs:" {
+		return "Only Outputs will be changed."
+	}
+	return s
+}
+
 type MovedResource struct {
 	Before string
 	After  string

diff --git a/pkg/terraform/parser_test.go b/pkg/terraform/parser_test.go
@@ -52,6 +52,69 @@ can't guarantee that exactly these actions will be performed if
 "terraform apply" is subsequently run.
 `
 
+const planOnlyOutputChangesSuccessResult0_12 = `
+Refreshing Terraform state in-memory prior to plan...
+The refreshed state will be used to calculate this plan, but will not be
+persisted to local or remote state storage.
+
+data.terraform_remote_state.teams_platform_development: Refreshing state...
+google_project.my_project: Refreshing state...
+aws_iam_policy.datadog_aws_integration: Refreshing state...
+aws_iam_user.teams_terraform: Refreshing state...
+aws_iam_role.datadog_aws_integration: Refreshing state...
+google_project_services.my_project: Refreshing state...
+google_bigquery_dataset.gateway_access_log: Refreshing state...
+aws_iam_role_policy_attachment.datadog_aws_integration: Refreshing state...
+google_logging_project_sink.gateway_access_log_bigquery_sink: Refreshing state...
+google_project_iam_member.gateway_access_log_bigquery_sink_writer_is_bigquery_data_editor: Refreshing state...
+google_dns_managed_zone.tfnotifyapps_com: Refreshing state...
+google_dns_record_set.dev_tfnotifyapps_com: Refreshing state...
+
+------------------------------------------------------------------------
+
+An execution plan has been generated and is shown below.
+Resource actions are indicated with the following symbols:
+  + create
+
+Terraform will perform the following actions:
+
+Plan: 0 to add, 0 to change, 0 to destroy.
+
+Changes to Outputs:
+  + aws_instance_name = "my-instance"
+
+------------------------------------------------------------------------
+
+Note: You didn't specify an "-out" parameter to save this plan, so Terraform
+can't guarantee that exactly these actions will be performed if
+"terraform apply" is subsequently run.
+`
+
+const planOnlyOutputChangesSuccessResult0_15 = `
+null_resource.this: Refreshing state... [id=6068603774747257119]
+
+Changes to Outputs:
+  + test = 42
+
+You can apply this plan to save these new output values to the Terraform
+state, without changing any real infrastructure.
+
+─────────────────────────────────────────────────────────────────────────────
+
+Note: You didn't use the -out option to save this plan, so Terraform can't
+guarantee to take exactly these actions if you run "terraform apply" now.
+`
+
+const planOnlyOutputChangesSuccessInAutomationResult = `
+null_resource.this: Refreshing state... [id=6068603774747257119]
+
+Changes to Outputs:
+  + test = 42
+
+You can apply this plan to save these new output values to the Terraform
+state, without changing any real infrastructure.
+`
+
 const planFailureResult = `
 xxxxxxxxx
 xxxxxxxxx
@@ -351,6 +414,60 @@ func TestPlanParserParse(t *testing.T) {
 Plan: 1 to add, 0 to change, 0 to destroy.`,
 			},
 		},
+		{
+			name: "plan output changes only pattern 0.12",
+			body: planOnlyOutputChangesSuccessResult0_12,
+			result: ParseResult{
+				Result:             "Plan: 0 to add, 0 to change, 0 to destroy.",
+				HasAddOrUpdateOnly: true,
+				HasDestroy:         false,
+				HasNoChanges:       false,
+				HasPlanError:       false,
+				ExitCode:           0,
+				Error:              nil,
+				ChangedResult: `
+Plan: 0 to add, 0 to change, 0 to destroy.
+
+Changes to Outputs:
+  + aws_instance_name = "my-instance"`,
+			},
+		},
+		{
+			name: "plan output changes only pattern 0.15",
+			body: planOnlyOutputChangesSuccessResult0_15,
+			result: ParseResult{
+				Result:             "Only Outputs will be changed.",
+				HasAddOrUpdateOnly: true,
+				HasDestroy:         false,
+				HasNoChanges:       false,
+				HasPlanError:       false,
+				ExitCode:           0,
+				Error:              nil,
+				ChangedResult: `Changes to Outputs:
+  + test = 42
+
+You can apply this plan to save these new output values to the Terraform
+state, without changing any real infrastructure.`,
+			},
+		},
+		{
+			name: "plan output changes only pattern with TF_IN_AUTOMATION",
+			body: planOnlyOutputChangesSuccessInAutomationResult,
+			result: ParseResult{
+				Result:             "Only Outputs will be changed.",
+				HasAddOrUpdateOnly: true,
+				HasDestroy:         false,
+				HasNoChanges:       false,
+				HasPlanError:       false,
+				ExitCode:           0,
+				Error:              nil,
+				ChangedResult: `Changes to Outputs:
+  + test = 42
+
+You can apply this plan to save these new output values to the Terraform
+state, without changing any real infrastructure.`,
+			},
+		},
 		{
 			name: "no stdin",
 			body: "",