OPRUN-4411: Add APIServer TLS controller for OpenShift cluster-wide TLS configuration #3739

tmshort · 2026-01-19T19:26:43Z

Implements a controller that watches the OpenShift APIServer resource and provides thread-safe access to TLS configuration for HTTPS servers/clients.

The controller is modeled after the existing proxy controller.

This is intended to be used by the OLM operator, Catalog operator and marketplace operator (but it will need to be copied). It will also be used by the downstream PSM component.

Assisted-By: Claude

bandrade · 2026-01-20T19:42:27Z

/label qe-approved
/verified by @bandrade

camilamacedo86 · 2026-01-21T17:41:03Z

@tmshort we need to rebase

camilamacedo86 · 2026-01-21T17:44:01Z

pkg/lib/apiserver/querier_test.go

+	require.NoError(t, err)
+
+	// Verify secure defaults
+	assert.GreaterOrEqual(t, config.MinVersion, uint16(tls.VersionTLS12), "Should use at least TLS 1.2")


Have we not a bug in some specific version / CVE that made we downgrade the version?
See: https://github.com/kubernetes-sigs/kubebuilder/blob/master/testdata/project-v4/cmd/main.go#L96-L105

Isn't it still valid?

HTTP version is different than TLS version.

What you're talking about is HTTP version, and the config is already done in server.go (lines not modified in this PR).

camilamacedo86 · 2026-01-21T17:44:49Z

pkg/lib/apiserver/syncer.go

+	case tls.VersionTLS13:
+		return "TLS 1.3"
+	default:
+		return "unknown"


I see we fail if not one of those right?

Correct; they specified an unknown TLS version, so we wouldn't know what to do.

camilamacedo86 · 2026-01-21T17:45:58Z

Hi @tmshort

We need to rebase this one.
But overall, I am good with this one.

/lgtm

It would be nice for this one to get a second reviewer as well, maybe @perdasilva

pkg/lib/apiserver/available.go

pkg/lib/apiserver/syncer.go

tmshort · 2026-01-21T20:38:55Z

There was some duplicate code; since @anik120's review requested changes in that area, I consolidated it.

HTTPS_SERVERS.md

WATCHED_RESOURCES.md

HTTPS_SERVERS.md

APISERVER_TLS_CONTROLLER.md

camilamacedo86 · 2026-01-22T05:54:11Z

@tmshort

After the changes, many MD files generated by CLAUDE are here.
IM,O they should not get merged.
We should have only a small README with the info consolidated and without emojis.

Can we change that?

anik120 · 2026-01-21T21:18:28Z

pkg/lib/apiserver/querier.go

+)
+
+// NoopQuerier returns an instance of noopQuerier. It's used for upstream where
+// we don't have any cluster APIServer configuration.


nit:

Suggested change

// we don't have any cluster APIServer configuration.

// we don't have any apiserver.config.openshift.io/cluster resource.

anik120 · 2026-01-21T21:19:02Z

pkg/lib/apiserver/syncer.go

+)
+
+const (
+	// This is the cluster level global APIServer object name.


nit:

Suggested change

// This is the cluster level global APIServer object name.

// This is the cluster level global apiserver.config.openshift.io/cluster object name.

anik120 · 2026-01-21T21:19:22Z

pkg/lib/apiserver/syncer.go

+	defaultSyncInterval = 30 * time.Minute
+)
+
+// NewSyncer returns informer and sync functions to enable watch of APIServer type.


Suggested change

// NewSyncer returns informer and sync functions to enable watch of APIServer type.

// NewSyncer returns informer and sync functions to enable watch of apiserver.config.openshift.io/cluster CRs.

anik120 · 2026-01-22T13:34:29Z

HTTPS_SERVERS.md

@@ -0,0 +1,387 @@
+# HTTPS Servers in OLM Codebase


These docs are definitely helpful, I like them.

But root is not the right place for them right? docs/design is a more suitable place for them no?

They weren't supposed to be committed, they were for code investigation purposes.

…tion Implements a controller that watches the OpenShift APIServer resource and provides thread-safe access to TLS configuration for HTTPS servers/clients. The controller is modeled after the existing proxy controller. This is intended to be used by the OLM operator, Catalog operator and marketplace operator (but it will need to be copied). It will also be used by the downstream PSM component. Signed-off-by: Todd Short <todd.short@me.com> Assisted-By: Claude

tmshort · 2026-01-22T14:57:47Z

@camilamacedo86

After the changes, many MD files generated by CLAUDE are here. IM,O they should not get merged. We should have only a small README with the info consolidated and without emojis.

Can we change that?

Those files were not supposed to have been included in the PR. They were part of my code investigation.

openshift-ci · 2026-01-22T15:20:29Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: anik120

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

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

anik120 · 2026-01-22T15:23:42Z

Looks like ci added both the labels as soon as I approved it. I'll hold it for @camilamacedo86 to cancel the hold since she was reviewing it too

/hold

tmshort · 2026-01-22T15:29:42Z

Weird how this merged with failing tests... :(

anik120 · 2026-01-22T15:30:07Z

woah did the CI bot just totally go rouge here? XD

It added both the labels and didn't even care about the hold XD

Fwiw I was okay with this merging.

openshift-ci bot requested review from anik120 and dtfranz January 19, 2026 19:26

tmshort force-pushed the apiserver branch from 4635e13 to dcd2d46 Compare January 19, 2026 20:30

openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Jan 20, 2026

camilamacedo86 reviewed Jan 21, 2026

View reviewed changes

tmshort force-pushed the apiserver branch from dcd2d46 to 3c2e445 Compare January 21, 2026 18:24

anik120 requested changes Jan 21, 2026

View reviewed changes

openshift-ci bot assigned anik120 Jan 21, 2026

tmshort force-pushed the apiserver branch from 3c2e445 to 5baa069 Compare January 21, 2026 20:34

tmshort force-pushed the apiserver branch 3 times, most recently from 98ad4dd to 4f0ce11 Compare January 21, 2026 20:54

camilamacedo86 reviewed Jan 22, 2026

View reviewed changes

HTTPS_SERVERS.md Outdated Show resolved Hide resolved

camilamacedo86 reviewed Jan 22, 2026

View reviewed changes

WATCHED_RESOURCES.md Outdated Show resolved Hide resolved

camilamacedo86 reviewed Jan 22, 2026

View reviewed changes

HTTPS_SERVERS.md Outdated Show resolved Hide resolved

camilamacedo86 reviewed Jan 22, 2026

View reviewed changes

APISERVER_TLS_CONTROLLER.md Outdated Show resolved Hide resolved

anik120 reviewed Jan 22, 2026

View reviewed changes

tmshort force-pushed the apiserver branch from 4f0ce11 to cea84a0 Compare January 22, 2026 14:27

anik120 approved these changes Jan 22, 2026

View reviewed changes

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

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 22, 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 Jan 22, 2026

openshift-merge-bot bot merged commit 6fc19e0 into operator-framework:master Jan 22, 2026
16 of 21 checks passed

tmshort deleted the apiserver branch January 22, 2026 15:29

	// we don't have any cluster APIServer configuration.
	// we don't have any apiserver.config.openshift.io/cluster resource.

	// This is the cluster level global APIServer object name.
	// This is the cluster level global apiserver.config.openshift.io/cluster object name.

	// NewSyncer returns informer and sync functions to enable watch of APIServer type.
	// NewSyncer returns informer and sync functions to enable watch of apiserver.config.openshift.io/cluster CRs.

OPRUN-4411: Add APIServer TLS controller for OpenShift cluster-wide TLS configuration #3739

OPRUN-4411: Add APIServer TLS controller for OpenShift cluster-wide TLS configuration #3739

Uh oh!

Conversation

tmshort commented Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bandrade commented Jan 20, 2026

Uh oh!

camilamacedo86 commented Jan 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tmshort Jan 21, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

camilamacedo86 commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tmshort commented Jan 21, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

camilamacedo86 commented Jan 22, 2026

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!

Choose a reason for hiding this comment

Uh oh!

tmshort commented Jan 22, 2026

Uh oh!

openshift-ci bot commented Jan 22, 2026

Uh oh!

anik120 commented Jan 22, 2026

Uh oh!

Uh oh!

tmshort commented Jan 22, 2026

Uh oh!

anik120 commented Jan 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tmshort commented Jan 19, 2026 •

edited

Loading

tmshort Jan 21, 2026 •

edited

Loading

camilamacedo86 commented Jan 21, 2026 •

edited

Loading