[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