From 8bc0be7b753881e17cdbdd46da6bb6f827007489 Mon Sep 17 00:00:00 2001 From: Danilo Cianfrone Date: Thu, 21 Dec 2023 09:34:55 +0100 Subject: [PATCH] feat(skaffold/build): use new BazelConfig to pass args in bazel.ArtifactBuilder --- pkg/skaffold/build/bazel/build.go | 15 +++++++++++---- pkg/skaffold/build/bazel/build_test.go | 16 +++++++++------- pkg/skaffold/build/bazel/types.go | 13 +++++++++---- pkg/skaffold/build/local/types.go | 2 +- 4 files changed, 30 insertions(+), 16 deletions(-) diff --git a/pkg/skaffold/build/bazel/build.go b/pkg/skaffold/build/bazel/build.go index ec6db37142a..5f2c884fbb6 100644 --- 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 index ea2e2cca22e..7e09f15cf65 100644 --- 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,7 +87,7 @@ 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) @@ -95,6 +95,8 @@ func TestBuildBazelFailInvalidTarget(t *testing.T) { } 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( @@ -102,7 +104,7 @@ func TestBazelTarPath(t *testing.T) { 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 index e04fbe30f5b..94c86501936 100644 --- 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 index fef419ae43a..05207b92211 100644 --- 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