From d3902b1cbada27d17ae290d1f80a123c55233ac5 Mon Sep 17 00:00:00 2001 From: Aran Donohue Date: Wed, 31 Jul 2024 09:51:36 -0700 Subject: [PATCH] fix: Allow MODULE.bazel for Bazel workspace root (#9445) Use bazel logic to determine bazel workspace root and ensure MODULE.bazel is added as a dep to Bazel targets if it is present. --- pkg/skaffold/build/bazel/dependencies.go | 56 +++++++++++-------- pkg/skaffold/build/bazel/dependencies_test.go | 25 ++++++--- pkg/skaffold/tag/custom_template_test.go | 6 +- 3 files changed, 56 insertions(+), 31 deletions(-) diff --git a/pkg/skaffold/build/bazel/dependencies.go b/pkg/skaffold/build/bazel/dependencies.go index b195132f4ca..6cb173a1b0a 100644 --- a/pkg/skaffold/build/bazel/dependencies.go +++ b/pkg/skaffold/build/bazel/dependencies.go @@ -18,7 +18,6 @@ package bazel import ( "context" - "errors" "fmt" "os" "os/exec" @@ -40,6 +39,8 @@ func query(target string) string { var once sync.Once +var workspaceFileCandidates = []string{"WORKSPACE", "WORKSPACE.bazel", "MODULE.bazel"} + // GetDependencies finds the sources dependencies for the given bazel artifact. // All paths are relative to the workspace. func GetDependencies(ctx context.Context, dir string, a *latest.BazelArtifact) ([]string, error) { @@ -51,14 +52,18 @@ func GetDependencies(ctx context.Context, dir string, a *latest.BazelArtifact) ( once.Do(func() { log.Entry(ctx).Warn("Retrieving Bazel dependencies can take a long time the first time") }) }() - workspaceDir, workspaceFile, err := findWorkspace(dir) + absDir, err := filepath.Abs(dir) if err != nil { - return nil, fmt.Errorf("unable to find the WORKSPACE file: %w", err) + return nil, fmt.Errorf("unable to find absolute path for %q: %w", dir, err) + } + absDir, err = filepath.EvalSymlinks(absDir) + if err != nil { + return nil, fmt.Errorf("unable to resolve symlinks in %q: %w", absDir, err) } - absDir, err := filepath.Abs(dir) + workspaceDir, workspaceFiles, err := findWorkspace(ctx, absDir) if err != nil { - return nil, fmt.Errorf("unable to find absolute path for %q: %w", dir, err) + return nil, fmt.Errorf("unable to find the WORKSPACE file: %w", err) } cmd := exec.CommandContext(ctx, "bazel", "query", query(a.BuildTarget), "--noimplicit_deps", "--order_output=no", "--output=label") @@ -82,17 +87,20 @@ func GetDependencies(ctx context.Context, dir string, a *latest.BazelArtifact) ( } rel, err := filepath.Rel(absDir, filepath.Join(workspaceDir, depToPath(l))) + if err != nil { return nil, fmt.Errorf("unable to find absolute path: %w", err) } deps = append(deps, rel) } - rel, err := filepath.Rel(absDir, filepath.Join(workspaceDir, workspaceFile)) - if err != nil { - return nil, fmt.Errorf("unable to find absolute path: %w", err) + for _, workspaceFile := range workspaceFiles { + rel, err := filepath.Rel(absDir, filepath.Join(workspaceDir, workspaceFile)) + if err != nil { + return nil, fmt.Errorf("unable to find absolute path: %w", err) + } + deps = append(deps, rel) } - deps = append(deps, rel) log.Entry(ctx).Debugf("Found dependencies for bazel artifact: %v", deps) @@ -103,25 +111,27 @@ func depToPath(dep string) string { return strings.TrimPrefix(strings.Replace(strings.TrimPrefix(dep, "//"), ":", "/", 1), "/") } -func findWorkspace(workingDir string) (string, string, error) { - dir, err := filepath.Abs(workingDir) +func findWorkspace(ctx context.Context, workingDir string) (string, []string, error) { + cmd := exec.CommandContext(ctx, "bazel", "info", "workspace") + cmd.Dir = workingDir + dirBytes, err := util.RunCmdOut(ctx, cmd) if err != nil { - return "", "", fmt.Errorf("invalid working dir: %w", err) + return "", nil, fmt.Errorf("getting bazel workspace: %w", err) } + dir := strings.TrimSpace(string(dirBytes)) - for { - if _, err := os.Stat(filepath.Join(dir, "WORKSPACE.bazel")); err == nil { - return dir, "WORKSPACE.bazel", nil - } + resolvedDir, err := filepath.EvalSymlinks(dir) + if err != nil { + return "", nil, fmt.Errorf("unable to resolve symlinks in %q: %w", dir, err) + } - if _, err := os.Stat(filepath.Join(dir, "WORKSPACE")); err == nil { - return dir, "WORKSPACE", nil - } + var workspaceFiles []string - parent := filepath.Dir(dir) - if parent == dir { - return "", "", errors.New("no WORKSPACE file found") + for _, workspaceFile := range workspaceFileCandidates { + if _, err := os.Stat(filepath.Join(resolvedDir, workspaceFile)); err == nil { + workspaceFiles = append(workspaceFiles, workspaceFile) } - dir = parent } + + return resolvedDir, workspaceFiles, nil } diff --git a/pkg/skaffold/build/bazel/dependencies_test.go b/pkg/skaffold/build/bazel/dependencies_test.go index 83c79286de0..8132166bc36 100644 --- a/pkg/skaffold/build/bazel/dependencies_test.go +++ b/pkg/skaffold/build/bazel/dependencies_test.go @@ -81,20 +81,18 @@ func TestGetDependencies(t *testing.T) { output: "@ignored\n//:BUILD\n//sub/folder:BUILD\n//external/ignored\n\n//sub/folder:dep1\n//sub/folder:dep2\n//sub/folder/baz:dep3\n", expected: []string{filepath.Join("..", "..", "BUILD"), "BUILD", "dep1", "dep2", filepath.Join("baz", "dep3"), filepath.Join("..", "..", "WORKSPACE")}, }, - { - description: "without WORKSPACE", - workspace: ".", - target: "target", - shouldErr: true, - }, } for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { + tmpDir := t.NewTempDir() t.Override(&util.DefaultExecCommand, testutil.CmdRunOut( + "bazel info workspace", + tmpDir.Root(), + ).AndRunOut( test.expectedQuery, test.output, )) - t.NewTempDir().WriteFiles(test.files).Chdir() + tmpDir.WriteFiles(test.files).Chdir() deps, err := GetDependencies(context.Background(), test.workspace, &latest.BazelArtifact{ BuildTarget: test.target, @@ -105,6 +103,19 @@ func TestGetDependencies(t *testing.T) { } } +func TestGetDependenciesWithNoWorkspace(t *testing.T) { + testutil.Run(t, "without WORKSPACE", func(t *testutil.T) { + tmpDir := t.NewTempDir() + + _, err := GetDependencies(context.Background(), tmpDir.Root(), &latest.BazelArtifact{ + BuildTarget: "target", + }) + + shouldErr := true + t.CheckError(shouldErr, err) + }) +} + func TestQuery(t *testing.T) { query := query("//:skaffold_example.tar") diff --git a/pkg/skaffold/tag/custom_template_test.go b/pkg/skaffold/tag/custom_template_test.go index 3b13e423448..7530809d9c5 100644 --- a/pkg/skaffold/tag/custom_template_test.go +++ b/pkg/skaffold/tag/custom_template_test.go @@ -149,12 +149,16 @@ func TestTagTemplate_GenerateTag(t *testing.T) { for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { t.Override(&util.OSEnviron, func() []string { return env }) + tmpDir := t.NewTempDir() t.Override(&util.DefaultExecCommand, testutil.CmdRunOut( + "bazel info workspace", + tmpDir.Root(), + ).AndRunOut( test.expectedQuery, test.output, )) - t.NewTempDir().WriteFiles(test.files).Chdir() + tmpDir.WriteFiles(test.files).Chdir() c, err := NewCustomTemplateTagger(runCtx, test.template, test.customMap) t.CheckNoError(err)