Add configurable TLS by RamLavi · Pull Request #600 · k8snetworkplumbingwg/kubemacpool

RamLavi · 2026-03-09T20:49:04Z

What this PR does / why we need it:
kubemacpool's two TLS endpoints (webhook on :8000, metrics proxy on :8443) had no explicit TLS configuration — the webhook was falling back to TLS 1.0, and kube-rbac-proxy used its built-in defaults.

This PR:

Bumps kube-rbac-proxy to v0.21.0 and removes the deprecated --logtostderr flag
Adds --tls-min-version and --tls-cipher-suites to kube-rbac-proxy
Sets TLS_MIN_VERSION and TLS_CIPHERS env vars on the manager container (already read by the webhook code, but never set in the manifest)
Unifies both containers on the VersionTLS13 format, with a small wrapper translating for kube-admission-webhook internally

Defaults: TLS 1.3 minimum, modern-equivalent cipher list. Deployers like CNAO can override with a single TLS version value for both containers.

Special notes for your reviewer:

Release note:

Add configurable TLS env vars.
Change TLS_MIN_VERSION format

Bump the kube-rbac-proxy sidecar image from v0.18.1 to v0.21.0. The legacy manager_auth_proxy_patch.yaml is also updated from the stale gcr.io/kubebuilder/kube-rbac-proxy:v0.4.0 reference. Assisted-by: Claude Upus 4.6 <noreply@anthropic.com> Signed-off-by: Ram Lavi <ralavi@redhat.com>

The --logtostderr flag was removed in klog v2+ and has no effect since stderr is the default output. Also remove --v=10 from the unused legacy patch file. Assisted-by: Claude Upus 4.6 <noreply@anthropic.com> Signed-off-by: Ram Lavi <ralavi@redhat.com>

kubevirt-bot · 2026-03-09T20:49:18Z

