Skip to content

chore: use centralised TLS Profile - #3234

Open
OpinionatedHeron wants to merge 14 commits into
redhat-developer:mainfrom
OpinionatedHeron:tls
Open

chore: use centralised TLS Profile#3234
OpinionatedHeron wants to merge 14 commits into
redhat-developer:mainfrom
OpinionatedHeron:tls

Conversation

@OpinionatedHeron

Copy link
Copy Markdown
Member

Description

Implemented centralized OpenShift TLS security profile support for the RHDH operator (controller-runtime path via github.com/openshift/controller-runtime-common), so the operator’s TLS servers honor apiservers.config.openshift.io/cluster.

Which issue(s) does this PR fix or relate to

PR acceptance criteria

  • Tests
  • Documentation

Building Container Images for Testing

Need to test container images from this PR?

For Maintainers: To trigger a test image build, review the code and comment /build-images.
This always builds the HEAD of the PR branch.

For Contributors: Ask a maintainer to run /build-images.

Images will be built and pushed to Quay with links posted in comments.

Signed-off-by: Leanne Ahern <lahern@redhat.com>
Signed-off-by: Leanne Ahern <lahern@redhat.com>
…ices

Signed-off-by: Leanne Ahern <lahern@redhat.com>
@OpinionatedHeron
OpinionatedHeron requested a review from a team as a code owner July 21, 2026 14:20
@rhdh-qodo-merge

rhdh-qodo-merge Bot commented Jul 21, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Context used
⚠️ Tickets: not configured — ticket URL found in PR but could not be fetched — check ticket provider credentials
✅ Compliance rules (platform): 18 rules
✅ Cross-repo context
  Not relevant to this PR: redhat-developer/rhdh
  Not relevant to this PR: redhat-developer/rhdh-plugins

Grey Divider


Action required

1. TLSProfiles nil deref 🐞 Bug ☼ Reliability ⭐ New
Description
cmd/main.go dereferences the result of a TLSProfiles map lookup without validating it, which can
panic the operator at startup if the Intermediate entry is unexpectedly absent/nil (e.g., due to an
incompatible openshift/api change). This panic happens before the later fallback logic, so the
intended Intermediate fallback cannot apply.
Code

cmd/main.go[111]

+	intermediateTLSProfile := *configv1.TLSProfiles[configv1.TLSProfileIntermediateType]
Relevance

●●● Strong

Team previously accepted changes to avoid operator panics/crashes; likely to add nil-check for
startup safety.

PR-#1949
PR-#2803

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The Intermediate fallback profile is obtained by dereferencing a map lookup result directly; in Go,
a missing key in a map of pointers yields a nil pointer, and dereferencing it panics.

cmd/main.go[109-116]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`intermediateTLSProfile := *configv1.TLSProfiles[configv1.TLSProfileIntermediateType]` can panic if the map lookup returns `nil`.

### Issue Context
This line runs during operator startup and is meant to set up the Intermediate fallback profile; a panic here prevents any subsequent fallback/error-handling from executing.

### Fix
- Change the Intermediate fallback initialization to a safe lookup:
 - Retrieve the pointer with `p, ok := configv1.TLSProfiles[...]`.
 - If `!ok || p == nil`, either:
   - exit with a clear error explaining the dependency incompatibility, or
   - use an explicitly defined safe default (do not dereference nil).

### Fix Focus Areas
- cmd/main.go[109-116]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. TLS fallback hard-exits ✓ Resolved 🐞 Bug ☼ Reliability
Description
cmd/main.go advertises an Intermediate fallback when TLS profile lookup/fetch fails, but it
terminates the process if tlspkg.GetTLSProfileSpec(nil) returns an error, preventing any fallback
and blocking operator startup in that case.
Code

cmd/main.go[R109-115]

+	// Fetch the TLS profile from apiservers.config.openshift.io/cluster.
+	// Fall back to Intermediate on non-OpenShift clusters (or if the fetch fails).
+	tlsSecurityProfileSpec, err := tlspkg.GetTLSProfileSpec(nil)
+	if err != nil {
+		setupLog.Error(err, "unable to get default TLS profile")
+		os.Exit(1)
+	}
Relevance

●● Moderate

