LOG-8973: Enhance the log-file-metrics-exporter to be configurable with TLS Profile that includes curves by Clee2691 · Pull Request #3239 · openshift/cluster-logging-operator

Clee2691 · 2026-04-01T21:14:04Z

Summary

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.

Details

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

🤖 Generated with Claude Code

openshift-ci-robot · 2026-04-01T21:14:08Z

@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:

Detects the change via an APIServer watch with a TLS-profile-specific predicate

Enqueues all LogFileMetricExporter resources for reconciliation

ReconcileDaemonset fetches the current TLS profile and rebuilds the DaemonSet spec

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 · 2026-04-01T21:14:22Z

[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 · 2026-04-01T21:21:54Z

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

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 · 2026-04-01T21:24:50Z

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 · 2026-04-02T13:36:35Z

/hold

jcantrill

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

Clee2691 · 2026-04-02T23:25:32Z

/retest

Clee2691 · 2026-04-03T14:40:32Z

/retest

openshift-ci-robot · 2026-04-14T00:27:23Z

@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:

Detects the change via an APIServer watch with a TLS-profile-specific predicate

Enqueues all LogFileMetricExporter resources for reconciliation

ReconcileDaemonset fetches the current TLS profile and rebuilds the DaemonSet spec

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.

…th TLS Profile that includes curves Co-Authored-By: Claude Opus 4.6

openshift-ci-robot · 2026-04-17T18:35:30Z

@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

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:

Detects the change via an APIServer watch with a TLS-profile-specific predicate

Enqueues all LogFileMetricExporter resources for reconciliation

ReconcileDaemonset fetches the current TLS profile and rebuilds the DaemonSet spec

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-robot · 2026-04-17T18:36:14Z

@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

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

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

🤖 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 · 2026-04-17T21:29:47Z

@Clee2691: all tests passed!

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.

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Apr 1, 2026

openshift-ci bot requested review from alanconway and cahartma April 1, 2026 21:14

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 2, 2026

jcantrill reviewed Apr 2, 2026

View reviewed changes

Comment thread internal/controller/logfilemetricsexporter/logfilemetricsexporter_controller.go Outdated

Comment thread internal/tls/tls.go Outdated

jcantrill added the release/6.6 label Apr 2, 2026

Clee2691 force-pushed the LOG-8968 branch from bf9aeba to a82e979 Compare April 2, 2026 17:38

Clee2691 mentioned this pull request Apr 14, 2026

LOG-8973: Enhance the log-file-metrics-exporter to be configurable with TLS Profile that includes curves ViaQ/log-file-metric-exporter#43

Merged

Clee2691 force-pushed the LOG-8968 branch from a82e979 to f30dc11 Compare April 14, 2026 00:23

Clee2691 changed the title ~~LOG-8973: Watch APIServer TLS profile changes to restart LFME~~ LOG-8973: Enhance the log-file-metrics-exporter to be configurable with TLS Profile that includes curves Apr 14, 2026

LOG-8973: Enhance the log-file-metrics-exporter to be configurable wi…

7fb4306

…th TLS Profile that includes curves Co-Authored-By: Claude Opus 4.6

Clee2691 force-pushed the LOG-8968 branch from f30dc11 to 7fb4306 Compare April 17, 2026 18:34

Conversation

Clee2691 commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Details

Uh oh!

openshift-ci-robot commented Apr 1, 2026 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Details

Test plan

Uh oh!

openshift-ci bot commented Apr 1, 2026

Uh oh!

qodo-code-review bot commented Apr 1, 2026

Review Summary by Qodo

Walkthroughs

File Changes

Uh oh!

qodo-code-review bot commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

jcantrill commented Apr 2, 2026

Uh oh!

jcantrill left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Clee2691 commented Apr 2, 2026

Uh oh!

Clee2691 commented Apr 3, 2026

Uh oh!

openshift-ci-robot commented Apr 14, 2026 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Details

Test plan

Uh oh!

openshift-ci-robot commented Apr 17, 2026 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Details

Test plan

Uh oh!

openshift-ci-robot commented Apr 17, 2026 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Details

Uh oh!

openshift-ci bot commented Apr 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Clee2691 commented Apr 1, 2026 •

edited

Loading

openshift-ci-robot commented Apr 1, 2026 •

edited by openshift-ci bot

Loading

qodo-code-review bot commented Apr 1, 2026 •

edited

Loading

openshift-ci-robot commented Apr 14, 2026 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Apr 17, 2026 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Apr 17, 2026 •

edited by openshift-ci bot

Loading