[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 ramlavi 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

gemini-code-assist · 2026-03-09T20:49:24Z

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the security posture of kubemacpool by introducing comprehensive and configurable TLS settings for its webhook and metrics proxy endpoints. Previously, these endpoints relied on default or outdated TLS configurations, which led to security vulnerabilities flagged by scanners. The changes standardize TLS versioning, update the kube-rbac-proxy component, and provide explicit control over minimum TLS versions and cipher suites, ensuring compliance with modern security standards and allowing deployers to override defaults.

Highlights

Updated kube-rbac-proxy: The kube-rbac-proxy image was updated to v0.21.0 across various configuration files, removing the deprecated --logtostderr flag.
Configurable TLS for kube-rbac-proxy: Added --tls-min-version and --tls-cipher-suites arguments to the kube-rbac-proxy container, allowing explicit configuration of TLS security parameters.
Configurable TLS for Manager Webhook: Introduced TLS_MIN_VERSION and TLS_CIPHERS environment variables for the manager container, enabling configurable TLS settings for the webhook.
Unified TLS Version Handling: Implemented a new goTLSVersionToHumanReadable function to convert Go's crypto/tls constant names (e.g., VersionTLS12) to a human-readable format (e.g., 1.2) expected by kube-admission-webhook.
Improved Security Compliance: Addressed security scanner flags by explicitly configuring TLS 1.2 as the minimum version and providing a robust, intermediate-equivalent cipher suite list for both webhook and metrics proxy endpoints.

Changelog

config/default/manager/manager.yaml
- Added TLS_MIN_VERSION and TLS_CIPHERS environment variables to the manager container.
- Removed the --logtostderr argument from the kube-rbac-proxy container.
- Added --tls-min-version and --tls-cipher-suites arguments to the kube-rbac-proxy container.
- Updated the kube-rbac-proxy image to v0.21.0.
config/default/manager_auth_proxy_patch.yaml
- Updated the kube-rbac-proxy image to v0.21.0.
- Removed --logtostderr and --v arguments from the kube-rbac-proxy container.
- Added --tls-min-version and --tls-cipher-suites arguments to the kube-rbac-proxy container.
config/release/kubemacpool.yaml
- Added TLS_MIN_VERSION and TLS_CIPHERS environment variables to the manager container.
- Removed the --logtostderr argument from the kube-rbac-proxy container.
- Added --tls-min-version and --tls-cipher-suites arguments to the kube-rbac-proxy container.
- Updated the kube-rbac-proxy image to v0.21.0.
config/test/kubemacpool.yaml
- Added TLS_MIN_VERSION and TLS_CIPHERS environment variables to the manager container.
- Removed the --logtostderr argument from the kube-rbac-proxy container.
- Added --tls-min-version and --tls-cipher-suites arguments to the kube-rbac-proxy container.
- Updated the kube-rbac-proxy image to v0.21.0.
pkg/webhook/webhook.go
- Modified the tlsMinVersion() call to use the new goTLSVersionToHumanReadable() function.
- Added the goTLSVersionToHumanReadable() function to convert Go TLS version constants to human-readable strings.
- Updated comments for the tlsMinVersion() function.

Activity

No human activity recorded for this pull request yet.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request effectively addresses the need for explicit TLS configuration for kubemacpool's webhook and metrics proxy endpoints, which is a great security enhancement. The changes to introduce configurable TLS versions and cipher suites, along with updating kube-rbac-proxy, are well-implemented. The Go code changes are clean and correct. My main feedback is about the duplication of the long cipher suite strings across multiple YAML files. While the changes are functionally correct, this duplication could pose a maintainability challenge in the future. Please see my specific comments on this.

config/default/manager/manager.yaml

Pass --tls-min-version and --tls-cipher-suites to the kube-rbac-proxy sidecar so the :8443 metrics endpoint enforces a TLS security profile. Default to VersionTLS13 for stronger security. TLS 1.3 cipher suites are not configurable in Go — they are always enabled — so --tls-cipher-suites is omitted. Assisted-by: Claude Upus 4.6 <noreply@anthropic.com> Signed-off-by: Ram Lavi <ralavi@redhat.com>

Currently Kubemacpool webhook allows internally configuring the TLS_MIN_VERSION and TLS_CIPHERS, but the manifests never configured them. This commit explicitly sets TLS_MIN_VERSION and TLS_CIPHERS environment variables on the manager container so the webhook server enforces a TLS security profile. Default to TLS 1.3 for stronger security. TLS 1.3 cipher suites are always enabled in Go and not individually configurable, so TLS_CIPHERS is left empty. Assisted-by: Claude Upus 4.6 <noreply@anthropic.com> Signed-off-by: Ram Lavi <ralavi@redhat.com>

Kube-admissions-webhook [0], which consumes the TLS_MIN_VERSION env var on kubemacpool - accepts TLS_MIN_VERSION from human-readable format (e.g. "1.3"). Align TLS_MIN_VERSION with Go crypto/tls constant names (e.g. "VersionTLS13") and internal wrapper translate foe kube-admissions-webhook using a helper goTLSVersionToHumanReadable. This allows deployers like CNAO to use a single TLS version format for both the webhook and kube-rbac-proxy containers. [0] https://github.com/qinqon/kube-admission-webhook Assisted-by: Claude Upus 4.6 <noreply@anthropic.com> Signed-off-by: Ram Lavi <ralavi@redhat.com>

RamLavi · 2026-03-10T13:33:39Z

Change: Move to TLS 1.3 default

RamLavi · 2026-03-11T12:44:21Z

gemini-code-assist is the code OK now? Can you re-review?

gemini-code-assist · 2026-03-11T13:20:29Z

Thanks for the prompt re-review, RamLavi! I've re-reviewed the code, focusing on the changes made since the last review. The TLS configurations and the updated kube-rbac-proxy version look good. I don't have any further comments at this time.

maiqueb

LGTM. @ormergi please review

kubevirt-bot · 2026-03-12T12:21:50Z

@maiqueb: changing LGTM is restricted to collaborators

Details

In response to this:

LGTM. @ormergi please review

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.

ormergi · 2026-03-12T12:26:19Z

config/default/manager/manager.yaml

        - --upstream=http://127.0.0.1:8080
        - --proxy-endpoints-port=8643
+        - --tls-min-version=VersionTLS13
+        - --tls-cipher-suites=


Please consider omit this flag because it not used.

I checked and it is fine to keep it empty. I think it's not worth it to omit it conditionally..

Why conditionally?
It use TLS 1.3 by default so no ciphers needed, right?

ormergi · 2026-03-12T12:31:14Z

config/default/manager/manager.yaml

+          - name: TLS_CIPHERS
+            value: ""


Please consider ommit this env var because its not used.

same answer

ormergi · 2026-03-12T12:35:12Z

pkg/webhook/webhook.go

 func AddToManager(mgr manager.Manager, poolManager *pool_manager.PoolManager) error {

-	tlsMinVersion, err := kawtls.TLSVersion(tlsMinVersion())
+	tlsMinVersion, err := kawtls.TLSVersion(goTLSVersionToHumanReadable(tlsMinVersion()))


Why do we need this translation if Go constant names are expected to be used by all components in the stack? (CNAO, KAW)

For some reason KAW decided on the human readable format. We can change that and then bump. But I suggest we do it on a follow up PR, you if want

We can drop kawtls.TLSVersion() usage altogether, and have simple translation from Go's TLS version constant name to IDs, for example https://github.com/kubevirt/ipam-extensions/blob/main/pkg/config/tls.go#L25-L35 (or use component-base).

ormergi · 2026-03-12T12:37:38Z

I know its not the PR scope but it would be nice to expose flags for the TLS settings instead of the existing env vars (similar to other projects).

RamLavi · 2026-03-12T14:01:32Z

I know its not the PR scope but it would be nice to expose flags for the TLS settings instead of the existing env vars (similar to other projects).

well it would be nice to use the "--help" functionality but in the end I don't see much merit in it, becuase:

env vars are more easily configured via kustomize.
We already use this for all the other internal params like RANGE, CA_ROTATE interval params.

ormergi · 2026-03-12T14:33:06Z

well it would be nice to use the "--help" functionality but in the end I don't see much merit in it, becuase:
* env vars are more easily configured via kustomize.

Flags are much more easy to work with and allow setting default values, no need for kustomize mambojambo.

* We already use this for all the other internal params like RANGE, CA_ROTATE interval params.

Well then we should revisit what should be considered environmental and is an application argument.

I dont want to make more noise , I will post an issue and we can discuss it there.

RamLavi added 2 commits March 9, 2026 15:56

kubevirt-bot added release-note dco-signoff: yes labels Mar 9, 2026

kubevirt-bot requested review from ormergi and oshoval March 9, 2026 20:49

kubevirt-bot added the size/M label Mar 9, 2026

gemini-code-assist bot reviewed Mar 9, 2026

View reviewed changes

config/default/manager/manager.yaml Outdated Show resolved Hide resolved

config/default/manager/manager.yaml Outdated Show resolved Hide resolved

RamLavi added 3 commits March 10, 2026 12:33

RamLavi force-pushed the kmp_enforce-kube-rbac-tls branch from 38ee7ce to 82c28cc Compare March 10, 2026 10:55

maiqueb approved these changes Mar 12, 2026

View reviewed changes

ormergi reviewed Mar 12, 2026

View reviewed changes

Conversation

RamLavi commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kubevirt-bot commented Mar 9, 2026

Uh oh!

gemini-code-assist bot commented Mar 9, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

RamLavi commented Mar 10, 2026

Uh oh!

RamLavi commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist bot commented Mar 11, 2026

Uh oh!

maiqueb left a comment

Choose a reason for hiding this comment

Uh oh!

kubevirt-bot commented Mar 12, 2026

Uh oh!

ormergi Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

RamLavi Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

ormergi Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

ormergi Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

RamLavi Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

ormergi Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

RamLavi Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ormergi Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

ormergi commented Mar 12, 2026

Uh oh!

RamLavi commented Mar 12, 2026

Uh oh!

ormergi commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

RamLavi commented Mar 9, 2026 •

edited

Loading

RamLavi commented Mar 11, 2026 •

edited

Loading

RamLavi Mar 12, 2026 •

edited

Loading

ormergi commented Mar 12, 2026 •

edited

Loading