OCPBUGS-82036: Restrict anonymous auth to only probe endpoints#8133
OCPBUGS-82036: Restrict anonymous auth to only probe endpoints#8133smrtrfszm wants to merge 1 commit intoopenshift:mainfrom
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughadaptAuthConfig now always constructs and writes an AuthenticationConfiguration (including TypeMeta, an initialized empty JWT slice, and an Anonymous block) regardless of whether HCP authentication or OIDC providers are present. When OIDC providers exist, JWT authenticators are generated per provider and appended; errors return with provider context. anonymousAuthConfig produces the probe/metadata anonymous condition only when integrated OAuth is enabled. The kube-apiserver args no longer include an unconditional Sequence Diagram(s)sequenceDiagram
participant HCPController as HostedControlPlane Controller
participant CM as ConfigMap (auth.json)
participant KAS as Kube‑APIServer
HCPController->>HCPController: adaptAuthConfig(hcp)
alt OIDC providers present
HCPController->>HCPController: generateJWTForProvider(...) for each provider
HCPController->>HCPController: append JWT authenticators to authConfig.JWT
else no OIDC providers
HCPController->>HCPController: leave authConfig.JWT empty
end
HCPController->>HCPController: set authConfig.Anonymous via anonymousAuthConfig(hcp)
HCPController->>CM: write AuthenticationConfiguration (auth.json)
KAS->>CM: mount/read authentication-config (auth.json)
KAS->>KAS: load JWT authenticators and Anonymous conditions
KAS->>KAS: apply authentication behavior (probe-only anonymous paths as configured)
🚥 Pre-merge checks | ✅ 8 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (8 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
Skipping CI for Draft Pull Request. |
openshift-ci
bot
added
do-not-merge/needs-area
area/control-plane-operator
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: smrtrfszm The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
smrtrfszm
force-pushed
the
smrtrfszm/restrict-anonymous-auth
branch
from
aad2562 to
1f9878e
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8133 +/- ##
=======================================
Coverage 35.70% 35.71%
=======================================
Files 767 767
Lines 93401 93407 +6
=======================================
+ Hits 33353 33362 +9
+ Misses 57346 57344 -2
+ Partials 2702 2701 -1
🚀 New features to boost your workflow:
|
openshift-ci
bot
added
the
needs-rebase
smrtrfszm
force-pushed
the
smrtrfszm/restrict-anonymous-auth
branch
from
1f9878e to
d50476f
Compare
openshift-ci
bot
removed
the
needs-rebase
smrtrfszm
changed the title
openshift-ci-robot
added
jira/valid-reference
|
@smrtrfszm: This pull request references Jira Issue OCPBUGS-82036, 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. |
|
@smrtrfszm: This pull request references Jira Issue OCPBUGS-82036, which is valid. 3 validation(s) were run on this bug
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. |
|
/jira refresh |
|
@smrtrfszm: This pull request references Jira Issue OCPBUGS-82036, which is valid. 3 validation(s) were run on this bug
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. |
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@control-plane-operator/controllers/hostedcontrolplane/v2/kas/auth.go`:
- Around line 36-62: The IBM anonymous auth block is applied unconditionally for
IBMCloud (isIBMCloud) which can emit unsupported anonymous.conditions on guest
clusters older than K8s v1.34; instead, derive the guest Kubernetes semantic
version from the hosted control plane's release image (via cpContext/HCP release
image) and only set authConfig.Anonymous = ibmCloudAnonymousAuthConfig() when
the parsed guest version is >= 1.34.0; keep the existing
generateAuthConfig()/hasOIDCProviders flow and add a version-check helper (e.g.,
parse release image to semver and compare) before calling
ibmCloudAnonymousAuthConfig(), so clusters running <1.34 skip that assignment.
🪄 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), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: e08e43a8-b3c2-42e2-8fc5-65e797de73db
⛔ Files ignored due to path filters (3)
control-plane-operator/controllers/hostedcontrolplane/testdata/kube-apiserver/IBMCloud/zz_fixture_TestControlPlaneComponents_auth_config_configmap.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-apiserver/IBMCloud/zz_fixture_TestControlPlaneComponents_kas_config_configmap.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-apiserver/IBMCloud/zz_fixture_TestControlPlaneComponents_kube_apiserver_deployment.yamlis excluded by!**/testdata/**
📒 Files selected for processing (7)
control-plane-operator/controllers/hostedcontrolplane/v2/kas/auth.gocontrol-plane-operator/controllers/hostedcontrolplane/v2/kas/auth_test.gocontrol-plane-operator/controllers/hostedcontrolplane/v2/kas/auth_types.gocontrol-plane-operator/controllers/hostedcontrolplane/v2/kas/config.gocontrol-plane-operator/controllers/hostedcontrolplane/v2/kas/config_test.gocontrol-plane-operator/controllers/hostedcontrolplane/v2/kas/params.gocontrol-plane-operator/controllers/hostedcontrolplane/v2/kas/params_test.go
|
We already set the This PR isn't currently needed unless I'm missing something |
|
@sjenning |
|
Per our discussion, let's refactor this to make in general policy in hypershift, not just for IBMCloud. Well look at merging next week after branch cut. |
smrtrfszm
force-pushed
the
smrtrfszm/restrict-anonymous-auth
branch
from
d50476f to
3652163
Compare
smrtrfszm
changed the title
smrtrfszm
force-pushed
the
smrtrfszm/restrict-anonymous-auth
branch
from
3652163 to
b86d203
Compare
|
@smrtrfszm: This pull request references Jira Issue OCPBUGS-82036, which is valid. 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. |
|
/retest |
| Conditions: []AnonymousAuthCondition{ | ||
| {Path: "/healthz"}, | ||
| {Path: "/livez"}, | ||
| {Path: "/livez/ping"}, |
There was a problem hiding this comment.
FYI: Vanilla HyperShift does not support /livez/ping like ROKS does. There was disagreement between IBM and Red Hat on supporting this endpoint. Is Red Hat okay with us adding this endpoint? See #6202 for some context on this.
|
/test security |
|
/test e2e-aws-conformance |
|
/test e2e-conformance |
openshift-ci
bot
added
the
do-not-merge/work-in-progress
smrtrfszm
force-pushed
the
smrtrfszm/restrict-anonymous-auth
branch
from
b86d203 to
519b23d
Compare
openshift-ci-robot
added
jira/invalid-bug
|
@smrtrfszm: This pull request references Jira Issue OCPBUGS-82036, which is invalid:
Comment 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. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@control-plane-operator/controllers/hostedcontrolplane/v2/kas/auth.go`:
- Around line 72-85: The anonymousAuthConfig function currently unconditionally
includes "/.well-known/oauth-authorization-server" which exposes the OAuth
discovery document even when OAuth/ODIC is not enabled; update
anonymousAuthConfig (or its caller) to only append the
AnonymousAuthCondition{Path: "/.well-known/oauth-authorization-server"} when the
HostedControlPlane configuration indicates integrated OAuth is enabled (e.g.,
configuration.Authentication != nil and/or OIDCProviders not empty), otherwise
omit that condition so anonymous access remains limited to the probe endpoints;
ensure you reference AnonymousAuthConfig and AnonymousAuthCondition when
implementing the conditional addition.
🪄 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: Pro Plus
Run ID: b54ef1c6-c20b-460f-a497-cb2d1ad94110
⛔ Files ignored due to path filters (15)
control-plane-operator/controllers/hostedcontrolplane/testdata/kube-apiserver/AROSwift/zz_fixture_TestControlPlaneComponents_auth_config_configmap.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-apiserver/AROSwift/zz_fixture_TestControlPlaneComponents_kas_config_configmap.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-apiserver/AROSwift/zz_fixture_TestControlPlaneComponents_kube_apiserver_deployment.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-apiserver/GCP/zz_fixture_TestControlPlaneComponents_auth_config_configmap.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-apiserver/GCP/zz_fixture_TestControlPlaneComponents_kas_config_configmap.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-apiserver/GCP/zz_fixture_TestControlPlaneComponents_kube_apiserver_deployment.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-apiserver/IBMCloud/zz_fixture_TestControlPlaneComponents_auth_config_configmap.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-apiserver/IBMCloud/zz_fixture_TestControlPlaneComponents_kas_config_configmap.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-apiserver/IBMCloud/zz_fixture_TestControlPlaneComponents_kube_apiserver_deployment.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-apiserver/TechPreviewNoUpgrade/zz_fixture_TestControlPlaneComponents_auth_config_configmap.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-apiserver/TechPreviewNoUpgrade/zz_fixture_TestControlPlaneComponents_kas_config_configmap.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-apiserver/TechPreviewNoUpgrade/zz_fixture_TestControlPlaneComponents_kube_apiserver_deployment.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-apiserver/zz_fixture_TestControlPlaneComponents_auth_config_configmap.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-apiserver/zz_fixture_TestControlPlaneComponents_kas_config_configmap.yamlis excluded by!**/testdata/**control-plane-operator/controllers/hostedcontrolplane/testdata/kube-apiserver/zz_fixture_TestControlPlaneComponents_kube_apiserver_deployment.yamlis excluded by!**/testdata/**
📒 Files selected for processing (5)
control-plane-operator/controllers/hostedcontrolplane/v2/kas/auth.gocontrol-plane-operator/controllers/hostedcontrolplane/v2/kas/auth_test.gocontrol-plane-operator/controllers/hostedcontrolplane/v2/kas/auth_types.gocontrol-plane-operator/controllers/hostedcontrolplane/v2/kas/config.gocontrol-plane-operator/controllers/hostedcontrolplane/v2/kas/config_test.go
✅ Files skipped from review due to trivial changes (2)
- control-plane-operator/controllers/hostedcontrolplane/v2/kas/config_test.go
- control-plane-operator/controllers/hostedcontrolplane/v2/kas/auth_types.go
🚧 Files skipped from review as they are similar to previous changes (2)
- control-plane-operator/controllers/hostedcontrolplane/v2/kas/config.go
- control-plane-operator/controllers/hostedcontrolplane/v2/kas/auth_test.go
smrtrfszm
force-pushed
the
smrtrfszm/restrict-anonymous-auth
branch
from
519b23d to
fa01ad2
Compare
Anonymous authentication on the kube-apiserver is currently globally enabled via --anonymous-auth=true. This unnecessarily exposes the KAS to unauthenticated requests beyond what is needed for a select few endpoints. Instead of globally enabling anonymous auth, use the structured AuthenticationConfiguration to allow anonymous access only to those endpoints. The --anonymous-auth flag is removed and replaced with --authentication-config, which is now always set. New internal types (AnonymousAuthConfig, AnonymousAuthCondition) support this and the generated authentication config always carries the anonymous block, regardless of platform or OIDC configuration.
smrtrfszm
force-pushed
the
smrtrfszm/restrict-anonymous-auth
branch
from
fa01ad2 to
7983f19
Compare
|
/test e2e-conformance |
|
/jira refresh |
openshift-ci-robot
added
jira/valid-bug
|
@TwoDCube: This pull request references Jira Issue OCPBUGS-82036, which is valid. 3 validation(s) were run on this bug
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. |
|
@smrtrfszm: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
Test Failure Analysis CompleteJob Information
Test Failure AnalysisErrorSummaryPR #8133 restricts anonymous authentication on the hosted cluster's kube-apiserver to only probe endpoints ( Root CauseCategory 1 — Auth behavior change (5 tests, directly caused by PR): The PR removes Affected tests and their specific mismatch:
Category 2 — Konnectivity tunnel breakdown (72+ tests, likely infrastructure/timing issue): Starting at The konnectivity tunnel failure is likely a transient infrastructure issue — konnectivity probes use the Recommendations
Evidence
|
5 participants
What this PR does / why we need it:
Anonymous authentication on the kube-apiserver is currently globally enabled via
--anonymous-auth=true. This unnecessarily exposes the KAS to unauthenticated requests beyond what is needed for health probes.Instead of globally enabling anonymous auth, use the structured
AuthenticationConfigurationto allow anonymous access only to probe endpoints (/healthz, /livez, /livez/ping, /readyz). The--anonymous-authflag is removed and replaced with--authentication-config, which is now always set.For this the
AnonymousAuthConfigurableEndpointsfeature gate is needed, but it was GAd in 1.34.Which issue(s) this PR fixes:
Fixes #OCPBUGS-82036
Special notes for your reviewer:
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Tests