From ca88bf30950cb8d5c89a274308bc254971886056 Mon Sep 17 00:00:00 2001 From: "John R. Lenton" Date: Wed, 17 Apr 2019 16:16:52 +0100 Subject: [PATCH] cmd/snap, store, image: support for cohorts in "snap download" This adds support for cohorts in 'snap download'. --- cmd/snap/cmd_download.go | 11 ++- image/export_test.go | 6 ++ image/helpers.go | 73 +++++++++++++++---- image/helpers_test.go | 112 +++++++++++++++++++++++++++++ image/image.go | 2 +- store/store.go | 19 +---- store/store_test.go | 4 ++ tests/main/snap-download/task.yaml | 6 ++ 8 files changed, 201 insertions(+), 32 deletions(-) create mode 100644 image/helpers_test.go diff --git a/cmd/snap/cmd_download.go b/cmd/snap/cmd_download.go index dab63c9341d..4358c7d9bca 100644 --- a/cmd/snap/cmd_download.go +++ b/cmd/snap/cmd_download.go @@ -38,6 +38,7 @@ type cmdDownload struct { channelMixin Revision string `long:"revision"` + CohortKey string `long:"cohort"` Positional struct { Snap remoteSnapName } `positional-args:"true" required:"true"` @@ -53,7 +54,10 @@ func init() { addCommand("download", shortDownloadHelp, longDownloadHelp, func() flags.Commander { return &cmdDownload{} }, channelDescs.also(map[string]string{ + // TRANSLATORS: This should not start with a lowercase letter. "revision": i18n.G("Download the given revision of a snap, to which you must have developer access"), + // TRANSLATORS: This should not start with a lowercase letter. + "cohort": i18n.G("Download from the given cohort"), }), []argDesc{{ name: "", // TRANSLATORS: This should not start with a lowercase letter. @@ -103,6 +107,9 @@ func (x *cmdDownload) Execute(args []string) error { if x.Channel != "" { return fmt.Errorf(i18n.G("cannot specify both channel and revision")) } + if x.CohortKey != "" { + return fmt.Errorf(i18n.G("cannot specify both cohort and revision")) + } var err error revision, err = snap.ParseRevision(x.Revision) if err != nil { @@ -121,8 +128,10 @@ func (x *cmdDownload) Execute(args []string) error { dlOpts := image.DownloadOptions{ TargetDir: "", // cwd Channel: x.Channel, + CohortKey: x.CohortKey, + Revision: revision, } - snapPath, snapInfo, err := tsto.DownloadSnap(snapName, revision, &dlOpts) + snapPath, snapInfo, err := tsto.DownloadSnap(snapName, &dlOpts) if err != nil { return err } diff --git a/image/export_test.go b/image/export_test.go index 99ce9f151a7..c538ba00364 100644 --- a/image/export_test.go +++ b/image/export_test.go @@ -48,3 +48,9 @@ func (ls *localInfos) NameToPath() map[string]string { func ToolingStoreContext() store.DeviceAndAuthContext { return toolingStoreContext{} } + +func (opts *DownloadOptions) Validate() error { + return opts.validate() +} + +var ErrRevisionAndCohort = errRevisionAndCohort diff --git a/image/helpers.go b/image/helpers.go index 087033700ce..142ac957a05 100644 --- a/image/helpers.go +++ b/image/helpers.go @@ -26,12 +26,14 @@ import ( "context" "crypto" "encoding/json" + "errors" "fmt" "io/ioutil" "net/url" "os" "os/signal" "path/filepath" + "strings" "syscall" "github.com/mvo5/goconfigparser" @@ -206,38 +208,81 @@ func NewToolingStore() (*ToolingStore, error) { // DownloadOptions carries options for downloading snaps plus assertions. type DownloadOptions struct { + Revision snap.Revision TargetDir string Channel string + CohortKey string } -// DownloadSnap downloads the snap with the given name and optionally revision using the provided store and options. It returns the final full path of the snap inside the opts.TargetDir and a snap.Info for the snap. -func (tsto *ToolingStore) DownloadSnap(name string, revision snap.Revision, opts *DownloadOptions) (targetFn string, info *snap.Info, err error) { - if opts == nil { - opts = &DownloadOptions{} +var errRevisionAndCohort = errors.New("cannot specify both revision and cohort") + +func (opts *DownloadOptions) validate() error { + if opts.Revision.Unset() || opts.CohortKey == "" { + return nil + } + return errRevisionAndCohort +} + +// TODO: maybe move this to strutil next to ElliptRight +func elliptLeft(str string) string { + if len(str) < 10 { + // shouldn't happen outside of tests + return str + } + return "…" + str[len(str)-8:] +} + +func (opts *DownloadOptions) String() string { + var spec []string + if !opts.Revision.Unset() { + spec = append(spec, fmt.Sprintf("(%s)", opts.Revision)) + } + if opts.Channel != "" { + spec = append(spec, fmt.Sprintf("from channel %q", opts.Channel)) + } + if opts.CohortKey != "" { + // cohort keys are really long, and the rightmost bit being the + // interesting bit, so ellipt the rest + spec = append(spec, fmt.Sprintf(`from cohort %q`, elliptLeft(opts.CohortKey))) + } + if opts.TargetDir != "" { + spec = append(spec, fmt.Sprintf("to %q", opts.TargetDir)) + } + return strings.Join(spec, " ") +} + +// DownloadSnap downloads the snap with the given name and optionally revision +// using the provided store and options. It returns the final full path of the +// snap inside the opts.TargetDir and a snap.Info for the snap. +func (tsto *ToolingStore) DownloadSnap(name string, opts DownloadOptions) (targetFn string, info *snap.Info, err error) { + if err := opts.validate(); err != nil { + return "", nil, err } sto := tsto.sto - targetDir := opts.TargetDir - if targetDir == "" { + if opts.TargetDir == "" { pwd, err := os.Getwd() if err != nil { return "", nil, err } - targetDir = pwd + opts.TargetDir = pwd } - logger.Debugf("Going to download snap %q (%s) from channel %q to %q.", name, revision, opts.Channel, opts.TargetDir) + if opts.CohortKey != "" || !opts.Revision.Unset() { + // XXX: is this really necessary (and, if it is, shoudn't we error out instead) + opts.Channel = "" + } + + logger.Debugf("Going to download snap %q %s.", name, &opts) actions := []*store.SnapAction{{ Action: "download", InstanceName: name, - Revision: revision, + Revision: opts.Revision, + CohortKey: opts.CohortKey, + Channel: opts.Channel, }} - if revision.Unset() { - actions[0].Channel = opts.Channel - } - snaps, err := sto.SnapAction(context.TODO(), nil, actions, tsto.user, nil) if err != nil { // err will be 'cannot download snap "foo": ' @@ -246,7 +291,7 @@ func (tsto *ToolingStore) DownloadSnap(name string, revision snap.Revision, opts snap := snaps[0] baseName := filepath.Base(snap.MountFile()) - targetFn = filepath.Join(targetDir, baseName) + targetFn = filepath.Join(opts.TargetDir, baseName) // check if we already have the right file if osutil.FileExists(targetFn) { diff --git a/image/helpers_test.go b/image/helpers_test.go new file mode 100644 index 00000000000..c39b288caf3 --- /dev/null +++ b/image/helpers_test.go @@ -0,0 +1,112 @@ +// -*- Mode: Go; indent-tabs-mode: t -*- + +/* + * Copyright (C) 2014-2019 Canonical Ltd + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +package image_test + +import ( + "os" + "path/filepath" + "runtime" + + "gopkg.in/check.v1" + + "github.com/snapcore/snapd/image" + "github.com/snapcore/snapd/logger" + "github.com/snapcore/snapd/snap" +) + +func (s *imageSuite) TestDownloadOptionsString(c *check.C) { + for opts, str := range map[image.DownloadOptions]string{ + {}: "", + {TargetDir: "foo"}: `to "foo"`, + {Channel: "foo"}: `from channel "foo"`, + {Revision: snap.R(42)}: `(42)`, + { + CohortKey: "AbCdEfGhIjKlMnOpQrStUvWxYz", + }: `from cohort "…StUvWxYz"`, + { + TargetDir: "foo", + Channel: "bar", + Revision: snap.R(13), + CohortKey: "MSBIc3dwOW9PemozYjRtdzhnY0MwMFh0eFduS0g5UWlDUSAxNTU1NDExNDE1IDBjYzJhNTc1ZjNjOTQ3ZDEwMWE1NTNjZWFkNmFmZDE3ZWJhYTYyNjM4ZWQ3ZGMzNjI5YmU4YjQ3NzAwMjdlMDk=", + }: `(13) from channel "bar" from cohort "…MjdlMDk=" to "foo"`, // note this one is not 'valid' so it's ok if the string is a bit wonky + } { + c.Check(opts.String(), check.Equals, str) + } +} + +func (s *imageSuite) TestDownloadOptionsValid(c *check.C) { + for opts, err := range map[image.DownloadOptions]error{ + {}: nil, // might want to error if no targetdir + {TargetDir: "foo"}: nil, + {Channel: "foo"}: nil, + {Revision: snap.R(42)}: nil, + { + CohortKey: "AbCdEfGhIjKlMnOpQrStUvWxYz", + }: nil, + { + Channel: "foo", + Revision: snap.R(42), + }: nil, + { + Channel: "foo", + CohortKey: "bar", + }: nil, + { + Revision: snap.R(1), + CohortKey: "bar", + }: image.ErrRevisionAndCohort, + } { + c.Check(opts.Validate(), check.Equals, err) + } +} + +func (s *imageSuite) TestDownloadSnap(c *check.C) { + // TODO: maybe expand on this (test coverage of DownloadSnap is really bad) + + // env shenanigans + runtime.LockOSThread() + defer runtime.UnlockOSThread() + + debug, hadDebug := os.LookupEnv("SNAPD_DEBUG") + os.Setenv("SNAPD_DEBUG", "1") + if hadDebug { + defer os.Setenv("SNAPD_DEBUG", debug) + } else { + defer os.Unsetenv("SNAPD_DEBUG") + } + logbuf, restore := logger.MockLogger() + defer restore() + + gadgetUnpackDir := c.MkDir() + s.setupSnaps(c, gadgetUnpackDir, map[string]string{ + "core": "canonical", + }) + + dlDir := c.MkDir() + opts := image.DownloadOptions{ + TargetDir: dlDir, + } + fn, info, err := s.tsto.DownloadSnap("core", opts) + c.Assert(err, check.IsNil) + c.Check(fn, check.Matches, filepath.Join(dlDir, `core_\d+.snap`)) + c.Check(info.SnapName(), check.Equals, "core") + + c.Check(logbuf.String(), check.Matches, `.* DEBUG: Going to download snap "core" `+opts.String()+".\n") +} diff --git a/image/image.go b/image/image.go index 7999856a00d..4d207467257 100644 --- a/image/image.go +++ b/image/image.go @@ -348,7 +348,7 @@ func acquireSnap(tsto *ToolingStore, name string, dlOpts *DownloadOptions, local } return dst, info, nil } - return tsto.DownloadSnap(name, snap.R(0), dlOpts) + return tsto.DownloadSnap(name, *dlOpts) } type addingFetcher struct { diff --git a/store/store.go b/store/store.go index 44076a93ceb..6603c946e68 100644 --- a/store/store.go +++ b/store/store.go @@ -1262,22 +1262,6 @@ func (s *Store) WriteCatalogs(ctx context.Context, names io.Writer, adder SnapAd return nil } -// RefreshCandidate contains information for the store about the currently -// installed snap so that the store can decide what update we should see -type RefreshCandidate struct { - SnapID string - Revision snap.Revision - Block []snap.Revision - - // the desired channel - Channel string - // whether validation should be ignored - IgnoreValidation bool - - // try to refresh a local snap to a store revision - Amend bool -} - func findRev(needle snap.Revision, haystack []snap.Revision) bool { for _, r := range haystack { if needle == r { @@ -1966,6 +1950,7 @@ type SnapAction struct { SnapID string Channel string Revision snap.Revision + CohortKey string Flags SnapActionFlags Epoch snap.Epoch } @@ -1986,6 +1971,7 @@ type snapActionJSON struct { SnapID string `json:"snap-id,omitempty"` Channel string `json:"channel,omitempty"` Revision int `json:"revision,omitempty"` + CohortKey string `json:"cohort-key,omitempty"` IgnoreValidation *bool `json:"ignore-validation,omitempty"` // NOTE the store needs an epoch (even if null) for the "install" and "download" @@ -2175,6 +2161,7 @@ func (s *Store) snapAction(ctx context.Context, currentSnaps []*CurrentSnap, act SnapID: a.SnapID, Channel: a.Channel, Revision: a.Revision.N, + CohortKey: a.CohortKey, IgnoreValidation: ignoreValidation, } if !a.Revision.Unset() { diff --git a/store/store_test.go b/store/store_test.go index d0ed608cc13..8ec97626fbf 100644 --- a/store/store_test.go +++ b/store/store_test.go @@ -4111,6 +4111,8 @@ func init() { helloRefreshedDate = t } +const helloCohortKey = "this is a very short cohort key, as cohort keys go, because those are *long*" + func (s *storeTestSuite) TestSnapAction(c *C) { restore := release.MockOnClassic(false) defer restore() @@ -4158,6 +4160,7 @@ func (s *storeTestSuite) TestSnapAction(c *C) { "action": "refresh", "instance-key": helloWorldSnapID, "snap-id": helloWorldSnapID, + "cohort-key": helloCohortKey, }) io.WriteString(w, `{ @@ -4205,6 +4208,7 @@ func (s *storeTestSuite) TestSnapAction(c *C) { Action: "refresh", SnapID: helloWorldSnapID, InstanceName: "hello-world", + CohortKey: helloCohortKey, }, }, nil, nil) c.Assert(err, IsNil) diff --git a/tests/main/snap-download/task.yaml b/tests/main/snap-download/task.yaml index f5537dd39d2..425a62a8515 100644 --- a/tests/main/snap-download/task.yaml +++ b/tests/main/snap-download/task.yaml @@ -23,6 +23,7 @@ execute: | snap download --edge test-snapd-tools ls test-snapd-tools_*.snap verify_asserts test-snapd-tools_*.assert + rm -v test-snapd-tools* echo "Snap download downloads devmode snaps" snap download --beta classic @@ -33,3 +34,8 @@ execute: | su -l -c "SNAPPY_USE_STAGING_STORE=$SNAPPY_USE_STAGING_STORE HTTPS_PROXY=$HTTPS_PROXY snap download test-snapd-tools" test ls /home/test/test-snapd-tools_*.snap verify_asserts /home/test/test-snapd-tools_*.assert + + echo "Snap download can download snaps from a cohort" + snap download --cohort="MSBlRmU4QlRSNUw1VjlGN3lIZU1BUHhrRXIyTmRVWE10dyAxNTU1NTE0MzA5IDg3ZmUwMjhkZDFjMTQ1MDY5N2QyZjdiMGZkMzgzODk0NjMzMmFhOTZmZmFjZmFlNmU2MGQyOTNjYzE1OTE3NWY=" test-snapd-tools + ls test-snapd-tools_*.snap + verify_asserts test-snapd-tools_*.assert