Skip to content

feat: have CVO inject the centralized TLS configuration into the operator's config #1167

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ingvagabund wants to merge 1 commit into
base: main
Choose a base branch
from
+7 −0
Open

feat: have CVO inject the centralized TLS configuration into the operator's config #1167

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions manifests/05-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ metadata:
include.release.openshift.io/self-managed-high-availability: "true"
include.release.openshift.io/single-node-developer: "true"
capability.openshift.io/name: Console
config.openshift.io/inject-tls: "true"
data:
controller-config.yaml: |
apiVersion: operator.openshift.io/v1alpha1
Expand Down
3 changes: 3 additions & 0 deletions manifests/07-operator-ibm-cloud-managed.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ spec:
containers:
- args:
- --config=/var/run/configmaps/config/controller-config.yaml
- --terminate-on-files=/var/run/configmaps/config/controller-config.yaml
- --terminate-on-files=/var/run/secrets/serving-cert/tls.crt
- --terminate-on-files=/var/run/secrets/serving-cert/tls.key
command:
- console
- operator
Expand Down
3 changes: 3 additions & 0 deletions manifests/07-operator.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ spec:
- operator
args:
- "--config=/var/run/configmaps/config/controller-config.yaml"
- "--terminate-on-files=/var/run/configmaps/config/controller-config.yaml"
- "--terminate-on-files=/var/run/secrets/serving-cert/tls.crt"
- "--terminate-on-files=/var/run/secrets/serving-cert/tls.key"
Comment on lines +61 to +63
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Search for file-watching implementation and error handling in the operator codebase

# Check for terminate-on-files flag implementation and how it handles missing files
rg -nP --type=go -C5 'terminate-on-files|TerminateOnFiles' 

# Look for file watching logic and error handling
rg -nP --type=go -C3 'fsnotify|file.*watch|inotify' 

# Check if there's logic to handle optional files or graceful degradation
rg -nP --type=go -C3 'optional.*file|file.*optional|skip.*missing'

Repository: openshift/console-operator

Length of output: 50383

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Find fileobserver implementation =="
rg -n "package fileobserver" -S vendor openshift || true
rg -n "fileobserver\.NewObserver|NewObserver\(" -S vendor | head -n 50 || true
rg -n "AddReactor\(" -S vendor | head -n 50 || true
rg -n "TerminateOnFiles" -S . | head -n 50 || true

echo
echo "== Show relevant manifest sections =="
# Print a manageable chunk around the mentioned lines
sed -n '40,140p' manifests/07-operator.yaml | cat -n

Repository: openshift/console-operator

Length of output: 6690

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== observer.go (fileobserver) =="
nl -ba vendor/github.com/openshift/library-go/pkg/controller/fileobserver/observer.go | sed -n '1,220p'

echo
echo "== observer_polling.go (fileobserver) =="
nl -ba vendor/github.com/openshift/library-go/pkg/controller/fileobserver/observer_polling.go | sed -n '1,240p'

echo
echo "== Any helper types in observer.go beyond 220 (if small) =="
nl -ba vendor/github.com/openshift/library-go/pkg/controller/fileobserver/observer.go | sed -n '220,420p'

Repository: openshift/console-operator

Length of output: 164

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== observer.go (fileobserver) =="
sed -n '1,260p' vendor/github.com/openshift/library-go/pkg/controller/fileobserver/observer.go | cat -n

echo
echo "== observer_polling.go (fileobserver) =="
sed -n '1,320p' vendor/github.com/openshift/library-go/pkg/controller/fileobserver/observer_polling.go | cat -n

echo
echo "== observer.go tail (if larger) =="
sed -n '260,520p' vendor/github.com/openshift/library-go/pkg/controller/fileobserver/observer.go | cat -n

Repository: openshift/console-operator

Length of output: 10870

🏁 Script executed:

sed -n '130,210p' vendor/github.com/openshift/library-go/pkg/controller/controllercmd/cmd.go | cat -n

Repository: openshift/console-operator

Length of output: 3301

Clarify --terminate-on-files behavior with optional serving-cert

The below args still terminate the operator when the watched files change:

- "--terminate-on-files=/var/run/secrets/serving-cert/tls.crt"
- "--terminate-on-files=/var/run/secrets/serving-cert/tls.key"
  • The file observer is polling-based and treats missing files as exists=false; it will not crash or exit just because tls.crt/tls.key don’t exist at startup.
  • However, the terminate reactor calls terminate() on the first observed file event, so when the optional serving-cert secret is later created and the tls.* files appear, the operator will terminate (causing a pod restart).
  • If a restart on secret injection/creation is not intended, adjust terminate-on-files for these optional TLS paths (or make the secret non-optional).
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@manifests/07-operator.yaml` around lines 61 - 63, The args
"--terminate-on-files=/var/run/secrets/serving-cert/tls.crt" and
"--terminate-on-files=/var/run/secrets/serving-cert/tls.key" in the operator
container args cause a terminate() on the first appearance event (e.g. when an
optional serving-cert secret is later created), so either remove these two
"--terminate-on-files=..." entries from the args list in 07-operator.yaml or
make the serving-cert secret mount non-optional (ensure the files exist at
startup); choose the approach you want and update the container args (or the
secret/mount) accordingly to prevent an unintended pod restart when
tls.crt/tls.key appear.

imagePullPolicy: IfNotPresent
volumeMounts:
- mountPath: /var/run/configmaps/config
Expand Down