Localize resources (#4912)

* Localize resources * Improve readability * Add integration tests * Group test helper functions
2026-05-17 18:25:26 +00:00 · 2023-01-05 11:47:59 -05:00
parent 6949c83840
commit 06da3b96a2
4 changed files with 466 additions and 25 deletions
--- a/api/internal/localizer/errors.go
+++ b/api/internal/localizer/errors.go
@@ -10,9 +10,18 @@ type ResourceLoadError struct {
 	FileError   error
 }

-var _ error = ResourceLoadError{}
-
 func (rle ResourceLoadError) Error() string {
 	return fmt.Sprintf(`when parsing as inline received error: %s
 when parsing as filepath received error: %s`, rle.InlineError, rle.FileError)
 }
+
+type PathLocalizeError struct {
+	Path      string
+	FileError error
+	RootError error
+}
+
+func (ple PathLocalizeError) Error() string {
+	return fmt.Sprintf(`could not localize path %q as file: %s; could not localize path %q as directory: %s`,
+		ple.Path, ple.FileError, ple.Path, ple.RootError)
+}
--- a/api/internal/localizer/localizer.go
+++ b/api/internal/localizer/localizer.go
@@ -148,11 +148,15 @@ func (lc *localizer) localizeNativeFields(kust *types.Kustomization) error {
 			kust.Crds,
 			lc.localizeFile,
 		},
+		"resources": {
+			kust.Resources,
+			lc.localizeResource,
+		},
 	} {
 		for i, path := range field.paths {
 			locPath, err := field.locFn(path)
 			if err != nil {
-				return errors.WrapPrefixf(err, "unable to localize %s path", fieldName)
+				return errors.WrapPrefixf(err, "unable to localize %s entry", fieldName)
 			}
 			field.paths[i] = locPath
 		}
@@ -198,8 +202,6 @@ func (lc *localizer) localizeNativeFields(kust *types.Kustomization) error {
 			kust.Replacements[i].Path = newPath
 		}
 	}
-
-	// TODO(annasong): localize all other kustomization fields: resources
 	return nil
 }

@@ -254,6 +256,39 @@ func (lc *localizer) localizePatches(patches []types.Patch) error {
 	return nil
 }

