NETOBSERV-2977: Add TLS support for collector when OpenShift by leandroberetta · Pull Request #552 · netobserv/netobserv-cli

leandroberetta · 2026-08-11T22:18:19Z

Description

Enable TLS for the collector↔agent gRPC connection when running on OpenShift, and make that connection honor the cluster's TLS security profile.

TLS enablement (service-ca)

Enable TLS for the gRPC collector server on OpenShift, using service-ca for automatic cert generation
Conditionally detect OpenShift and annotate the collector Service for cert generation, create a CA ConfigMap with inject-cabundle, and mount certs in the collector pod
Add CA volume and tls.caCertPath to the agent DaemonSet FLP config so agents verify the collector's certificate
On non-OpenShift clusters, everything works without TLS as before

Honor the cluster TLS security profile

Both ends previously hardcoded MinVersion: TLS 1.3. The connection now derives min version / cipher suites / curves from the OpenShift tlsSecurityProfile (apiservers.config.openshift.io/cluster), like every other netobserv component — no hardcoded version
A new resolve-tls subcommand runs as an initContainer on the collector pod: it reads the profile, resolves it, and writes the collector-tls-config ConfigMap. Both the collector container and the agent DaemonSet consume it via envFrom, so a single resolver run drives both ends
When the cluster has no explicit profile, it falls back to the Intermediate preset, mirroring the operator's default (no invented default)
Install order flips on the TLS path (collector first, then agents) so the ConfigMap exists before agents start; agents reference it with optional: false to fail loudly rather than silently downgrade
OpenShift-only, flows/packets only; metrics, --yaml, and non-OCP runs are unchanged

Dependencies

n/a

Checklist

