LOG-8973: Enhance the log-file-metrics-exporter to be configurable with TLS Profile that includes curves

[Clee2691]

Summary

• Add APIServer watch to the LFME controller so LogFileMetricExporter DaemonSets are reconciled when the cluster TLS security profile changes
• Add IsClusterAPIServer and APIServerTLSProfileChangedPredicate helpers to the TLS package
• LFME pods are rolling-restarted with updated TLS settings (cipher suites, min version) automatically
• Added support for TLS curves when it gets implemented in github.com/NE-2334: Implement enhancement in OpenShift API to support for TLS curves in TLSProfile api#2583.

This PR also depends on ViaQ/log-file-metric-exporter#43 so that the LFME actually honors the curves.

Fixes LOG-8973

Details

When the cluster TLS profile is updated (e.g. oc patch apiserver cluster --type=merge -p '{"spec":{"tlsSecurityProfile":{"type":"Modern"}}}'), the LFME controller now:

1. Detects the change via an APIServer watch with a TLS-profile-specific predicate
2. Enqueues all LogFileMetricExporter resources for reconciliation
3. ReconcileDaemonset fetches the current TLS profile and rebuilds the DaemonSet spec
4. Kubernetes rolls out updated pods with the new TLS configuration

Curves are implicitly supported through ECDHE cipher suites — Go's crypto/tls handles curve negotiation automatically.

Test plan

• go build ./... passes
• go test ./internal/tls/... — new predicate/helper tests pass
• go test ./internal/metrics/logfilemetricexporter/... — regression tests pass
• Deploy to test cluster and verify LFME pods restart after oc patch apiserver cluster with a different TLS profile
• Verify LFME metrics endpoint is scrapable with the updated TLS profile

🤖 Generated with Claude Code

[openshift-ci-robot]

@Clee2691: This pull request references LOG-8973 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.8.0" version, but no target version was set.

Details

In response to this:

Summary

• Add APIServer watch to the LFME controller so LogFileMetricExporter DaemonSets are reconciled when the cluster TLS security profile changes
• Add IsClusterAPIServer and APIServerTLSProfileChangedPredicate helpers to the TLS package
• LFME pods are rolling-restarted with updated TLS settings (cipher suites, min version) automatically

Fixes LOG-8973

Details

When the cluster TLS profile is updated (e.g. oc patch apiserver cluster --type=merge -p '{"spec":{"tlsSecurityProfile":{"type":"Modern"}}}'), the LFME controller now:

1. Detects the change via an APIServer watch with a TLS-profile-specific predicate
2. Enqueues all LogFileMetricExporter resources for reconciliation
3. ReconcileDaemonset fetches the current TLS profile and rebuilds the DaemonSet spec
4. Kubernetes rolls out updated pods with the new TLS configuration

Curves are implicitly supported through ECDHE cipher suites — Go's crypto/tls handles curve negotiation automatically.

Test plan

• go build ./... passes
• go test ./internal/tls/... — new predicate/helper tests pass
• go test ./internal/metrics/logfilemetricexporter/... — regression tests pass
• Deploy to test cluster and verify LFME pods restart after oc patch apiserver cluster with a different TLS profile
• Verify LFME metrics endpoint is scrapable with the updated TLS profile

🤖 Generated with Claude Code

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Clee2691
Once this PR has been reviewed and has the lgtm label, please assign cahartma 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

[qodo-code-review]

Review Summary by Qodo

Watch APIServer TLS profile changes to trigger LFME reconciliation

✨ Enhancement 🧪 Tests

Walkthroughs

Description

• Add APIServer watch to LFME controller for TLS profile changes
• New IsClusterAPIServer and APIServerTLSProfileChangedPredicate helpers in TLS package
• LFME resources are enqueued for reconciliation on TLS profile updates
• Comprehensive tests for new TLS predicate and helper functions

Diagram

flowchart LR
  A["APIServer Resource"] -- "TLS profile update" --> B["APIServerTLSProfileChangedPredicate"]
  B -- "filters event" --> C["mapAPIServerToLogFileMetricExporters"]
  C -- "lists & enqueues" --> D["LogFileMetricExporter resources"]
  D -- "reconcile" --> E["DaemonSet updated with new TLS config"]
  F["IsClusterAPIServer"] -- "validates name == cluster" --> B

Loading

File Changes

1. internal/controller/logfilemetricsexporter/logfilemetricsexporter_controller.go ✨ Enhancement +34/-0

Add APIServer watch to enqueue LFME on TLS profile change

• Added configv1, tls, types, and handler imports
• Added Watches call for configv1.APIServer with APIServerTLSProfileChangedPredicate in
 SetupWithManager
• Implemented mapAPIServerToLogFileMetricExporters to list and enqueue all LFME resources on
 APIServer TLS changes

internal/controller/logfilemetricsexporter/logfilemetricsexporter_controller.go

---

2. internal/tls/tls.go ✨ Enhancement +37/-0

Add TLS predicate and APIServer helper functions

