Split too big comment per cluster

Makefile

@@ -25,5 +25,5 @@ clean:
 
 .PHONY: test
 test: $(VENDOR_DIR)
- go test -v -timeout 30s ./...
+ TEMPLATES_PATH=../../../templates/ go test -v -timeout 30s ./...
 

internal/pkg/githubapi/github.go

@@ -30,6 +30,13 @@ import (
 
 const githubCommentMaxSize = 65536
 
+type DiffCommentData struct {
+ DiffOfChangedComponents []argocd.DiffResult
+ HasSyncableComponents bool
+ BranchName string
+ Header string
+}
+
 type promotionInstanceMetaData struct {
  SourcePath string `json:"sourcePath"`
  TargetPaths []string `json:"targetPaths"`
@@ -169,40 +176,29 @@ func HandlePREvent(eventPayload *github.PullRequestEvent, ghPrClientDetails GhPr
  }
 
  if len(diffOfChangedComponents) > 0 {
- diffCommentData := struct {
- DiffOfChangedComponents []argocd.DiffResult
- HasSyncableComponens bool
- BranchName string
- }{
+ diffCommentData := DiffCommentData{
  DiffOfChangedComponents: diffOfChangedComponents,
  BranchName: ghPrClientDetails.Ref,
  }
 
  for _, componentPath := range componentPathList {
  if isSyncFromBranchAllowedForThisPath(config.Argocd.AllowSyncfromBranchPathRegex, componentPath) {
- diffCommentData.HasSyncableComponens = true
+ diffCommentData.HasSyncableComponents = true
  break
  }
  }
-
- err, templateOutput := executeTemplate(ghPrClientDetails.PrLogger, "argoCdDiff", "argoCD-diff-pr-comment.gotmpl", diffCommentData)
+ comments, err := generateArgoCdDiffComments(diffCommentData, githubCommentMaxSize)
  if err != nil {
  prHandleError = err
- ghPrClientDetails.PrLogger.Errorf("Failed to generate ArgoCD diff comment template: err=%s\n", err)
- } else if len(templateOutput) > githubCommentMaxSize {
-  ghPrClientDetails.PrLogger.Warnf("Diff comment is too large (%d bytes), using concise template", len(templateOutput))
- err, templateOutput = executeTemplate(ghPrClientDetails.PrLogger, "argoCdDiffConcise", "argoCD-diff-pr-comment-concise.gotmpl", diffCommentData)
+ ghPrClientDetails.PrLogger.Errorf("Failed to comment ArgoCD diff: err=%s\n", err)
+ }
+ for _, comment := range comments {
+ err = commentPR(ghPrClientDetails, comment)
  if err != nil {
  prHandleError = err
- ghPrClientDetails.PrLogger.Errorf("Failed to generate ArgoCD diff comment template: err=%s\n", err)
+ ghPrClientDetails.PrLogger.Errorf("Failed to comment on PR: err=%s\n", err)
  }
  }
-
- err = commentPR(ghPrClientDetails, templateOutput)
- if err != nil {
- prHandleError = err
- ghPrClientDetails.PrLogger.Errorf("Failed to comment ArgoCD diff: err=%s\n", err)
- }
  } else {
  ghPrClientDetails.PrLogger.Debugf("Diff not find affected ArogCD apps")
  }
@@ -230,6 +226,47 @@ func HandlePREvent(eventPayload *github.PullRequestEvent, ghPrClientDetails GhPr
  }
 }
 
+func generateArgoCdDiffComments(diffCommentData DiffCommentData, githubCommentMaxSize int) (comments []string, err error) {
+ err, templateOutput := executeTemplate("argoCdDiff", "argoCD-diff-pr-comment.gotmpl", diffCommentData)
+ if err != nil {
+ return nil, fmt.Errorf("failed to generate ArgoCD diff comment template: %w", err)
+ }
+
+ // Happy path, the diff comment is small enough to be posted in one comment
+ if len(templateOutput) < githubCommentMaxSize {
+ comments = append(comments, templateOutput)
+ return comments, nil
+ }
+
+ // If the diff comment is too large, we'll split it into multiple comments, one per component
+ totalComponents := len(diffCommentData.DiffOfChangedComponents)
+ for i, singleComponentDiff := range diffCommentData.DiffOfChangedComponents {
+ componentTemplateData := diffCommentData
+ componentTemplateData.DiffOfChangedComponents = []argocd.DiffResult{singleComponentDiff}
+ componentTemplateData.Header = fmt.Sprintf("Component %d/%d: %s (Split for comment size)", i+1, totalComponents, singleComponentDiff.ComponentPath)
+ err, templateOutput := executeTemplate("argoCdDiff", "argoCD-diff-pr-comment.gotmpl", componentTemplateData)
+ if err != nil {
+ return nil, fmt.Errorf("failed to generate ArgoCD diff comment template: %w", err)
+ }
+
+ // Even per component comments can be too large, in that case we'll just use the concise template
+ // Somewhat Happy path, the per-component diff comment is small enough to be posted in one comment
+ if len(templateOutput) < githubCommentMaxSize {
+ comments = append(comments, templateOutput)
+ continue
+ }
+
+ // now we don't have much choice, this is the saddest path, we'll use the concise template
+ err, templateOutput = executeTemplate("argoCdDiffConcise", "argoCD-diff-pr-comment-concise.gotmpl", componentTemplateData)
+ if err != nil {
+ return comments, fmt.Errorf("failed to generate ArgoCD diff comment template: %w", err)
+ }
+ comments = append(comments, templateOutput)
+ }
+
+ return comments, nil
+}
+
 // ReciveEventFile this one is similar to ReciveWebhook but it's used for CLI triggering, i simulates a webhook event to use the same code path as the webhook handler.
 func ReciveEventFile(eventType string, eventFilePath string, mainGhClientCache *lru.Cache[string, GhClientPair], prApproverGhClientCache *lru.Cache[string, GhClientPair]) {
  log.Infof("Event type: %s", eventType)
@@ -449,21 +486,23 @@ func handleCommentPrEvent(ghPrClientDetails GhPrClientDetails, ce *github.IssueC
 }
 
 func commentPlanInPR(ghPrClientDetails GhPrClientDetails, promotions map[string]PromotionInstance) {
- _, templateOutput := executeTemplate(ghPrClientDetails.PrLogger, "dryRunMsg", "dry-run-pr-comment.gotmpl", promotions)
+ err, templateOutput := executeTemplate("dryRunMsg", "dry-run-pr-comment.gotmpl", promotions)
+ if err != nil {
+ ghPrClientDetails.PrLogger.Errorf("Failed to generate dry-run comment template: err=%s\n", err)
+ return
+ }
  _ = commentPR(ghPrClientDetails, templateOutput)
 }
 
-func executeTemplate(logger *log.Entry, templateName string, templateFile string, data interface{}) (error, string) {
+func executeTemplate(templateName string, templateFile string, data interface{}) (error, string) {
  var templateOutput bytes.Buffer
  messageTemplate, err := template.New(templateName).ParseFiles(getEnv("TEMPLATES_PATH", "templates/") + templateFile)
  if err != nil {
- logger.Errorf("Failed to parse template: err=%v", err)
- return err, ""
+ return fmt.Errorf("failed to parse template: %w", err), ""
  }
  err = messageTemplate.ExecuteTemplate(&templateOutput, templateName, data)
  if err != nil {
- logger.Errorf("Failed to execute template: err=%v", err)
- return err, ""
+ return fmt.Errorf("failed to execute template: %w", err), ""
  }
  return nil, templateOutput.String()
 }
@@ -594,7 +633,7 @@ func handleMergedPrEvent(ghPrClientDetails GhPrClientDetails, prApproverGithubCl
  templateData := map[string]interface{}{
  "prNumber": *pull.Number,
  }
- err, templateOutput := executeTemplate(ghPrClientDetails.PrLogger, "autoMerge", "auto-merge-comment.gotmpl", templateData)
+ err, templateOutput := executeTemplate("autoMerge", "auto-merge-comment.gotmpl", templateData)
  if err != nil {
  return err
  }

internal/pkg/githubapi/github_test.go

@@ -2,6 +2,7 @@ package githubapi
 
 import (
  "bytes"
+ "encoding/json"
  "os"
  "testing"
 
@@ -161,6 +162,69 @@ func TestIsSyncFromBranchAllowedForThisPath(t *testing.T) {
  }
 }
 
+func TestGenerateArgoCdDiffComments(t *testing.T) {
+ t.Parallel()
+ tests := map[string]struct {
+ diffCommentDataTestDataFileName string
+ expectedNumberOfComments int
+ maxCommentLength int
+ }{
+ "All cluster diffs fit in one comment": {
+ diffCommentDataTestDataFileName: "./testdata/diff_comment_data_test.json",
+ expectedNumberOfComments: 1,
+ maxCommentLength: 65535,
+ },
+ "Split diffs, one cluster per comment": {
+ diffCommentDataTestDataFileName: "./testdata/diff_comment_data_test.json",
+ expectedNumberOfComments: 3,
+ maxCommentLength: 1000,
+ },
+ "Split diffs, but maxCommentLength is very small so need to use the concise template": {
+ diffCommentDataTestDataFileName: "./testdata/diff_comment_data_test.json",
+ expectedNumberOfComments: 3,
+ maxCommentLength: 600,
+ },
+ }
+
+ for name, tc := range tests {
+ tc := tc // capture range variable
+ name := name
+ t.Run(name, func(t *testing.T) {
+ t.Parallel()
+ var diffCommentData DiffCommentData
+ readJSONFromFile(t, tc.diffCommentDataTestDataFileName, &diffCommentData)
+
+ result, err := generateArgoCdDiffComments(diffCommentData, tc.maxCommentLength)
+ if err != nil {
+ t.Errorf("Error generating diff comments: %s", err)
+ }
+ if len(result) != tc.expectedNumberOfComments {
+ t.Errorf("%s: Expected number of comments to be %v, got %v", name, tc.expectedNumberOfComments, len(result))
+ }
+ for _, comment := range result {
+ if len(comment) > tc.maxCommentLength {
+ t.Errorf("%s: Expected comment length to be less than %d, got %d", name, tc.maxCommentLength, len(comment))
+ }
+ }
+ })
+ }
+}
+
+func readJSONFromFile(t *testing.T, filename string, data interface{}) {
+ t.Helper()
+ // Read the JSON from the file
+ jsonData, err := os.ReadFile(filename)
+ if err != nil {
+ t.Fatalf("Error loading test data file: %s", err)
+ }
+
+ // Unserialize the JSON into the provided struct
+ err = json.Unmarshal(jsonData, data)
+ if err != nil {
+ t.Fatalf("Error unmarshalling JSON: %s", err)
+ }
+}
+
 func TestPrBody(t *testing.T) {
  t.Parallel()
  keys := []int{1, 2, 3}

internal/pkg/githubapi/promotion.go

@@ -76,7 +76,7 @@ func DetectDrift(ghPrClientDetails GhPrClientDetails) error {
  }
  }
  if len(diffOutputMap) != 0 {
- err, templateOutput := executeTemplate(ghPrClientDetails.PrLogger, "driftMsg", "drift-pr-comment.gotmpl", diffOutputMap)
+ err, templateOutput := executeTemplate("driftMsg", "drift-pr-comment.gotmpl", diffOutputMap)
  if err != nil {
  return err
  }

internal/pkg/githubapi/testdata/diff_comment_data_test.json

@@ -0,0 +1,80 @@
+
+{
+ "DiffOfChangedComponents": [
+ {
+ "ComponentPath": "clusters/playground/aws/eu-central-1/v1/special-delivery/ssllab-test/ssllab-test",
+ "ArgoCdAppName": "temp-ssllab-test-plg-aws-eu-central1-v1",
+ "ArgoCdAppURL": "https://argocd-lab.example.com/applications/temp-ssllab-test-plg-aws-eu-central1-v1",
+ "DiffElements": [
+ {
+ "ObjectGroup": "",
+ "ObjectName": "ssllabs-exporter",
+ "ObjectKind": "Service",
+ "ObjectNamespace": "",
+ "Diff": " (*unstructured.Unstructured)(\n- \tnil,\n+ \t\u0026{\n+ \t\tObject: map[string]any{\n+ \t\t\t\"apiVersion\": string(\"v1\"),\n+ \t\t\t\"kind\": string(\"Service\"),\n+ \t\t\t\"metadata\": map[string]any{\"labels\": map[string]any{...}, \"name\": string(\"ssllabs-exporter\")},\n+ \t\t\t\"spec\": map[string]any{\n+ \t\t\t\t\"ports\": []any{...},\n+ \t\t\t\t\"selector\": map[string]any{...},\n+ \t\t\t\t\"type\": string(\"ClusterIP\"),\n+ \t\t\t},\n+ \t\t},\n+ \t},\n )\n"
+ },
+ {
+ "ObjectGroup": "apps",
+ "ObjectName": "ssllabs-exporter",
+ "ObjectKind": "Deployment",
+ "ObjectNamespace": "",
+ "Diff": " (*unstructured.Unstructured)(\n- \tnil,\n+ \t\u0026{\n+ \t\tObject: map[string]any{\n+ \t\t\t\"apiVersion\": string(\"apps/v1\"),\n+ \t\t\t\"kind\": string(\"Deployment\"),\n+ \t\t\t\"metadata\": map[string]any{\"labels\": map[string]any{...}, \"name\": string(\"ssllabs-exporter\")},\n+ \t\t\t\"spec\": map[string]any{\n+ \t\t\t\t\"replicas\": int64(2),\n+ \t\t\t\t\"selector\": map[string]any{...},\n+ \t\t\t\t\"template\": map[string]any{...},\n+ \t\t\t},\n+ \t\t},\n+ \t},\n )\n"
+ }
+ ],
+ "HasDiff": true,
+ "DiffError": null,
+ "AppWasTemporarilyCreated": false
+ },
+ {
+ "ComponentPath": "clusters/playground/aws/eu-central-1/v2/special-delivery/ssllab-test/ssllab-test",
+ "ArgoCdAppName": "temp-ssllab-test-plg-aws-eu-central1-v2",
+ "ArgoCdAppURL": "https://argocd-lab.example.com/applications/temp-ssllab-test-plg-aws-eu-central1-v1",
+ "DiffElements": [
+ {
+ "ObjectGroup": "",
+ "ObjectName": "ssllabs-exporter",
+ "ObjectKind": "Service",
+ "ObjectNamespace": "",
+ "Diff": " (*unstructured.Unstructured)(\n- \tnil,\n+ \t\u0026{\n+ \t\tObject: map[string]any{\n+ \t\t\t\"apiVersion\": string(\"v1\"),\n+ \t\t\t\"kind\": string(\"Service\"),\n+ \t\t\t\"metadata\": map[string]any{\"labels\": map[string]any{...}, \"name\": string(\"ssllabs-exporter\")},\n+ \t\t\t\"spec\": map[string]any{\n+ \t\t\t\t\"ports\": []any{...},\n+ \t\t\t\t\"selector\": map[string]any{...},\n+ \t\t\t\t\"type\": string(\"ClusterIP\"),\n+ \t\t\t},\n+ \t\t},\n+ \t},\n )\n"
+ },
+ {
+ "ObjectGroup": "apps",
+ "ObjectName": "ssllabs-exporter",
+ "ObjectKind": "Deployment",
+ "ObjectNamespace": "",
+ "Diff": " (*unstructured.Unstructured)(\n- \tnil,\n+ \t\u0026{\n+ \t\tObject: map[string]any{\n+ \t\t\t\"apiVersion\": string(\"apps/v1\"),\n+ \t\t\t\"kind\": string(\"Deployment\"),\n+ \t\t\t\"metadata\": map[string]any{\"labels\": map[string]any{...}, \"name\": string(\"ssllabs-exporter\")},\n+ \t\t\t\"spec\": map[string]any{\n+ \t\t\t\t\"replicas\": int64(2),\n+ \t\t\t\t\"selector\": map[string]any{...},\n+ \t\t\t\t\"template\": map[string]any{...},\n+ \t\t\t},\n+ \t\t},\n+ \t},\n )\n"
+ }
+ ],
+ "HasDiff": true,
+ "DiffError": null,
+ "AppWasTemporarilyCreated": false
+ },
+ {
+ "ComponentPath": "clusters/playground/aws/eu-central-1/v3/special-delivery/ssllab-test/ssllab-test",
+ "ArgoCdAppName": "temp-ssllab-test-plg-aws-eu-central1-v3",
+ "ArgoCdAppURL": "https://argocd-lab.example.com/applications/temp-ssllab-test-plg-aws-eu-central1-v1",
+ "DiffElements": [
+ {
+ "ObjectGroup": "",
+ "ObjectName": "ssllabs-exporter",
+ "ObjectKind": "Service",
+ "ObjectNamespace": "",
+ "Diff": " (*unstructured.Unstructured)(\n- \tnil,\n+ \t\u0026{\n+ \t\tObject: map[string]any{\n+ \t\t\t\"apiVersion\": string(\"v1\"),\n+ \t\t\t\"kind\": string(\"Service\"),\n+ \t\t\t\"metadata\": map[string]any{\"labels\": map[string]any{...}, \"name\": string(\"ssllabs-exporter\")},\n+ \t\t\t\"spec\": map[string]any{\n+ \t\t\t\t\"ports\": []any{...},\n+ \t\t\t\t\"selector\": map[string]any{...},\n+ \t\t\t\t\"type\": string(\"ClusterIP\"),\n+ \t\t\t},\n+ \t\t},\n+ \t},\n )\n"
+ },
+ {
+ "ObjectGroup": "apps",
+ "ObjectName": "ssllabs-exporter",
+ "ObjectKind": "Deployment",
+ "ObjectNamespace": "",
+ "Diff": " (*unstructured.Unstructured)(\n- \tnil,\n+ \t\u0026{\n+ \t\tObject: map[string]any{\n+ \t\t\t\"apiVersion\": string(\"apps/v1\"),\n+ \t\t\t\"kind\": string(\"Deployment\"),\n+ \t\t\t\"metadata\": map[string]any{\"labels\": map[string]any{...}, \"name\": string(\"ssllabs-exporter\")},\n+ \t\t\t\"spec\": map[string]any{\n+ \t\t\t\t\"replicas\": int64(2),\n+ \t\t\t\t\"selector\": map[string]any{...},\n+ \t\t\t\t\"template\": map[string]any{...},\n+ \t\t\t},\n+ \t\t},\n+ \t},\n )\n"
+ }
+ ],
+ "HasDiff": true,
+ "DiffError": null,
+ "AppWasTemporarilyCreated": false
+ }
+ ],
+ "HasSyncableComponents": false,
+ "BranchName": "promotions/284-simulate-error-5c159151017f",
+ "Header": ""
+}

templates/argoCD-diff-pr-comment-concise.gotmpl

@@ -37,7 +37,7 @@ It will not be present in ArgoCD UI for more than a few seconds and it can not b
 
 {{- end }}
 
-{{- if .HasSyncableComponens }}
+{{- if .HasSyncableComponents }}
 
 - [ ] <!-- telefonistka-argocd-branch-sync --> Set ArgoCD apps Target Revision to `{{ .BranchName }}` 
 

templates/argoCD-diff-pr-comment.gotmpl

@@ -1,4 +1,7 @@
 {{define "argoCdDiff"}}
+{{ if .Header }}
+{{ .Header }}
+{{- end}}
 Diff of ArgoCD applications:
 {{ range $appDiffResult := .DiffOfChangedComponents }}
 
@@ -40,7 +43,7 @@ It will not be present in ArgoCD UI for more than a few seconds and it can not b
 
 {{- end }}
 
-{{- if .HasSyncableComponens }}
+{{- if .HasSyncableComponents }}
 
 - [ ] <!-- telefonistka-argocd-branch-sync --> Set ArgoCD apps Target Revision to `{{ .BranchName }}`