fix(repo-server): fixed repeated resource generation for source directory type (#356)

* reposerver(repository): fixed repeated resources generation for ApplicationSourceTypeDirectory

* reposerver(repository): fixed unit tests after fix of repeated resources generation for ApplicationSourceTypeDirectory

Author: oleksandr-codefresh

changelog/CHANGELOG.md

@@ -1,2 +1,2 @@
-### Fixed
--  fix: failures in update revision for path should not affect sync
+### Changes
+- fix(repo-server): fixed repeated resources generation for ApplicationSourceTypeDirectory

reposerver/repository/repository.go

@@ -1812,21 +1812,18 @@ var manifestFile = regexp.MustCompile(`^.*\.(yaml|yml|json|jsonnet)$`)
 
 // findManifests looks at all yaml files in a directory and unmarshals them into a list of unstructured objects
 func findManifests(logCtx *log.Entry, appPath string, repoRoot string, env *v1alpha1.Env, directory v1alpha1.ApplicationSourceDirectory, enabledManifestGeneration map[string]bool, maxCombinedManifestQuantity resource.Quantity) ([]manifest, error) {
-	absRepoRoot, err := filepath.Abs(repoRoot)
-	if err != nil {
-		return nil, err
-	}
-
-	var manifests []manifest
-
 	// Validate the directory before loading any manifests to save memory.
 	potentiallyValidManifests, err := getPotentiallyValidManifests(logCtx, appPath, repoRoot, directory.Recurse, directory.Include, directory.Exclude, maxCombinedManifestQuantity)
 	if err != nil {
 		logCtx.Errorf("failed to get potentially valid manifests: %s", err)
 		return nil, fmt.Errorf("failed to get potentially valid manifests: %w", err)
 	}
 
-	var objs []*unstructured.Unstructured
+	absRepoRoot, err := filepath.Abs(repoRoot)
+	if err != nil {
+		return nil, err
+	}
+	var manifests []manifest
 	for _, potentiallyValidManifest := range potentiallyValidManifests {
 		manifestPath := potentiallyValidManifest.path
 		manifestFileInfo := potentiallyValidManifest.fileInfo
@@ -1836,6 +1833,7 @@ func findManifests(logCtx *log.Entry, appPath string, repoRoot string, env *v1al
 		repoRelPath, _ := filepath.Rel(absRepoRoot, absPath)
 
 		if strings.HasSuffix(manifestFileInfo.Name(), ".jsonnet") {
+			var objs []*unstructured.Unstructured
 			if !discovery.IsManifestGenerationEnabled(v1alpha1.ApplicationSourceTypeDirectory, enabledManifestGeneration) {
 				continue
 			}
@@ -1875,6 +1873,7 @@ func findManifests(logCtx *log.Entry, appPath string, repoRoot string, env *v1al
 				})
 			}
 		} else {
+			var objs []*unstructured.Unstructured
 			err := getObjsFromYAMLOrJson(logCtx, manifestPath, manifestFileInfo.Name(), &objs)
 			if err != nil {
 				return nil, err

reposerver/repository/repository_test.go

@@ -661,7 +661,7 @@ func TestGenerateManifestsUseExactRevision(t *testing.T) {
 
 	res1, err := service.GenerateManifest(context.Background(), &q)
 	require.NoError(t, err)
-	assert.Len(t, res1.Manifests, 6)
+	assert.Len(t, res1.Manifests, 4)
 	assert.Equal(t, "abc", gitClient.Calls[0].Arguments[0])
 }
 
@@ -677,7 +677,7 @@ func TestRecurseManifestsInDir(t *testing.T) {
 
 	res1, err := service.GenerateManifest(context.Background(), &q)
 	require.NoError(t, err)
-	assert.Len(t, res1.Manifests, 6)
+	assert.Len(t, res1.Manifests, 4)
 }
 
 func TestInvalidManifestsInDir(t *testing.T) {
@@ -2199,7 +2199,7 @@ func TestFindResources(t *testing.T) {
 	}, {
 		name:          "Include Everything",
 		include:       "*.yaml",
-		expectedNames: []string{"nginx-deployment", "nginx-deployment", "nginx-deployment-sub"},
+		expectedNames: []string{"nginx-deployment", "nginx-deployment-sub"},
 	}, {
 		name:          "Include Subdirectory",
 		include:       "**/*.yaml",
@@ -2254,10 +2254,10 @@ func TestFindManifests_Exclude_NothingMatches(t *testing.T) {
 	}, map[string]bool{}, resource.MustParse("0"))
 
 	require.NoError(t, err)
-	require.Len(t, objs, 3)
+	require.Len(t, objs, 2)
 
 	assert.ElementsMatch(t,
-		[]string{"nginx-deployment", "nginx-deployment-sub"}, []string{objs[0].obj.GetName(), objs[2].obj.GetName()})
+		[]string{"nginx-deployment", "nginx-deployment-sub"}, []string{objs[0].obj.GetName(), objs[1].obj.GetName()})
 }
 
 func tempDir(t *testing.T) string {
@@ -2606,7 +2606,7 @@ func Test_findManifests(t *testing.T) {
 	t.Run("recursion when recursion is enabled", func(t *testing.T) {
 		recurse := argoappv1.ApplicationSourceDirectory{Recurse: true}
 		manifests, err := findManifests(logCtx, "./testdata/recurse", "./testdata/recurse", nil, recurse, nil, resource.MustParse("0"))
-		assert.Len(t, manifests, 6)
+		assert.Len(t, manifests, 4)
 		require.NoError(t, err)
 	})
 
@@ -2665,7 +2665,7 @@ func Test_findManifests(t *testing.T) {
 	t.Run("group of files should be limited at precisely the sum of their size", func(t *testing.T) {
 		// There is a total of 10 files, each file being 10 bytes.
 		manifests, err := findManifests(logCtx, "./testdata/several-files", "./testdata/several-files", nil, noRecurse, nil, resource.MustParse("365"))
-		assert.Len(t, manifests, 55)
+		assert.Len(t, manifests, 10)
 		require.NoError(t, err)
 
 		manifests, err = findManifests(logCtx, "./testdata/several-files", "./testdata/several-files", nil, noRecurse, nil, resource.MustParse("364"))
@@ -2676,7 +2676,7 @@ func Test_findManifests(t *testing.T) {
 	t.Run("jsonnet isn't counted against size limit", func(t *testing.T) {
 		// Each file is 36 bytes. Only the 36-byte json file should be counted against the limit.
 		manifests, err := findManifests(logCtx, "./testdata/jsonnet-and-json", "./testdata/jsonnet-and-json", nil, noRecurse, nil, resource.MustParse("36"))
-		assert.Len(t, manifests, 3)
+		assert.Len(t, manifests, 2)
 		require.NoError(t, err)
 
 		manifests, err = findManifests(logCtx, "./testdata/jsonnet-and-json", "./testdata/jsonnet-and-json", nil, noRecurse, nil, resource.MustParse("35"))