[YUNIKORN-2541] Upgrade golang.org/x/net for CVEs #135
Conversation
go.mod
Outdated
| golang.org/x/net v0.20.0 // indirect | ||
| golang.org/x/sys v0.17.0 // indirect |
There was a problem hiding this comment.
Don't change these, leave these as they are the replace below will force the later version. Those we can update without the system touching them. The indirect ones are managed by the go module system.
There was a problem hiding this comment.
Done. Thanks for the feedback!
There was a problem hiding this comment.
Hi @targetoee,
We don't manually change indirect dependency.
However, golang.org/x/sys v0.18.0 // indirect could be kept since it was resovled by 'go mod tidy' after upgrade to golang.org/x/net v0.23.0.
Here is our previous discussion for indirect/replace directives
There was a problem hiding this comment.
Don't change these, leave these as they are the replace below will force the later version.
Hi @wilfred-s, I still think we have a valid reason(Fix CVE) to update the automatically generated dependencies that were updated by 'go mod tidy'.
There was a problem hiding this comment.
I think it's reasonable even though the dependencies will propagate to downstream packages.
There was a problem hiding this comment.
Except it doesn't work like that. The purpose of 'go mod tidy' is to remove redundant information and 'tidy up' dependencies. We should never be committing any go.mod / go.sum changes which go against what 'go mod tidy' produces, otherwise every other user will have to undo work that it performs every time. Those dependencies are indirect, and therefore not our responsibility. Instead we use replacements to ensure that our own builds are forced to use newer versions. Please do as Wilfred asks and leave these alone.
There was a problem hiding this comment.
Let me explain my thoughts step by step,
We changed ‘replace' directive to 'golang.org/x/net v0.23.0' first.
Then run 'go mod tidy' and it will automatilcally update 'golang.org/x/sys' to v0.18.0. (The minimum required version)
Below is the tidy result:
Since 'replace' directive take higher precedence than indirect dependencies, we should also change the 'replace' directive to 'golang.org/x/sys v0.18.0'. (Otherwise Go will use the outdated version)
So the correct change for this PR is:
There was a problem hiding this comment.
Ok, I think I understand now. 'Go mod tidy' updated the indirect version so you are updating the replacement to match. I think that's fine.
There was a problem hiding this comment.
My fix CVE dependencies steps:
- Upgrade replace directives to CVE fixed version
- Run 'go mod tidy'
- Check all the existing replace directives, if there have newer version in indirect require dependencies. Update replace directive to newer version too.
We should keep the indirect directives resolved by 'go mod tidy'.
golang.org/x/sys v0.18.0 // indirect
upgrade golang.org/x/net to v0.23 Closes: #135 Signed-off-by: Yu-Lin Chen <[email protected]> (cherry picked from commit c40fbd2)
What is this PR for?
This PR upgrades
golang.org/x/netto 0.23.0 to address CVEs.What type of PR is it?
Todos
After this PR is merged, core and k8shim should also be upgraded.
What is the Jira issue?
YUNIKORN-2541
How should this be tested?
Screenshots (if appropriate)
Questions: