From 3abe243f66d2f17299e2a5ec97367c0bd5297ed4 Mon Sep 17 00:00:00 2001 From: oleksandr-codefresh Date: Tue, 12 Nov 2024 21:02:31 +0200 Subject: [PATCH 1/2] reposerver(repository): fixed repeated resources generation for ApplicationSourceTypeDirectory --- changelog/CHANGELOG.md | 4 ++-- reposerver/repository/repository.go | 15 +++++++-------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/changelog/CHANGELOG.md b/changelog/CHANGELOG.md index ac73e1b0295b1..d21451b57c2d7 100644 --- a/changelog/CHANGELOG.md +++ b/changelog/CHANGELOG.md @@ -1,2 +1,2 @@ -### Fixed -- fix: failures in update revision for path should not affect sync \ No newline at end of file +### Changes +- fix(repo-server): fixed repeated resources generation for ApplicationSourceTypeDirectory \ No newline at end of file diff --git a/reposerver/repository/repository.go b/reposerver/repository/repository.go index 62b95137c439c..530a20f014d92 100644 --- a/reposerver/repository/repository.go +++ b/reposerver/repository/repository.go @@ -1812,13 +1812,6 @@ 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 { @@ -1826,7 +1819,11 @@ func findManifests(logCtx *log.Entry, appPath string, repoRoot string, env *v1al 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 From 97d9c2547afbd012f342db5ac6772ed581b82af2 Mon Sep 17 00:00:00 2001 From: oleksandr-codefresh Date: Wed, 13 Nov 2024 13:48:44 +0200 Subject: [PATCH 2/2] reposerver(repository): fixed unit tests after fix of repeated resources generation for ApplicationSourceTypeDirectory --- reposerver/repository/repository_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/reposerver/repository/repository_test.go b/reposerver/repository/repository_test.go index f99f26d6ec8f1..371f4fc523921 100644 --- a/reposerver/repository/repository_test.go +++ b/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"))