No historical evidence on preferring fallback vs hard-exit for TLS default profile errors in main
startup.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The code comment says it should fall back to Intermediate, but the error branch exits the process
immediately, so the fallback cannot occur when the default-profile call errors.

cmd/main.go[109-132]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`cmd/main.go` intends to fall back to an Intermediate TLS profile when OpenShift TLS profile discovery fails, but currently calls `os.Exit(1)` if `tlspkg.GetTLSProfileSpec(nil)` returns an error. This bypasses the fallback behavior entirely.

### Issue Context
The code path logs and comments about using an "Intermediate fallback" for non-OpenShift clusters or fetch failures, but it still hard-exits before the fetch/fallback code can run.

### Fix Focus Areas
- cmd/main.go[109-132]

### Suggested fix
- Replace the `os.Exit(1)` branch with:
 - a log that you’re falling back, and
 - an explicit initialization of `tlsSecurityProfileSpec` to the Intermediate profile (or another safe default), then continue startup.
- Ensure the log messages match the actual behavior (i.e., only claim "Intermediate fallback" when you actually set it).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Deprecated Apply lint bypass 🐞 Bug ⚙ Maintainability ⭐ New
Description
New //nolint:staticcheck suppressions were added to keep using the deprecated client.Apply patch
type, which increases upgrade risk and obscures when/why migration is blocked. Since
controller-runtime v0.23 introduces client.Writer.Apply, these suppressions should be tracked with
a concrete migration plan (or centralized behind a helper) instead of being copy-pasted at call
sites.
Code

internal/controller/backstage_controller.go[173]

