From 217318c3b8e3aea04c135097619d83b22ed25fe2 Mon Sep 17 00:00:00 2001 From: Jason Hall Date: Wed, 19 Apr 2023 12:09:03 -0400 Subject: [PATCH] deprecate estargz (#1660) * deprecate estargz * also deprecate crane optimize * just delete 'crane optimize' and crane.Optimize --- README.md | 2 +- cmd/crane/cmd/optimize.go | 42 ------- cmd/crane/cmd/root.go | 1 - pkg/crane/crane_test.go | 3 - pkg/crane/optimize.go | 237 ------------------------------------- pkg/crane/optimize_test.go | 179 ---------------------------- pkg/v1/tarball/layer.go | 5 + 7 files changed, 6 insertions(+), 463 deletions(-) delete mode 100644 cmd/crane/cmd/optimize.go delete mode 100644 pkg/crane/optimize.go delete mode 100644 pkg/crane/optimize_test.go diff --git a/README.md b/README.md index c77c03ed6..f1d8fd130 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,7 @@ Over time, we will add new functionality under experimental environment variable | Env Var | Value(s) | What is does | |---------|----------|--------------| -| `GGCR_EXPERIMENT_ESTARGZ` | `"1"` | When enabled this experiment will direct `tarball.LayerFromOpener` to emit [estargz](https://github.com/opencontainers/image-spec/issues/815) compatible layers, which enable them to be lazily loaded by an appropriately configured containerd. | +| `GGCR_EXPERIMENT_ESTARGZ` | `"1"` | ⚠️DEPRECATED⚠️: When enabled this experiment will direct `tarball.LayerFromOpener` to emit [estargz](https://github.com/opencontainers/image-spec/issues/815) compatible layers, which enable them to be lazily loaded by an appropriately configured containerd. | ### `v1.Image` diff --git a/cmd/crane/cmd/optimize.go b/cmd/crane/cmd/optimize.go deleted file mode 100644 index 89c470664..000000000 --- a/cmd/crane/cmd/optimize.go +++ /dev/null @@ -1,42 +0,0 @@ -// Copyright 2020 Google LLC All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package cmd - -import ( - "github.com/google/go-containerregistry/pkg/crane" - "github.com/spf13/cobra" -) - -// NewCmdOptimize creates a new cobra.Command for the optimize subcommand. -func NewCmdOptimize(options *[]crane.Option) *cobra.Command { - var files []string - - cmd := &cobra.Command{ - Use: "optimize SRC DST", - Hidden: true, - Aliases: []string{"opt"}, - Short: "Optimize a remote container image from src to dst", - Args: cobra.ExactArgs(2), - RunE: func(_ *cobra.Command, args []string) error { - src, dst := args[0], args[1] - return crane.Optimize(src, dst, files, *options...) - }, - } - - cmd.Flags().StringSliceVar(&files, "prioritize", nil, - "The list of files to prioritize in the optimized image.") - - return cmd -} diff --git a/cmd/crane/cmd/root.go b/cmd/crane/cmd/root.go index c6ca2b44b..3faa85f5f 100644 --- a/cmd/crane/cmd/root.go +++ b/cmd/crane/cmd/root.go @@ -123,7 +123,6 @@ func New(use, short string, options []crane.Option) *cobra.Command { NewCmdList(&options), NewCmdManifest(&options), NewCmdMutate(&options), - NewCmdOptimize(&options), NewCmdPull(&options), NewCmdPush(&options), NewCmdRebase(&options), diff --git a/pkg/crane/crane_test.go b/pkg/crane/crane_test.go index 9f4d124b3..63dcc566a 100644 --- a/pkg/crane/crane_test.go +++ b/pkg/crane/crane_test.go @@ -549,9 +549,6 @@ func TestBadInputs(t *testing.T) { {"Tag(invalid, invalid)", crane.Tag(invalid, invalid)}, {"Tag(404, invalid)", crane.Tag(valid404, invalid)}, {"Tag(404, 404)", crane.Tag(valid404, valid404)}, - {"Optimize(invalid, invalid)", crane.Optimize(invalid, invalid, []string{})}, - {"Optimize(404, invalid)", crane.Optimize(valid404, invalid, []string{})}, - {"Optimize(404, 404)", crane.Optimize(valid404, valid404, []string{})}, // These return multiple values, which are hard to use as expressions. {"Pull(invalid)", e(crane.Pull(invalid))}, {"Digest(invalid)", e(crane.Digest(invalid))}, diff --git a/pkg/crane/optimize.go b/pkg/crane/optimize.go deleted file mode 100644 index 74c665df7..000000000 --- a/pkg/crane/optimize.go +++ /dev/null @@ -1,237 +0,0 @@ -// Copyright 2020 Google LLC All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package crane - -import ( - "errors" - "fmt" - - "github.com/containerd/stargz-snapshotter/estargz" - "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/empty" - "github.com/google/go-containerregistry/pkg/v1/mutate" - "github.com/google/go-containerregistry/pkg/v1/remote" - "github.com/google/go-containerregistry/pkg/v1/tarball" - "github.com/google/go-containerregistry/pkg/v1/types" -) - -// Optimize optimizes a remote image or index from src to dst. -// THIS API IS EXPERIMENTAL AND SUBJECT TO CHANGE WITHOUT WARNING. -func Optimize(src, dst string, prioritize []string, opt ...Option) error { - pset := newStringSet(prioritize) - o := makeOptions(opt...) - srcRef, err := name.ParseReference(src, o.Name...) - if err != nil { - return fmt.Errorf("parsing reference %q: %w", src, err) - } - - dstRef, err := name.ParseReference(dst, o.Name...) - if err != nil { - return fmt.Errorf("parsing reference for %q: %w", dst, err) - } - - logs.Progress.Printf("Optimizing from %v to %v", srcRef, dstRef) - desc, err := remote.Get(srcRef, o.Remote...) - if err != nil { - return fmt.Errorf("fetching %q: %w", src, err) - } - - switch desc.MediaType { - case types.OCIImageIndex, types.DockerManifestList: - // Handle indexes separately. - if o.Platform != nil { - // If platform is explicitly set, don't optimize the whole index, just the appropriate image. - if err := optimizeAndPushImage(desc, dstRef, pset, o); err != nil { - return fmt.Errorf("failed to optimize image: %w", err) - } - } else { - if err := optimizeAndPushIndex(desc, dstRef, pset, o); err != nil { - return fmt.Errorf("failed to optimize index: %w", err) - } - } - - case types.DockerManifestSchema1, types.DockerManifestSchema1Signed: - return errors.New("docker schema 1 images are not supported") - - default: - // Assume anything else is an image, since some registries don't set mediaTypes properly. - if err := optimizeAndPushImage(desc, dstRef, pset, o); err != nil { - return fmt.Errorf("failed to optimize image: %w", err) - } - } - - return nil -} - -func optimizeAndPushImage(desc *remote.Descriptor, dstRef name.Reference, prioritize stringSet, o Options) error { - img, err := desc.Image() - if err != nil { - return err - } - - missing, oimg, err := optimizeImage(img, prioritize) - if err != nil { - return err - } - - if len(missing) > 0 { - return fmt.Errorf("the following prioritized files were missing from image: %v", missing.List()) - } - - return remote.Write(dstRef, oimg, o.Remote...) -} - -func optimizeImage(img v1.Image, prioritize stringSet) (stringSet, v1.Image, error) { - cfg, err := img.ConfigFile() - if err != nil { - return nil, nil, err - } - ocfg := cfg.DeepCopy() - ocfg.History = nil - ocfg.RootFS.DiffIDs = nil - - oimg, err := mutate.ConfigFile(empty.Image, ocfg) - if err != nil { - return nil, nil, err - } - - layers, err := img.Layers() - if err != nil { - return nil, nil, err - } - - missingFromImage := newStringSet(prioritize.List()) - olayers := make([]mutate.Addendum, 0, len(layers)) - for _, layer := range layers { - missingFromLayer := []string{} - olayer, err := tarball.LayerFromOpener(layer.Uncompressed, - tarball.WithEstargz, - tarball.WithEstargzOptions( - estargz.WithPrioritizedFiles(prioritize.List()), - estargz.WithAllowPrioritizeNotFound(&missingFromLayer), - )) - if err != nil { - return nil, nil, err - } - missingFromImage = missingFromImage.Intersection(newStringSet(missingFromLayer)) - - olayers = append(olayers, mutate.Addendum{ - Layer: olayer, - MediaType: types.DockerLayer, - }) - } - - oimg, err = mutate.Append(oimg, olayers...) - if err != nil { - return nil, nil, err - } - return missingFromImage, oimg, nil -} - -func optimizeAndPushIndex(desc *remote.Descriptor, dstRef name.Reference, prioritize stringSet, o Options) error { - idx, err := desc.ImageIndex() - if err != nil { - return err - } - - missing, oidx, err := optimizeIndex(idx, prioritize) - if err != nil { - return err - } - - if len(missing) > 0 { - return fmt.Errorf("the following prioritized files were missing from all images: %v", missing.List()) - } - - return remote.WriteIndex(dstRef, oidx, o.Remote...) -} - -func optimizeIndex(idx v1.ImageIndex, prioritize stringSet) (stringSet, v1.ImageIndex, error) { - im, err := idx.IndexManifest() - if err != nil { - return nil, nil, err - } - - missingFromIndex := newStringSet(prioritize.List()) - - // Build an image for each child from the base and append it to a new index to produce the result. - adds := make([]mutate.IndexAddendum, 0, len(im.Manifests)) - for _, desc := range im.Manifests { - img, err := idx.Image(desc.Digest) - if err != nil { - return nil, nil, err - } - - missingFromImage, oimg, err := optimizeImage(img, prioritize) - if err != nil { - return nil, nil, err - } - missingFromIndex = missingFromIndex.Intersection(missingFromImage) - adds = append(adds, mutate.IndexAddendum{ - Add: oimg, - Descriptor: v1.Descriptor{ - URLs: desc.URLs, - MediaType: desc.MediaType, - Annotations: desc.Annotations, - Platform: desc.Platform, - }, - }) - } - - idxType, err := idx.MediaType() - if err != nil { - return nil, nil, err - } - - return missingFromIndex, mutate.IndexMediaType(mutate.AppendManifests(empty.Index, adds...), idxType), nil -} - -type stringSet map[string]struct{} - -func newStringSet(in []string) stringSet { - ss := stringSet{} - for _, s := range in { - ss[s] = struct{}{} - } - return ss -} - -func (s stringSet) List() []string { - result := make([]string, 0, len(s)) - for k := range s { - result = append(result, k) - } - return result -} - -func (s stringSet) Intersection(rhs stringSet) stringSet { - // To appease ST1016 - lhs := s - - // Make sure len(lhs) >= len(rhs) - if len(lhs) < len(rhs) { - return rhs.Intersection(lhs) - } - - result := stringSet{} - for k := range lhs { - if _, ok := rhs[k]; ok { - result[k] = struct{}{} - } - } - return result -} diff --git a/pkg/crane/optimize_test.go b/pkg/crane/optimize_test.go deleted file mode 100644 index 11aaf5706..000000000 --- a/pkg/crane/optimize_test.go +++ /dev/null @@ -1,179 +0,0 @@ -// Copyright 2020 Google LLC All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package crane - -import ( - "net/http/httptest" - "net/url" - "path" - "strings" - "testing" - - "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" - "github.com/google/go-containerregistry/pkg/name" - "github.com/google/go-containerregistry/pkg/registry" - v1 "github.com/google/go-containerregistry/pkg/v1" - "github.com/google/go-containerregistry/pkg/v1/empty" - "github.com/google/go-containerregistry/pkg/v1/mutate" - "github.com/google/go-containerregistry/pkg/v1/remote" -) - -func TestStringSet(t *testing.T) { - for _, tc := range []struct { - lhs []string - rhs []string - result []string - }{{ - lhs: []string{}, - rhs: []string{}, - result: []string{}, - }, { - lhs: []string{"a"}, - rhs: []string{}, - result: []string{}, - }, { - lhs: []string{}, - rhs: []string{"a"}, - result: []string{}, - }, { - lhs: []string{"a", "b", "c"}, - rhs: []string{"a", "b", "c"}, - result: []string{"a", "b", "c"}, - }, { - lhs: []string{"a", "b"}, - rhs: []string{"a"}, - result: []string{"a"}, - }, { - lhs: []string{"a"}, - rhs: []string{"a", "b"}, - result: []string{"a"}, - }} { - got := newStringSet(tc.lhs).Intersection(newStringSet(tc.rhs)) - want := newStringSet(tc.result) - if diff := cmp.Diff(want, got); diff != "" { - t.Errorf("%v.intersect(%v) (-want +got): %s", tc.lhs, tc.rhs, diff) - } - - less := func(a, b string) bool { - return strings.Compare(a, b) <= -1 - } - if diff := cmp.Diff(tc.result, got.List(), cmpopts.SortSlices(less)); diff != "" { - t.Errorf("%v.List() (-want +got): = %v", tc.result, diff) - } - } -} - -func TestOptimize(t *testing.T) { - // Set up a fake registry. - s := httptest.NewServer(registry.New()) - defer s.Close() - u, err := url.Parse(s.URL) - if err != nil { - t.Fatal(err) - } - - imgs := []mutate.IndexAddendum{} - for _, plat := range []string{ - "linux/amd64", - "linux/arm", - } { - img, err := Image(map[string][]byte{ - "unimportant": []byte(strings.Repeat("deadbeef", 128)), - "important": []byte(`abc`), - "platform.txt": []byte(plat), - }) - if err != nil { - t.Fatal(err) - } - parts := strings.Split(plat, "/") - imgs = append(imgs, mutate.IndexAddendum{ - Add: img, - Descriptor: v1.Descriptor{ - Platform: &v1.Platform{ - OS: parts[0], - Architecture: parts[1], - }, - }, - }) - } - - idx := mutate.AppendManifests(empty.Index, imgs...) - - slow := path.Join(u.Host, "slow") - fast := path.Join(u.Host, "fast") - - ref, err := name.ParseReference(slow) - if err != nil { - t.Fatal(err) - } - - if err := remote.WriteIndex(ref, idx); err != nil { - t.Fatal(err) - } - - if err := Optimize(slow, fast, []string{"important"}); err != nil { - t.Fatal(err) - } - - if err := Optimize(slow, fast, []string{"important"}, WithPlatform(imgs[1].Platform)); err != nil { - t.Fatal(err) - } - - // Compare optimize WithPlatform path to optimizing just an image. - got, err := Digest(fast) - if err != nil { - t.Fatal(err) - } - - dig, err := Digest(slow, WithPlatform(imgs[1].Platform)) - if err != nil { - t.Fatal(err) - } - - slowImgRef := slow + "@" + dig - if err := Optimize(slowImgRef, fast, []string{"important"}, WithPlatform(imgs[1].Platform)); err != nil { - t.Fatal(err) - } - - want, err := Digest(fast) - if err != nil { - t.Fatal(err) - } - if got != want { - t.Errorf("Optimize(WithPlatform) != Optimize(bydigest): %q != %q", got, want) - } - - for i, ref := range []string{slow, slow, slowImgRef} { - opts := []Option{} - // Silly, but use WithPlatform to get some more coverage. - if i != 0 { - opts = []Option{WithPlatform(imgs[1].Platform)} - } - dig, err := Digest(ref, opts...) - if err != nil { - t.Errorf("Digest(%q): %v", ref, err) - continue - } - // Make sure we fail if there's a missing file in the optimize set - // Use the image digest because it's ~impossible to exist in img. - if err := Optimize(ref, fast, []string{dig}, opts...); err == nil { - t.Errorf("Optimize(%q, prioritize=%q): got nil, want err", ref, dig) - } else if !strings.Contains(err.Error(), dig) { - // Make sure this contains the missing file (dig) - t.Errorf("Optimize(%q) error should contain %q, got: %v", ref, dig, err) - } - } -} diff --git a/pkg/v1/tarball/layer.go b/pkg/v1/tarball/layer.go index a344e9206..8a2630961 100644 --- a/pkg/v1/tarball/layer.go +++ b/pkg/v1/tarball/layer.go @@ -160,6 +160,8 @@ func WithCompressedCaching(l *layer) { // WithEstargzOptions is a functional option that allow the caller to pass // through estargz.Options to the underlying compression layer. This is // only meaningful when estargz is enabled. +// +// Deprecated: WithEstargz is deprecated, and will be removed in a future release. func WithEstargzOptions(opts ...estargz.Option) LayerOption { return func(l *layer) { l.estgzopts = opts @@ -167,6 +169,8 @@ func WithEstargzOptions(opts ...estargz.Option) LayerOption { } // WithEstargz is a functional option that explicitly enables estargz support. +// +// Deprecated: WithEstargz is deprecated, and will be removed in a future release. func WithEstargz(l *layer) { oguncompressed := l.uncompressedopener estargz := func() (io.ReadCloser, error) { @@ -238,6 +242,7 @@ func LayerFromOpener(opener Opener, opts ...LayerOption) (v1.Layer, error) { } if estgz := os.Getenv("GGCR_EXPERIMENT_ESTARGZ"); estgz == "1" { + logs.Warn.Println("GGCR_EXPERIMENT_ESTARGZ is deprecated, and will be removed in a future release.") opts = append([]LayerOption{WithEstargz}, opts...) }