Skip to content

CCO-848: Add tls-min-version and tls-cipher-suites CLI flags to CCO#1063

Open
dlom wants to merge 1 commit into
openshift:masterfrom
dlom:CCO-848
Open

CCO-848: Add tls-min-version and tls-cipher-suites CLI flags to CCO#1063
dlom wants to merge 1 commit into
openshift:masterfrom
dlom:CCO-848

Conversation

@dlom

@dlom dlom commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

xref: CCO-848
slack thread: https://redhat-internal.slack.com/archives/CBZHF4DHC/p1782826611730719

Summary by CodeRabbit

  • New Features

    • Added CLI options to override TLS minimum version and cipher suites at startup.
    • TLS behavior is now driven by the provided overrides when enabled, including webhook TLS argumenting derived from those values.
  • Bug Fixes

    • Improved TLS initialization so secure serving and webhook setup share consistent TLS configuration logic.
    • When TLS is overridden via CLI, TLS change watching is disabled to prevent unintended reconciliations; webhook deployment now updates correctly under override mode.

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

openshift-ci-robot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

@dlom: This pull request references CCO-848 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 "5.0.0" version, but no target version was set.

Details

In response to this:

xref: CCO-848
slack thread: https://redhat-internal.slack.com/archives/CBZHF4DHC/p1782826611730719

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.

@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 558696f0-400c-4790-84e1-76785963e8b6

📥 Commits

Reviewing files that changed from the base of the PR and between 2094d85 and 4ac3f86.

📒 Files selected for processing (3)
  • pkg/cmd/operator/cmd.go
  • pkg/operator/controller.go
  • pkg/operator/podidentity/podidentitywebhook_controller.go
🚧 Files skipped from review as they are similar to previous changes (3)
  • pkg/operator/controller.go
  • pkg/cmd/operator/cmd.go
  • pkg/operator/podidentity/podidentitywebhook_controller.go

Walkthrough

This PR adds CLI flags for overriding TLS minimum version and cipher suites in the operator command, centralizes TLS setup in resolveTLSConfig, threads TLS settings through controller registration into pod identity setup, and changes webhook reconciliation to use either CLI-derived TLS arguments or cluster-derived profile substitutions.

Changes

TLS override wiring

Layer / File(s) Summary
CLI flags and TLS resolution
pkg/cmd/operator/cmd.go
Adds TLSMinVersion and TLSCipherSuites options, registers --tls-min-version and --tls-cipher-suites, and resolves TLS config through resolveTLSConfig using either CLI inputs or cluster state.
Controller wiring with TLS overrides
pkg/operator/controller.go, pkg/cmd/operator/cmd.go
Updates controller startup to pass TLS override inputs through AddToManager, removes implicit pod identity registration from the default list, and calls podidentity.AddWithTLS explicitly.
Pod identity TLS-aware setup
pkg/operator/podidentity/podidentitywebhook_controller.go
Adds AddWithTLS, carries TLS override state into reconciliation, branches TLS profile initialization on flags vs. API server state, and appends TLS command-line args when CLI overrides are enabled.

Estimated code review effort: 4 (Complex) | ~45 minutes

🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: adding tls-min-version and tls-cipher-suites CLI flags to CCO.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed PR diff touches only non-test files; no *_test.go or Ginkgo titles were introduced.
Test Structure And Quality ✅ Passed No *_test.go or Ginkgo changes were in the PR, so the test-structure check is not applicable.
Microshift Test Compatibility ✅ Passed No *_test.go files changed, and the touched files add no Ginkgo tests or MicroShift-sensitive API use.
Single Node Openshift (Sno) Test Compatibility ✅ Passed No new Ginkgo e2e tests were added; the patch only changes operator/controller implementation files, so SNO test compatibility is not applicable.
Topology-Aware Scheduling Compatibility ✅ Passed PASS: The touched controller code only gates deployment/PDB behavior on ControlPlaneTopology (External/SingleReplica) and adds no nodeSelector, affinity, or spread constraints.
Ote Binary Stdout Contract ✅ Passed No new stdout writes appear in main/init/TestMain code; the added startup paths use logrus/stderr, and the only stdout redirection found is the existing Go log->glog setup.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed No Ginkgo e2e tests were added or modified; the PR only changes CLI/TLS operator code and controller wiring.
No-Weak-Crypto ✅ Passed No hardcoded weak crypto or insecure secret compares were added; the diff only wires user-configurable TLS flags into Go tls.Config.
Container-Privileges ✅ Passed No privilege-related fields were added; manifests still use runAsNonRoot:true and allowPrivilegeEscalation:false, and changed code only wires TLS flags.
No-Sensitive-Data-In-Logs ✅ Passed New logs only report TLS config/policy state and control flow; no passwords, tokens, PII, hostnames, or customer data are logged.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@openshift-ci openshift-ci Bot requested a review from jstuever July 7, 2026 19:18
@openshift-ci

