refactor: extract GetManifestGeneratePaths() and add tests

go.mod

@@ -6,6 +6,7 @@ require (
 	github.com/argoproj/argo-cd/v2 v2.3.3
 	github.com/argoproj/gitops-engine v0.6.2
 	github.com/go-logr/logr v1.2.3
+	github.com/google/go-cmp v0.5.6
 	github.com/google/go-github/v39 v39.2.0
 	github.com/int128/oauth2-github-app v0.1.0
 	github.com/onsi/ginkgo v1.16.5

pkg/notification/comment.go

@@ -3,7 +3,6 @@ package notification
 import (
 	"context"
 	"fmt"
-	"path/filepath"
 	"strings"
 
 	"github.com/argoproj/gitops-engine/pkg/health"
@@ -60,24 +59,14 @@ func filterPullRequestsRelatedToEvent(pulls []github.PullRequest, e Event) []int
 func isPullRequestRelatedToEvent(pull github.PullRequest, e Event) bool {
 	// support manifest path annotation
 	// see https://argo-cd.readthedocs.io/en/stable/operator-manual/high_availability/#webhook-and-manifest-paths-annotation
-	manifestPaths := []string{e.Application.Spec.Source.Path}
-	annotatedPaths := strings.Split(e.Application.Annotations["argocd.argoproj.io/manifest-generate-paths"], ";")
-	for _, path := range annotatedPaths {
-		// convert to absolute path
-		absolutePath := path
-		if !filepath.IsAbs(path) {
-			absolutePath = filepath.Join(e.Application.Spec.Source.Path, path)
-		}
-		// remove leading slash
-		if absolutePath[0:1] == "/" {
-			absolutePath = absolutePath[1:]
-		}
-		// add to list of manifest paths
-		manifestPaths = append(manifestPaths, absolutePath)
-	}
+	// https://github.com/int128/argocd-commenter/pull/656
+	manifestGeneratePaths := e.GetManifestGeneratePaths()
 
 	for _, file := range pull.Files {
-		for _, path := range manifestPaths {
+		if strings.HasPrefix(file, e.Application.Spec.Source.Path) {
+			return true
+		}
+		for _, path := range manifestGeneratePaths {
 			if strings.HasPrefix(file, path) {
 				return true
 			}

pkg/notification/event.go

@@ -0,0 +1,42 @@
+package notification
+
+import (
+	argocdv1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
+	"path/filepath"
+	"strings"
+)
+
+type Event struct {
+	PhaseIsChanged  bool
+	HealthIsChanged bool
+	Application     argocdv1alpha1.Application
+	ArgoCDURL       string
+}
+
+// GetManifestGeneratePaths returns canonical paths of "argocd.argoproj.io/manifest-generate-paths" annotation.
+// It returns nil if the field is nil or empty.
+// https://argo-cd.readthedocs.io/en/stable/operator-manual/high_availability/#webhook-and-manifest-paths-annotation
+func (e Event) GetManifestGeneratePaths() []string {
+	if e.Application.Annotations == nil {
+		return nil
+	}
+	var canonicalPaths []string
+	annotatedPaths := strings.Split(e.Application.Annotations["argocd.argoproj.io/manifest-generate-paths"], ";")
+	for _, path := range annotatedPaths {
+		if path == "" {
+			return nil
+		}
+		// convert to absolute path
+		absolutePath := path
+		if !filepath.IsAbs(path) {
+			absolutePath = filepath.Join(e.Application.Spec.Source.Path, path)
+		}
+		// remove leading slash
+		if absolutePath[0:1] == "/" {
+			absolutePath = absolutePath[1:]
+		}
+		// add to list of manifest paths
+		canonicalPaths = append(canonicalPaths, absolutePath)
+	}
+	return canonicalPaths
+}

pkg/notification/event_test.go

@@ -0,0 +1,83 @@
+package notification
+
+import (
+	"github.com/google/go-cmp/cmp"
+	"testing"
+)
+
+func TestEvent_GetManifestGeneratePaths(t *testing.T) {
+	t.Run("nil annotation", func(t *testing.T) {
+		var e Event
+		manifestGeneratePaths := e.GetManifestGeneratePaths()
+		if manifestGeneratePaths != nil {
+			t.Errorf("manifestGeneratePaths wants nil but was %+v", manifestGeneratePaths)
+		}
+	})
+
+	t.Run("empty annotation", func(t *testing.T) {
+		var e Event
+		e.Application.Spec.Source.Path = "/applications/app1"
+		e.Application.Annotations = map[string]string{
+			"argocd.argoproj.io/manifest-generate-paths": "",
+		}
+		manifestGeneratePaths := e.GetManifestGeneratePaths()
+		if manifestGeneratePaths != nil {
+			t.Errorf("manifestGeneratePaths wants nil but was %+v", manifestGeneratePaths)
+		}
+	})
+
+	t.Run("absolute path", func(t *testing.T) {
+		var e Event
+		e.Application.Spec.Source.Path = "/applications/app1"
+		e.Application.Annotations = map[string]string{
+			"argocd.argoproj.io/manifest-generate-paths": "/components/app1",
+		}
+		manifestGeneratePaths := e.GetManifestGeneratePaths()
+		want := []string{"components/app1"}
+		if diff := cmp.Diff(want, manifestGeneratePaths); diff != "" {
+			t.Errorf("want != manifestGeneratePaths:\n%s", diff)
+		}
+	})
+
+	t.Run("relative path of ascendant", func(t *testing.T) {
+		var e Event
+		e.Application.Spec.Source.Path = "/applications/app1"
+		e.Application.Annotations = map[string]string{
+			"argocd.argoproj.io/manifest-generate-paths": "../manifests1",
+		}
+		manifestGeneratePaths := e.GetManifestGeneratePaths()
+		want := []string{"applications/manifests1"}
+		if diff := cmp.Diff(want, manifestGeneratePaths); diff != "" {
+			t.Errorf("want != manifestGeneratePaths:\n%s", diff)
+		}
+	})
+
+	t.Run("relative path of period", func(t *testing.T) {
+		var e Event
+		e.Application.Spec.Source.Path = "/applications/app1"
+		e.Application.Annotations = map[string]string{
+			"argocd.argoproj.io/manifest-generate-paths": ".",
+		}
+		manifestGeneratePaths := e.GetManifestGeneratePaths()
+		want := []string{"applications/app1"}
+		if diff := cmp.Diff(want, manifestGeneratePaths); diff != "" {
+			t.Errorf("want != manifestGeneratePaths:\n%s", diff)
+		}
+	})
+
+	t.Run("multiple paths", func(t *testing.T) {
+		var e Event
+		e.Application.Spec.Source.Path = "/applications/app1"
+		e.Application.Annotations = map[string]string{
+			"argocd.argoproj.io/manifest-generate-paths": ".;../manifests1",
+		}
+		manifestGeneratePaths := e.GetManifestGeneratePaths()
+		want := []string{
+			"applications/app1",
+			"applications/manifests1",
+		}
+		if diff := cmp.Diff(want, manifestGeneratePaths); diff != "" {
+			t.Errorf("want != manifestGeneratePaths:\n%s", diff)
+		}
+	})
+}

pkg/notification/types.go

@@ -3,17 +3,9 @@ package notification
 import (
 	"context"
 
-	argocdv1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
 	"github.com/int128/argocd-commenter/pkg/github"
 )
 
-type Event struct {
-	PhaseIsChanged  bool
-	HealthIsChanged bool
-	Application     argocdv1alpha1.Application
-	ArgoCDURL       string
-}
-
 type Client interface {
 	Comment(context.Context, Event) error
 	Deployment(context.Context, Event) error