OCPBUGS-62238: configure tls profile for router metrics by jcmoraisjr · Pull Request #1378 · openshift/cluster-ingress-operator

jcmoraisjr · 2026-03-06T17:08:21Z

Router has two new envvars to configure TLS minimal version and allowed ciphers. This update reads the configuration from the apiserver TLS profile and configures those envvars accordingly.

TLS profile has cipher names in OpenSSL format, while internally library-go uses IANA names. The name is converted before sending to router, which is an internal component.

https://issues.redhat.com/browse/OCPBUGS-62238

openshift-ci-robot · 2026-03-06T17:08:28Z

@jcmoraisjr: This pull request references Jira Issue OCPBUGS-62238, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.22.0) matches configured target version for branch (4.22.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @ShudiLi

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Router has two new envvars to configure TLS minimal version and allowed ciphers. This update reads the configuration from the apiserver TLS profile and configures those envvars accordingly.

TLS profile has cipher names in OpenSSL format, while internally library-go uses IANA names. The name is converted before sending to router, which is an internal component.

https://issues.redhat.com/browse/OCPBUGS-62238

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 openshift-eng/jira-lifecycle-plugin repository.

coderabbitai · 2026-03-06T17:08:40Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: e576e6e0-f0f2-4500-a259-e3a95a72fdf6

📥 Commits

Reviewing files that changed from the base of the PR and between 8d9a3fd and 9ddddd1.

📒 Files selected for processing (1)

pkg/operator/controller/ingress/deployment.go

🚧 Files skipped from review as they are similar to previous changes (1)

pkg/operator/controller/ingress/deployment.go

📝 Walkthrough

Walkthrough

This change updates the ingress deployment controller to add TLS metrics configuration. It computes OpenSSL-to-IANA cipher suite names via crypto.OpenSSLToIANACipherSuites, imports crypto from library-go, and exposes two new environment variables—ROUTER_METRICS_TLS_CIPHERS and ROUTER_METRICS_TLS_MIN_VERSION—derived from the TLS profile metrics. These metrics are added alongside existing TLS settings in the router deployment specification. The go.mod file was also modified.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately reflects the main change: configuring TLS profile metrics for the router component, which matches the primary objective of the pull request.
Description check	✅ Passed	The description is clearly related to the changeset, explaining how the PR reads the apiserver TLS profile and configures router environment variables, including the cipher name conversion from OpenSSL to IANA format.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Stable And Deterministic Test Names	✅ Passed	Pull request adds new test file with standard Go testing conventions; no Ginkgo tests with dynamic names.
Test Structure And Quality	✅ Passed	The custom check for Ginkgo test structure is not applicable to this PR as no test files were modified.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📝 Coding Plan

Generate coding plan for human review comments

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

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

jcmoraisjr · 2026-03-06T20:29:57Z

/retest

ShudiLi · 2026-03-10T03:09:54Z

tested it with 4.21.0-0-2026-03-10-115701-test-ci-ln-mwiic1k-latest, two new envs ROUTER_METRICS_TLS_CIPHERS and ROUTER_METRICS_TLS_MIN_VERSION were added

1.
% oc get clusterversion
NAME      VERSION                                                AVAILABLE   PROGRESSING   SINCE   STATUS
version   4.21.0-0-2026-03-10-115701-test-ci-ln-mwiic1k-latest   True        False         46m     Cluster version is 4.21.0-0-2026-03-10-115701-test-ci-ln-mwiic1k-latest

2. Configure Modern tlsSecurityProfile for the apiserver and the ingresscontroller
% oc -n openshift-ingress-operator get ingresscontroller default -ojsonpath="{.spec.tlsSecurityProfile}"
{"type":"Modern"}%                                                                                                                                                                                                                                                         % oc get apiserver cluster -ojsonpath="{.spec.tlsSecurityProfile}"                                      
{"modern":{},"type":"Modern"}% 

3.
% oc -n openshift-ingress get deployment router-default -oyaml | grep -A1 -E "ROUTER_METRICS_TLS_CIPHERS|ROUTER_METRICS_TLS_MIN_VERSION"
        - name: ROUTER_METRICS_TLS_CIPHERS
          value: TLS_AES_128_GCM_SHA256:TLS_AES_256_GCM_SHA384:TLS_CHACHA20_POLY1305_SHA256
--
        - name: ROUTER_METRICS_TLS_MIN_VERSION
          value: VersionTLS13


4. 
% oc -n openshift-ingress rsh router-default-54f6f49c64-6rkx8
sh-5.1$ env | grep -i tls
ROUTER_METRICS_TLS_CERT_FILE=/etc/pki/tls/metrics-certs/tls.crt
ROUTER_METRICS_TLS_CIPHERS=TLS_AES_128_GCM_SHA256:TLS_AES_256_GCM_SHA384:TLS_CHACHA20_POLY1305_SHA256
ROUTER_METRICS_TLS_MIN_VERSION=VersionTLS13
SSL_MIN_VERSION=TLSv1.3
ROUTER_CIPHERSUITES=TLS_AES_128_GCM_SHA256:TLS_AES_256_GCM_SHA384:TLS_CHACHA20_POLY1305_SHA256
ROUTER_METRICS_TLS_KEY_FILE=/etc/pki/tls/metrics-certs/tls.key
DEFAULT_CERTIFICATE_DIR=/etc/pki/tls/private
sh-5.1$

5.
% nmap --script ssl-enum-ciphers -p 1936 127.0.0.1
Starting Nmap 7.98 ( https://nmap.org ) at 2026-03-10 21:30 +0800
Nmap scan report for localhost (127.0.0.1)
Host is up (0.000066s latency).

PORT     STATE SERVICE
1936/tcp open  jetcmeserver
| ssl-enum-ciphers: 
|   TLSv1.3: 
|     ciphers: 
|       TLS_AKE_WITH_AES_128_GCM_SHA256 (X25519MLKEM768) - A
|       TLS_AKE_WITH_AES_256_GCM_SHA384 (X25519MLKEM768) - A
|       TLS_AKE_WITH_CHACHA20_POLY1305_SHA256 (X25519MLKEM768) - A
|     cipher preference: server
|_  least strength: A

Nmap done: 1 IP address (1 host up) scanned in 12.15 seconds

davidesalerno · 2026-03-10T15:40:20Z

/assign

davidesalerno

A couple of comments

davidesalerno · 2026-03-10T16:47:55Z

pkg/operator/controller/ingress/deployment.go

+	// User facing config uses OpenSSL names. Internally we always use IANA ones.
+	// OpenSSLToIANACipherSuites() converts and also removes any invalid cipher, otherwise router would crash.
+	ianaNames := crypto.OpenSSLToIANACipherSuites(tlsProfileMetrics.Ciphers)
+	env = append(env, corev1.EnvVar{Name: "ROUTER_METRICS_TLS_CIPHERS", Value: strings.Join(ianaNames, ":")})


If tlsProfileMetrics.Ciphers is empty OR conversion drops everything, strings.Join(ianaNames, ":") becomes "".

Isn't it better to set ROUTER_METRICS_TLS_CIPHERS only when len(ianaNames) > 0

Just realized that this is already handled by router: in case envvar is missing or empty, it defaults to the intermediate profile. So assigning empty is fine. Moreover apiserver api already validates if the user input has at least one valid tls1.0-1.2 cipher name, so it ensures that this will never be empty.

davidesalerno · 2026-03-10T16:51:10Z

pkg/operator/controller/ingress/deployment.go

 	}
 	env = append(env, corev1.EnvVar{Name: "SSL_MIN_VERSION", Value: minTLSVersion})

+	tlsProfileMetrics := tlsProfileSpecForSecurityProfile(apiConfig.Spec.TLSSecurityProfile)


I could be useful to add a small guard/comment or unit test to cover the case when TLSSecurityProfile is nil.

This is already implemented inside tlsProfileSpecForSecurityProfile(): in case the profile is nil, it uses the intermediate profile.

bentito · 2026-03-13T14:34:43Z

/lgtm

bentito · 2026-03-13T14:34:50Z

/approve

openshift-ci · 2026-03-13T14:35:42Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bentito

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [bentito]

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

bentito · 2026-03-13T15:00:33Z

@ShudiLi can you re-verify please?

davidesalerno · 2026-03-13T16:45:52Z

/lgtm

jcmoraisjr · 2026-03-13T19:10:47Z

/hold

I'll add another validation on the incoming ciphers, on top of the apiserver api ones, to ensure we send only a valid list to the router.

Router has two new envvars to configure TLS minimal version and allowed ciphers. This update reads the configuration from the apiserver TLS profile and configures those envvars accordingly. TLS profile has cipher names in OpenSSL format, while internally library-go uses IANA names. The name is converted before sending to router, which is an internal component. https://issues.redhat.com/browse/OCPBUGS-62238

davidesalerno · 2026-03-16T14:08:07Z

/lgtm

candita · 2026-03-16T15:27:46Z

/tide refresh

candita · 2026-03-16T15:31:13Z

/test unit

lihongan · 2026-03-17T01:49:41Z

/retest-required

ShudiLi · 2026-03-17T04:41:06Z

tested it with 4.22.0-0-2026-03-17-024743-test-ci-ln-yy3r7fk-latest

1.
% oc get clusterversion
NAME      VERSION                                                AVAILABLE   PROGRESSING   SINCE   STATUS
version   4.22.0-0-2026-03-17-024743-test-ci-ln-yy3r7fk-latest   True        False         64m     Cluster version is 4.22.0-0-2026-03-17-024743-test-ci-ln-yy3r7fk-latest

2.
% oc get apiserver cluster -ojsonpath="{.spec.tlsSecurityProfile}"
{"modern":{},"type":"Modern"}%                                                                                                                                                                                                                                        % oc -n openshift-ingress-operator get ingresscontroller default -ojsonpath="{.spec.tlsSecurityProfile}"
{"type":"Modern"}%          

3.
% oc -n openshift-ingress get deployment router-default -oyaml | grep -A1 -E "ROUTER_METRICS_TLS_CIPHERS|ROUTER_METRICS_TLS_MIN_VERSION"
        - name: ROUTER_METRICS_TLS_CIPHERS
          value: TLS_AES_128_GCM_SHA256:TLS_AES_256_GCM_SHA384:TLS_CHACHA20_POLY1305_SHA256
--
        - name: ROUTER_METRICS_TLS_MIN_VERSION
          value: VersionTLS13

4.
% nmap --script ssl-enum-ciphers -p 1936 127.0.0.1
Starting Nmap 7.98 ( https://nmap.org ) at 2026-03-17 12:38 +0800
Nmap scan report for localhost (127.0.0.1)
Host is up (0.000065s latency).

PORT     STATE SERVICE
1936/tcp open  jetcmeserver
| ssl-enum-ciphers: 
|   TLSv1.3: 
|     ciphers: 
|       TLS_AKE_WITH_AES_128_GCM_SHA256 (X25519MLKEM768) - A
|       TLS_AKE_WITH_AES_256_GCM_SHA384 (X25519MLKEM768) - A
|       TLS_AKE_WITH_CHACHA20_POLY1305_SHA256 (X25519MLKEM768) - A
|     cipher preference: server
|_  least strength: A

Nmap done: 1 IP address (1 host up) scanned in 11.75 seconds

ShudiLi · 2026-03-17T04:41:30Z

/verified by @ShudiLi

openshift-ci-robot · 2026-03-17T04:41:41Z

@ShudiLi: This PR has been marked as verified by @ShudiLi.

Details

In response to this:

/verified by @ShudiLi

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 openshift-eng/jira-lifecycle-plugin repository.

bentito · 2026-03-17T15:23:26Z

/test hypershift-e2e-aks

jcmoraisjr · 2026-03-18T11:25:03Z

/unhold

jcmoraisjr · 2026-03-18T15:06:28Z

/retest

bentito · 2026-03-18T22:48:48Z

/retest

jcmoraisjr · 2026-03-19T12:45:27Z

/retest

jcmoraisjr · 2026-03-19T20:20:48Z

/test e2e-aws-ovn-hypershift-conformance

jcmoraisjr · 2026-03-23T14:30:14Z

/override ci/prow/e2e-aws-ovn-hypershift-conformance

openshift-ci · 2026-03-23T14:30:44Z

@jcmoraisjr: Overrode contexts on behalf of jcmoraisjr: ci/prow/e2e-aws-ovn-hypershift-conformance

Details

In response to this:

/override ci/prow/e2e-aws-ovn-hypershift-conformance

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.

openshift-ci · 2026-03-23T14:31:46Z

@jcmoraisjr: all tests passed!

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.

openshift-ci bot requested a review from ShudiLi March 6, 2026 17:08

openshift-ci bot requested review from alebedev87 and rfredette March 6, 2026 17:11

openshift-ci bot assigned davidesalerno Mar 10, 2026

davidesalerno reviewed Mar 10, 2026

View reviewed changes

openshift-ci bot assigned bentito Mar 13, 2026

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 13, 2026

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 13, 2026

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 13, 2026

jcmoraisjr force-pushed the OCPBUGS-62238-router-tls-metrics branch from 8d9a3fd to 9ddddd1 Compare March 13, 2026 19:52

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 13, 2026

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 16, 2026

openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Mar 17, 2026

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 18, 2026

openshift-ci-robot mentioned this pull request Mar 18, 2026

OCPBUGS-62238: configure TLS profile for metrics endpoint openshift/router#748

Merged

Conversation

jcmoraisjr commented Mar 6, 2026

Uh oh!

openshift-ci-robot commented Mar 6, 2026

Uh oh!

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

Uh oh!

Walkthrough

Uh oh!

jcmoraisjr commented Mar 6, 2026

Uh oh!

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

Uh oh!

Uh oh!

davidesalerno commented Mar 10, 2026

Uh oh!

davidesalerno left a comment

Choose a reason for hiding this comment

Uh oh!

davidesalerno Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

jcmoraisjr Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

davidesalerno Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

jcmoraisjr Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

bentito commented Mar 13, 2026

Uh oh!

bentito commented Mar 13, 2026

Uh oh!

openshift-ci bot commented Mar 13, 2026

Uh oh!

bentito commented Mar 13, 2026

Uh oh!

davidesalerno commented Mar 13, 2026

Uh oh!

jcmoraisjr commented Mar 13, 2026

Uh oh!

davidesalerno commented Mar 16, 2026

Uh oh!

candita commented Mar 16, 2026

Uh oh!

candita commented Mar 16, 2026

Uh oh!

lihongan commented Mar 17, 2026

Uh oh!

ShudiLi commented Mar 17, 2026

Uh oh!

ShudiLi commented Mar 17, 2026

Uh oh!

openshift-ci-robot commented Mar 17, 2026

Uh oh!

bentito commented Mar 17, 2026

Uh oh!

jcmoraisjr commented Mar 18, 2026

Uh oh!

jcmoraisjr commented Mar 18, 2026

Uh oh!

bentito commented Mar 18, 2026

Uh oh!

jcmoraisjr commented Mar 19, 2026

Uh oh!

jcmoraisjr commented Mar 19, 2026

Uh oh!

jcmoraisjr commented Mar 23, 2026

Uh oh!

openshift-ci bot commented Mar 23, 2026

Uh oh!

openshift-ci bot commented Mar 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

coderabbitai bot commented Mar 6, 2026 •

edited

Loading

ShudiLi commented Mar 10, 2026 •

edited

Loading