fix: conslidate use of masterminds semver across the codebase#8399
Merged
awesomenix merged 1 commit intomainfrom Apr 24, 2026
Merged
fix: conslidate use of masterminds semver across the codebase#8399awesomenix merged 1 commit intomainfrom
awesomenix merged 1 commit intomainfrom
Conversation
awesomenix
requested review from
AbelHu,
Devinwong,
SriHarsha001,
calvin197,
cameronmeissner,
djsly,
ganeshkumarashok,
junjiezhang1997,
lilypan26,
mxj220,
pdamianov-dev,
phealy,
r2k1,
sulixu,
surajssd,
timmy-wright and
zachary-bailey
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Consolidates semantic version parsing/comparison to use github.com/Masterminds/semver/v3 across the main module, e2e, and aks-node-controller, replacing remaining usage of github.com/blang/semver (and a lingering v1 Masterminds import).
Changes:
- Replace
blang/semverusage withMasterminds/semver/v3and update comparisons to v3 APIs. - Harden version-handling helpers to skip/return on parse errors instead of ignoring them.
- Update module dependencies (
go.mod/go.sum) in root,e2e, andaks-node-controlleraccordingly.
Reviewed changes
Copilot reviewed 10 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/agent/utils.go | Switch IsKubernetesVersionGe to Masterminds semver v3 parsing/comparison. |
| pkg/agent/datamodel/versions.go | Replace multiple semver operations (filtering/sorting/comparisons) with v3 APIs and add parse-error handling. |
| go.mod | Drop github.com/blang/semver requirement. |
| go.sum | Remove github.com/blang/semver checksums. |
| e2e/validators.go | Use v3 semver parsing for runc version validation. |
| e2e/toolkit/k8s.go | Fix semver import path to /v3. |
| e2e/scenario_win_test.go | Fix semver import path to /v3. |
| e2e/node_config.go | Fix semver import path to /v3. |
| e2e/go.mod | Replace blang/semver with Masterminds/semver/v3. |
| e2e/go.sum | Remove blang/semver and Masterminds semver v1 entries. |
| aks-node-controller/helpers/utils.go | Switch IsKubernetesVersionGe to v3 parsing/comparison. |
| aks-node-controller/go.mod | Replace blang/semver with Masterminds/semver/v3 (and tidy yaml dep). |
| aks-node-controller/go.sum | Remove blang/semver checksums. |
Comment on lines
1254
to
+1260
| require.Lenf(s.T, versions, 1, "Expected exactly one version for moby-runc but got %d", len(versions)) | ||
| // check if versions[0] is great than or equal to 1.2.0 | ||
| // check semantic version | ||
| semver, err := semver.ParseTolerant(versions[0]) | ||
| parsedVersion, err := semver.NewVersion(versions[0]) | ||
| require.NoError(s.T, err, "failed to parse semver from moby-runc version") | ||
| require.GreaterOrEqual(s.T, int(semver.Major), 1, "expected moby-runc major version to be at least 1, got %d", semver.Major) | ||
| require.GreaterOrEqual(s.T, int(semver.Minor), 2, "expected moby-runc minor version to be at least 2, got %d", semver.Minor) | ||
| require.GreaterOrEqual(s.T, int(parsedVersion.Major()), 1, "expected moby-runc major version to be at least 1, got %d", parsedVersion.Major()) | ||
| require.GreaterOrEqual(s.T, int(parsedVersion.Minor()), 2, "expected moby-runc minor version to be at least 2, got %d", parsedVersion.Minor()) |
There was a problem hiding this comment.
ValidateRuncVersion claims to check that the version is >= 1.2.0, but the current assertions only check Major >= 1 and Minor >= 2. This will incorrectly fail for versions like 2.0.0 (which is >= 1.2.0) and can also pass versions like 1.2.0-alpha depending on intent. Consider using a semver constraint check (e.g., ">= 1.2.0") against the parsed version instead of separate major/minor comparisons.
cameronmeissner
approved these changes
Apr 24, 2026
awesomenix
force-pushed
the
nishp/usemasterminds
branch
from
b4e44e0 to
3f73deb
Compare
lilypan26
approved these changes
Apr 24, 2026
Devinwong
approved these changes
Apr 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We seem to be using some old library for semver, consolidate use of masterminds semver across the codebase