Skip to content

Conversation

@Sunnatillo
Copy link
Member

What this PR does / why we need it:

Uplifts Crypto to v0.43.0 and Go to 1.24 to fix vulnerability

Fixes #

Checklist:

  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • E2E tests have been added, if necessary.
  • Integration tests have been added, if necessary.

@metal3-io-bot metal3-io-bot added this to the CAPM3 - v1.10 milestone Nov 14, 2025
@metal3-io-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign sunnatillo for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@metal3-io-bot metal3-io-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 14, 2025
@Sunnatillo
Copy link
Member Author

/test metal3-centos-e2e-integration-test-release-1-10

@Sunnatillo
Copy link
Member Author

/test metal3-centos-e2e-integration-test-release-1-10

@Sunnatillo
Copy link
Member Author

/test metal3-centos-e2e-integration-test-release-1-10

Copy link
Member

@lentzi90 lentzi90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a minor go version bump. Can we really do it on a release branch?
I think we usually avoid that.

@Sunnatillo
Copy link
Member Author

Sunnatillo commented Nov 14, 2025

This is a minor go version bump. Can we really do it on a release branch? I think we usually avoid that.

Bump Crypto to v0.43.0 is a vulnerability fix. And it specifically requires v1.24.9 version. I was hesitant at first.
I am not sure. Should we uplift or not?

@Rozzii
Copy link
Member

Rozzii commented Nov 14, 2025

This is a minor go version bump. Can we really do it on a release branch? I think we usually avoid that.

Bump Crypto to v0.43.0 is a vulnerability fix. And it specifically requires v1.24.9 version. I was hesitant at first, because we use v1.24.9 in Dockerfile. I am not sure. Should we uplift or not?

Also the go.mod's go version bump is quite big for a patch release so we might need some discussion about it with the whole release team and maintainers.

@Rozzii
Copy link
Member

Rozzii commented Nov 14, 2025

This is a minor go version bump. Can we really do it on a release branch? I think we usually avoid that.

Bump Crypto to v0.43.0 is a vulnerability fix. And it specifically requires v1.24.9 version. I was hesitant at first, because we use v1.24.9 in Dockerfile. I am not sure. Should we uplift or not?

Also the go.mod's go version bump is quite big for a patch release so we might need some discussion about it with the whole release team and maintainers.

Let's wait until monday, My assesment is that although in my eyes fixing the CVE would justify the go.mod GO version bump, the CVE description tells me that we are not affected even do this is a 7,5 CVE as we are not using the effected lib to facilitate SSH connections. So at least IMO we don't need to hurry this let's wait with the patch release until Monday let's the actual security expert take a look.
vuln: GHSA-56w8-48fp-6mgv

Copy link
Member

@Rozzii Rozzii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/hold
Let's hold to evaluate the vuln.

@metal3-io-bot metal3-io-bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged. labels Nov 14, 2025
@Rozzii
Copy link
Member

Rozzii commented Nov 14, 2025

@adilGhaffarDev noticed CVE is only affecting our e2e test code not he actual application so
/lgtm cancel
We can skip this bump.

@metal3-io-bot metal3-io-bot removed the lgtm Indicates that a PR is ready to be merged. label Nov 14, 2025
@Rozzii Rozzii added the invalid This doesn't seem right label Nov 14, 2025
Copy link
Member

@tuminoid tuminoid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really vulnerable, thus we don't bump the go.mod versioning to avoid breaking downstream users.

/close

module github.com/metal3-io/cluster-api-provider-metal3/api

go 1.23.7
go 1.24.9
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't do this in release branches

@metal3-io-bot
Copy link
Contributor

@tuminoid: Closed this PR.

In response to this:

Not really vulnerable, thus we don't bump the go.mod versioning to avoid breaking downstream users.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@tuminoid tuminoid deleted the Sunnatillo/bump-go branch November 17, 2025 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. invalid This doesn't seem right size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants