From 4040dee0eec3fe444fcf5f622492871541ac0e10 Mon Sep 17 00:00:00 2001 From: Michael Crenshaw Date: Tue, 24 May 2022 14:12:03 -0700 Subject: [PATCH] test: directory app manifest generation (#9503) * test: directory app manifest generation Signed-off-by: Michael Crenshaw * git doesn't support empty dirs Signed-off-by: Michael Crenshaw fix bad cherry-pick Signed-off-by: Michael Crenshaw --- reposerver/repository/repository.go | 2 +- reposerver/repository/repository_test.go | 121 ++++++++++++++++++ .../repository/testdata/circular-link/a.json | 1 + .../repository/testdata/circular-link/b.json | 1 + .../testdata/in-bounds-link/app/cm.link.yaml | 1 + .../testdata/in-bounds-link/cm.yaml | 2 + .../testdata/invalid-json/invalid.json | 1 + .../testdata/irrelevant-yaml/irrelevant.yaml | 1 + .../testdata/irrelevant-yaml/relevant.yaml | 2 + .../repository/testdata/json-list/list.json | 2 + .../testdata/link-to-nowhere/nowhere.json | 1 + .../non-manifest-file/not-json-or-yaml | 0 .../out-of-bounds-link/out-of-bounds.json | 1 + .../repository/testdata/out-of-bounds.json | 0 .../partially-valid-yaml/partially-valid.yaml | 4 + .../repository/testdata/valid-json/valid.json | 1 + .../yaml-with-empty-document/has-empty.yaml | 4 + 17 files changed, 144 insertions(+), 1 deletion(-) create mode 120000 reposerver/repository/testdata/circular-link/a.json create mode 120000 reposerver/repository/testdata/circular-link/b.json create mode 120000 reposerver/repository/testdata/in-bounds-link/app/cm.link.yaml create mode 100644 reposerver/repository/testdata/in-bounds-link/cm.yaml create mode 100644 reposerver/repository/testdata/invalid-json/invalid.json create mode 100644 reposerver/repository/testdata/irrelevant-yaml/irrelevant.yaml create mode 100644 reposerver/repository/testdata/irrelevant-yaml/relevant.yaml create mode 100644 reposerver/repository/testdata/json-list/list.json create mode 120000 reposerver/repository/testdata/link-to-nowhere/nowhere.json create mode 100644 reposerver/repository/testdata/non-manifest-file/not-json-or-yaml create mode 120000 reposerver/repository/testdata/out-of-bounds-link/out-of-bounds.json create mode 100644 reposerver/repository/testdata/out-of-bounds.json create mode 100644 reposerver/repository/testdata/partially-valid-yaml/partially-valid.yaml create mode 100644 reposerver/repository/testdata/valid-json/valid.json create mode 100644 reposerver/repository/testdata/yaml-with-empty-document/has-empty.yaml diff --git a/reposerver/repository/repository.go b/reposerver/repository/repository.go index 068b86a47a79..db7a5bb6b914 100644 --- a/reposerver/repository/repository.go +++ b/reposerver/repository/repository.go @@ -974,7 +974,7 @@ func findManifests(logCtx *log.Entry, appPath string, repoRoot string, env *v1al return fmt.Errorf("failed to evaluate symlink at %q: %w", relPath, err) } } - if !files.Inbound(realPath, appPath) { + if !files.Inbound(realPath, repoRoot) { logCtx.Warnf("illegal filepath in symlink: %s", realPath) return fmt.Errorf("illegal filepath in symlink at %q", relPath) } diff --git a/reposerver/repository/repository_test.go b/reposerver/repository/repository_test.go index 1ecaa7d0d230..0715c9f5f3fe 100644 --- a/reposerver/repository/repository_test.go +++ b/reposerver/repository/repository_test.go @@ -1704,6 +1704,127 @@ func TestFindManifests_Exclude_NothingMatches(t *testing.T) { []string{"nginx-deployment", "nginx-deployment-sub"}, []string{objs[0].GetName(), objs[1].GetName()}) } + + +func Test_findManifests(t *testing.T) { + logCtx := log.WithField("test", "test") + noRecurse := argoappv1.ApplicationSourceDirectory{Recurse: false} + + t.Run("unreadable file throws error", func(t *testing.T) { + appDir := t.TempDir() + unreadablePath := filepath.Join(appDir, "unreadable.json") + err := os.WriteFile(unreadablePath, []byte{}, 0666) + require.NoError(t, err) + err = os.Chmod(appDir, 0000) + require.NoError(t, err) + + manifests, err := findManifests(logCtx, appDir, appDir, nil, noRecurse) + assert.Empty(t, manifests) + assert.Error(t, err) + + // allow cleanup + err = os.Chmod(appDir, 0777) + if err != nil { + panic(err) + } + }) + + t.Run("no recursion when recursion is disabled", func(t *testing.T) { + manifests, err := findManifests(logCtx, "./testdata/recurse", "./testdata/recurse", nil, noRecurse) + assert.Len(t, manifests, 2) + assert.NoError(t, err) + }) + + 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) + assert.Len(t, manifests, 4) + assert.NoError(t, err) + }) + + t.Run("non-JSON/YAML is skipped", func(t *testing.T) { + manifests, err := findManifests(logCtx, "./testdata/non-manifest-file", "./testdata/non-manifest-file", nil, noRecurse) + assert.Empty(t, manifests) + assert.NoError(t, err) + }) + + t.Run("circular link should throw an error", func(t *testing.T) { + require.DirExists(t, "./testdata/circular-link") + manifests, err := findManifests(logCtx, "./testdata/circular-link", "./testdata/circular-link", nil, noRecurse) + assert.Empty(t, manifests) + assert.Error(t, err) + }) + + t.Run("out-of-bounds symlink should throw an error", func(t *testing.T) { + require.DirExists(t, "./testdata/out-of-bounds-link") + manifests, err := findManifests(logCtx, "./testdata/out-of-bounds-link", "./testdata/out-of-bounds-link", nil, noRecurse) + assert.Empty(t, manifests) + assert.Error(t, err) + }) + + t.Run("symlink to a regular file works", func(t *testing.T) { + repoRoot, err := filepath.Abs("./testdata/in-bounds-link") + require.NoError(t, err) + appPath, err := filepath.Abs("./testdata/in-bounds-link/app") + require.NoError(t, err) + manifests, err := findManifests(logCtx, appPath, repoRoot, nil, noRecurse) + assert.Len(t, manifests, 1) + assert.NoError(t, err) + }) + + t.Run("symlink to nowhere should be ignored", func(t *testing.T) { + manifests, err := findManifests(logCtx, "./testdata/link-to-nowhere", "./testdata/link-to-nowhere", nil, noRecurse) + assert.Empty(t, manifests) + assert.NoError(t, err) + }) + + t.Run("partially valid YAML file throws an error", func(t *testing.T) { + require.DirExists(t, "./testdata/partially-valid-yaml") + manifests, err := findManifests(logCtx, "./testdata/partially-valid-yaml", "./testdata/partially-valid-yaml", nil, noRecurse) + assert.Empty(t, manifests) + assert.Error(t, err) + }) + + t.Run("invalid manifest throws an error", func(t *testing.T) { + require.DirExists(t, "./testdata/invalid-manifests") + manifests, err := findManifests(logCtx, "./testdata/invalid-manifests", "./testdata/invalid-manifests", nil, noRecurse) + assert.Empty(t, manifests) + assert.Error(t, err) + }) + + t.Run("irrelevant YAML gets skipped, relevant YAML gets parsed", func(t *testing.T) { + manifests, err := findManifests(logCtx, "./testdata/irrelevant-yaml", "./testdata/irrelevant-yaml", nil, noRecurse) + assert.Len(t, manifests, 1) + assert.NoError(t, err) + }) + + t.Run("multiple JSON objects in one file throws an error", func(t *testing.T) { + require.DirExists(t, "./testdata/json-list") + manifests, err := findManifests(logCtx, "./testdata/json-list", "./testdata/json-list", nil, noRecurse) + assert.Empty(t, manifests) + assert.Error(t, err) + }) + + t.Run("invalid JSON throws an error", func(t *testing.T) { + require.DirExists(t, "./testdata/invalid-json") + manifests, err := findManifests(logCtx, "./testdata/invalid-json", "./testdata/invalid-json", nil, noRecurse) + assert.Empty(t, manifests) + assert.Error(t, err) + }) + + t.Run("valid JSON returns manifest and no error", func(t *testing.T) { + manifests, err := findManifests(logCtx, "./testdata/valid-json", "./testdata/valid-json", nil, noRecurse) + assert.Len(t, manifests, 1) + assert.NoError(t, err) + }) + + t.Run("YAML with an empty document doesn't throw an error", func(t *testing.T) { + manifests, err := findManifests(logCtx, "./testdata/yaml-with-empty-document", "./testdata/yaml-with-empty-document", nil, noRecurse) + assert.Len(t, manifests, 1) + assert.NoError(t, err) + }) +} + func TestTestRepoOCI(t *testing.T) { service := newService(".") _, err := service.TestRepository(context.Background(), &apiclient.TestRepositoryRequest{ diff --git a/reposerver/repository/testdata/circular-link/a.json b/reposerver/repository/testdata/circular-link/a.json new file mode 120000 index 000000000000..ce080bc5ebcc --- /dev/null +++ b/reposerver/repository/testdata/circular-link/a.json @@ -0,0 +1 @@ +b.json \ No newline at end of file diff --git a/reposerver/repository/testdata/circular-link/b.json b/reposerver/repository/testdata/circular-link/b.json new file mode 120000 index 000000000000..8eacec086127 --- /dev/null +++ b/reposerver/repository/testdata/circular-link/b.json @@ -0,0 +1 @@ +a.json \ No newline at end of file diff --git a/reposerver/repository/testdata/in-bounds-link/app/cm.link.yaml b/reposerver/repository/testdata/in-bounds-link/app/cm.link.yaml new file mode 120000 index 000000000000..ecb1855f4775 --- /dev/null +++ b/reposerver/repository/testdata/in-bounds-link/app/cm.link.yaml @@ -0,0 +1 @@ +../cm.yaml \ No newline at end of file diff --git a/reposerver/repository/testdata/in-bounds-link/cm.yaml b/reposerver/repository/testdata/in-bounds-link/cm.yaml new file mode 100644 index 000000000000..e4ec2f4d22c2 --- /dev/null +++ b/reposerver/repository/testdata/in-bounds-link/cm.yaml @@ -0,0 +1,2 @@ +apiVersion: v1 +kind: ServiceAccount \ No newline at end of file diff --git a/reposerver/repository/testdata/invalid-json/invalid.json b/reposerver/repository/testdata/invalid-json/invalid.json new file mode 100644 index 000000000000..558ed37d93c5 --- /dev/null +++ b/reposerver/repository/testdata/invalid-json/invalid.json @@ -0,0 +1 @@ +[ diff --git a/reposerver/repository/testdata/irrelevant-yaml/irrelevant.yaml b/reposerver/repository/testdata/irrelevant-yaml/irrelevant.yaml new file mode 100644 index 000000000000..2f0be7139258 --- /dev/null +++ b/reposerver/repository/testdata/irrelevant-yaml/irrelevant.yaml @@ -0,0 +1 @@ +some: [irrelevant, yaml] diff --git a/reposerver/repository/testdata/irrelevant-yaml/relevant.yaml b/reposerver/repository/testdata/irrelevant-yaml/relevant.yaml new file mode 100644 index 000000000000..c66c31b731c7 --- /dev/null +++ b/reposerver/repository/testdata/irrelevant-yaml/relevant.yaml @@ -0,0 +1,2 @@ +apiVersion: v1 +kind: ConfigMap diff --git a/reposerver/repository/testdata/json-list/list.json b/reposerver/repository/testdata/json-list/list.json new file mode 100644 index 000000000000..75da3b904143 --- /dev/null +++ b/reposerver/repository/testdata/json-list/list.json @@ -0,0 +1,2 @@ +{"apiVersion": "v1", "kind": "ConfigMap"} +{"apiVersion": "v1", "kind": "ConfigMap"} diff --git a/reposerver/repository/testdata/link-to-nowhere/nowhere.json b/reposerver/repository/testdata/link-to-nowhere/nowhere.json new file mode 120000 index 000000000000..5425ec0feb1e --- /dev/null +++ b/reposerver/repository/testdata/link-to-nowhere/nowhere.json @@ -0,0 +1 @@ +nowhere \ No newline at end of file diff --git a/reposerver/repository/testdata/non-manifest-file/not-json-or-yaml b/reposerver/repository/testdata/non-manifest-file/not-json-or-yaml new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/reposerver/repository/testdata/out-of-bounds-link/out-of-bounds.json b/reposerver/repository/testdata/out-of-bounds-link/out-of-bounds.json new file mode 120000 index 000000000000..828c1b4cd37d --- /dev/null +++ b/reposerver/repository/testdata/out-of-bounds-link/out-of-bounds.json @@ -0,0 +1 @@ +../out-of-bounds.json \ No newline at end of file diff --git a/reposerver/repository/testdata/out-of-bounds.json b/reposerver/repository/testdata/out-of-bounds.json new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/reposerver/repository/testdata/partially-valid-yaml/partially-valid.yaml b/reposerver/repository/testdata/partially-valid-yaml/partially-valid.yaml new file mode 100644 index 000000000000..a57a81a9dca5 --- /dev/null +++ b/reposerver/repository/testdata/partially-valid-yaml/partially-valid.yaml @@ -0,0 +1,4 @@ +apiVersion: v1 +kind: ConfigMap +--- +invalid: diff --git a/reposerver/repository/testdata/valid-json/valid.json b/reposerver/repository/testdata/valid-json/valid.json new file mode 100644 index 000000000000..dbcf15c7b9b0 --- /dev/null +++ b/reposerver/repository/testdata/valid-json/valid.json @@ -0,0 +1 @@ +{"apiVersion": "v1", "kind": "ConfigMap"} diff --git a/reposerver/repository/testdata/yaml-with-empty-document/has-empty.yaml b/reposerver/repository/testdata/yaml-with-empty-document/has-empty.yaml new file mode 100644 index 000000000000..20335c7ff509 --- /dev/null +++ b/reposerver/repository/testdata/yaml-with-empty-document/has-empty.yaml @@ -0,0 +1,4 @@ +apiVersion: v1 +kind: ConfigMap +--- +---