From 8febbd7a0fdefc659f2a166d08a3ba378de6efbc Mon Sep 17 00:00:00 2001 From: marcin Date: Thu, 23 Apr 2026 16:43:23 +0100 Subject: [PATCH 1/3] feat: private slices --- cmd/chisel/cmd_cut.go | 3 + cmd/chisel/cmd_cut_test.go | 13 +++ cmd/chisel/cmd_find.go | 5 +- cmd/chisel/cmd_find_test.go | 23 ++++- cmd/chisel/cmd_info_test.go | 40 +++++++++ internal/apacheutil/util.go | 11 ++- internal/apacheutil/util_test.go | 18 ++++ internal/setup/setup.go | 26 ++++++ internal/setup/setup_test.go | 143 ++++++++++++++++++++++++++++++- internal/setup/yaml.go | 5 +- 10 files changed, 281 insertions(+), 6 deletions(-) create mode 100644 cmd/chisel/cmd_cut_test.go diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 35c81a79..782543ff 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -55,6 +55,9 @@ func (cmd *cmdCut) Execute(args []string) error { if err != nil { return err } + if sliceKey.IsPrivate() { + return fmt.Errorf("cannot cut private slice %s", sliceRef) + } sliceKeys[i] = sliceKey } diff --git a/cmd/chisel/cmd_cut_test.go b/cmd/chisel/cmd_cut_test.go new file mode 100644 index 00000000..55304dd2 --- /dev/null +++ b/cmd/chisel/cmd_cut_test.go @@ -0,0 +1,13 @@ +package main_test + +import ( + . "gopkg.in/check.v1" + + chisel "github.com/canonical/chisel/cmd/chisel" +) + +func (s *ChiselSuite) TestCutRejectsPrivateSlice(c *C) { + dir := c.MkDir() + _, err := chisel.Parser().ParseArgs([]string{"cut", "--root", dir, "mypkg__priv"}) + c.Assert(err, ErrorMatches, `cannot cut private slice mypkg__priv`) +} diff --git a/cmd/chisel/cmd_find.go b/cmd/chisel/cmd_find.go index 1a208d4b..42e687e1 100644 --- a/cmd/chisel/cmd_find.go +++ b/cmd/chisel/cmd_find.go @@ -87,7 +87,7 @@ func match(slice *setup.Slice, query string) bool { } // findSlices returns slices from the provided release that match all of the -// query strings (AND). +// query strings (AND). Private slices are never returned. func findSlices(release *setup.Release, query []string) (slices []*setup.Slice, err error) { slices = []*setup.Slice{} for _, pkg := range release.Packages { @@ -95,6 +95,9 @@ func findSlices(release *setup.Release, query []string) (slices []*setup.Slice, if slice == nil { continue } + if (setup.SliceKey{Package: slice.Package, Slice: slice.Name}).IsPrivate() { + continue + } allMatch := true for _, term := range query { if !match(slice, term) { diff --git a/cmd/chisel/cmd_find_test.go b/cmd/chisel/cmd_find_test.go index a2a68c2a..e9eec2b1 100644 --- a/cmd/chisel/cmd_find_test.go +++ b/cmd/chisel/cmd_find_test.go @@ -43,7 +43,7 @@ var sampleRelease = &setup.Release{ }, Packages: map[string]*setup.Package{ "openjdk-8-jdk": makeSamplePackage("openjdk-8-jdk", []string{"bins", "config", "core", "libs", "utils"}), - "python3.10": makeSamplePackage("python3.10", []string{"bins", "config", "core", "libs", "utils"}), + "python3.10": makeSamplePackage("python3.10", []string{"bins", "config", "core", "libs", "utils", "_priv"}), }, } @@ -123,6 +123,27 @@ var findTests = []findTest{{ release: sampleRelease, query: []string{"python", "slice"}, result: []*setup.Slice{}, +}, { + summary: "Private slices are hidden when matching by package", + release: sampleRelease, + query: []string{"python3.10"}, + result: []*setup.Slice{ + sampleRelease.Packages["python3.10"].Slices["bins"], + sampleRelease.Packages["python3.10"].Slices["config"], + sampleRelease.Packages["python3.10"].Slices["core"], + sampleRelease.Packages["python3.10"].Slices["libs"], + sampleRelease.Packages["python3.10"].Slices["utils"], + }, +}, { + summary: "Private slices are hidden when matching by full slice name", + release: sampleRelease, + query: []string{"python3.10__priv"}, + result: []*setup.Slice{}, +}, { + summary: "Private slices are hidden when matching by slice-only query", + release: sampleRelease, + query: []string{"__priv"}, + result: []*setup.Slice{}, }} func (s *ChiselSuite) TestFindSlices(c *C) { diff --git a/cmd/chisel/cmd_info_test.go b/cmd/chisel/cmd_info_test.go index fc462487..898b4b63 100644 --- a/cmd/chisel/cmd_info_test.go +++ b/cmd/chisel/cmd_info_test.go @@ -217,3 +217,43 @@ func (s *ChiselSuite) TestInfoCommand(c *C) { c.Assert(s.Stdout(), Equals, strings.TrimSpace(test.stdout)+"\n") } } + +func (s *ChiselSuite) TestInfoPrivateSlice(c *C) { + s.ResetStdStreams() + + input := map[string]string{ + "chisel.yaml": strings.ReplaceAll(testutil.DefaultChiselYaml, "format: v1", "format: v3"), + "slices/mypkg.yaml": ` + package: mypkg + slices: + myslice: + essential: + mypkg__priv: {} + _priv: + contents: + /usr/bin/cc: + `, + } + + dir := c.MkDir() + for path, data := range input { + fpath := filepath.Join(dir, path) + err := os.MkdirAll(filepath.Dir(fpath), 0755) + c.Assert(err, IsNil) + err = os.WriteFile(fpath, testutil.Reindent(data), 0644) + c.Assert(err, IsNil) + } + + _, err := chisel.Parser().ParseArgs([]string{"info", "--release", dir, "mypkg__priv"}) + c.Assert(err, IsNil) + + expected := ` + package: mypkg + slices: + _priv: + contents: + /usr/bin/cc: {} + ` + expected = string(testutil.Reindent(expected)) + c.Assert(s.Stdout(), Equals, strings.TrimSpace(expected)+"\n") +} diff --git a/internal/apacheutil/util.go b/internal/apacheutil/util.go index 7e0f8e6d..7084cb14 100644 --- a/internal/apacheutil/util.go +++ b/internal/apacheutil/util.go @@ -5,6 +5,7 @@ package apacheutil import ( "fmt" "regexp" + "strings" ) type SliceKey struct { @@ -14,14 +15,20 @@ type SliceKey struct { func (s SliceKey) String() string { return s.Package + "_" + s.Slice } +// IsPrivate reports whether the slice name marks the slice as private. +// Private slices begin with a single underscore and may only be used as +// essentials of other slices; they cannot be selected directly. +func (s SliceKey) IsPrivate() bool { return strings.HasPrefix(s.Slice, "_") } + // FnameExp matches the slice definition file basename. var FnameExp = regexp.MustCompile(`^([a-z0-9](?:-?[.a-z0-9+]){1,})\.yaml$`) // SnameExp matches only the slice name, without the leading package name. -var SnameExp = regexp.MustCompile(`^([a-z](?:-?[a-z0-9]){2,})$`) +// An optional single leading underscore marks the slice as private. +var SnameExp = regexp.MustCompile(`^(_?[a-z](?:-?[a-z0-9]){2,})$`) // knameExp matches the slice full name in pkg_slice format. -var knameExp = regexp.MustCompile(`^([a-z0-9](?:-?[.a-z0-9+]){1,})_([a-z](?:-?[a-z0-9]){2,})$`) +var knameExp = regexp.MustCompile(`^([a-z0-9](?:-?[.a-z0-9+]){1,})_(_?[a-z](?:-?[a-z0-9]){2,})$`) func ParseSliceKey(sliceKey string) (SliceKey, error) { match := knameExp.FindStringSubmatch(sliceKey) diff --git a/internal/apacheutil/util_test.go b/internal/apacheutil/util_test.go index 6289473c..627344ab 100644 --- a/internal/apacheutil/util_test.go +++ b/internal/apacheutil/util_test.go @@ -42,6 +42,18 @@ var sliceKeyTests = []struct { }, { input: "a._bar", expected: apacheutil.SliceKey{Package: "a.", Slice: "bar"}, +}, { + input: "foo__bar", + expected: apacheutil.SliceKey{Package: "foo", Slice: "_bar"}, +}, { + input: "foo__abc", + expected: apacheutil.SliceKey{Package: "foo", Slice: "_abc"}, +}, { + input: "foo___bar", + err: `invalid slice reference: "foo___bar"`, +}, { + input: "foo__ab", + err: `invalid slice reference: "foo__ab"`, }, { input: "foo_ba", err: `invalid slice reference: "foo_ba"`, @@ -94,3 +106,9 @@ func (s *S) TestParseSliceKey(c *C) { c.Assert(key, DeepEquals, test.expected) } } + +func (s *S) TestIsPrivate(c *C) { + c.Assert(apacheutil.SliceKey{Package: "foo", Slice: "_bar"}.IsPrivate(), Equals, true) + c.Assert(apacheutil.SliceKey{Package: "foo", Slice: "bar"}.IsPrivate(), Equals, false) + c.Assert(apacheutil.SliceKey{Package: "foo", Slice: ""}.IsPrivate(), Equals, false) +} diff --git a/internal/setup/setup.go b/internal/setup/setup.go index 6ed24abe..28e85273 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -304,6 +304,26 @@ func (r *Release) validate() error { } } + // Check that every private slice is referenced as an essential by some + // other slice. Private slices cannot be selected directly, so an + // unreferenced one is dead code. + referenced := make(map[SliceKey]bool) + for _, pkg := range r.Packages { + for _, slice := range pkg.Slices { + for ref := range slice.Essential { + referenced[ref] = true + } + } + } + for _, pkg := range r.Packages { + for _, slice := range pkg.Slices { + key := SliceKey{Package: pkg.Name, Slice: slice.Name} + if key.IsPrivate() && !referenced[key] { + return fmt.Errorf("private slice %s is not referenced by any other slice", key) + } + } + } + // Check for cycles. // Note: For release validation an essential with a specific arch is the // same as an essential with all archs, i.e. Chisel does not use arch to @@ -468,6 +488,12 @@ func stripBase(baseDir, path string) string { func Select(release *Release, slices []SliceKey, arch string) (*Selection, error) { logf("Selecting slices...") + for _, key := range slices { + if key.IsPrivate() { + return nil, fmt.Errorf("cannot select private slice %s", key) + } + } + var err error if arch == "" { arch, err = deb.InferArch() diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index d669943f..bc204d4b 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -1391,7 +1391,7 @@ var setupTests = []setupTest{{ /usr/bin/cc: `, }, - relerror: `invalid slice name "cc" in slices/mydir/mypkg.yaml \(must start with a-z, len >= 3, only a-z / 0-9 / -\)`, + relerror: `invalid slice name "cc" in slices/mydir/mypkg.yaml \(must start with a-z or _, len >= 3 after the optional _, only a-z / 0-9 / -\)`, }, { summary: "Invalid slice hint - too long", input: map[string]string{ @@ -3896,6 +3896,103 @@ var setupTests = []setupTest{{ `, }, relerror: `package "mypkg" slices defined more than once: slices/dir1/mypkg.yaml and slices/dir2/mypkg.yaml`, +}, { + summary: "Private slice referenced as essential by a public slice (same package)", + input: map[string]string{ + "chisel.yaml": strings.ReplaceAll(testutil.DefaultChiselYaml, "format: v1", "format: v3"), + "slices/mydir/mypkg.yaml": ` + package: mypkg + slices: + myslice: + essential: + mypkg__priv: {} + _priv: + contents: + /usr/bin/cc: + `, + }, + selslices: []setup.SliceKey{{"mypkg", "myslice"}}, + selection: &setup.Selection{ + Slices: []*setup.Slice{{ + Package: "mypkg", + Name: "_priv", + Contents: map[string]setup.PathInfo{ + "/usr/bin/cc": {Kind: setup.CopyPath}, + }, + }, { + Package: "mypkg", + Name: "myslice", + Essential: map[setup.SliceKey]setup.EssentialInfo{ + {"mypkg", "_priv"}: {}, + }, + }}, + }, +}, { + summary: "Private slice referenced as essential by a public slice (cross-package)", + input: map[string]string{ + "chisel.yaml": strings.ReplaceAll(testutil.DefaultChiselYaml, "format: v1", "format: v3"), + "slices/mydir/mypkg1.yaml": ` + package: mypkg1 + slices: + myslice: + essential: + mypkg2__priv: {} + `, + "slices/mydir/mypkg2.yaml": ` + package: mypkg2 + slices: + _priv: + contents: + /usr/bin/cc: + `, + }, + selslices: []setup.SliceKey{{"mypkg1", "myslice"}}, + selection: &setup.Selection{ + Slices: []*setup.Slice{{ + Package: "mypkg2", + Name: "_priv", + Contents: map[string]setup.PathInfo{ + "/usr/bin/cc": {Kind: setup.CopyPath}, + }, + }, { + Package: "mypkg1", + Name: "myslice", + Essential: map[setup.SliceKey]setup.EssentialInfo{ + {"mypkg2", "_priv"}: {}, + }, + }}, + }, +}, { + summary: "Unreferenced private slice is rejected", + input: map[string]string{ + "chisel.yaml": strings.ReplaceAll(testutil.DefaultChiselYaml, "format: v1", "format: v3"), + "slices/mydir/mypkg.yaml": ` + package: mypkg + slices: + myslice: + _priv: + contents: + /usr/bin/cc: + `, + }, + relerror: `private slice mypkg__priv is not referenced by any other slice`, +}, { + summary: "Private slice cannot be selected directly", + input: map[string]string{ + "chisel.yaml": strings.ReplaceAll(testutil.DefaultChiselYaml, "format: v1", "format: v3"), + "slices/mydir/mypkg.yaml": ` + package: mypkg + slices: + myslice: + essential: + mypkg__priv: {} + _priv: + contents: + /usr/bin/cc: + `, + }, + selslices: []setup.SliceKey{{"mypkg", "_priv"}}, + selerror: `cannot select private slice mypkg__priv`, }} func (s *S) TestParseRelease(c *C) { @@ -4344,6 +4441,50 @@ func (s *S) TestSelectEmptyArch(c *C) { c.Assert(sliceNames, DeepEquals, expected) } +var privateSliceFormatGatingTests = []struct { + summary string + format string +}{{ + summary: "v1", + format: "v1", +}, { + summary: "v2", + format: "v2", +}} + +func (s *S) TestPrivateSliceFormatGating(c *C) { + mypkgYAML := ` + package: mypkg + slices: + myslice: + essential: + - mypkg__priv + _priv: + contents: + /usr/bin/cc: + ` + for _, test := range privateSliceFormatGatingTests { + c.Logf("Format: %s", test.summary) + + input := map[string]string{ + "chisel.yaml": strings.ReplaceAll(string(testutil.DefaultChiselYaml), "format: v1", "format: "+test.format), + "slices/mydir/mypkg.yaml": mypkgYAML, + } + + dir := c.MkDir() + for path, data := range input { + fpath := filepath.Join(dir, path) + err := os.MkdirAll(filepath.Dir(fpath), 0o755) + c.Assert(err, IsNil) + err = os.WriteFile(fpath, testutil.Reindent(data), 0o644) + c.Assert(err, IsNil) + } + + _, err := setup.ReadRelease(dir) + c.Assert(err, ErrorMatches, `private slice mypkg__priv requires format v3`) + } +} + // oldEssentialToV3 converts the essentials in v1 and v2, both 'essential', and // 'v3-essential' to the shape expected by the v3 format. // skip is set to true when an accurate translation of the test is not diff --git a/internal/setup/yaml.go b/internal/setup/yaml.go index cefe86cb..5a1a0355 100644 --- a/internal/setup/yaml.go +++ b/internal/setup/yaml.go @@ -485,7 +485,10 @@ func parsePackage(format, pkgName, pkgPath string, data []byte) (*Package, error for sliceName, yamlSlice := range yamlPkg.Slices { match := apacheutil.SnameExp.FindStringSubmatch(sliceName) if match == nil { - return nil, fmt.Errorf("invalid slice name %q in %s (must start with a-z, len >= 3, only a-z / 0-9 / -)", sliceName, pkgPath) + return nil, fmt.Errorf("invalid slice name %q in %s (must start with a-z or _, len >= 3 after the optional _, only a-z / 0-9 / -)", sliceName, pkgPath) + } + if strings.HasPrefix(sliceName, "_") && format != "v3" { + return nil, fmt.Errorf("private slice %s requires format v3", SliceKey{pkgName, sliceName}) } hintNotPrintable := strings.ContainsFunc(yamlSlice.Hint, func(r rune) bool { return !unicode.IsPrint(r) From f955d5935369ffa3da08a2437c36b73beba19934 Mon Sep 17 00:00:00 2001 From: marcin Date: Thu, 23 Apr 2026 17:17:44 +0100 Subject: [PATCH 2/3] chore: retrigger From 5b3be9c0fee66194b1a59baea3ffb86e19ba1011 Mon Sep 17 00:00:00 2001 From: marcin Date: Thu, 23 Apr 2026 17:33:34 +0100 Subject: [PATCH 3/3] feat: chained private test --- internal/setup/setup_test.go | 64 ++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index bc204d4b..3b551b03 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -3993,6 +3993,70 @@ var setupTests = []setupTest{{ }, selslices: []setup.SliceKey{{"mypkg", "_priv"}}, selerror: `cannot select private slice mypkg__priv`, +}, { + summary: "Chained private slices: pub -> _priv1 -> _priv2", + input: map[string]string{ + "chisel.yaml": strings.ReplaceAll(testutil.DefaultChiselYaml, "format: v1", "format: v3"), + "slices/mydir/mypkg.yaml": ` + package: mypkg + slices: + pub: + essential: + mypkg__priv1: {} + _priv1: + essential: + mypkg__priv2: {} + contents: + /usr/bin/one: + _priv2: + contents: + /usr/bin/two: + `, + }, + selslices: []setup.SliceKey{{"mypkg", "pub"}}, + selection: &setup.Selection{ + Slices: []*setup.Slice{{ + Package: "mypkg", + Name: "_priv2", + Contents: map[string]setup.PathInfo{ + "/usr/bin/two": {Kind: setup.CopyPath}, + }, + }, { + Package: "mypkg", + Name: "_priv1", + Essential: map[setup.SliceKey]setup.EssentialInfo{ + {"mypkg", "_priv2"}: {}, + }, + Contents: map[string]setup.PathInfo{ + "/usr/bin/one": {Kind: setup.CopyPath}, + }, + }, { + Package: "mypkg", + Name: "pub", + Essential: map[setup.SliceKey]setup.EssentialInfo{ + {"mypkg", "_priv1"}: {}, + }, + }}, + }, +}, { + summary: "Chained private slice with cycle detected", + input: map[string]string{ + "chisel.yaml": strings.ReplaceAll(testutil.DefaultChiselYaml, "format: v1", "format: v3"), + "slices/mydir/mypkg.yaml": ` + package: mypkg + slices: + pub: + essential: + mypkg__priv1: {} + _priv1: + essential: + mypkg__priv2: {} + _priv2: + essential: + mypkg__priv1: {} + `, + }, + relerror: `essential loop detected: mypkg__priv1, mypkg__priv2`, }} func (s *S) TestParseRelease(c *C) {