cmd/snap, store, image: support for cohorts in "snap download"

This adds support for cohorts in 'snap download'.

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: "<snap>",
 		// 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
 	}

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

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": <reasons>'
@@ -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) {

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 <http://www.gnu.org/licenses/>.
+ *
+ */
+
+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")
+}

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 {

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() {

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)

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