From 2bcb4370f1b14f9ef993c14569e33600ea3fba0d Mon Sep 17 00:00:00 2001 From: michaelact <86778470+michaelact@users.noreply.github.com> Date: Fri, 7 Jun 2024 14:03:34 +0700 Subject: [PATCH 1/7] feat(smtp): subject templating instead of hard-coded --- smtp/main.go | 32 +++++++++++++++++++++++++------- smtp/main_test.go | 44 ++++++++++++++++++++++++++++++++------------ 2 files changed, 57 insertions(+), 19 deletions(-) diff --git a/smtp/main.go b/smtp/main.go index dd49c87f..510bd419 100644 --- a/smtp/main.go +++ b/smtp/main.go @@ -18,7 +18,8 @@ import ( "bytes" "context" "fmt" - "html/template" + htmlTemplate "html/template" + textTemplate "text/template" "mime/quotedprintable" "net/smtp" "strings" @@ -41,15 +42,16 @@ func main() { type smtpNotifier struct { filter notifiers.EventFilter - tmpl *template.Template + htmlTmpl *htmlTemplate.Template + textTmpl *textTemplate.Template mcfg mailConfig br notifiers.BindingResolver tmplView *notifiers.TemplateView } type mailConfig struct { - server, port, sender, from, password string - recipients []string + server, port, sender, from, password, subject string + recipients []string } func (s *smtpNotifier) SetUp(ctx context.Context, cfg *notifiers.Config, cfgTemplate string, sg notifiers.SecretGetter, br notifiers.BindingResolver) error { @@ -58,11 +60,19 @@ func (s *smtpNotifier) SetUp(ctx context.Context, cfg *notifiers.Config, cfgTemp return fmt.Errorf("failed to create CELPredicate: %w", err) } s.filter = prd - tmpl, err := template.New("email_template").Parse(cfgTemplate) + htmlTmpl, err := htmlTemplate.New("email_template").Parse(cfgTemplate) if err != nil { return fmt.Errorf("failed to parse HTML email template: %w", err) } - s.tmpl = tmpl + s.htmlTmpl = htmlTmpl + + if _, subjectFound := cfg.Spec.Notification.Delivery["Subject"]; subjectFound { + textTmpl, err := textTemplate.New("subject_template").Parse(cfg.Spec.Notification.Delivery["Subject"].(string)) + if err != nil { + return fmt.Errorf("failed to parse TEXT subject template: %w", err) + } + s.textTmpl = textTmpl + } mcfg, err := getMailConfig(ctx, sg, cfg.Spec) if err != nil { @@ -175,11 +185,19 @@ func (s *smtpNotifier) buildEmail() (string, error) { build.LogUrl = logURL body := new(bytes.Buffer) - if err := s.tmpl.Execute(body, s.tmplView); err != nil { + if err := s.htmlTmpl.Execute(body, s.tmplView); err != nil { return "", err } subject := fmt.Sprintf("Cloud Build [%s]: %s", build.ProjectId, build.Id) + if s.textTmpl != nil { + subjectTmpl := new(bytes.Buffer) + if err := s.textTmpl.Execute(subjectTmpl, s.tmplView); err != nil { + return "", err + } + + subject = subjectTmpl.String() + } header := make(map[string]string) if s.mcfg.from != s.mcfg.sender { diff --git a/smtp/main_test.go b/smtp/main_test.go index 229349b2..a58a9e97 100644 --- a/smtp/main_test.go +++ b/smtp/main_test.go @@ -47,16 +47,16 @@ const htmlBody = `
- + - - + + - - + + - +
Status{{.Params.buildStatus}}Status{{.Params.buildStatus}}
Log URLClick HereLog URLClick Here
@@ -66,6 +66,8 @@ const htmlBody = ` ` +const templateSubject = `Build {{.Build.Id}} Status: {{.Build.Status}}` + type fakeSecretGetter struct{} func (f *fakeSecretGetter) GetSecret(_ context.Context, _ string) (string, error) { @@ -147,7 +149,7 @@ metadata: name: failed-build-email-notification spec: notification: - filter: event.buildTriggerStatus == “STATUS_FAILED” + filter: event.buildTriggerStatus == "STATUS_FAILED" delivery: server: smtp.example.com port: '587' @@ -190,10 +192,6 @@ spec: } func TestDefaultEmailTemplate(t *testing.T) { - tmpl, err := template.New("email_template").Parse(htmlBody) - if err != nil { - t.Fatalf("template.Parse failed: %v", err) - } build := &cbpb.Build{ Id: "some-build-id", ProjectId: "my-project-id", @@ -209,8 +207,30 @@ func TestDefaultEmailTemplate(t *testing.T) { Params: map[string]string{"buildStatus": "SUCCESS"}, } + // Subject email template + subjectTmpl, err := template.New("subject_template").Parse(templateSubject) + if err != nil { + t.Fatalf("failed to parse subject template: %v", err) + } + + subject := new(bytes.Buffer) + if err := subjectTmpl.Execute(subject, view); err != nil { + t.Fatalf("failed to execute subject template: %v", err) + } + + expectedSubject := "Build some-build-id Status: SUCCESS" + if subject.String() != expectedSubject { + t.Errorf("expected subject %q, but got %q", expectedSubject, subject.String()) + } + + // Body email template + bodyTmpl, err := template.New("email_template").Parse(htmlBody) + if err != nil { + t.Fatalf("template.Parse failed: %v", err) + } + body := new(bytes.Buffer) - if err := tmpl.Execute(body, view); err != nil { + if err := bodyTmpl.Execute(body, view); err != nil { t.Fatalf("failed to execute template: %v", err) } From e260e5fee36a25dbb360b15e99b0bd51c5afada5 Mon Sep 17 00:00:00 2001 From: michaelact <86778470+michaelact@users.noreply.github.com> Date: Fri, 7 Jun 2024 15:43:00 +0700 Subject: [PATCH 2/7] fix: subject yaml key name --- smtp/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/smtp/main.go b/smtp/main.go index 510bd419..7c20dee4 100644 --- a/smtp/main.go +++ b/smtp/main.go @@ -66,7 +66,7 @@ func (s *smtpNotifier) SetUp(ctx context.Context, cfg *notifiers.Config, cfgTemp } s.htmlTmpl = htmlTmpl - if _, subjectFound := cfg.Spec.Notification.Delivery["Subject"]; subjectFound { + if _, subjectFound := cfg.Spec.Notification.Delivery["subject"]; subjectFound { textTmpl, err := textTemplate.New("subject_template").Parse(cfg.Spec.Notification.Delivery["Subject"].(string)) if err != nil { return fmt.Errorf("failed to parse TEXT subject template: %w", err) From 3ccad2704bf42d42749de955f05347ada10a2759 Mon Sep 17 00:00:00 2001 From: michaelact <86778470+michaelact@users.noreply.github.com> Date: Fri, 7 Jun 2024 15:49:39 +0700 Subject: [PATCH 3/7] fix: get subject key --- smtp/main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/smtp/main.go b/smtp/main.go index 7c20dee4..f51ec34a 100644 --- a/smtp/main.go +++ b/smtp/main.go @@ -66,8 +66,8 @@ func (s *smtpNotifier) SetUp(ctx context.Context, cfg *notifiers.Config, cfgTemp } s.htmlTmpl = htmlTmpl - if _, subjectFound := cfg.Spec.Notification.Delivery["subject"]; subjectFound { - textTmpl, err := textTemplate.New("subject_template").Parse(cfg.Spec.Notification.Delivery["Subject"].(string)) + if subject, subjectFound := cfg.Spec.Notification.Delivery["subject"]; subjectFound { + textTmpl, err := textTemplate.New("subject_template").Parse(subject.(string)) if err != nil { return fmt.Errorf("failed to parse TEXT subject template: %w", err) } From 0e8e1146cfee7aa61bbb2de63d7be9c24d813f7a Mon Sep 17 00:00:00 2001 From: michaelact <86778470+michaelact@users.noreply.github.com> Date: Fri, 7 Jun 2024 17:26:22 +0700 Subject: [PATCH 4/7] chore: add subject: key in example smtp.yaml --- smtp/smtp.yaml.example | 1 + 1 file changed, 1 insertion(+) diff --git a/smtp/smtp.yaml.example b/smtp/smtp.yaml.example index 05bc859b..c2041e2e 100644 --- a/smtp/smtp.yaml.example +++ b/smtp/smtp.yaml.example @@ -48,6 +48,7 @@ spec: delivery: server: smtp.gmail.com port: '587' + subject: '{{ if eq .Build.Status 3 }}✅ Build Successful ({{ .Build.Substitutions.REPO_NAME }}){{ else }}❌ Build Failure ({{ .Build.Substitutions.REPO_NAME }}){{ end }} | {{.Build.Substitutions._COMMIT_MESSAGE}}' sender: example-sender@gmail.com from: example-from@gmail.com recipients: From 39a811ee7d29c35d8d3095aee6505b38b030ec2b Mon Sep 17 00:00:00 2001 From: michaelact <86778470+michaelact@users.noreply.github.com> Date: Thu, 13 Jun 2024 00:49:52 +0700 Subject: [PATCH 5/7] chore: escape any string formatter like \n are removed --- smtp/main.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/smtp/main.go b/smtp/main.go index f51ec34a..568bc685 100644 --- a/smtp/main.go +++ b/smtp/main.go @@ -196,7 +196,8 @@ func (s *smtpNotifier) buildEmail() (string, error) { return "", err } - subject = subjectTmpl.String() + // Escape any string formatter + subject = strings.Join(strings.Fields(subjectTmpl.String()), " ") } header := make(map[string]string) From a217c6264186940042c3627c68dc5039608fe8ac Mon Sep 17 00:00:00 2001 From: michaelact <86778470+michaelact@users.noreply.github.com> Date: Wed, 24 Jul 2024 09:03:44 +0700 Subject: [PATCH 6/7] test: separate test between default and subject template --- smtp/main_test.go | 59 +++++++++++++++++++++++++++++------------------ 1 file changed, 37 insertions(+), 22 deletions(-) diff --git a/smtp/main_test.go b/smtp/main_test.go index a58a9e97..959e2223 100644 --- a/smtp/main_test.go +++ b/smtp/main_test.go @@ -192,6 +192,11 @@ spec: } func TestDefaultEmailTemplate(t *testing.T) { + tmpl, err := template.New("email_template").Parse(htmlBody) + if err != nil { + t.Fatalf("template.Parse failed: %v", err) + } + build := &cbpb.Build{ Id: "some-build-id", ProjectId: "my-project-id", @@ -207,42 +212,52 @@ func TestDefaultEmailTemplate(t *testing.T) { Params: map[string]string{"buildStatus": "SUCCESS"}, } - // Subject email template - subjectTmpl, err := template.New("subject_template").Parse(templateSubject) - if err != nil { - t.Fatalf("failed to parse subject template: %v", err) + body := new(bytes.Buffer) + if err := tmpl.Execute(body, view); err != nil { + t.Fatalf("failed to execute template: %v", err) } - subject := new(bytes.Buffer) - if err := subjectTmpl.Execute(subject, view); err != nil { - t.Fatalf("failed to execute subject template: %v", err) + if !strings.Contains(body.String(), `
my-project-id: some-trigger-id
`) { + t.Error("missing correct .card-title div") } - expectedSubject := "Build some-build-id Status: SUCCESS" - if subject.String() != expectedSubject { - t.Errorf("expected subject %q, but got %q", expectedSubject, subject.String()) + if !strings.Contains(body.String(), `SUCCESS`) { + t.Error("missing status") + } + + if !strings.Contains(body.String(), ``) { + t.Error("missing Log URL") } +} - // Body email template - bodyTmpl, err := template.New("email_template").Parse(htmlBody) +func TestSubjectEmailTemplate(t *testing.T) { + tmpl, err := template.New("subject_template").Parse(templateSubject) if err != nil { - t.Fatalf("template.Parse failed: %v", err) + t.Fatalf("failed to parse subject template: %v", err) } - body := new(bytes.Buffer) - if err := bodyTmpl.Execute(body, view); err != nil { - t.Fatalf("failed to execute template: %v", err) + build := &cbpb.Build{ + Id: "some-build-id", + ProjectId: "my-project-id", + BuildTriggerId: "some-trigger-id", + Status: cbpb.Build_SUCCESS, + LogUrl: "https://some.example.com/log/url", } - if !strings.Contains(body.String(), `
my-project-id: some-trigger-id
`) { - t.Error("missing correct .card-title div") + view := ¬ifiers.TemplateView{ + Build: ¬ifiers.BuildView{ + Build: build, + }, + Params: map[string]string{"buildStatus": "SUCCESS"}, } - if !strings.Contains(body.String(), `SUCCESS`) { - t.Error("missing status") + subject := new(bytes.Buffer) + if err := tmpl.Execute(subject, view); err != nil { + t.Fatalf("failed to execute subject template: %v", err) } - if !strings.Contains(body.String(), ``) { - t.Error("missing Log URL") + expectedSubject := "Build some-build-id Status: SUCCESS" + if subject.String() != expectedSubject { + t.Errorf("expected subject %q, but got %q", expectedSubject, subject.String()) } } From 89522fa41e32af6b23cd9ec1451df2b6c450cfc0 Mon Sep 17 00:00:00 2001 From: michaelact <86778470+michaelact@users.noreply.github.com> Date: Wed, 24 Jul 2024 09:06:00 +0700 Subject: [PATCH 7/7] fix: subject example --- smtp/smtp.yaml.example | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/smtp/smtp.yaml.example b/smtp/smtp.yaml.example index c2041e2e..ced70dd0 100644 --- a/smtp/smtp.yaml.example +++ b/smtp/smtp.yaml.example @@ -48,7 +48,7 @@ spec: delivery: server: smtp.gmail.com port: '587' - subject: '{{ if eq .Build.Status 3 }}✅ Build Successful ({{ .Build.Substitutions.REPO_NAME }}){{ else }}❌ Build Failure ({{ .Build.Substitutions.REPO_NAME }}){{ end }} | {{.Build.Substitutions._COMMIT_MESSAGE}}' + subject: '{{ if eq .Build.Status 3 }}✅ Build Successful ({{ .Build.Substitutions.REPO_NAME }}){{ else }}❌ Build Failure ({{ .Build.Substitutions.REPO_NAME }}){{ end }} | {{ if .Build.Substitutions._COMMIT_MESSAGE }} {{ if gt (len .Build.Substitutions._COMMIT_MESSAGE) 100 }}{{ slice .Build.Substitutions._COMMIT_MESSAGE 0 100 }}{{ else }}{{ .Build.Substitutions._COMMIT_MESSAGE }}{{ end }}{{ else }}{{ .Build.Substitutions.COMMIT_SHA }}{{ end }} [...]' sender: example-sender@gmail.com from: example-from@gmail.com recipients: