PUT request that modifies route certificate fails

[spadgett]

This looks like an API breaking change in 3.6. If I do a PUT on a route and change the spec.tls.certificate, I'm told the field is immutable. But oc edit allows me to change the value.

The error breaks the web console route editor in 3.6.

@knobunc Let me know if I'm wrong and the field should be immutable, but I believe we were able to change it in previous releases.

@smarterclayton @andrewklau

See openshift/origin-web-console#1930

Version

3.6.0

Steps To Reproduce

1. Edit a route in the web console (or edit the route YAML in the web console).
2. Only change the route certificate.

You'll see an error saying the field is immutable. If you inspect the web console PUT request in developer tools, the only value changed in the request body is spec.tls.certificate.

[smarterclayton]

Are you using the new endpoint or the old endpoint?

…

On Mon, Aug 14, 2017 at 4:20 PM, Sam Padgett ***@***.***> wrote: This looks like an API breaking change in 3.6. If I do a PUT on a route and change the spec.tls.certificate, I'm told the field is immutable. But oc edit allows me to change the value. The error breaks the web console route editor in 3.6. @knobunc <https://github.com/knobunc> Let me know if I'm wrong and the field should be immutable, but I believe we were able to change it in previous releases. @smarterclayton <https://github.com/smarterclayton> @andrewklau <https://github.com/andrewklau> See openshift/origin-web-console#1930 <openshift/origin-web-console#1930> Version 3.6.0 Steps To Reproduce 1. Edit a route in the web console (or edit the route YAML in the web console). 2. Only change the route certificate. You'll see an error saying the field is immutable. If you inspect the web console PUT request in developer tools, the only value changed in the request body is spec.tls.certificate. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#15772>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABG_p1lETduIsTVJ68PJfkOlvQZ1yHxpks5sYKvxgaJpZM4O22WT> .

[smarterclayton]

And are you posting empty field, or doing a patch with no field provided? On Mon, Aug 14, 2017 at 4:28 PM, Clayton Coleman <ccoleman@redhat.com> wrote:

…

Are you using the new endpoint or the old endpoint? On Mon, Aug 14, 2017 at 4:20 PM, Sam Padgett ***@***.***> wrote: > This looks like an API breaking change in 3.6. If I do a PUT on a route > and change the spec.tls.certificate, I'm told the field is immutable. > But oc edit allows me to change the value. > > The error breaks the web console route editor in 3.6. > > @knobunc <https://github.com/knobunc> Let me know if I'm wrong and the > field should be immutable, but I believe we were able to change it in > previous releases. > > @smarterclayton <https://github.com/smarterclayton> @andrewklau > <https://github.com/andrewklau> > > See openshift/origin-web-console#1930 > <openshift/origin-web-console#1930> > Version > > 3.6.0 > Steps To Reproduce > > 1. Edit a route in the web console (or edit the route YAML in the web > console). > 2. Only change the route certificate. > > You'll see an error saying the field is immutable. If you inspect the web > console PUT request in developer tools, the only value changed in the > request body is spec.tls.certificate. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#15772>, or mute the thread > <https://github.com/notifications/unsubscribe-auth/ABG_p1lETduIsTVJ68PJfkOlvQZ1yHxpks5sYKvxgaJpZM4O22WT> > . >

[spadgett]

Old endpoint

I can reproduce using the same object returned by the API, just changing the certificate value. No empty fields added

[smarterclayton]

For normal users, certificates are now immutable.

…

On Mon, Aug 14, 2017 at 4:35 PM, Sam Padgett ***@***.***> wrote: Old endpoint I can reproduce using the same object returned by the API, just changing the certificate value. No empty fields added — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#15772 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABG_p7YTS4cpUSv0RdACmCokow66YjnPks5sYK-vgaJpZM4O22WT> .

[smarterclayton]

You should be getting a 422 with a forbidden field. On Mon, Aug 14, 2017 at 5:53 PM, Clayton Coleman <ccoleman@redhat.com> wrote:

…

