Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 34 additions & 1 deletion cmd/chisel/cmd_cut.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
package main

import (
"errors"
"fmt"
"os"
"path/filepath"
"slices"
"time"

Expand All @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
47 changes: 43 additions & 4 deletions internal/manifestutil/manifestutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"io"
"io/fs"
"maps"
"path/filepath"
"slices"
"sort"
Expand All @@ -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) {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Note to reviewer]: This addresses the previous review comment:

This is too loose. Imagine what happens if path is "/hmm/oops**", for instance, or just "/oops".

We need to be strict about how a manifest path looks like. Hopefully this is already the case elsewhere so there's already a pattern to follow. If not, please let me know.

From #264 (comment)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a validateManifestPath function verifying that the received path matches our definition of a valid manifest path before doing any manipulation on it. This function is now used in FindPaths and FindPathsInRelease. I also added a couple of tests to make sure the issue you pointed out was handled properly.

From #264 (comment)

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
Expand Down
153 changes: 153 additions & 0 deletions internal/manifestutil/manifestutil_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading
Loading