From 4fba909a72fff8e1f98f73a8d31b29a0547fb5f4 Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Fri, 11 Aug 2023 16:06:24 -0400 Subject: [PATCH] Fix layer-related issues for `oci_rebase_image` Specifically: * Make sure we're using `platforms.OnlyStrict` instead of `platforms.Only` when constructing the platform matcher, because with `Only`, `arm/v8` will also match the other `arm/*` platforms, and, more immediately relevant, `amd64` will match the first item in the index with _either_ `amd64` or `386`. Which is...not what we want, obviously. * When generating the build files in `GenerateBuildFilesCmd`, don't add `oci_blob`s and traverse children for blobs which we've already added an `oci_blob` and traversed for. If there's an identical layer in more than one platform's image, and we've got `shallow` set to false (so that we're pulling layers as well as configs/indexes/manifests), the generated `BUILD.bazel` will end up invalid if we don't do this, because there'll be duplicate `oci_blob` rules (and duplicate rules for children, but in practice I didn't run into that because the only duplicates I hit in testing were layers). * Add the OCI base image name/digest annotations to the layers we're copying from the original image to the rebased image. `CopyContent` is leveraging those annotations to determine when we can avoid actually pushing a layer to the registry in favor of using `Mount`, and we're already doing that for the base image layers in `AppendLayers`. Note that there are still some interesting/annoying issues or quirks, and not just with `oci_rebase_image`: * With either `oci_image` or `oci_rebase_image` followed by an `oci_push`, if you're pushing to a different registry than the image(s) containing the pre-existing layers in the new image, you'll get a confusing error (`failed to push child content to registry: failed to create reader from ingestor: not found`). I think this is because the `Mount` fails, because you can't cross-repo mount a blob from a different registry host, and `oci_push` doesn't know where to find the layers from the pre-existing image(s) locally. I'm still trying to unravel that one, but am guessing it's happening because the layout passed to `ocitool push` doesn't contain the actual layers from the pre-existing image(s) and I'm not yet sure how to solve that. * With `oci_rebase_image`, the `oci_pull` for the original image _must_ have `shallow = False`, or `RebaseImage`'s call to `AppendLayers` can't locate the diff ID for the layers from the original image, for the fairly obvious in retrospect reason that...we're not pulling the layers unless we pass `shallow = False` to `oci_pull`. Oops. Signed-off-by: Andrew Bayer --- go/cmd/ocitool/desc_helpers.go | 2 +- go/pkg/layer/rebase.go | 26 ++++++++++++++++++++++++-- go/pkg/ociutil/bazel.go | 14 +++++++++++++- tests/go-multiarch-image/BUILD.bazel | 10 +++++----- tests/test_images.bzl | 24 +++++++++++++++++++----- 5 files changed, 62 insertions(+), 14 deletions(-) diff --git a/go/cmd/ocitool/desc_helpers.go b/go/cmd/ocitool/desc_helpers.go index 5968223..5e02c6c 100644 --- a/go/cmd/ocitool/desc_helpers.go +++ b/go/cmd/ocitool/desc_helpers.go @@ -78,7 +78,7 @@ func loadManifestForImage(ctx context.Context, allLocalProviders content.Provide OS: os, Architecture: arch, } - targetPlatformMatch := platforms.Only(targetPlatform) + targetPlatformMatch := platforms.OnlyStrict(targetPlatform) // Resolve the unknown descriptor into an image manifest, if an index // match the requested platform. diff --git a/go/pkg/layer/rebase.go b/go/pkg/layer/rebase.go index 3960b35..424d896 100644 --- a/go/pkg/layer/rebase.go +++ b/go/pkg/layer/rebase.go @@ -7,6 +7,7 @@ import ( "github.com/DataDog/rules_oci/go/pkg/ociutil" "github.com/containerd/containerd/content" + "github.com/containerd/containerd/images/converter" ocispec "github.com/opencontainers/image-spec/specs-go/v1" ) @@ -38,9 +39,30 @@ func RebaseImage(ctx context.Context, store content.Store, originalImageDesc oci } } - // Get the original image's layers after the old base image to append to the new base image. + // Get the original image's layers after the old base image to append to the new base image and add the original image + // as the OCI base image annotations to that layer, as is done for the layers from the base image in AppendLayers. var layersToAppend []ocispec.Descriptor - layersToAppend = append(layersToAppend, originalManifest.Layers[len(oldBaseManifest.Layers):]...) + origRef := originalImageDesc.Annotations[ocispec.AnnotationRefName] + + for _, origImgLayer := range originalManifest.Layers[len(oldBaseManifest.Layers):] { + // If we have an original image ref, set the OCI base image annotations on the original image layers. This allows + // ociutil.CopyContent to determine whether to copy the layer into the target repo via an OCI mount request. + if origRef != "" { + if origImgLayer.Annotations == nil { + origImgLayer.Annotations = make(map[string]string) + } + if _, ok := origImgLayer.Annotations[ocispec.AnnotationBaseImageName]; !ok { + origImgLayer.Annotations[ocispec.AnnotationBaseImageName] = origRef + } + + if _, ok := origImgLayer.Annotations[ocispec.AnnotationBaseImageDigest]; !ok { + origImgLayer.Annotations[ocispec.AnnotationBaseImageDigest] = originalImageDesc.Digest.String() + } + } + + origImgLayer.MediaType = converter.ConvertDockerMediaTypeToOCI(origImgLayer.MediaType) + layersToAppend = append(layersToAppend, origImgLayer) + } rebasedManifest, rebasedConfig, err := AppendLayers(ctx, store, newBaseImageDesc, layersToAppend, nil, originalImageConfig.Config.Labels, createdTimestamp, originalImageConfig.Config.Entrypoint) if err != nil { diff --git a/go/pkg/ociutil/bazel.go b/go/pkg/ociutil/bazel.go index 050477c..016321b 100644 --- a/go/pkg/ociutil/bazel.go +++ b/go/pkg/ociutil/bazel.go @@ -24,6 +24,9 @@ func GenerateBuildFilesHandler(handler images.HandlerFunc, layoutRoot string, pr // TODO Currently only supporting SHA256 blobBuildFiles[digest.SHA256] = rule.EmptyFile(algoBUILDPath(layoutRoot, digest.SHA256), "") + insertedBlobRules := make(map[digest.Algorithm]map[string]bool) + insertedBlobRules[digest.SHA256] = make(map[string]bool) + // Add load statements for all of the oci_* rules ldBlob := rule.NewLoad("@com_github_datadog_rules_oci//oci:blob.bzl") ldBlob.Add("oci_blob") @@ -65,7 +68,16 @@ func GenerateBuildFilesHandler(handler images.HandlerFunc, layoutRoot string, pr return nil, fmt.Errorf("no build file for algo '%v'", algo) } - // Insert a rule for each blob + if _, ok := insertedBlobRules[algo]; !ok { + return nil, fmt.Errorf("no inserted blob rules map for algo '%v'", algo) + } + // If we've already inserted this descriptor's blob rule, skip it and its children. + if _, ok := insertedBlobRules[algo][dgstToLabelName(desc.Digest)]; ok { + return nil, nil + } + insertedBlobRules[algo][dgstToLabelName(desc.Digest)] = true + + // Insert a rule for each blob. blobRuleFromDescriptor(desc).Insert(f) // if the manifest is an manifest or index, add an additional rule to diff --git a/tests/go-multiarch-image/BUILD.bazel b/tests/go-multiarch-image/BUILD.bazel index b93a972..79461f1 100644 --- a/tests/go-multiarch-image/BUILD.bazel +++ b/tests/go-multiarch-image/BUILD.bazel @@ -36,10 +36,10 @@ oci_push( oci_rebase_image( name = "rebase", - arch = "arm64", - new_base = "@ubuntu_jammy//image", - old_base = "@ubuntu_focal//image", - original = ":image", + arch = "amd64", + new_base = "@new-base-for-rebase//image", + old_base = "@old-base-for-rebase//image", + original = "@tekton-45//image", os = "linux", visibility = ["//visibility:public"], ) @@ -48,7 +48,7 @@ oci_push( name = "push-rebase", manifest = ":rebase", registry = "ghcr.io", - repository = "datadog/rules_oci/hello-world-rebase", + repository = "datadog/rules_oci/tekton-rebase", ) bzl_library( diff --git a/tests/test_images.bzl b/tests/test_images.bzl index 74d0a43..aea691c 100644 --- a/tests/test_images.bzl +++ b/tests/test_images.bzl @@ -9,11 +9,25 @@ def pull_test_images(): digest = "sha256:9d6a8699fb5c9c39cf08a0871bd6219f0400981c570894cd8cbea30d3424a31f", ) - # TODO(abayer): Temporarily using a public image I pushed to my own gcr.io repo. oci_pull( - name = "ubuntu_jammy", + name = "tekton-45", registry = "gcr.io", - repository = "abayer-jclouds-test1/rules_oci/ubuntu", - # Latest at "jammy" tag - digest = "sha256:99f98de8a0a27a7e1b3979238d17422ae3359573bda3beed0906da7e2d42e8c3", + repository = "tekton-releases/github.com/tektoncd/pipeline/cmd/controller", + # Latest at "v0.45.0" tag + digest = "sha256:8a302dab54484bbb83d46ff9455b077ea51c1c189641dcda12575f8301bfb257", + shallow = False, + ) + + oci_pull( + name = "old-base-for-rebase", + registry = "cgr.dev", + repository = "chainguard/static", + digest = "sha256:d9dd790fb308621ac4a5d648a852fbc455cda12f487eb30fb775a479c4f90703", + ) + + oci_pull( + name = "new-base-for-rebase", + registry = "cgr.dev", + repository = "chainguard/static", + digest = "sha256:76bde0b3719bbb65c1b39cd6c0f75fbbe0e24c115a40040ac50361cd8774d913", )