Does the changes in PR need specific configuration or environment set up for testing?
- if so please describe it in PR description.
I have added thorough unit tests for the change.
QE requirements (check 1 from the list):
- Standard QE validation, with pre-merge tests unless stated otherwise.
- Regression tests only (e.g. refactoring with no user-facing change).
- No QE (e.g. trivial change with high reviewer's confidence, or per agreement with the QE team).

openshift-ci-robot · 2026-08-11T22:18:23Z

openshift-ci · 2026-08-11T22:18:25Z

[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 memodi 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:

DOWNSTREAM_OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

jpinsonneau · 2026-08-25T14:35:53Z

 	sigs.k8s.io/yaml v1.6.0 // indirect
 )
+
+replace github.com/netobserv/flowlogs-pipeline => github.com/leandroberetta/flowlogs-pipeline v0.0.0-20260810170916-6c5c94ab0294


Don't forget to remove this :)

jpinsonneau · 2026-08-25T14:39:54Z

that deserve at least a unit test

jpinsonneau · 2026-08-25T14:41:31Z

+  if [[ "$tlsEnabled" == "true" ]]; then
+    cmd="${K8S_CLI_BIN} run -n $namespace collector \\
+    --image=$img --image-pull-policy='Always' --restart='Never' \\
+    --override-type=strategic \\
+    --overrides=$overrides \\
+    --command -- $runCommand"
+  else
+    cmd="${K8S_CLI_BIN} run -n $namespace collector \\
+    --image=$img --image-pull-policy='Always' --restart='Never' \\
+    --overrides=$overrides \\
+    --command -- $runCommand"
+  fi


Suggested change

if [[ "$tlsEnabled" == "true" ]]; then

cmd="${K8S_CLI_BIN} run -n $namespace collector \\

--image=$img --image-pull-policy='Always' --restart='Never' \\

--override-type=strategic \\

--overrides=$overrides \\

--command -- $runCommand"

else

cmd="${K8S_CLI_BIN} run -n $namespace collector \\

--image=$img --image-pull-policy='Always' --restart='Never' \\

--overrides=$overrides \\

--command -- $runCommand"

fi

overrideType=""

if [[ "$tlsEnabled" == "true" ]]; then

overrideType="--override-type=strategic"

fi

cmd="${K8S_CLI_BIN} run -n $namespace collector \

--image=$img --image-pull-policy='Always' --restart='Never' \

$overrideType --overrides=$overrides \

--command -- $runCommand"

jpinsonneau · 2026-08-25T14:43:55Z

We should simplify this to something like:

# Create collector service for flows/packets captures if [[ "$command" = "flows" || "$command" = "packets" ]]; then echo "creating collector service" applyYAML "$collectorServiceYAML" if [[ "$tlsEnabled" == "true" ]]; then echo "creating CA configmap for TLS" createCAConfigMap fi fi if [ "$command" = "flows" ]; then echo "creating flow-capture agents" elif [ "$command" = "packets" ]; then echo "creating packet-capture agents" elif [ "$command" = "metrics" ]; then echo "creating service monitor" applyYAML "$smYAML" echo "creating metric-capture agents:"

jpinsonneau · 2026-08-25T14:46:24Z

+function isOpenShift() {
+  ${K8S_CLI_BIN} get clusterversion version &>/dev/null
+}


You should rely on checkClusterVersion here instead.

Feel free to add a global variable like isOCP in it for your usage 😉

jpinsonneau · 2026-08-25T14:48:12Z

 	golang.org/x/tools v0.45.0 // indirect
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20260526163538-3dc84a4a5aaa // indirect
-	google.golang.org/grpc v1.81.1 // indirect
+	google.golang.org/grpc v1.82.0


nit: Is that needed here ?

It's a direct dependency now: collector_tls.go uses grpc.Creds(credentials.NewTLS(...)) to enable TLS on the collector, so grpc is imported directly rather than transitively.

leandroberetta · 2026-09-01T13:50:37Z

@jpinsonneau I addressed the feedback, the only missing one is the dependency update, I'm waiting to merge this: netobserv/flowlogs-pipeline#1297

openshift-ci-robot · 2026-09-02T13:46:14Z

@leandroberetta: This pull request references NETOBSERV-2515 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 epic to target either version "5.1.0." or "openshift-5.1.0.", but it targets "netobserv-2.0" instead.

Details

In response to this:

Description

Enable TLS for the collector↔agent gRPC connection when running on OpenShift, and make that connection honor the cluster's TLS security profile.

TLS enablement (service-ca)

Enable TLS for the gRPC collector server on OpenShift, using service-ca for automatic cert generation

Conditionally detect OpenShift and annotate the collector Service for cert generation, create a CA ConfigMap with inject-cabundle, and mount certs in the collector pod

Add CA volume and tls.caCertPath to the agent DaemonSet FLP config so agents verify the collector's certificate

On non-OpenShift clusters, everything works without TLS as before

Honor the cluster TLS security profile

Both ends previously hardcoded MinVersion: TLS 1.3. The connection now derives min version / cipher suites / curves from the OpenShift tlsSecurityProfile (apiservers.config.openshift.io/cluster), like every other netobserv component — no hardcoded version

A new resolve-tls subcommand runs as an initContainer on the collector pod: it reads the profile, resolves it, and writes the collector-tls-config ConfigMap. Both the collector container and the agent DaemonSet consume it via envFrom, so a single resolver run drives both ends

When the cluster has no explicit profile, it falls back to the Intermediate preset, mirroring the operator's default (no invented default)

Install order flips on the TLS path (collector first, then agents) so the ConfigMap exists before agents start; agents reference it with optional: false to fail loudly rather than silently downgrade

OpenShift-only, flows/packets only; metrics, --yaml, and non-OCP runs are unchanged

Dependencies

n/a

Checklist

Does the changes in PR need specific configuration or environment set up for testing?

if so please describe it in PR description.

I have added thorough unit tests for the change.

QE requirements (check 1 from the list):

Standard QE validation, with pre-merge tests unless stated otherwise.

Regression tests only (e.g. refactoring with no user-facing change).

No QE (e.g. trivial change with high reviewer's confidence, or per agreement with the QE team).

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.

The gRPC connection between the capture agent (FLP client) and the CLI collector (gRPC server) hardcoded MinVersion: TLS 1.3 on both ends. Every other netobserv component derives its TLS settings from the OpenShift tlsSecurityProfile (apiservers.config.openshift.io/cluster); the CLI now does the same instead of pinning a version. A new `resolve-tls` subcommand runs as an initContainer on the collector pod. It reads the cluster's tlsSecurityProfile, resolves it to concrete min version / cipher suites / curves (falling back to the Intermediate preset when no profile is set, mirroring the operator's default), and writes them into the `collector-tls-config` ConfigMap. Both the collector container and the agent DaemonSet consume that ConfigMap via envFrom, so a single resolver run drives both ends. The collector server applies the resolved settings through flowlogs-pipeline's tlsprofile.Apply. Because the ConfigMap only exists once the collector's init completes, the install order flips on the TLS path: the collector is created and waited on first, then the agents (which reference the ConfigMap with optional:false, failing loudly rather than silently downgrading TLS). This path stays OpenShift-only and applies to flows/packets captures; metrics, --yaml output and non-OCP runs are unchanged. - internal/pkg/tlsresolver: profile resolution + ConfigMap write, with tests - cmd/resolve_tls.go, cmd/root.go: new resolve-tls subcommand - cmd/collector_tls.go: drop hardcoded TLS 1.3, apply resolved profile - res/service-account.yml: RBAC to read apiservers/cluster and write the CM - commands/netobserv, scripts/functions.sh: initContainer, envFrom, ordering - go.mod: add openshift/api and openshift/library-go/pkg/crypto Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

openshift-ci-robot · 2026-09-02T13:55:01Z

@leandroberetta: This pull request references NETOBSERV-2977 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 story to target the "5.1.0" version, but no target version was set.

Details

In response to this:

Description

Enable TLS for the collector↔agent gRPC connection when running on OpenShift, and make that connection honor the cluster's TLS security profile.

TLS enablement (service-ca)

Enable TLS for the gRPC collector server on OpenShift, using service-ca for automatic cert generation

Conditionally detect OpenShift and annotate the collector Service for cert generation, create a CA ConfigMap with inject-cabundle, and mount certs in the collector pod

Add CA volume and tls.caCertPath to the agent DaemonSet FLP config so agents verify the collector's certificate

On non-OpenShift clusters, everything works without TLS as before

Honor the cluster TLS security profile

Both ends previously hardcoded MinVersion: TLS 1.3. The connection now derives min version / cipher suites / curves from the OpenShift tlsSecurityProfile (apiservers.config.openshift.io/cluster), like every other netobserv component — no hardcoded version

A new resolve-tls subcommand runs as an initContainer on the collector pod: it reads the profile, resolves it, and writes the collector-tls-config ConfigMap. Both the collector container and the agent DaemonSet consume it via envFrom, so a single resolver run drives both ends

When the cluster has no explicit profile, it falls back to the Intermediate preset, mirroring the operator's default (no invented default)

Install order flips on the TLS path (collector first, then agents) so the ConfigMap exists before agents start; agents reference it with optional: false to fail loudly rather than silently downgrade

OpenShift-only, flows/packets only; metrics, --yaml, and non-OCP runs are unchanged

Dependencies

n/a

Checklist

Does the changes in PR need specific configuration or environment set up for testing?

if so please describe it in PR description.

I have added thorough unit tests for the change.

QE requirements (check 1 from the list):

Standard QE validation, with pre-merge tests unless stated otherwise.

Regression tests only (e.g. refactoring with no user-facing change).

No QE (e.g. trivial change with high reviewer's confidence, or per agreement with the QE team).

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.

The TLS profile resolver maps the OpenShift SecP256r1MLKEM768 / SecP384r1MLKEM1024 groups to their crypto/tls.CurveID constants, which are Go 1.26+. CI (setup-go 1.26), the Dockerfile builder (golang:1.26) and the operator (go 1.26.3) are already on 1.26; only the go.mod directive lagged at 1.25.7, which made govet's stdversion reject those constants and the exhaustive linter reject dropping them. Bump the directive to 1.26.0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

agentManifest is set in functions.sh setup() but consumed in commands/netobserv (applied after the collector is ready). shellcheck analyzes each file in isolation, so it flags the assignment as unused; add the same disable directive the file already uses elsewhere. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Moving the resolve-tls initContainer's namespaced Role and RoleBinding to the end of res/service-account.yml keeps the existing service-account document indices stable, but still adds two documents to every generated capture manifest. Update the positional assertions in the flow, packet and metric YAML e2e tests accordingly: - bump the expected document counts (8 -> 10 for flows/packets, 12 -> 14 for metrics), - assert the new Role (configmaps get/create/update) at index [6] and RoleBinding (ServiceAccount -> Role netobserv-cli) at index [7], - assert the new config.openshift.io/apiservers get rule on the netobserv-cli ClusterRole, - shift the collector Service / DaemonSet and the metric-specific documents to their new indices. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

openshift-ci · 2026-09-02T16:01:41Z

@leandroberetta: The following test 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/integration-tests	`90e2ebe`	link	true	`/test integration-tests`

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 · 2026-09-02T16:01:50Z

PR needs rebase.

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.

openshift-ci-robot added the jira/valid-reference label Aug 11, 2026

leandroberetta mentioned this pull request Aug 11, 2026

NETOBSERV-2515: Implement dynamic TLS configuration from OpenShift API Server netobserv/netobserv-operator#2823

Merged

6 tasks

leandroberetta requested review from jotak and jpinsonneau August 20, 2026 14:04

jpinsonneau reviewed Aug 25, 2026

View reviewed changes

openshift-ci Bot added the needs-rebase label Aug 26, 2026

leandroberetta force-pushed the netobserv-2515 branch from f86c30c to 54b1101 Compare August 31, 2026 12:39

openshift-ci Bot removed the needs-rebase label Aug 31, 2026

leandroberetta force-pushed the netobserv-2515 branch from 47414b5 to 565fb2e Compare September 2, 2026 12:15

openshift-ci Bot added the needs-rebase label Sep 2, 2026

leandroberetta and others added 4 commits September 2, 2026 10:47

grpc writer with tls in openshift

bc15c36

feedback addressed

1d3fbf0

update flp dependency

1c48dea

leandroberetta force-pushed the netobserv-2515 branch from fd8151c to 8956222 Compare September 2, 2026 13:50

openshift-ci Bot removed the needs-rebase label Sep 2, 2026

leandroberetta changed the title ~~NETOBSERV-2515: Add TLS support for collector when OpenShift~~ NETOBSERV-2977: Add TLS support for collector when OpenShift Sep 2, 2026

leandroberetta and others added 3 commits September 2, 2026 10:59

openshift-ci Bot added the needs-rebase label Sep 2, 2026

Uh oh!

Conversation

leandroberetta commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Dependencies

Checklist

Uh oh!

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

Uh oh!

Description

Test plan

Dependencies

Checklist

Uh oh!

openshift-ci Bot commented Aug 11, 2026

Uh oh!

jpinsonneau Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

jpinsonneau Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

jpinsonneau Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

jpinsonneau Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

jpinsonneau Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

jpinsonneau Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

leandroberetta Sep 1, 2026

Choose a reason for hiding this comment

Uh oh!

leandroberetta commented Sep 1, 2026

Uh oh!

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

Uh oh!

Description

Dependencies

Checklist

Uh oh!

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

Uh oh!

Description

Dependencies

Checklist

Uh oh!

openshift-ci Bot commented Sep 2, 2026

Uh oh!

openshift-ci Bot commented Sep 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

leandroberetta commented Aug 11, 2026 •

edited

Loading

openshift-ci-robot commented Aug 11, 2026 •

edited by openshift-ci Bot

Loading

openshift-ci-robot commented Sep 2, 2026 •

edited by openshift-ci Bot

Loading

openshift-ci-robot commented Sep 2, 2026 •

edited by openshift-ci Bot

Loading