For normal users, certificates are now immutable. On Mon, Aug 14, 2017 at 4:35 PM, Sam Padgett ***@***.***> wrote: > Old endpoint > > I can reproduce using the same object returned by the API, just changing > the certificate value. No empty fields added > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#15772 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/ABG_p7YTS4cpUSv0RdACmCokow66YjnPks5sYK-vgaJpZM4O22WT> > . >

[spadgett]

@smarterclayton Thanks. I'm getting a 422 with the following response body.

Debating making the field read only in the web console, although it would mean users who can edit the certificate won't be able to through the console.

{
  "kind": "Status",
  "apiVersion": "v1",
  "metadata": {},
  "status": "Failure",
  "message": "Route \"node-edge\" is invalid: spec.tls.certificate: Invalid value: \"foo\": field is immutable",
  "reason": "Invalid",
  "details": {
    "name": "node-edge",
    "kind": "Route",
    "causes": [
      {
        "reason": "FieldValueInvalid",
        "message": "Invalid value: \"foo\": field is immutable",
        "field": "spec.tls.certificate"
      }
    ]
  },
  "code": 422
}

[smarterclayton]

Some clusters may allow it for all users. You need the "update" verb on resource "routes/custom-host" and group "route.openshift.io" to change a route host/cert once created. It's only the most restrictive clusters that need this. It was made the default behavior because it was technically an oversight when we made spec.host immutable.

…

On Mon, Aug 14, 2017 at 6:50 PM, Sam Padgett ***@***.***> wrote: @smarterclayton <https://github.com/smarterclayton> Thanks. I'm getting a 422 with the following response body. Debating making the field read only in the web console, although it would mean users who can edit the certificate won't be able to through the console. { "kind": "Status", "apiVersion": "v1", "metadata": {}, "status": "Failure", "message": "Route \"node-edge\" is invalid: spec.tls.certificate: Invalid value: \"foo\": field is immutable", "reason": "Invalid", "details": { "name": "node-edge", "kind": "Route", "causes": [ { "reason": "FieldValueInvalid", "message": "Invalid value: \"foo\": field is immutable", "field": "spec.tls.certificate" } ] }, "code": 422 } — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#15772 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABG_p36Sf1V5lbboxCbqTlO0qFf53_Hnks5sYM8kgaJpZM4O22WT> .

[andrewklau]

Confirmed, adding the update verb to the routes/custom-host allows a user to edit it again on the web console. I couldn't find any reference release note for this change though.

[spadgett]

@smarterclayton bump for updating the error message to say why it's immutable and possibly changing the default for update (per our conversation)

[smarterclayton]

I don't remember why I didn't give "update" to users by default - I think it was because of the custom host being immutable which was our current behavior. @abhgupta @Miciah see the thread above.

I'll definitely improve the message for update.

[ValentinFunk]

@smarterclayton is there a way to get the update permission somehow in the dev preview? I've tried using a local role but there seems to be a rolebindingrestriction that prevents actually using the role

[rajatchopra]

pull #18177 to allow admin to update routes/custom-host by default.
Also see discussion in https://bugzilla.redhat.com/show_bug.cgi?id=1524707

[liggitt]

For normal users, certificates are now immutable.

how is that reasonable? certificates expire.

pull #18177 to allow admin to update routes/custom-host by default.

gating updating the certificate on an update host permission doesn't make sense to me

[rajatchopra]

@liggitt
Would you prefer something like this: #18195
This removes TLS update out of routes/custom-host resource and keeps it in 'routes' resource only.

[liggitt]

Would you prefer something like this: #18195

the check in that PR is a duplicate of the check the apiserver already does. it's still unclear to me why we are preventing updates of the certificate.

secondly, even if we did restrict that, it should not be coupled to the same permission that allows updating the host field. updating the host is very privileged. I wouldn't expect updating a certificate to be that privileged.

[rajatchopra]

@liggitt
Fixed the duplicate in the PR.
TLS is not coupled to routes/custom-host now (through #18195).

I agree that TLS update should not need that much privilege, but we can discuss that as follow up. The current behaviour is that not even the project admin can modify the fields, and that needs to be fixed immediately.

[openshift-bot]

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

[openshift-bot]

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

[openshift-bot]

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close