[kolla] [train] [cinder] Volume multiattach exposed to non-admin users via API
Rajat Dhasmana
rdhasman at redhat.com
Fri Mar 3 09:49:09 UTC 2023
Thanks Brian and Ghanshyam for the discussion. I've updated the tempest
patch[1] to update one test that I missed earlier and also the cinder
patch[2] which now returns a BadRequest stating the reason for the error
and how to fix it.
$ curl -g -i -X POST
http://127.0.0.1/volume/v3/d6634f35c00f409883ecb10361b556c3/volumes -H
"Accept: application/json" -H "Content-Type: application/json" -H
"User-Agent: python-cinderclient" -H "X-Auth-Token:
gAAAAABkAbZkWgdbpXNgObizvGy8jS6LoMGuxzMnMaMOw6wm2j5i5KrG2xIzWCDxrSAiaMJWqneNpKrwn8P852mPOyJB_WmxrhrmKiuafcP0KSljyW44mFwDtGN74VL50NLoVC-srL63L3xduyeF5EIlPEyDsWRqPSRZZwau7wQrngAZ8XBP3M8"
-d '{"volume": {"size": 1, "consistencygroup_id": null, "snapshot_id":
null, "name": null, "description": null, "volume_type": null,
"availability_zone": null, "metadata": {}, "imageRef": null,
"source_volid": null, "backup_id": null, "multiattach": "true"}}'
HTTP/1.1 400 Bad Request
Date: Fri, 03 Mar 2023 09:04:38 GMT
Server: Apache/2.4.41 (Ubuntu)
OpenStack-API-Version: volume 3.0
Vary: OpenStack-API-Version
Content-Length: 261
Content-Type: application/json
x-compute-request-id: req-a9f9999e-01e3-4970-9c32-35de193c04c1
x-openstack-request-id: req-a9f9999e-01e3-4970-9c32-35de193c04c1
Connection: close
{"badRequest": {"code": 400, "message": "multiattach parameter has been
removed. The default behavior is to use multiattach enabled volume types.
Contact your administrator to create a multiattach enabled volume type and
use it to create multiattach volumes."}}
[1] https://review.opendev.org/c/openstack/tempest/+/875372
[2] https://review.opendev.org/c/openstack/cinder/+/874865
On Fri, Mar 3, 2023 at 9:00 AM Ghanshyam Mann <gmann at ghanshyammann.com>
wrote:
> ---- On Thu, 02 Mar 2023 19:17:32 -0800 Brian Rosmaita wrote ---
> > On 3/1/23 12:19 PM, Ghanshyam Mann wrote:
> > > ---- On Wed, 01 Mar 2023 09:02:59 -0800 Brian Rosmaita wrote ---
> > > > On 2/28/23 9:02 PM, Ghanshyam Mann wrote:
> > > > [snip]
> > > >
> > > > > I think removing from client is good way to stop exposing this
> old/not-recommended way to users
> > > > > but API is separate things and removing the API request
> parameter 'multiattach' from it can break
> > > > > the existing users using it this way. Tempest test is one good
> example of such users use case. To maintain
> > > > > the backward compatibility/interoperability it should be
> removed by bumping the microversion so that
> > > > > it continue working for older microversions. This way we will
> not break the existing users and will
> > > > > provide the new way for users to start using.
> > > >
> > > > It's not just that this is not recommended, it can lead to data
> loss.
> > > > We should only allow multiattach for volume types that actually
> support
> > > > it. So I see this as a case of "I broke your script now, but
> you'll
> > > > thank me later".
> > > >
> > > > We could microversion this, but then an end user has to go out of
> the
> > > > way and add the correct mv to their request to get the correct
> behavior.
> > > > Someone using the default mv + multiattach=true will
> unknowingly put
> > > > themselves into a data loss situation. I think it's better to
> break
> > > > that person's API request.
> > >
> > > Ok, if multiattach=True in the request is always an unsuccessful case
> (or unknown successful sometimes)
> > > then I think changing it without microversion bump makes sense. But
> if we know there is any success case
> > > for xyz configuration/backend then I feel we should not break such
> success use case.
> >
> > Thanks, Ghanshyam. An end user is setting themselves up for data loss
> > if they rely on the request parameter rather than on using a volume
> type
> > that explicitly supports multiattach. They could get lucky and not
> lose
> > any data, but that's not really a success, so I think the best thing to
> > do here is make this breaking change without a microversion.
> >
> > > I was just thinking from the Tempest test perspective which was
> passing but as you corrected me in IRC,
> > > the test does not check the data things so we do not completely test
> it in Tempest.
> >
> > It's good that Tempest is there to keep us honest! I think what we can
> > do to help out people whose scripts break is to return a specific error
> > message explaining that the 'multiattach' element is not allowed in a
> > volume-create request and instead the user should select a
> > multiattach-capable volume type.
>
> Thanks, Brian for explaining. This sounds good to me. Explaining the
> situation in release notes and error message
> will be really helpful for users.
>
> I am +2 on the tempest change now -
> https://review.opendev.org/c/openstack/tempest/+/875372
>
> -gmann
>
> >
> > >
> > > -gmann
> > >
> > > >
> > > >
> > > > cheers,
> > > > brian
> > > >
> > > >
> > > >
> >
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.openstack.org/pipermail/openstack-discuss/attachments/20230303/9a19e0d8/attachment-0001.htm>
More information about the openstack-discuss
mailing list