Open Stack

On 29/01, Ghanshyam Mann wrote:
>  ---- On Fri, 29 Jan 2021 11:23:47 -0600 Gorka Eguileor <geguileo at redhat.com> wrote ----
>  > On 28/01, Lance Bragstad wrote:
>  > > Hey folks,
>  > >
>  > > As I'm sure some of the cinder folks are aware, I'm updating cinder
>  > > policies to include support for some default personas keystone ships with.
>  > > Some of those personas use system-scope (e.g., system-reader and
>  > > system-admin) and I've already proposed a series of patches that describe
>  > > what those changes look like from a policy perspective [0].
>  > >
>  > > The question now is how we test those changes. To help guide that decision,
>  > > I worked on three different testing approaches. The first was to continue
>  > > testing policy using unit tests in cinder with mocked context objects. The
>  > > second was to use DDT with keystonemiddleware mocked to remove a dependency
>  > > on keystone. The third also used DDT, but included changes to update
>  > > NoAuthMiddleware so that it wasn't as opinionated about authentication or
>  > > authorization. I brought each approach in the cinder meeting this week
>  > > where we discussed a fourth approach, doing everything in tempest. I
>  > > summarized all of this in an etherpad [1]
>  > >
>  > > Up to yesterday morning, the only approach I hadn't tinkered with manually
>  > > was tempest. I spent some time today figuring that out, resulting in a
>  > > patch to cinderlib [2] to enable a protection test job, and
>  > > cinder_tempest_plugin [3] that adds the plumbing and some example tests.
>  > >
>  > > In the process of implementing support for tempest testing, I noticed that
>  > > service catalogs for system-scoped tokens don't contain cinder endpoints
>  > > [4]. This is because the cinder endpoint contains endpoint templating in
>  > > the URL [5], which keystone will substitute with the project ID of the
>  > > token, if and only if the catalog is built for a project-scoped token.
>  > > System and domain-scoped tokens do not have a reasonable project ID to use
>  > > in this case, so the templating is skipped, resulting in a cinder service
>  > > in the catalog without endpoints [6].
>  > >
>  > > This cascades in the client, specifically tempest's volume client, because
>  > > it can't find a suitable endpoint for request to the volume service [7].
>  > >
>  > > Initially, my testing approaches were to provide examples for cinder
>  > > developers to assess the viability of each approach before committing to a
>  > > protection testing strategy. But, the tempest approach highlighted a larger
>  > > issue for how we integrate system-scope support into cinder because of the
>  > > assumption there will always be a project ID in the path (for the majority
>  > > of the cinder API). I can think of two ways to approach the problem, but
>  > > I'm hoping others have more.
>  > >
>  >
>  > Hi Lance,
>  >
>  > Sorry to hear that the Cinder is giving you such trouble.
>  >
>  > > First, we remove project IDs from cinder's API path.
>  > >
>  > > This would be similar to how nova (and I assume other services) moved away
>  > > from project-specific URLs (e.g., /v3/%{project_id}s/volumes would become
>  > > /v3/volumes). This would obviously require refactoring to remove any
>  > > assumptions cinder has about project IDs being supplied on the request
>  > > path. But, this would force all authorization information to come from the
>  > > context object. Once a deployer removes the endpoint URL templating, the
>  > > endpoints will populate in the cinder entry of the service catalog. Brian's
>  > > been helping me understand this and we're unsure if this is something we
>  > > could even do with a microversion. I think nova did it moving from /v2/ to
>  > > /v2.0/, which was technically classified as a major bump? This feels like a
>  > > moon shot.
>  > >
>  >
>  > In my opinion such a change should not be treated as a microversion and
>  > would require us to go into v4, which is not something that is feasible
>  > in the short term.
>
> We can do it by supporting both URL with and without project_id. Nova did the same way

Hi,

I was not doubting that this was technically possible, I was arguing
that a change that affects every single API endpoint in Cinder would not
be described as "micro" and doing so could be considered a bit of abuse
to the microversion infrastructure.

This is just my opinion, not the Cinder official position.


> in Mitaka cycle and also bumped the microversion but just for
> notification. It was done in 2.18 microversion[1].
>
> That way you can request compute API with or without project_id and later is recommended.
> I think the same approach Cinder can consider.
>

Thanks for this information.  It will definitely come in handy knowing
where we have code references if we decide to go with the microversion
route.

Cheers,
Gorka.


> [1] https://docs.openstack.org/nova/latest/reference/api-microversion-history.html#id16
>
> -gmann
>
>  >
>  >
>  > > Second, we update cinder's clients, including tempest, to put the project
>  > > ID on the URL.
>  > >
>  > > After we update the clients to append the project ID for cinder endpoints,
>  > > we should be able to remove the URL templating in keystone, allowing cinder
>  > > endpoints to appear in system-scoped service catalogs (just like the first
>  > > approach). Clients can use the base URL from the catalog and append the
>  >
>  > I'm not familiar with keystone catalog entries, so maybe I'm saying
>  > something stupid, but couldn't we have multiple entries?  A
>  > project-specific URL and another one for the project and system scoped
>  > requests?
>  >
>  > I know it sounds kind of hackish, but if we add them in the right order,
>  > first the project one and then the new one, it would probably be
>  > backward compatible, as older clients would get the first endpoint and
>  > new clients would be able to select the right one.
>  >
>  > > admin project ID before putting the request on the wire. Even though the
>  > > request has a project ID in the path, cinder would ignore it for
>  > > system-specific APIs. This is already true for users with an admin role on
>  > > a project because cinder will allow you to get volumes in one project if
>  > > you have a token scoped to another with the admin role [8]. One potential
>  > > side-effect is that cinder clients would need *a* project ID to build a
>  > > request, potentially requiring another roundtrip to keystone.
>  >
>  > What would happen in this additional roundtrip? Would we be converting
>  > provided project's name into its UUID?
>  >
>  > If that's the case then it wouldn't happen when UUIDs are being
>  > provided, so for cases where this extra request means a performance
>  > problem they could just provide the UUID.
>  >
>  > >
>  > > Thoughts?
>  >
>  > Truth is that I would love to see the Cinder API move into URLs without
>  > the project id as well as move out everything from contrib, but that
>  > doesn't seem like a realistic piece of work we can bite right now.
>  >
>  > So I think your second proposal is the way to go.
>  >
>  > Thanks for all the work you are putting into this.
>  >
>  > Cheers,
>  > Gorka.
>  >
>  >
>  > >
>  > > [0] https://review.opendev.org/q/project:openstack/cinder+topic:secure-rbac
>  > > [1] https://etherpad.opendev.org/p/cinder-secure-rbac-protection-testing
>  > > [2] https://review.opendev.org/c/openstack/cinderlib/+/772770
>  > > [3] https://review.opendev.org/c/openstack/cinder-tempest-plugin/+/772915
>  > > [4] http://paste.openstack.org/show/802117/
>  > > [5] http://paste.openstack.org/show/802097/
>  > > [6]
>  > > https://opendev.org/openstack/keystone/src/commit/c239cc66615b41a0c09e031b3e268c82678bac12/keystone/catalog/backends/sql.py
>  > > [7] http://paste.openstack.org/show/802092/
>  > > [8] http://paste.openstack.org/show/802118/
>  >
>  >
>  >
>