diff --git a/api/krusty/accumulation_test.go b/api/krusty/accumulation_test.go index c9c9703aba..e8c8b55523 100644 --- a/api/krusty/accumulation_test.go +++ b/api/krusty/accumulation_test.go @@ -4,12 +4,16 @@ package krusty_test import ( + "fmt" "path/filepath" + "regexp" "strings" "testing" + "github.com/stretchr/testify/require" . "sigs.k8s.io/kustomize/api/internal/target" "sigs.k8s.io/kustomize/api/konfig" + "sigs.k8s.io/kustomize/api/krusty" kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" ) @@ -160,3 +164,72 @@ spec: secretName: cert-tls `) } + +func TestAccumulateResourcesErrors(t *testing.T) { + type testcase struct { + name string + resource string + // regex error message that is output when kustomize tries to + // accumulate resource as file and dir, respectively + errFile, errDir string + } + buildError := func(tc testcase) string { + const ( + prefix = "accumulating resources" + filePrefixf = "accumulating resources from '%s'" + fileWrapperIfDirf = "accumulation err='%s'" + separator = ": " + ) + parts := []string{ + prefix, + strings.Join([]string{ + fmt.Sprintf(filePrefixf, regexp.QuoteMeta(tc.resource)), + tc.errFile, + }, separator), + } + if tc.errDir != "" { + parts[1] = fmt.Sprintf(fileWrapperIfDirf, parts[1]) + parts = append(parts, tc.errDir) + } + return strings.Join(parts, separator) + } + for _, test := range []testcase{ + { + name: "remote file not considered repo", + // This url is too short to be considered a remote repo. + resource: "https://raw.githubusercontent.com/kustomize", + // It is acceptable that the error for a remote file-like reference + // (that is not long enough to be considered a repo) does not + // indicate said reference is not a local directory. + // Though it is possible for the remote file-like reference to be + // a local directory, it is very unlikely. + errFile: `HTTP Error: status code 400 \(Bad Request\)\z`, + }, + { + name: "remote file qualifies as repo", + resource: "https://raw.githubusercontent.com/kubernetes-sigs/kustomize/kustomize/v5.0.0/examples/helloWorld/configMap", + // TODO(4788): This error message is technically wrong. Just + // because we fail to GET a reference does not mean the reference is + // not a remote file. We should return the GET status code instead. + errFile: "URL is a git repository", + errDir: `failed to run \S+/git fetch --depth=1 .+`, + }, + } { + t.Run(test.name, func(t *testing.T) { + // should use real file system to indicate that we are creating + // new temporary directories on disk when we attempt to fetch repos + fSys, tmpDir := kusttest_test.CreateKustDir(t, fmt.Sprintf(` +resources: +- %s +`, test.resource)) + b := krusty.MakeKustomizer(krusty.MakeDefaultOptions()) + _, err := b.Run(fSys, tmpDir.String()) + require.Regexp(t, buildError(test), err.Error()) + }) + } + // TODO(annasong): add tests that check accumulateResources errors for + // - local files + // - repos + // - local directories + // - files that yield malformed yaml errors +} diff --git a/api/krusty/remoteloader_test.go b/api/krusty/remoteloader_test.go index b0e456c8c8..2538fc92c2 100644 --- a/api/krusty/remoteloader_test.go +++ b/api/krusty/remoteloader_test.go @@ -10,7 +10,6 @@ import ( "io" "os" "os/exec" - "path/filepath" "strings" "testing" @@ -19,7 +18,7 @@ import ( "sigs.k8s.io/kustomize/api/krusty" "sigs.k8s.io/kustomize/api/loader" "sigs.k8s.io/kustomize/api/resmap" - "sigs.k8s.io/kustomize/kyaml/filesys" + kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" "sigs.k8s.io/kustomize/kyaml/yaml" ) @@ -264,7 +263,7 @@ resources: } kust := strings.ReplaceAll(test.kustomization, "$ROOT", repos.root) - fSys, tmpDir := createKustDir(t, kust) + fSys, tmpDir := kusttest_test.CreateKustDir(t, kust) b := krusty.MakeKustomizer(krusty.MakeDefaultOptions()) m, err := b.Run( @@ -368,7 +367,7 @@ resources: if test.beforeTest != nil { test.beforeTest(t) } - fSys, tmpDir := createKustDir(t, test.kustomization) + fSys, tmpDir := kusttest_test.CreateKustDir(t, test.kustomization) b := krusty.MakeKustomizer(krusty.MakeDefaultOptions()) m, err := b.Run( @@ -424,16 +423,6 @@ func configureGitSSHCommand(t *testing.T) { }) } -func createKustDir(t *testing.T, content string) (filesys.FileSystem, filesys.ConfirmedDir) { - t.Helper() - - fSys := filesys.MakeFsOnDisk() - tmpDir, err := filesys.NewTmpConfirmedDir() - require.NoError(t, err) - require.NoError(t, fSys.WriteFile(filepath.Join(tmpDir.String(), "kustomization.yaml"), []byte(content))) - return fSys, tmpDir -} - func checkYaml(t *testing.T, actual resmap.ResMap, expected string) { t.Helper() diff --git a/api/loader/fileloader_test.go b/api/loader/fileloader_test.go index 0cfb4679fe..df5729cfef 100644 --- a/api/loader/fileloader_test.go +++ b/api/loader/fileloader_test.go @@ -196,13 +196,31 @@ func TestLoaderBadRelative(t *testing.T) { require.Error(err) } -func TestLoaderMisc(t *testing.T) { - l := makeLoader() - _, err := l.New("") +func TestNewEmptyLoader(t *testing.T) { + _, err := makeLoader().New("") require.Error(t, err) +} - _, err = l.New("https://google.com/project") - require.Error(t, err) +func TestNewLoaderHTTP(t *testing.T) { + t.Run("doesn't exist", func(t *testing.T) { + _, err := makeLoader().New("https://google.com/project") + require.Error(t, err) + }) + // Though it is unlikely and we do not weigh this use case in our designs, + // it is possible for a reference that is considered a remote file to + // actually be a directory + t.Run("exists", func(t *testing.T) { + const remoteFileLikeRoot = "https://domain" + require.True(t, IsRemoteFile(remoteFileLikeRoot)) + fSys, dir := setupOnDisk(t) + require.NoError(t, fSys.MkdirAll(dir.Join("https:/domain"))) + ldr, err := newLoaderOrDie(RestrictionRootOnly, + fSys, + dir.String(), + ).New(remoteFileLikeRoot) + require.NoError(t, err) + require.Empty(t, ldr.Repo()) + }) } const ( @@ -212,17 +230,17 @@ const ( // Create a structure like this // -// /tmp/kustomize-test-random -// ├── base -// │ ├── okayData -// │ ├── symLinkToOkayData -> okayData -// │ └── symLinkToExteriorData -> ../exteriorData -// └── exteriorData -// +// /tmp/kustomize-test-random +// ├── base +// │ ├── okayData +// │ ├── symLinkToOkayData -> okayData +// │ └── symLinkToExteriorData -> ../exteriorData +// └── exteriorData func commonSetupForLoaderRestrictionTest(t *testing.T) (string, filesys.FileSystem) { t.Helper() - dir := t.TempDir() - fSys := filesys.MakeFsOnDisk() + fSys, tmpDir := setupOnDisk(t) + dir := tmpDir.String() + fSys.Mkdir(filepath.Join(dir, "base")) fSys.WriteFile( @@ -446,12 +464,8 @@ func TestLoaderDisallowsLocalBaseFromRemoteOverlay(t *testing.T) { } func TestLoaderDisallowsRemoteBaseExitRepo(t *testing.T) { - fSys := filesys.MakeFsOnDisk() - dir, err := filesys.NewTmpConfirmedDir() - require.NoError(t, err) - t.Cleanup(func() { - _ = fSys.RemoveAll(dir.String()) - }) + fSys, dir := setupOnDisk(t) + repo := dir.Join("repo") require.NoError(t, fSys.Mkdir(repo)) @@ -618,3 +632,17 @@ func TestLoaderHTTP(t *testing.T) { require.Error(err) } } + +// setupOnDisk sets up a file system on disk and directory that is cleaned after +// test completion. +func setupOnDisk(t *testing.T) (filesys.FileSystem, filesys.ConfirmedDir) { + t.Helper() + + fSys := filesys.MakeFsOnDisk() + dir, err := filesys.NewTmpConfirmedDir() + require.NoError(t, err) + t.Cleanup(func() { + _ = fSys.RemoveAll(dir.String()) + }) + return fSys, dir +} diff --git a/api/testutils/kusttest/ondisk.go b/api/testutils/kusttest/ondisk.go new file mode 100644 index 0000000000..f2bc2e8165 --- /dev/null +++ b/api/testutils/kusttest/ondisk.go @@ -0,0 +1,29 @@ +// Copyright 2023 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package kusttest_test + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + "sigs.k8s.io/kustomize/kyaml/filesys" +) + +// CreateKustDir creates a file system on disk and a new directory +// that holds a kustomization file with content. The directory is removed on +// test completion. +func CreateKustDir(t *testing.T, content string) (filesys.FileSystem, filesys.ConfirmedDir) { + t.Helper() + + fSys := filesys.MakeFsOnDisk() + tmpDir, err := filesys.NewTmpConfirmedDir() + require.NoError(t, err) + require.NoError(t, fSys.WriteFile(filepath.Join(tmpDir.String(), "kustomization.yaml"), []byte(content))) + + t.Cleanup(func() { + require.NoError(t, fSys.RemoveAll(tmpDir.String())) + }) + return fSys, tmpDir +}