From de8aff85c9090d37ed6fe436d696f3a820e5c767 Mon Sep 17 00:00:00 2001 From: Jason Hall Date: Mon, 16 Aug 2021 14:25:56 -0400 Subject: [PATCH] Implement annotation-based rebase hints (#960) * Implement annotation-based rebase hints `crane rebase` now inspects the original image for annotations to identify its old base and new base images. If those are found, and --old_base and --new_base flags aren't specified, those will be used. If the --rebased flag is not passed, the rebased image will be the tagged with the original image reference; if it was originally specified by digest, the rebased image will be tagged with :rebased (instead of stripping the digest and pushing to :latest) `crane rebase` now prints the full pushed image reference, instead of just the digest (aiding embedding in other bash pipelines), and adds annotations to aid future rebasings. This also adds a bash test that covers the rebase case for detected base image information. * rm rebase_cloudbuild.yaml, exercise :rebased tagging logic in rebase_test.sh * remove debug logging * use crane.Pull to honor flags * pick up improvements from crane flatten * keep --rebased for backcompat * review feedback * update codegen * remove Verify GitHub Action; it did nothing that Presubmit didn't do * update rebase.md docs --- .github/workflows/verify.yaml | 59 ---------- cmd/crane/cmd/rebase.go | 184 ++++++++++++++++++++++++++----- cmd/crane/doc/crane_rebase.md | 5 +- cmd/crane/rebase.md | 57 ++++++++-- cmd/crane/rebase_cloudbuild.yaml | 55 --------- cmd/crane/rebase_test.sh | 62 +++++++++++ 6 files changed, 270 insertions(+), 152 deletions(-) delete mode 100644 .github/workflows/verify.yaml delete mode 100644 cmd/crane/rebase_cloudbuild.yaml create mode 100755 cmd/crane/rebase_test.sh diff --git a/.github/workflows/verify.yaml b/.github/workflows/verify.yaml deleted file mode 100644 index b696f4903..000000000 --- a/.github/workflows/verify.yaml +++ /dev/null @@ -1,59 +0,0 @@ -name: Verify - -on: - pull_request: - branches: ['main'] - -jobs: - - verify: - name: Verify Deps and Codegen - runs-on: ubuntu-latest - - env: - GOPATH: ${{ github.workspace }} - - steps: - - uses: actions/setup-go@v2 - with: - go-version: 1.16.x - - name: Check out code onto GOPATH - uses: actions/checkout@v2 - with: - path: ./src/${{ github.event.repository.name }} - - - name: Install Dependencies - run: | - go get github.com/google/go-licenses - - - name: Update Codegen - shell: bash - run: | - pushd ./src/${{ github.event.repository.name }} - [[ ! -f hack/update-codegen.sh ]] || ./hack/update-codegen.sh - popd - - - name: Verify - shell: bash - run: | - # From: https://backreference.org/2009/12/23/how-to-match-newlines-in-sed/ - # This is to leverage this workaround: - # https://github.com/actions/toolkit/issues/193#issuecomment-605394935 - function urlencode() { - sed ':begin;$!N;s/\n/%0A/;tbegin' - } - - pushd ./src/${{ github.event.repository.name }} - if [[ -z "$(git status --porcelain)" ]]; then - echo "${{ github.repository }} up to date." - else - # Print it both ways because sometimes we haven't modified the file (e.g. zz_generated), - # and sometimes we have (e.g. configmap checksum). - echo "Found diffs in: $(git diff-index --name-only HEAD --)" - for x in $(git diff-index --name-only HEAD --); do - echo "::error file=$x::Please run ./hack/update-codegen.sh.%0A$(git diff $x | urlencode)" - done - echo "${{ github.repository }} is out of date. Please run hack/update-codegen.sh" - exit 1 - fi - popd diff --git a/cmd/crane/cmd/rebase.go b/cmd/crane/cmd/rebase.go index 8fe916fe0..bcb24b71d 100644 --- a/cmd/crane/cmd/rebase.go +++ b/cmd/crane/cmd/rebase.go @@ -15,10 +15,16 @@ package cmd import ( + "errors" "fmt" + "log" "github.com/google/go-containerregistry/pkg/crane" + "github.com/google/go-containerregistry/pkg/logs" + "github.com/google/go-containerregistry/pkg/name" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/mutate" + specsv1 "github.com/opencontainers/image-spec/specs-go/v1" "github.com/spf13/cobra" ) @@ -29,48 +35,176 @@ func NewCmdRebase(options *[]crane.Option) *cobra.Command { rebaseCmd := &cobra.Command{ Use: "rebase", Short: "Rebase an image onto a new base image", - Args: cobra.NoArgs, - RunE: func(*cobra.Command, []string) error { - origImg, err := crane.Pull(orig, *options...) - if err != nil { - return fmt.Errorf("pulling %s: %v", orig, err) + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if orig == "" { + orig = args[0] + } else if len(args) != 0 || args[0] != "" { + return fmt.Errorf("cannot use --original with positional argument") + } + + // If the new ref isn't provided, write over the original image. + // If that ref was provided by digest (e.g., output from + // another crane command), then strip that and push the + // mutated image by digest instead. + if rebased == "" { + rebased = orig } - oldBaseImg, err := crane.Pull(oldBase, *options...) + // Stupid hack to support insecure flag. + nameOpt := []name.Option{} + if ok, err := cmd.Parent().PersistentFlags().GetBool("insecure"); err != nil { + log.Fatalf("flag problems: %v", err) + } else if ok { + nameOpt = append(nameOpt, name.Insecure) + } + r, err := name.ParseReference(rebased, nameOpt...) if err != nil { - return fmt.Errorf("pulling %s: %v", oldBase, err) + log.Fatalf("parsing %s: %v", rebased, err) } - newBaseImg, err := crane.Pull(newBase, *options...) + desc, err := crane.Head(orig, *options...) if err != nil { - return fmt.Errorf("pulling %s: %v", newBase, err) + log.Fatalf("checking %s: %v", orig, err) + } + if !cmd.Parent().PersistentFlags().Changed("platform") && desc.MediaType.IsIndex() { + log.Fatalf("rebasing an index is not yet supported") } - img, err := mutate.Rebase(origImg, oldBaseImg, newBaseImg) + origImg, err := crane.Pull(orig, *options...) + if err != nil { + return err + } + origMf, err := origImg.Manifest() + if err != nil { + return err + } + anns := origMf.Annotations + if newBase == "" && anns != nil { + newBase = anns[specsv1.AnnotationBaseImageName] + } + if newBase == "" { + return errors.New("could not determine new base image from annotations") + } + newBaseRef, err := name.ParseReference(newBase) if err != nil { - return fmt.Errorf("rebasing: %v", err) + return err + } + if oldBase == "" && anns != nil { + oldBaseDigest := anns[specsv1.AnnotationBaseImageDigest] + oldBase = newBaseRef.Context().Digest(oldBaseDigest).String() + } + if oldBase == "" { + return errors.New("could not determine old base image by digest from annotations") } - if err := crane.Push(img, rebased, *options...); err != nil { - return fmt.Errorf("pushing %s: %v", rebased, err) + rebasedImg, err := rebaseImage(origImg, oldBase, newBase, *options...) + if err != nil { + return fmt.Errorf("rebasing image: %v", err) } - digest, err := img.Digest() + rebasedDigest, err := rebasedImg.Digest() if err != nil { - return fmt.Errorf("digesting rebased: %v", err) + return fmt.Errorf("digesting new image: %v", err) + } + origDigest, err := origImg.Digest() + if err != nil { + return err + } + if rebasedDigest == origDigest { + logs.Warn.Println("rebasing was no-op") + } + + if _, ok := r.(name.Digest); ok { + rebased = r.Context().Digest(rebasedDigest.String()).String() + } + logs.Progress.Println("pushing rebased image as", rebased) + if err := crane.Push(rebasedImg, rebased, *options...); err != nil { + log.Fatalf("pushing %s: %v", rebased, err) } - fmt.Println(digest.String()) + + fmt.Println(r.Context().Digest(rebasedDigest.String())) return nil }, } - rebaseCmd.Flags().StringVarP(&orig, "original", "", "", "Original image to rebase") - rebaseCmd.Flags().StringVarP(&oldBase, "old_base", "", "", "Old base image to remove") - rebaseCmd.Flags().StringVarP(&newBase, "new_base", "", "", "New base image to insert") - rebaseCmd.Flags().StringVarP(&rebased, "rebased", "", "", "Tag to apply to rebased image") - - rebaseCmd.MarkFlagRequired("original") - rebaseCmd.MarkFlagRequired("old_base") - rebaseCmd.MarkFlagRequired("new_base") - rebaseCmd.MarkFlagRequired("rebased") + rebaseCmd.Flags().StringVar(&orig, "original", "", "Original image to rebase (DEPRECATED: use positional arg instead)") + rebaseCmd.Flags().StringVar(&oldBase, "old_base", "", "Old base image to remove") + rebaseCmd.Flags().StringVar(&newBase, "new_base", "", "New base image to insert") + rebaseCmd.Flags().StringVar(&rebased, "rebased", "", "Tag to apply to rebased image (DEPRECATED: use --tag)") + rebaseCmd.Flags().StringVarP(&rebased, "tag", "t", "", "Tag to apply to rebased image") return rebaseCmd } + +// rebaseImage parses the references and uses them to perform a rebase on the +// original image. +// +// If oldBase or newBase are "", rebaseImage attempts to derive them using +// annotations in the original image. If those annotations are not found, +// rebaseImage returns an error. +// +// If rebasing is successful, base image annotations are set on the resulting +// image to facilitate implicit rebasing next time. +func rebaseImage(orig v1.Image, oldBase, newBase string, opt ...crane.Option) (v1.Image, error) { + m, err := orig.Manifest() + if err != nil { + return nil, err + } + if newBase == "" && m.Annotations != nil { + newBase = m.Annotations[specsv1.AnnotationBaseImageName] + if newBase != "" { + logs.Debug.Printf("Detected new base from %q annotation: %s", specsv1.AnnotationBaseImageName, newBase) + } + } + if newBase == "" { + return nil, fmt.Errorf("either new base or %q annotation is required", specsv1.AnnotationBaseImageName) + } + newBaseImg, err := crane.Pull(newBase, opt...) + if err != nil { + return nil, err + } + + if oldBase == "" && m.Annotations != nil { + oldBase = m.Annotations[specsv1.AnnotationBaseImageDigest] + if oldBase != "" { + newBaseRef, err := name.ParseReference(newBase) + if err != nil { + return nil, err + } + + oldBase = newBaseRef.Context().Digest(oldBase).String() + logs.Debug.Printf("Detected old base from %q annotation: %s", specsv1.AnnotationBaseImageDigest, oldBase) + } + } + if oldBase == "" { + return nil, fmt.Errorf("either old base or %q annotation is required", specsv1.AnnotationBaseImageDigest) + } + + oldBaseImg, err := crane.Pull(oldBase, opt...) + if err != nil { + return nil, err + } + + // NB: if newBase is an index, we need to grab the index's digest to + // annotate the resulting image, even though we pull the + // platform-specific image to rebase. + // crane.Digest will pull a platform-specific image, so use crane.Head + // here instead. + newBaseDesc, err := crane.Head(newBase, opt...) + if err != nil { + return nil, err + } + newBaseDigest := newBaseDesc.Digest.String() + + rebased, err := mutate.Rebase(orig, oldBaseImg, newBaseImg) + if err != nil { + return nil, err + } + + // Update base image annotations for the new image manifest. + logs.Debug.Printf("Setting annotation %q: %q", specsv1.AnnotationBaseImageDigest, newBaseDigest) + logs.Debug.Printf("Setting annotation %q: %q", specsv1.AnnotationBaseImageName, newBase) + return mutate.Annotations(rebased, map[string]string{ + specsv1.AnnotationBaseImageDigest: newBaseDigest, + specsv1.AnnotationBaseImageName: newBase, + }).(v1.Image), nil +} diff --git a/cmd/crane/doc/crane_rebase.md b/cmd/crane/doc/crane_rebase.md index 64eb4fbcd..0dd9e4827 100644 --- a/cmd/crane/doc/crane_rebase.md +++ b/cmd/crane/doc/crane_rebase.md @@ -12,8 +12,9 @@ crane rebase [flags] -h, --help help for rebase --new_base string New base image to insert --old_base string Old base image to remove - --original string Original image to rebase - --rebased string Tag to apply to rebased image + --original string Original image to rebase (DEPRECATED: use positional arg instead) + --rebased string Tag to apply to rebased image (DEPRECATED: use --tag) + -t, --tag string Tag to apply to rebased image ``` ### Options inherited from parent commands diff --git a/cmd/crane/rebase.md b/cmd/crane/rebase.md index 1eedf06e9..da4b25531 100644 --- a/cmd/crane/rebase.md +++ b/cmd/crane/rebase.md @@ -1,7 +1,7 @@ ### This code is experimental and might break you if not used correctly. The `rebase` command efficiently rewrites an image to replace the base image it -is FROM with a new base image. +is `FROM` with a new base image. ![rebase visualization](./rebase.png) @@ -48,20 +48,55 @@ layers in your image with the patched base image layers, without requiring a full rebuild from source. ``` -$ crane rebase \ - --original=my-app:latest \ +$ crane rebase my-app:latest \ --old_base=ubuntu@sha256:deadbeef... \ --new_base=ubuntu:latest \ - --rebased=my-app:rebased + --tag=my-app:rebased ``` This command: -1. fetches the manifest for `original`, `old_base` and `new_base` -1. checks that `old_base` is indeed the basis for `original` -1. removes `old_base`'s layers from `original` +1. fetches the manifest for the original image `my-app:latest`, and the + `old_base` and `new_base` images +1. checks that the original image is indeed based on `old_base` +1. removes `old_base`'s layers from the original image 1. replaces them with `new_base`'s layers -1. computes and uploads a new manifest for the image, tagged as `rebased`. +1. computes and uploads a new manifest for the image, tagged as `--tag`. + +If `--tag` is not specified, its value will be assumed to be the original +image's name. If the original image was specified by digest, the resulting +image will be pushed by digest only. + +`crane rebase` will print the rebased image name by digest to `stdout`. + +### Base Image Annotation Hints + +The OCI image spec includes some [standard image +annotations](https://github.com/opencontainers/image-spec/blob/main/annotations.md) +that can provide hints for the `--old_base` and `--new_base` flag values, so +these don't need to be specified: + +- **`org.opencontainers.image.base.digest`** specifies the original digest of + the base image +- **`org.opencontainers.image.base.name`** specifies the original base image's + reference + +If the original image has these annotations, you can omit the `--old_base` and +`--new_base` flags, and their values will be assumed to be: + +- `--old_base`: the `base.name` annotation value, plus the `base.digest` + annotation value +- `--new_base`: the `base.name` annotation value + +If these annotation values are invalid, and the flags aren't set, the operation +will fail. + +Whether or not the annotation values were set on the original image, they +_will_ be set on the resulting rebased image, to ease future rebase operations +on that image. + +`crane append` also supports the `--set-base-image-annotations` flag, which, if +true, will set these annotations on the resulting image. ## Caveats @@ -78,9 +113,9 @@ layers should expect from base layers. In the example above, for instance, we assume that the Ubuntu base image is adhering to some contract with downstream app layers, that it won't remove or drastically change what it provides to the app layer. If the `new_base` layers -removed some installed package, or made a breaking change to the version of some -compiler expected by the uppermost app layers, the resulting rebased image might -be invalid. +removed some installed package, or made a breaking change to the version of +some compiler expected by the uppermost app layers, the resulting rebased image +might be invalid. In general, it's a good practice to tag rebased images to some other tag than the `original` tag, perform some sanity checks, then tag the image to the diff --git a/cmd/crane/rebase_cloudbuild.yaml b/cmd/crane/rebase_cloudbuild.yaml deleted file mode 100644 index c1c7b5e28..000000000 --- a/cmd/crane/rebase_cloudbuild.yaml +++ /dev/null @@ -1,55 +0,0 @@ -steps: -# This test assumes that gcr.io/$PROJECT_ID/crane exists, as built according to -# this repo's cloudbuild.yaml - -# Run a simple example that builds two base images and an image based on one, -# then rebases it on the other. -- name: 'gcr.io/cloud-builders/docker' - entrypoint: 'bash' - args: - - -c - - | - # Build old base, rebase-base. - cat > Dockerfile.base << EOF - FROM ubuntu - RUN echo foo > /base.txt - EOF - docker build -t gcr.io/$PROJECT_ID/rebase-base -f Dockerfile.base . - - # Build image FROM base, base-test - cat > Dockerfile << EOF - FROM gcr.io/$PROJECT_ID/rebase-base - ENTRYPOINT ["cat", "/base.txt"] - EOF - docker build -t gcr.io/$PROJECT_ID/rebase-test . - - # Build new base, rebase-newbase. - cat > Dockerfile.newbase << EOF - FROM ubuntu - RUN echo bar > /base.txt - EOF - docker build -t gcr.io/$PROJECT_ID/rebase-newbase -f Dockerfile.newbase . - - # Check that image FROM base does what base would have it do. - docker run gcr.io/$PROJECT_ID/rebase-test | grep foo - - # Push all images. Rebasing is performed on images already in the registry. - docker push gcr.io/$PROJECT_ID/rebase-base - docker push gcr.io/$PROJECT_ID/rebase-test - docker push gcr.io/$PROJECT_ID/rebase-newbase - -# Perform rebase in the registry, producing image tagged :rebased. -- name: 'gcr.io/$PROJECT_ID/crane' - args: - - rebase - - --original=gcr.io/$PROJECT_ID/rebase-test:latest - - --old_base=gcr.io/$PROJECT_ID/rebase-base:latest - - --new_base=gcr.io/$PROJECT_ID/rebase-newbase:latest - - --rebased=gcr.io/$PROJECT_ID/rebase-test:rebased - -# Check that rebased image does what *newbase* would have it do. -- name: 'gcr.io/cloud-builders/docker' - entrypoint: 'bash' - args: - - -c - - docker run gcr.io/$PROJECT_ID/rebase-test:rebased | grep bar diff --git a/cmd/crane/rebase_test.sh b/cmd/crane/rebase_test.sh new file mode 100755 index 000000000..886f81c36 --- /dev/null +++ b/cmd/crane/rebase_test.sh @@ -0,0 +1,62 @@ +#!/bin/bash +set -ex + +tmp=$(mktemp -d) + +go install ./cmd/registry +go build -o ./crane ./cmd/crane + +# Start a local registry. +registry & +PID=$! +function cleanup { + kill $PID + rm -r ${tmp} + rm ./crane +} +trap cleanup EXIT + +sleep 1 # Wait for registry to be up. + +# Create an image localhost:1338/base containing a.txt +echo a > ${tmp}/a.txt +old_base=$(./crane append -f <(tar -f - -c ${tmp}) -t localhost:1338/base) +rm ${tmp}/a.txt + +# Append to that image localhost:1338/rebaseme +echo top > ${tmp}/top.txt +orig=$(./crane append -f <(tar -f - -c ${tmp}) -b ${old_base} -t localhost:1338/rebaseme) +rm ${tmp}/top.txt + +# Annotate that image as the base image (by ref and digest) +# TODO: do this with a flag to --append +orig=$(./crane mutate ${orig} \ + --annotation org.opencontainers.image.base.name=localhost:1338/base \ + --annotation org.opencontainers.image.base.digest=$(./crane digest localhost:1338/base)) + +# Update localhost:1338/base containing b.txt +echo b > ${tmp}/b.txt +new_base=$(./crane append -f <(tar -f - -c ${tmp}) -t localhost:1338/base) +rm ${tmp}/b.txt + +# Rebase using annotations +rebased=$(./crane rebase ${orig}) + +# List files in the rebased image. +./crane export ${rebased} - | tar -tvf - + +# Extract b.txt out of the rebased image. +./crane export ${rebased} - | tar -Oxf - ${tmp:1}/b.txt + +# Extract top.txt out of the rebased image. +./crane export ${rebased} - | tar -Oxf - ${tmp:1}/top.txt + +# a.txt is _not_ in the rebased image. +set +e +./crane export ${rebased} - | tar -Oxf - ${tmp:1}/a.txt # this should fail +code=$? +echo "finding a.txt exited ${code}" +if [[ $code -ne 1 ]]; then + echo "a.txt was found in rebased image" + exit 1 +fi