✨ Add support for TLS profiles #2246

tmshort · 2025-10-02T21:13:10Z

Use Mozilla's profiles to define TLS profiles for operator-controller and catalogd. These are configured via command-line options, and can be customized.

The idea is that downstream, cluster-olm-operator will be able to glean the appropriate configuration, and provide that to the components.

There is a semi-automatic method to update the profiles, if that ever happens (make update-tls-profiles).

This adds gojq via bingo, which is a golang implementation of jq for the update-tls-profiles target.

Description

Reviewer Checklist

API Go Documentation
Tests: Unit Tests (and E2E Tests, if appropriate)
Comprehensive Commit Messages
Links to related GitHub Issue(s)

netlify · 2025-10-02T21:13:16Z

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	`4d799f1`
🔍 Latest deploy log	https://app.netlify.com/projects/olmv1/deploys/68e3d53c9ce37d00080b702a
😎 Deploy Preview	https://deploy-preview-2246--olmv1.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

codecov · 2025-10-02T23:16:29Z

Codecov Report

❌ Patch coverage is 83.87097% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.69%. Comparing base (5f37a67) to head (4d799f1).
⚠️ Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/shared/util/tlsprofiles/flags.go	83.96%	13 Missing and 4 partials ⚠️
cmd/catalogd/main.go	50.00%	3 Missing and 1 partial ⚠️
cmd/operator-controller/main.go	55.55%	3 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2246      +/-   ##
==========================================
+ Coverage   72.46%   72.69%   +0.23%     
==========================================
  Files          86       89       +3     
  Lines        8592     8747     +155     
==========================================
+ Hits         6226     6359     +133     
- Misses       1954     1970      +16     
- Partials      412      418       +6

