search

kubernetes-sigs / cluster-api-provider-openstack

Cluster API implementation for OpenStack
https://cluster-api-openstack.sigs.k8s.io/
Apache License 2.0
289 stars 253 forks source link

:seedling: Refactoring: never assign unacceptable TLS versions #2037

Closed pierreprinetti closed 5 months ago

pierreprinetti commented 5 months ago

What this PR does / why we need it:

Our downstream security scan is confused by GetTLSVersion returning 0 as a value (even if coupled by a non-nil error), which could end up being assigned to the same identifier that (in a non-error context) would set the TLS version.

This patch makes security linting easier by never setting a TLS version outside v1.2 or v1.3, even in case of an unacceptable user input.

Special notes for your reviewer:

This is expected to be a pure refactoring. Please reject this patch if it introduces any change in behaviour.

TODOs:

/hold

Fixes: #2034

netlify[bot] commented 5 months ago

Deploy Preview for kubernetes-sigs-cluster-api-openstack ready!

Name Link
Latest commit 27526d5f37d843c6b9ba15d302ec026c0e7da227
Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-cluster-api-openstack/deploys/662a79c8b541e500098bbd28
Deploy Preview https://deploy-preview-2037--kubernetes-sigs-cluster-api-openstack.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

mdbooth commented 5 months ago

Thanks. Can you please do a couple of things for me before merging?

pierreprinetti commented 5 months ago
  • To make a static analyzer happy, ensure there is no code path

done. PTAL

mdbooth commented 5 months ago

/approve

Thank you!

k8s-ci-robot commented 5 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mdbooth

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: - ~~[OWNERS](https://github.com/kubernetes-sigs/cluster-api-provider-openstack/blob/main/OWNERS)~~ [mdbooth] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
pierreprinetti commented 5 months ago

/hold cancel I think this is ready for a review.

MaysaMacedo commented 5 months ago

/lgtm

pierreprinetti commented 5 months ago

/hold good that I fixed the tests

EmilienM commented 5 months ago

tests still failing. Happy to LGTM once they're fixed.

pierreprinetti commented 5 months ago

/hold cancel :crossed_fingers:

mdbooth commented 5 months ago

Unit tests pass, and this PR doesn't do anything which should affect the e2e tests.

/lgtm

EmilienM commented 4 months ago

@pierreprinetti do we want/need it in release-0.10?

pierreprinetti commented 4 months ago

@pierreprinetti do we want/need it in release-0.10?

that'd be swell.

In the meantime, the bug has been validated by Snyk and reportedly passed on to their engineering.

EmilienM commented 4 months ago

/cherry-pick release-0.10

k8s-infra-cherrypick-robot commented 4 months ago

@EmilienM: new pull request created: #2062

In response to [this](https://github.com/kubernetes-sigs/cluster-api-provider-openstack/pull/2037#issuecomment-2096192580): >/cherry-pick release-0.10 Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.