From ecf7199663e6a25ee1d8523b0a0df53bbc5e443c Mon Sep 17 00:00:00 2001 From: Danilo Cianfrone Date: Thu, 21 Dec 2023 09:33:51 +0100 Subject: [PATCH 1/3] feat(skaffold/schema): add BazelConfig to latest.LocalBuild --- pkg/skaffold/schema/latest/config.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/pkg/skaffold/schema/latest/config.go b/pkg/skaffold/schema/latest/config.go index 7e1f34303ca..1d85160a572 100644 --- a/pkg/skaffold/schema/latest/config.go +++ b/pkg/skaffold/schema/latest/config.go @@ -327,6 +327,10 @@ type LocalBuild struct { // connects to a remote cluster. Push *bool `yaml:"push,omitempty"` + // BazelConfig contains some configuration to apply to `bazel` commands + // when building `BazelArtifact` targets locally. + BazelConfig *BazelConfig `yaml:"bazel,omitempty"` + // TryImportMissing whether to attempt to import artifacts from // Docker (either a local or remote registry) if not in the cache. TryImportMissing bool `yaml:"tryImportMissing,omitempty"` @@ -342,6 +346,16 @@ type LocalBuild struct { Concurrency *int `yaml:"concurrency,omitempty"` } +// BazelConfig contains some configuration to apply to `bazel` commands +// when building `BazelArtifact` targets locally. +type BazelConfig struct { + // BuildArgs are args to pass to `bazel build` for all the `bazel` targets + // in the build step. + // + // For example: `["-flag", "--otherflag"]`. + BuildArgs []string `yaml:"args,omitempty"` +} + // GoogleCloudBuild *beta* describes how to do a remote build on // [Google Cloud Build](https://cloud.google.com/cloud-build/docs/). // Docker and Jib artifacts can be built on Cloud Build. The `projectId` needs From 8bc0be7b753881e17cdbdd46da6bb6f827007489 Mon Sep 17 00:00:00 2001 From: Danilo Cianfrone Date: Thu, 21 Dec 2023 09:34:55 +0100 Subject: [PATCH 2/3] 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 From 6cbd3a376b4c1e8a0fa44daedcc41445de6bdb47 Mon Sep 17 00:00:00 2001 From: Danilo Cianfrone Date: Thu, 21 Dec 2023 10:49:21 +0100 Subject: [PATCH 3/3] fix: run make generate-schemas-v2 --- docs-v2/content/en/schemas/v4beta9.json | 29 +++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/docs-v2/content/en/schemas/v4beta9.json b/docs-v2/content/en/schemas/v4beta9.json index 0e8eedfac80..7d3e5984621 100755 --- a/docs-v2/content/en/schemas/v4beta9.json +++ b/docs-v2/content/en/schemas/v4beta9.json @@ -712,6 +712,29 @@ "description": "describes an artifact built with [Bazel](https://bazel.build/).", "x-intellij-html-description": "describes an artifact built with Bazel." }, + "BazelConfig": { + "properties": { + "args": { + "items": { + "type": "string" + }, + "type": "array", + "description": "args to pass to `bazel build` for all the `bazel` targets in the build step.", + "x-intellij-html-description": "args to pass to bazel build for all the bazel targets in the build step.", + "default": "[]", + "examples": [ + "[\"-flag\", \"--otherflag\"]" + ] + } + }, + "preferredOrder": [ + "args" + ], + "additionalProperties": false, + "type": "object", + "description": "contains some configuration to apply to `bazel` commands when building `BazelArtifact` targets locally.", + "x-intellij-html-description": "contains some configuration to apply to bazel commands when building BazelArtifact targets locally." + }, "BuildConfig": { "type": "object", "anyOf": [ @@ -3198,6 +3221,11 @@ }, "LocalBuild": { "properties": { + "bazel": { + "$ref": "#/definitions/BazelConfig", + "description": "contains some configuration to apply to `bazel` commands when building `BazelArtifact` targets locally.", + "x-intellij-html-description": "contains some configuration to apply to bazel commands when building BazelArtifact targets locally." + }, "concurrency": { "type": "integer", "description": "how many artifacts can be built concurrently. 0 means \"no-limit\".", @@ -3229,6 +3257,7 @@ }, "preferredOrder": [ "push", + "bazel", "tryImportMissing", "useDockerCLI", "useBuildkit",