Find Importers: Add missing break in relative paths logic (#785)

* Find Importers: Add missing `break` in relative paths logic

Jsonnet is weird with relative paths
Given two envs in their own folder. The two following imports in `env1/main.jsonnet will work`: `../env2/main.jsonnet` and `../../env2/main.jsonnet`, so we have to try all of them

The logic does so in a loop. Without a `break`, the logic worked when the match was found on the first case (and correct one) but failed on the second

* Simpler logic + add test

* Add new test

pkg/jsonnet/find_importers.go

@@ -163,24 +163,23 @@ func findImporters(root string, searchForFile string, chain map[string]struct{})
 				continue
 			}
 
+			// Remove any `./` or `../` that can be removed just by looking at the given path
+			// ex: `./foo/bar.jsonnet` -> `foo/bar.jsonnet` or `/foo/../bar.jsonnet` -> `/bar.jsonnet`
+			importPath = filepath.Clean(importPath)
+
 			// Match on relative imports with ..
-			// Jsonnet also matches all intermediary paths for some reason, so we look at them too
-			doubleDotCount := strings.Count(importPath, "..")
-			if doubleDotCount > 0 {
-				importPath = strings.ReplaceAll(importPath, "../", "")
-				for i := 0; i <= doubleDotCount; i++ {
-					dir := filepath.Dir(jsonnetFilePath)
-					for j := 0; j < i; j++ {
-						dir = filepath.Dir(dir)
-					}
-					testImportPath := filepath.Join(dir, importPath)
-					isImporter = pathMatches(searchForFile, testImportPath)
-				}
+			// Jsonnet also matches relative imports that are one level deeper than they should be
+			// Example: Given two envs (env1 and env2), the two following imports in `env1/main.jsonnet will work`: `../env2/main.jsonnet` and `../../env2/main.jsonnet`
+			// This can lead to false positives, but ruling them out would require much more complex logic
+			if strings.HasPrefix(importPath, "..") {
+				shallowImport := filepath.Clean(filepath.Join(filepath.Dir(jsonnetFilePath), strings.Replace(importPath, "../", "", 1)))
+				importPath = filepath.Clean(filepath.Join(filepath.Dir(jsonnetFilePath), importPath))
+
+				isImporter = pathMatches(searchForFile, importPath) || pathMatches(searchForFile, shallowImport)
 			}
 
 			// Match on imports to lib/ or vendor/
 			if !isImporter {
-				importPath = strings.ReplaceAll(importPath, "./", "")
 				isImporter = pathMatches(searchForFile, filepath.Join(root, "vendor", importPath)) || pathMatches(searchForFile, filepath.Join(root, "lib", importPath))
 			}
 

pkg/jsonnet/find_importers_test.go

@@ -9,6 +9,17 @@ import (
 )
 
 func TestFindImportersForFiles(t *testing.T) {
+	// Make sure the main files all eval correctly
+	// We want to make sure that the importers command works correctly,
+	// but there's no point in testing on invalid jsonnet files
+	files, err := FindFiles("testdata", nil)
+	require.NoError(t, err)
+	require.NotEmpty(t, files)
+	for _, file := range files {
+		_, err := EvaluateFile(file, Opts{})
+		require.NoError(t, err, "failed to eval %s", file)
+	}
+
 	cases := []struct {
 		name              string
 		files             []string
@@ -73,6 +84,36 @@ func TestFindImportersForFiles(t *testing.T) {
 				absPath(t, "testdata/findImporters/environments/imports-symlinked-vendor/main.jsonnet"),
 			},
 		},
+		{
+			name:  "relative imported environment",
+			files: []string{"testdata/findImporters/environments/relative-imported/main.jsonnet"},
+			expectedImporters: []string{
+				absPath(t, "testdata/findImporters/environments/relative-import/main.jsonnet"),
+				absPath(t, "testdata/findImporters/environments/relative-imported/main.jsonnet"), // itself, it's a main file
+			},
+		},
+		{
+			name:  "relative imported environment with doubled '..'",
+			files: []string{"testdata/findImporters/environments/relative-imported2/main.jsonnet"},
+			expectedImporters: []string{
+				absPath(t, "testdata/findImporters/environments/relative-import/main.jsonnet"),
+				absPath(t, "testdata/findImporters/environments/relative-imported2/main.jsonnet"), // itself, it's a main file
+			},
+		},
+		{
+			name:  "relative imported text file",
+			files: []string{"testdata/findImporters/other-files/test.txt"},
+			expectedImporters: []string{
+				absPath(t, "testdata/findImporters/environments/relative-import/main.jsonnet"),
+			},
+		},
+		{
+			name:  "relative imported text file with doubled '..'",
+			files: []string{"testdata/findImporters/other-files/test2.txt"},
+			expectedImporters: []string{
+				absPath(t, "testdata/findImporters/environments/relative-import/main.jsonnet"),
+			},
+		},
 	}
 
 	for _, c := range cases {

pkg/jsonnet/testdata/findImporters/environments/relative-import/main.jsonnet

@@ -0,0 +1,8 @@
+{
+  // jsonnet supports going one level lower than files really are
+  first: import '../relative-imported/main.jsonnet',
+  second: import '../../relative-imported2/main.jsonnet',
+
+  externalFile: importstr '../../other-files/test.txt',
+  externalFile2: importstr '../../../other-files/test2.txt',
+}

pkg/jsonnet/testdata/findImporters/environments/relative-imported/main.jsonnet

@@ -0,0 +1 @@
+{}

pkg/jsonnet/testdata/findImporters/environments/relative-imported2/main.jsonnet

@@ -0,0 +1 @@
+{}

pkg/jsonnet/testdata/findImporters/other-files/test.txt

@@ -0,0 +1 @@
+test1

pkg/jsonnet/testdata/findImporters/other-files/test2.txt

@@ -0,0 +1 @@
+test2