Skip to content

Commit

Permalink
tk tool importers: Handle deleted files (#872)
Browse files Browse the repository at this point in the history
* `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
```

* Rename var to make function clearer
  • Loading branch information
julienduchesne authored Jul 5, 2023
1 parent e264b4d commit e0bb1eb
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 8 deletions.
6 changes: 5 additions & 1 deletion cmd/tk/tool.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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)
}
Expand Down
50 changes: 43 additions & 7 deletions pkg/jsonnet/find_importers.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package jsonnet

import (
"fmt"
"os"
"path/filepath"
"sort"
Expand Down Expand Up @@ -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{}{}
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
60 changes: 60 additions & 0 deletions pkg/jsonnet/find_importers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package jsonnet
import (
"fmt"
"path/filepath"
"strings"
"testing"

"github.com/grafana/tanka/pkg/jsonnet/jpath"
Expand Down Expand Up @@ -143,17 +144,76 @@ 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
files, err := FindFiles("testdata", nil)
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
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
myimport: import 'my-import-dir/main.libsonnet',
}

0 comments on commit e0bb1eb

Please sign in to comment.