fix: Allow MODULE.bazel for Bazel workspace root (GoogleContainerTool…

…s#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.
renzodavid9 · Jul 31, 2024 · d3902b1 · d3902b1
1 parent 4497086
commit d3902b1
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 31 deletions.
diff --git 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
@@ -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
@@ -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)