Support two way to fetch matching ArgoCD apps from a Telefonistka

component the telefonistka dedicated sha1 label and the ArgoCD native
`manifest-generate-paths`  annotation

Author: Oded-B

go.mod

@@ -6,8 +6,8 @@ toolchain go1.22.1
 
 require (
 	github.com/alexliesenfeld/health v0.8.0
-	github.com/argoproj/argo-cd/v2 v2.11.0-rc1
-	github.com/argoproj/gitops-engine v0.7.1-0.20240411122334-1ade3a199867
+	github.com/argoproj/argo-cd/v2 v2.11.2
+	github.com/argoproj/gitops-engine v0.7.1-0.20240416142647-fbecbb86e412
 	github.com/bradleyfalzon/ghinstallation/v2 v2.10.0
 	github.com/go-test/deep v1.1.0
 	github.com/google/go-github/v52 v52.0.0
@@ -77,6 +77,7 @@ require (
 	github.com/gogo/protobuf v1.3.2 // indirect
 	github.com/golang-jwt/jwt/v4 v4.5.0 // indirect
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
+	github.com/golang/mock v1.6.0 // indirect
 	github.com/golang/protobuf v1.5.4 // indirect
 	github.com/google/btree v1.1.2 // indirect
 	github.com/google/gnostic v0.7.0 // indirect

go.sum

@@ -651,8 +651,12 @@ github.com/apache/arrow/go/v11 v11.0.0/go.mod h1:Eg5OsL5H+e299f7u5ssuXsuHQVEGC4x
 github.com/apache/thrift v0.16.0/go.mod h1:PHK3hniurgQaNMZYaCLEqXKsYK8upmhPbmdP2FXSqgU=
 github.com/argoproj/argo-cd/v2 v2.11.0-rc1 h1:VCjpw0bwPcULNJ/FG8BnIyeesyOpT+1MUWuXPskbsWQ=
 github.com/argoproj/argo-cd/v2 v2.11.0-rc1/go.mod h1:/KySYrOzPQupCh7E1pzg6011p4AaRLszbUtkaWyATTU=
+github.com/argoproj/argo-cd/v2 v2.11.2 h1:NygNrTFIMWUe1b48ddUuH+q2vRTHB+dFk3NcErx6GcM=
+github.com/argoproj/argo-cd/v2 v2.11.2/go.mod h1:nUOZqAT9f3GewdG/dzYgrpwqOSMj5ukoWw4yAV2/WXA=
 github.com/argoproj/gitops-engine v0.7.1-0.20240411122334-1ade3a199867 h1:zMATM3uzAQHBLJ142MEGrZ4+3+xsXT36hzB1Dj2jptE=
 github.com/argoproj/gitops-engine v0.7.1-0.20240411122334-1ade3a199867/go.mod h1:gWE8uROi7hIkWGNAVM+8FWkMfo0vZ03SLx/aFw/DBzg=
+github.com/argoproj/gitops-engine v0.7.1-0.20240416142647-fbecbb86e412 h1:je2wJpWtaoS55mA5MBPCeDnKMeF42pkxO9Oa5KbWrdg=
+github.com/argoproj/gitops-engine v0.7.1-0.20240416142647-fbecbb86e412/go.mod h1:gWE8uROi7hIkWGNAVM+8FWkMfo0vZ03SLx/aFw/DBzg=
 github.com/argoproj/pkg v0.13.7-0.20230626144333-d56162821bd1 h1:qsHwwOJ21K2Ao0xPju1sNuqphyMnMYkyB3ZLoLtxWpo=
 github.com/argoproj/pkg v0.13.7-0.20230626144333-d56162821bd1/go.mod h1:CZHlkyAD1/+FbEn6cB2DQTj48IoLGvEYsWEvtzP3238=
 github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio=

internal/pkg/argocd/argocd.go

@@ -7,7 +7,9 @@ import (
 	"encoding/json"
 	"fmt"
 	"os"
+	"path/filepath"
 	"strconv"
+	"strings"
 
 	cmdutil "github.com/argoproj/argo-cd/v2/cmd/util"
 	"github.com/argoproj/argo-cd/v2/pkg/apiclient"
@@ -151,42 +153,102 @@ func createArgoCdClient() (apiclient.Client, error) {
 	return clientset, nil
 }
 
-func generateDiffOfAComponent(ctx context.Context, componentPath string, prBranch string, repo string, appIf application.ApplicationServiceClient, projIf projectpkg.ProjectServiceClient, argoSettings *settings.Settings) (componentDiffResult DiffResult) {
-	componentDiffResult.ComponentPath = componentPath
-
+// This function is used to find an ArgoCD application by the SHA1 label of the component path its supposed to avoid perfromance issues with the "manifest-generate-paths" annotation method with requires pulling all ArgoCD applications(!) on every PR event.
+// The SHA1 label is assumed to be populated by the ApplicationSet controller(or apps of apps  or similar).
+func findArgocdAppBySHA1Label(ctx context.Context, componentPath string, repo string, appIf application.ApplicationServiceClient) (app *argoappv1.Application, err error) {
 	// Calculate sha1 of component path to use in a label selector
 	cPathBa := []byte(componentPath)
 	hasher := sha1.New() //nolint:gosec // G505: Blocklisted import crypto/sha1: weak cryptographic primitive (gosec), this is not a cryptographic use case
 	hasher.Write(cPathBa)
 	componentPathSha1 := hex.EncodeToString(hasher.Sum(nil))
-
-	// Find ArgoCD application by the path SHA1 label selector and repo name
-	// That label is assumed to be pupulated by the ApplicationSet controller(or apps of apps  or similar).
 	labelSelector := fmt.Sprintf("telefonistka.io/component-path-sha1=%s", componentPathSha1)
 	appLabelQuery := application.ApplicationQuery{
 		Selector: &labelSelector,
 		Repo:     &repo,
 	}
 	foundApps, err := appIf.List(ctx, &appLabelQuery)
 	if err != nil {
-		componentDiffResult.DiffError = err
-		return componentDiffResult
+		return nil, err
 	}
 	if len(foundApps.Items) == 0 {
-		componentDiffResult.DiffError = fmt.Errorf("No ArgoCD application found for component path %s(repo %s), used this label selector: %s", componentPath, repo, labelSelector)
+		return nil, fmt.Errorf("No ArgoCD application found for component path sha1 %s(repo %s), used this label selector: %s", componentPathSha1, repo, labelSelector)
+	}
+
+	// we expect only one app with this label and repo selectors
+	return &foundApps.Items[0], nil
+}
+
+// This is the default method to find an ArgoCD application by the manifest-generate-paths annotation.
+// It assume the ArgoCD (optional) manifest-generate-paths annotation is set on all relevent apps.
+// Notice that this method include a full list all ArgoCD applications in the repo, this could be a performance issue if there are many apps in the repo.
+func findArgocdAppByManifestPathAnnotation(ctx context.Context, componentPath string, repo string, appIf application.ApplicationServiceClient) (app *argoappv1.Application, err error) {
+	//argocd.argoproj.io/manifest-generate-paths
+	appQuery := application.ApplicationQuery{
+		Repo: &repo,
+	}
+	// TODO Instrument this, we might have a lot of apps in a repo, performance might be an issue
+	allRepoApps, err := appIf.List(ctx, &appQuery)
+	if err != nil {
+		return nil, err
+	}
+	for _, app := range allRepoApps.Items {
+		// Check if the app has the annotation
+		// https://argo-cd.readthedocs.io/en/stable/operator-manual/high_availability/#manifest-paths-annotation
+		// Consider the annotation content can a semi-colon separated list of paths, an absolute path or a relative path(start with a ".")  and the manifest-paths-annotation could be a subpath of componentPath.
+		// We need to check if the annotation is a subpath of componentPath
+
+		appManifestPathsAnnotation := app.Annotations["argocd.argoproj.io/manifest-generate-paths"]
+
+		for _, manifetsPathElement := range strings.Split(appManifestPathsAnnotation, ";") {
+			// if `manifest-generate-paths` element starts with a "." it is a relative path(relative to repo root), we need to join it with the app source path
+			if strings.HasPrefix(manifetsPathElement, ".") {
+				manifetsPathElement = filepath.Join(app.Spec.Source.Path, manifetsPathElement)
+			}
+
+			// Checking is componentPath is a subpath of the manifetsPathElement
+			// Using filepath.Rel solves all kinds of path issues, like double slashes, etc.
+			rel, err := filepath.Rel(manifetsPathElement, componentPath)
+			if !strings.HasPrefix(rel, "..") && err == nil {
+				log.Debugf("Found app %s with manifest-generate-paths(\"%s\") annotation that matches %s", app.Name, appManifestPathsAnnotation, componentPath)
+				return &app, nil
+			}
+
+		}
+
+	}
+	return nil, fmt.Errorf("No ArgoCD application found with manifest-generate-paths annotation that matches %s(looked at repo %s, checked %v apps)	", componentPath, repo, len(allRepoApps.Items))
+}
+
+func generateDiffOfAComponent(ctx context.Context, componentPath string, prBranch string, repo string, appIf application.ApplicationServiceClient, projIf projectpkg.ProjectServiceClient, argoSettings *settings.Settings, usaSHALabelForArgoDicovery bool) (componentDiffResult DiffResult) {
+	componentDiffResult.ComponentPath = componentPath
+
+	// Find ArgoCD application by the path SHA1 label selector and repo name
+	// At the moment we assume one to one mapping between Telefonistka components and ArgoCD application
+
+	var foundApp *argoappv1.Application
+	var err error
+	if usaSHALabelForArgoDicovery {
+		foundApp, err = findArgocdAppBySHA1Label(ctx, componentPath, repo, appIf)
+	} else {
+		foundApp, err = findArgocdAppByManifestPathAnnotation(ctx, componentPath, repo, appIf)
+
+	}
+	if err != nil {
+		componentDiffResult.DiffError = err
 		return componentDiffResult
 	}
 
 	// Get the application and its resources, resources are the live state of the application objects.
+	// The 2nd "app fetch" is needed for the "refreshTypeHArd", we don't want to do that to non-relevant apps"
 	refreshType := string(argoappv1.RefreshTypeHard)
 	appNameQuery := application.ApplicationQuery{
-		Name:    &foundApps.Items[0].Name, // we expect only one app with this label and repo selectors
+		Name:    &foundApp.Name, // we expect only one app with this label and repo selectors
 		Refresh: &refreshType,
 	}
 	app, err := appIf.Get(ctx, &appNameQuery)
 	if err != nil {
 		componentDiffResult.DiffError = err
-		log.Errorf("Error getting app %s: %v", foundApps.Items[0].Name, err)
+		log.Errorf("Error getting app %s: %v", foundApp.Name, err)
 		return componentDiffResult
 	}
 	componentDiffResult.ArgoCdAppName = app.Name
@@ -232,7 +294,7 @@ func generateDiffOfAComponent(ctx context.Context, componentPath string, prBranc
 }
 
 // GenerateDiffOfChangedComponents generates diff of changed components
-func GenerateDiffOfChangedComponents(ctx context.Context, componentPathList []string, prBranch string, repo string) (hasComponentDiff bool, hasComponentDiffErrors bool, diffResults []DiffResult, err error) {
+func GenerateDiffOfChangedComponents(ctx context.Context, componentPathList []string, prBranch string, repo string, usaSHALabelForArgoDicovery bool) (hasComponentDiff bool, hasComponentDiffErrors bool, diffResults []DiffResult, err error) {
 	hasComponentDiff = false
 	hasComponentDiffErrors = false
 	// env var should be centralized
@@ -264,7 +326,7 @@ func GenerateDiffOfChangedComponents(ctx context.Context, componentPathList []st
 	}
 
 	for _, componentPath := range componentPathList {
-		currentDiffResult := generateDiffOfAComponent(ctx, componentPath, prBranch, repo, appIf, projIf, argoSettings)
+		currentDiffResult := generateDiffOfAComponent(ctx, componentPath, prBranch, repo, appIf, projIf, argoSettings, usaSHALabelForArgoDicovery)
 		if currentDiffResult.DiffError != nil {
 			hasComponentDiffErrors = true
 			err = currentDiffResult.DiffError

internal/pkg/argocd/argocd_test.go

@@ -0,0 +1,196 @@
+package argocd
+
+// @Title
+// @Description
+// @Author
+// @Update
+
+import (
+	"context"
+	"testing"
+
+	argoappv1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
+	"github.com/golang/mock/gomock"
+	argo_app_mock "github.com/wayfair-incubator/telefonistka/mocks/argocd"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestFindArgocdAppBySHA1Label(t *testing.T) {
+	// Here the filtering is done on the ArgoCD server side, so we are just testing the function returns a app
+	t.Parallel()
+	ctx := context.Background()
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+	mockApplicationClient := argo_app_mock.NewMockApplicationServiceClient(ctrl)
+	expectedResponse := &argoappv1.ApplicationList{
+		Items: []argoappv1.Application{
+			{
+				ObjectMeta: metav1.ObjectMeta{
+					Labels: map[string]string{
+						"telefonistka.io/component-path-sha1": "111111",
+					},
+					Name: "right-app",
+				},
+			},
+		},
+	}
+
+	mockApplicationClient.EXPECT().List(gomock.Any(), gomock.Any()).Return(expectedResponse, nil)
+
+	app, err := findArgocdAppBySHA1Label(ctx, "random/path", "some-repo", mockApplicationClient)
+	if err != nil {
+		t.Errorf("Error: %v", err)
+	}
+	if app.Name != "right-app" {
+		t.Errorf("App name is not right-app")
+	}
+}
+
+func TestFindArgocdAppByPathAnnotation(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+	mockApplicationClient := argo_app_mock.NewMockApplicationServiceClient(ctrl)
+	expectedResponse := &argoappv1.ApplicationList{
+		Items: []argoappv1.Application{
+			{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						"argocd.argoproj.io/manifest-generate-paths": "wrong/path/",
+					},
+					Name: "wrong-app",
+				},
+			},
+			{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						"argocd.argoproj.io/manifest-generate-paths": "right/path/",
+					},
+					Name: "right-app",
+				},
+			},
+		},
+	}
+
+	mockApplicationClient.EXPECT().List(gomock.Any(), gomock.Any()).Return(expectedResponse, nil)
+
+	apps, err := findArgocdAppByManifestPathAnnotation(ctx, "right/path", "some-repo", mockApplicationClient)
+	if err != nil {
+		t.Errorf("Error: %v", err)
+	}
+	t.Logf("apps: %v", apps)
+
+}
+
+// Here I'm testing a ";" delimted path annotation
+func TestFindArgocdAppByPathAnnotationSemiColon(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+	mockApplicationClient := argo_app_mock.NewMockApplicationServiceClient(ctrl)
+	expectedResponse := &argoappv1.ApplicationList{
+		Items: []argoappv1.Application{
+			{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						"argocd.argoproj.io/manifest-generate-paths": "wrong/path/;wrong/path2/",
+					},
+					Name: "wrong-app",
+				},
+			},
+			{ // This is the app we want to find - it has the right path as one of the elements in the annotation
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						"argocd.argoproj.io/manifest-generate-paths": "wrong/path/;right/path/",
+					},
+					Name: "right-app",
+				},
+			},
+		},
+	}
+
+	mockApplicationClient.EXPECT().List(gomock.Any(), gomock.Any()).Return(expectedResponse, nil)
+
+	app, err := findArgocdAppByManifestPathAnnotation(ctx, "right/path", "some-repo", mockApplicationClient)
+	if err != nil {
+		t.Errorf("Error: %v", err)
+	}
+	if app.Name != "right-app" {
+		t.Errorf("App name is not right-app")
+	}
+
+}
+
+// Here I'm testing a "." path annotation - this is a special case where the path is relative to the repo root specified in the application .spec
+func TestFindArgocdAppByPathAnnotationRelative(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+	mockApplicationClient := argo_app_mock.NewMockApplicationServiceClient(ctrl)
+	expectedResponse := &argoappv1.ApplicationList{
+		Items: []argoappv1.Application{
+			{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						"argocd.argoproj.io/manifest-generate-paths": ".",
+					},
+					Name: "right-app",
+				},
+				Spec: argoappv1.ApplicationSpec{
+					Source: &argoappv1.ApplicationSource{
+						RepoURL: "",
+						Path:    "right/path",
+					},
+				},
+			},
+		},
+	}
+
+	mockApplicationClient.EXPECT().List(gomock.Any(), gomock.Any()).Return(expectedResponse, nil)
+	app, err := findArgocdAppByManifestPathAnnotation(ctx, "right/path", "some-repo", mockApplicationClient)
+	if err != nil {
+		t.Errorf("Error: %v", err)
+	} else if app.Name != "right-app" {
+		t.Errorf("App name is not right-app")
+	}
+
+}
+
+// Here I'm testing a "." path annotation - this is a special case where the path is relative to the repo root specified in the application .spec
+func TestFindArgocdAppByPathAnnotationRelative2(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+	mockApplicationClient := argo_app_mock.NewMockApplicationServiceClient(ctrl)
+	expectedResponse := &argoappv1.ApplicationList{
+		Items: []argoappv1.Application{
+			{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						"argocd.argoproj.io/manifest-generate-paths": "./path",
+					},
+					Name: "right-app",
+				},
+				Spec: argoappv1.ApplicationSpec{
+					Source: &argoappv1.ApplicationSource{
+						RepoURL: "",
+						Path:    "right/",
+					},
+				},
+			},
+		},
+	}
+
+	mockApplicationClient.EXPECT().List(gomock.Any(), gomock.Any()).Return(expectedResponse, nil)
+	app, err := findArgocdAppByManifestPathAnnotation(ctx, "right/path", "some-repo", mockApplicationClient)
+	if err != nil {
+		t.Errorf("Error: %v", err)
+	} else if app.Name != "right-app" {
+		t.Errorf("App name is not right-app")
+	}
+
+}

