From 675e03c3ffd3332106d3fe18fe5411a8e891b527 Mon Sep 17 00:00:00 2001 From: Danilo Cianfrone Date: Tue, 12 Dec 2023 13:16:35 +0100 Subject: [PATCH 1/4] test: add integration test data for rules_oci multi-arch --- integration/run_test.go | 13 ++- .../bazel-rules-oci-multi-arch/.bazelrc | 1 + .../bazel-rules-oci-multi-arch/BUILD.bazel | 80 +++++++++++++++++++ .../bazel-rules-oci-multi-arch/MODULE.bazel | 22 +++++ .../bazel-rules-oci-multi-arch/WORKSPACE | 0 .../bazel-rules-oci-multi-arch/deploy.yaml | 25 ++++++ .../bazel-rules-oci-multi-arch/main.go | 7 ++ .../bazel-rules-oci-multi-arch/skaffold.yaml | 20 +++++ 8 files changed, 164 insertions(+), 4 deletions(-) create mode 100644 integration/testdata/bazel-rules-oci-multi-arch/.bazelrc create mode 100644 integration/testdata/bazel-rules-oci-multi-arch/BUILD.bazel create mode 100644 integration/testdata/bazel-rules-oci-multi-arch/MODULE.bazel create mode 100644 integration/testdata/bazel-rules-oci-multi-arch/WORKSPACE create mode 100644 integration/testdata/bazel-rules-oci-multi-arch/deploy.yaml create mode 100644 integration/testdata/bazel-rules-oci-multi-arch/main.go create mode 100644 integration/testdata/bazel-rules-oci-multi-arch/skaffold.yaml diff --git a/integration/run_test.go b/integration/run_test.go index 5409b8da40e..abc7dbb2678 100644 --- a/integration/run_test.go +++ b/integration/run_test.go @@ -115,6 +115,11 @@ var tests = []struct { args: []string{"-p", "target-with-package"}, deployments: []string{"helloweb"}, }, + { + description: "bazel oci with multi-arch image tarball", + dir: "testdata/bazel-rules-oci-multi-arch", + deployments: []string{"web"}, + }, { description: "jib", dir: "testdata/jib", @@ -198,7 +203,7 @@ func TestRun(t *testing.T) { ns, client := SetupNamespace(t) args := append(test.args, "--cache-artifacts=false") if test.dir == emptydir { - err := os.MkdirAll(filepath.Join(test.dir, "emptydir"), 0755) + err := os.MkdirAll(filepath.Join(test.dir, "emptydir"), 0o755) t.Log("Creating empty directory") if err != nil { t.Errorf("Error creating empty dir: %s", err) @@ -223,7 +228,7 @@ func TestRunTail(t *testing.T) { t.SkipNow() } if test.dir == emptydir { - err := os.MkdirAll(filepath.Join(test.dir, "emptydir"), 0755) + err := os.MkdirAll(filepath.Join(test.dir, "emptydir"), 0o755) t.Log("Creating empty directory") if err != nil { t.Errorf("Error creating empty dir: %s", err) @@ -250,7 +255,7 @@ func TestRunTailDefaultNamespace(t *testing.T) { t.SkipNow() } if test.dir == emptydir { - err := os.MkdirAll(filepath.Join(test.dir, "emptydir"), 0755) + err := os.MkdirAll(filepath.Join(test.dir, "emptydir"), 0o755) t.Log("Creating empty directory") if err != nil { t.Errorf("Error creating empty dir: %s", err) @@ -266,7 +271,7 @@ func TestRunTailDefaultNamespace(t *testing.T) { } func TestRunTailTolerateFailuresUntilDeadline(t *testing.T) { - var tsts = []struct { + tsts := []struct { description string dir string args []string diff --git a/integration/testdata/bazel-rules-oci-multi-arch/.bazelrc b/integration/testdata/bazel-rules-oci-multi-arch/.bazelrc new file mode 100644 index 00000000000..f8662a58745 --- /dev/null +++ b/integration/testdata/bazel-rules-oci-multi-arch/.bazelrc @@ -0,0 +1 @@ +common --enable-bzlmod diff --git a/integration/testdata/bazel-rules-oci-multi-arch/BUILD.bazel b/integration/testdata/bazel-rules-oci-multi-arch/BUILD.bazel new file mode 100644 index 00000000000..e63d32da7c1 --- /dev/null +++ b/integration/testdata/bazel-rules-oci-multi-arch/BUILD.bazel @@ -0,0 +1,80 @@ +load("@rules_go//go:def.bzl", "go_binary", "go_cross_binary", "go_library") +load("@rules_oci//oci:defs.bzl", "oci_image", "oci_image_index", "oci_tarball") +load("@rules_pkg//:pkg.bzl", "pkg_tar") + +go_library( + name = "app_lib", + srcs = ["main.go"], + importpath = "github.com/GoogleContainerTools/skaffold/v2/integration/testdata/bazel-rules-oci-multi-arch", + visibility = ["//visibility:private"], + deps = [], +) + +go_binary( + name = "app", + embed = [":app_lib"], + visibility = ["//visibility:public"], +) + +linux_amd64_binary = "app_linux-amd64" + +linux_amd64_layer = "{}_layer".format(linux_amd64_binary) + +linux_amd64_image = "{}_image".format(linux_amd64_binary) + +go_cross_binary( + name = linux_amd64_binary, + platform = "@rules_go//go/toolchain:linux_amd64", + target = target, +) + +pkg_tar( + name = linux_amd64_layer, + srcs = [linux_amd64_binary], +) + +oci_image( + name = linux_amd64_image, + base = base, + entrypoint = ["/app"], + tars = [linux_amd64_layer], +) + +linux_arm64_binary = "app_linux-arm64" + +linux_arm64_layer = "{}_layer".format(linux_arm64_binary) + +linux_arm64_image = "{}_image".format(linux_arm64_binary) + +go_cross_binary( + name = linux_arm64_binary, + platform = "@rules_go//go/toolchain:linux_arm64", + target = target, +) + +pkg_tar( + name = linux_arm64_layer, + srcs = [linux_arm64_binary], +) + +oci_image( + name = linux_arm64_image, + base = base, + entrypoint = ["/app"], + tars = [linux_arm64_layer], +) + +oci_image_index( + name = "image", + images = [ + linux_amd64_image, + linux_arm64_image, + ], +) + +oci_tarball( + name = "image.tar", + format = "oci", + image = ":image", + repo_tags = ["app:latest"], +) diff --git a/integration/testdata/bazel-rules-oci-multi-arch/MODULE.bazel b/integration/testdata/bazel-rules-oci-multi-arch/MODULE.bazel new file mode 100644 index 00000000000..29c0dc6ac22 --- /dev/null +++ b/integration/testdata/bazel-rules-oci-multi-arch/MODULE.bazel @@ -0,0 +1,22 @@ +module(name = "bazel-rules-oci-multi-arch") + +bazel_dep(name = "rules_go", version = "0.43.0") +bazel_dep(name = "rules_oci", version = "1.4.3") + +oci = use_extension("@rules_oci//oci:extensions.bzl", "oci") +oci.pull( + name = "distroless_base", + digest = "sha256:ccaef5ee2f1850270d453fdf700a5392534f8d1a8ca2acda391fbb6a06b81c86", + image = "gcr.io/distroless/base", + platforms = [ + "linux/amd64", + "linux/arm64", + ], +) +use_repo( + oci, + "distroless_base", +) + +go_sdk = use_extension("@rules_go//go:extensions.bzl", "go_sdk") +go_sdk.download(version = "1.21") diff --git a/integration/testdata/bazel-rules-oci-multi-arch/WORKSPACE b/integration/testdata/bazel-rules-oci-multi-arch/WORKSPACE new file mode 100644 index 00000000000..e69de29bb2d diff --git a/integration/testdata/bazel-rules-oci-multi-arch/deploy.yaml b/integration/testdata/bazel-rules-oci-multi-arch/deploy.yaml new file mode 100644 index 00000000000..1774a5bad1b --- /dev/null +++ b/integration/testdata/bazel-rules-oci-multi-arch/deploy.yaml @@ -0,0 +1,25 @@ +apiVersion: apps/v1 +kind: Deployment + +metadata: + name: &name hello + labels: + app: *name + +spec: + selector: + matchLabels: + app: *name + template: + metadata: + labels: + app: *name + spec: + containers: + - name: app + image: app + ports: + - containerPort: 8080 + resources: + requests: + cpu: 200m diff --git a/integration/testdata/bazel-rules-oci-multi-arch/main.go b/integration/testdata/bazel-rules-oci-multi-arch/main.go new file mode 100644 index 00000000000..b3afe954e2c --- /dev/null +++ b/integration/testdata/bazel-rules-oci-multi-arch/main.go @@ -0,0 +1,7 @@ +package main + +import "fmt" + +func main() { + fmt.Println("Hey, it's working!") +} diff --git a/integration/testdata/bazel-rules-oci-multi-arch/skaffold.yaml b/integration/testdata/bazel-rules-oci-multi-arch/skaffold.yaml new file mode 100644 index 00000000000..86916bdba31 --- /dev/null +++ b/integration/testdata/bazel-rules-oci-multi-arch/skaffold.yaml @@ -0,0 +1,20 @@ +apiVersion: skaffold/v4beta8 +kind: Config + +metadata: + name: hello + +build: + platforms: ["linux/amd64", "linux/arm64"] + artifacts: + - image: image + bazel: + target: //:image.tar + +deploy: + kubectl: {} + +manifests: + rawYaml: + - deploy.yaml + From c72784558a9d8abbf18494f47742b80cdc292194 Mon Sep 17 00:00:00 2001 From: Danilo Cianfrone Date: Tue, 12 Dec 2023 13:38:05 +0100 Subject: [PATCH 2/4] feat(build/bazel): remove restriction on '.tar' suffix --- pkg/skaffold/build/bazel/build.go | 6 ------ pkg/skaffold/build/bazel/build_test.go | 21 ++------------------- 2 files changed, 2 insertions(+), 25 deletions(-) diff --git a/pkg/skaffold/build/bazel/build.go b/pkg/skaffold/build/bazel/build.go index ec6db37142a..2587a8dbfd7 100644 --- a/pkg/skaffold/build/bazel/build.go +++ b/pkg/skaffold/build/bazel/build.go @@ -18,7 +18,6 @@ package bazel import ( "context" - "errors" "fmt" "io" "os" @@ -59,10 +58,6 @@ func (b *Builder) Build(ctx context.Context, out io.Writer, artifact *latest.Art func (b *Builder) SupportedPlatforms() platform.Matcher { return platform.All } func (b *Builder) buildTar(ctx context.Context, out io.Writer, workspace string, a *latest.BazelArtifact) (string, error) { - if !strings.HasSuffix(a.BuildTarget, ".tar") { - return "", errors.New("the bazel build target should end with .tar, see https://github.com/bazelbuild/rules_docker#using-with-docker-locally") - } - args := []string{"build"} args = append(args, a.BuildArgs...) args = append(args, a.BuildTarget) @@ -94,7 +89,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) } diff --git a/pkg/skaffold/build/bazel/build_test.go b/pkg/skaffold/build/bazel/build_test.go index ea2e2cca22e..fab06cc96ee 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.CheckNoError(testutil.CreateFakeImageTar("bazel:app", "bin/app.tar")) artifact := &latest.Artifact{ Workspace: ".", @@ -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.CheckNoError(testutil.CreateFakeImageTar("bazel:app", "../app.tar")) artifact := &latest.Artifact{ Workspace: "..", @@ -77,23 +77,6 @@ func TestBazelTarPathPrependExecutionRoot(t *testing.T) { }) } -func TestBuildBazelFailInvalidTarget(t *testing.T) { - testutil.Run(t, "", func(t *testutil.T) { - artifact := &latest.Artifact{ - ArtifactType: latest.ArtifactType{ - BazelArtifact: &latest.BazelArtifact{ - BuildTarget: "//:invalid-target", - }, - }, - } - - builder := NewArtifactBuilder(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) { testutil.Run(t, "EmptyExecutionRoot", func(t *testutil.T) { osSpecificPath := filepath.Join("absolute", "path", "bin") From 37123d4d6b08cb03465b075b3b093c6fcd2fd6ba Mon Sep 17 00:00:00 2001 From: Danilo Cianfrone Date: Tue, 12 Dec 2023 19:31:43 +0100 Subject: [PATCH 3/4] wip(build/bazel): add support to load image indexes --- pkg/skaffold/build/bazel/build.go | 174 ++++++++++++++++++++++++++++-- 1 file changed, 167 insertions(+), 7 deletions(-) diff --git a/pkg/skaffold/build/bazel/build.go b/pkg/skaffold/build/bazel/build.go index 2587a8dbfd7..60a96dce2f4 100644 --- a/pkg/skaffold/build/bazel/build.go +++ b/pkg/skaffold/build/bazel/build.go @@ -17,14 +17,20 @@ limitations under the License. package bazel import ( + "archive/tar" "context" + "encoding/json" + "errors" "fmt" "io" "os" "os/exec" + "path" "path/filepath" "strings" + v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/google/go-containerregistry/pkg/v1/layout" "github.com/google/go-containerregistry/pkg/v1/tarball" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/docker" @@ -49,10 +55,24 @@ func (b *Builder) Build(ctx context.Context, out io.Writer, artifact *latest.Art return "", err } - if b.pushImages { + tarballManifest, indexManifest, err := b.tarballOrIndexManifest(ctx, tarPath, tag) + if err != nil { + return "", err + } + + switch { + case tarballManifest != nil && b.pushImages: return docker.Push(tarPath, tag, b.cfg, nil) + case tarballManifest != nil && !b.pushImages: + return b.loadImage(ctx, out, tarballManifest, tarPath, tag) + case indexManifest != nil && b.pushImages: + // TODO: should push the image index using docker push + panic("implement me!") + case indexManifest != nil && !b.pushImages: + return b.loadImageIndex(ctx, out, indexManifest, tarPath, tag) + default: + return "", fmt.Errorf("unexpected state, neither manifest nor image index was found") } - return b.loadImage(ctx, out, tarPath, tag) } func (b *Builder) SupportedPlatforms() platform.Matcher { return platform.All } @@ -85,21 +105,78 @@ func (b *Builder) buildTar(ctx context.Context, out io.Writer, workspace string, return tarPath, nil } -func (b *Builder) loadImage(ctx context.Context, out io.Writer, tarPath string, tag string) (string, error) { - manifest, err := tarball.LoadManifest(func() (io.ReadCloser, error) { - return os.Open(tarPath) - }) +func (b *Builder) tarballOrIndexManifest(ctx context.Context, tarPath, tag string) (tarball.Manifest, *v1.IndexManifest, error) { + manifestFile, err := b.findFileInTar(tarPath, "manifest.json") + if err != nil && !errors.Is(err, errFileNotFound) { + return nil, nil, fmt.Errorf("load manifest from tarball failed: %w", err) + } + + if err == nil { + manifest, err := b.parseManifest(ctx, manifestFile) + return manifest, nil, err + } + + // NOTE: the index.json file needs to be extracted, as the current `layout` + // package does not allow for passing an io.Reader. + tmpDirPath, err := b.extractFileFromTar(tarPath, "index.json", tag) + if err != nil { + return nil, nil, fmt.Errorf("load image index from tarball failed: %w", err) + } + + lp, err := layout.ImageIndexFromPath(tmpDirPath) + if err != nil { + return nil, nil, fmt.Errorf("load image index from path failed: %w", err) + } + + manifest, err := lp.IndexManifest() if err != nil { - return "", fmt.Errorf("loading manifest from tarball failed: %w", err) + return nil, nil, fmt.Errorf("load index manifest from image index failed: %w", err) } + return nil, manifest, nil +} + +func (b *Builder) parseManifest(ctx context.Context, manifestFile io.ReadCloser) (tarball.Manifest, error) { + defer manifestFile.Close() + + var manifest tarball.Manifest + + if err := json.NewDecoder(manifestFile).Decode(&manifest); err != nil { + return nil, fmt.Errorf("parsing manifest file failed: %w", err) + } + + return manifest, nil +} + +func (b *Builder) loadImage(ctx context.Context, out io.Writer, manifest tarball.Manifest, tarPath, tag string) (string, error) { imageTar, err := os.Open(tarPath) if err != nil { return "", fmt.Errorf("opening image tarball: %w", err) } + defer imageTar.Close() bazelTag := manifest[0].RepoTags[0] + + return b.localDockerLoadImage(ctx, out, imageTar, bazelTag, tag) +} + +const ociImageRefName = "org.opencontainers.image.ref.name" + +func (b *Builder) loadImageIndex(ctx context.Context, out io.Writer, manifest *v1.IndexManifest, tarPath, tag string) (string, error) { + imageTar, err := os.Open(tarPath) + if err != nil { + return "", fmt.Errorf("opening image tarball: %w", err) + } + + defer imageTar.Close() + + bazelTag := manifest.Annotations[ociImageRefName] + + return b.localDockerLoadImage(ctx, out, imageTar, bazelTag, tag) +} + +func (b *Builder) localDockerLoadImage(ctx context.Context, out io.Writer, imageTar io.ReadCloser, bazelTag, tag string) (string, error) { imageID, err := b.localDocker.Load(ctx, out, imageTar, bazelTag) if err != nil { return "", fmt.Errorf("loading image into docker daemon: %w", err) @@ -147,3 +224,86 @@ func bazelTarPath(ctx context.Context, workspace string, a *latest.BazelArtifact return filepath.Join(execRoot, targetPath), nil } + +// tarFile represents a single file inside a tar. +// Closing it closes the tar itself. +type tarFile struct { + io.Reader + io.Closer +} + +var errFileNotFound = errors.New("bazel.findFileInTar: file not found in tar") + +// extractFileFromTar extracts the specified file in the path, into a temp folder. +// Returns the path of the temp folder, or an error if it occurred. +func (b *Builder) extractFileFromTar(tarPath, filePath, tag string) (string, error) { + file, err := b.findFileInTar(tarPath, filePath) + if err != nil { + return "", err + } + + defer file.Close() + + tmpDir, err := os.MkdirTemp("", fmt.Sprintf("%s-*", tag)) + if err != nil { + return "", nil + } + + extractedFilePath := path.Join(tmpDir, filePath) + + dest, err := os.Create(extractedFilePath) + if err != nil { + return "", fmt.Errorf("failed to build temp file to be extracted from tar: %w", err) + } + + defer dest.Close() + + if _, err := io.Copy(dest, file); err != nil { + return "", fmt.Errorf("failed to extract file from tar: %w", err) + } + + return tmpDir, nil +} + +// Copied from tarball.extractFileFromTar: +// https://github.com/google/go-containerregistry/blob/4fdaa32ee934cd178b6eb41b3096419a52ef426a/pkg/v1/tarball/image.go#L221-L255 +func (b *Builder) findFileInTar(tarPath, filePath string) (io.ReadCloser, error) { + f, err := os.Open(tarPath) + if err != nil { + return nil, fmt.Errorf("failed to open the tar file: %w", err) + } + + needClose := true + defer func() { + if needClose { + f.Close() + } + }() + + tf := tar.NewReader(f) + + for { + hdr, err := tf.Next() + if errors.Is(err, io.EOF) { + break + } else if err != nil { + return nil, err + } + + if hdr.Name == filePath { + if hdr.Typeflag == tar.TypeSymlink || hdr.Typeflag == tar.TypeLink { + currentDir := filepath.Dir(filePath) + return b.findFileInTar(tarPath, path.Join(currentDir, path.Clean(hdr.Linkname))) + } + + needClose = false + + return tarFile{ + Reader: tf, + Closer: f, + }, nil + } + } + + return nil, errFileNotFound +} From e2d3313f97b1f2f370fbb1c5a3a1deb28e270f47 Mon Sep 17 00:00:00 2001 From: Danilo Cianfrone Date: Tue, 12 Dec 2023 21:13:02 +0100 Subject: [PATCH 4/4] fix(build/bazel): remove unused params --- pkg/skaffold/build/bazel/build.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/skaffold/build/bazel/build.go b/pkg/skaffold/build/bazel/build.go index 60a96dce2f4..5a57326f23c 100644 --- a/pkg/skaffold/build/bazel/build.go +++ b/pkg/skaffold/build/bazel/build.go @@ -55,7 +55,7 @@ func (b *Builder) Build(ctx context.Context, out io.Writer, artifact *latest.Art return "", err } - tarballManifest, indexManifest, err := b.tarballOrIndexManifest(ctx, tarPath, tag) + tarballManifest, indexManifest, err := b.tarballOrIndexManifest(tarPath, tag) if err != nil { return "", err } @@ -105,14 +105,14 @@ func (b *Builder) buildTar(ctx context.Context, out io.Writer, workspace string, return tarPath, nil } -func (b *Builder) tarballOrIndexManifest(ctx context.Context, tarPath, tag string) (tarball.Manifest, *v1.IndexManifest, error) { +func (b *Builder) tarballOrIndexManifest(tarPath, tag string) (tarball.Manifest, *v1.IndexManifest, error) { manifestFile, err := b.findFileInTar(tarPath, "manifest.json") if err != nil && !errors.Is(err, errFileNotFound) { return nil, nil, fmt.Errorf("load manifest from tarball failed: %w", err) } if err == nil { - manifest, err := b.parseManifest(ctx, manifestFile) + manifest, err := b.parseManifest(manifestFile) return manifest, nil, err } @@ -136,7 +136,7 @@ func (b *Builder) tarballOrIndexManifest(ctx context.Context, tarPath, tag strin return nil, manifest, nil } -func (b *Builder) parseManifest(ctx context.Context, manifestFile io.ReadCloser) (tarball.Manifest, error) { +func (b *Builder) parseManifest(manifestFile io.ReadCloser) (tarball.Manifest, error) { defer manifestFile.Close() var manifest tarball.Manifest