OCPBUGS-61038: SSL Medium Strength Cipher Suites Supported for operator #1911
Conversation
WalkthroughAdds a kube-apiserver-operator-config ConfigMap with servingInfo TLS settings, syncs that ConfigMap from the operator to the target namespace via ResourceSyncController, registers it in CertConfigMaps for static kube-apiserver resources, and passes a --config argument to kube-apiserver-check-endpoints to load the synced config. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Manifest as Manifest (ConfigMap)
participant OperNS as Operator Namespace
participant RSC as ResourceSyncController
participant TargetNS as Target Namespace
participant APIPod as kube-apiserver Pod
participant CkEp as check-endpoints Container
Manifest->>OperNS: Apply `kube-apiserver-operator-config` (servingInfo TLS)
OperNS->>RSC: RSC observes ConfigMap
RSC->>TargetNS: Sync `kube-apiserver-operator-config` -> target namespace
Note right of RSC: contains cipherSuites & minTLSVersion
TargetNS->>APIPod: Mount synced ConfigMap into static-pod-certs path
APIPod->>CkEp: Start with `--config=/etc/kubernetes/static-pod-certs/configmaps/kube-apiserver-operator-config/config.yaml`
CkEp->>CkEp: Load TLS settings and perform endpoint checks
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lance5890 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Hi @lance5890. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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
bot
added
the
needs-ok-to-test
lance5890
force-pushed
the
support_safe_cipherSuites
branch
from
9fb1d1d to
c10fe5a
Compare
lance5890
changed the title
openshift-ci-robot
added
jira/severity-important
|
@lance5890: This pull request references Jira Issue OCPBUGS-61038, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. In response to this: 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
added
the
jira/invalid-bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/operator/starter.go (1)
663-666: Include operator config CM in Unrevisioned resources — OK.This ensures the file is mounted for check-endpoints. Consider calling out in a code comment that changes here won’t restart the static pod; config becomes effective on next restart.
-// kube-apiserver-operator-config(contains safe cipherSuites and minTLSVersion) for check-endpoints +// kube-apiserver-operator-config (TLS cipherSuites/minTLSVersion for check-endpoints). +// Note: Unrevisioned; updates take effect on next pod restart.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
bindata/assets/kube-apiserver/pod.yaml(1 hunks)manifests/0000_20_kube-apiserver-operator_03_configmap.yaml(1 hunks)pkg/operator/resourcesynccontroller/resourcesynccontroller.go(1 hunks)pkg/operator/starter.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
pkg/operator/resourcesynccontroller/resourcesynccontroller.go (3)
vendor/github.com/openshift/library-go/pkg/operator/resource/resourceapply/core.go (1)
SyncConfigMap(465-467)vendor/github.com/openshift/library-go/pkg/operator/resourcesynccontroller/interfaces.go (1)
ResourceLocation(6-13)pkg/operator/operatorclient/interfaces.go (2)
TargetNamespace(7-7)OperatorNamespace(6-6)
🔇 Additional comments (2)
manifests/0000_20_kube-apiserver-operator_03_configmap.yaml (1)
14-22: Strong TLS suites locked; config parsing and wiring confirmed
- GenericOperatorConfig.servingInfo (minTLSVersion, cipherSuites) is synced by ResourceSyncController and passed to check-endpoints via
--config /etc/kubernetes/static-pod-certs/configmaps/kube-apiserver-operator-config/config.yaml.- ConfigMap name/path (
kube-apiserver-operator-config/config.yaml) is consistent across manifests, operator deployment, and static-pod assets.- Manual FIPS cluster validation required: BoringCrypto should silently ignore unsupported CHACHA20 suites without causing failures.
pkg/operator/resourcesynccontroller/resourcesynccontroller.go (1)
98-105: Approve SyncConfigMap usage. Verified that the only SyncConfigMap invocation for “kube-apiserver-operator-config” uses the correct destination (TargetNamespace) ← source (OperatorNamespace) ordering and no duplicate syncs exist.
| - --config | ||
| - /etc/kubernetes/static-pod-certs/configmaps/kube-apiserver-operator-config/config.yaml |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Pass --config to check-endpoints matches the synced mount path.
The path aligns with CertConfigMaps sync to /etc/kubernetes/static-pod-certs/configmaps/kube-apiserver-operator-config/config.yaml. Please ensure the flag is mandatory-tolerant (does not crash if the file is temporarily absent during cert-sync) and logs the applied TLS params.
🏁 Script executed:
#!/bin/bash
# Verify no other places start check-endpoints without --config
rg -nP 'check-endpoints\b' -g '!vendor' -C2 | rg -v -- '--config'Length of output: 16368
Handle missing --config file gracefully and log TLS parameters (cipherSuites/minTLSVersion): The only check-endpoints invocation is in bindata/assets/kube-apiserver/pod.yaml with --config. In pkg/cmd/checkendpoints, wrap loading of the --config path so a temporarily absent file does not crash the binary and emit the effective cipherSuites and minTLSVersion on startup.
🤖 Prompt for AI Agents
In bindata/assets/kube-apiserver/pod.yaml around lines 242-243 and in
pkg/cmd/checkendpoints, the current invocation passes a --config path that may
be temporarily missing which will crash the binary; update the checkendpoints
command to attempt to read and parse the provided --config file inside a guarded
block that treats a missing file as non-fatal (log a warning and continue with
defaults) and only returns a fatal error for unrecoverable parse errors, and
after configuration is loaded (or defaults chosen) emit an info log stating the
effective TLS parameters including cipherSuites and minTLSVersion so operators
can see what TLS settings are in use at startup.
Signed-off-by: lan.tian <[email protected]>
lance5890
force-pushed
the
support_safe_cipherSuites
branch
from
c10fe5a to
c0cbbda
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/operator/starter.go (1)
664-665: Confirm intended update semantics; consider revisioning if config must change atomically with KAS.If check-endpoints only reads the file at startup, updates to cipher suites won’t take effect until the next pod restart. If you need the TLS policy to be strictly bound to each revision (to avoid transient mismatch with kube-apiserver), consider moving this CM to RevisionConfigMaps instead of CertConfigMaps. Otherwise, document that changes take effect on next rollout.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
bindata/assets/kube-apiserver/pod.yaml(1 hunks)manifests/0000_20_kube-apiserver-operator_03_configmap.yaml(1 hunks)pkg/operator/resourcesynccontroller/resourcesynccontroller.go(1 hunks)pkg/operator/starter.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- bindata/assets/kube-apiserver/pod.yaml
- manifests/0000_20_kube-apiserver-operator_03_configmap.yaml
- pkg/operator/resourcesynccontroller/resourcesynccontroller.go
🔇 Additional comments (1)
pkg/operator/starter.go (1)
664-665: LGTM: registering kube-apiserver-operator-config as an unrevisioned CM fits the static-pod cert-syncer model.This enables check-endpoints to read TLS policy from a well-known mount path. Name and comment look consistent.
| // kube-apiserver-operator-config (TLS cipherSuites/minTLSVersion for check-endpoints). | ||
| {Name: "kube-apiserver-operator-config"}, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify resource-sync direction and asset presence to avoid rollout races.
The pod now relies on /etc/kubernetes/static-pod-certs/configmaps/kube-apiserver-operator-config/config.yaml. Ensure:
- The ConfigMap is created by manifests before the pod revision that adds --config.
- ResourceSyncController copies it into the namespace that cert-syncer watches for CertConfigMaps (TargetNamespace).
Run to confirm sources/targets and the pod arg:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Where is the CM created?
rg -nC2 'kube-apiserver-operator-config' manifests || true
rg -nC2 'kube-apiserver-operator-config' bindata || true
# 2) Verify ResourceSyncController maps it to the correct namespace (expect: operator -> target or vice versa, as intended).
rg -nC5 'SyncConfigMap\(.+kube-apiserver-operator-config' pkg || true
# 3) Confirm the pod.yaml uses the expected path/filename.
rg -nC3 '/etc/kubernetes/static-pod-certs/configmaps/kube-apiserver-operator-config/config\.yaml' bindata/assets/kube-apiserver/pod.yamlLength of output: 2034
Configure ResourceSyncController to sync kube-apiserver-operator-config ConfigMap. The static pod now mounts /etc/kubernetes/static-pod-certs/configmaps/kube-apiserver-operator-config/config.yaml, but no SyncConfigMap call exists to copy that CM into the cert-syncer’s target namespace.
🤖 Prompt for AI Agents
In pkg/operator/starter.go around lines 664-665, the ResourceSyncController list
includes "kube-apiserver-operator-config" but no SyncConfigMap call is added;
add a SyncConfigMap registration to copy the kube-apiserver-operator-config
ConfigMap from its source namespace into the cert-syncer’s target namespace so
the static pod path
/etc/kubernetes/static-pod-certs/configmaps/kube-apiserver-operator-config/config.yaml
is populated. Ensure you call the controller's SyncConfigMap method with the
correct source namespace/name and the cert-syncer destination namespace, wire it
into the existing resourceSyncController setup, and handle returned errors
consistently with other SyncConfigMap registrations.
The
kube-apiserver-check-endpointsin kube-apiserver static pod which usekube-apiserver-operator check-endpoints, but with unsafe cipherSuites, we should use the safe cipherSuites to be compatible with thekube-apiservercipherSuitesThis PR try to add the
kube-apiserver-operator-configto the UnrevisionedResource, maybe should we add that to the RevisionedResource（I will post it in the other PR）Summary by CodeRabbit
New Features
Chores