Repository: grafana/tanka
Pull request #872 (2023-06-01 19:19:28)

`tk tool importers`: Handle deleted files
One of the issues we are facing with this command is whenever someone deletes a file and it's still being used somewhere:
- We can't pass this file to the `importers` command because it will fail trying to find symlinks
- Jsonnet main files which are using this file will not be found
- We end up with erroring projects that are only found out the next time they are evaluated

This PR adds support for deleted files via a `deleted:` prefix that can be added to files that were deleted. In that case, a simple filepath.Abs logic is used instead of trying to find symlinks and the same import resolution process happens as with existing files

Ex:
```console
tk tool importers deleted:lib/external-secrets/main.libsonnet
```

github-actions[bot]: [PR Preview Action](https://github.com/rossjrw/pr-preview-action) v1.4.4
:---:
:rocket: Deployed preview to https://grafana.github.io/tanka/pr-preview/pr-872/
on branch [`gh-pages`](https://github.com/grafana/tanka/tree/gh-pages) at 2023-06-01 19:22 UTC
<!-- Sticky Pull Request Commentpr-preview -->

Patch:
diff --git a/cmd/tk/tool.go b/cmd/tk/tool.go
index 1e15e8d7..9ed56e65 100644
--- a/cmd/tk/tool.go
+++ b/cmd/tk/tool.go
@@ -137,9 +137,10 @@ func importsCmd() *cli.Command {
 
 func importersCmd() *cli.Command {
 	cmd := &cli.Command{
-		Use:   "importers <file> <file...>",
+		Use:   "importers <file> <file...> <deleted:file...>",
 		Short: "list all environments that either directly or transitively import the given files",
 		Long: `list all environments that either directly or transitively import the given files
+If the file being looked up was deleted, it should be prefixed with "deleted:".
 
 As optimization,
 if the file is not a vendored (located at <tk-root>/vendor/) or a lib file (located at <tk-root>/lib/), we assume:
@@ -162,6 +163,9 @@ if the file is not a vendored (located at <tk-root>/vendor/) or a lib file (loca
 		}
 
 		for _, f := range args {
+			if strings.HasPrefix(f, "deleted:") {
+				continue
+			}
 			if _, err := os.Stat(f); os.IsNotExist(err) {
 				return fmt.Errorf("file %q does not exist", f)
 			}
diff --git a/pkg/jsonnet/find_importers.go b/pkg/jsonnet/find_importers.go
index 0ca17101..4419f71a 100644
--- a/pkg/jsonnet/find_importers.go
+++ b/pkg/jsonnet/find_importers.go
@@ -1,6 +1,7 @@
 package jsonnet
 
 import (
+	"fmt"
 	"os"
 	"path/filepath"
 	"sort"
@@ -34,12 +35,33 @@ func FindImporterForFiles(root string, files []string) ([]string, error) {
 
 	importers := map[string]struct{}{}
 
-	if files, err = expandSymlinksInFiles(root, files); err != nil {
+	// Handle files prefixed with `deleted:`. They need to be made absolute and we shouldn't try to find symlinks for them
+	var filesToCheck, existingFiles []string
+	for _, file := range files {
+		if strings.HasPrefix(file, "deleted:") {
+			deletedFile := strings.TrimPrefix(file, "deleted:")
+			// Try with both the absolute path and the path relative to the root
+			if !filepath.IsAbs(deletedFile) {
+				absFilePath, err := filepath.Abs(deletedFile)
+				if err != nil {
+					return nil, err
+				}
+				filesToCheck = append(filesToCheck, absFilePath)
+				filesToCheck = append(filesToCheck, filepath.Clean(filepath.Join(root, deletedFile)))
+			}
+			continue
+		}
+
+		existingFiles = append(existingFiles, file)
+	}
+
+	if existingFiles, err = expandSymlinksInFiles(root, existingFiles); err != nil {
 		return nil, err
 	}
+	filesToCheck = append(filesToCheck, existingFiles...)
 
 	// Loop through all given files and add their importers to the list
-	for _, file := range files {
+	for _, file := range filesToCheck {
 		if filepath.Base(file) == jpath.DefaultEntrypoint {
 			importers[file] = struct{}{}
 		}
@@ -171,15 +193,14 @@ func findImporters(root string, searchForFile string, chain map[string]struct{})
 	searchedFileIsLibOrVendored := isFileLibOrVendored(searchForFile)
 	if !searchedFileIsLibOrVendored {
 		searchedDir := filepath.Dir(searchForFile)
-		searchedFileEntrypoint, err := jpath.Entrypoint(searchedDir)
-		if err == nil && searchedFileEntrypoint != "" {
+		if entrypoint := findEntrypoint(searchedDir); entrypoint != "" {
 			// Found the main file for the searched file, add it as an importer
-			importers = append(importers, searchedFileEntrypoint)
-		} else {
+			importers = append(importers, entrypoint)
+		} else if _, err := os.Stat(searchedDir); err == nil {
 			// No main file found, add all main files in child dirs as importers
 			files, err := FindFiles(searchedDir, nil)
 			if err != nil {
-				return nil, err
+				return nil, fmt.Errorf("failed to find files in %s: %w", searchedDir, err)
 			}
 			for _, file := range files {
 				if filepath.Base(file) == jpath.DefaultEntrypoint {
@@ -317,6 +338,21 @@ func createJsonnetFileCache(root string) (map[string]*cachedJsonnetFile, error)
 	return jsonnetFilesCache[root], nil
 }
 
+// findEntrypoint finds the nearest main.jsonnet file in the given file's directory or parent directories
+func findEntrypoint(searchedDir string) string {
+	for {
+		if _, err := os.Stat(searchedDir); err == nil {
+			break
+		}
+		searchedDir = filepath.Dir(searchedDir)
+	}
+	searchedFileEntrypoint, err := jpath.Entrypoint(searchedDir)
+	if err != nil {
+		return ""
+	}
+	return searchedFileEntrypoint
+}
+
 func pathMatches(path1, path2 string) bool {
 	if path1 == path2 {
 		return true


Test patch:
diff --git a/pkg/jsonnet/find_importers_test.go b/pkg/jsonnet/find_importers_test.go
index 4ba636ad..7b2f7c0a 100644
--- a/pkg/jsonnet/find_importers_test.go
+++ b/pkg/jsonnet/find_importers_test.go
@@ -3,6 +3,7 @@ package jsonnet
 import (
 	"fmt"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/grafana/tanka/pkg/jsonnet/jpath"
@@ -143,10 +144,64 @@ func findImportersTestCases(t testing.TB) []findImportersTestCase {
 				absPath(t, "testdata/findImporters/environments/lib-import-relative-to-env/folder1/folder2/main.jsonnet"),
 			},
 		},
+		{
+			name: "unused deleted file",
+			files: []string{
+				"deleted:testdata/findImporters/vendor/deleted-vendor/main.libsonnet",
+			},
+			expectedImporters: nil,
+		},
+		{
+			name: "deleted local path that is still potentially imported",
+			files: []string{
+				"deleted:testdata/findImporters/environments/using-deleted-stuff/my-import-dir/main.libsonnet",
+			},
+			expectedImporters: []string{
+				absPath(t, "testdata/findImporters/environments/using-deleted-stuff/main.jsonnet"),
+			},
+		},
+		{
+			name: "deleted lib that is still potentially imported",
+			files: []string{
+				"deleted:testdata/findImporters/lib/my-import-dir/main.libsonnet",
+			},
+			expectedImporters: []string{
+				absPath(t, "testdata/findImporters/environments/using-deleted-stuff/main.jsonnet"),
+			},
+		},
+		{
+			name: "deleted vendor that is still potentially imported",
+			files: []string{
+				"deleted:testdata/findImporters/vendor/my-import-dir/main.libsonnet",
+			},
+			expectedImporters: []string{
+				absPath(t, "testdata/findImporters/environments/using-deleted-stuff/main.jsonnet"),
+			},
+		},
+		{
+			name: "deleted lib that is still potentially imported, relative path from root",
+			files: []string{
+				"deleted:lib/my-import-dir/main.libsonnet",
+			},
+			expectedImporters: []string{
+				absPath(t, "testdata/findImporters/environments/using-deleted-stuff/main.jsonnet"),
+			},
+		},
+		{
+			// All files in an environment are considered to be imported by the main file, so the same should apply for deleted files
+			name: "deleted dir in environment",
+			files: []string{
+				"deleted:testdata/findImporters/environments/no-imports/deleted-dir/deleted-file.libsonnet",
+			},
+			expectedImporters: []string{
+				absPath(t, "testdata/findImporters/environments/no-imports/main.jsonnet"),
+			},
+		},
 	}
 }
 
 func TestFindImportersForFiles(t *testing.T) {
+	// Sanity check
 	// 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
@@ -154,6 +209,11 @@ func TestFindImportersForFiles(t *testing.T) {
 	require.NoError(t, err)
 	require.NotEmpty(t, files)
 	for _, file := range files {
+		// This project is known to be invalid (as the name suggests)
+		if strings.Contains(file, "using-deleted-stuff") {
+			continue
+		}
+
 		// Skip non-main files
 		if filepath.Base(file) != jpath.DefaultEntrypoint {
 			continue
diff --git a/pkg/jsonnet/testdata/findImporters/environments/using-deleted-stuff/main.jsonnet b/pkg/jsonnet/testdata/findImporters/environments/using-deleted-stuff/main.jsonnet
new file mode 100644
index 00000000..96567dc3
--- /dev/null
+++ b/pkg/jsonnet/testdata/findImporters/environments/using-deleted-stuff/main.jsonnet
@@ -0,0 +1,3 @@
+{
+  myimport: import 'my-import-dir/main.libsonnet',
+}