OCPBUGS-99014: Authenticate Konnectivity agents with cluster CA - #9031
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@hlipsig: This pull request explicitly references no jira issue. DetailsIn 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. |
|
Skipping CI for Draft Pull Request. |
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (6)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe 🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
control-plane-operator/controllers/hostedcontrolplane/v2/assets/assets_test.go (1)
55-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the required “When ... it should ...” test description format.
Rename or wrap this test so its case description follows the repository convention, for example
When configured with the cluster CA, it should authenticate agents.🤖 Prompt for 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. In `@control-plane-operator/controllers/hostedcontrolplane/v2/assets/assets_test.go` at line 55, Rename TestKonnectivityServerAuthenticatesAgents to use the required “When ..., it should ...” test description convention, such as describing the cluster CA configuration and expected agent authentication behavior.Source: Coding guidelines
🤖 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
`@control-plane-operator/controllers/hostedcontrolplane/v2/assets/assets_test.go`:
- Around line 59-60: Update the test around
LoadDeploymentManifest("kube-apiserver") to stop execution immediately when the
lookup returns an error or nil deployment, before any deployment.Spec
dereference; use the test’s fatal/early-return mechanism and apply the same
guard before dereferencing konnectivityServer.Args.
- Around line 71-74: Update the konnectivityServer.Args assertion in the
relevant test to verify that “--cluster-ca-cert” is immediately followed by
“/etc/konnectivity/ca/ca.crt”, using an ordered-pair or exact-subsequence
assertion instead of ContainElements.
---
Nitpick comments:
In
`@control-plane-operator/controllers/hostedcontrolplane/v2/assets/assets_test.go`:
- Line 55: Rename TestKonnectivityServerAuthenticatesAgents to use the required
“When ..., it should ...” test description convention, such as describing the
cluster CA configuration and expected agent authentication behavior.
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: fa825fbc-fa44-44fa-aec7-729246c79fbe
📒 Files selected for processing (2)
control-plane-operator/controllers/hostedcontrolplane/v2/assets/assets_test.gocontrol-plane-operator/controllers/hostedcontrolplane/v2/assets/kube-apiserver/deployment.yaml
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9031 +/- ##
=======================================
Coverage 44.44% 44.44%
=======================================
Files 774 774
Lines 96977 96977
=======================================
Hits 43105 43105
Misses 50897 50897
Partials 2975 2975
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
7af17d9 to
69928e6
Compare
|
/retitle OCPBUGS-99014: Authenticate Konnectivity agents with cluster CA |
|
@hlipsig: This pull request references Jira Issue OCPBUGS-99014, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn 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. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bryan-cox, hlipsig The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
The UPDATE=true go test ./control-plane-operator/controllers/hostedcontrolplane/...This will update all the |
|
The PR lgtm, pls fix the unit tests and I can tag. |
|
/lgtm |
|
Scheduling tests matching the |
|
/test e2e-aro-hcp |
|
/retest-required |
|
/lgtm |
|
/hold cancel hold was automatically put on from failing CI tests. |
| } | ||
| } | ||
|
|
||
| func TestKonnectivityServerAuthenticatesAgents(t *testing.T) { |
There was a problem hiding this comment.
why is this test here? it should be in the kube-apiserver pkg
|
@hlipsig: Jira Issue Verification Checks: Jira Issue OCPBUGS-99014 Jira Issue OCPBUGS-99014 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 DetailsIn 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. |
|
/cherry-pick release-4.22 |
|
@clebs: #9031 failed to apply on top of branch "release-4.22": DetailsIn 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 kubernetes-sigs/prow repository. |
Backport pull request openshift#9031 from hlipsig/fix-konnectivity-agent-auth OCPBUGS-99014: Authenticate Konnectivity agents with cluster CA Signed-off-by: Borja Clemente <bclement@redhat.com>
|
Fix included in release 5.0.0-0.nightly-2026-07-23-224236 |
Backport pull request openshift#9031 from hlipsig/fix-konnectivity-agent-auth OCPBUGS-99014: Authenticate Konnectivity agents with cluster CA Signed-off-by: Borja Clemente <bclement@redhat.com>
Backport pull request openshift#9031 from hlipsig/fix-konnectivity-agent-auth OCPBUGS-99014: Authenticate Konnectivity agents with cluster CA Signed-off-by: Borja Clemente <bclement@redhat.com>
Backport pull request openshift#9031 from hlipsig/fix-konnectivity-agent-auth OCPBUGS-99014: Authenticate Konnectivity agents with cluster CA Signed-off-by: Borja Clemente <bclement@redhat.com>
Backport pull request openshift#9031 from hlipsig/fix-konnectivity-agent-auth OCPBUGS-99014: Authenticate Konnectivity agents with cluster CA Signed-off-by: Borja Clemente <bclement@redhat.com>
Backport pull request #9031 from hlipsig/fix-konnectivity-agent-auth OCPBUGS-99014: Authenticate Konnectivity agents with cluster CA Signed-off-by: Borja Clemente <bclement@redhat.com>
Backport pull request #9031 from hlipsig/fix-konnectivity-agent-auth OCPBUGS-99014: Authenticate Konnectivity agents with cluster CA Signed-off-by: Borja Clemente <bclement@redhat.com>
What this PR does / why we need it:
The Konnectivity proxy-server currently accepts agent connections without validating client certificates because its agent listener is not configured with a cluster CA. A reachable client can therefore connect without a certificate signed by the hosted cluster's Konnectivity CA.
Configure
--cluster-ca-certwith the existing mounted Konnectivity CA bundle. Legitimate agents already use certificates signed by this CA. Add a regression test that verifies the embedded kube-apiserver manifest retains the authentication setting.Which issue(s) this PR fixes:
OCPBUGS-99014
Special notes for your reviewer:
Focused tests passed:
GO111MODULE=on GOWORK=off GOFLAGS=-mod=vendor go test -race ./control-plane-operator/controllers/hostedcontrolplane/v2/assets ./control-plane-operator/controllers/hostedcontrolplane/v2/kasChecklist:
Summary by CodeRabbit
--cluster-ca-certargument pointing to the mounted Konnectivity CA file.--cluster-ca-certsetting.