openshift-ci Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dlom

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 7, 2026
@dlom

dlom commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

/jira refresh

@openshift-ci-robot

openshift-ci-robot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

@dlom: This pull request references CCO-848 which is a valid jira issue.

Details

In response to this:

/jira refresh

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with 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.

Inline comments:
In `@pkg/operator/podidentity/podidentitywebhook_controller.go`:
- Around line 243-248: The TLS profile reads in AddToManager/AddWithTLS are
using a non-cancellable startup context, so operator registration can’t be
interrupted or time-bounded if the apiserver hangs. Thread the startup context
from the caller into AddToManager/AddWithTLS, or create a bounded derived
context there, and pass it into utiltls.FetchAPIServerTLSAdherencePolicy and
utiltls.FetchAPIServerTLSProfile so these startup API calls respect cancellation
and timeouts.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 1815426b-3c0a-4fd6-97ce-7e9a4d88fced

📥 Commits

Reviewing files that changed from the base of the PR and between 9710974 and 2094d85.

📒 Files selected for processing (3)
  • pkg/cmd/operator/cmd.go
  • pkg/operator/controller.go
  • pkg/operator/podidentity/podidentitywebhook_controller.go

Comment thread pkg/operator/podidentity/podidentitywebhook_controller.go
@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 1.94175% with 101 lines in your changes missing coverage. Please review.
✅ Project coverage is 46.82%. Comparing base (9710974) to head (4ac3f86).

Files with missing lines Patch % Lines
pkg/cmd/operator/cmd.go 0.00% 65 Missing ⚠️
...rator/podidentity/podidentitywebhook_controller.go 5.71% 31 Missing and 2 partials ⚠️
pkg/operator/controller.go 0.00% 3 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1063      +/-   ##
==========================================
- Coverage   47.03%   46.82%   -0.22%     
==========================================
  Files          97       97              
  Lines       12583    12643      +60     
==========================================
+ Hits         5919     5920       +1     
- Misses       6008     6066      +58     
- Partials      656      657       +1     
Files with missing lines Coverage Δ
pkg/operator/controller.go 0.00% <0.00%> (ø)
...rator/podidentity/podidentitywebhook_controller.go 41.94% <5.71%> (-2.99%) ⬇️
pkg/cmd/operator/cmd.go 0.00% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dlom

dlom commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

/override ci/prow/security

@dlom

dlom commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

/test e2e-hypershift

@openshift-ci

openshift-ci Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

@dlom: Overrode contexts on behalf of dlom: ci/prow/security

Details

In response to this:

/override ci/prow/security

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.

@dlom

dlom commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

/test e2e-hypershift

@openshift-ci

openshift-ci Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

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

@dlom

dlom commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

/payload 5.0 nightly informing

@openshift-ci

openshift-ci Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

