From ccc997ede5775c3f255f1f87d951a8159696f7c8 Mon Sep 17 00:00:00 2001 From: Elizabeth Myers Date: Tue, 19 Sep 2023 15:21:10 -0700 Subject: [PATCH 1/3] Add post-file walk linting and empty package linting Signed-off-by: Elizabeth Myers --- pkg/build/linter.go | 59 +++++++++++++++++++++++++++++++++++++++- pkg/build/linter_test.go | 58 +++++++++++++++++++++++++++------------ pkg/config/config.go | 3 +- 3 files changed, 101 insertions(+), 19 deletions(-) diff --git a/pkg/build/linter.go b/pkg/build/linter.go index 273da6167..d2b826b41 100644 --- a/pkg/build/linter.go +++ b/pkg/build/linter.go @@ -35,6 +35,13 @@ type linter struct { Explain string } +type postLinterFunc func(lctx LinterContext, fsys fs.FS) error + +type postLinter struct { + LinterFunc postLinterFunc + Explain string +} + var linterMap = map[string]linter{ "dev": linter{devLinter, "If this package is creating /dev nodes, it should use udev instead; otherwise, remove any files in /dev"}, "opt": linter{optLinter, "This package should be a -compat package"}, @@ -46,6 +53,10 @@ var linterMap = map[string]linter{ "worldwrite": linter{worldWriteableLinter, "Change the permissions of any world-writeable files in the package, disable the linter, or make this a -compat package"}, } +var postLinterMap = map[string]postLinter{ + "empty": postLinter{emptyPostLinter, "Verify that this package is supposed to be empty, if so set the no-provides package option; otherwise, check the build"}, +} + var isDevRegex = regexp.MustCompile("^dev/") var isOptRegex = regexp.MustCompile("^opt/") var isSrvRegex = regexp.MustCompile("^srv/") @@ -141,12 +152,42 @@ func worldWriteableLinter(_ LinterContext, path string, d fs.DirEntry) error { return nil } +func emptyPostLinter(lctx LinterContext, fsys fs.FS) error { + foundfile := false + walkCb := func(path string, _ fs.DirEntry, err error) error { + if err != nil { + return err + } + + if path == "." { + // Skip root + return nil + } + + foundfile = true + return fs.SkipAll + } + + err := fs.WalkDir(fsys, ".", walkCb) + if err != nil { + return err + } + + if foundfile || lctx.cfg.Package.Options.NoProvides { + // Nothing to do + return nil + } + + return fmt.Errorf("Package is empty but no-provides is not set") +} + func lintPackageFs(lctx LinterContext, fsys fs.FS, linters []string) error { // If this is a compat package, do nothing. if isCompatPackage.MatchString(lctx.pkgname) { return nil } + postLinters := []string{} walkCb := func(path string, d fs.DirEntry, err error) error { if err != nil { return fmt.Errorf("Error traversing tree at %s: %w", path, err) @@ -155,7 +196,14 @@ func lintPackageFs(lctx LinterContext, fsys fs.FS, linters []string) error { for _, linterName := range linters { linter, present := linterMap[linterName] if !present { - return fmt.Errorf("Linter %s is unknown", linterName) + // Check if it's a post linter instead + _, present = postLinterMap[linterName] + if !present { + return fmt.Errorf("Linter %s is unknown", linterName) + } + + postLinters = append(postLinters, linterName) + continue } err = linter.LinterFunc(lctx, path, d) @@ -172,5 +220,14 @@ func lintPackageFs(lctx LinterContext, fsys fs.FS, linters []string) error { return err } + // Run post-walking linters + for _, linterName := range postLinters { + linter := postLinterMap[linterName] + err = linter.LinterFunc(lctx, fsys) + if err != nil { + return fmt.Errorf("Linter %s failed; suggest: %s", linterName, linter.Explain) + } + } + return nil } diff --git a/pkg/build/linter_test.go b/pkg/build/linter_test.go index 3f18b6926..e73ea1ab0 100644 --- a/pkg/build/linter_test.go +++ b/pkg/build/linter_test.go @@ -25,6 +25,30 @@ import ( "chainguard.dev/melange/pkg/config" ) +func Test_emptyLinter(t *testing.T) { + dir, err := os.MkdirTemp("", "melange.XXXXX") + defer os.RemoveAll(dir) + assert.NoError(t, err) + + cfg := config.Configuration{ + Package: config.Package{ + Name: "testempty", + Version: "4.2.0", + Epoch: 0, + Checks: config.Checks{ + Enabled: []string{"empty"}, + Disabled: []string{"dev", "opt", "setuidgid", "srv", "tempdir", "usrlocal", "varempty", "worldwrite"}, + }, + }, + } + + linters := cfg.Package.Checks.GetLinters() + assert.Equal(t, linters, []string{"empty"}) + fsys := os.DirFS(dir) + lctx := LinterContext{cfg.Package.Name, &cfg, &cfg.Package.Checks} + assert.Error(t, lintPackageFs(lctx, fsys, linters)) +} + func Test_usrLocalLinter(t *testing.T) { dir, err := os.MkdirTemp("", "melange.XXXXX") defer os.RemoveAll(dir) @@ -32,12 +56,12 @@ func Test_usrLocalLinter(t *testing.T) { cfg := config.Configuration{ Package: config.Package{ - Name: "test", + Name: "testusrlocal", Version: "4.2.0", Epoch: 0, Checks: config.Checks{ Enabled: []string{"usrlocal"}, - Disabled: []string{"dev", "opt", "setuidgid", "srv", "tempdir", "varempty", "worldwrite"}, + Disabled: []string{"dev", "empty", "opt", "setuidgid", "srv", "tempdir", "varempty", "worldwrite"}, }, }, } @@ -61,12 +85,12 @@ func Test_varEmptyLinter(t *testing.T) { cfg := config.Configuration{ Package: config.Package{ - Name: "test", + Name: "testvarempty", Version: "4.2.0", Epoch: 0, Checks: config.Checks{ Enabled: []string{"varempty"}, - Disabled: []string{"dev", "opt", "setuidgid", "srv", "tempdir", "usrlocal", "worldwrite"}, + Disabled: []string{"dev", "empty", "opt", "setuidgid", "srv", "tempdir", "usrlocal", "worldwrite"}, }, }, } @@ -91,12 +115,12 @@ func Test_devLinter(t *testing.T) { cfg := config.Configuration{ Package: config.Package{ - Name: "test", + Name: "testdev", Version: "4.2.0", Epoch: 0, Checks: config.Checks{ Enabled: []string{"dev"}, - Disabled: []string{"opt", "setuidgid", "srv", "tempdir", "usrlocal", "varempty", "worldwrite"}, + Disabled: []string{"empty", "opt", "setuidgid", "srv", "tempdir", "usrlocal", "varempty", "worldwrite"}, }, }, } @@ -121,12 +145,12 @@ func Test_optLinter(t *testing.T) { cfg := config.Configuration{ Package: config.Package{ - Name: "test", + Name: "testopt", Version: "4.2.0", Epoch: 0, Checks: config.Checks{ Enabled: []string{"opt"}, - Disabled: []string{"dev", "setuidgid", "srv", "tempdir", "usrlocal", "varempty", "worldwrite"}, + Disabled: []string{"dev", "empty", "setuidgid", "srv", "tempdir", "usrlocal", "varempty", "worldwrite"}, }, }, } @@ -151,12 +175,12 @@ func Test_srvLinter(t *testing.T) { cfg := config.Configuration{ Package: config.Package{ - Name: "test", + Name: "testsrv", Version: "4.2.0", Epoch: 0, Checks: config.Checks{ Enabled: []string{"srv"}, - Disabled: []string{"dev", "opt", "setuidgid", "tempdir", "usrlocal", "varempty", "worldwrite"}, + Disabled: []string{"dev", "empty", "opt", "setuidgid", "tempdir", "usrlocal", "varempty", "worldwrite"}, }, }, } @@ -181,12 +205,12 @@ func Test_tempDirLinter(t *testing.T) { cfg := config.Configuration{ Package: config.Package{ - Name: "test", + Name: "testtempdir", Version: "4.2.0", Epoch: 0, Checks: config.Checks{ Enabled: []string{"tempdir"}, - Disabled: []string{"dev", "opt", "setuidgid", "srv", "usrlocal", "varempty", "worldwrite"}, + Disabled: []string{"dev", "empty", "opt", "setuidgid", "srv", "usrlocal", "varempty", "worldwrite"}, }, }, } @@ -242,12 +266,12 @@ func Test_setUidGidLinter(t *testing.T) { cfg := config.Configuration{ Package: config.Package{ - Name: "test", + Name: "testsetuidgid", Version: "4.2.0", Epoch: 0, Checks: config.Checks{ Enabled: []string{"setuidgid"}, - Disabled: []string{"dev", "opt", "srv", "tempdir", "usrlocal", "varempty", "worldwrite"}, + Disabled: []string{"dev", "empty", "opt", "srv", "tempdir", "usrlocal", "varempty", "worldwrite"}, }, }, } @@ -278,12 +302,12 @@ func Test_worldWriteLinter(t *testing.T) { cfg := config.Configuration{ Package: config.Package{ - Name: "test", + Name: "testworldwrite", Version: "4.2.0", Epoch: 0, Checks: config.Checks{ Enabled: []string{"worldwrite"}, - Disabled: []string{"dev", "opt", "srv", "setuidgid", "tempdir", "usrlocal", "varempty"}, + Disabled: []string{"dev", "empty", "opt", "srv", "setuidgid", "tempdir", "usrlocal", "varempty"}, }, }, } @@ -333,7 +357,7 @@ func Test_disableDefaultLinter(t *testing.T) { cfg := config.Configuration{ Package: config.Package{ - Name: "test", + Name: "testdisable", Version: "4.2.0", Epoch: 0, Checks: config.Checks{ diff --git a/pkg/config/config.go b/pkg/config/config.go index 8ae253be5..d6c729c50 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -69,7 +69,7 @@ type Scriptlets struct { type PackageOption struct { // Optional: Signify this package as a virtual package which does not provide - // any files, executables, librariries, etc... and is otherwise empty + // any files, executables, libraries, etc... and is otherwise empty NoProvides bool `yaml:"no-provides"` // Optional: Mark this package as a self contained package that does not // depend on any other package @@ -339,6 +339,7 @@ func (cfg Configuration) Name() string { var defaultLinters = []string{ "dev", + "empty", "opt", "srv", "setuidgid", From 49688bf3866dd02942f303d43a3fe9f14e0423fd Mon Sep 17 00:00:00 2001 From: Elizabeth Myers Date: Wed, 20 Sep 2023 11:16:44 -0700 Subject: [PATCH 2/3] Respect subpackage no-provides Signed-off-by: Elizabeth Myers --- pkg/build/linter.go | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/pkg/build/linter.go b/pkg/build/linter.go index d2b826b41..2ceca5558 100644 --- a/pkg/build/linter.go +++ b/pkg/build/linter.go @@ -28,6 +28,21 @@ type LinterContext struct { chk *config.Checks } +func (lctx LinterContext) getPackageOptions() (*config.PackageOption, error) { + if lctx.pkgname == lctx.cfg.Package.Name { + return &lctx.cfg.Package.Options, nil + } + + for _, spkg := range lctx.cfg.Subpackages { + if lctx.pkgname == spkg.Name { + return &spkg.Options, nil + } + } + + // Shouldn't get here + return nil, fmt.Errorf("Could not locate package or subpackage!") +} + type linterFunc func(lctx LinterContext, path string, d fs.DirEntry) error type linter struct { @@ -173,7 +188,17 @@ func emptyPostLinter(lctx LinterContext, fsys fs.FS) error { return err } - if foundfile || lctx.cfg.Package.Options.NoProvides { + // Nothing to do + if foundfile { + return nil + } + + options, err := lctx.getPackageOptions() + if err != nil { + return err + } + + if options.NoProvides { // Nothing to do return nil } From cf1ccd417fdf10e6a1f376439f4d75d7a10acb30 Mon Sep 17 00:00:00 2001 From: Elizabeth Myers Date: Wed, 20 Sep 2023 13:16:27 -0700 Subject: [PATCH 3/3] Remove no provides check per @kaniini Signed-off-by: Elizabeth Myers --- pkg/build/linter.go | 29 ++--------------------------- 1 file changed, 2 insertions(+), 27 deletions(-) diff --git a/pkg/build/linter.go b/pkg/build/linter.go index 2ceca5558..21c467884 100644 --- a/pkg/build/linter.go +++ b/pkg/build/linter.go @@ -28,21 +28,6 @@ type LinterContext struct { chk *config.Checks } -func (lctx LinterContext) getPackageOptions() (*config.PackageOption, error) { - if lctx.pkgname == lctx.cfg.Package.Name { - return &lctx.cfg.Package.Options, nil - } - - for _, spkg := range lctx.cfg.Subpackages { - if lctx.pkgname == spkg.Name { - return &spkg.Options, nil - } - } - - // Shouldn't get here - return nil, fmt.Errorf("Could not locate package or subpackage!") -} - type linterFunc func(lctx LinterContext, path string, d fs.DirEntry) error type linter struct { @@ -69,7 +54,7 @@ var linterMap = map[string]linter{ } var postLinterMap = map[string]postLinter{ - "empty": postLinter{emptyPostLinter, "Verify that this package is supposed to be empty, if so set the no-provides package option; otherwise, check the build"}, + "empty": postLinter{emptyPostLinter, "Verify that this package is supposed to be empty; if it is, disable this linter; otherwise check the build"}, } var isDevRegex = regexp.MustCompile("^dev/") @@ -167,7 +152,7 @@ func worldWriteableLinter(_ LinterContext, path string, d fs.DirEntry) error { return nil } -func emptyPostLinter(lctx LinterContext, fsys fs.FS) error { +func emptyPostLinter(_ LinterContext, fsys fs.FS) error { foundfile := false walkCb := func(path string, _ fs.DirEntry, err error) error { if err != nil { @@ -193,16 +178,6 @@ func emptyPostLinter(lctx LinterContext, fsys fs.FS) error { return nil } - options, err := lctx.getPackageOptions() - if err != nil { - return err - } - - if options.NoProvides { - // Nothing to do - return nil - } - return fmt.Errorf("Package is empty but no-provides is not set") }