Skip to content

Commit

Permalink
Merge pull request #700 from Elizafox/binarylinter
Browse files Browse the repository at this point in the history 
Add post-file walk linting and empty package linting
  • Loading branch information
@kaniini
kaniini authored Sep 20, 2023
2 parents ef430e5 + cf1ccd4 commit a06539b
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 19 deletions.
59 changes: 58 additions & 1 deletion pkg/build/linter.go
View file
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
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
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

0 comments on commit a06539b

Please sign in to comment.