fix error when config file is from local relative directory (#350)

* fix local dir issue * add one unit test case
kubeflow · Jun 15, 2020 · 31fdc70 · 31fdc70
1 parent 7be0541
commit 31fdc70
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 6 deletions.
diff --git a/pkg/kfconfig/types.go b/pkg/kfconfig/types.go
@@ -504,12 +504,23 @@ func (c *KfConfig) SyncCache() error {
 		// Manifests are local dir
 		if fi, err := os.Stat(r.URI); err == nil && fi.Mode().IsDir() {
 			// check whether the cache directory is a sub directory of manifests
-			if relDir, err := filepath.Rel(r.URI, cacheDir); err != nil {
+			absCacheDir, err := filepath.Abs(cacheDir)
+			if err != nil {
 				return errors.WithStack(err)
-			} else {
-				if !strings.HasPrefix(relDir, ".."+string(filepath.Separator)) {
-					return errors.WithStack(errors.New("SyncCache: could not sync cache when the cache path " + cacheDir + " is sub directory of manifests " + r.URI))
-				}
+			}
+
+			absURI, err := filepath.Abs(r.URI)
+			if err != nil {
+				return errors.WithStack(err)
+			}
+
+			relDir, err := filepath.Rel(absURI, absCacheDir)
+			if err != nil {
+				return errors.WithStack(err)
+			}
+
+			if !strings.HasPrefix(relDir, ".."+string(filepath.Separator)) {
+				return errors.WithStack(errors.New("SyncCache: could not sync cache when the cache path " + cacheDir + " is sub directory of manifests " + r.URI))
 			}
 
 			if err := copy.Copy(r.URI, cacheDir); err != nil {

diff --git a/pkg/kfconfig/types_test.go b/pkg/kfconfig/types_test.go
@@ -102,6 +102,25 @@ func TestSyncCache(t *testing.T) {
 			expected:    nil,
 			expectedErr: errors.New("SyncCache: could not sync cache when the cache path " + path.Join(srcDir, "app1", ".cache", repoName) + " is sub directory of manifests " + srcDir),
 		},
+		{
+			input: &KfConfig{
+				Spec: KfConfigSpec{
+					AppDir: ".",
+					Repos: []Repo{{
+						Name: repoName,
+						URI:  srcDir,
+					},
+					},
+				},
+			},
+			expected: []Cache{
+				{
+					Name:      repoName,
+					LocalPath: path.Join(".cache", repoName),
+				},
+			},
+			expectedErr: nil,
+		},
 		{
 			input: &KfConfig{
 				Spec: KfConfigSpec{
@@ -162,8 +181,13 @@ func TestSyncCache(t *testing.T) {
 	for _, c := range testCases {
 		err = c.input.SyncCache()
 
+		// remove the local path for the test case whose AppDir is "."
+		if c.input.Spec.AppDir == "." {
+			os.RemoveAll(path.Join(".cache", repoName))
+		}
+
 		if err != nil {
-			if err.Error() != c.expectedErr.Error() {
+			if c.expectedErr == nil || err.Error() != c.expectedErr.Error() {
 				t.Fatalf("Could not sync cache; %v", err)
 			}
 		}