TRACING-6116: Add TLS min version and cipher suite configuration support by ozzywalsh · Pull Request #248 · openshift/distributed-tracing-console-plugin

ozzywalsh · 2026-03-25T14:19:18Z

Add support for passing tls min version & cipher suites to the backend.
See similar PR in otel-operator: open-telemetry/opentelemetry-operator#4669
Preparatory work for pulling in the cluster tls security profile in rhobs/observability-operator

Add --tls-min-version and --tls-cipher-suites flags with env var fallback (TLS_MIN_VERSION, TLS_CIPHER_SUITES).
Defaults to TLS 1.2 when no min version is specified.

Add --tls-min-version and --tls-cipher-suites flags with env var fallback (TLS_MIN_VERSION, TLS_CIPHER_SUITES). Uses k8s component-base for TLS version and cipher suite validation, matching the approach in the opentelemetry-operator. Defaults to TLS 1.2 when no min version is specified.

openshift-ci · 2026-03-25T14:20:03Z

Hi @ozzywalsh. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

andreasgerstmayr

Looks good overall! 💯

For simplicity, I would remove the whitespace trimming and the custom cipherSuitesSlice type - the flag or env var will be set by the operator, which is expected to provide a valid value

cc @jgbernalp the other UI plugins will need the same, we should keep the naming of the flags/env vars identical across UI plugins

zhuje · 2026-03-25T15:11:34Z

cc @jgbernalp the other UI plugins will need the same, we should keep the naming of the flags/env vars identical across UI plugins

@andreasgerstmayr Good idea, I'll add comments to the JIRAs to mention this. Also do we also need a tlsMaxVersionArg for the distributed-plugin? (Reference to monitoring-plugin flags)

Simplify TLS cipher suites flag handling by using a plain string flag instead of a custom flag.Value type. The operator is expected to provide valid values, so whitespace trimming is unnecessary.

ozzywalsh · 2026-03-25T17:30:47Z

Looks good overall! 💯

For simplicity, I would remove the whitespace trimming and the custom cipherSuitesSlice type - the flag or env var will be set by the operator, which is expected to provide a valid value

cc @jgbernalp the other UI plugins will need the same, we should keep the naming of the flags/env vars identical across UI plugins

Ok. I removed the whitespace traimming & custom type.

ozzywalsh · 2026-03-25T20:57:13Z

cc @jgbernalp the other UI plugins will need the same, we should keep the naming of the flags/env vars identical across UI plugins

@andreasgerstmayr Good idea, I'll add comments to the JIRAs to mention this. Also do we also need a tlsMaxVersionArg for the distributed-plugin? (Reference to monitoring-plugin flags)

@zhuje the OpenShift TLS Security Profiles don't specify a max TLS version. So for the purpose of supporting the cluster wide tls profiles; that flag is not needed.

https://docs.redhat.com/en/documentation/openshift_container_platform/4.21/html/security_and_compliance/tls-security-profiles#tls-profiles-kubernetes-configuring_tls-security-profiles

andreasgerstmayr · 2026-03-26T10:47:56Z

/lgtm

ozzywalsh · 2026-03-26T13:26:26Z

I have also opened a PR for the observability-operator to pull in the cluster tls profile & pass the args into the console plugin container (--tls-min-version *& --tls-cipher-suites; which this PR adds).

Note

The console plugin should be released first , then the observability-operator. Otherwise the operator will pass in args that plugin is not expecting; and throw an error.

At least that's how my understanding. Perhaps someone can chime in on how that is handled.

rhobs/observability-operator#1041

andreasgerstmayr · 2026-03-26T15:11:32Z

I just realized this only applies the TLS config to the API server, could you also configure the TLS config of the proxy in proxy.go?

The UI plugin proxies /proxy/<namespace>/<tempo_instance>/<tenant> to the Tempo instance (e.g. https://tempo-<tempo_instance>-gateway.<namespace>.svc:8080/api/traces/v1/<tenant>/tempo). This is required because of the Same-Origin Policy (SOP) of browsers.

ozzywalsh · 2026-03-30T12:33:56Z

I just realized this only applies the TLS config to the API server, could you also configure the TLS config of the proxy in proxy.go?

The UI plugin proxies /proxy/<namespace>/<tempo_instance>/<tenant> to the Tempo instance (e.g. https://tempo-<tempo_instance>-gateway.<namespace>.svc:8080/api/traces/v1/<tenant>/tempo). This is required because of the Same-Origin Policy (SOP) of browsers.

@andreasgerstmayr ok, i'll take a look & update the PR.

ozzywalsh · 2026-03-30T16:28:12Z

@andreasgerstmayr I have updated this PR to also include the proxy. Let me know what you think.

andreasgerstmayr · 2026-04-03T11:55:57Z

-	r.PathPrefix("/proxy/{namespace}/{name}/{tenant}").Handler(proxy.NewProxyHandler(k8sclient, cfg.CertFile))
+	var proxyTLSMinVersion uint16
+	if cfg.TLSMinVersion != "" {
+		proxyTLSMinVersion, _ = k8sapiflag.TLSVersion(cfg.TLSMinVersion)


Let's add error handling here instead of silently dismissing the error and not using the configured TLS min version.

Technically the

logrus.WithError(err).Fatal("invalid TLS min version")

which is called a few lines after setupRoutes() will exit the process in case of error, but I suggest to be overcautious here, in case for example the code gets refactored in the future.

Ok; I've changed it throw an error with logrus; as is done elsewhere if this function fails. I also did the same where we convert the ciphers to golang format.

andreasgerstmayr · 2026-04-03T12:06:24Z

-		}
+func (h *ProxyHandler) buildTLSConfig() (*tls.Config, error) {
+	if h.serviceCAfile == "" {
+		return nil, nil


Because of this early exit, the TLS min version and cipher suite won't be set if serviceCAfile is empty. imho it still makes sense to configure these settings, as the CA is just an optional field in the TLS configuration.

Ok, fixed. We configure the min_version & cipher suite even if no cert is pased now.

jgbernalp · 2026-04-07T07:22:41Z

/ok-to-test

Previously errors from k8sapiflag.TLSVersion and TLSCipherSuites were silently discarded when configuring the proxy TLS settings. This adds fatal error logging consistent with how the server TLS config handles parse failures.

ozzywalsh · 2026-04-08T14:21:42Z

+	if len(cfg.TLSCipherSuites) > 0 {
+		cipherSuiteIDs, err := k8sapiflag.TLSCipherSuites(cfg.TLSCipherSuites)
+		if err != nil {
+			logrus.WithError(err).Fatal("invalid TLS cipher suites")


Let's add error handling here instead of silently dismissing the error and not using the configured TLS min version.

Technically the

logrus.WithError(err).Fatal("invalid TLS min version")

which is called a few lines after setupRoutes() will exit the process in case of error, but I suggest to be overcautious here, in case for example the code gets refactored in the future.

I made the same change here also.

openshift-ci · 2026-04-08T15:45:54Z

@ozzywalsh: 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.

andreasgerstmayr · 2026-04-13T13:16:55Z

Thanks for the changes!

/lgtm

openshift-ci · 2026-04-13T13:17:16Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andreasgerstmayr

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~~ [andreasgerstmayr]

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

andreasgerstmayr · 2026-04-13T13:20:44Z

Fixes: https://redhat.atlassian.net/browse/TRACING-6116

openshift-ci-robot · 2026-04-13T14:06:58Z

@ozzywalsh: This pull request references TRACING-6116 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the sub-task to target the "4.22.0" version, but no target version was set.

Details

In response to this:

Add support for passing tls min version & cipher suites to the backend.
See similar PR in otel-operator: open-telemetry/opentelemetry-operator#4669
Preparatory work for pulling in the cluster tls security profile in rhobs/observability-operator

Add --tls-min-version and --tls-cipher-suites flags with env var fallback (TLS_MIN_VERSION, TLS_CIPHER_SUITES).

Defaults to TLS 1.2 when no min version is specified.

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.

IshwarKanse · 2026-04-14T08:15:11Z

Will test this PR once I finish testing rhobs/observability-operator#1041

IshwarKanse · 2026-04-15T10:57:33Z

/label qe-approved

openshift-ci-robot · 2026-04-15T10:57:40Z

@ozzywalsh: This pull request references TRACING-6116 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the sub-task to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Add support for passing tls min version & cipher suites to the backend.
See similar PR in otel-operator: open-telemetry/opentelemetry-operator#4669
Preparatory work for pulling in the cluster tls security profile in rhobs/observability-operator

Add --tls-min-version and --tls-cipher-suites flags with env var fallback (TLS_MIN_VERSION, TLS_CIPHER_SUITES).

Defaults to TLS 1.2 when no min version is specified.

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.

IshwarKanse · 2026-04-15T10:58:54Z

@ozzywalsh @andreasgerstmayr Finished testing, added test cases with PR #249

We can go ahead and merge the PR.

zhuje · 2026-05-04T14:08:23Z

@ozzywalsh @andreasgerstmayr

Could you backport this to the COO release branches?
release-coo-ocp-4.19
release-coo-ocp-4.15
release-coo-ocp-4.12

andreasgerstmayr · 2026-05-05T17:59:39Z

/cherry-pick release-coo-ocp-4.19 release-coo-ocp-4.15 release-coo-ocp-4.12

openshift-cherrypick-robot · 2026-05-05T18:00:23Z

@andreasgerstmayr: new pull request created: #258

Details

In response to this:

/cherry-pick release-coo-ocp-4.19 release-coo-ocp-4.15 release-coo-ocp-4.12

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 Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 25, 2026

ozzywalsh marked this pull request as ready for review March 25, 2026 14:19

openshift-ci Bot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Mar 25, 2026

openshift-ci Bot requested review from IshwarKanse and zhuje March 25, 2026 14:20

andreasgerstmayr reviewed Mar 25, 2026

View reviewed changes

Remove custom cipherSuitesSlice type and whitespace trimming

ff91130

Simplify TLS cipher suites flag handling by using a plain string flag instead of a custom flag.Value type. The operator is expected to provide valid values, so whitespace trimming is unnecessary.

openshift-ci Bot assigned andreasgerstmayr Mar 26, 2026

openshift-ci Bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Mar 26, 2026

Add TLS min version and cipher suite configuration for proxy

b0adc4c

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

andreasgerstmayr reviewed Apr 3, 2026

View reviewed changes

openshift-ci Bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 7, 2026

ozzywalsh added 2 commits April 8, 2026 14:50

Apply TLS config to proxy even when no CA file is provided

99b8e2e

ozzywalsh commented Apr 8, 2026

View reviewed changes

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

ozzywalsh changed the title ~~Add TLS min version and cipher suite configuration support~~ TRACING-6116: Add TLS min version and cipher suite configuration support Apr 13, 2026

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Apr 13, 2026

openshift-ci Bot added the qe-approved Signifies that QE has signed off on this PR label Apr 15, 2026

openshift-merge-bot Bot merged commit 5850ddd into openshift:main Apr 15, 2026
5 checks passed

openshift-cherrypick-robot mentioned this pull request May 5, 2026

[release-coo-ocp-4.19] TRACING-6116: Add TLS min version and cipher suite configuration support #258

Merged

Conversation

ozzywalsh commented Mar 25, 2026

Uh oh!

openshift-ci Bot commented Mar 25, 2026

Uh oh!

andreasgerstmayr left a comment

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

ozzywalsh commented Mar 25, 2026

Uh oh!

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

Uh oh!

Uh oh!

andreasgerstmayr commented Mar 26, 2026

Uh oh!

ozzywalsh commented Mar 26, 2026

Note

Uh oh!

andreasgerstmayr commented Mar 26, 2026

Uh oh!

ozzywalsh commented Mar 30, 2026

Uh oh!

ozzywalsh commented Mar 30, 2026

Uh oh!

andreasgerstmayr Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

ozzywalsh Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

andreasgerstmayr Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

ozzywalsh Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

jgbernalp commented Apr 7, 2026

Uh oh!

ozzywalsh Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

openshift-ci Bot commented Apr 8, 2026

Uh oh!

andreasgerstmayr commented Apr 13, 2026

Uh oh!

openshift-ci Bot commented Apr 13, 2026

Uh oh!

andreasgerstmayr commented Apr 13, 2026

Uh oh!

openshift-ci-robot commented Apr 13, 2026 • edited by openshift-ci Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IshwarKanse commented Apr 14, 2026

Uh oh!

IshwarKanse commented Apr 15, 2026

Uh oh!

openshift-ci-robot commented Apr 15, 2026 • edited by openshift-ci Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IshwarKanse commented Apr 15, 2026

Uh oh!

Uh oh!

zhuje commented May 4, 2026

Uh oh!

andreasgerstmayr commented May 5, 2026

Uh oh!

openshift-cherrypick-robot commented May 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

zhuje commented Mar 25, 2026 •

edited

Loading

ozzywalsh commented Mar 25, 2026 •

edited

Loading

openshift-ci-robot commented Apr 13, 2026 •

edited by openshift-ci Bot

Loading

openshift-ci-robot commented Apr 15, 2026 •

edited by openshift-ci Bot

Loading