OCPBUGS-98575: watch APIServer TLS profile and restart on change by tmshort · Pull Request #3867 · operator-framework/operator-lifecycle-manager

tmshort · 2026-07-13T20:25:21Z

When --tls-min-version is not supplied via flags, packageserver now sets up a watch on APIServer.config.openshift.io/cluster after applying the initial TLS profile at startup. If the profile changes, the process exits so it restarts with the updated settings — the same restart pattern used for OLMConfig interval changes.

The watch is intentionally skipped when --tls-min-version is present (i.e. flags are injected externally). In that case the external manager is responsible for triggering restarts on profile changes, avoiding unnecessary restarts.

openshift-ci · 2026-07-13T20:25:38Z

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

Copilot

Pull request overview

This PR updates the package-server startup behavior on OpenShift to monitor the cluster-wide APIServer.config.openshift.io/cluster TLS profile and trigger a process restart when it changes, so the package-server can come back up using the updated TLS serving settings (mirroring the existing restart-on-OLMConfig-change pattern).

Changes:

Add an APIServer informer/queue to detect TLS profile changes and exit(0) to trigger a restart.
Introduce a new operator workqueue dedicated to APIServer TLS change handling.
Compare current serving TLS settings against the APIServer TLS profile to decide whether a restart is necessary.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+	// If the TLS profile was self-applied at startup (no --tls-min-version flag),
+	// watch APIServer for changes and restart so the new profile is picked up.
+	// When flags are externally injected (e.g. by PSM), this watch is skipped;
+	// the external manager is responsible for triggering restarts on profile changes.
+	if o.SecureServing.MinTLSVersion != "" && len(o.SecureServing.CipherSuites) > 0 {


+	newMinVersion, newCipherSuites := olmapiserver.GetSecurityProfileConfig(apiServer.Spec.TLSSecurityProfile)
+	newMinVersionStr := libcrypto.TLSVersionToNameOrDie(newMinVersion)
+	newCipherNames := libcrypto.CipherSuitesToNamesOrDie(newCipherSuites)
+
+	currentMinVersion := op.options.SecureServing.MinTLSVersion
+	currentCipherNames := op.options.SecureServing.CipherSuites
+
+	if newMinVersionStr == currentMinVersion && slices.Equal(newCipherNames, currentCipherNames) {
+		return nil
+	}
+
+	log.Warnf("APIServer TLS profile changed (minVersion: %s → %s), restarting to apply new profile",
+		currentMinVersion, newMinVersionStr)
+	os.Exit(0)
+	return nil


+// syncAPIServerTLSProfile is called by the APIServer informer whenever the
+// cluster singleton APIServer object changes. If the TLS profile differs from
+// the one applied at startup the process exits so it can be restarted with the
+// updated profile. This watch is only active when --tls-min-version was not
+// supplied via flags; when flags are externally managed (e.g. by PSM) this
+// handler is never registered.
+func (op *Operator) syncAPIServerTLSProfile(obj interface{}) error {


Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

+	selfApplied := o.SecureServing.MinTLSVersion == ""
+	if selfApplied {
 		// Warn and continue on failure: the API server may not be reachable
 		// during initial startup (e.g. cluster bootstrap). Packageserver starts
 		// with secure TLS defaults; operators can supply --tls-min-version and


perdasilva · 2026-07-14T07:09:42Z

+
+	newMinVersion, newCipherSuites := olmapiserver.GetSecurityProfileConfig(apiServer.Spec.TLSSecurityProfile)
+	newMinVersionStr := libcrypto.TLSVersionToNameOrDie(newMinVersion)
+	newCipherNames := libcrypto.CipherSuitesToNamesOrDie(newCipherSuites)


Panic risk in watch handler: TLSVersionToNameOrDie and CipherSuitesToNamesOrDie both call panic() when the input value is not in their known maps (verified in vendor/github.com/openshift/library-go/pkg/crypto/crypto.go). The same functions are used at startup in applyClusterTLSProfileWithClients, but a panic there fails the process once and is caught by the pod restart policy. Here, this handler fires on every APIServer CR update, so if the cluster admin changes to a Custom profile that contains a cipher suite or TLS version not yet in library-go's map, the handler will panic on each reconcile, causing a crash loop rather than a clean error.

Consider wrapping with a recover, or switching to returning-an-error variants (e.g. TLSVersionToName/CipherSuiteToName) so the handler can return an error that the queue will log and retry safely:

newMinVersionStr, err := libcrypto.TLSVersionToName(newMinVersion) if err != nil { return fmt.Errorf("unknown TLS version %d in APIServer profile: %w", newMinVersion, err) }

perdasilva · 2026-07-14T07:09:53Z

+	}
+
+	log.Warnf("APIServer TLS profile changed (minVersion: %s → %s, ciphers: %v → %v), restarting to apply new profile",
+		currentMinVersion, newMinVersionStr, currentCipherNames, newCipherNames)


Log shows unsorted ciphers, comparison uses sorted: The restart decision is correctly made on sorted copies (newSorted/currentSorted), but the Warnf on the next line logs the original unsorted slices (currentCipherNames, newCipherNames). When a restart fires, an operator reading the log sees the raw cipher order, which may differ from how the sorted comparison actually saw them. Logging the sorted versions makes the log easier to reconcile against the comparison:

Suggested change

currentMinVersion, newMinVersionStr, currentCipherNames, newCipherNames)

log.Warnf("APIServer TLS profile changed (minVersion: %s → %s, ciphers: %v → %v), restarting to apply new profile",

currentMinVersion, newMinVersionStr, currentSorted, newSorted)

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

+	// ensuring that externally-managed instances (e.g. PSM-injected flags) are not
+	// restarted by this code path. Providing --tls-cipher-suites without
+	// --tls-min-version would create a mixed state that the watch cannot reconcile
+	// safely, so the watch is gated on both flags being absent.
+	selfApplied := o.SecureServing.MinTLSVersion == "" && len(o.SecureServing.CipherSuites) == 0


+				apiServerInformer := configinformers.NewSharedInformerFactory(cfgClient, 0).Config().V1().APIServers()
+				apiServerQueueInformer, qiErr := queueinformer.NewQueueInformer(
+					ctx,
+					queueinformer.WithInformer(apiServerInformer.Informer()),
+					queueinformer.WithQueue(op.apiServerTLSQueue),


+	apiServer, ok := obj.(*apiconfigv1.APIServer)
+	if !ok {
+		return fmt.Errorf("casting APIServer failed")
+	}


Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

pkg/package-server/server/server.go:452

The applyClusterTLSProfile godoc still states it is only used when --tls-min-version is not provided, but the startup logic now calls it when either TLS field is unset (min version or cipher suites). Please update this comment to reflect the current behavior.

// This is the fallback path used when --tls-min-version is not provided via flags
// (i.e. before the PSM has had a chance to inject them).

          verbs:
          - get
+          - list
+          - watch


          verbs:
          - get
+          - list
+          - watch


+// updated profile. This watch is only active when --tls-min-version was not
+// supplied via flags; when flags are externally managed (e.g. by PSM) this
+// handler is never registered.


+			// Snapshot the applied profile as raw uint16 values so the watch
+			// handler can compare without calling TLSVersionToNameOrDie or
+			// CipherSuitesToNamesOrDie, which panic on unrecognised inputs.
+			apiServerCR, getErr := cfgClient.ConfigV1().APIServers().Get(ctx, "cluster", metav1.GetOptions{})


Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

+	apiServer, ok := obj.(*apiconfigv1.APIServer)
+	if !ok {
+		return fmt.Errorf("expected *configv1.APIServer, got %T", obj)
+	}


+	newVersionStr := libcrypto.TLSVersionToNameOrDie(newMinVersion)
+	currentVersionStr := libcrypto.TLSVersionToNameOrDie(op.appliedTLSMinVersion)
+	newCipherNames := libcrypto.CipherSuitesToNamesOrDie(newSortedCiphers)
+	currentCipherNames := libcrypto.CipherSuitesToNamesOrDie(currentSortedCiphers)
+	log.Warnf("APIServer TLS profile changed (minVersion: %s → %s, ciphers: %v → %v), restarting to apply new profile",
+		currentVersionStr, newVersionStr, currentCipherNames, newCipherNames)


When --tls-min-version is not supplied via flags, packageserver now sets up a watch on APIServer.config.openshift.io/cluster after applying the initial TLS profile at startup. If the profile changes, the process exits so it restarts with the updated settings — the same restart pattern used for OLMConfig interval changes. The watch is intentionally skipped when --tls-min-version is present (i.e. flags are injected externally). In that case the external manager is responsible for triggering restarts on profile changes, avoiding unnecessary restarts.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

pkg/package-server/server/server.go:451

The comment above applyClusterTLSProfile still describes this as a fallback only when --tls-min-version is unset, but the call site now invokes it when either the min version or cipher suites are unset. Updating the comment will prevent future confusion about when this path runs.

// applyClusterTLSProfile fetches the cluster-wide APIServer TLS security profile
// and applies it to the SecureServingOptions. It is a no-op on non-OpenShift clusters.
// This is the fallback path used when --tls-min-version is not provided via flags
// (i.e. before the PSM has had a chance to inject them).

+			if getErr != nil {
+				log.WithError(getErr).Warn("Failed to snapshot APIServer TLS profile; watch will not be registered")
+			} else {
+				op.appliedTLSMinVersion, op.appliedTLSCipherSuites = olmapiserver.GetSecurityProfileConfig(apiServerCR.Spec.TLSSecurityProfile)


+	// The APIServer watch is only registered when *neither* TLS flag was originally
+	// supplied and the CR lookup succeeded. When flags are externally injected (e.g.
+	// by PSM), the watch is skipped; the external manager is responsible for
+	// triggering restarts on profile changes.
+	selfApplied := minVersionWasEmpty && cipherSuitesWasEmpty &&
+		o.SecureServing.MinTLSVersion != "" && len(o.SecureServing.CipherSuites) > 0


Copilot AI review requested due to automatic review settings July 13, 2026 20:25

openshift-ci Bot requested review from camilamacedo86 and kevinrizza July 13, 2026 20:25

tmshort changed the title ~~watch APIServer TLS profile and restart on change~~ OCPBUGS-98575: watch APIServer TLS profile and restart on change Jul 13, 2026

Copilot started reviewing on behalf of tmshort July 13, 2026 20:25 View session

Copilot AI reviewed Jul 13, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 13, 2026 21:15

tmshort force-pushed the fix-ocpbugs-98575 branch from db27d68 to 853f1ab Compare July 13, 2026 21:15

Copilot started reviewing on behalf of tmshort July 13, 2026 21:16 View session

Copilot AI reviewed Jul 13, 2026

View reviewed changes

perdasilva reviewed Jul 14, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 14, 2026 13:43

tmshort force-pushed the fix-ocpbugs-98575 branch from 853f1ab to d1b788a Compare July 14, 2026 13:43

Copilot started reviewing on behalf of tmshort July 14, 2026 13:52 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 14, 2026 18:05

tmshort force-pushed the fix-ocpbugs-98575 branch from d1b788a to effa3b8 Compare July 14, 2026 18:05

Copilot started reviewing on behalf of tmshort July 14, 2026 18:05 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 14, 2026 18:39

tmshort force-pushed the fix-ocpbugs-98575 branch from effa3b8 to ebe053e Compare July 14, 2026 18:39

Copilot started reviewing on behalf of tmshort July 14, 2026 18:39 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 14, 2026 19:21

tmshort force-pushed the fix-ocpbugs-98575 branch from ebe053e to 7c44b60 Compare July 14, 2026 19:21

Copilot started reviewing on behalf of tmshort July 14, 2026 19:21 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

OCPBUGS-98575: watch APIServer TLS profile and restart on change#3867

OCPBUGS-98575: watch APIServer TLS profile and restart on change#3867
tmshort wants to merge 1 commit into
operator-framework:masterfrom
tmshort:fix-ocpbugs-98575

tmshort commented Jul 13, 2026

Uh oh!

openshift-ci Bot commented Jul 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

perdasilva Jul 14, 2026

Uh oh!

perdasilva Jul 14, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	currentMinVersion, newMinVersionStr, currentCipherNames, newCipherNames)
	log.Warnf("APIServer TLS profile changed (minVersion: %s → %s, ciphers: %v → %v), restarting to apply new profile",
	currentMinVersion, newMinVersionStr, currentSorted, newSorted)

Uh oh!

Conversation

tmshort commented Jul 13, 2026

Uh oh!

openshift-ci Bot commented Jul 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

perdasilva Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

perdasilva Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants