-
Notifications
You must be signed in to change notification settings - Fork 232
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
OCPBUGS-22332: route: Improve errors for missing custom-host permissions #1677
OCPBUGS-22332: route: Improve errors for missing custom-host permissions #1677
Conversation
@Miciah: This pull request references Jira Issue OCPBUGS-22332, 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. |
Return "you do not have permission" error messages instead of "field is immutable" error messages when the user tries to update a route's spec.host, spec.subdomain, or spec.tls field without the required "custom-host" permission. Note that this change does not affect the HTTP status code for these errors, which will continue to be HTTP 422, "Unprocessable Entity", as both the openshift-apiserver admission[1] and the kube-apiserver admission[2] wrap errors from the route validation's ValidateHostUpdate function using "k8s.io/apimachinery/pkg/api/errors".NewInvalid[3]. 1. https://github.com/openshift/openshift-apiserver/blob/c5ba848e78463f9c73e1d2daec9e565daa4a6f4a/vendor/k8s.io/apiserver/pkg/registry/rest/update.go#L154-L156 2. https://github.com/openshift/kubernetes/blob/5a4819c6048e42515c3c5538b723ba6272440eee/openshift-kube-apiserver/admission/route/hostassignment/admission.go#L135-L137 3. https://github.com/openshift/kubernetes/blob/5a4819c6048e42515c3c5538b723ba6272440eee/staging/src/k8s.io/apimachinery/pkg/api/errors/errors.go#L281-L310 This commit is related to OCPBUGS-22332. https://issues.redhat.com/browse/OCPBUGS-22332 * pkg/route/hostassignment/assignment.go (routeHostPermissionErrMsg) (routeSubdomainPermissionErrMsg, routeTLSPermissionErrMsg): New consts. (AllocateHost): Use routeHostPermissionErrMsg and routeTLSPermissionErrMsg instead of string literals. (validateImmutableField): New helper function, based on ValidateImmutableField from apimachinery but using a custom error message. (ValidateHostUpdate): Use the new consts and helper function instead of ValidateImmutableField from apimachinery.
0c62e25
to
46855ec
Compare
/assign @gcs278 |
/jira refresh |
@Miciah: This pull request references Jira Issue OCPBUGS-22332, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
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. |
Reviewed. Usually I like to test out PRs that I review, but since this also needs vendoring elsewhere, I'll just provide LGTM based on my assessment that the code updates are only impacting error messages, which feels low-risk to me. /hold |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gcs278, Miciah The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
LGTM |
@Miciah: all tests passed! Full PR test history. Your PR dashboard. 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. |
@Miciah: Jira Issue OCPBUGS-22332: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-22332 has been moved to the MODIFIED state. 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. |
Return "you do not have permission" error messages instead of "field is immutable" error messages when the user tries to update a route's
spec.host
,spec.subdomain
, orspec.tls
field without the required "custom-host" permission.Note that this change does not affect the HTTP status code for these errors, which will continue to be HTTP 422, "Unprocessable Entity", as both the openshift-apiserver admission and the kube-apiserver admission wrap errors from the route validation's
ValidateHostUpdate
function using"k8s.io/apimachinery/pkg/api/errors".NewInvalid
.pkg/route/hostassignment/assignment.go
(routeHostPermissionErrMsg
,routeSubdomainPermissionErrMsg
,routeTLSPermissionErrMsg
): New consts.(
AllocateHost
): UserouteHostPermissionErrMsg
androuteTLSPermissionErrMsg
instead of string literals.(
validateImmutableField
): New helper function, based onValidateImmutableField
from apimachinery but using a custom error message.(
ValidateHostUpdate
): Use the new consts and helper function instead ofValidateImmutableField
from apimachinery.