+	if err := r.Patch(ctx, obj, client.Apply, &client.PatchOptions{FieldManager: BackstageFieldManager, Force: ptr.To(true)}); err != nil { //nolint:staticcheck // SA1019: client.Apply is deprecated: Further investigation needed
Relevance

●● Moderate

Team accepted some staticcheck nolints (QF1003), but no clear precedent on SA1019/deprecation
suppression tracking.

PR-#1797

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new changes explicitly add SA1019 suppressions on client.Apply usage at multiple call sites;
the repo also shows controller-runtime v0.23 introduced a dedicated Apply method on the writer
interface, indicating a newer API exists even though migration may require work.

internal/controller/backstage_controller.go[160-176]
internal/controller/monitor.go[57-65]
internal/controller/plugin-deps.go[34-42]
internal/controller/mock_client.go[128-131]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Multiple call sites now suppress SA1019 to keep using deprecated `client.Apply` via `Patch`, spreading tech debt.

### Issue Context
This PR upgrades controller-runtime to v0.23.x (which introduced `client.Writer.Apply`), and adds suppressions at three reconciler call sites.

### Fix
Do one of:
- Add a clear TODO with a tracking issue link (e.g., `TODO(RHIDP-XXXXX): ...`) explaining why migration isn’t feasible yet, and consider centralizing the apply logic in one helper to avoid repeating suppressions.
- If feasible for your object types, migrate from `Patch(..., client.Apply, ...)` to the v0.23 `Apply(...)` API and remove the suppressions.

### Fix Focus Areas
- internal/controller/backstage_controller.go[173-175]
- internal/controller/monitor.go[62-65]
- internal/controller/plugin-deps.go[38-42]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Mock Apply panics 🐞 Bug ☼ Reliability
Description
MockClient.Apply unconditionally panics, so any unit test (or test helper) that starts using
controller-runtime v0.23+ Apply semantics will crash instead of failing with a usable error.
Code

internal/controller/mock_client.go[R128-131]

+// Apply was added to client.Writer in controller-runtime v0.23.
+func (m MockClient) Apply(_ context.Context, _ runtime.ApplyConfiguration, _ ...client.ApplyOption) error {
+	panic(implementMe)
+}
Relevance

●●● Strong

Team accepted removing panics in core code; likely expects MockClient.Apply to return error not
panic.

PR-#1949

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR adds a new Apply method that panics, and the mock client is actively used as the reconciler
client in unit tests, so invoking Apply would currently crash the tests.

internal/controller/mock_client.go[125-135]
internal/controller/monitor_test.go[20-30]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`MockClient.Apply(...)` was added to satisfy controller-runtime v0.23’s `client.Writer` interface, but it panics. This turns any future use of server-side apply in unit tests into a hard crash.

### Issue Context
The mock client is used in controller unit tests as the reconciler client.

### Fix Focus Areas
- internal/controller/mock_client.go[128-135]

### Suggested fix
- Replace the `panic(implementMe)` with a deterministic, non-panicking behavior:
 - Prefer returning a descriptive error like `fmt.Errorf("mock Apply not implemented")`.
 - Alternatively, implement minimal Apply semantics backed by the in-memory store (if tests are expected to use Apply soon).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. TLS profile fetch unguarded ✓ Resolved 📘 Rule violation ≡ Correctness
Description
cmd/main.go calls tlspkg.FetchAPIServerTLSProfile/FetchAPIServerTLSAdherencePolicy before any
explicit check that the OpenShift apiservers.config.openshift.io API is available. On
non-OpenShift clusters this performs requests against a potentially-missing API and relies on error
fallback instead of guarding access up front as required.
Code

cmd/main.go[R107-132]

+	restConfig := ctrl.GetConfigOrDie()
+
+	// Fetch the TLS profile from apiservers.config.openshift.io/cluster.
+	// Fall back to Intermediate on non-OpenShift clusters (or if the fetch fails).
+	tlsSecurityProfileSpec, err := tlspkg.GetTLSProfileSpec(nil)
+	if err != nil {
+		setupLog.Error(err, "unable to get default TLS profile")
+		os.Exit(1)
+	}
+	tlsAdherence := configv1.TLSAdherencePolicyNoOpinion
+
+	k8sClient, err := client.New(restConfig, client.Options{Scheme: scheme})
+	if err != nil {
+		setupLog.Info("unable to create client for TLS profile fetch, using Intermediate fallback", "error", err)
+	} else {
+		if profile, fetchErr := tlspkg.FetchAPIServerTLSProfile(ctx, k8sClient); fetchErr != nil {
+			setupLog.Info("unable to get TLS profile from API server, using Intermediate fallback", "error", fetchErr)
+		} else {
+			tlsSecurityProfileSpec = profile
+		}
+		if adherence, fetchErr := tlspkg.FetchAPIServerTLSAdherencePolicy(ctx, k8sClient); fetchErr != nil {
+			setupLog.Info("unable to get TLS adherence policy from API server", "error", fetchErr)
+		} else {
+			tlsAdherence = adherence
+		}
+	}
Relevance

●●● Strong

Team favored explicit CRD/API guards; kept ServiceMonitor CRD existence check, rejected relying on
fetch errors.

PR-#1374

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 11 requires an explicit CRD/API existence guard before accessing optional kinds.
The added code performs FetchAPIServerTLSProfile and FetchAPIServerTLSAdherencePolicy calls
prior to any OpenShift/CRD availability check (platform detection happens later), so access is not
properly guarded.

Rule 11: Guard reconciliation logic on optional CRD existence
cmd/main.go[107-132]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`cmd/main.go` fetches the OpenShift APIServer TLS profile/adherence policy before verifying that the OpenShift `apiservers.config.openshift.io` API/CRD is installed. The compliance rule requires guarding access to optional CRDs/APIs with an explicit availability check before making client calls.

## Issue Context
The platform detection (`plf, err := controller.DetectPlatform()`) and OpenShift-only gating is currently done later (for the watcher), but the initial fetch happens unconditionally.

## Fix Focus Areas
- cmd/main.go[107-132]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Previous review results

Review updated until commit d9a3542

Results up to commit 0f6515c ⚖️ Balanced


🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Action required
1. TLS fallback hard-exits ✓ Resolved 🐞 Bug ☼ Reliability
Description
cmd/main.go advertises an Intermediate fallback when TLS profile lookup/fetch fails, but it
terminates the process if tlspkg.GetTLSProfileSpec(nil) returns an error, preventing any fallback
and blocking operator startup in that case.
Code

cmd/main.go[R109-115]

+	// Fetch the TLS profile from apiservers.config.openshift.io/cluster.
+	// Fall back to Intermediate on non-OpenShift clusters (or if the fetch fails).
+	tlsSecurityProfileSpec, err := tlspkg.GetTLSProfileSpec(nil)
+	if err != nil {
+		setupLog.Error(err, "unable to get default TLS profile")
+		os.Exit(1)
+	}
Relevance

●● Moderate

No historical evidence on preferring fallback vs hard-exit for TLS default profile errors in main
startup.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The code comment says it should fall back to Intermediate, but the error branch exits the process
immediately, so the fallback cannot occur when the default-profile call errors.

cmd/main.go[109-132]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`cmd/main.go` intends to fall back to an Intermediate TLS profile when OpenShift TLS profile discovery fails, but currently calls `os.Exit(1)` if `tlspkg.GetTLSProfileSpec(nil)` returns an error. This bypasses the fallback behavior entirely.

### Issue Context
The code path logs and comments about using an "Intermediate fallback" for non-OpenShift clusters or fetch failures, but it still hard-exits before the fetch/fallback code can run.

### Fix Focus Areas
- cmd/main.go[109-132]

### Suggested fix
- Replace the `os.Exit(1)` branch with:
 - a log that you’re falling back, and
 - an explicit initialization of `tlsSecurityProfileSpec` to the Intermediate profile (or another safe default), then continue startup.
- Ensure the log messages match the actual behavior (i.e., only claim "Intermediate fallback" when you actually set it).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended
2. TLS profile fetch unguarded ✓ Resolved 📘 Rule violation ≡ Correctness
Description
cmd/main.go calls tlspkg.FetchAPIServerTLSProfile/FetchAPIServerTLSAdherencePolicy before any
explicit check that the OpenShift apiservers.config.openshift.io API is available. On
non-OpenShift clusters this performs requests against a potentially-missing API and relies on error
fallback instead of guarding access up front as required.
Code

cmd/main.go[R107-132]

+	restConfig := ctrl.GetConfigOrDie()
+
+	// Fetch the TLS profile from apiservers.config.openshift.io/cluster.
+	// Fall back to Intermediate on non-OpenShift clusters (or if the fetch fails).
+	tlsSecurityProfileSpec, err := tlspkg.GetTLSProfileSpec(nil)
+	if err != nil {
+		setupLog.Error(err, "unable to get default TLS profile")
+		os.Exit(1)
+	}
+	tlsAdherence := configv1.TLSAdherencePolicyNoOpinion
+
+	k8sClient, err := client.New(restConfig, client.Options{Scheme: scheme})
+	if err != nil {
+		setupLog.Info("unable to create client for TLS profile fetch, using Intermediate fallback", "error", err)
+	} else {
+		if profile, fetchErr := tlspkg.FetchAPIServerTLSProfile(ctx, k8sClient); fetchErr != nil {
+			setupLog.Info("unable to get TLS profile from API server, using Intermediate fallback", "error", fetchErr)
+		} else {
+			tlsSecurityProfileSpec = profile
+		}
+		if adherence, fetchErr := tlspkg.FetchAPIServerTLSAdherencePolicy(ctx, k8sClient); fetchErr != nil {
+			setupLog.Info("unable to get TLS adherence policy from API server", "error", fetchErr)
+		} else {
+			tlsAdherence = adherence
+		}
+	}
Relevance

●●● Strong

Team favored explicit CRD/API guards; kept ServiceMonitor CRD existence check, rejected relying on
fetch errors.

PR-#1374

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 11 requires an explicit CRD/API existence guard before accessing optional kinds.
The added code performs FetchAPIServerTLSProfile and FetchAPIServerTLSAdherencePolicy calls
prior to any OpenShift/CRD availability check (platform detection happens later), so access is not
properly guarded.

Rule 11: Guard reconciliation logic on optional CRD existence
cmd/main.go[107-132]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`cmd/main.go` fetches the OpenShift APIServer TLS profile/adherence policy before verifying that the OpenShift `apiservers.config.openshift.io` API/CRD is installed. The compliance rule requires guarding access to optional CRDs/APIs with an explicit availability check before making client calls.

## Issue Context
The platform detection (`plf, err := controller.DetectPlatform()`) and OpenShift-only gating is currently done later (for the watcher), but the initial fetch happens unconditionally.

## Fix Focus Areas
- cmd/main.go[107-132]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Mock Apply panics 🐞 Bug ☼ Reliability
Description
MockClient.Apply unconditionally panics, so any unit test (or test helper) that starts using
controller-runtime v0.23+ Apply semantics will crash instead of failing with a usable error.
Code

internal/controller/mock_client.go[R128-131]

+// Apply was added to client.Writer in controller-runtime v0.23.
+func (m MockClient) Apply(_ context.Context, _ runtime.ApplyConfiguration, _ ...client.ApplyOption) error {
+	panic(implementMe)
+}
Relevance

●●● Strong

Team accepted removing panics in core code; likely expects MockClient.Apply to return error not
panic.

PR-#1949

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR adds a new Apply method that panics, and the mock client is actively used as the reconciler
client in unit tests, so invoking Apply would currently crash the tests.

internal/controller/mock_client.go[125-135]
internal/controller/monitor_test.go[20-30]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`MockClient.Apply(...)` was added to satisfy controller-runtime v0.23’s `client.Writer` interface, but it panics. This turns any future use of server-side apply in unit tests into a hard crash.

### Issue Context
The mock client is used in controller unit tests as the reconciler client.

### Fix Focus Areas
- internal/controller/mock_client.go[128-135]

### Suggested fix
- Replace the `panic(implementMe)` with a deterministic, non-panicking behavior:
 - Prefer returning a descriptive error like `fmt.Errorf("mock Apply not implemented")`.
 - Alternatively, implement minimal Apply semantics backed by the in-memory store (if tests are expected to use Apply soon).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Qodo Logo

@rhdh-qodo-merge

rhdh-qodo-merge Bot commented Jul 21, 2026

Copy link
Copy Markdown

PR Summary by Qodo

Adopt OpenShift centralized TLS profile for operator servers

✨ Enhancement ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Read OpenShift APIServer TLS profile and apply it to controller-runtime TLS servers.
• Restart the operator when the cluster TLS profile/adherence policy changes.
• Update RBAC/CSV/manifests to watch apiservers and advertise TLS profile support.
Diagram

graph TD
  P["Operator Pod"] --> M["cmd/main.go"] --> K["k8s client"] --> A[("apiservers.config.openshift.io/cluster")]
  M --> T["TLS config"] --> R["controller-runtime mgr"] --> S["TLS servers"]
  A --> W["SecurityProfileWatcher"] --> P
  M -. "setup watcher" .-> W
  subgraph "Manifests/RBAC"
    C["CSV / RBAC"]
  end
  C -. "permits list/watch" .-> A
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Hot-reload TLS without restarting the pod
  • ➕ Avoids transient downtime during restarts
  • ➕ Applies TLS changes immediately without relying on external restart policy
  • ➖ Harder to implement safely across all controller-runtime TLS listeners
  • ➖ Risk of partial reload / mixed TLS state across servers and clients
2. Use openshift/library-go directly for profile plumbing
  • ➕ Reduces reliance on controller-runtime-common abstractions
  • ➕ Potentially more control over edge cases and defaults
  • ➖ More bespoke integration code to maintain
  • ➖ Greater chance of diverging from OpenShift-supported patterns

Recommendation: Keep the PR’s approach: controller-runtime-common is the OpenShift-aligned integration point, and restarting on profile/adherence changes is a pragmatic, low-risk way to ensure all TLS listeners pick up the new settings consistently. Consider hot-reload only if restart downtime becomes a documented problem.

Files changed (18) +157 / -112

Enhancement (1) +64 / -3
main.goFetch and apply OpenShift APIServer TLS profile; restart on changes +64/-3

Fetch and apply OpenShift APIServer TLS profile; restart on changes

• Adds startup logic to fetch apiservers.config.openshift.io/cluster TLS profile and adherence policy via controller-runtime-common, with Intermediate fallback when unavailable. Builds a tls.Config from the profile, passes it into controller-runtime TLS options, and installs a SecurityProfileWatcher on OpenShift to cancel the manager context and restart when settings change.

cmd/main.go

Refactor (7) +7 / -7
zz_generated.deepcopy.goRegenerate deepcopy imports (gofmt alias cleanup) +1/-1

Regenerate deepcopy imports (gofmt alias cleanup)

• Removes an unnecessary import alias in generated deepcopy code. No functional behavior change.

api/v1alpha1/zz_generated.deepcopy.go

zz_generated.deepcopy.goRegenerate deepcopy imports (gofmt alias cleanup) +1/-1

Regenerate deepcopy imports (gofmt alias cleanup)

• Removes an unnecessary import alias in generated deepcopy code. No functional behavior change.

api/v1alpha2/zz_generated.deepcopy.go

zz_generated.deepcopy.goRegenerate deepcopy imports (gofmt alias cleanup) +1/-1

Regenerate deepcopy imports (gofmt alias cleanup)

• Removes an unnecessary import alias in generated deepcopy code. No functional behavior change.

api/v1alpha3/zz_generated.deepcopy.go

zz_generated.deepcopy.goRegenerate deepcopy imports (gofmt alias cleanup) +1/-1

Regenerate deepcopy imports (gofmt alias cleanup)

• Removes an unnecessary import alias in generated deepcopy code. No functional behavior change.

api/v1alpha4/zz_generated.deepcopy.go

zz_generated.deepcopy.goRegenerate deepcopy imports (gofmt alias cleanup) +1/-1

Regenerate deepcopy imports (gofmt alias cleanup)

• Removes an unnecessary import alias in generated deepcopy code. No functional behavior change.

api/v1alpha5/zz_generated.deepcopy.go

monitor.goSuppress SSA deprecation lint for ServiceMonitor apply +1/-1

Suppress SSA deprecation lint for ServiceMonitor apply

• Adds a nolint annotation for deprecated server-side apply constant usage to keep staticcheck clean under the upgraded controller-runtime toolchain.

internal/controller/monitor.go

plugin-deps.goSuppress SSA deprecation lint for plugin deps apply +1/-1

Suppress SSA deprecation lint for plugin deps apply

• Adds a nolint annotation for deprecated server-side apply constant usage to keep staticcheck clean under the upgraded controller-runtime toolchain.

internal/controller/plugin-deps.go

Tests (1) +5 / -0
mock_client.goMockClient: implement new Apply method for controller-runtime v0.23 +5/-0

MockClient: implement new Apply method for controller-runtime v0.23

• Adds the Apply method stub required by the updated controller-runtime client.Writer interface. Prevents compilation/test failures after the dependency bump.

internal/controller/mock_client.go

Other (9) +81 / -102
backstage-operator.clusterserviceversion.yamlBundle CSV: add apiservers RBAC and refresh metadata +4/-1

Bundle CSV: add apiservers RBAC and refresh metadata

• Updates the CSV createdAt and extends config.openshift.io permissions to include apiservers with list/watch, enabling TLS profile watching. This ensures the operator can observe centralized TLS configuration.

bundle/backstage.io/manifests/backstage-operator.clusterserviceversion.yaml

backstage-operator.clusterserviceversion.yamlRHDH CSV: advertise tls-profiles and add apiservers RBAC +5/-2

RHDH CSV: advertise tls-profiles and add apiservers RBAC

• Marks features.operators.openshift.io/tls-profiles as true and grants list/watch permissions for apiservers (plus list/watch for ingresses). Enables consuming OpenShift centralized TLS profiles.

bundle/rhdh/manifests/backstage-operator.clusterserviceversion.yaml

backstage-operator.clusterserviceversion.yamlBase CSV: declare tls-profiles support +1/-1

Base CSV: declare tls-profiles support

• Flips features.operators.openshift.io/tls-profiles from false to true so the operator advertises centralized TLS profile support in OLM metadata.

config/manifests/rhdh/bases/backstage-operator.clusterserviceversion.yaml

role.yamlRBAC: allow list/watch apiservers for TLS profile watching +3/-0

RBAC: allow list/watch apiservers for TLS profile watching

• Adds apiservers to config.openshift.io resources and expands verbs to include list/watch (also for ingresses). Required for reading and watching the cluster TLS profile.

config/rbac/role.yaml

install.yamlDistribution manifest: apiservers RBAC for TLS profile support +3/-0

Distribution manifest: apiservers RBAC for TLS profile support

• Updates the rendered install manifest to grant list/watch on config.openshift.io apiservers (and list/watch for ingresses). Keeps dist output aligned with source RBAC.

dist/backstage.io/install.yaml

install.yamlDistribution manifest: apiservers RBAC for TLS profile support +3/-0

Distribution manifest: apiservers RBAC for TLS profile support

• Updates the rendered install manifest to grant list/watch on config.openshift.io apiservers (and list/watch for ingresses). Keeps dist output aligned with source RBAC.

dist/rhdh/install.yaml

go.modDependencies: add controller-runtime-common and bump controller-runtime +21/-29

Dependencies: add controller-runtime-common and bump controller-runtime

• Introduces github.com/openshift/controller-runtime-common for TLS profile support and bumps sigs.k8s.io/controller-runtime to v0.23.3. Updates k8s.io pinning to v0.35.4 to keep module minors aligned across transitive deps.

go.mod

go.sumUpdate dependency lockfile for TLS profile and controller-runtime bumps +38/-67

Update dependency lockfile for TLS profile and controller-runtime bumps

• Refreshes go.sum to reflect the new controller-runtime-common dependency and upgraded Kubernetes/controller-runtime ecosystem modules. Includes updated checksums for transitive libraries (e.g., structured-merge-diff v6).

go.sum

backstage_controller.goController RBAC: add apiservers permissions; suppress SSA deprecation lint +3/-2

Controller RBAC: add apiservers permissions; suppress SSA deprecation lint

• Extends kubebuilder RBAC markers to include apiservers and list/watch verbs needed for TLS profile watching. Adds a targeted nolint annotation for deprecated server-side apply constant usage under the newer controller-runtime toolchain.

internal/controller/backstage_controller.go

@rhdh-qodo-merge rhdh-qodo-merge Bot added documentation Improvements or additions to documentation enhancement New feature or request labels Jul 21, 2026
@OpinionatedHeron
OpinionatedHeron marked this pull request as draft July 21, 2026 14:28
Signed-off-by: Leanne Ahern <lahern@redhat.com>
Signed-off-by: Leanne Ahern <lahern@redhat.com>
@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 20.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.44%. Comparing base (789c71c) to head (d9a3542).

Files with missing lines Patch % Lines
internal/controller/mock_client.go 0.00% 2 Missing ⚠️
internal/controller/backstage_controller.go 0.00% 1 Missing ⚠️
internal/controller/plugin-deps.go 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3234      +/-   ##
==========================================
- Coverage   63.49%   63.44%   -0.06%     
==========================================
  Files          38       38              
  Lines        2356     2358       +2     
==========================================
  Hits         1496     1496              
- Misses        711      713       +2     
  Partials      149      149              
Flag Coverage Δ
nightly ?
unittests 63.44% <20.00%> (-0.06%) ⬇️

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

Files with missing lines Coverage Δ
internal/controller/monitor.go 94.87% <100.00%> (ø)
internal/controller/backstage_controller.go 0.00% <0.00%> (ø)
internal/controller/plugin-deps.go 0.00% <0.00%> (ø)
internal/controller/mock_client.go 36.00% <0.00%> (-0.99%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Leanne Ahern <lahern@redhat.com>
@OpinionatedHeron

Copy link
Copy Markdown
Member Author

/agentic_review

@rhdh-qodo-merge

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit e060c61

Signed-off-by: Leanne Ahern <lahern@redhat.com>
@OpinionatedHeron

Copy link
Copy Markdown
Member Author

/agentic_review

@rhdh-qodo-merge

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 8d6e98f

@gazarenkov

Copy link
Copy Markdown
Member

/build-images

@github-actions

Copy link
Copy Markdown
Contributor

PR images built successfully!

Images are available for testing:

  1. Operator: quay.io/rhdh-community/operator:0.11.0-pr-3234-8d6e98f
  2. Bundle: quay.io/rhdh-community/operator-bundle:0.11.0-pr-3234-8d6e98f
  3. Catalog: quay.io/rhdh-community/operator-catalog:0.11.0-pr-3234-8d6e98f

Also available with PR number tag:

  • quay.io/rhdh-community/operator:0.11.0-pr-3234
  • quay.io/rhdh-community/operator-bundle:0.11.0-pr-3234
  • quay.io/rhdh-community/operator-catalog:0.11.0-pr-3234

Triggered by @gazarenkov

@OpinionatedHeron
OpinionatedHeron marked this pull request as ready for review July 28, 2026 01:46
@rhdh-qodo-merge

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 8d6e98f

Comment thread cmd/main.go Outdated
Comment thread cmd/main.go Outdated
Resolve CSV conflicts by taking upstream 2.0 image/timestamps while
keeping TLS profile annotation and apiservers RBAC from this branch.

Co-authored-by: Cursor <cursoragent@cursor.com>
@sonarqubecloud

sonarqubecloud Bot commented Aug 3, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants