Merge pull request #111 from suzuki-shunsuke/feat/unify-template

feat: add HasDestry variable and remove the template for destroy
suzuki-shunsuke · Jul 17, 2021 · b7dca74 · b7dca74
2 parents 3305a3e + fe2d1af
commit b7dca74
Show file tree

Hide file tree

Showing 11 changed files with 70 additions and 253 deletions.
diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md
@@ -26,6 +26,7 @@ Placeholder | Usage
 `{{ .Stderr }}` | The standard error output of terraform command
 `{{ .CombinedOutput }}` | The output of terraform command
 `{{ .ExitCode }}` | The exit code of terraform command
+`{{ .HasDestroy }}` | Whether there are destroyed resources
 `{{ .ErrorMessages }}` | a list of error messages which occur in tfcmt
 `{{ .CreatedResources }}` | a list of created resource paths. This variable can be used at only plan
 `{{ .UpdatedResources }}` | a list of updated resource paths. This variable can be used at only plan
@@ -90,6 +91,7 @@ terraform:
 
       {{if .Link}}[CI link]({{.Link}}){{end}}
 
+      {{if .HasDestroy}}{{template "deletion_warning" .}}{{end}}
       {{template "result" .}}
       {{template "updated_resources" .}}
       <details><summary>Details (Click me)</summary>
@@ -106,18 +108,6 @@ terraform:
     when_destroy:
       label: "{{if .Vars.target}}{{.Vars.target}}/{{end}}destroy"
       label_color: d93f0b # red
-      template: |
-        {{template "plan_title" .}}
-
-        {{if .Link}}[CI link]({{.Link}}){{end}}
-
-        {{template "deletion_warning" .}}
-        {{template "result" .}}
-
-        {{template "updated_resources" .}}
-        <details><summary>Details (Click me)</summary>
-        {{wrapCode .CombinedOutput}}
-        </details>
     when_no_changes:
       label: "{{if .Vars.target}}{{.Vars.target}}/{{end}}no-changes"
       label_color: 0e8a16 # green
@@ -156,8 +146,6 @@ terraform:
         </details>
 ```
 
-If the plan contains resource deletion, the template of `when_destroy` is used.
-
 If you don't want to update labels, please set `terraform.plan.disable_label: true`.
 
 ```yaml

diff --git a/example-with-destroy-and-result-labels.tfcmt.yaml b/example-with-destroy-and-result-labels.tfcmt.yaml
@@ -1,6 +1,11 @@
 terraform:
   plan:
     template: |
+      {{if .HasDestroy}}
+      ## :warning: WARNING: Resource Deletion will happen :warning:
+
+      This plan contains **resource deletion**. Please check the plan result very carefully!
+      {{else}}
       ## Plan Result
       {{if .Result}}
       <pre><code>{{ .Result }}
@@ -10,14 +15,11 @@ terraform:
 
       <pre><code>{{ .CombinedOutput }}
       </pre></code></details>
+      {{end}}
     when_add_or_update_only:
       label: "add-or-update"
     when_destroy:
       label: "destroy"
-      template: |
-        ## :warning: WARNING: Resource Deletion will happen :warning:
-
-        This plan contains **resource deletion**. Please check the plan result very carefully!
     when_no_changes:
       label: "no-changes"
     when_plan_error:

diff --git a/pkg/cli/plan.go b/pkg/cli/plan.go
@@ -24,11 +24,10 @@ func cmdPlan(ctx *cli.Context) error {
 	}
 
 	t := &controller.Controller{
-		Config:                 cfg,
-		Parser:                 terraform.NewPlanParser(),
-		Template:               terraform.NewPlanTemplate(cfg.Terraform.Plan.Template),
-		DestroyWarningTemplate: terraform.NewDestroyWarningTemplate(cfg.Terraform.Plan.WhenDestroy.Template),
-		ParseErrorTemplate:     terraform.NewPlanParseErrorTemplate(cfg.Terraform.Plan.WhenParseError.Template),
+		Config:             cfg,
+		Parser:             terraform.NewPlanParser(),
+		Template:           terraform.NewPlanTemplate(cfg.Terraform.Plan.Template),
+		ParseErrorTemplate: terraform.NewPlanParseErrorTemplate(cfg.Terraform.Plan.WhenParseError.Template),
 	}
 	args := ctx.Args()
 

diff --git a/pkg/config/config.go b/pkg/config/config.go
@@ -62,9 +62,8 @@ type WhenAddOrUpdateOnly struct {
 
 // WhenDestroy is a configuration to notify the plan result contains destroy operation
 type WhenDestroy struct {
-	Label    string
-	Template string
-	Color    string `yaml:"label_color"`
+	Label string
+	Color string `yaml:"label_color"`
 }
 
 // WhenNoChanges is a configuration to add a label when the plan result contains no change

diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go
@@ -3,9 +3,9 @@ package config
 import (
 	"os"
 	"path/filepath"
-	"reflect"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
 	"github.com/suzuki-shunsuke/tfcmt/pkg/domain"
 )
 
@@ -53,13 +53,27 @@ func TestLoadFile(t *testing.T) {
 			cfg: Config{
 				Terraform: Terraform{
 					Plan: Plan{
-						Template: "## Plan Result\n{{if .Result}}\n<pre><code>{{ .Result }}\n</pre></code>\n{{end}}\n<details><summary>Details (Click me)</summary>\n\n<pre><code>{{ .CombinedOutput }}\n</pre></code></details>\n",
+						Template: `{{if .HasDestroy}}
+## :warning: WARNING: Resource Deletion will happen :warning:
+
+This plan contains **resource deletion**. Please check the plan result very carefully!
+{{else}}
+## Plan Result
+{{if .Result}}
+<pre><code>{{ .Result }}
+</pre></code>
+{{end}}
+<details><summary>Details (Click me)</summary>
+
+<pre><code>{{ .CombinedOutput }}
+</pre></code></details>
+{{end}}
+`,
 						WhenAddOrUpdateOnly: WhenAddOrUpdateOnly{
 							Label: "add-or-update",
 						},
 						WhenDestroy: WhenDestroy{
-							Label:    "destroy",
-							Template: "## :warning: WARNING: Resource Deletion will happen :warning:\n\nThis plan contains **resource deletion**. Please check the plan result very carefully!\n",
+							Label: "destroy",
 						},
 						WhenPlanError: WhenPlanError{
 							Label: "error",
@@ -102,8 +116,8 @@ func TestLoadFile(t *testing.T) {
 			if err := cfg.LoadFile(testCase.file); err == nil {
 				if !testCase.ok {
 					t.Error("got no error but want error")
-				} else if !reflect.DeepEqual(cfg, testCase.cfg) {
-					t.Errorf("got %#v but want: %#v", cfg, testCase.cfg)
+				} else if diff := cmp.Diff(cfg, testCase.cfg); diff != "" {
+					t.Errorf(diff)
 				}
 			} else {
 				if testCase.ok {

diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go
@@ -21,11 +21,10 @@ import (
 )
 
 type Controller struct {
-	Config                 config.Config
-	Parser                 terraform.Parser
-	Template               *terraform.Template
-	DestroyWarningTemplate *terraform.Template
-	ParseErrorTemplate     *terraform.Template
+	Config             config.Config
+	Parser             terraform.Parser
+	Template           *terraform.Template
+	ParseErrorTemplate *terraform.Template
 }
 
 type Command struct {
@@ -179,15 +178,14 @@ func (ctrl *Controller) getNotifier(ctx context.Context) (notifier.Notifier, err
 			Revision: ctrl.Config.CI.SHA,
 			Number:   ctrl.Config.CI.PRNumber,
 		},
-		CI:                     ctrl.Config.CI.Link,
-		Parser:                 ctrl.Parser,
-		UseRawOutput:           ctrl.Config.Terraform.UseRawOutput,
-		Template:               ctrl.Template,
-		DestroyWarningTemplate: ctrl.DestroyWarningTemplate,
-		ParseErrorTemplate:     ctrl.ParseErrorTemplate,
-		ResultLabels:           labels,
-		Vars:                   ctrl.Config.Vars,
-		Templates:              ctrl.Config.Templates,
+		CI:                 ctrl.Config.CI.Link,
+		Parser:             ctrl.Parser,
+		UseRawOutput:       ctrl.Config.Terraform.UseRawOutput,
+		Template:           ctrl.Template,
+		ParseErrorTemplate: ctrl.ParseErrorTemplate,
+		ResultLabels:       labels,
+		Vars:               ctrl.Config.Vars,
+		Templates:          ctrl.Config.Templates,
 	})
 	if err != nil {
 		return nil, err

diff --git a/pkg/notifier/github/client.go b/pkg/notifier/github/client.go
@@ -46,11 +46,8 @@ type Config struct {
 	CI      string
 	Parser  terraform.Parser
 	// Template is used for all Terraform command output
-	Template *terraform.Template
-	// DestroyWarningTemplate is used only for additional warning
-	// the plan result contains destroy operation
-	DestroyWarningTemplate *terraform.Template
-	ParseErrorTemplate     *terraform.Template
+	Template           *terraform.Template
+	ParseErrorTemplate *terraform.Template
 	// ResultLabels is a set of labels to apply depending on the plan result
 	ResultLabels ResultLabels
 	Vars         map[string]string

diff --git a/pkg/notifier/github/notify.go b/pkg/notifier/github/notify.go
@@ -37,9 +37,6 @@ func (g *NotifyService) Notify(ctx context.Context, param notifier.ParamExec) (i
 
 	_, isPlan := parser.(*terraform.PlanParser)
 	if isPlan {
-		if result.HasDestroy {
-			template = g.client.Config.DestroyWarningTemplate
-		}
 		if cfg.PR.IsNumber() && cfg.ResultLabels.HasAnyLabelDefined() {
 			errMsgs = append(errMsgs, g.updateLabels(ctx, result)...)
 		}
@@ -50,6 +47,7 @@ func (g *NotifyService) Notify(ctx context.Context, param notifier.ParamExec) (i
 		ChangedResult:          result.ChangedResult,
 		ChangeOutsideTerraform: result.OutsideTerraform,
 		Warning:                result.Warning,
+		HasDestroy:             result.HasDestroy,
 		Link:                   cfg.CI,
 		UseRawOutput:           cfg.UseRawOutput,
 		Vars:                   cfg.Vars,

diff --git a/pkg/notifier/github/notify_test.go b/pkg/notifier/github/notify_test.go
@@ -139,10 +139,9 @@ func TestNotifyNotify(t *testing.T) {
 					Revision: "",
 					Number:   1,
 				},
-				Parser:                 terraform.NewPlanParser(),
-				Template:               terraform.NewPlanTemplate(terraform.DefaultPlanTemplate),
-				DestroyWarningTemplate: terraform.NewDestroyWarningTemplate(terraform.DefaultDestroyWarningTemplate),
-				ParseErrorTemplate:     terraform.NewPlanParseErrorTemplate(terraform.DefaultPlanTemplate),
+				Parser:             terraform.NewPlanParser(),
+				Template:           terraform.NewPlanTemplate(terraform.DefaultPlanTemplate),
+				ParseErrorTemplate: terraform.NewPlanParseErrorTemplate(terraform.DefaultPlanTemplate),
 			},
 			paramExec: notifier.ParamExec{
 				Stdout:   "Plan: 1 to add, 1 to destroy",
@@ -191,10 +190,9 @@ func TestNotifyNotify(t *testing.T) {
 					Revision: "",
 					Number:   1,
 				},
-				Parser:                 terraform.NewPlanParser(),
-				Template:               terraform.NewPlanTemplate(terraform.DefaultPlanTemplate),
-				DestroyWarningTemplate: terraform.NewDestroyWarningTemplate(terraform.DefaultDestroyWarningTemplate),
-				ParseErrorTemplate:     terraform.NewPlanParseErrorTemplate(terraform.DefaultPlanTemplate),
+				Parser:             terraform.NewPlanParser(),
+				Template:           terraform.NewPlanTemplate(terraform.DefaultPlanTemplate),
+				ParseErrorTemplate: terraform.NewPlanParseErrorTemplate(terraform.DefaultPlanTemplate),
 			},
 			paramExec: notifier.ParamExec{
 				Stdout:   "Plan: 1 to add, 1 to destroy",

diff --git a/pkg/terraform/template.go b/pkg/terraform/template.go
@@ -16,6 +16,7 @@ const (
 
 {{if .Link}}[CI link]({{.Link}}){{end}}
 
+{{if .HasDestroy}}{{template "deletion_warning" .}}{{end}}
 {{template "result" .}}
 {{template "updated_resources" .}}
 <details><summary>Details (Click me)</summary>
@@ -44,21 +45,6 @@ const (
 * {{. -}}
 {{- end}}{{end}}`
 
-	// DefaultDestroyWarningTemplate is a default template for terraform plan
-	DefaultDestroyWarningTemplate = `
-{{template "plan_title" .}}
-
-{{if .Link}}[CI link]({{.Link}}){{end}}
-
-{{template "deletion_warning" .}}
-{{template "result" .}}
-
-{{template "updated_resources" .}}
-<details><summary>Details (Click me)</summary>
-{{wrapCode .CombinedOutput}}
-</details>
-`
-
 	// DefaultPlanParseErrorTemplate is a default template for terraform plan parse error
 	DefaultPlanParseErrorTemplate = `
 {{template "plan_title" .}}
@@ -94,6 +80,7 @@ type CommonTemplate struct {
 	Warning                string
 	Link                   string
 	UseRawOutput           bool
+	HasDestroy             bool
 	Vars                   map[string]string
 	Templates              map[string]string
 	Stdout                 string
@@ -123,16 +110,6 @@ func NewPlanTemplate(template string) *Template {
 	}
 }
 
-// NewDestroyWarningTemplate is DestroyWarningTemplate initializer
-func NewDestroyWarningTemplate(template string) *Template {
-	if template == "" {
-		template = DefaultDestroyWarningTemplate
-	}
-	return &Template{
-		Template: template,
-	}
-}
-
 // NewApplyTemplate is ApplyTemplate initializer
 func NewApplyTemplate(template string) *Template {
 	if template == "" {
@@ -220,6 +197,7 @@ func (t *Template) Execute() (string, error) {
 		"UpdatedResources":       t.UpdatedResources,
 		"DeletedResources":       t.DeletedResources,
 		"ReplacedResources":      t.ReplacedResources,
+		"HasDestroy":             t.HasDestroy,
 	}
 
 	templates := map[string]string{