Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add post-file walk linting and empty package linting #700

Merged
merged 3 commits into from
Sep 20, 2023
Merged

Add post-file walk linting and empty package linting #700

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ccc997e
Add post-file walk linting and empty package linting
Elizafox Sep 19, 2023
49688bf
Respect subpackage no-provides
Elizafox Sep 20, 2023
cf1ccd4
Remove no provides check per @kaniini
Elizafox Sep 20, 2023
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
59 changes: 58 additions & 1 deletion pkg/build/linter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand All @@ -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 it is, disable this linter; otherwise check the build"},
}

var isDevRegex = regexp.MustCompile("^dev/")
var isOptRegex = regexp.MustCompile("^opt/")
var isSrvRegex = regexp.MustCompile("^srv/")
Expand Down Expand Up @@ -141,12 +152,42 @@ func worldWriteableLinter(_ LinterContext, path string, d fs.DirEntry) error {
return nil
}

func emptyPostLinter(_ 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
}

// Nothing to do
if foundfile {
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)
Expand All @@ -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)
Expand All @@ -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
}
58 changes: 41 additions & 17 deletions pkg/build/linter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,43 @@ 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)
assert.NoError(t, err)

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"},
},
},
}
Expand All @@ -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"},
},
},
}
Expand All @@ -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"},
},
},
}
Expand All @@ -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"},
},
},
}
Expand All @@ -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"},
},
},
}
Expand All @@ -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"},
},
},
}
Expand Down Expand Up @@ -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"},
},
},
}
Expand Down Expand Up @@ -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"},
},
},
}
Expand Down Expand Up @@ -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{
Expand Down
3 changes: 2 additions & 1 deletion pkg/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -339,6 +339,7 @@ func (cfg Configuration) Name() string {

var defaultLinters = []string{
"dev",
"empty",
"opt",
"srv",
"setuidgid",
Expand Down