From 61fbe99d3b8f148a34b5571e6f647e7e102e3932 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Mon, 27 Jul 2026 14:52:40 +0200 Subject: [PATCH] refactor: extract PackageInfo to a dedicated pkg --- internal/archive/archive.go | 29 +++--- internal/archive/archive_test.go | 81 ++++++++-------- internal/manifestutil/manifestutil.go | 27 ++++-- internal/manifestutil/manifestutil_test.go | 102 +++++++++++++-------- internal/pkgutil/pkgutil.go | 19 ++++ internal/slicer/slicer.go | 6 +- internal/testutil/archive.go | 32 ++++--- 7 files changed, 182 insertions(+), 114 deletions(-) create mode 100644 internal/pkgutil/pkgutil.go diff --git a/internal/archive/archive.go b/internal/archive/archive.go index 7c90a41dd..f2403435a 100644 --- a/internal/archive/archive.go +++ b/internal/archive/archive.go @@ -16,20 +16,14 @@ import ( "github.com/canonical/chisel/internal/control" "github.com/canonical/chisel/internal/deb" "github.com/canonical/chisel/internal/pgputil" + "github.com/canonical/chisel/internal/pkgutil" ) type Archive interface { Options() *Options - Fetch(pkg string) (io.ReadSeekCloser, *PackageInfo, error) + Fetch(pkg string) (io.ReadSeekCloser, *pkgutil.Info, error) Exists(pkg string) bool - Info(pkg string) (*PackageInfo, error) -} - -type PackageInfo struct { - Name string - Version string - Arch string - SHA256 string + Info(pkg string) (*pkgutil.Info, error) } type Options struct { @@ -133,7 +127,7 @@ func (a *ubuntuArchive) selectPackage(pkg string) (control.Section, *ubuntuIndex return selectedSection, selectedIndex, nil } -func (a *ubuntuArchive) Fetch(pkg string) (io.ReadSeekCloser, *PackageInfo, error) { +func (a *ubuntuArchive) Fetch(pkg string) (io.ReadSeekCloser, *pkgutil.Info, error) { section, index, err := a.selectPackage(pkg) if err != nil { return nil, nil, err @@ -148,7 +142,7 @@ func (a *ubuntuArchive) Fetch(pkg string) (io.ReadSeekCloser, *PackageInfo, erro return reader, info, nil } -func (a *ubuntuArchive) Info(pkg string) (*PackageInfo, error) { +func (a *ubuntuArchive) Info(pkg string) (*pkgutil.Info, error) { section, _, err := a.selectPackage(pkg) if err != nil { return nil, err @@ -466,12 +460,13 @@ func (index *ubuntuIndex) fetch(path, digest string, flags fetchFlags) (io.ReadS return index.archive.cache.Open(digestKind, writer.Digest()) } -func sectionPackageInfo(section control.Section) *PackageInfo { - return &PackageInfo{ - Name: section.Get("Package"), - Version: section.Get("Version"), - Arch: section.Get("Architecture"), - SHA256: section.Get("SHA256"), +func sectionPackageInfo(section control.Section) *pkgutil.Info { + return &pkgutil.Info{ + Name: section.Get("Package"), + Version: section.Get("Version"), + Arch: section.Get("Architecture"), + DigestKind: cache.SHA256, + Digest: section.Get("SHA256"), } } diff --git a/internal/archive/archive_test.go b/internal/archive/archive_test.go index 90e3f0047..ee46fef8c 100644 --- a/internal/archive/archive_test.go +++ b/internal/archive/archive_test.go @@ -18,6 +18,8 @@ import ( "github.com/canonical/chisel/internal/archive" "github.com/canonical/chisel/internal/archive/testarchive" + "github.com/canonical/chisel/internal/cache" + "github.com/canonical/chisel/internal/pkgutil" "github.com/canonical/chisel/internal/tarball" "github.com/canonical/chisel/internal/testutil" ) @@ -244,22 +246,24 @@ func (s *httpSuite) TestFetchPackage(c *C) { // First on component main. pkg, info, err := testArchive.Fetch("mypkg1") c.Assert(err, IsNil) - c.Assert(info, DeepEquals, &archive.PackageInfo{ - Name: "mypkg1", - Version: "1.1", - Arch: "amd64", - SHA256: "1f08ef04cfe7a8087ee38a1ea35fa1810246648136c3c42d5a61ad6503d85e05", + c.Assert(info, DeepEquals, &pkgutil.Info{ + Name: "mypkg1", + Version: "1.1", + Arch: "amd64", + DigestKind: cache.SHA256, + Digest: "1f08ef04cfe7a8087ee38a1ea35fa1810246648136c3c42d5a61ad6503d85e05", }) c.Assert(read(pkg), Equals, "mypkg1 1.1 data") // Last on component universe. pkg, info, err = testArchive.Fetch("mypkg4") c.Assert(err, IsNil) - c.Assert(info, DeepEquals, &archive.PackageInfo{ - Name: "mypkg4", - Version: "1.4", - Arch: "amd64", - SHA256: "54af70097b30b33cfcbb6911ad3d0df86c2d458928169e348fa7873e4fc678e4", + c.Assert(info, DeepEquals, &pkgutil.Info{ + Name: "mypkg4", + Version: "1.4", + Arch: "amd64", + DigestKind: cache.SHA256, + Digest: "54af70097b30b33cfcbb6911ad3d0df86c2d458928169e348fa7873e4fc678e4", }) c.Assert(read(pkg), Equals, "mypkg4 1.4 data") } @@ -286,22 +290,24 @@ func (s *httpSuite) TestFetchPortsPackage(c *C) { // First on component main. pkg, info, err := testArchive.Fetch("mypkg1") c.Assert(err, IsNil) - c.Assert(info, DeepEquals, &archive.PackageInfo{ - Name: "mypkg1", - Version: "1.1", - Arch: "arm64", - SHA256: "1f08ef04cfe7a8087ee38a1ea35fa1810246648136c3c42d5a61ad6503d85e05", + c.Assert(info, DeepEquals, &pkgutil.Info{ + Name: "mypkg1", + Version: "1.1", + Arch: "arm64", + DigestKind: cache.SHA256, + Digest: "1f08ef04cfe7a8087ee38a1ea35fa1810246648136c3c42d5a61ad6503d85e05", }) c.Assert(read(pkg), Equals, "mypkg1 1.1 data") // Last on component universe. pkg, info, err = testArchive.Fetch("mypkg4") c.Assert(err, IsNil) - c.Assert(info, DeepEquals, &archive.PackageInfo{ - Name: "mypkg4", - Version: "1.4", - Arch: "arm64", - SHA256: "54af70097b30b33cfcbb6911ad3d0df86c2d458928169e348fa7873e4fc678e4", + c.Assert(info, DeepEquals, &pkgutil.Info{ + Name: "mypkg4", + Version: "1.4", + Arch: "arm64", + DigestKind: cache.SHA256, + Digest: "54af70097b30b33cfcbb6911ad3d0df86c2d458928169e348fa7873e4fc678e4", }) c.Assert(read(pkg), Equals, "mypkg4 1.4 data") } @@ -335,21 +341,23 @@ func (s *httpSuite) TestFetchSecurityPackage(c *C) { pkg, info, err := testArchive.Fetch("mypkg1") c.Assert(err, IsNil) - c.Assert(info, DeepEquals, &archive.PackageInfo{ - Name: "mypkg1", - Version: "1.1.2.2", - Arch: "amd64", - SHA256: "5448585bdd916e5023eff2bc1bc3b30bcc6ee9db9c03e531375a6a11ddf0913c", + c.Assert(info, DeepEquals, &pkgutil.Info{ + Name: "mypkg1", + Version: "1.1.2.2", + Arch: "amd64", + DigestKind: cache.SHA256, + Digest: "5448585bdd916e5023eff2bc1bc3b30bcc6ee9db9c03e531375a6a11ddf0913c", }) c.Assert(read(pkg), Equals, "package from jammy-security") pkg, info, err = testArchive.Fetch("mypkg2") c.Assert(err, IsNil) - c.Assert(info, DeepEquals, &archive.PackageInfo{ - Name: "mypkg2", - Version: "1.2", - Arch: "amd64", - SHA256: "a4b4f3f3a8fa09b69e3ba23c60a41a1f8144691fd371a2455812572fd02e6f79", + c.Assert(info, DeepEquals, &pkgutil.Info{ + Name: "mypkg2", + Version: "1.2", + Arch: "amd64", + DigestKind: cache.SHA256, + Digest: "a4b4f3f3a8fa09b69e3ba23c60a41a1f8144691fd371a2455812572fd02e6f79", }) c.Assert(read(pkg), Equals, "mypkg2 1.2 data") } @@ -585,16 +593,17 @@ func (s *httpSuite) TestVerifyArchiveRelease(c *C) { var packageInfoTests = []struct { summary string pkg string - info *archive.PackageInfo + info *pkgutil.Info error string }{{ summary: "Basic", pkg: "mypkg1", - info: &archive.PackageInfo{ - Name: "mypkg1", - Version: "1.1", - Arch: "amd64", - SHA256: "1f08ef04cfe7a8087ee38a1ea35fa1810246648136c3c42d5a61ad6503d85e05", + info: &pkgutil.Info{ + Name: "mypkg1", + Version: "1.1", + Arch: "amd64", + DigestKind: cache.SHA256, + Digest: "1f08ef04cfe7a8087ee38a1ea35fa1810246648136c3c42d5a61ad6503d85e05", }, }, { summary: "Package not found in archive", diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index 16b054022..a17244b54 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -10,7 +10,8 @@ import ( "strings" "github.com/canonical/chisel/internal/apacheutil" - "github.com/canonical/chisel/internal/archive" + "github.com/canonical/chisel/internal/cache" + "github.com/canonical/chisel/internal/pkgutil" "github.com/canonical/chisel/internal/setup" "github.com/canonical/chisel/public/jsonwall" "github.com/canonical/chisel/public/manifest" @@ -35,7 +36,7 @@ func FindPaths(slices []*setup.Slice) map[string][]*setup.Slice { } type WriteOptions struct { - PackageInfo []*archive.PackageInfo + PackageInfo []*pkgutil.Info Selection []*setup.Slice Report *Report } @@ -69,13 +70,13 @@ func Write(options *WriteOptions, writer io.Writer) error { return err } -func manifestAddPackages(dbw *jsonwall.DBWriter, infos []*archive.PackageInfo) error { +func manifestAddPackages(dbw *jsonwall.DBWriter, infos []*pkgutil.Info) error { for _, info := range infos { err := dbw.Add(&manifest.Package{ Kind: "package", Name: info.Name, Version: info.Version, - Digest: info.SHA256, + Digest: info.Digest, Arch: info.Arch, }) if err != nil { @@ -250,19 +251,31 @@ func validateReportEntry(entry *ReportEntry) (err error) { return nil } -func validatePackage(pkg *archive.PackageInfo) (err error) { +func validatePackage(pkg *pkgutil.Info) (err error) { if pkg.Name == "" { return fmt.Errorf("package name not set") } if pkg.Arch == "" { return fmt.Errorf("package %q missing arch", pkg.Name) } - if pkg.SHA256 == "" { - return fmt.Errorf("package %q missing sha256", pkg.Name) + // The manifest records the package digest as a SHA256 one. Fail rather than + // recording a digest of another kind under that name. + // TODO: record packages whose digest is not a SHA256 one, such as the ones + // coming from a store. This also requires recording the release unique + // package name instead of the source one, as slices are recorded with the + // former. + if pkg.DigestKind != cache.SHA256 { + return fmt.Errorf("package %q has unsupported digest kind %q", pkg.Name, pkg.DigestKind) + } + if pkg.Digest == "" { + return fmt.Errorf("package %q missing digest", pkg.Name) } if pkg.Version == "" { return fmt.Errorf("package %q missing version", pkg.Name) } + if pkg.Revision < 0 { + return fmt.Errorf("package %q has invalid revision", pkg.Name) + } return nil } diff --git a/internal/manifestutil/manifestutil_test.go b/internal/manifestutil/manifestutil_test.go index 2bab0a684..a3e8fa284 100644 --- a/internal/manifestutil/manifestutil_test.go +++ b/internal/manifestutil/manifestutil_test.go @@ -12,8 +12,9 @@ import ( . "gopkg.in/check.v1" "github.com/canonical/chisel/internal/apachetestutil" - "github.com/canonical/chisel/internal/archive" + "github.com/canonical/chisel/internal/cache" "github.com/canonical/chisel/internal/manifestutil" + "github.com/canonical/chisel/internal/pkgutil" "github.com/canonical/chisel/internal/setup" "github.com/canonical/chisel/public/manifest" ) @@ -122,7 +123,7 @@ var slice2 = &setup.Slice{ var generateManifestTests = []struct { summary string report *manifestutil.Report - packageInfo []*archive.PackageInfo + packageInfo []*pkgutil.Info selection []*setup.Slice expected *apachetestutil.ManifestContents error string @@ -148,16 +149,18 @@ var generateManifestTests = []struct { }, }, }, - packageInfo: []*archive.PackageInfo{{ - Name: "package1", - Version: "v1", - Arch: "a1", - SHA256: "s1", + packageInfo: []*pkgutil.Info{{ + Name: "package1", + Version: "v1", + Arch: "a1", + DigestKind: cache.SHA256, + Digest: "s1", }, { - Name: "package2", - Version: "v2", - Arch: "a2", - SHA256: "s2", + Name: "package2", + Version: "v2", + Arch: "a2", + DigestKind: cache.SHA256, + Digest: "s2", }}, expected: &apachetestutil.ManifestContents{ Paths: []*manifest.Path{{ @@ -241,7 +244,7 @@ var generateManifestTests = []struct { }, }, }, - packageInfo: []*archive.PackageInfo{}, + packageInfo: []*pkgutil.Info{}, error: `internal error: invalid manifest: slice package1_slice1 refers to missing package "package1"`, }, { summary: "Invalid path: slices is empty", @@ -395,11 +398,12 @@ var generateManifestTests = []struct { }, }, }, - packageInfo: []*archive.PackageInfo{{ - Name: "package1", - Version: "v1", - Arch: "a1", - SHA256: "s1", + packageInfo: []*pkgutil.Info{{ + Name: "package1", + Version: "v1", + Arch: "a1", + DigestKind: cache.SHA256, + Digest: "s1", }}, expected: &apachetestutil.ManifestContents{ Paths: []*manifest.Path{{ @@ -494,36 +498,59 @@ var generateManifestTests = []struct { error: `internal error: invalid manifest: hard linked paths "/file" and "/hardlink" have diverging contents`, }, { summary: "Invalid package: missing name", - packageInfo: []*archive.PackageInfo{{ - Version: "v1", - Arch: "a1", - SHA256: "s1", + packageInfo: []*pkgutil.Info{{ + Version: "v1", + Arch: "a1", + DigestKind: cache.SHA256, + Digest: "s1", }}, error: `internal error: invalid manifest: package name not set`, }, { summary: "Invalid package: missing version", - packageInfo: []*archive.PackageInfo{{ - Name: "package-1", - Arch: "a1", - SHA256: "s1", + packageInfo: []*pkgutil.Info{{ + Name: "package-1", + Arch: "a1", + DigestKind: cache.SHA256, + Digest: "s1", }}, error: `internal error: invalid manifest: package "package-1" missing version`, }, { summary: "Invalid package: missing arch", - packageInfo: []*archive.PackageInfo{{ - Name: "package-1", - Version: "v1", - SHA256: "s1", + packageInfo: []*pkgutil.Info{{ + Name: "package-1", + Version: "v1", + DigestKind: cache.SHA256, + Digest: "s1", }}, error: `internal error: invalid manifest: package "package-1" missing arch`, }, { - summary: "Invalid package: missing sha256", - packageInfo: []*archive.PackageInfo{{ + summary: "Invalid package: missing digest kind", + packageInfo: []*pkgutil.Info{{ Name: "package-1", Version: "v1", Arch: "a1", + Digest: "s1", + }}, + error: `internal error: invalid manifest: package "package-1" has unsupported digest kind ""`, +}, { + summary: "Invalid package: unsupported digest kind", + packageInfo: []*pkgutil.Info{{ + Name: "package-1", + Version: "v1", + Arch: "a1", + DigestKind: cache.SHA384, + Digest: "s1", + }}, + error: `internal error: invalid manifest: package "package-1" has unsupported digest kind "sha384"`, +}, { + summary: "Invalid package: missing digest", + packageInfo: []*pkgutil.Info{{ + Name: "package-1", + Version: "v1", + Arch: "a1", + DigestKind: cache.SHA256, }}, - error: `internal error: invalid manifest: package "package-1" missing sha256`, + error: `internal error: invalid manifest: package "package-1" missing digest`, }} func (s *S) TestGenerateManifests(c *C) { @@ -533,11 +560,12 @@ func (s *S) TestGenerateManifests(c *C) { test.selection = []*setup.Slice{slice1} } if test.packageInfo == nil { - test.packageInfo = []*archive.PackageInfo{{ - Name: "package1", - Version: "v1", - Arch: "a1", - SHA256: "s1", + test.packageInfo = []*pkgutil.Info{{ + Name: "package1", + Version: "v1", + Arch: "a1", + DigestKind: cache.SHA256, + Digest: "s1", }} } diff --git a/internal/pkgutil/pkgutil.go b/internal/pkgutil/pkgutil.go new file mode 100644 index 000000000..cff98dba7 --- /dev/null +++ b/internal/pkgutil/pkgutil.go @@ -0,0 +1,19 @@ +// Package pkgutil provides types and helpers describing packages +// independently of where they come from (a Debian archive, a store, ...). +package pkgutil + +import ( + "github.com/canonical/chisel/internal/cache" +) + +// Info describes a package as obtained from its source. +type Info struct { + Name string // Name as known by the source (e.g. "curl") + Version string + // Revision further identifies the package when the source versions are not + // unique on their own. It is zero when the source does not use revisions. + Revision int + Arch string + DigestKind cache.DigestKind + Digest string +} diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 6f4783b88..694fab6ff 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -18,6 +18,7 @@ import ( "github.com/canonical/chisel/internal/archive" "github.com/canonical/chisel/internal/fsutil" "github.com/canonical/chisel/internal/manifestutil" + "github.com/canonical/chisel/internal/pkgutil" "github.com/canonical/chisel/internal/scripts" "github.com/canonical/chisel/internal/setup" "github.com/canonical/chisel/internal/tarball" @@ -147,7 +148,7 @@ func Run(options *RunOptions) error { // Fetch all packages, using the selection order. packages := make(map[string]io.ReadSeekCloser) - var pkgInfos []*archive.PackageInfo + var pkgInfos []*pkgutil.Info for _, slice := range options.Selection.Slices { if packages[slice.Package] != nil { continue @@ -352,7 +353,8 @@ func Run(options *RunOptions) error { } func generateManifests(targetDir string, selection *setup.Selection, - report *manifestutil.Report, pkgInfos []*archive.PackageInfo) error { + report *manifestutil.Report, pkgInfos []*pkgutil.Info, +) error { manifestSlices := manifestutil.FindPaths(selection.Slices) if len(manifestSlices) == 0 { // Nothing to do. diff --git a/internal/testutil/archive.go b/internal/testutil/archive.go index d06fd1b0c..ddca19595 100644 --- a/internal/testutil/archive.go +++ b/internal/testutil/archive.go @@ -6,6 +6,8 @@ import ( "io" "github.com/canonical/chisel/internal/archive" + "github.com/canonical/chisel/internal/cache" + "github.com/canonical/chisel/internal/pkgutil" ) type TestArchive struct { @@ -26,18 +28,12 @@ func (a *TestArchive) Options() *archive.Options { return &a.Opts } -func (a *TestArchive) Fetch(pkgName string) (io.ReadSeekCloser, *archive.PackageInfo, error) { +func (a *TestArchive) Fetch(pkgName string) (io.ReadSeekCloser, *pkgutil.Info, error) { pkg, ok := a.Packages[pkgName] if !ok { return nil, nil, fmt.Errorf("cannot find package %q in archive", pkgName) } - info := &archive.PackageInfo{ - Name: pkg.Name, - Version: pkg.Version, - SHA256: pkg.Hash, - Arch: pkg.Arch, - } - return ReadSeekNopCloser(bytes.NewReader(pkg.Data)), info, nil + return ReadSeekNopCloser(bytes.NewReader(pkg.Data)), pkg.info(), nil } func (a *TestArchive) Exists(pkg string) bool { @@ -45,15 +41,21 @@ func (a *TestArchive) Exists(pkg string) bool { return ok } -func (a *TestArchive) Info(pkgName string) (*archive.PackageInfo, error) { +func (a *TestArchive) Info(pkgName string) (*pkgutil.Info, error) { pkg, ok := a.Packages[pkgName] if !ok { return nil, fmt.Errorf("cannot find package %q in archive", pkgName) } - return &archive.PackageInfo{ - Name: pkg.Name, - Version: pkg.Version, - SHA256: pkg.Hash, - Arch: pkg.Arch, - }, nil + return pkg.info(), nil +} + +// info returns the package information as a package source would report it. +func (p *TestPackage) info() *pkgutil.Info { + return &pkgutil.Info{ + Name: p.Name, + Version: p.Version, + Arch: p.Arch, + DigestKind: cache.SHA256, + Digest: p.Hash, + } }