• Added reflect import and controller-runtime event/predicate imports
• Implemented IsClusterAPIServer helper to check if an object is the cluster APIServer
• Implemented APIServerTLSProfileChangedPredicate returning a predicate that filters on TLS
 profile changes, with configurable create-event behavior

internal/tls/tls.go

---

3. internal/tls/tls_test.go 🧪 Tests +109/-0

Add unit tests for new TLS predicate helpers

• Added tests for IsClusterAPIServer covering cluster and non-cluster names
• Added tests for APIServerTLSProfileChangedPredicate covering create (with/without
 reconcileOnCreate), update (changed/unchanged TLS profile), and delete events

internal/tls/tls_test.go

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. RBAC annotation drift risk 🐞 Bug ☼ Reliability

Description

The LFME controller now watches config.openshift.io APIServer objects, but the centralized
kubebuilder RBAC annotation file does not declare apiservers permissions, so regenerating manifests
(controller-gen) can drop required RBAC and break the watch/reconcile-on-TLS-change behavior.

Code

internal/controller/logfilemetricsexporter/logfilemetricsexporter_controller.go[R127-131]

+		Watches(
+			&configv1.APIServer{},
+			handler.EnqueueRequestsFromMapFunc(r.mapAPIServerToLogFileMetricExporters),
+			builder.WithPredicates(tls.APIServerTLSProfileChangedPredicate(true)),
+		).

Evidence

The controller adds a watch on configv1.APIServer, which requires get/list/watch RBAC. The repo’s
kubebuilder RBAC annotations collector does not include apiservers in config.openshift.io, creating
a mismatch with the checked-in generated ClusterRole (which currently does include apiservers). If
manifests are regenerated from annotations, the apiservers rule may be lost.

internal/controller/logfilemetricsexporter/logfilemetricsexporter_controller.go[120-132]
internal/controller/kubebuilder_rbac.go[6-22]
config/rbac/role.yaml[21-39]

Agent prompt

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

### Issue description
The LFME controller now watches `config.openshift.io/APIServer`, but the kubebuilder RBAC annotations collector file (`internal/controller/kubebuilder_rbac.go`) does not declare RBAC for `apiservers`. This risks losing the permission if RBAC manifests are regenerated via `controller-gen`.

### Issue Context
The checked-in `config/rbac/role.yaml` currently includes `apiservers`, but that rule is not backed by a kubebuilder RBAC annotation in the repo, so it can be overwritten/removed during regeneration.

### Fix Focus Areas
- internal/controller/kubebuilder_rbac.go[6-22]

### Suggested change
Add a kubebuilder RBAC annotation (or extend the existing config.openshift.io line) to include:
- `groups=config.openshift.io,resources=apiservers,verbs=get;list;watch`

Keep it consistent with how other config.openshift.io resources are declared in that file.

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

---

ⓘ The new review experience is currently in Beta. Learn more

[jcantrill]

/hold

[jcantrill]

#3238 adds an e2e test for scraping metrics we may be able to build off for changes like this one

[Clee2691]

/retest

[Clee2691]

/retest

[openshift-ci-robot]

@Clee2691: This pull request references LOG-8973 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.8.0" version, but no target version was set.

Details

In response to this:

Summary

• Add APIServer watch to the LFME controller so LogFileMetricExporter DaemonSets are reconciled when the cluster TLS security profile changes
• Add IsClusterAPIServer and APIServerTLSProfileChangedPredicate helpers to the TLS package
• LFME pods are rolling-restarted with updated TLS settings (cipher suites, min version) automatically
• Added support for TLS curves when it gets implemented in github.com/NE-2334: Implement enhancement in OpenShift API to support for TLS curves in TLSProfile api#2583.

This PR also depends on ViaQ/log-file-metric-exporter#43 so that the LFME actually honors the curves.

Fixes LOG-8973

Details

When the cluster TLS profile is updated (e.g. oc patch apiserver cluster --type=merge -p '{"spec":{"tlsSecurityProfile":{"type":"Modern"}}}'), the LFME controller now:

1. Detects the change via an APIServer watch with a TLS-profile-specific predicate
2. Enqueues all LogFileMetricExporter resources for reconciliation
3. ReconcileDaemonset fetches the current TLS profile and rebuilds the DaemonSet spec
4. Kubernetes rolls out updated pods with the new TLS configuration

Curves are implicitly supported through ECDHE cipher suites — Go's crypto/tls handles curve negotiation automatically.

Test plan

• go build ./... passes
• go test ./internal/tls/... — new predicate/helper tests pass
• go test ./internal/metrics/logfilemetricexporter/... — regression tests pass
• Deploy to test cluster and verify LFME pods restart after oc patch apiserver cluster with a different TLS profile
• Verify LFME metrics endpoint is scrapable with the updated TLS profile

🤖 Generated with Claude Code

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

@Clee2691: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/functional-target	f30dc11	link	true	/test functional-target
ci/prow/e2e-target	f30dc11	link	true	/test e2e-target

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.