Flag	Coverage Δ
e2e	`39.16% <64.51%> (+0.57%)`	⬆️
experimental-e2e	`45.84% <64.51%> (+0.23%)`	⬆️
unit	`57.96% <66.45%> (+0.18%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

grokspawn · 2025-10-03T16:27:40Z

helm/olmv1/templates/deployment-olmv1-system-catalogd-controller-manager.yml

            - --v=${LOG_VERBOSITY}
            - --global-pull-secret=openshift-config/pull-secret
            {{- end }}
+            {{- if not .Values.options.openshift.enabled }}


Don't love a distro-specific flag here. Can we do this in a generic way?

This was really meant to be an e2e flag.

grokspawn · 2025-10-03T16:28:52Z

Is this PR solving an issue or adding a new feature?

joelanford · 2025-10-03T16:35:01Z

helm/olmv1/templates/deployment-olmv1-system-operator-controller-controller-manager.yml

+            {{- if not .Values.options.openshift.enabled }}
+            - --tls-profile=modern
+            {{- end }}


For OCP, I think the expectation is that the TLS settings will always be dynamically configured based on the contents of apiservers.config.openshift.io.Spec.TLSSecurityProfile and the logic in https://github.com/openshift/library-go/blob/f838eb5c601908101db3c84f925c0093b5304611/pkg/operator/configobserver/apiserver/observe_tlssecurityprofile.go#L85-L109

RH probably needs to export that function (or at least use/implement some other helper that calls it) so that they can use OCP's opinions of what the default should be and what the different profiles actually are when they run OLM in OpenShift.

I overlooked the not part of this, and realized that this templating is setting a default for upstream, so all good! But hopefully the info in my previous comment is helpful for the OCP integration bits of this that would come later.

At this point, cluster-olm-operator will watch that API on behalf of op-con and catd, and update the manifests.

Yeah, I'm mainly just saying now that downstream will likely want to follow whatever OCP's opinions are on:

what the default profile is

what the definition of each profile is.

Makefile

joelanford · 2025-10-03T16:47:05Z

internal/shared/util/tlsprofiles/flags.go

+
+func AddFlags(fs *pflag.FlagSet) {
+	fs.Var(&configuredProfile, "tls-profile", "The TLS profile to use. One of "+fmt.Sprintf("%v", slices.Sorted(maps.Keys(profiles))))
+	fs.Var(&customTLSProfile.ciphers, "tls-custom-cipher", "List of ciphers to be used with the custom TLS profile")


Suggested change

fs.Var(&customTLSProfile.ciphers, "tls-custom-cipher", "List of ciphers to be used with the custom TLS profile")

fs.Var(&customTLSProfile.ciphers, "tls-custom-ciphers", "List of ciphers to be used with the custom TLS profile")

joelanford · 2025-10-03T16:48:02Z

internal/shared/util/tlsprofiles/flags.go

+
+func (p *tlsProfileName) Set(value string) error {
+	newValue := tlsProfileName(value)
+	_, err := findTlsProfile(newValue)


Suggested change

_, err := findTlsProfile(newValue)

_, err := findTLSProfile(newValue)

joelanford · 2025-10-03T16:53:07Z

helm/olmv1/templates/deployment-olmv1-system-catalogd-controller-manager.yml

+            - --tls-profile=custom
+            - --tls-custom-version=TLSv1.3
+            - --tls-custom-cipher=TLS_AES_128_GCM_SHA256,TLS_AES_256_GCM_SHA384


To match operator controller's upstream default?

Suggested change

- --tls-profile=custom

- --tls-custom-version=TLSv1.3

- --tls-custom-cipher=TLS_AES_128_GCM_SHA256,TLS_AES_256_GCM_SHA384

- --tls-profile=modern

The custom is basically modern missing one cipher. I really meant for it to be e2e.

tmshort · 2025-10-03T17:39:12Z

Is this PR solving an issue or adding a new feature?

@grokspawn there's a downstream bug to support TLS profiles, this is adding the functionality upstream without having to be "too" openshift aware.

grokspawn · 2025-10-03T18:38:11Z

Is this PR solving an issue or adding a new feature?

@grokspawn there's a downstream bug to support TLS profiles, this is adding the functionality upstream without having to be "too" openshift aware.

Given that this is considered a GA component, I would expect to see something like:

default behavior which aligned with current behavior (unless there is a defect in that behavior);
input options which would allow distros to specify non-default behavior in a non-distro-specific way

tmshort · 2025-10-03T18:45:51Z

Is this PR solving an issue or adding a new feature?

@grokspawn there's a downstream bug to support TLS profiles, this is adding the functionality upstream without having to be "too" openshift aware.

Given that this is considered a GA component, I would expect to see something like:

default behavior which aligned with current behavior (unless there is a defect in that behavior);

input options which would allow distros to specify non-default behavior in a non-distro-specific way

The default behavior is "intermediate", which is a reasonably secure option. Using "old" would be too open, and "modern" might be too limiting. Given that we never specified any of these parameters, we were kinda at the whim of the golang developer's defaults (which could change with every release). "Intermediate" is a reasonable default given that we want to be reasonably secure.
The input options use command-line options to the deployment, so are completely non-distro-specific.

grokspawn · 2025-10-03T18:56:51Z

2. The input options use command-line options to the deployment, so are completely non-distro-specific.

https://github.com/operator-framework/operator-controller/pull/2246/files#diff-38ad0fe7f1443195aef2d54ca6608694fc2306bcbc738a294655dc8ed4408351R56 ?

tmshort · 2025-10-03T20:01:55Z

2. The input options use command-line options to the deployment, so are completely non-distro-specific.
https://github.com/operator-framework/operator-controller/pull/2246/files#diff-38ad0fe7f1443195aef2d54ca6608694fc2306bcbc738a294655dc8ed4408351R56 ?

That is to include the options for the upstream-e2e only, and not on the downstream-e2e; it's for test purposes during the upstream-e2e, and to avoid erroneously adding it to OpenShift. Nothing precludes adding those values during helm template via --set or --values.

grokspawn · 2025-10-05T16:38:53Z

That is to include the options for the upstream-e2e only, and not on the downstream-e2e; it's for test purposes during the upstream-e2e, and to avoid erroneously adding it to OpenShift. Nothing precludes adding those values during helm template via --set or --values.

IMHO, the upstream should not have to care about any distro. Responsibility for using it properly in a distro lies with that distro.
Ideally, this is because the mechanism is general and can be customized as desired by users.

tmshort · 2025-10-06T13:35:01Z

IMHO, the upstream should not have to care about any distro. Responsibility for using it properly in a distro lies with that distro.

We already have Openshift as an option in the helm charts for downstream even before this PR. I can remove the check from this PR, but it won't remove the fact that there's an Openshift option in the chart.

Ideally, this is because the mechanism is general and can be customized as desired by users.

Again, this is possible via --set and/or --values provided by the user/distro when they generate the manifests.

Use Mozilla's profiles to define TLS profiles for operator-controller and catalogd. These are configured via command-line options, and can be customized. The idea is that downstream, cluster-olm-operator will be able to glean the appropriate configuration, and provide that to the components. There is a semi-automatic method to update the profiles, if that ever happens (`make update-tls-profiles`). This adds `gojq` via bingo, which is a golang implementation of jq for the update-tls-profiles target. Signed-off-by: Todd Short <tshort@redhat.com>

tmshort · 2025-10-06T14:42:30Z

Removed the openshift check.

joelanford · 2025-10-06T19:52:18Z

/approve

openshift-ci · 2025-10-06T19:52:36Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: grokspawn, joelanford

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

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

tmshort requested a review from a team as a code owner October 2, 2025 21:13

openshift-ci bot requested review from bentito and grokspawn October 2, 2025 21:13

tmshort force-pushed the tls-profiles branch from ce2557d to e63a5d9 Compare October 2, 2025 23:04

tmshort force-pushed the tls-profiles branch 3 times, most recently from ca6bb26 to 8692861 Compare October 3, 2025 16:04

grokspawn reviewed Oct 3, 2025

View reviewed changes

joelanford reviewed Oct 3, 2025

View reviewed changes

Makefile Show resolved Hide resolved

joelanford reviewed Oct 3, 2025

View reviewed changes

tmshort force-pushed the tls-profiles branch from 8692861 to 0792a91 Compare October 3, 2025 18:28

tmshort force-pushed the tls-profiles branch from 0792a91 to 32674c7 Compare October 3, 2025 18:50

tmshort force-pushed the tls-profiles branch from 32674c7 to 4d799f1 Compare October 6, 2025 14:42

grokspawn approved these changes Oct 6, 2025

View reviewed changes

openshift-ci bot assigned grokspawn Oct 6, 2025

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 6, 2025

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 6, 2025

openshift-merge-bot bot merged commit 029484d into operator-framework:main Oct 6, 2025
33 of 35 checks passed

tmshort deleted the tls-profiles branch October 8, 2025 14:06

	fs.Var(&customTLSProfile.ciphers, "tls-custom-cipher", "List of ciphers to be used with the custom TLS profile")
	fs.Var(&customTLSProfile.ciphers, "tls-custom-ciphers", "List of ciphers to be used with the custom TLS profile")

	_, err := findTlsProfile(newValue)
	_, err := findTLSProfile(newValue)

✨ Add support for TLS profiles #2246

✨ Add support for TLS profiles #2246

Uh oh!

Conversation

tmshort commented Oct 2, 2025

Description

Reviewer Checklist

Uh oh!

netlify bot commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for olmv1 ready!

Uh oh!

codecov bot commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

grokspawn commented Oct 3, 2025

Uh oh!

joelanford Oct 3, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tmshort commented Oct 3, 2025

Uh oh!

grokspawn commented Oct 3, 2025

Uh oh!

tmshort commented Oct 3, 2025

Uh oh!

grokspawn commented Oct 3, 2025

Uh oh!

tmshort commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

grokspawn commented Oct 5, 2025

Uh oh!

tmshort commented Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tmshort commented Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joelanford commented Oct 6, 2025

Uh oh!

openshift-ci bot commented Oct 6, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

netlify bot commented Oct 2, 2025 •

edited

Loading

codecov bot commented Oct 2, 2025 •

edited

Loading

joelanford Oct 3, 2025 •

edited

Loading

tmshort commented Oct 3, 2025 •

edited

Loading

tmshort commented Oct 6, 2025 •

edited

Loading

tmshort commented Oct 6, 2025 •

edited

Loading