-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[v4.2.0-rhel] Adjust x/text, x/tools, and x/net versions #25650
[v4.2.0-rhel] Adjust x/text, x/tools, and x/net versions #25650
Conversation
@cevich seeing downgrades in Libraries like this makes me nervous. Are you able to run https://pkg.go.dev/golang.org/x/vuln/cmd/govulncheck to make sure nothing pops with these changes? |
@cevich @TomSweeneyRedHat we need to compare against the base of f34c272 as this was the first commit which bumped these things. So in total all these x versions still receive several updates as such I think saying downgrade is wrong, since none of the bump to far versions build in RHEL it means we never included the newer libs in the package and as such even after this we still should only get upgrades. Overall sure it might be that there are other CVE's fixed in these versions so it is good to double check that but AFAIK if they were not reported before then we should have no CVE of concerns in them. |
Correct, "downgrade" is maybe not the best word. It makes sense prior to this commit, but not as of a few commits ago. Let me see if I can come up with a better message. |
Commits f34c272 and d25cb5f upgraded these modules along with `golang.org/x/crypto`. PR containers#25624 subsequently downgraded the crypto module but missed rolling back these other changes to Unfortunately the newer versions of these other modules fall between the differences from Fedora to RHEL, so CI missed their RHEL incompatibility. Under RHEL podman fails to compile with the error: ``` _build/src/github.com/containers/podman/vendor/golang.org/x/net/http2/transport.go:1109:13: tc.NetConn undefined (type *tls.Conn has no field or method NetConn) ``` Rollback `x/text` -> `v0.15.0`, which then through `make vendor` pulls in adjustments to `x/tools` and `x/net`. Though the versions are still newer than what they were prior to f34c272/d25cb5f, so as far as podman releases go, they're actually newer than what was available previously. Manually tested on both RHEL 9.0 & 8.6 Signed-off-by: Chris Evich <[email protected]>
I attempted to install/run govulncheck but I don't think this branch's golang (1.17) is supported.
|
That is not what I see, go 1.23.0 should only be set on main and not on this branch so I am not sure why this error would say 1.23.0 if you are on the branch? I had to only run on These are the only two
Either way both would already be in there before we started patching this so I doubt it matters. |
Oh woops, I was indeed running it on the wrong branch. I had to swap to a modern CI container to install/build Regardless, I trust Paul's analysis and concur, there have been more than a handful we've discarded due to non-applicability (e.x. buildah/podman not using the affected function) or as not severe enough to warrant attention. |
Are there any other concerns here? This PR is currently blocking my packaging work for two RHEL releases. |
LGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cevich, Luap99 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
adb8ba5
into
containers:v4.2.0-rhel
Thanks guys for the oversight, review, and help on this. I appreciate it. |
Commits f34c272 and d25cb5f upgraded these modules along with
golang.org/x/crypto
. PR #25624 subsequently downgraded thecrypto module but missed rolling back these other changes to
Unfortunately the newer versions of these other modules fall
between the differences from Fedora to RHEL, so CI missed
their RHEL incompatibility. Under RHEL podman fails to
compile with the error:
Rollback
x/text
->v0.15.0
, which then throughmake vendor
pulls in adjustments tox/tools
andx/net
. Thoughthe versions are still newer than what they were prior to
f34c272/d25cb5f, so as far as podman releases go, they're actually
newer than what was available previously.
Manually tested on both RHEL 9.0 & 8.6
Does this PR introduce a user-facing change?