@dlom: trigger 69 job(s) of type informing for the nightly release of OCP 5.0

  • periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-upgrade-fips
  • periodic-ci-openshift-hypershift-release-5.0-periodics-e2e-azure-aks-ovn-conformance
  • periodic-ci-openshift-release-main-nightly-5.0-console-aws
  • periodic-ci-openshift-cluster-control-plane-machine-set-operator-release-5.0-periodics-e2e-aws
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-csi
  • periodic-ci-openshift-release-main-ci-5.0-e2e-aws-ovn
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-cgroupsv2
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-fips
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-single-node
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-single-node-csi
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-single-node-serial
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-single-node-techpreview
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-single-node-techpreview-serial
  • periodic-ci-openshift-release-main-nightly-5.0-upgrade-from-stable-4.22-e2e-aws-upgrade-ovn-single-node
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-upgrade-fips-no-nat-instance
  • periodic-ci-openshift-release-main-ci-5.0-e2e-aws-ovn-upgrade-out-of-change
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-upi
  • periodic-ci-openshift-cluster-control-plane-machine-set-operator-release-5.0-periodics-e2e-azure
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-azure-csi
  • periodic-ci-openshift-release-main-ci-5.0-e2e-azure-ovn
  • periodic-ci-openshift-release-main-ci-5.0-e2e-azure-ovn-serial
  • periodic-ci-openshift-release-main-ci-5.0-e2e-azure-ovn-techpreview
  • periodic-ci-openshift-release-main-ci-5.0-e2e-azure-ovn-techpreview-serial-1of2
  • periodic-ci-openshift-release-main-ci-5.0-e2e-azure-ovn-techpreview-serial-2of2
  • periodic-ci-openshift-release-main-ci-5.0-e2e-azure-ovn-upgrade-out-of-change
  • periodic-ci-openshift-release-main-cnv-nightly-5.0-deploy-azure-kubevirt-ovn
  • periodic-ci-openshift-cluster-control-plane-machine-set-operator-release-5.0-periodics-e2e-gcp
  • periodic-ci-openshift-release-main-ci-5.0-e2e-gcp-ovn
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-gcp-ovn-csi
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-gcp-ovn-rt
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-gcp-ovn-serial
  • periodic-ci-openshift-release-main-ci-5.0-e2e-gcp-ovn-techpreview
  • periodic-ci-openshift-release-main-ci-5.0-e2e-gcp-ovn-techpreview-serial-1of2
  • periodic-ci-openshift-release-main-ci-5.0-e2e-gcp-ovn-techpreview-serial-2of2
  • periodic-ci-openshift-release-main-ci-5.0-upgrade-from-stable-4.22-e2e-gcp-ovn-upgrade
  • periodic-ci-openshift-release-main-ci-5.0-e2e-gcp-ovn-upgrade
  • periodic-ci-openshift-hypershift-release-5.0-periodics-e2e-azure-kubevirt-ovn
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ipi-ovn-dualstack
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ipi-ovn-dualstack-techpreview
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ipi-ovn-ipv6-techpreview
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ipi-ovn-serial-ipv4
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ipi-ovn-serial-virtualmedia-1of2
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ipi-ovn-serial-virtualmedia-2of2
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ipi-ovn-techpreview
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ipi-ovn-upgrade
  • periodic-ci-openshift-release-main-nightly-5.0-upgrade-from-stable-4.22-e2e-metal-ipi-ovn-upgrade
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ipi-serial-ovn-ipv6
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ipi-serial-ovn-dualstack
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ipi-upgrade-ovn-ipv6
  • periodic-ci-openshift-release-main-nightly-5.0-upgrade-from-stable-4.22-e2e-metal-ipi-upgrade-ovn-ipv6
  • periodic-ci-openshift-release-main-nightly-5.0-metal-ovn-single-node-recert-cluster-rename
  • periodic-ci-openshift-microshift-release-5.0-periodics-e2e-aws-ovn-ocp-conformance
  • periodic-ci-openshift-microshift-release-5.0-periodics-e2e-aws-ovn-ocp-conformance-serial
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-osd-ccs-gcp
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-proxy
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ovn-single-node-live-iso
  • periodic-ci-openshift-eng-ocp-qe-perfscale-ci-main-aws-5.0-nightly-x86-payload-control-plane-6nodes
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-telco5g
  • periodic-ci-openshift-release-main-nightly-5.0-tls-scan
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-vsphere-ovn
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-vsphere-ovn-csi
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-vsphere-ovn-serial
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-vsphere-ovn-techpreview
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-vsphere-ovn-techpreview-serial
  • periodic-ci-openshift-release-main-ci-5.0-upgrade-from-stable-4.22-e2e-vsphere-ovn-upgrade
  • periodic-ci-openshift-release-main-ci-5.0-e2e-vsphere-ovn-upgrade
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-vsphere-ovn-upi
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-vsphere-ovn-upi-serial
  • periodic-ci-openshift-release-main-nightly-5.0-e2e-vsphere-static-ovn

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/a1e0bfa0-7a96-11f1-8cf1-80db4e21938d-0

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

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants