Service-catalog: Instance update resends all parameters from secret, not just what changed

Created on 27 Oct 2017 · 9Comments · Source: kubernetes-sigs/service-catalog

If an instance references parameters from a secret and the user updates just one value in the secret, the service catalog appears to send all parameters to the broker. This causes a problem for brokers that only support updating a subset of the parameters and results in the service catalog sending parameters that don't validate against the plan's instance update schema.

There doesn't appear to be a way for service catalog clients to work around this.

cc @pmorie @jwforres

lifecyclrotten

Source

spadgett

Most helpful comment

I'm starting to think based on our previous discussion that this area of the spec is actually overspecified.

I don't think I want the user to have to specify which parameters they want to send for an update - I think that would be a very bad user experience. I'd much rather either detect which parameters had changed values and only send those, or just send them all and force the broker to figure out what changed. Note, this is another area where it seems likely that the mission of enabling stateless brokers has complicated the entire problem space.

pmorie on 30 Oct 2017

👍3

All 9 comments

This is probably more of a spec question but a really interesting one.

Do we even know what params were changed when they're stored in a secret? I'm guessing no, which means we have no choice but to send all of them (well, all of the ones per the "update" schema).

When the schema is specified then the platform(svc-cat) should probably limit the list of params to just what was specified as the valid subset in the schema.

However, schema is optional. And when not known, I don't think we have a choice but to send it all. Which means the spec will need to clarify what should happen if there's too much - fail vs ignore 'em.

I don't think CF has this problem (yet) because when you update an instance the user provides the set of parameters to send to the broker. It doesn't do anything with the old values used during the provision, nor do they have to worry about secrets and checking for changes. But, even with that, the spec doesn't say whether the set of params provided on the update is meant to totally replace the existing set or whether it should be merged with existing params. We have https://github.com/openservicebrokerapi/servicebroker/issues/350 to discuss parts of this.

duglin on 27 Oct 2017

Do we even know what params were changed when they're stored in a secret?

Yeah, I see the problem.

If the update parameters were stored in a separate property, would it work? So instead of updating the original secret, I could set a value like instance.spec.updateParametersFrom with just the values I want to change? Then I as the user can decide what gets sent.

spadgett on 27 Oct 2017

@jmontleon

spadgett on 27 Oct 2017

That would certainly align with that CF does (meaning, asking the user to explicitly say what params should be sent), but are we setting up the user for a funky UX? What if they update 3 properties in the secret, but then only specify 2 in the "updateParameteresFrom" property? I guess we could just claim its a user error (and they got what they asked for), but debugging why things aren't working right later on would be a real nightmare if more updates were done after that - assuming there's no GET to query the broker for the current set of params.

See: https://github.com/openservicebrokerapi/servicebroker/issues/350#issuecomment-339971319

duglin on 27 Oct 2017

I'm starting to think based on our previous discussion that this area of the spec is actually overspecified.

pmorie on 30 Oct 2017

👍3

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

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

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

fejta-bot on 22 Apr 2019

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

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

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

fejta-bot on 22 May 2019

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

fejta-bot on 21 Jun 2019

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

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/test-infra repository.