Remove TLS min version validation for OpenShift profile compatibility

[tpantelis]

Removed the validation that enforced TLS 1.2 as the minimum version when the --tls-min-version CLI flag is specified. This allows OpenShift TLS profiles that support older versions (e.g., the "Old" profile with TLS 1.0) to be used when explicitly configured.

When --tls-min-version is not specified, the webhook server still defaults to TLS 1.2, maintaining secure defaults while allowing backwards compatibility when needed.

Summary by CodeRabbit

• Refactor

  • Removed the enforcement of a minimum TLS version requirement that previously restricted configurations to TLS 1.2 and above. The webhook now supports lower TLS versions, providing better compatibility with legacy systems and infrastructure environments

• Tests

  • Reorganized and expanded the test suite to thoroughly validate all supported TLS versions, including legacy versions TLS 1.0 and 1.1, as well as current versions 1.2 and above

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

Details

Needs approval from an approver in each of these files:

• OWNERS

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

[coderabbitai]

Walkthrough

The enforcement that --tls-min-version must be at least TLS 1.2 is removed from startHTTPServers in cmd/webhook/main.go. The corresponding tests in cmd/webhook/main_test.go are consolidated from two separate DescribeTable blocks (reject below 1.2, accept 1.2+) into one table that expects successful startup for all TLS minimum versions, including TLS 1.0 and 1.1.

Changes

Remove TLS 1.2 minimum version floor

Layer / File(s)	Summary
Remove TLS 1.2 floor and update acceptance tests cmd/webhook/main.go, cmd/webhook/main_test.go	The post-parse guard in startHTTPServers that rejected --tls-min-version values below TLS 1.2 is deleted. The test file replaces two DescribeTable blocks (one asserting rejection for TLS 1.0/1.1, one asserting acceptance for 1.2+) with a single table that expects startHTTPServers to succeed for all versions including TLS 1.0 and 1.1.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

🚥 Pre-merge checks | ✅ 12 | ❌ 3

❌ Failed checks (3 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Test Structure And Quality	⚠️ Warning	Assertion on line 139 lacks a meaningful failure message: Expect(err).NotTo(HaveOccurred()) should include a diagnostic message to help identify test failures.	Add failure message to line 139: Expect(err).NotTo(HaveOccurred(), "failed to start HTTP servers with TLS version %s", version)
Ipv6 And Disconnected Network Test Compatibility	⚠️ Warning	New Ginkgo e2e tests added with IPv4 hardcoding: "127.0.0.1", "127.0.0.5", and URL building without IPv6 bracket formatting in testHTTPEndpoint/testHTTPSEndpoint.	Replace hardcoded "127.0.0.1" with "::1" for IPv6 testing; use net.JoinHostPort() for URL construction to properly format IPv6 addresses with brackets. Or add [Skipped:Disconnected] if IPv6 testing is not required.

✅ Passed checks (12 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: removing TLS min version validation to support OpenShift profiles with older TLS versions.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	All Ginkgo test names in the PR are stable and deterministic, containing only static descriptive strings with no dynamic values, generated identifiers, timestamps, or brittle specificity.
Microshift Test Compatibility	✅ Passed	The PR modifies cmd/webhook/main_test.go, which is a unit test using Ginkgo, not an e2e test. The test uses only standard library and doesn't access Kubernetes/OpenShift APIs or cluster resources,...
Single Node Openshift (Sno) Test Compatibility	✅ Passed	The modified tests are unit tests for a webhook admission controller, not OpenShift e2e tests. They test server-side TLS configuration locally and don't assume multi-node cluster topologies.
Topology-Aware Scheduling Compatibility	✅ Passed	PR only modifies TLS configuration in webhook server initialization (cmd/webhook/main.go and main_test.go). No deployment manifests, scheduling constraints, pod affinity rules, disruption budgets,...
Ote Binary Stdout Contract	✅ Passed	PR removes TLS validation and updates test tables with no process-level stdout writes; glog defaults to files not stdout, no fmt.Print/direct stdout writes found, Ginkgo's Describe() only registers...
No-Weak-Crypto	✅ Passed	PR does not introduce weak crypto algorithms (MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB), custom crypto implementations, or non-constant-time secret comparisons. Uses SHA512 hashing and strong ECDHE...
Container-Privileges	✅ Passed	PR changes are limited to TLS validation logic in Go source files (main.go and main_test.go). No Kubernetes manifests, Dockerfiles, or container configurations were modified; therefore, no containe...
No-Sensitive-Data-In-Logs	✅ Passed	PR removes TLS validation logic only; introduces no new logging. Existing error messages log only non-sensitive TLS configuration values (cipher suite names, version strings).
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[tpantelis]

/cc @anuragthehatter @raphaelvrosa @bpickard22

[raphaelvrosa]

Is this mandatory? Why?

[tpantelis]

Is this mandatory? Why?

@raphaelvrosa I explained that in the PR description: "This allows OpenShift TLS profiles that support older versions (e.g., the Old profile with TLS 1.0) to be used when explicitly configured.". While TLS < 1.2 is considered insecure and deprecated, OCP still supports it, at least in 5.0. So if a user did configure the Old TLS profile, the multus admission controller would fail to start.

[tpantelis]

/retest

[openshift-ci]

@tpantelis: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/security	949e73d	link	false	/test security

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.