From 0d5d78ccf0b91055e1919831b29f6f6291249ccc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Virtus?= Date: Sun, 18 Jun 2023 00:46:03 +0200 Subject: [PATCH 1/3] slicer/test: Support for custom packages Currently we only test with the embedded base-files files package. This quite limits what we can test. But since commit a3315e3 ("testutil/pkgdata: Create deb programatically") we have the ability to define our own custom package content. Add support for testing with custom packages. These can be defined in each test case per archive name. The content can be defined directly in test case definition with the help of testutil.MustMakeDeb(). The package content is wrapped in the testPackage structure. This introduces one level of nesting in each test case package definition. In future, we will add package metadata to the package definition, namely version. So wrapping the package content in the structure now and later just adding a new field to it the structure later will make the future diff much smaller. --- internal/slicer/slicer_test.go | 112 ++++++++++++++++++++++++++++++--- 1 file changed, 104 insertions(+), 8 deletions(-) diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index aa9d0f51..471d64a1 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -16,9 +16,20 @@ import ( "github.com/canonical/chisel/internal/testutil" ) +var ( + Reg = testutil.Reg + Dir = testutil.Dir + Lnk = testutil.Lnk +) + +type testPackage struct { + content []byte +} + type slicerTest struct { summary string arch string + pkgs map[string]map[string]testPackage release map[string]string slices []setup.SliceKey hackopt func(c *C, opts *slicer.RunOptions) @@ -515,6 +526,77 @@ var slicerTests = []slicerTest{{ "/usr/bin/": "dir 0755", "/usr/bin/hello": "file 0775 eaf29575", }, +}, { + summary: "Custom archives with custom packages", + pkgs: map[string]map[string]testPackage{ + "leptons": { + "electron": testPackage{ + content: testutil.MustMakeDeb([]testutil.TarEntry{ + Dir(0755, "./"), + Dir(0755, "./mass/"), + Reg(0644, "./mass/electron", "9.1093837015E−31 kg\n"), + Dir(0755, "./usr/"), + Dir(0755, "./usr/share/"), + Dir(0755, "./usr/share/doc/"), + Dir(0755, "./usr/share/doc/electron/"), + Reg(0644, "./usr/share/doc/electron/copyright", ""), + }), + }, + }, + "hadrons": { + "proton": testPackage{ + content: testutil.MustMakeDeb([]testutil.TarEntry{ + Dir(0755, "./"), + Dir(0755, "./mass/"), + Reg(0644, "./mass/proton", "1.67262192369E−27 kg\n"), + }), + }, + }, + }, + release: map[string]string{ + "chisel.yaml": ` + format: chisel-v1 + archives: + leptons: + version: 1 + suites: [main] + components: [main, universe] + default: true + hadrons: + version: 1 + suites: [main] + components: [main] + `, + "slices/mydir/electron.yaml": ` + package: electron + slices: + mass: + contents: + /mass/electron: + `, + "slices/mydir/proton.yaml": ` + package: proton + archive: hadrons + slices: + mass: + contents: + /mass/proton: + `, + }, + slices: []setup.SliceKey{ + {"electron", "mass"}, + {"proton", "mass"}, + }, + result: map[string]string{ + "/mass/": "dir 0755", + "/mass/electron": "file 0644 a1258e30", + "/mass/proton": "file 0644 a2390d10", + "/usr/": "dir 0755", + "/usr/share/": "dir 0755", + "/usr/share/doc/": "dir 0755", + "/usr/share/doc/electron/": "dir 0755", + "/usr/share/doc/electron/copyright": "file 0644 empty", + }, }} const defaultChiselYaml = ` @@ -527,7 +609,7 @@ const defaultChiselYaml = ` type testArchive struct { options archive.Options - pkgs map[string][]byte + pkgs map[string]testPackage } func (a *testArchive) Options() *archive.Options { @@ -536,7 +618,7 @@ func (a *testArchive) Options() *archive.Options { func (a *testArchive) Fetch(pkg string) (io.ReadCloser, error) { if data, ok := a.pkgs[pkg]; ok { - return io.NopCloser(bytes.NewBuffer(data)), nil + return io.NopCloser(bytes.NewBuffer(data.content)), nil } return nil, fmt.Errorf("attempted to open %q package", pkg) } @@ -569,16 +651,23 @@ func (s *S) TestRun(c *C) { selection, err := setup.Select(release, test.slices) c.Assert(err, IsNil) - pkgs := map[string][]byte{ - "base-files": testutil.PackageData["base-files"], + pkgs := map[string]testPackage{ + "base-files": testPackage{content: testutil.PackageData["base-files"]}, } for name, entries := range packageEntries { deb, err := testutil.MakeDeb(entries) c.Assert(err, IsNil) - pkgs[name] = deb + pkgs[name] = testPackage{content: deb} } archives := map[string]archive.Archive{} for name, setupArchive := range release.Archives { + var archivePkgs map[string]testPackage + if test.pkgs != nil { + archivePkgs = test.pkgs[name] + } + if archivePkgs == nil { + archivePkgs = pkgs + } archive := &testArchive{ options: archive.Options{ Label: setupArchive.Name, @@ -587,7 +676,7 @@ func (s *S) TestRun(c *C) { Components: setupArchive.Components, Arch: test.arch, }, - pkgs: pkgs, + pkgs: archivePkgs, } archives[name] = archive } @@ -611,8 +700,15 @@ func (s *S) TestRun(c *C) { if test.result != nil { result := make(map[string]string, len(copyrightEntries)+len(test.result)) - for k, v := range copyrightEntries { - result[k] = v + if test.pkgs == nil { + // This was added in order to not specify copyright entries for each + // existing test. These tests use only the base-files embedded + // package. Custom packages may not include copyright entries + // though. So if a test defines any custom packages, it must include + // copyright entries explicitly in the results. + for k, v := range copyrightEntries { + result[k] = v + } } for k, v := range test.result { result[k] = v From 19c9af6869a302d4b07a4d3a5ee23530818733ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Virtus?= Date: Fri, 16 Jun 2023 07:32:31 +0200 Subject: [PATCH 2/3] archive: Add Info() method At some point we will need to get information about (deb) packages from (Go) packages other than archive, namely from the slicer package. For instance: - Package entries in Chisel DB will include version, digest and architecture. - Multi-archive package selection will need to know versions of a package in all configured archives. Add Info() method to the Archive interface that returns a new PackageInfo interface that contains accessors for the underlying control.Section of the package. Currently these accessors are Name(), Version(), Arch() and SHA256() methods. The reason to introduce PackageInfo interface instead of returing control.Section directly is keep the archive abstraction and not leak implementation details. --- internal/archive/archive.go | 24 +++++++++++++++++++++++ internal/archive/archive_test.go | 33 ++++++++++++++++++++++++++++++++ internal/slicer/slicer_test.go | 29 ++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+) diff --git a/internal/archive/archive.go b/internal/archive/archive.go index 63badc8f..56a3fcd9 100644 --- a/internal/archive/archive.go +++ b/internal/archive/archive.go @@ -13,9 +13,17 @@ import ( "github.com/canonical/chisel/internal/deb" ) +type PackageInfo interface { + Name() string + Version() string + Arch() string + SHA256() string +} + type Archive interface { Options() *Options Fetch(pkg string) (io.ReadCloser, error) + Info(pkg string) PackageInfo Exists(pkg string) bool } @@ -86,6 +94,22 @@ func (a *ubuntuArchive) Exists(pkg string) bool { return err == nil } +type pkgInfo struct{ control.Section } + +var _ PackageInfo = pkgInfo{} + +func (info pkgInfo) Name() string { return info.Get("Package") } +func (info pkgInfo) Version() string { return info.Get("Version") } +func (info pkgInfo) Arch() string { return info.Get("Architecture") } +func (info pkgInfo) SHA256() string { return info.Get("SHA256") } + +func (a *ubuntuArchive) Info(pkg string) PackageInfo { + if section, _, _ := a.selectPackage(pkg); section != nil { + return &pkgInfo{section} + } + return nil +} + func (a *ubuntuArchive) selectPackage(pkg string) (control.Section, *ubuntuIndex, error) { var selectedVersion string var selectedSection control.Section diff --git a/internal/archive/archive_test.go b/internal/archive/archive_test.go index cb9f3278..b18fc60c 100644 --- a/internal/archive/archive_test.go +++ b/internal/archive/archive_test.go @@ -332,6 +332,39 @@ func (s *httpSuite) TestArchiveLabels(c *C) { c.Assert(err, ErrorMatches, `.*\bno Ubuntu section`) } +func (s *httpSuite) TestPackageInfo(c *C) { + s.prepareArchive("jammy", "22.04", "amd64", []string{"main", "universe"}) + + options := archive.Options{ + Label: "ubuntu", + Version: "22.04", + Arch: "amd64", + Suites: []string{"jammy"}, + Components: []string{"main", "universe"}, + CacheDir: c.MkDir(), + } + + archive, err := archive.Open(&options) + c.Assert(err, IsNil) + + info1 := archive.Info("mypkg1") + c.Assert(info1, NotNil) + c.Assert(info1.Name(), Equals, "mypkg1") + c.Assert(info1.Version(), Equals, "1.1") + c.Assert(info1.Arch(), Equals, "amd64") + c.Assert(info1.SHA256(), Equals, "1f08ef04cfe7a8087ee38a1ea35fa1810246648136c3c42d5a61ad6503d85e05") + + info3 := archive.Info("mypkg3") + c.Assert(info3, NotNil) + c.Assert(info3.Name(), Equals, "mypkg3") + c.Assert(info3.Version(), Equals, "1.3") + c.Assert(info3.Arch(), Equals, "amd64") + c.Assert(info3.SHA256(), Equals, "fe377bf13ba1a5cb287cb4e037e6e7321281c929405ae39a72358ef0f5d179aa") + + info99 := archive.Info("mypkg99") + c.Assert(info99, IsNil) +} + func read(r io.Reader) string { data, err := io.ReadAll(r) if err != nil { diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index 471d64a1..cdb68f12 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -23,6 +23,7 @@ var ( ) type testPackage struct { + info map[string]string content []byte } @@ -607,6 +608,22 @@ const defaultChiselYaml = ` components: [main, universe] ` +type testPackageInfo map[string]string + +var _ archive.PackageInfo = (testPackageInfo)(nil) + +func (info testPackageInfo) Name() string { return info["Package"] } +func (info testPackageInfo) Version() string { return info["Version"] } +func (info testPackageInfo) Arch() string { return info["Architecture"] } +func (info testPackageInfo) SHA256() string { return info["SHA256"] } + +func (s testPackageInfo) Get(key string) (value string) { + if s != nil { + value = s[key] + } + return +} + type testArchive struct { options archive.Options pkgs map[string]testPackage @@ -628,6 +645,18 @@ func (a *testArchive) Exists(pkg string) bool { return ok } +func (a *testArchive) Info(pkg string) archive.PackageInfo { + var info map[string]string + if pkgData, ok := a.pkgs[pkg]; ok { + if info = pkgData.info; info == nil { + info = map[string]string{ + "Version": "1.0", + } + } + } + return testPackageInfo(info) +} + func (s *S) TestRun(c *C) { for _, test := range slicerTests { c.Logf("Summary: %s", test.summary) From 7febfeb63e4f01af41393ef995e5d2be2902ecc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Virtus?= Date: Thu, 1 Jun 2023 23:44:33 +0200 Subject: [PATCH 3/3] fsutil: Add SlashedPathDir() and SlashedPathClean() functions From the code: These functions exists because we work with slash terminated directory paths that come from deb package tarballs but standard library path functions treat slash terminated paths as unclean. We use ad-hoc code to make filepath.Dir() slash terminated in two places right now. For example this code targetDir := filepath.Dir(strings.TrimRight(targetPath, "/")) + "/" is not strictly correct as "/a/b/.///" will be turned into "/a/b/./" which is still "/a/b". Since we deal with slash terminated directory paths everywhere, create and use dedicated helper functions to process them. --- internal/fsutil/path.go | 66 ++++++++++++++++++++++++++++++++++++ internal/fsutil/path_test.go | 54 +++++++++++++++++++++++++++++ internal/slicer/slicer.go | 18 ++++------ 3 files changed, 127 insertions(+), 11 deletions(-) create mode 100644 internal/fsutil/path.go create mode 100644 internal/fsutil/path_test.go diff --git a/internal/fsutil/path.go b/internal/fsutil/path.go new file mode 100644 index 00000000..34469b46 --- /dev/null +++ b/internal/fsutil/path.go @@ -0,0 +1,66 @@ +package fsutil + +import ( + "path/filepath" +) + +// isDirPath returns whether the path refers to a directory. +// The path refers to a directory when it ends with "/", "/." or "/..", or when +// it equals "." or "..". +func isDirPath(path string) bool { + i := len(path) - 1 + if i < 0 { + return true + } + if path[i] == '.' { + i-- + if i < 0 { + return true + } + if path[i] == '.' { + i-- + if i < 0 { + return true + } + } + } + if path[i] == '/' { + return true + } + return false +} + +// Debian package tarballs present paths slightly differently to what we would +// normally classify as clean paths. While a traditional clean file path is identical +// to a clean deb package file path, the deb package directory path always ends +// with a slash. Although the change only affects directory paths, the implication +// is that a directory path without a slash is interpreted as a file path. For this +// reason, we need to be very careful and handle both file and directory paths using +// a new set of functions. We call this new path type a Slashed Path. A slashed path +// allows us to identify a file or directory simply using lexical analysis. + +// SlashedPathClean takes a file or slashed directory path as input, and produces +// the shortest equivalent as output. An input path ending without a slash will be +// interpreted as a file path. Directory paths should always end with a slash. +// These functions exists because we work with slash terminated directory paths +// that come from deb package tarballs but standard library path functions +// treat slash terminated paths as unclean. +func SlashedPathClean(path string) string { + clean := filepath.Clean(path) + if clean != "/" && isDirPath(path) { + clean += "/" + } + return clean +} + +// SlashedPathDir takes a file or slashed directory path as input, cleans the +// path and returns the parent directory. An input path ending without a slash +// will be interpreted as a file path. Directory paths should always end with a slash. +// Clean is like filepath.Clean() but trailing slash is kept. +func SlashedPathDir(path string) string { + parent := filepath.Dir(filepath.Clean(path)) + if parent != "/" { + parent += "/" + } + return parent +} diff --git a/internal/fsutil/path_test.go b/internal/fsutil/path_test.go new file mode 100644 index 00000000..864ec594 --- /dev/null +++ b/internal/fsutil/path_test.go @@ -0,0 +1,54 @@ +package fsutil_test + +import ( + . "gopkg.in/check.v1" + + "github.com/canonical/chisel/internal/fsutil" +) + +var cleanAndDirTestCases = []struct { + inputPath string + resultClean string + resultDir string +}{ + {"/a/b/c", "/a/b/c", "/a/b/"}, + {"/a/b/c/", "/a/b/c/", "/a/b/"}, + {"/a/b/c//", "/a/b/c/", "/a/b/"}, + {"/a/b//c", "/a/b/c", "/a/b/"}, + {"/a/b/c/.", "/a/b/c/", "/a/b/"}, + {"/a/b/c/.///.", "/a/b/c/", "/a/b/"}, + {"/a/b/./c/", "/a/b/c/", "/a/b/"}, + {"/a/b/.///./c", "/a/b/c", "/a/b/"}, + {"/a/b/c/..", "/a/b/", "/a/"}, + {"/a/b/c/..///./", "/a/b/", "/a/"}, + {"/a/b/c/../.", "/a/b/", "/a/"}, + {"/a/b/../c/", "/a/c/", "/a/"}, + {"/a/b/..///./c", "/a/c", "/a/"}, + {"a/b/./c", "a/b/c", "a/b/"}, + {"./a/b/./c", "a/b/c", "a/b/"}, + {"/", "/", "/"}, + {"///", "/", "/"}, + {"///.///", "/", "/"}, + {"/././.", "/", "/"}, + {".", "./", "./"}, + {".///", "./", "./"}, + {"..", "../", "./"}, + {"..///.", "../", "./"}, + {"../../..", "../../../", "../../"}, + {"..///.///../..", "../../../", "../../"}, + {"", "./", "./"}, +} + +func (s *S) TestSlashedPathClean(c *C) { + for _, t := range cleanAndDirTestCases { + c.Logf("%s => %s", t.inputPath, t.resultClean) + c.Assert(fsutil.SlashedPathClean(t.inputPath), Equals, t.resultClean) + } +} + +func (s *S) TestSlashedPathDir(c *C) { + for _, t := range cleanAndDirTestCases { + c.Logf("%s => %s", t.inputPath, t.resultDir) + c.Assert(fsutil.SlashedPathDir(t.inputPath), Equals, t.resultDir) + } +} diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 2b684b5d..9cd35aec 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -32,25 +32,21 @@ func Run(options *RunOptions) error { knownPaths["/"] = true + // addKnownPath path adds path and all its directory parent paths into + // knownPaths set. addKnownPath := func(path string) { if path[0] != '/' { panic("bug: tried to add relative path to known paths") } - cleanPath := filepath.Clean(path) - slashPath := cleanPath - if path[len(path)-1] == '/' && cleanPath != "/" { - slashPath += "/" - } + path = fsutil.SlashedPathClean(path) for { - if _, ok := knownPaths[slashPath]; ok { + if _, ok := knownPaths[path]; ok { break } - knownPaths[slashPath] = true - cleanPath = filepath.Dir(cleanPath) - if cleanPath == "/" { + knownPaths[path] = true + if path = fsutil.SlashedPathDir(path); path == "/" { break } - slashPath = cleanPath + "/" } } @@ -113,7 +109,7 @@ func Run(options *RunOptions) error { hasCopyright = true } } else { - targetDir := filepath.Dir(strings.TrimRight(targetPath, "/")) + "/" + targetDir := fsutil.SlashedPathDir(targetPath) if targetDir == "" || targetDir == "/" { continue }