diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 35c81a79..104c3b84 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -1,7 +1,10 @@ package main import ( + "errors" "fmt" + "os" + "path/filepath" "slices" "time" @@ -11,6 +14,7 @@ import ( "github.com/canonical/chisel/internal/cache" "github.com/canonical/chisel/internal/setup" "github.com/canonical/chisel/internal/slicer" + "github.com/canonical/chisel/public/manifest" ) var shortCutHelp = "Cut a tree with selected slices" @@ -73,6 +77,35 @@ func (cmd *cmdCut) Execute(args []string) error { } } + targetDir := filepath.Clean(cmd.RootDir) + if !filepath.IsAbs(targetDir) { + dir, err := os.Getwd() + if err != nil { + return fmt.Errorf("cannot obtain current directory: %w", err) + } + targetDir = filepath.Join(dir, targetDir) + } + + var mfest *manifest.Manifest + // TODO: Remove this gating once the final upgrading strategy is in place. + if os.Getenv("CHISEL_RECUT_EXPERIMENTAL") != "" { + mfest, err = slicer.SelectValidManifest(targetDir, release) + if err == nil { + err = mfest.IterateSlices("", func(slice *manifest.Slice) error { + sk, err := setup.ParseSliceKey(slice.Name) + if err != nil { + return err + } + sliceKeys = append(sliceKeys, sk) + return nil + }) + if err != nil { + return err + } + } else if !errors.Is(err, slicer.ErrNoManifest) { + return err + } + } selection, err := setup.Select(release, sliceKeys, cmd.Arch) if err != nil { return err @@ -124,7 +157,7 @@ func (cmd *cmdCut) Execute(args []string) error { err = slicer.Run(&slicer.RunOptions{ Selection: selection, Archives: archives, - TargetDir: cmd.RootDir, + TargetDir: targetDir, }) return err } diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index 16b05402..46cc27ea 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -4,6 +4,7 @@ import ( "fmt" "io" "io/fs" + "maps" "path/filepath" "slices" "sort" @@ -18,22 +19,60 @@ import ( const DefaultFilename = "manifest.wall" +// validateManifestPath validates that the path follows the following format: +// - /slashed/path/to/dir/**: {generate: manifest} +// +// Wildcard characters can only appear at the end as **, and the path before +// those wildcards must be a directory. +func validateManifestPath(path string, info setup.PathInfo) (string, error) { + if info.Generate != setup.GenerateManifest { + return "", fmt.Errorf("%s generate property not set to 'manifest'", path) + } + if !strings.HasSuffix(path, "/**") { + return "", fmt.Errorf("%s does not end with /**", path) + } + dirPath := strings.TrimSuffix(path, "**") + if strings.ContainsAny(dirPath, "*?") { + return "", fmt.Errorf("%s contains wildcard characters in addition to trailing **", path) + } + return dirPath, nil +} + // FindPaths finds the paths marked with "generate:manifest" and // returns a map from the manifest path to all the slices that declare it. func FindPaths(slices []*setup.Slice) map[string][]*setup.Slice { manifestSlices := make(map[string][]*setup.Slice) for _, slice := range slices { for path, info := range slice.Contents { - if info.Generate == setup.GenerateManifest { - dir := strings.TrimSuffix(path, "**") - path = filepath.Join(dir, DefaultFilename) - manifestSlices[path] = append(manifestSlices[path], slice) + dir, err := validateManifestPath(path, info) + if err != nil { + continue } + path = filepath.Join(dir, DefaultFilename) + manifestSlices[path] = append(manifestSlices[path], slice) } } return manifestSlices } +// FindPathsInRelease finds all the valid manifest paths for the given release. +func FindPathsInRelease(r *setup.Release) []string { + manifestPaths := make(map[string]bool) + for _, pkg := range r.Packages { + for _, slice := range pkg.Slices { + for path, info := range slice.Contents { + dir, err := validateManifestPath(path, info) + if err != nil { + continue + } + path = filepath.Join(dir, DefaultFilename) + manifestPaths[path] = true + } + } + } + return slices.Sorted(maps.Keys(manifestPaths)) +} + type WriteOptions struct { PackageInfo []*archive.PackageInfo Selection []*setup.Slice diff --git a/internal/manifestutil/manifestutil_test.go b/internal/manifestutil/manifestutil_test.go index 2bab0a68..bf6eb6fd 100644 --- a/internal/manifestutil/manifestutil_test.go +++ b/internal/manifestutil/manifestutil_test.go @@ -110,10 +110,163 @@ func (s *S) TestFindPaths(c *C) { } } +var findPathsInReleaseTests = []struct { + summary string + release *setup.Release + expected []string +}{{ + summary: "Single package with single slice", + release: &setup.Release{ + Packages: map[string]*setup.Package{ + "package1": { + Name: "package1", + Slices: map[string]*setup.Slice{ + "slice1": { + Name: "slice1", + Contents: map[string]setup.PathInfo{ + "/folder/**": { + Kind: "generate", + Generate: "manifest", + }, + }, + }, + }, + }, + }, + }, + expected: []string{"/folder/manifest.wall"}, +}, { + summary: "No slices with generate:manifest", + release: &setup.Release{ + Packages: map[string]*setup.Package{ + "package1": { + Name: "package1", + Slices: map[string]*setup.Slice{ + "slice1": { + Name: "slice1", + Contents: map[string]setup.PathInfo{}, + }, + }, + }, + }, + }, + expected: nil, +}, { + summary: "Slice with invalid generate property", + release: &setup.Release{ + Packages: map[string]*setup.Package{ + "package1": { + Name: "package1", + Slices: map[string]*setup.Slice{ + "slice1": { + Name: "slice1", + Contents: map[string]setup.PathInfo{ + "/folder/**": { + Kind: "generate", + Generate: "invalid", + }, + }, + }, + }, + }, + }, + }, + expected: nil, +}, { + summary: "Slice with invalid path", + release: &setup.Release{ + Packages: map[string]*setup.Package{ + "package1": { + Name: "package1", + Slices: map[string]*setup.Slice{ + "slice1": { + Name: "slice1", + Contents: map[string]setup.PathInfo{ + "/folder/foo*/**": { + Kind: "generate", + Generate: "manifest", + }, + }, + }, + }, + }, + }, + }, + expected: nil, +}, { + summary: "Multiple packages with multiple slices", + release: &setup.Release{ + Packages: map[string]*setup.Package{ + "package1": { + Name: "package1", + Slices: map[string]*setup.Slice{ + "slice1": { + Name: "slice1", + Contents: map[string]setup.PathInfo{ + "/folder/**": { + Kind: "generate", + Generate: "manifest", + }, + }, + }, + "slice2": { + Name: "slice2", + Contents: map[string]setup.PathInfo{ + "/folder/**": { + Kind: "generate", + Generate: "manifest", + }, + }, + }, + }, + }, + "package2": { + Name: "package2", + Slices: map[string]*setup.Slice{ + "slice3": { + Name: "slice3", + Contents: map[string]setup.PathInfo{}, + }, + "slice4": { + Name: "slice4", + Contents: map[string]setup.PathInfo{ + "/other-folder/**": { + Kind: "generate", + Generate: "manifest", + }, + }, + }, + }, + }, + }, + }, + expected: []string{"/folder/manifest.wall", "/other-folder/manifest.wall"}, +}, { + summary: "Empty release", + release: &setup.Release{ + Packages: map[string]*setup.Package{}, + }, + expected: nil, +}} + +func (s *S) TestFindPathsInRelease(c *C) { + for _, test := range findPathsInReleaseTests { + c.Logf("Summary: %s", test.summary) + + manifestPaths := manifestutil.FindPathsInRelease(test.release) + + c.Assert(manifestPaths, HasLen, len(test.expected)) + slices.Sort(manifestPaths) + slices.Sort(test.expected) + c.Assert(manifestPaths, DeepEquals, test.expected) + } +} + var slice1 = &setup.Slice{ Package: "package1", Name: "slice1", } + var slice2 = &setup.Slice{ Package: "package2", Name: "slice2", diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 9d3447fb..24c17bf7 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -3,6 +3,9 @@ package slicer import ( "archive/tar" "bytes" + "crypto/sha256" + "encoding/hex" + "errors" "fmt" "io" "io/fs" @@ -21,6 +24,7 @@ import ( "github.com/canonical/chisel/internal/manifestutil" "github.com/canonical/chisel/internal/scripts" "github.com/canonical/chisel/internal/setup" + "github.com/canonical/chisel/public/manifest" ) const manifestMode fs.FileMode = 0644 @@ -83,11 +87,7 @@ func Run(options *RunOptions) error { targetDir := filepath.Clean(options.TargetDir) if !filepath.IsAbs(targetDir) { - dir, err := os.Getwd() - if err != nil { - return fmt.Errorf("cannot obtain current directory: %w", err) - } - targetDir = filepath.Join(dir, targetDir) + return fmt.Errorf("internal error: cannot use a relative target directory %s", targetDir) } pkgArchive, err := selectPkgArchives(options.Archives, options.Selection) @@ -537,3 +537,103 @@ func selectPkgArchives(archives map[string]archive.Archive, selection *setup.Sel } return pkgArchive, nil } + +var ErrNoManifest = errors.New("cannot find valid manifest file") + +// SelectValidManifest looks in the targetDir for manifests declared in the +// release, reads and validates those found. The selection ensures that all +// valid manifests found are identical, so only one is kept and returned. +// +// A file matching a manifest path declared in the release is considered valid +// if it is a zstd file, readable by manifest.Read (with a known schema +// version) and successfully validated by manifestutil.Validate. +// +// Finding only manifests with unknown schema version means the targetDir may +// have been produced by Chisel, but possibly by a future/incompatible version. +// Chisel cannot safely proceed and so errors out. +func SelectValidManifest(targetDir string, release *setup.Release) (*manifest.Manifest, error) { + targetDir = filepath.Clean(targetDir) + if !filepath.IsAbs(targetDir) { + return nil, fmt.Errorf("internal error: cannot use a relative target directory %s", targetDir) + } + manifestPaths := manifestutil.FindPathsInRelease(release) + if len(manifestPaths) == 0 { + return nil, ErrNoManifest + } + + var selected *manifest.Manifest + var selectedHash string + var selectedPath string + foundUnknownSchema := false + for _, manifestPath := range manifestPaths { + manifestFullPath := filepath.Join(targetDir, manifestPath) + mfest, manifestHash, err := tryLoadManifest(manifestFullPath) + if err != nil { + if os.IsNotExist(err) { + continue + } + var unknownSchemaError *manifest.UnknownSchemaError + if errors.As(err, &unknownSchemaError) { + foundUnknownSchema = true + // Ignore manifests with unknown (potentially future) schema versions. + continue + } + return nil, err + } + if selected == nil { + selected = mfest + selectedHash = manifestHash + selectedPath = manifestPath + } else if selectedHash != manifestHash { + return nil, fmt.Errorf("cannot select between conflicting manifests: %q != %q", selectedPath, manifestPath) + } + } + if selected == nil { + if foundUnknownSchema { + return nil, fmt.Errorf("cannot select a manifest: schema version is unknown") + } else { + return nil, ErrNoManifest + } + } + return selected, nil +} + +func tryLoadManifest(manifestFullPath string) (*manifest.Manifest, string, error) { + f, err := os.Open(manifestFullPath) + if err != nil { + return nil, "", err + } + defer f.Close() + r, err := zstd.NewReader(f) + if err != nil { + return nil, "", err + } + defer r.Close() + mfest, err := manifest.Read(r) + if err != nil { + return nil, "", err + } + err = manifestutil.Validate(mfest) + if err != nil { + return nil, "", err + } + manifestHash, err := contentHash(manifestFullPath) + if err != nil { + return nil, "", fmt.Errorf("cannot compute hash for %q: %s", manifestFullPath, err) + } + return mfest, manifestHash, nil +} + +func contentHash(path string) (string, error) { + f, err := os.Open(path) + if err != nil { + return "", err + } + defer f.Close() + + h := sha256.New() + if _, err := io.Copy(h, f); err != nil { + return "", err + } + return hex.EncodeToString(h.Sum(nil)), nil +} diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index fe408e6d..3552ee4b 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -19,6 +19,7 @@ import ( "github.com/canonical/chisel/internal/setup" "github.com/canonical/chisel/internal/slicer" "github.com/canonical/chisel/internal/testutil" + "github.com/canonical/chisel/public/jsonwall" "github.com/canonical/chisel/public/manifest" ) @@ -614,7 +615,7 @@ var slicerTests = []slicerTest{{ `, }, }, { - summary: "Relative content root directory must not error", + summary: "Content root directory cannot be relative", slices: []setup.SliceKey{{"test-package", "myslice"}}, release: map[string]string{ "slices/mydir/test-package.yaml": ` @@ -634,6 +635,7 @@ var slicerTests = []slicerTest{{ opts.TargetDir, err = filepath.Rel(dir, opts.TargetDir) c.Assert(err, IsNil) }, + error: `internal error: cannot use a relative target directory \.\./.*`, }, { summary: "Can list parent directories of normal paths", slices: []setup.SliceKey{{"test-package", "myslice"}}, @@ -2165,6 +2167,149 @@ func runSlicerTests(s *S, c *C, tests []slicerTest) { } } +type selectValidManifestTest struct { + summary string + setup func(c *C, targetDir string, release *setup.Release) + releaseManifests []string + noMatch bool + error string +} + +var selectValidManifestTests = []selectValidManifestTest{{ + summary: "No manifest paths in release", + noMatch: true, + error: "cannot find valid manifest file", +}, { + summary: "Manifest path missing in target", + releaseManifests: []string{"/chisel/**"}, + noMatch: true, + error: "cannot find valid manifest file", +}, { + summary: "Unknown schema error ignored when other valid found", + releaseManifests: []string{"/chisel-a/**", "/chisel-b/**"}, + setup: func(c *C, targetDir string, release *setup.Release) { + manifestPathA := manifestPathForDir("/chisel-a/**") + manifestPathB := manifestPathForDir("/chisel-b/**") + slice := release.Packages["test-package"].Slices["manifest"] + writeSampleManifest(c, targetDir, manifestPathA, slice, "hash1") + writeInvalidSchemaManifest(c, targetDir, manifestPathB) + }, +}, { + summary: "Unknown schema error raised when no other valid found", + releaseManifests: []string{"/chisel/**"}, + setup: func(c *C, targetDir string, release *setup.Release) { + manifestPath := manifestPathForDir("/chisel/**") + writeInvalidSchemaManifest(c, targetDir, manifestPath) + }, + error: `cannot select a manifest: schema version is unknown`, +}, { + summary: "Valid manifest selected", + releaseManifests: []string{"/chisel/**"}, + setup: func(c *C, targetDir string, release *setup.Release) { + manifestPath := manifestPathForDir("/chisel/**") + slice := release.Packages["test-package"].Slices["manifest"] + writeSampleManifest(c, targetDir, manifestPath, slice, "hash1") + }, +}, { + summary: "Two consistent manifests are accepted", + releaseManifests: []string{"/chisel-a/**", "/chisel-b/**"}, + setup: func(c *C, targetDir string, release *setup.Release) { + manifestPathA := manifestPathForDir("/chisel-a/**") + manifestPathB := manifestPathForDir("/chisel-b/**") + slice := release.Packages["test-package"].Slices["manifest"] + writeSampleManifest(c, targetDir, manifestPathA, slice, "hash1") + writeSampleManifest(c, targetDir, manifestPathB, slice, "hash1") + }, +}, { + summary: "Inconsistent manifests with same schema are rejected", + releaseManifests: []string{"/chisel-a/**", "/chisel-b/**"}, + setup: func(c *C, targetDir string, release *setup.Release) { + manifestPathA := manifestPathForDir("/chisel-a/**") + manifestPathB := manifestPathForDir("/chisel-b/**") + slice := release.Packages["test-package"].Slices["manifest"] + writeSampleManifest(c, targetDir, manifestPathA, slice, "hash1") + writeSampleManifest(c, targetDir, manifestPathB, slice, "hash2") + }, + error: `cannot select between conflicting manifests: "/chisel-a/manifest.wall" != "/chisel-b/manifest.wall"`, +}, { + summary: "Invalid manifest data returns error", + releaseManifests: []string{"/chisel/**"}, + setup: func(c *C, targetDir string, release *setup.Release) { + manifestPath := filepath.Join(targetDir, manifestPathForDir("/chisel/**")) + err := os.MkdirAll(filepath.Dir(manifestPath), 0o755) + c.Assert(err, IsNil) + err = os.WriteFile(manifestPath, []byte("not-a-zstd-manifest"), 0o644) + c.Assert(err, IsNil) + }, + error: "cannot read manifest: invalid input: .*", +}, { + summary: "Manifest validation error is returned", + releaseManifests: []string{"/chisel/**"}, + setup: func(c *C, targetDir string, release *setup.Release) { + manifestPath := manifestPathForDir("/chisel/**") + writeInvalidManifest(c, targetDir, manifestPath) + }, + error: `invalid manifest: path /file has no matching entry in contents`, +}} + +func (s *S) TestSelectValidManifest(c *C) { + for _, test := range selectValidManifestTests { + c.Logf("Summary: %s", test.summary) + packages := map[string]*setup.Package{} + if len(test.releaseManifests) > 0 { + contents := map[string]setup.PathInfo{} + for _, dir := range test.releaseManifests { + contents[dir] = setup.PathInfo{Kind: "generate", Generate: "manifest"} + } + packages = map[string]*setup.Package{ + "test-package": { + Name: "test-package", + Slices: map[string]*setup.Slice{ + "manifest": { + Package: "test-package", + Name: "manifest", + Contents: contents, + }, + }, + }, + } + } + release := &setup.Release{ + Packages: packages, + } + targetDir := c.MkDir() + if test.setup != nil { + test.setup(c, targetDir, release) + } + mfest, err := slicer.SelectValidManifest(targetDir, release) + if test.error != "" { + c.Assert(err, ErrorMatches, test.error) + continue + } + c.Assert(err, IsNil) + if test.noMatch { + c.Assert(mfest, IsNil) + continue + } + c.Assert(mfest, NotNil) + } +} + +func (s *S) TestSelectValidManifestRelativeTarget(c *C) { + targetDir := c.MkDir() + dir, err := os.Getwd() + c.Assert(err, IsNil) + targetDir, err = filepath.Rel(dir, targetDir) + c.Assert(err, IsNil) + _, err = slicer.SelectValidManifest(targetDir, nil) + c.Assert(err, ErrorMatches, `internal error: cannot use a relative target directory \.\./.*`) +} + +func manifestPathForDir(dir string) string { + base := strings.TrimSuffix(dir, "**") + return path.Join(base, manifestutil.DefaultFilename) +} + func treeDumpManifestPaths(mfest *manifest.Manifest) (map[string]string, error) { result := make(map[string]string) err := mfest.IteratePaths("", func(path *manifest.Path) error { @@ -2241,3 +2386,68 @@ func readManifest(c *C, targetDir, manifestPath string) *manifest.Manifest { return mfest } + +func writeSampleManifest(c *C, targetDir, manifestPath string, slice *setup.Slice, hash string) { + manifestFullPath := filepath.Join(targetDir, manifestPath) + err := os.MkdirAll(filepath.Dir(manifestFullPath), 0o755) + c.Assert(err, IsNil) + f, err := os.Create(manifestFullPath) + c.Assert(err, IsNil) + zw, err := zstd.NewWriter(f) + c.Assert(err, IsNil) + options := &manifestutil.WriteOptions{ + PackageInfo: []*archive.PackageInfo{{ + Name: slice.Package, + Version: "1.0", + Arch: "amd64", + SHA256: "pkg-hash", + }}, + Selection: []*setup.Slice{slice}, + Report: &manifestutil.Report{Root: "/", Entries: map[string]manifestutil.ReportEntry{ + "/file": { + Path: "/file", + Mode: 0o644, + SHA256: hash, + Size: 3, + Slices: map[*setup.Slice]bool{slice: true}, + }, + }}, + } + err = manifestutil.Write(options, zw) + c.Assert(err, IsNil) + c.Assert(zw.Close(), IsNil) + c.Assert(f.Close(), IsNil) + c.Assert(os.Chmod(manifestFullPath, 0o644), IsNil) +} + +func writeInvalidManifest(c *C, targetDir, manifestPath string) { + manifestFullPath := filepath.Join(targetDir, manifestPath) + err := os.MkdirAll(filepath.Dir(manifestFullPath), 0o755) + c.Assert(err, IsNil) + f, err := os.Create(manifestFullPath) + c.Assert(err, IsNil) + zw, err := zstd.NewWriter(f) + c.Assert(err, IsNil) + dbw := jsonwall.NewDBWriter(&jsonwall.DBWriterOptions{Schema: manifest.Schema}) + err = dbw.Add(&manifest.Path{Kind: "path", Path: "/file", Mode: "0644"}) + c.Assert(err, IsNil) + _, err = dbw.WriteTo(zw) + c.Assert(err, IsNil) + c.Assert(zw.Close(), IsNil) + c.Assert(f.Close(), IsNil) +} + +func writeInvalidSchemaManifest(c *C, targetDir, manifestPath string) { + manifestFullPath := filepath.Join(targetDir, manifestPath) + err := os.MkdirAll(filepath.Dir(manifestFullPath), 0o755) + c.Assert(err, IsNil) + f, err := os.Create(manifestFullPath) + c.Assert(err, IsNil) + zw, err := zstd.NewWriter(f) + c.Assert(err, IsNil) + dbw := jsonwall.NewDBWriter(&jsonwall.DBWriterOptions{Schema: "invalid"}) + _, err = dbw.WriteTo(zw) + c.Assert(err, IsNil) + c.Assert(zw.Close(), IsNil) + c.Assert(f.Close(), IsNil) +} diff --git a/public/manifest/manifest.go b/public/manifest/manifest.go index 1e4809b8..7e1c6829 100644 --- a/public/manifest/manifest.go +++ b/public/manifest/manifest.go @@ -46,12 +46,20 @@ type Manifest struct { db *jsonwall.DB } +type UnknownSchemaError struct { + Version string +} + +func (e *UnknownSchemaError) Error() string { + return fmt.Sprintf("unknown manifest schema version %q", e.Version) +} + // Read loads a Manifest without performing any validation. The data is assumed // to be both valid jsonwall and a valid Manifest (see Validate). func Read(reader io.Reader) (manifest *Manifest, err error) { defer func() { if err != nil { - err = fmt.Errorf("cannot read manifest: %s", err) + err = fmt.Errorf("cannot read manifest: %w", err) } }() @@ -61,7 +69,7 @@ func Read(reader io.Reader) (manifest *Manifest, err error) { } mfestSchema := db.Schema() if mfestSchema != Schema { - return nil, fmt.Errorf("unknown schema version %q", mfestSchema) + return nil, &UnknownSchemaError{Version: mfestSchema} } manifest = &Manifest{db: db} diff --git a/public/manifest/manifest_test.go b/public/manifest/manifest_test.go index d710e121..c9ecef11 100644 --- a/public/manifest/manifest_test.go +++ b/public/manifest/manifest_test.go @@ -76,7 +76,7 @@ var readManifestTests = []struct { {"jsonwall":"1.0","schema":"2.0","count":1} {"kind":"package","name":"pkg1","version":"v1","sha256":"hash1","arch":"arch1"} `, - error: `cannot read manifest: unknown schema version "2.0"`, + error: `cannot read manifest: unknown manifest schema version "2.0"`, }} func (s *S) TestManifestRead(c *C) {