From 81fd929ebbb500f7a57fdd117f8c8cb0cd099dc4 Mon Sep 17 00:00:00 2001 From: Andrew Phelps <136256549+andrewphelpsj@users.noreply.github.com> Date: Thu, 30 Jan 2025 16:40:49 -0500 Subject: [PATCH] many: allow downloading components with "snap download" (#14630) --- cmd/snap/cmd_download.go | 223 ++++++++++++++++++++++------- cmd/snap/cmd_download_test.go | 116 +++++++++++++-- cmd/snap/export_test.go | 16 ++- store/tooling/tooling.go | 5 +- tests/main/snap-download/task.yaml | 56 ++++++++ 5 files changed, 345 insertions(+), 71 deletions(-) diff --git a/cmd/snap/cmd_download.go b/cmd/snap/cmd_download.go index 2cca2dd846e..29c2a5284fa 100644 --- a/cmd/snap/cmd_download.go +++ b/cmd/snap/cmd_download.go @@ -33,14 +33,17 @@ import ( "github.com/snapcore/snapd/i18n" "github.com/snapcore/snapd/image" "github.com/snapcore/snapd/snap" + "github.com/snapcore/snapd/snap/naming" "github.com/snapcore/snapd/store/tooling" + "github.com/snapcore/snapd/strutil" ) type cmdDownload struct { channelMixin - Revision string `long:"revision"` - Basename string `long:"basename"` - TargetDir string `long:"target-directory"` + Revision string `long:"revision"` + Basename string `long:"basename"` + TargetDir string `long:"target-directory"` + OnlyComponents bool `long:"only-components"` CohortKey string `long:"cohort"` Positional struct { @@ -50,8 +53,9 @@ type cmdDownload struct { var shortDownloadHelp = i18n.G("Download the given snap") var longDownloadHelp = i18n.G(` -The download command downloads the given snap and its supporting assertions -to the current directory with .snap and .assert file extensions, respectively. +The download command downloads the given snap, components, and their supporting +assertions to the current directory with .snap, .comp, and .assert file +extensions, respectively. `) func init() { @@ -59,91 +63,196 @@ func init() { 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"), + "revision": i18n.G("Download the given revision of a snap. When downloading components, download the components associated with the given snap revision."), // TRANSLATORS: This should not start with a lowercase letter. "cohort": i18n.G("Download from the given cohort"), // TRANSLATORS: This should not start with a lowercase letter. - "basename": i18n.G("Use this basename for the snap and assertion files (defaults to _)"), + "basename": i18n.G("Use this basename for the snap, component, and assertion files (defaults to _)"), // TRANSLATORS: This should not start with a lowercase letter. "target-directory": i18n.G("Download to this directory (defaults to the current directory)"), + // TRANSLATORS: This should not start with a lowercase letter. + "only-components": i18n.G("Only download the given components, not the snap"), }), []argDesc{{ - name: "", + name: "", // TRANSLATORS: This should not start with a lowercase letter. - desc: i18n.G("Snap name"), + desc: i18n.G("Snap and, optionally, component names"), }}) } -func fetchSnapAssertionsDirect(tsto *tooling.ToolingStore, snapPath string, snapInfo *snap.Info) (string, error) { - db, err := asserts.OpenDatabase(&asserts.DatabaseConfig{ - Backstore: asserts.NewMemoryBackstore(), - Trusted: sysdb.Trusted(), - }) - if err != nil { - return "", err - } - - assertPath := strings.TrimSuffix(snapPath, filepath.Ext(snapPath)) + ".assert" - w, err := os.Create(assertPath) - if err != nil { - return "", fmt.Errorf(i18n.G("cannot create assertions file: %v"), err) - } - defer w.Close() - - encoder := asserts.NewEncoder(w) - save := func(a asserts.Assertion) error { - return encoder.Encode(a) - } - f := tsto.AssertionFetcher(db, save) - - // TODO:COMPS: support downloading components - _, err = image.FetchAndCheckSnapAssertions(snapPath, snapInfo, nil, nil, f, db) - return assertPath, err -} - -func printInstallHint(assertPath, snapPath string) { +func printInstallHint(assertPath string, containerPaths []string) { // simplify paths wd, _ := os.Getwd() if p, err := filepath.Rel(wd, assertPath); err == nil { assertPath = p } - if p, err := filepath.Rel(wd, snapPath); err == nil { - snapPath = p + + relativePaths := make([]string, 0, len(containerPaths)) + for _, path := range containerPaths { + if rel, err := filepath.Rel(wd, path); err == nil { + relativePaths = append(relativePaths, rel) + } else { + relativePaths = append(relativePaths, path) + } } + // add a hint what to do with the downloaded snap (LP:1676707) fmt.Fprintf(Stdout, i18n.G(`Install the snap with: snap ack %s snap install %s -`), assertPath, snapPath) +`), assertPath, strings.Join(relativePaths, " ")) } -// for testing -var downloadDirect = downloadDirectImpl +func downloadDirect(snapName string, components []string, opts tooling.DownloadSnapOptions) error { + compRefs := make([]string, 0, len(components)) + for _, comp := range components { + compRefs = append(compRefs, naming.NewComponentRef(snapName, comp).String()) + } + + if opts.OnlyComponents { + if len(compRefs) == 1 { + fmt.Fprintf(Stdout, i18n.G("Fetching component %q\n"), compRefs[0]) + } else { + fmt.Fprintf(Stdout, i18n.G("Fetching components %s\n"), strutil.Quoted(compRefs)) + } + } else { + switch len(components) { + case 0: + fmt.Fprintf(Stdout, i18n.G("Fetching snap %q\n"), snapName) + case 1: + fmt.Fprintf(Stdout, i18n.G("Fetching snap %q and component %q\n"), snapName, compRefs[0]) + default: + fmt.Fprintf(Stdout, i18n.G("Fetching snap %q and components %s\n"), snapName, strutil.Quoted(compRefs)) + } + } -func downloadDirectImpl(snapName string, revision snap.Revision, dlOpts tooling.DownloadSnapOptions) error { tsto, err := tooling.NewToolingStore() if err != nil { return err } tsto.Stdout = Stdout - fmt.Fprintf(Stdout, i18n.G("Fetching snap %q\n"), snapName) - // TODO:COMPS: consider downloading components - dlSnap, err := tsto.DownloadSnap(snapName, nil, dlOpts) + dl, err := downloadContainers(snapName, components, tsto, opts) if err != nil { return err } - fmt.Fprintf(Stdout, i18n.G("Fetching assertions for %q\n"), snapName) - assertPath, err := fetchSnapAssertionsDirect(tsto, dlSnap.Path, dlSnap.Info) + downloaded := make([]string, 0, len(compRefs)+1) + if !opts.OnlyComponents { + downloaded = append(downloaded, snapName) + } + downloaded = append(downloaded, compRefs...) + + fmt.Fprintf(Stdout, i18n.G("Fetching assertions for %s\n"), strutil.Quoted(downloaded)) + + compInfos := make(map[string]*snap.ComponentInfo, len(components)) + for _, comp := range dl.Components { + compInfos[comp.Path] = comp.Info + } + + snapPath := dl.Path + + // if we're only downloading components, then we won't have a snap path to + // work with. since downloadAssertions derives where it downloads the + // assertions to based on the snap path, we need to set it to something + // appropriate here. + if opts.OnlyComponents { + if opts.Basename == "" { + snapPath = fmt.Sprintf("%s_%s.snap", dl.Info.SnapName(), dl.Info.Revision) + } else { + snapPath = fmt.Sprintf("%s.snap", opts.Basename) + } + } + + assertsPath, err := downloadAssertions(dl.Info, snapPath, compInfos, tsto, opts) if err != nil { return err } - printInstallHint(assertPath, dlSnap.Path) + + containerPaths := make([]string, 0, len(components)+1) + if !opts.OnlyComponents { + containerPaths = append(containerPaths, dl.Path) + } + for _, c := range dl.Components { + containerPaths = append(containerPaths, c.Path) + } + + printInstallHint(assertsPath, containerPaths) + return nil } -func (x *cmdDownload) downloadFromStore(snapName string, revision snap.Revision) error { - dlOpts := tooling.DownloadSnapOptions{ +var ( + downloadAssertions = downloadAssertionsImpl + downloadContainers = downloadContainersImpl +) + +func downloadContainersImpl(snapName string, components []string, tsto *tooling.ToolingStore, opts tooling.DownloadSnapOptions) (*tooling.DownloadedSnap, error) { + dl, err := tsto.DownloadSnap(snapName, components, opts) + if err != nil { + return nil, err + } + return dl, nil +} + +func downloadAssertionsImpl( + info *snap.Info, + snapPath string, + components map[string]*snap.ComponentInfo, + tsto *tooling.ToolingStore, + opts tooling.DownloadSnapOptions, +) (string, error) { + db, err := asserts.OpenDatabase(&asserts.DatabaseConfig{ + Backstore: asserts.NewMemoryBackstore(), + Trusted: sysdb.Trusted(), + }) + if err != nil { + return "", err + } + + assertPath := strings.TrimSuffix(snapPath, filepath.Ext(snapPath)) + ".assert" + + w, err := os.Create(assertPath) + if err != nil { + return "", fmt.Errorf(i18n.G("cannot create assertions file: %v"), err) + } + defer w.Close() + + encoder := asserts.NewEncoder(w) + save := func(a asserts.Assertion) error { + return encoder.Encode(a) + } + f := tsto.AssertionFetcher(db, save) + + comps := make([]image.CompInfoPath, 0, len(components)) + for path, ci := range components { + comps = append(comps, image.CompInfoPath{ + Info: ci, + Path: path, + }) + } + + if !opts.OnlyComponents { + _, err := image.FetchAndCheckSnapAssertions(snapPath, info, comps, nil, f, db) + if err != nil { + return "", err + } + } else { + for _, c := range comps { + if err := image.FetchAndCheckComponentAssertions(c, info, nil, f, db); err != nil { + return "", err + } + } + } + + if err := w.Close(); err != nil { + return "", err + } + + return assertPath, nil +} + +func (x *cmdDownload) downloadFromStore(snap string, comps []string, revision snap.Revision) error { + return downloadDirect(snap, comps, tooling.DownloadSnapOptions{ TargetDir: x.TargetDir, Basename: x.Basename, Channel: x.Channel, @@ -151,8 +260,8 @@ func (x *cmdDownload) downloadFromStore(snapName string, revision snap.Revision) Revision: revision, // if something goes wrong, don't force it to start over again LeavePartialOnError: true, - } - return downloadDirect(snapName, revision, dlOpts) + OnlyComponents: x.OnlyComponents, + }) } func (x *cmdDownload) Execute(args []string) error { @@ -184,6 +293,10 @@ func (x *cmdDownload) Execute(args []string) error { } } - snapName := string(x.Positional.Snap) - return x.downloadFromStore(snapName, revision) + snap, comps := snap.SplitSnapInstanceAndComponents(string(x.Positional.Snap)) + if x.OnlyComponents && len(comps) == 0 { + return errors.New(i18n.G("cannot specify --only-components without providing any components;")) + } + + return x.downloadFromStore(snap, comps, revision) } diff --git a/cmd/snap/cmd_download_test.go b/cmd/snap/cmd_download_test.go index 08fb24da543..4efef270431 100644 --- a/cmd/snap/cmd_download_test.go +++ b/cmd/snap/cmd_download_test.go @@ -28,6 +28,7 @@ import ( snapCmd "github.com/snapcore/snapd/cmd/snap" "github.com/snapcore/snapd/snap" + "github.com/snapcore/snapd/snap/naming" "github.com/snapcore/snapd/store/tooling" ) @@ -67,7 +68,7 @@ func (s *SnapSuite) TestDownloadChannelAndRevision(c *check.C) { } func (s *SnapSuite) TestPrintInstalHint(c *check.C) { - snapCmd.PrintInstallHint("foo_1.assert", "foo_1.snap") + snapCmd.PrintInstallHint("foo_1.assert", []string{"foo_1.snap"}) c.Check(s.Stdout(), check.Equals, `Install the snap with: snap ack foo_1.assert snap install foo_1.snap @@ -78,7 +79,7 @@ func (s *SnapSuite) TestPrintInstalHint(c *check.C) { c.Assert(err, check.IsNil) as := filepath.Join(cwd, "some-dir/foo_1.assert") sn := filepath.Join(cwd, "some-dir/foo_1.snap") - snapCmd.PrintInstallHint(as, sn) + snapCmd.PrintInstallHint(as, []string{sn}) c.Check(s.Stdout(), check.Equals, `Install the snap with: snap ack some-dir/foo_1.assert snap install some-dir/foo_1.snap @@ -87,15 +88,31 @@ func (s *SnapSuite) TestPrintInstalHint(c *check.C) { func (s *SnapSuite) TestDownloadDirect(c *check.C) { var n int - restore := snapCmd.MockDownloadDirect(func(snapName string, revision snap.Revision, dlOpts tooling.DownloadSnapOptions) error { + restore := snapCmd.MockDownloadContainers(func(snapName string, components []string, tsto *tooling.ToolingStore, opts tooling.DownloadSnapOptions) (*tooling.DownloadedSnap, error) { c.Check(snapName, check.Equals, "a-snap") - c.Check(revision, check.Equals, snap.R(0)) - c.Check(dlOpts.Basename, check.Equals, "some-base-name") - c.Check(dlOpts.TargetDir, check.Equals, "some-target-dir") - c.Check(dlOpts.Channel, check.Equals, "some-channel") - c.Check(dlOpts.CohortKey, check.Equals, "some-cohort") + c.Check(opts.Revision, check.Equals, snap.R(0)) + c.Check(opts.Basename, check.Equals, "some-base-name") + c.Check(opts.TargetDir, check.Equals, "some-target-dir") + c.Check(opts.Channel, check.Equals, "some-channel") + c.Check(opts.CohortKey, check.Equals, "some-cohort") + c.Check(opts.OnlyComponents, check.Equals, false) n++ - return nil + return &tooling.DownloadedSnap{ + Path: "a-snap_1.snap", + Info: &snap.Info{ + SideInfo: snap.SideInfo{ + RealName: "a-snap", + }, + }, + }, nil + }) + defer restore() + + restore = snapCmd.MockDownloadAssertions(func(info *snap.Info, snapPath string, components map[string]*snap.ComponentInfo, tsto *tooling.ToolingStore, opts tooling.DownloadSnapOptions) (string, error) { + c.Check(info.RealName, check.Equals, "a-snap") + c.Check(snapPath, check.Equals, "a-snap_1.snap") + c.Check(components, check.HasLen, 0) + return "foo_1.assert", nil }) defer restore() @@ -112,11 +129,88 @@ func (s *SnapSuite) TestDownloadDirect(c *check.C) { c.Check(n, check.Equals, 1) } +func (s *SnapSuite) TestDownloadDirectWithComponents(c *check.C) { + const basename = "" + const onlyComponents = false + s.testDownloadDirectWithComponents(c, basename, onlyComponents) +} + +func (s *SnapSuite) TestDownloadDirectWithComponentsOnlyComponents(c *check.C) { + const basename = "" + const onlyComponents = true + s.testDownloadDirectWithComponents(c, basename, onlyComponents) +} + +func (s *SnapSuite) TestDownloadDirectWithComponentsBasename(c *check.C) { + const basename = "base-name" + const onlyComponents = false + s.testDownloadDirectWithComponents(c, basename, onlyComponents) +} + +func (s *SnapSuite) testDownloadDirectWithComponents(c *check.C, basename string, onlyComponents bool) { + var n int + restore := snapCmd.MockDownloadContainers(func(snapName string, components []string, tsto *tooling.ToolingStore, opts tooling.DownloadSnapOptions) (*tooling.DownloadedSnap, error) { + c.Check(snapName, check.Equals, "a-snap") + c.Check(components, check.DeepEquals, []string{"comp-1", "comp-2"}) + c.Check(opts.Revision, check.Equals, snap.R(0)) + c.Check(opts.OnlyComponents, check.Equals, onlyComponents) + c.Check(opts.Basename, check.Equals, basename) + n++ + return &tooling.DownloadedSnap{ + Path: "a-snap_1.snap", + Info: &snap.Info{ + SideInfo: snap.SideInfo{ + RealName: "a-snap", + Revision: snap.R(1), + }, + }, + Components: []*tooling.DownloadedComponent{ + { + Path: "a-snap+comp-1_2.comp", + Info: &snap.ComponentInfo{ + Component: naming.NewComponentRef("a-snap", "comp-1"), + Type: snap.StandardComponent, + }, + }, + { + Path: "a-snapcomp-2_3.comp", + Info: &snap.ComponentInfo{ + Component: naming.NewComponentRef("a-snap", "comp-2"), + Type: snap.StandardComponent, + }, + }, + }, + }, nil + }) + defer restore() + + restore = snapCmd.MockDownloadAssertions(func(info *snap.Info, snapPath string, components map[string]*snap.ComponentInfo, tsto *tooling.ToolingStore, opts tooling.DownloadSnapOptions) (string, error) { + c.Check(info.RealName, check.Equals, "a-snap") + c.Check(snapPath, check.Equals, "a-snap_1.snap") + c.Check(components, check.HasLen, 2) + return "a-snap_1.assert", nil + }) + defer restore() + + args := []string{"download", "a-snap+comp-1+comp-2"} + if basename != "" { + args = append(args, "--basename="+basename) + } + if onlyComponents { + args = append(args, "--only-components") + } + + // check that a direct download got issued + _, err := snapCmd.Parser(snapCmd.Client()).ParseArgs(args) + c.Assert(err, check.IsNil) + c.Check(n, check.Equals, 1) +} + func (s *SnapSuite) TestDownloadDirectErrors(c *check.C) { var n int - restore := snapCmd.MockDownloadDirect(func(snapName string, revision snap.Revision, dlOpts tooling.DownloadSnapOptions) error { + restore := snapCmd.MockDownloadContainers(func(snapName string, components []string, tsto *tooling.ToolingStore, opts tooling.DownloadSnapOptions) (*tooling.DownloadedSnap, error) { n++ - return fmt.Errorf("some-error") + return nil, fmt.Errorf("some-error") }) defer restore() diff --git a/cmd/snap/export_test.go b/cmd/snap/export_test.go index 1131ca91f0e..c254f57e235 100644 --- a/cmd/snap/export_test.go +++ b/cmd/snap/export_test.go @@ -397,11 +397,19 @@ func MockIoutilTempDir(f func(string, string) (string, error)) (restore func()) } } -func MockDownloadDirect(f func(snapName string, revision snap.Revision, dlOpts tooling.DownloadSnapOptions) error) (restore func()) { - old := downloadDirect - downloadDirect = f +func MockDownloadContainers(f func(snapName string, components []string, tsto *tooling.ToolingStore, opts tooling.DownloadSnapOptions) (*tooling.DownloadedSnap, error)) (restore func()) { + old := downloadContainers + downloadContainers = f return func() { - downloadDirect = old + downloadContainers = old + } +} + +func MockDownloadAssertions(f func(info *snap.Info, snapPath string, components map[string]*snap.ComponentInfo, tsto *tooling.ToolingStore, opts tooling.DownloadSnapOptions) (string, error)) (restore func()) { + old := downloadAssertions + downloadAssertions = f + return func() { + downloadAssertions = old } } diff --git a/store/tooling/tooling.go b/store/tooling/tooling.go index 6e44df09a91..aa287b97239 100644 --- a/store/tooling/tooling.go +++ b/store/tooling/tooling.go @@ -557,6 +557,8 @@ func (tsto *ToolingStore) componentDownload(targetFn string, snapName string, sr return nil, err } + cref := naming.NewComponentRef(snapName, srr.Name) + // check if we already have the right file if osutil.FileExists(targetFn) { sha3_384Dgst, size, err := osutil.FileDigest(targetFn, crypto.SHA3_384) @@ -566,12 +568,13 @@ func (tsto *ToolingStore) componentDownload(targetFn string, snapName string, sr logger.Debugf("not downloading, using existing file %s", targetFn) return &DownloadedComponent{ Path: targetFn, + Info: snap.NewComponentInfo(cref, ctyp, srr.Version, + "", "", "", snap.NewComponentSideInfo(cref, snap.R(srr.Revision))), }, nil } logger.Debugf("File exists but has wrong hash, ignoring (here).") } - cref := naming.NewComponentRef(snapName, srr.Name) download := func(pb progress.Meter) error { dlOpts := &store.DownloadOptions{LeavePartialOnError: opts.LeavePartialOnError} return tsto.sto.Download(context.TODO(), cref.String(), targetFn, diff --git a/tests/main/snap-download/task.yaml b/tests/main/snap-download/task.yaml index 85f99d6dcc9..75ae2548ee5 100644 --- a/tests/main/snap-download/task.yaml +++ b/tests/main/snap-download/task.yaml @@ -19,6 +19,28 @@ execute: | MATCH "type: snap-declaration" < "$fn" MATCH "type: snap-revision" < "$fn" } + + verify_component_asserts() { + fn="${1}" + shift + component_count="$#" + + MATCH "type: account-key" < "$fn" + MATCH "type: snap-declaration" < "$fn" + + # if we don't download the component, then we will not have a + # snap-revision assertion, since it isn't a prereq of either assertions + # that the component needs + + # should be one of these for each component + grep -c "type: snap-resource-pair" < "${fn}" | MATCH "${component_count}" + grep -c "type: snap-resource-revision" < "${fn}" | MATCH "${component_count}" + + for component in "$@"; do + MATCH "resource-name: ${component}" < "${fn}" + done + } + echo "Snap download can download snaps" snap download test-snapd-control-consumer ls test-snapd-control-consumer_*.snap @@ -59,3 +81,37 @@ execute: | snap download --target-directory=foo --basename=bar test-snapd-tools ls -l foo/bar.snap verify_asserts foo/bar.assert + + echo "Snap download can download snaps and components" + snap download test-snap-with-components+one+two + ls test-snap-with-components_*.snap test-snap-with-components+one_*.comp test-snap-with-components+two_*.comp + verify_asserts test-snap-with-components_*.assert + verify_component_asserts test-snap-with-components_*.assert one two + rm -v test-snap-with-components* + + echo "Snap download can download snaps and components with a specified basename" + snap download --basename=base test-snap-with-components+one+two + ls base.snap base+one.comp base+two.comp + verify_asserts base.assert + verify_component_asserts base.assert one two + rm -v base* + + echo "Snap download can download only components" + snap download --only-components test-snap-with-components+one+two + ls test-snap-with-components+one_*.comp test-snap-with-components+two_*.comp + not ls test-snap-with-components_*.snap + verify_component_asserts test-snap-with-components_*.assert one two + rm -v test-snap-with-components* + + echo "Snap download can download only components with a specified basename" + snap download --only-components --basename=base test-snap-with-components+one+two + ls base+one.comp base+two.comp + not ls base.snap + verify_component_asserts base.assert one two + rm -v base* + + echo "Snap download can download only components with a specific revision of a snap" + snap download --revision=7 --only-components test-snap-with-components+one+two + ls test-snap-with-components+one_4.comp test-snap-with-components+two_4.comp + verify_component_asserts test-snap-with-components_7.assert one two + rm -v test-snap-with-components*