Skip to content

Commit

Permalink
feat(updater): use upgrade config for Maven version updater (#1636)
Browse files Browse the repository at this point in the history
This PR aligns the flags of `update`, especially the `upgrade-config`
with `fix` subcommand.

To be able to parse the upgrade config, this PR moves the parsing
function to `upgrade` package.

The next step is to make
[`comparisonFunctionWithWorkarounds`](https://github.com/google/osv-scanner/blob/e0206dedd17080021b768526f2a6fde635497c25/internal/remediation/override.go#L271)
reusable and used in Maven updater as well.
  • Loading branch information
cuixq authored Feb 19, 2025
1 parent ac5a74c commit e2eb458
Show file tree
Hide file tree
Showing 9 changed files with 145 additions and 95 deletions.
39 changes: 1 addition & 38 deletions cmd/osv-scanner/fix/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,43 +216,6 @@ func Command(stdout, stderr io.Writer, r *reporter.Reporter) *cli.Command {
}
}

func parseUpgradeConfig(ctx *cli.Context, r reporter.Reporter) upgrade.Config {
config := upgrade.NewConfig()

for _, spec := range ctx.StringSlice("upgrade-config") {
idx := strings.LastIndex(spec, ":")
if idx == 0 {
r.Warnf("WARNING: `--upgrade-config %s` - skipping empty package name\n", spec)
continue
}
pkg := ""
levelStr := spec
if idx > 0 {
pkg = spec[:idx]
levelStr = spec[idx+1:]
}
var level upgrade.Level
switch levelStr {
case "major":
level = upgrade.Major
case "minor":
level = upgrade.Minor
case "patch":
level = upgrade.Patch
case "none":
level = upgrade.None
default:
r.Warnf("WARNING: `--upgrade-config %s` - invalid level string '%s'\n", spec, levelStr)
continue
}
if config.Set(pkg, level) { // returns true if was previously set
r.Warnf("WARNING: `--upgrade-config %s` - config for package specified multiple times\n", spec)
}
}

return config
}

func action(ctx *cli.Context, stdout, stderr io.Writer) (reporter.Reporter, error) {
if !ctx.IsSet("manifest") && !ctx.IsSet("lockfile") {
return nil, errors.New("manifest or lockfile is required")
Expand Down Expand Up @@ -282,7 +245,7 @@ func action(ctx *cli.Context, stdout, stderr io.Writer) (reporter.Reporter, erro
DevDeps: !ctx.Bool("ignore-dev"),
MinSeverity: ctx.Float64("min-severity"),
MaxDepth: ctx.Int("max-depth"),
UpgradeConfig: parseUpgradeConfig(ctx, r),
UpgradeConfig: upgrade.ParseUpgradeConfig(ctx.StringSlice("upgrade-config"), r),
},
Manifest: ctx.String("manifest"),
Lockfile: ctx.String("lockfile"),
Expand Down
2 changes: 1 addition & 1 deletion cmd/osv-scanner/fix/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func TestParseUpgradeConfig(t *testing.T) {
if err != nil {
t.Fatalf("error parsing flags: %v", err)
}
config := parseUpgradeConfig(ctx, &reporter.VoidReporter{})
config := upgrade.ParseUpgradeConfig(ctx.StringSlice("upgrade-config"), &reporter.VoidReporter{})
for pkg, want := range tt.want {
if got := config.Get(pkg); got != want {
t.Errorf("Config.Get(%s) got = %v, want %v", pkg, got, want)
Expand Down
88 changes: 60 additions & 28 deletions cmd/osv-scanner/update/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ import (
"io"
"os"

"deps.dev/util/resolve"
"github.com/google/osv-scanner/v2/internal/depsdev"
"github.com/google/osv-scanner/v2/internal/remediation/suggest"
"github.com/google/osv-scanner/v2/internal/remediation/upgrade"
"github.com/google/osv-scanner/v2/internal/resolution/client"
"github.com/google/osv-scanner/v2/internal/resolution/depfile"
"github.com/google/osv-scanner/v2/internal/resolution/manifest"
Expand All @@ -29,18 +31,27 @@ func Command(stdout, stderr io.Writer, r *reporter.Reporter) *cli.Command {
TakesFile: true,
Required: true,
},
&cli.StringSliceFlag{
Name: "disallow-package-upgrades",
Usage: "list of packages that disallow updates",
},
&cli.StringSliceFlag{
Name: "disallow-major-upgrades",
Usage: "list of packages that disallow major updates",
},
&cli.BoolFlag{
Name: "ignore-dev",
Usage: "whether to ignore development dependencies for updates",
},
&cli.StringSliceFlag{
Name: "upgrade-config",
Usage: "the allowed package upgrades, in the format `[package-name:]level`. If package-name is omitted, level is applied to all packages. level must be one of (major, minor, patch, none).",
DefaultText: "major",
},
&cli.StringFlag{
Name: "data-source",
Usage: "source to fetch package information from; value can be: deps.dev, native",
Value: "deps.dev",
Action: func(_ *cli.Context, s string) error {
if s != "deps.dev" && s != "native" {
return fmt.Errorf("unsupported data-source \"%s\" - must be one of: deps.dev, native", s)
}

return nil
},
},
},
Action: func(ctx *cli.Context) error {
var err error
Expand All @@ -52,36 +63,58 @@ func Command(stdout, stderr io.Writer, r *reporter.Reporter) *cli.Command {
}

type updateOptions struct {
Manifest string
NoUpdates []string
AvoidMajor []string
IgnoreDev bool
Manifest string
IgnoreDev bool
UpgradeConfig upgrade.Config // Allowed upgrade levels per package.

Client client.ResolutionClient
Client client.DependencyClient
ManifestRW manifest.ReadWriter
}

func action(ctx *cli.Context, stdout, stderr io.Writer) (reporter.Reporter, error) {
r := reporter.NewTableReporter(stdout, stderr, reporter.InfoLevel, false, 0)

options := updateOptions{
Manifest: ctx.String("manifest"),
NoUpdates: ctx.StringSlice("disallow-package-upgrades"),
AvoidMajor: ctx.StringSlice("disallow-major-upgrades"),
IgnoreDev: ctx.Bool("ignore-dev"),
Manifest: ctx.String("manifest"),
IgnoreDev: ctx.Bool("ignore-dev"),
UpgradeConfig: upgrade.ParseUpgradeConfig(ctx.StringSlice("upgrade-config"), r),
}

if _, err := os.Stat(options.Manifest); errors.Is(err, os.ErrNotExist) {
return nil, fmt.Errorf("file not found: %s", options.Manifest)
} else if err != nil {
return nil, err
}

var err error
options.Client.DependencyClient, err = client.NewDepsDevClient(depsdev.DepsdevAPI, "osv-scanner_update/"+version.OSVVersion)
if err != nil {
return nil, err
system := resolve.UnknownSystem
if options.Manifest != "" {
rw, err := manifest.GetReadWriter(options.Manifest, ctx.String("maven-registry"))
if err != nil {
return nil, err
}
options.ManifestRW = rw
system = rw.System()
}
options.ManifestRW, err = manifest.GetReadWriter(options.Manifest, "")
if err != nil {
return nil, err

var err error
switch ctx.String("data-source") {
case "deps.dev":
options.Client, err = client.NewDepsDevClient(depsdev.DepsdevAPI, "osv-scanner_update/"+version.OSVVersion)
if err != nil {
return nil, err
}
case "native":
switch system {
case resolve.Maven:
options.Client, err = client.NewMavenRegistryClient(ctx.String("maven-registry"))
if err != nil {
return nil, err
}
case resolve.NPM, resolve.UnknownSystem:
fallthrough
default:
return nil, fmt.Errorf("native data-source currently unsupported for %s ecosystem", system.String())
}
}

df, err := depfile.OpenLocalDepFile(options.Manifest)
Expand All @@ -99,13 +132,12 @@ func action(ctx *cli.Context, stdout, stderr io.Writer) (reporter.Reporter, erro
return nil, err
}
patch, err := suggester.Suggest(ctx.Context, options.Client, mf, suggest.Options{
IgnoreDev: options.IgnoreDev,
NoUpdates: options.NoUpdates,
AvoidMajor: options.AvoidMajor,
IgnoreDev: options.IgnoreDev,
UpgradeConfig: options.UpgradeConfig,
})
if err != nil {
return nil, err
}

return reporter.NewTableReporter(stdout, stderr, reporter.InfoLevel, false, 0), manifest.Overwrite(options.ManifestRW, options.Manifest, patch)
return r, manifest.Overwrite(options.ManifestRW, options.Manifest, patch)
}
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ require (
deps.dev/api/v3 v3.0.0-20250210015309-e519ac173dde
deps.dev/util/maven v0.0.0-20250210015309-e519ac173dde
deps.dev/util/resolve v0.0.0-20250210015309-e519ac173dde
deps.dev/util/semver v0.0.0-20250210015309-e519ac173dde
deps.dev/util/semver v0.0.0-20250219000316-bc85dc8a8bd7
github.com/BurntSushi/toml v1.4.0
github.com/CycloneDX/cyclonedx-go v0.9.2
github.com/charmbracelet/bubbles v0.20.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ deps.dev/util/maven v0.0.0-20250210015309-e519ac173dde h1:Ng5gs7X6Bx83nFMtS7H3S3
deps.dev/util/maven v0.0.0-20250210015309-e519ac173dde/go.mod h1:95XAmYKjcTdXRC6BRVwH0sqVZ35SafZq/9jgnus0nYw=
deps.dev/util/resolve v0.0.0-20250210015309-e519ac173dde h1:Rt0iztauNfzV2aEAMbLNBVxKbD18RUsmDOLOs6YU4fk=
deps.dev/util/resolve v0.0.0-20250210015309-e519ac173dde/go.mod h1:iRm31pzwP9sNQ2yCXKXsqO4E1lVgbIhHQX1q/SX+Etc=
deps.dev/util/semver v0.0.0-20250210015309-e519ac173dde h1:qyaH6Mm6lASpYSoFWSwNru6IsHXlDQYnenozh3EdYzI=
deps.dev/util/semver v0.0.0-20250210015309-e519ac173dde/go.mod h1:jjJweVqtuMQ7Q4zlTQ/kCHpboojkRvpMYlhy/c93DVU=
deps.dev/util/semver v0.0.0-20250219000316-bc85dc8a8bd7 h1:KFlPBD/9HYJb8oJvrvafCzAzZb72T+0aJPVEhGbec6Q=
deps.dev/util/semver v0.0.0-20250219000316-bc85dc8a8bd7/go.mod h1:jjJweVqtuMQ7Q4zlTQ/kCHpboojkRvpMYlhy/c93DVU=
github.com/AdaLogics/go-fuzz-headers v0.0.0-20230811130428-ced1acdcaa24 h1:bvDV9vkmnHYOMsOr4WLk+Vo07yKIzd94sVoIqshQ4bU=
github.com/AdaLogics/go-fuzz-headers v0.0.0-20230811130428-ced1acdcaa24/go.mod h1:8o94RPi1/7XTJvwPpRSzSUedZrtlirdB3r9Z20bi2f8=
github.com/AdamKorcz/go-118-fuzz-build v0.0.0-20230306123547-8075edf89bb0 h1:59MxjQVfjXsBpLy+dbd2/ELV5ofnUkUZBvWSC85sheA=
Expand Down
19 changes: 13 additions & 6 deletions internal/remediation/suggest/maven.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@ import (
"errors"
"fmt"
"log"
"strings"

"deps.dev/util/resolve"
"deps.dev/util/semver"
"github.com/google/osv-scanner/v2/internal/remediation/upgrade"
"github.com/google/osv-scanner/v2/internal/resolution/manifest"
"golang.org/x/exp/slices"
)
Expand All @@ -17,15 +19,15 @@ type MavenSuggester struct{}
// Suggest returns the ManifestPatch to update Maven dependencies to a newer
// version based on the options.
// ManifestPatch also includes the property patches to update.
func (ms *MavenSuggester) Suggest(ctx context.Context, client resolve.Client, mf manifest.Manifest, opts Options) (manifest.Patch, error) {
func (ms *MavenSuggester) Suggest(ctx context.Context, cl resolve.Client, mf manifest.Manifest, opts Options) (manifest.Patch, error) {
specific, ok := mf.EcosystemSpecific.(manifest.MavenManifestSpecific)
if !ok {
return manifest.Patch{}, errors.New("invalid MavenManifestSpecific data")
}

var changedDeps []manifest.DependencyPatch //nolint:prealloc
for _, req := range append(mf.Requirements, specific.RequirementsForUpdates...) {
if slices.Contains(opts.NoUpdates, req.Name) {
if opts.UpgradeConfig.Get(req.Name) == upgrade.None {
continue
}

Expand All @@ -34,8 +36,12 @@ func (ms *MavenSuggester) Suggest(ctx context.Context, client resolve.Client, mf
// and updates on development dependencies are not desired
continue
}
if strings.Contains(req.Name, "${") && strings.Contains(req.Version, "${") {
// If there are unresolved properties, we should skip this version.
continue
}

latest, err := suggestMavenVersion(ctx, client, req, slices.Contains(opts.AvoidMajor, req.Name))
latest, err := suggestMavenVersion(ctx, cl, req, opts.UpgradeConfig.Get(req.Name))
if err != nil {
return manifest.Patch{}, fmt.Errorf("suggesting latest version of %s: %w", req.Version, err)
}
Expand Down Expand Up @@ -65,8 +71,8 @@ func (ms *MavenSuggester) Suggest(ctx context.Context, client resolve.Client, mf
// update is a major update or not.
// - if the latest version does not satisfy the constraint, this version is returned;
// otherwise, the original version range requirement is returned.
func suggestMavenVersion(ctx context.Context, client resolve.Client, req resolve.RequirementVersion, noMajorUpdates bool) (resolve.RequirementVersion, error) {
versions, err := client.Versions(ctx, req.PackageKey)
func suggestMavenVersion(ctx context.Context, cl resolve.Client, req resolve.RequirementVersion, level upgrade.Level) (resolve.RequirementVersion, error) {
versions, err := cl.Versions(ctx, req.PackageKey)
if err != nil {
return resolve.RequirementVersion{}, fmt.Errorf("requesting versions of Maven package %s: %w", req.Name, err)
}
Expand Down Expand Up @@ -107,7 +113,8 @@ func suggestMavenVersion(ctx context.Context, client resolve.Client, req resolve
// Skip versions smaller than the current requirement
continue
}
if _, diff := v.Difference(current); diff == semver.DiffMajor && noMajorUpdates {
_, diff := v.Difference(current)
if !level.Allows(diff) {
continue
}
newReq = v
Expand Down
37 changes: 21 additions & 16 deletions internal/remediation/suggest/maven_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"deps.dev/util/resolve"
"deps.dev/util/resolve/dep"
"github.com/google/osv-scanner/v2/internal/remediation/upgrade"
"github.com/google/osv-scanner/v2/internal/resolution/manifest"
)

Expand Down Expand Up @@ -257,9 +258,11 @@ func TestSuggest(t *testing.T) {
}

got, err := suggester.Suggest(ctx, client, mf, Options{
IgnoreDev: true, // Do no update test dependencies.
NoUpdates: []string{"org.example:no-updates"},
AvoidMajor: []string{"org.import:xyz"},
IgnoreDev: true, // Do no update test dependencies.
UpgradeConfig: upgrade.Config{
"org.example:no-updates": upgrade.None,
"org.import:xyz": upgrade.Minor,
},
})
if err != nil {
t.Fatalf("failed to suggest ManifestPatch: %v", err)
Expand Down Expand Up @@ -382,22 +385,24 @@ func TestSuggestVersion(t *testing.T) {
}

tests := []struct {
requirement string
noMajorUpdates bool
want string
requirement string
level upgrade.Level
want string
}{
{"1.0.0", false, "2.3.4"},
{"1.0.0", upgrade.Major, "2.3.4"},
// No major updates allowed
{"1.0.0", true, "1.2.3"},
{"1.0.0", upgrade.Minor, "1.2.3"},
// Only allow patch updates
{"1.0.0", upgrade.Patch, "1.0.1"},
// Version range requirement is not outdated
{"[1.0.0,)", false, "[1.0.0,)"},
{"[2.0.0, 2.3.4]", false, "[2.0.0, 2.3.4]"},
{"[1.0.0,)", upgrade.Major, "[1.0.0,)"},
{"[2.0.0, 2.3.4]", upgrade.Major, "[2.0.0, 2.3.4]"},
// Version range requirement is outdated
{"[2.0.0, 2.3.4)", false, "2.3.4"},
{"[2.0.0, 2.2.2]", false, "2.3.4"},
{"[2.0.0, 2.3.4)", upgrade.Major, "2.3.4"},
{"[2.0.0, 2.2.2]", upgrade.Major, "2.3.4"},
// Version range requirement is outdated but latest version is a major update
{"[1.0.0,2.0.0)", false, "2.3.4"},
{"[1.0.0,2.0.0)", true, "[1.0.0,2.0.0)"},
{"[1.0.0,2.0.0)", upgrade.Major, "2.3.4"},
{"[1.0.0,2.0.0)", upgrade.Minor, "[1.0.0,2.0.0)"},
}
for _, tt := range tests {
vk := resolve.VersionKey{
Expand All @@ -412,12 +417,12 @@ func TestSuggestVersion(t *testing.T) {
Version: tt.want,
},
}
got, err := suggestMavenVersion(ctx, lc, resolve.RequirementVersion{VersionKey: vk}, tt.noMajorUpdates)
got, err := suggestMavenVersion(ctx, lc, resolve.RequirementVersion{VersionKey: vk}, tt.level)
if err != nil {
t.Fatalf("fail to suggest a new version for %v: %v", vk, err)
}
if !reflect.DeepEqual(got, want) {
t.Errorf("suggestMavenVersion(%v, %t): got %s want %s", vk, tt.noMajorUpdates, got, want)
t.Errorf("suggestMavenVersion(%v, %v): got %s want %s", vk, tt.level, got, want)
}
}
}
6 changes: 3 additions & 3 deletions internal/remediation/suggest/suggest.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ import (
"fmt"

"deps.dev/util/resolve"
"github.com/google/osv-scanner/v2/internal/remediation/upgrade"
"github.com/google/osv-scanner/v2/internal/resolution/manifest"
)

type Options struct {
IgnoreDev bool // Whether we should ignore development dependencies for updates
NoUpdates []string // List of packages that disallow updates
AvoidMajor []string // List of packages that disallow major updates
IgnoreDev bool // Whether we should ignore development dependencies for updates
UpgradeConfig upgrade.Config // Allowed upgrade levels per package.
}

// A PatchSuggester provides an ecosystem-specific method for 'suggesting'
Expand Down
Loading

0 comments on commit e2eb458

Please sign in to comment.