+// localizeResource localizes resource path, a file or root, and returns the
+// localized path
+func (lc *localizer) localizeResource(path string) (string, error) {
+	var locPath string
+
+	content, fileErr := lc.ldr.Load(path)
+	// The following check catches the case where path is a repo root.
+	// Load on a repo will successfully return its README in HTML.
+	// Because HTML does not follow resource formatting, we then correctly try
+	// to localize path as a root.
+	if fileErr == nil {
+		_, resErr := lc.rFactory.NewResMapFromBytes(content)
+		if resErr != nil {
+			fileErr = errors.WrapPrefixf(resErr, "invalid resource at file %q", path)
+		} else {
+			locPath, fileErr = lc.localizeFileWithContent(path, content)
+		}
+	}
+	if fileErr != nil {
+		var rootErr error
+		locPath, rootErr = lc.localizeDir(path)
+		if rootErr != nil {
+			err := PathLocalizeError{
+				Path:      path,
+				FileError: fileErr,
+				RootError: rootErr,
+			}
+			return "", err
+		}
+	}
+	return locPath, nil
+}
+
 // localizeFile localizes file path and returns the localized path
 func (lc *localizer) localizeFile(path string) (string, error) {
 	content, err := lc.ldr.Load(path)
@@ -267,8 +302,9 @@ func (lc *localizer) localizeFile(path string) (string, error) {
 func (lc *localizer) localizeFileWithContent(path string, content []byte) (string, error) {
 	var locPath string
 	if loader.IsRemoteFile(path) {
-		// TODO(annasong): You need to check if you can add a localize directory here to store
-		// the remote file content. There may be a directory that shares the localize directory name.
+		if lc.fSys.Exists(lc.root.Join(LocalizeDir)) {
+			return "", errors.Errorf("%s already contains %s needed to store file %q", lc.root, LocalizeDir, path)
+		}
 		locPath = locFilePath(path)
 	} else {
 		// ldr has checked that path must be relative; this is subject to change in beta.
@@ -306,8 +342,9 @@ func (lc *localizer) localizeDir(path string) (string, error) {
 	}
 	var locPath string
 	if repo := ldr.Repo(); repo != "" {
-		// TODO(annasong): You need to check if you can add a localize directory here to store
-		// the remote file content. There may be a directory that shares the localize directory name.
+		if lc.fSys.Exists(lc.root.Join(LocalizeDir)) {
+			return "", errors.Errorf("%s already contains %s needed to store root %q", lc.root, LocalizeDir, path)
+		}
 		locPath, err = locRootPath(path, repo, root, lc.fSys)
 		if err != nil {
 			return "", err
--- a/api/internal/localizer/localizer_test.go
+++ b/api/internal/localizer/localizer_test.go
@@ -1013,3 +1013,53 @@ nameSuffix: -test
 	}
 	checkLocalizeInTargetSuccess(t, kustAndComponents)
 }
+
+func TestLocalizeResources(t *testing.T) {
+	kustAndResources := map[string]string{
+		"kustomization.yaml": `apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+resources:
+- pod.yaml
+- ../../alpha
+`,
+		"pod.yaml": podConfiguration,
+		"../../alpha/kustomization.yaml": `apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+namePrefix: my-
+`,
+	}
+	expected, actual := makeFileSystems(t, "/a/b", kustAndResources)
+
+	err := Run("/a/b", "/", "", actual)
+	require.NoError(t, err)
+
+	addFiles(t, expected, "/localized-b/a/b", kustAndResources)
+	checkFSys(t, expected, actual)
+}
+
+func TestLocalizePathError(t *testing.T) {
+	kustAndResources := map[string]string{
+		"kustomization.yaml": `apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+resources:
+- b
+`,
+	}
+	expected, actual := makeFileSystems(t, "/a", kustAndResources)
+
+	err := Run("/a", "/", "", actual)
+
+	const expectedFileErr = `invalid file reference: '/a/b' must resolve to a file`
+	const expectedRootErr = `unable to localize root "b": unable to find one of 'kustomization.yaml', 'kustomization.yml' or 'Kustomization' in directory '/a/b'`
+	var actualErr PathLocalizeError
+	require.ErrorAs(t, err, &actualErr)
+	require.Equal(t, "b", actualErr.Path)
+	require.EqualError(t, actualErr.FileError, expectedFileErr)
+	require.EqualError(t, actualErr.RootError, expectedRootErr)
+
+	const expectedErrPrefix = `unable to localize target "/a": unable to localize resources entry`
+	require.EqualError(t, err, fmt.Sprintf(`%s: could not localize path "b" as file: %s; could not localize path "b" as directory: %s`,
+		expectedErrPrefix, expectedFileErr, expectedRootErr))
+
+	checkFSys(t, expected, actual)
+}
--- a/api/krusty/localizer/runner_test.go
+++ b/api/krusty/localizer/runner_test.go
@@ -141,9 +141,68 @@ const (
  }
 }
 `
+
+	simpleURL = "https://github.com/kubernetes-sigs/kustomize//api/krusty/testdata/localize/simple"
+
+	simpleKustomization = `apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+namePrefix: localize-
+resources:
+- deployment.yaml
+- service.yaml
+`
+
+	simpleDeployment = `apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: test-deployment-simple
+  labels:
+    app: deployment-simple
+spec:
+  selector:
+    matchLabels:
+      app: simple
+  template:
+    metadata:
+      labels:
+        app: simple
+    spec:
+      containers:
+      - name: nginx
+        image: nginx:1.16
+        ports:
+        - containerPort: 8080
+`
+	simpleService = `apiVersion: v1
+kind: Service
+metadata:
+  name: test-service-simple
+spec:
+  selector:
+    app: deployment-simple
+  ports:
+    - protocol: TCP
+      port: 80
+      targetPort: 8080 
+`
+
+	remoteHPA = `apiVersion: autoscaling/v2
+kind: HorizontalPodAutoscaler
+metadata:
+  name: hpa-deployment
+spec:
+  scaleTargetRef:
+    apiVersion: apps/v1
+    kind: Deployment
+    name: localize-test-deployment-simple
+  minReplicas: 1
+  maxReplicas: 10`
+
+	urlQuery = "?submodules=0&ref=kustomize/v4.5.7&timeout=300"
 )

-func prepareFs(t *testing.T, files map[string]string) (memoryFs filesys.FileSystem, actualFs filesys.FileSystem, testDir filesys.ConfirmedDir) {
+func prepareFs(t *testing.T, dirs []string, files map[string]string) (
+	memoryFs filesys.FileSystem, actualFs filesys.FileSystem, testDir filesys.ConfirmedDir) {
 	t.Helper()

 	memoryFs = filesys.MakeFsInMemory()
@@ -153,6 +212,9 @@ func prepareFs(t *testing.T, files map[string]string) (memoryFs filesys.FileSyst
 	require.NoError(t, err)

 	setupDir(t, memoryFs, testDir.String(), files)
+	for _, dirPath := range dirs {
+		require.NoError(t, actualFs.MkdirAll(testDir.Join(dirPath)))
+	}
 	setupDir(t, actualFs, testDir.String(), files)

 	t.Cleanup(func() {
@@ -170,20 +232,80 @@ func setupDir(t *testing.T, targetFs filesys.FileSystem, parentDir string, files
 	}
 }

-func getLocFilePath(t *testing.T, pathFromTestdata []string) string {
+func setWorkingDir(t *testing.T, workingDir string) {
 	t.Helper()

-	localizedPathDirs := []string{LocalizeDir, "raw.githubusercontent.com", "kubernetes-sigs", "kustomize",
-		"kustomize", "v4.5.7", "api", "krusty", "testdata"}
-	return filepath.Join(append(localizedPathDirs, pathFromTestdata...)...)
+	wd, err := os.Getwd()
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		require.NoError(t, os.Chdir(wd))
+	})
+
+	err = os.Chdir(workingDir)
+	require.NoError(t, err)
+}
+
+func link(t *testing.T, testDir filesys.ConfirmedDir, links map[string]string) {
+	t.Helper()
+
+	for newLink, file := range links {
+		require.NoError(t, os.Symlink(testDir.Join(file), testDir.Join(newLink)))
+	}
+}
+
+func simplePathAndFiles(t *testing.T) (locPath string, files map[string]string) {
+	t.Helper()
+
+	locPath = filepath.Join(LocalizeDir, "github.com",
+		"kubernetes-sigs", "kustomize", "kustomize", "v4.5.7",
+		"api", "krusty", "testdata", "localize", "simple")
+	files = map[string]string{
+		"kustomization.yaml": simpleKustomization,
+		"deployment.yaml":    simpleDeployment,
+		"service.yaml":       simpleService,
+	}
+	return
+}
+
+func remotePathAndFiles(t *testing.T) (locPath string, files map[string]string) {
+	t.Helper()
+
+	locPath = filepath.Join(LocalizeDir, "github.com",
+		"kubernetes-sigs", "kustomize", "master",
+		"api", "krusty", "testdata", "localize", "remote")
+	simplePath, simpleFiles := simplePathAndFiles(t)
+	files = map[string]string{
+		"kustomization.yaml": fmt.Sprintf(`apiVersion: kustomize.config.k8s.io/v1beta1
+commonLabels:
+  purpose: remoteReference
+kind: Kustomization
+resources:
+- %s
+- hpa.yaml
+`, simplePath),
+		"hpa.yaml": remoteHPA,
+	}
+	for path, content := range simpleFiles {
+		files[filepath.Join(simplePath, path)] = content
+	}
+	return
 }

 // checkFs checks fsActual, the real file system, against fsExpected, a file system in memory, for contents
-// in directory walkDir.
+// in directory walkDir. checkFs does not allow symlinks.
 func checkFs(t *testing.T, walkDir string, fsExpected filesys.FileSystem, fsActual filesys.FileSystem) {
 	t.Helper()

-	err := fsExpected.Walk(walkDir, func(path string, info fs.FileInfo, err error) error {
+	err := fsActual.Walk(walkDir, func(path string, info fs.FileInfo, err error) error {
+		require.NoError(t, err)
+
+		require.NotEqual(t, os.ModeSymlink, info.Mode()&os.ModeSymlink)
+		require.True(t, fsExpected.Exists(path), "unexpected file %q", path)
+		return nil
+	})
+	require.NoError(t, err)
+
+	err = fsExpected.Walk(walkDir, func(path string, info fs.FileInfo, err error) error {
 		require.NoError(t, err)

 		if info.IsDir() {
@@ -200,16 +322,129 @@ func checkFs(t *testing.T, walkDir string, fsExpected filesys.FileSystem, fsActu
 		return nil
 	})
 	require.NoError(t, err)
+}

-	err = fsActual.Walk(walkDir, func(path string, info fs.FileInfo, err error) error {
-		require.NoError(t, err)
+func TestWorkingDir(t *testing.T) {
+	files := map[string]string{
+		filepath.Join("target", "kustomization.yaml"): fmt.Sprintf(`resources:
+- %s
+`, filepath.Join("..", "base")),
+		filepath.Join("base", "kustomization.yaml"): `resources:
+- deployment.yaml
+`,
+		filepath.Join("base", "deployment.yaml"): simpleDeployment,
+	}
+	fsExpected, fsActual, wd := prepareFs(t, []string{"target", "base"}, files)
+	setWorkingDir(t, wd.String())

-		// no symlinks yet
-		require.NotEqual(t, os.ModeSymlink, info.Mode()&os.ModeSymlink)
-		require.True(t, fsExpected.Exists(path))
-		return nil
-	})
+	err := localizer.Run(fsActual, "target", ".", "")
 	require.NoError(t, err)
+
+	dst := wd.Join("localized-target")
+	setupDir(t, fsExpected, dst, files)
+	checkFs(t, dst, fsExpected, fsActual)
+}
+
+func TestSymlinks(t *testing.T) {
+	// test directory
+	// - link to target
+	// - link to base
+	// - link to file
+	// - target (and scope)
+	//   - link to kustomization
+	//   - base
+	//   - nested root
+	//     - file
+	//     - kustomization
+	fsExpected, fsActual, testDir := prepareFs(t, []string{"target",
+		filepath.Join("target", "base"),
+		filepath.Join("target", "nested")}, map[string]string{
+		filepath.Join("target", "base", "kustomization.yaml"): `namePrefix: test-
+`,
+		filepath.Join("target", "nested", "kustomization"): fmt.Sprintf(`resources:
+- %s
+- %s
+`, filepath.Join("..", "file-link"), filepath.Join("..", "base-link")),
+		filepath.Join("target", "nested", "file"): simpleDeployment,
+	})
+	link(t, testDir, map[string]string{
+		"target-link": "target",
+		"base-link":   filepath.Join("target", "base"),
+		"file-link":   filepath.Join("target", "nested", "file"),
+		filepath.Join("target", "kustomization.yaml"): filepath.Join("target", "nested", "kustomization"),
+	})
+	setWorkingDir(t, testDir.String())
+
+	err := localizer.Run(fsActual, "target-link", "target", "")
+	require.NoError(t, err)
+
+	dst := testDir.Join("localized-target")
+	setupDir(t, fsExpected, dst, map[string]string{
+		"kustomization.yaml": fmt.Sprintf(`resources:
+- %s
+- base
+`, filepath.Join("nested", "file")),
+		filepath.Join("base", "kustomization.yaml"): `namePrefix: test-
+`,
+		filepath.Join("nested", "file"): simpleDeployment,
+	})
+	checkFs(t, dst, fsExpected, fsActual)
+}
+
+func TestRemoteTargetDefaultDst(t *testing.T) {
+	fsExpected, fsActual, testDir := prepareFs(t, nil, nil)
+	setWorkingDir(t, testDir.String())
+
+	const target = simpleURL + urlQuery
+	err := localizer.Run(fsActual, target, "", "")
+	require.NoError(t, err)
+
+	dst := testDir.Join("localized-simple-kustomize-v4.5.7")
+	_, files := simplePathAndFiles(t)
+	setupDir(t, fsExpected,
+		filepath.Join(dst, "api", "krusty", "testdata", "localize", "simple"),
+		files)
+	checkFs(t, testDir.String(), fsExpected, fsActual)
+}
+
+func TestBadArgs(t *testing.T) {
+	badDst := filepath.Join("non-existing", "dst")
+
+	for name, test := range map[string]struct {
+		target string
+		scope  string
+		dst    string
+		err    string
+	}{
+		"target_no_ref": {
+			target: simpleURL,
+			err:    `localize remote root "https://github.com/kubernetes-sigs/kustomize//api/krusty/testdata/localize/simple" missing ref query string parameter`,
+		},
+		"non-empty_scope": {
+			target: simpleURL + urlQuery,
+			scope:  ".",
+			err:    fmt.Sprintf(`invalid localize scope ".": scope "." specified for remote localize target "%s"`, simpleURL+urlQuery),
+		},
+		"dst_in_non-existing_dir": {
+			target: ".",
+			dst:    badDst,
+			err:    fmt.Sprintf(`invalid localize destination "%s": unable to create localize destination directory: mkdir %s: no such file or directory`, badDst, badDst),
+		},
+	} {
+		t.Run(name, func(t *testing.T) {
+			kust := map[string]string{
+				"kustomization.yaml": "namePrefix: test-",
+			}
+			fsExpected, fsActual, testDir := prepareFs(t, nil, kust)
+			setWorkingDir(t, testDir.String())
+
+			err := localizer.Run(fsActual, test.target, test.scope, test.dst)
+			require.EqualError(t, err, test.err)
+
+			setupDir(t, fsExpected, testDir.String(), kust)
+			checkFs(t, testDir.String(), fsExpected, fsActual)
+		})
+	}
 }

 func TestRemoteFile(t *testing.T) {
@@ -218,7 +453,7 @@ kind: Kustomization
 openapi:
  path: %s
 `
-	fsExpected, fsActual, testDir := prepareFs(t, map[string]string{
+	fsExpected, fsActual, testDir := prepareFs(t, nil, map[string]string{
 		"kustomization.yaml": fmt.Sprintf(kustf, `https://raw.githubusercontent.com/kubernetes-sigs/kustomize/kustomize/v4.5.7/api/krusty/testdata/customschema.json`),
 	})

@@ -226,10 +461,120 @@ openapi:
 	err := localizer.Run(fsActual, testDir.String(), "", dst)
 	require.NoError(t, err)

-	localizedPath := getLocFilePath(t, []string{"customschema.json"})
+	localizedPath := filepath.Join(LocalizeDir, "raw.githubusercontent.com",
+		"kubernetes-sigs", "kustomize", "kustomize", "v4.5.7", "api", "krusty",
+		"testdata", "customschema.json")
 	setupDir(t, fsExpected, dst, map[string]string{
 		"kustomization.yaml": fmt.Sprintf(kustf, localizedPath),
 		localizedPath:        customSchema,
 	})
 	checkFs(t, testDir.String(), fsExpected, fsActual)
 }
+
+func TestRemoteRoot(t *testing.T) {
+	fsExpected, fsActual, testDir := prepareFs(t, nil, map[string]string{
+		"kustomization.yaml": fmt.Sprintf(`resources:
+- %s
+`, simpleURL+urlQuery),
+	})
+
+	dst := testDir.Join("dst")
+	err := localizer.Run(fsActual, testDir.String(), "", dst)
+	require.NoError(t, err)
+
+	localizedPath, files := simplePathAndFiles(t)
+	setupDir(t, fsExpected, dst, map[string]string{
+		"kustomization.yaml": fmt.Sprintf(`resources:
+- %s
+`, localizedPath),
+	})
+	setupDir(t, fsExpected, filepath.Join(dst, localizedPath), files)
+	checkFs(t, dst, fsExpected, fsActual)
+}
+
+func TestNestedRemoteRoots(t *testing.T) {
+	fsExpected, fsActual, testDir := prepareFs(t, nil, map[string]string{
+		// TODO(annasong): Change the ref to the release after kustomize/v4.5.7.
+		// We need changes to remote post-kustomize/v4.5.7.
+		"kustomization.yaml": `resources:
+- https://github.com/kubernetes-sigs/kustomize//api/krusty/testdata/localize/remote?submodules=0&ref=master&timeout=300
+`,
+	})
+
+	dst := testDir.Join("dst")
+	err := localizer.Run(fsActual, testDir.String(), "", dst)
+	require.NoError(t, err)
+
+	localizedPath, files := remotePathAndFiles(t)
+	setupDir(t, fsExpected, dst, map[string]string{
+		"kustomization.yaml": fmt.Sprintf(`resources:
+- %s
+`, localizedPath),
+	})
+	setupDir(t, fsExpected, filepath.Join(dst, localizedPath), files)
+	checkFs(t, dst, fsExpected, fsActual)
+}
+
+func TestResourcesRepoNotFile(t *testing.T) {
+	const repo = "https://github.com/kubernetes-sigs/kustomize" + urlQuery
+	kustomization := map[string]string{
+		"kustomization.yaml": fmt.Sprintf(`resources:
+- %s
+`, repo),
+	}
+	fsExpected, fsActual, testDir := prepareFs(t, nil, kustomization)
+
+	err := localizer.Run(fsActual, testDir.String(), "", testDir.Join("dst"))
+
+	const readmeErr = `yaml: line 28: mapping values are not allowed in this context`
+	fileErr := fmt.Sprintf(`invalid resource at file "%s": MalformedYAMLError: %s`, repo, readmeErr)
+	rootErr := fmt.Sprintf(`unable to localize root "%s": unable to find one of 'kustomization.yaml', 'kustomization.yml' or 'Kustomization'`, repo)
+	var actualErr PathLocalizeError
+	require.ErrorAs(t, err, &actualErr)
+	require.Equal(t, repo, actualErr.Path)
+	require.EqualError(t, actualErr.FileError, fileErr)
+	require.ErrorContains(t, actualErr.RootError, rootErr)
+
+	setupDir(t, fsExpected, testDir.String(), kustomization)
+	checkFs(t, testDir.String(), fsExpected, fsActual)
+}
+
+func TestRemoteRootNoRef(t *testing.T) {
+	const root = simpleURL + "?submodules=0&timeout=300"
+	kustomization := map[string]string{
+		"kustomization.yaml": fmt.Sprintf(`resources:
+- %s
+`, root),
+	}
+	fsExpected, fsActual, testDir := prepareFs(t, nil, kustomization)
+
+	err := localizer.Run(fsActual, testDir.String(), "", testDir.Join("dst"))
+
+	const fileErr = "invalid file reference: URL is a git repository"
+	rootErr := fmt.Sprintf(`localize remote root "%s" missing ref query string parameter`, root)
+	var actualErr PathLocalizeError
+	require.ErrorAs(t, err, &actualErr)
+	require.Equal(t, root, actualErr.Path)
+	require.EqualError(t, actualErr.FileError, fileErr)
+	require.EqualError(t, actualErr.RootError, rootErr)
+
+	setupDir(t, fsExpected, testDir.String(), kustomization)
+	checkFs(t, testDir.String(), fsExpected, fsActual)
+}
+
+func TestExistingCacheDir(t *testing.T) {
+	const remoteFile = `https://raw.githubusercontent.com/kubernetes-sigs/kustomize/master/api/krusty/testdata/localize/simple/deployment.yaml`
+	file := map[string]string{
+		"kustomization.yaml": fmt.Sprintf(`resources:
+- %s
+`, remoteFile),
+		filepath.Join(LocalizeDir, "file"): "existing",
+	}
+	fsExpected, fsActual, testDir := prepareFs(t, []string{LocalizeDir}, file)
+
+	err := localizer.Run(fsActual, testDir.String(), "", testDir.Join("dst"))
+	require.ErrorContains(t, err, fmt.Sprintf(`already contains localized-files needed to store file "%s"`, remoteFile))
+
+	setupDir(t, fsExpected, testDir.String(), file)
+	checkFs(t, testDir.String(), fsExpected, fsActual)
+}