Endpoints write permissions removed in OCP 4.21#642
Merged
Conversation
Reviewer's GuideAdjusts OpenShift cleanup and application-building logic to stop managing Endpoints (which now require stricter RBAC and are deprecated), documents permission requirements on remaining endpoint helpers, and deprecates the EndpointBuilder API. Sequence diagram for updated OpenShift.clean without Endpoints deletionsequenceDiagram
participant TestCode
participant OpenShift
participant KubernetesAPI
TestCode->>OpenShift: clean()
activate OpenShift
OpenShift->>KubernetesAPI: replicationControllers().withLabelNotIn(...).delete()
KubernetesAPI-->>OpenShift: deletion result
OpenShift->>KubernetesAPI: buildConfigs().withLabelNotIn(...).delete()
KubernetesAPI-->>OpenShift: deletion result
OpenShift->>KubernetesAPI: imageStreams().withLabelNotIn(...).delete()
KubernetesAPI-->>OpenShift: deletion result
Note over OpenShift,KubernetesAPI: No call to endpoints().withLabelNotIn(...).delete()
OpenShift->>KubernetesAPI: services().withLabelNotIn(...).delete()
KubernetesAPI-->>OpenShift: deletion result
OpenShift->>KubernetesAPI: builds().withLabelNotIn(...).delete()
KubernetesAPI-->>OpenShift: deletion result
OpenShift->>KubernetesAPI: routes().withLabelNotIn(...).delete()
KubernetesAPI-->>OpenShift: deletion result
OpenShift-->>TestCode: Waiter
deactivate OpenShift
Class diagram for updated OpenShift, OpenShiftApplication, and EndpointBuilderclassDiagram
class OpenShift {
+Waiter clean()
+List~HasMetadata~ listRemovableResources()
+Endpoints createEndpoint(Endpoints endpoint)
+List~Endpoints~ getEndpoints()
+boolean deleteEndpoint(Endpoints endpoint)
+boolean deleteService(Service service)
}
class OpenShiftApplication {
-OpenShift openShift
-List~PersistentVolumeClaim~ persistentVolumeClaims
-List~DeploymentConfig~ deploymentConfigs
-List~Service~ services
-List~Route~ routes
-List~ConfigMap~ configMaps
-List~HorizontalPodAutoscaler~ autoScalers
-List~ImageStream~ imageStreams
-List~BuildConfig~ buildConfigs
-List~Build~ builds
-List~RoleBinding~ roleBindings
-List~ServiceAccount~ serviceAccounts
-List~Secret~ secrets
-List~PodSecurityPolicy~ podSecurityPolicies
-List~ClusterRole~ clusterRoles
-List~ClusterRoleBinding~ clusterRoleBindings
-List~ImageStreamTag~ imageStreamTags
-List~Template~ templates
-List~ProjectRequest~ projectRequests
-List~Namespace~ namespaces
-List~LimitRange~ limitRanges
-List~ResourceQuota~ resourceQuotas
-List~NetworkPolicy~ networkPolicies
-List~PodDisruptionBudget~ podDisruptionBudgets
-List~Job~ jobs
-List~CronJob~ cronJobs
-List~DaemonSet~ daemonSets
-List~StatefulSet~ statefulSets
-List~ServiceMonitor~ serviceMonitors
-List~Alertmanager~ alertmanagers
-List~Prometheus~ prometheuses
-List~Route~ routes
-void createResources()
}
class EndpointBuilder {
<<deprecated>>
-List~String~ endpointIPs
-List~Port~ ports
+EndpointBuilder addIp(String ip)
+EndpointBuilder addPort(Port port)
+Endpoints build()
}
class AbstractBuilder~T, B~ {
+B withName(String name)
+B addLabel(String key, String value)
+T build()
}
class Endpoints
class Service
class Route
EndpointBuilder --|> AbstractBuilder~Endpoints, EndpointBuilder~
OpenShiftApplication o--> OpenShift : uses
OpenShiftApplication o--> Service
OpenShiftApplication o--> Route
OpenShiftApplication o--> DeploymentConfig
OpenShiftApplication o--> PersistentVolumeClaim
OpenShift --> Endpoints : create/delete
OpenShift --> Service : deleteService
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Since
EndpointBuilderis now deprecated in the Javadoc, consider also adding the@Deprecatedannotation on the class to ensure tooling and callers are properly warned at compile time. - For
createEndpointanddeleteEndpoint, now that the Javadoc explicitly calls out the need for write permissions, it may be helpful to document (or standardize) what exception is thrown on 403 responses so callers can reliably handle the permission failure case.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Since `EndpointBuilder` is now deprecated in the Javadoc, consider also adding the `@Deprecated` annotation on the class to ensure tooling and callers are properly warned at compile time.
- For `createEndpoint` and `deleteEndpoint`, now that the Javadoc explicitly calls out the need for write permissions, it may be helpful to document (or standardize) what exception is thrown on 403 responses so callers can reliably handle the permission failure case.
## Individual Comments
### Comment 1
<location path="core/src/main/java/cz/xtf/core/openshift/OpenShift.java" line_range="684-686" />
<code_context>
+ * @param endpoint
+ * @return Endpoints created
+ */
public Endpoints createEndpoint(Endpoints endpoint) {
return endpoints().resource(endpoint).create();
}
</code_context>
<issue_to_address>
**suggestion:** Consider deprecating the OpenShift endpoint helpers to align with EndpointBuilder and Kubernetes Endpoints deprecation.
Since EndpointBuilder and Kubernetes Endpoints are now deprecated, consider annotating `createEndpoint`/`deleteEndpoint` with `@Deprecated` and referencing the preferred alternative (e.g., EndpointSlice-based flows) so callers don’t add new usage of a phased-out API.
Suggested implementation:
```java
// Endpoints
/**
* Needs a role with write permissions see
* <a href="https://kubernetes.io/docs/reference/access-authn-authz/rbac/#write-access-for-endpoints">Endpoint write
* access</a>
*
* @param endpoint
* @return Endpoints created
* @deprecated Use EndpointSlice-based flows instead of Endpoints. This helper is deprecated to align with
* {@code EndpointBuilder} and Kubernetes {@code Endpoints} deprecation.
*/
@Deprecated
public Endpoints createEndpoint(Endpoints endpoint) {
return endpoints().resource(endpoint).create();
}
```
You should similarly deprecate the `deleteEndpoint` helper in the same class. For the `deleteEndpoint` method:
1. Add a Javadoc `@deprecated` tag explaining that callers should migrate to EndpointSlice-based flows (and, if you have a specific replacement API in this project, reference it explicitly).
2. Add the `@Deprecated` annotation to the method declaration.
For example, if it currently looks like:
```java
/**
* Deletes an Endpoints resource.
*
* @param name name of the Endpoints resource
*/
public void deleteEndpoint(String name) {
endpoints().withName(name).delete();
}
```
update it to:
```java
/**
* Deletes an Endpoints resource.
*
* @param name name of the Endpoints resource
* @deprecated Use EndpointSlice-based flows instead of Endpoints. This helper is deprecated to align with
* {@code EndpointBuilder} and Kubernetes {@code Endpoints} deprecation.
*/
@Deprecated
public void deleteEndpoint(String name) {
endpoints().withName(name).delete();
}
```
Adjust the `SEARCH` portion of the edit block to match the exact existing Javadoc/method signature in your file.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
684
to
686
| public Endpoints createEndpoint(Endpoints endpoint) { | ||
| return endpoints().resource(endpoint).create(); | ||
| } |
There was a problem hiding this comment.
suggestion: Consider deprecating the OpenShift endpoint helpers to align with EndpointBuilder and Kubernetes Endpoints deprecation.
Since EndpointBuilder and Kubernetes Endpoints are now deprecated, consider annotating createEndpoint/deleteEndpoint with @Deprecated and referencing the preferred alternative (e.g., EndpointSlice-based flows) so callers don’t add new usage of a phased-out API.
Suggested implementation:
// Endpoints
/**
* Needs a role with write permissions see
* <a href="https://kubernetes.io/docs/reference/access-authn-authz/rbac/#write-access-for-endpoints">Endpoint write
* access</a>
*
* @param endpoint
* @return Endpoints created
* @deprecated Use EndpointSlice-based flows instead of Endpoints. This helper is deprecated to align with
* {@code EndpointBuilder} and Kubernetes {@code Endpoints} deprecation.
*/
@Deprecated
public Endpoints createEndpoint(Endpoints endpoint) {
return endpoints().resource(endpoint).create();
}You should similarly deprecate the deleteEndpoint helper in the same class. For the deleteEndpoint method:
- Add a Javadoc
@deprecatedtag explaining that callers should migrate to EndpointSlice-based flows (and, if you have a specific replacement API in this project, reference it explicitly). - Add the
@Deprecatedannotation to the method declaration.
For example, if it currently looks like:
/**
* Deletes an Endpoints resource.
*
* @param name name of the Endpoints resource
*/
public void deleteEndpoint(String name) {
endpoints().withName(name).delete();
}update it to:
/**
* Deletes an Endpoints resource.
*
* @param name name of the Endpoints resource
* @deprecated Use EndpointSlice-based flows instead of Endpoints. This helper is deprecated to align with
* {@code EndpointBuilder} and Kubernetes {@code Endpoints} deprecation.
*/
@Deprecated
public void deleteEndpoint(String name) {
endpoints().withName(name).delete();
}Adjust the SEARCH portion of the edit block to match the exact existing Javadoc/method signature in your file.
Contributor
|
Can we have attached CI run confirming everything is still ok? My understanding of change is we will be not explicitly deleting Endpoint objects. We are not creating Enpoint objects directly, but just through Service object. So once we delete Service, dependant Endpoints will be removed as well. Do I understand it correctly? |
Contributor
|
@liborfuka could you take a look at this PR as well please? |
Contributor
Contributor
Author
@mchoma Yes, that's correct. |
mchoma
approved these changes
Apr 16, 2026
Contributor
mchoma
left a comment
mchoma
left a comment
There was a problem hiding this comment.
Ok to merge this. Runs attached to https://redhat.atlassian.net/browse/EAPQE-5152
Contributor
|
LGTM, merging. Thansk for the fix. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is related to [#639].
This is related to CVE-2021-25740 . XTF's OpenShift.clean() calls endpoints().delete() and endpoints().list(), which require Endpoints write/read permissions. On OCP 4.21, users with the standard edit or admin role no longer have Endpoints write access, causing a 403 KubernetesClientException that aborts the entire cleanup - leaving services, pods, routes, and everything after the endpoints call uncleaned.
Why do we see it now:
Even this change was implemented in Kubernetes 1.22, we see the it first time in OCP 4.21 (Kubernetes 1.34) because until OCP 4.20, Openshift was reverting this with each release, because this issue was evaluated by RedHat as not affected. Recently it was removed from list of patches applied with each OCP release.
What has been changed
The endpoints() call in Openshift.clean() and listRemovableResources().
This endpoints are managed by Service resource.
Openshift.createEndpoint and Openshift.deleteEndpoint()
They will work only when user with edit role on Endpoints is used - Write access for endpoints
Deprecated EndpointBuilder
Endpoints were deprecated in Kubernetes v1.33.
OpenshiftApplication
Links
CVE-2021-25740: Endpoint & EndpointSlice permissions allow cross-Namespace forwarding · Issue #103675 · kubernetes/kubernetes
Using RBAC Authorization
1982470 – (CVE-2021-25740) CVE-2021-25740 kubernetes: Endpoint & EndpointSlice permissions allow cross-Namespace forwarding
Revert "Remove Endpoints write access from aggregated edit role" by danwinship · Pull Request #908 · openshift/kubernetes
Summary by Sourcery
Adjust OpenShift cleanup and application wiring to avoid relying on Endpoints resources that now require elevated permissions, and mark legacy Endpoints usage as deprecated in favor of EndpointSlice.
Bug Fixes:
Enhancements: