diff --git a/docs/md/melange_build.md b/docs/md/melange_build.md index c2bddde8e..ab73180d9 100644 --- a/docs/md/melange_build.md +++ b/docs/md/melange_build.md @@ -42,6 +42,7 @@ melange build [flags] --dependency-log string log dependencies to a specified file --empty-workspace whether the build workspace should be empty --env-file string file to use for preloaded environment variables + --fail-on-lint-warning turns linter warnings into failures --generate-index whether to generate APKINDEX.tar.gz (default true) --guest-dir string directory used for the build environment guest -h, --help help for build diff --git a/pkg/build/build.go b/pkg/build/build.go index 53b45e5a6..c3fe44b76 100644 --- a/pkg/build/build.go +++ b/pkg/build/build.go @@ -91,6 +91,7 @@ type Build struct { Debug bool DebugRunner bool LogPolicy []string + FailOnLintWarning bool EnabledBuildOptions []string } @@ -252,6 +253,14 @@ func WithConfig(configFile string) Option { } } +// WithFailOnLintWarning sets whether or not to fail on linter warnings. +func WithFailOnLintWarning(fail bool) Option { + return func(b *Build) error { + b.FailOnLintWarning = fail + return nil + } +} + // WithBuildDate sets the timestamps for the build context. // The string is parsed according to RFC3339. // An empty string is a special case and will default to @@ -1133,9 +1142,18 @@ func (b *Build) BuildPackage(ctx context.Context) error { lctx := linter.NewLinterContext(lt.pkgName, fsys, &b.Configuration, <.checks) linters := lt.checks.GetLinters() - err = lctx.LintPackageFs(fsys, linters) + var innerErr error + err = lctx.LintPackageFs(fsys, func(err error) { + if b.FailOnLintWarning { + innerErr = err + } else { + b.Logger.Warnf("WARNING: %v", err) + } + }, linters) if err != nil { return fmt.Errorf("package linter error: %w", err) + } else if innerErr != nil { + return fmt.Errorf("package linter warning: %w", err) } } diff --git a/pkg/cli/build.go b/pkg/cli/build.go index f5a311c55..86b6778f2 100644 --- a/pkg/cli/build.go +++ b/pkg/cli/build.go @@ -60,6 +60,7 @@ func Build() *cobra.Command { var debug bool var debugRunner bool var runner string + var failOnLintWarning bool cmd := &cobra.Command{ Use: "build", @@ -97,6 +98,7 @@ func Build() *cobra.Command { build.WithDebugRunner(debugRunner), build.WithLogPolicy(logPolicy), build.WithRunner(runner), + build.WithFailOnLintWarning(failOnLintWarning), } if len(args) > 0 { @@ -144,6 +146,7 @@ func Build() *cobra.Command { cmd.Flags().BoolVar(&createBuildLog, "create-build-log", false, "creates a package.log file containing a list of packages that were built by the command") cmd.Flags().BoolVar(&debug, "debug", false, "enables debug logging of build pipelines") cmd.Flags().BoolVar(&debugRunner, "debug-runner", false, "when enabled, the builder pod will persist after the build succeeds or fails") + cmd.Flags().BoolVar(&failOnLintWarning, "fail-on-lint-warning", false, "turns linter warnings into failures") return cmd } diff --git a/pkg/linter/linter.go b/pkg/linter/linter.go index b051e7f47..61f59fbe5 100644 --- a/pkg/linter/linter.go +++ b/pkg/linter/linter.go @@ -40,31 +40,73 @@ func NewLinterContext(name string, fsys fs.FS, cfg *config.Configuration, chk *c type linterFunc func(lctx LinterContext, path string, d fs.DirEntry) error type linter struct { - LinterFunc linterFunc - Explain string + LinterFunc linterFunc + FailOnError bool + Explain string } type postLinterFunc func(lctx LinterContext, fsys fs.FS) error type postLinter struct { - LinterFunc postLinterFunc - Explain string + LinterFunc postLinterFunc + FailOnError bool + 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"}, - "setuidgid": linter{isSetUidOrGidLinter, "Unset the setuid/setgid bit on the relevant files, or remove this linter"}, - "srv": linter{srvLinter, "This package should be a -compat package"}, - "tempdir": linter{tempDirLinter, "Remove any offending files in temporary dirs in the pipeline"}, - "usrlocal": linter{usrLocalLinter, "This package should be a -compat package"}, - "varempty": linter{varEmptyLinter, "Remove any offending files in /var/empty in the pipeline"}, - "worldwrite": linter{worldWriteableLinter, "Change the permissions of any world-writeable files in the package, disable the linter, or make this a -compat package"}, - "strip": linter{strippedLinter, "Properly strip all binaries in the pipeline"}, + "dev": linter{ + LinterFunc: devLinter, + FailOnError: false, + Explain: "If this package is creating /dev nodes, it should use udev instead; otherwise, remove any files in /dev", + }, + "opt": linter{ + LinterFunc: optLinter, + FailOnError: false, + Explain: "This package should be a -compat package", + }, + "setuidgid": linter{ + LinterFunc: isSetUidOrGidLinter, + FailOnError: false, + Explain: "Unset the setuid/setgid bit on the relevant files, or remove this linter", + }, + "srv": linter{ + LinterFunc: srvLinter, + FailOnError: false, + Explain: "This package should be a -compat package", + }, + "tempdir": linter{ + LinterFunc: tempDirLinter, + FailOnError: false, + Explain: "Remove any offending files in temporary dirs in the pipeline", + }, + "usrlocal": linter{ + LinterFunc: usrLocalLinter, + FailOnError: false, + Explain: "This package should be a -compat package", + }, + "varempty": linter{ + LinterFunc: varEmptyLinter, + FailOnError: false, + Explain: "Remove any offending files in /var/empty in the pipeline", + }, + "worldwrite": linter{ + LinterFunc: worldWriteableLinter, + FailOnError: false, + Explain: "Change the permissions of any world-writeable files in the package, disable the linter, or make this a -compat package", + }, + "strip": linter{ + LinterFunc: strippedLinter, + FailOnError: false, + Explain: "Properly strip all binaries in the pipeline", + }, } 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"}, + "empty": postLinter{ + LinterFunc: emptyPostLinter, + FailOnError: false, + Explain: "Verify that this package is supposed to be empty; if it is, disable this linter; otherwise check the build", + }, } var isDevRegex = regexp.MustCompile("^dev/") @@ -251,7 +293,7 @@ func emptyPostLinter(_ LinterContext, fsys fs.FS) error { return fmt.Errorf("Package is empty but no-provides is not set") } -func (lctx LinterContext) LintPackageFs(fsys fs.FS, linters []string) error { +func (lctx LinterContext) LintPackageFs(fsys fs.FS, warn func(error), linters []string) error { // If this is a compat package, do nothing. if isCompatPackageRegex.MatchString(lctx.pkgname) { return nil @@ -278,7 +320,10 @@ func (lctx LinterContext) LintPackageFs(fsys fs.FS, linters []string) error { err = linter.LinterFunc(lctx, path, d) if err != nil { - return fmt.Errorf("Linter %s failed at path %q: %w; suggest: %s", linterName, path, err, linter.Explain) + if linter.FailOnError { + return fmt.Errorf("Linter %s failed at path %q: %w; suggest: %s", linterName, path, err, linter.Explain) + } + warn(err) } } @@ -294,7 +339,10 @@ func (lctx LinterContext) LintPackageFs(fsys fs.FS, linters []string) error { linter := postLinterMap[linterName] err := linter.LinterFunc(lctx, fsys) if err != nil { - return fmt.Errorf("Linter %s failed; suggest: %s", linterName, linter.Explain) + if linter.FailOnError { + return fmt.Errorf("Linter %s failed; suggest: %s", linterName, linter.Explain) + } + warn(err) } } diff --git a/pkg/linter/linter_test.go b/pkg/linter/linter_test.go index 6ada50293..f5fcc7f73 100644 --- a/pkg/linter/linter_test.go +++ b/pkg/linter/linter_test.go @@ -46,7 +46,11 @@ func Test_emptyLinter(t *testing.T) { assert.Equal(t, linters, []string{"empty"}) fsys := os.DirFS(dir) lctx := NewLinterContext(cfg.Package.Name, fsys, &cfg, &cfg.Package.Checks) - assert.Error(t, lctx.LintPackageFs(fsys, linters)) + called := false + assert.NoError(t, lctx.LintPackageFs(fsys, func(err error) { + called = true + }, linters)) + assert.True(t, called) } func Test_usrLocalLinter(t *testing.T) { @@ -75,7 +79,11 @@ func Test_usrLocalLinter(t *testing.T) { assert.Equal(t, linters, []string{"usrlocal"}) fsys := os.DirFS(dir) lctx := NewLinterContext(cfg.Package.Name, fsys, &cfg, &cfg.Package.Checks) - assert.Error(t, lctx.LintPackageFs(fsys, linters)) + called := false + assert.NoError(t, lctx.LintPackageFs(fsys, func(err error) { + called = true + }, linters)) + assert.True(t, called) } func Test_varEmptyLinter(t *testing.T) { @@ -105,7 +113,11 @@ func Test_varEmptyLinter(t *testing.T) { assert.Equal(t, linters, []string{"varempty"}) fsys := os.DirFS(dir) lctx := NewLinterContext(cfg.Package.Name, fsys, &cfg, &cfg.Package.Checks) - assert.Error(t, lctx.LintPackageFs(fsys, linters)) + called := false + assert.NoError(t, lctx.LintPackageFs(fsys, func(err error) { + called = true + }, linters)) + assert.True(t, called) } func Test_devLinter(t *testing.T) { @@ -135,7 +147,11 @@ func Test_devLinter(t *testing.T) { assert.Equal(t, linters, []string{"dev"}) fsys := os.DirFS(dir) lctx := NewLinterContext(cfg.Package.Name, fsys, &cfg, &cfg.Package.Checks) - assert.Error(t, lctx.LintPackageFs(fsys, linters)) + called := false + assert.NoError(t, lctx.LintPackageFs(fsys, func(err error) { + called = true + }, linters)) + assert.True(t, called) } func Test_optLinter(t *testing.T) { @@ -165,7 +181,11 @@ func Test_optLinter(t *testing.T) { assert.Equal(t, linters, []string{"opt"}) fsys := os.DirFS(dir) lctx := NewLinterContext(cfg.Package.Name, fsys, &cfg, &cfg.Package.Checks) - assert.Error(t, lctx.LintPackageFs(fsys, linters)) + called := false + assert.NoError(t, lctx.LintPackageFs(fsys, func(err error) { + called = true + }, linters)) + assert.True(t, called) } func Test_srvLinter(t *testing.T) { @@ -195,7 +215,11 @@ func Test_srvLinter(t *testing.T) { assert.Equal(t, linters, []string{"srv"}) fsys := os.DirFS(dir) lctx := NewLinterContext(cfg.Package.Name, fsys, &cfg, &cfg.Package.Checks) - assert.Error(t, lctx.LintPackageFs(fsys, linters)) + called := false + assert.NoError(t, lctx.LintPackageFs(fsys, func(err error) { + called = true + }, linters)) + assert.True(t, called) } func Test_tempDirLinter(t *testing.T) { @@ -237,7 +261,11 @@ func Test_tempDirLinter(t *testing.T) { _, err = os.Create(filename) assert.NoError(t, err) lctx := NewLinterContext(cfg.Package.Name, fsys, &cfg, &cfg.Package.Checks) - assert.Error(t, lctx.LintPackageFs(fsys, linters)) + called := false + assert.NoError(t, lctx.LintPackageFs(fsys, func(err error) { + called = true + }, linters)) + assert.True(t, called) os.Remove(filename) // Test /var/tmp check @@ -292,7 +320,11 @@ func Test_setUidGidLinter(t *testing.T) { assert.Equal(t, linters, []string{"setuidgid"}) fsys := os.DirFS(dir) lctx := NewLinterContext(cfg.Package.Name, fsys, &cfg, &cfg.Package.Checks) - assert.Error(t, lctx.LintPackageFs(fsys, linters)) + called := false + assert.NoError(t, lctx.LintPackageFs(fsys, func(err error) { + called = true + }, linters)) + assert.True(t, called) } func Test_worldWriteLinter(t *testing.T) { @@ -321,7 +353,11 @@ func Test_worldWriteLinter(t *testing.T) { assert.Equal(t, linters, []string{"worldwrite"}) fsys := os.DirFS(dir) lctx := NewLinterContext(cfg.Package.Name, fsys, &cfg, &cfg.Package.Checks) - assert.NoError(t, lctx.LintPackageFs(fsys, linters)) + called := false + assert.NoError(t, lctx.LintPackageFs(fsys, func(err error) { + called = true + }, linters)) + assert.False(t, called) // Create test file filePath := filepath.Join(usrLocalDirPath, "test.txt") @@ -333,21 +369,33 @@ func Test_worldWriteLinter(t *testing.T) { assert.NoError(t, err) // Linter should not trigger - assert.NoError(t, lctx.LintPackageFs(fsys, linters)) + called = false + assert.NoError(t, lctx.LintPackageFs(fsys, func(err error) { + called = true + }, linters)) + assert.False(t, called) // Set writeable bit (but not executable bit) err = os.Chmod(filePath, 0776) assert.NoError(t, err) // Linter should trigger - assert.Error(t, lctx.LintPackageFs(fsys, linters)) + called = false + assert.NoError(t, lctx.LintPackageFs(fsys, func(err error) { + called = true + }, linters)) + assert.True(t, called) // Set writeable and executable bit err = os.Chmod(filePath, 0777) assert.NoError(t, err) // Linter should trigger - assert.Error(t, lctx.LintPackageFs(fsys, linters)) + called = false + assert.NoError(t, lctx.LintPackageFs(fsys, func(err error) { + called = true + }, linters)) + assert.True(t, called) } func Test_disableDefaultLinter(t *testing.T) { @@ -378,5 +426,9 @@ func Test_disableDefaultLinter(t *testing.T) { linters := cfg.Package.Checks.GetLinters() fsys := os.DirFS(dir) lctx := NewLinterContext(cfg.Package.Name, fsys, &cfg, &cfg.Package.Checks) - assert.NoError(t, lctx.LintPackageFs(fsys, linters)) + called := false + assert.NoError(t, lctx.LintPackageFs(fsys, func(err error) { + called = true + }, linters)) + assert.False(t, called) }