internal/pkg/configuration/config.go

@@ -35,13 +35,14 @@ type Config struct {
 	PromotionPaths []PromotionPath `yaml:"promotionPaths"`
 
 	// Generic configuration
-	PromtionPrLables        []string               `yaml:"promtionPRlables"`
-	DryRunMode              bool                   `yaml:"dryRunMode"`
-	AutoApprovePromotionPrs bool                   `yaml:"autoApprovePromotionPrs"`
-	ToggleCommitStatus      map[string]string      `yaml:"toggleCommitStatus"`
-	WebhookEndpointRegexs   []WebhookEndpointRegex `yaml:"webhookEndpointRegexs"`
-	CommentArgocdDiffonPR   bool                   `yaml:"commentArgocdDiffonPR"`
-	AutoMergeNoDiffPRs      bool                   `yaml:"autoMergeNoDiffPRs"`
+	PromtionPrLables           []string               `yaml:"promtionPRlables"`
+	DryRunMode                 bool                   `yaml:"dryRunMode"`
+	AutoApprovePromotionPrs    bool                   `yaml:"autoApprovePromotionPrs"`
+	ToggleCommitStatus         map[string]string      `yaml:"toggleCommitStatus"`
+	WebhookEndpointRegexs      []WebhookEndpointRegex `yaml:"webhookEndpointRegexs"`
+	CommentArgocdDiffonPR      bool                   `yaml:"commentArgocdDiffonPR"`
+	AutoMergeNoDiffPRs         bool                   `yaml:"autoMergeNoDiffPRs"`
+	UsaSHALabelForArgoDicovery bool                   `yaml:"usaSHALabelForArgoDicovery"`
 }
 
 func ParseConfigFromYaml(y string) (*Config, error) {