Merge pull request #190 from karlderkaefer/add-configurable-suppress-…

…regex

feat: configurable hash detector regex

README.md

@@ -225,7 +225,7 @@ my custom template
 
 ## Config Priority Mapping
 The config for CDK-Notifier is mapping in following priority (from low to high)
-1. Environment Variables of Map Struct
+1. Environment Variables of Map Struct. For full list of Envs please check [code](https://github.com/karlderkaefer/cdk-notifier/blob/7e8b72d91096f7ee1c3fc1d97fb68ab84a129bc2/cmd/root.go#L109-L130)
     ```
     REPO_NAME
     REPO_OWNER
@@ -256,7 +256,11 @@ The diff output file is using same path of cdk log file, but is appending `.diff
 
 See github issue [issue#125](https://github.com/karlderkaefer/cdk-notifier/issues/125).
 In case you want to suppress any hash changes in the diff, you can use the flag `--suppress-hash-changes`.
-For example this would not post a diff for any changes in the hash of the resources [see example](./data/cdk-diff-resources-changes.log).
+For example this would not post a diff for any changes in the hash of the resources [see example](./data/cdk-diff-resources-changes.log). You can override the regex that is used to detect hash changes with the flag `--suppress-hash-changes-regex` or setting the Environment Variable `SUPPRESS_HASH_CHANGES_REGEX`.
+
+```bash
+cdk-notifier -l data/cdk-suppress-regex.log --tag-id test --suppress-hash-changes --suppress-hash-changes-regex "^[+-].*?[a-fA-F0-9]{40}"
+```
 
 ## Security
 **Disclaimer**: Consider using on private repositories only.

cmd/root.go

@@ -39,7 +39,8 @@ var rootCmd = &cobra.Command{
 			logrus.Warnf("Suppressing hash changes detected %d hash changes and %d total changes", transformer.HashChanges, transformer.TotalChanges)
 			if transformer.TotalChanges == transformer.HashChanges {
 				logrus.Warnf("Skipping... because suppress-hash-changes is set and only hash changes detected")
-				return
+				// if there are only hash changes we also want to delete the comment
+				appConfig.ForceDeleteComment = true
 			}
 		}
 
@@ -105,6 +106,7 @@ func init() {
 	rootCmd.Flags().String("template", "default", "Template to use for comment [default|extended|extendedWithResources]")
 	rootCmd.Flags().String("custom-template", "", "File path or string input to custom template. When set it will override the template flag.")
 	rootCmd.Flags().Bool("suppress-hash-changes", false, "EXPERIMENTAL: when set to true it will ignore changes in hash values")
+	rootCmd.Flags().String("suppress-hash-changes-regex", config.DefaultSuppressHashChangesRegex, "Define Regex to suppress hash changes. Only used when suppress-hash-changes is set to true")
 
 	// mapping for viper [mapstruct value, flag name]
 	viperMappings := make(map[string]string)
@@ -128,6 +130,7 @@ func init() {
 	viperMappings["GITHUB_ENTERPRISE_HOST"] = "github-host"
 	viperMappings["GITHUB_ENTERPRISE_MAX_COMMENT_LENGTH"] = "github-max-comment-length"
 	viperMappings["SUPPRESS_HASH_CHANGES"] = "suppress-hash-changes"
+	viperMappings["SUPPRESS_HASH_CHANGES_REGEX"] = "suppress-hash-changes-regex"
 
 	for k, v := range viperMappings {
 		err := viper.BindPFlag(k, rootCmd.Flags().Lookup(v))

config/app.go

@@ -11,6 +11,11 @@ import (
 	"github.com/spf13/viper"
 )
 
+const (
+	// Default regex for supressing hash changes
+	DefaultSuppressHashChangesRegex string = `^[+-].*?[a-fA-F0-9]{64,65}`
+)
+
 // ValidationError indicated a missing configuration either CLI argument or environment variable
 type ValidationError struct {
 	CliArg string
@@ -115,26 +120,27 @@ const (
 
 // NotifierConfig holds configuration
 type NotifierConfig struct {
-	LogFile                string `mapstructure:"LOG_FILE"`
-	TagID                  string `mapstructure:"TAG_ID"`
-	RepoName               string `mapstructure:"REPO_NAME"`
-	RepoOwner              string `mapstructure:"REPO_OWNER"`
-	Token                  string `mapstructure:"TOKEN"`
-	TokenUser              string `mapstructure:"TOKEN_USER"`
-	PullRequestID          int    `mapstructure:"PR_ID"`
-	DeleteComment          bool   `mapstructure:"DELETE_COMMENT"`
-	Vcs                    string `mapstructure:"VERSION_CONTROL_SYSTEM"`
-	Ci                     string `mapstructure:"CI_SYSTEM"`
-	Url                    string `mapstructure:"URL"`
-	GithubHost             string `mapstructure:"GITHUB_ENTERPRISE_HOST"`
-	GithubMaxCommentLength int    `mapstructure:"GITHUB_ENTERPRISE_MAX_COMMENT_LENGTH"`
-	NoPostMode             bool   `mapstructure:"NO_POST_MODE"`
-	DisableCollapse        bool   `mapstructure:"DISABLE_COLLAPSE"`
-	// TODO deprecated
-	ShowOverview        bool   `mapstructure:"SHOW_OVERVIEW"`
-	Template            string `mapstructure:"NOTIFIER_TEMPLATE"`
-	CustomTemplate      string `mapstructure:"CUSTOM_TEMPLATE"`
-	SuppressHashChanges bool   `mapstructure:"SUPPRESS_HASH_CHANGES"`
+	LogFile                  string `mapstructure:"LOG_FILE"`
+	TagID                    string `mapstructure:"TAG_ID"`
+	RepoName                 string `mapstructure:"REPO_NAME"`
+	RepoOwner                string `mapstructure:"REPO_OWNER"`
+	Token                    string `mapstructure:"TOKEN"`
+	TokenUser                string `mapstructure:"TOKEN_USER"`
+	PullRequestID            int    `mapstructure:"PR_ID"`
+	DeleteComment            bool   `mapstructure:"DELETE_COMMENT"`
+	Vcs                      string `mapstructure:"VERSION_CONTROL_SYSTEM"`
+	Ci                       string `mapstructure:"CI_SYSTEM"`
+	Url                      string `mapstructure:"URL"`
+	GithubHost               string `mapstructure:"GITHUB_ENTERPRISE_HOST"`
+	GithubMaxCommentLength   int    `mapstructure:"GITHUB_ENTERPRISE_MAX_COMMENT_LENGTH"`
+	NoPostMode               bool   `mapstructure:"NO_POST_MODE"`
+	DisableCollapse          bool   `mapstructure:"DISABLE_COLLAPSE"`
+	Template                 string `mapstructure:"NOTIFIER_TEMPLATE"`
+	CustomTemplate           string `mapstructure:"CUSTOM_TEMPLATE"`
+	SuppressHashChanges      bool   `mapstructure:"SUPPRESS_HASH_CHANGES"`
+	SuppressHashChangesRegex string `mapstructure:"SUPPRESS_HASH_CHANGES_REGEX"`
+	ShowOverview             bool   `mapstructure:"SHOW_OVERVIEW"` // TODO deprecated
+	ForceDeleteComment       bool   // only used for suppress hash changes in order to delete comment if no-op
 }
 
 // Init will create default NotifierConfig with following priority

data/cdk-suppress-regex.log

@@ -0,0 +1,12 @@
+Resources
+[~] AWS::ECS::TaskDefinition Service/Kitu/TaskDef KituTaskDef078AED91 replace
+ └─ [~] ContainerDefinitions (requires replacement)
+     └─ @@ -67,7 +67,7 @@
+        [ ]       {
+        [ ]         "Ref": "AWS::URLSuffix"
+        [ ]       },
+        [-]       "/kitu:444056a84a62f4af0a34df9913fb73d5010f20af"
+        [+]       "/kitu:005327c14a639fb661a33c45cde3cad8e663b990"
+        [ ]     ]
+        [ ]   ]
+        [ ] },

provider/api.go

@@ -77,7 +77,8 @@ func postComment(ns NotifierService, config config.NotifierConfig) (CommentOpera
 	}
 	if comment != nil {
 		// if commit exists but there are no change then delete comment in case DeleteComment is active
-		if config.DeleteComment && !diffHasChanges(ns.GetCommentContent()) {
+		// always execute if DeleteComment and ForceDeleteComment is true
+		if config.DeleteComment && (config.ForceDeleteComment || !diffHasChanges(ns.GetCommentContent())) {
 			err = ns.DeleteComment(comment.Id)
 			if err != nil {
 				logrus.Error(err)

provider/api_test.go

@@ -6,9 +6,173 @@ import (
 	"testing"
 
 	"github.com/karlderkaefer/cdk-notifier/config"
+	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 )
 
+// mockNotifierService simulates the NotifierService interface for testing postComment.
+type mockNotifierService struct {
+	commentExists     bool
+	returnFindErr     bool
+	deleteErr         error
+	updateErr         error
+	createErr         error
+	commentContent    string
+	deletedCommentId  int64
+	updatedCommentId  int64
+	createdCommentId  int64
+}
+
+func (m *mockNotifierService) CreateComment() (*Comment, error) {
+	if m.createErr != nil {
+		return nil, m.createErr
+	}
+	m.createdCommentId = 789
+	return &Comment{Id: m.createdCommentId, Body: m.commentContent}, nil
+}
+
+func (m *mockNotifierService) UpdateComment(id int64) (*Comment, error) {
+	if m.updateErr != nil {
+		return nil, m.updateErr
+	}
+	m.updatedCommentId = id
+	return &Comment{Id: id, Body: m.commentContent}, nil
+}
+
+func (m *mockNotifierService) DeleteComment(id int64) error {
+	if m.deleteErr != nil {
+		return m.deleteErr
+	}
+	m.deletedCommentId = id
+	return nil
+}
+
+func (m *mockNotifierService) SetCommentContent(content string) {
+	m.commentContent = content
+}
+
+func (m *mockNotifierService) GetCommentContent() string {
+	return m.commentContent
+}
+
+// PostComment is not used directly here because postComment calls findComment() itself.
+func (m *mockNotifierService) PostComment() (CommentOperation, error) {
+	return API_COMMENT_NOTHING, nil
+}
+
+// ListComments is called within findComment()
+func (m *mockNotifierService) ListComments() ([]Comment, error) {
+	if m.returnFindErr {
+		return nil, errors.New("mock: error on findComment")
+	}
+	if m.commentExists {
+		return []Comment{{Id: 123, Body: "## cdk diff for myTag\nSomething"}}, nil
+	}
+	return []Comment{}, nil
+}
+
+// TestPostCommentDataDriven tests all branches of postComment with data-driven style.
+func TestPostCommentDataDriven(t *testing.T) {
+    logrus.SetLevel(logrus.DebugLevel)
+
+    tests := []struct {
+        name          string
+        ms            mockNotifierService
+        cfg           config.NotifierConfig
+        wantOp        CommentOperation
+        wantErr       bool
+        wantDeletedID int64
+        wantUpdatedID int64
+        wantCreatedID int64
+    }{
+        {
+            name: "errorOnFindComment",
+            ms: mockNotifierService{
+                returnFindErr:   true,
+                commentExists:   false,
+                commentContent:  "Policy Changes",
+            },
+            cfg:     config.NotifierConfig{},
+            wantOp:  API_COMMENT_NOTHING,
+            wantErr: true,
+        },
+        {
+            name: "existingCommentDeleteCommentForceDelete",
+            ms: mockNotifierService{
+                commentExists:  true,
+                commentContent: "No changes",
+            },
+            cfg: config.NotifierConfig{
+                DeleteComment:      true,
+                ForceDeleteComment: true,
+                TagID:              "myTag",
+            },
+            wantOp:        API_COMMENT_DELETED,
+            wantErr:       false,
+            wantDeletedID: 123,
+        },
+        {
+            name: "existingCommentDeleteCommentNoDiff",
+            ms: mockNotifierService{
+                commentExists:  true,
+                commentContent: "No changes",
+            },
+            cfg: config.NotifierConfig{
+                DeleteComment:      true,
+                ForceDeleteComment: false,
+                TagID:              "myTag",
+            },
+            wantOp:        API_COMMENT_DELETED,
+            wantErr:       false,
+            wantDeletedID: 123,
+        },
+        {
+            name: "existingCommentHasDiff->Update",
+            ms: mockNotifierService{
+                commentExists:  true,
+                commentContent: "Stack resources\nPolicy Changes",
+            },
+            cfg:           config.NotifierConfig{TagID: "myTag"},
+            wantOp:        API_COMMENT_UPDATED,
+            wantUpdatedID: 123, // match updated comment ID
+        },
+        {
+            name: "noCommentNoDiff->Nothing",
+            ms: mockNotifierService{
+                commentExists:  false,
+                commentContent: "no meaningful changes here",
+            },
+            cfg:    config.NotifierConfig{TagID: "myTag"},
+            wantOp: API_COMMENT_NOTHING,
+        },
+        {
+            name: "noCommentHasDiff->Create",
+            ms: mockNotifierService{
+                commentExists:  false,
+                commentContent: "Policy Changes found",
+            },
+            cfg:           config.NotifierConfig{TagID: "myTag"},
+            wantOp:        API_COMMENT_CREATED,
+            wantCreatedID: 789, // match created comment ID
+        },
+    }
+
+    for _, tt := range tests {
+        t.Run(tt.name, func(t *testing.T) {
+            op, err := postComment(&tt.ms, tt.cfg)
+            if tt.wantErr {
+                assert.Error(t, err)
+            } else {
+                assert.NoError(t, err)
+            }
+            assert.Equal(t, tt.wantOp, op)
+            assert.Equal(t, tt.wantDeletedID, tt.ms.deletedCommentId, "deleted comment ID")
+            assert.Equal(t, tt.wantUpdatedID, tt.ms.updatedCommentId, "updated comment ID")
+            assert.Equal(t, tt.wantCreatedID, tt.ms.createdCommentId, "created comment ID")
+        })
+    }
+}
+
 type testCaseCreateService struct {
 	vcs           string
 	expectedType  interface{}

transform/transformer.go

@@ -36,6 +36,7 @@ type LogTransformer struct {
 	ProcessorsChain           LineProcessor
 	TotalChanges              int
 	HashChanges               int
+	SuppressHashChangesRegex  string
 }
 
 type ResourceMetric struct {
@@ -69,16 +70,17 @@ func (p *BaseProcessor) ProcessLine(line string, lt *LogTransformer) string {
 // NewLogTransformer create new log transfer based on config.AppConfig
 func NewLogTransformer(config *config.NotifierConfig) *LogTransformer {
 	lt := &LogTransformer{
-		LogContent:             "",
-		Logfile:                config.LogFile,
-		TagID:                  config.TagID,
-		NoPostMode:             config.NoPostMode,
-		Vcs:                    config.Vcs,
-		DisableCollapse:        config.DisableCollapse,
-		ShowOverview:           config.ShowOverview,
-		Template:               config.Template,
-		CustomTemplate:         config.CustomTemplate,
-		GithubMaxCommentLength: config.GithubMaxCommentLength,
+		LogContent:               "",
+		Logfile:                  config.LogFile,
+		TagID:                    config.TagID,
+		NoPostMode:               config.NoPostMode,
+		Vcs:                      config.Vcs,
+		DisableCollapse:          config.DisableCollapse,
+		ShowOverview:             config.ShowOverview,
+		Template:                 config.Template,
+		CustomTemplate:           config.CustomTemplate,
+		GithubMaxCommentLength:   config.GithubMaxCommentLength,
+		SuppressHashChangesRegex: config.SuppressHashChangesRegex,
 	}
 	lt.initProcessorsChain()
 	return lt
@@ -216,7 +218,7 @@ func (p *IgnoreHashesProcessor) ProcessLine(line string, lt *LogTransformer) str
 	if regex.MatchString(line) {
 		lt.TotalChanges++
 		// https://regex101.com/r/WbUrKv/1
-		regexHash := regexp.MustCompile(`^[+-].*?[a-fA-F0-9]{64,65}`)
+		regexHash := regexp.MustCompile(lt.SuppressHashChangesRegex)
 		if regexHash.MatchString(line) {
 			lt.HashChanges++
 		}

transform/transformer_test.go

@@ -488,6 +488,7 @@ func TestIgnoreHashesProcessor_ProcessLine(t *testing.T) {
 		line           string
 		expectedTotal  int
 		expectedHashes int
+		suppressRegex  string
 	}{
 		{
 			line:           "+ some line",
@@ -529,12 +530,23 @@ func TestIgnoreHashesProcessor_ProcessLine(t *testing.T) {
 			expectedTotal:  1,
 			expectedHashes: 1,
 		},
+		{
+			line:           "+        [+]       \"/kitu:005327c14a639fb661a33c45cde3cad8e663b990",
+			expectedTotal:  1,
+			expectedHashes: 0,
+		},
+		{
+			line:           "+        [+]       \"/kitu:005327c14a639fb661a33c45cde3cad8e663b990",
+			expectedTotal:  1,
+			expectedHashes: 1,
+			suppressRegex:  `^[+-].*?[a-fA-F0-9]{40}`,
+		},
 	}
 
 	processor := &IgnoreHashesProcessor{
 		BaseProcessor: BaseProcessor{},
 	}
-
+	
 	lt := &LogTransformer{
 		TotalChanges: 0,
 		HashChanges:  0,
@@ -543,6 +555,10 @@ func TestIgnoreHashesProcessor_ProcessLine(t *testing.T) {
 	for _, c := range cases {
 		lt.TotalChanges = 0
 		lt.HashChanges = 0
+		lt.SuppressHashChangesRegex = config.DefaultSuppressHashChangesRegex
+		if c.suppressRegex != "" {
+			lt.SuppressHashChangesRegex = c.suppressRegex
+		}
 		processor.ProcessLine(c.line, lt)
 		assert.Equal(t, c.expectedTotal, lt.TotalChanges)
 		assert.Equal(t, c.expectedHashes, lt.HashChanges)