feat(skaffold/build): use new BazelConfig to pass args in bazel.Artif…

…actBuilder
ar3s3ru · Dec 21, 2023 · 8bc0be7 · 8bc0be7
1 parent ecf7199
commit 8bc0be7
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 16 deletions.
diff --git a/pkg/skaffold/build/bazel/build.go b/pkg/skaffold/build/bazel/build.go
@@ -51,7 +51,7 @@ func (b *Builder) Build(ctx context.Context, out io.Writer, artifact *latest.Art
 	}
 
 	if b.pushImages {
-		return docker.Push(tarPath, tag, b.cfg, nil)
+		return docker.Push(tarPath, tag, b.dockerCfg, nil)
 	}
 	return b.loadImage(ctx, out, tarPath, tag)
 }
@@ -67,6 +67,10 @@ func (b *Builder) buildTar(ctx context.Context, out io.Writer, workspace string,
 	args = append(args, a.BuildArgs...)
 	args = append(args, a.BuildTarget)
 
+	if b.cfg != nil {
+		args = append(args, b.cfg.BuildArgs...)
+	}
+
 	if output.IsColorable(out) {
 		args = append(args, "--color=yes")
 	} else {
@@ -82,7 +86,7 @@ func (b *Builder) buildTar(ctx context.Context, out io.Writer, workspace string,
 		return "", fmt.Errorf("running command: %w", err)
 	}
 
-	tarPath, err := bazelTarPath(ctx, workspace, a)
+	tarPath, err := b.bazelTarPath(ctx, workspace, a)
 	if err != nil {
 		return "", fmt.Errorf("getting bazel tar path: %w", err)
 	}
@@ -94,7 +98,6 @@ func (b *Builder) loadImage(ctx context.Context, out io.Writer, tarPath string,
 	manifest, err := tarball.LoadManifest(func() (io.ReadCloser, error) {
 		return os.Open(tarPath)
 	})
-
 	if err != nil {
 		return "", fmt.Errorf("loading manifest from tarball failed: %w", err)
 	}
@@ -118,7 +121,7 @@ func (b *Builder) loadImage(ctx context.Context, out io.Writer, tarPath string,
 	return imageID, nil
 }
 
-func bazelTarPath(ctx context.Context, workspace string, a *latest.BazelArtifact) (string, error) {
+func (b *Builder) bazelTarPath(ctx context.Context, workspace string, a *latest.BazelArtifact) (string, error) {
 	args := []string{
 		"cquery",
 		a.BuildTarget,
@@ -131,6 +134,10 @@ func bazelTarPath(ctx context.Context, workspace string, a *latest.BazelArtifact
 	}
 	args = append(args, a.BuildArgs...)
 
+	if b.cfg != nil {
+		args = append(args, b.cfg.BuildArgs...)
+	}
+
 	cmd := exec.CommandContext(ctx, "bazel", args...)
 	cmd.Dir = workspace
 

diff --git a/pkg/skaffold/build/bazel/build_test.go b/pkg/skaffold/build/bazel/build_test.go
@@ -36,7 +36,7 @@ func TestBuildBazel(t *testing.T) {
 		t.Override(&util.DefaultExecCommand, testutil.CmdRun("bazel build //:app.tar --color=no").AndRunOut(
 			"bazel cquery //:app.tar --output starlark --starlark:expr target.files.to_list()[0].path",
 			"bin/app.tar").AndRunOut("bazel info execution_root", ""))
-		testutil.CreateFakeImageTar("bazel:app", "bin/app.tar")
+		t.RequireNoError(testutil.CreateFakeImageTar("bazel:app", "bin/app.tar"))
 
 		artifact := &latest.Artifact{
 			Workspace: ".",
@@ -47,7 +47,7 @@ func TestBuildBazel(t *testing.T) {
 			},
 		}
 
-		builder := NewArtifactBuilder(fakeLocalDaemon(), &mockConfig{}, false)
+		builder := NewArtifactBuilder(&latest.BazelConfig{}, fakeLocalDaemon(), &mockConfig{}, false)
 		_, err := builder.Build(context.Background(), io.Discard, artifact, "img:tag", platform.Matcher{})
 
 		t.CheckNoError(err)
@@ -59,7 +59,7 @@ func TestBazelTarPathPrependExecutionRoot(t *testing.T) {
 		t.Override(&util.DefaultExecCommand, testutil.CmdRun("bazel build //:app.tar --color=no").AndRunOut(
 			"bazel cquery //:app.tar --output starlark --starlark:expr target.files.to_list()[0].path",
 			"app.tar").AndRunOut("bazel info execution_root", ".."))
-		testutil.CreateFakeImageTar("bazel:app", "../app.tar")
+		t.RequireNoError(testutil.CreateFakeImageTar("bazel:app", "../app.tar"))
 
 		artifact := &latest.Artifact{
 			Workspace: "..",
@@ -70,7 +70,7 @@ func TestBazelTarPathPrependExecutionRoot(t *testing.T) {
 			},
 		}
 
-		builder := NewArtifactBuilder(fakeLocalDaemon(), &mockConfig{}, false)
+		builder := NewArtifactBuilder(&latest.BazelConfig{}, fakeLocalDaemon(), &mockConfig{}, false)
 		_, err := builder.Build(context.Background(), io.Discard, artifact, "img:tag", platform.Matcher{})
 
 		t.CheckNoError(err)
@@ -87,22 +87,24 @@ func TestBuildBazelFailInvalidTarget(t *testing.T) {
 			},
 		}
 
-		builder := NewArtifactBuilder(nil, &mockConfig{}, false)
+		builder := NewArtifactBuilder(&latest.BazelConfig{}, nil, &mockConfig{}, false)
 		_, err := builder.Build(context.Background(), io.Discard, artifact, "img:tag", platform.Matcher{})
 
 		t.CheckErrorContains("the bazel build target should end with .tar", err)
 	})
 }
 
 func TestBazelTarPath(t *testing.T) {
+	builder := NewArtifactBuilder(&latest.BazelConfig{}, nil, &mockConfig{}, false)
+
 	testutil.Run(t, "EmptyExecutionRoot", func(t *testutil.T) {
 		osSpecificPath := filepath.Join("absolute", "path", "bin")
 		t.Override(&util.DefaultExecCommand, testutil.CmdRunOut(
 			"bazel cquery //:skaffold_example.tar --output starlark --starlark:expr target.files.to_list()[0].path --arg1 --arg2",
 			fmt.Sprintf("%s\n", osSpecificPath),
 		).AndRunOut("bazel info execution_root", ""))
 
-		bazelBin, err := bazelTarPath(context.Background(), ".", &latest.BazelArtifact{
+		bazelBin, err := builder.bazelTarPath(context.Background(), ".", &latest.BazelArtifact{
 			BuildArgs:   []string{"--arg1", "--arg2"},
 			BuildTarget: "//:skaffold_example.tar",
 		})
@@ -117,7 +119,7 @@ func TestBazelTarPath(t *testing.T) {
 			"bazel-bin/darwin-fastbuild-ST-confighash/path/to/bin\n",
 		).AndRunOut("bazel info execution_root", osSpecificPath))
 
-		bazelBin, err := bazelTarPath(context.Background(), ".", &latest.BazelArtifact{
+		bazelBin, err := builder.bazelTarPath(context.Background(), ".", &latest.BazelArtifact{
 			BuildArgs:   []string{"--arg1", "--arg2"},
 			BuildTarget: "//:skaffold_example.tar",
 		})

diff --git a/pkg/skaffold/build/bazel/types.go b/pkg/skaffold/build/bazel/types.go
@@ -16,20 +16,25 @@ limitations under the License.
 
 package bazel
 
-import "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/docker"
+import (
+	"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/docker"
+	"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/schema/latest"
+)
 
 // Builder is an artifact builder that uses Bazel
 type Builder struct {
+	cfg         *latest.BazelConfig
 	localDocker docker.LocalDaemon
-	cfg         docker.Config
+	dockerCfg   docker.Config
 	pushImages  bool
 }
 
 // NewArtifactBuilder returns a new bazel artifact builder
-func NewArtifactBuilder(localDocker docker.LocalDaemon, cfg docker.Config, pushImages bool) *Builder {
+func NewArtifactBuilder(cfg *latest.BazelConfig, localDocker docker.LocalDaemon, dockerCfg docker.Config, pushImages bool) *Builder {
 	return &Builder{
-		localDocker: localDocker,
 		cfg:         cfg,
+		localDocker: localDocker,
+		dockerCfg:   dockerCfg,
 		pushImages:  pushImages,
 	}
 }
diff --git a/pkg/skaffold/build/local/types.go b/pkg/skaffold/build/local/types.go
@@ -136,7 +136,7 @@ func newPerArtifactBuilder(b *Builder, a *latest.Artifact) (artifactBuilder, err
 		return dockerbuilder.NewArtifactBuilder(b.localDocker, b.cfg, b.local.UseDockerCLI, b.local.UseBuildkit, b.pushImages, b.artifactStore, b.sourceDependencies), nil
 
 	case a.BazelArtifact != nil:
-		return bazel.NewArtifactBuilder(b.localDocker, b.cfg, b.pushImages), nil
+		return bazel.NewArtifactBuilder(b.local.BazelConfig, b.localDocker, b.cfg, b.pushImages), nil
 
 	case a.JibArtifact != nil:
 		return jib.NewArtifactBuilder(b.localDocker, b.cfg, b.pushImages, b.skipTests, b.artifactStore), nil