Open Stack

Brian and I had a discussion about all of this yesterday and we revisited
the idea of a project-less URL template. This would allow us to revisit
system-scope support for Wallaby under the assumption the client handles
project IDs properly for system-scoped requests and cinder relaxes its
project ID validation for system-scoped contexts.

It's possible to get a cinder endpoint in the service catalog if you create
a separate endpoint without project ID templating in the URL. I hacked this
together in devstack [0] using a couple of changes to python-cinderclient
[1] and cinder's API [2]. After that, I was able to list all volumes in a
deployment as a system-administrator (using a system-scoped admin token)
[3].

The only hiccup I hit was that I was supplying two endpoints for the
volumev3 service. If the endpoint without project ID templating appears
first in the catalog for project-scoped tokens, then requests to cinder
will fail because the project ID isn't in the URL. Remember, the only
cinder endpoint in the catalog for system-scoped tokens was the one without
templating, so this issue doesn't appear there. Also, we would need a
separate patch to the tempest volume client before we could add any
system-scope testing there.

Thoughts?

[0] https://review.opendev.org/c/openstack/devstack/+/776520
[1] https://review.opendev.org/c/openstack/python-cinderclient/+/776469
[2] https://review.opendev.org/c/openstack/cinder/+/776468
[3] http://paste.openstack.org/show/802786/




On Wed, Feb 17, 2021 at 12:11 PM Lance Bragstad <lbragstad at gmail.com> wrote:

> Circling back on this topic.
>
> I marked all the patches that incorporate system-scope support as WIP [0].
> I think we can come back to these after we have a chance to decouple
> project IDs from cinder's API in Xena. I imagine that's going to be a
> pretty big change so we can push those reviews to the back burner for now.
>
> In the meantime, I reproposed all patches that touch the ADMIN_OR_OWNER
> rule and updated them to use the member and reader roles [1]. I also
> removed any system-scope policies from those patches. The surface area of
> these changes is a lot less than what we were originally expecting to get
> done for Wallaby. These changes should at least allow operators to use the
> member and reader roles on projects consistently with cinder when Wallaby
> goes out the door.
>
> To recap, this would mean anyone with the admin role on a project is still
> considered a system administrator in cinder (we can try and fix this in
> Xena). Operators can now use the member role to denote owners and give
> users the reader role on a project and those users shouldn't be able to
> make writable changes within cinder.
>
> [0]
> https://review.opendev.org/q/project:openstack/cinder+topic:secure-rbac+label:Workflow%253C0
> [1]
> https://review.opendev.org/q/project:openstack/cinder+topic:secure-rbac+label:Workflow%253E-1
>
> On Fri, Jan 29, 2021 at 11:24 AM 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.
>>
>>
>> > 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/
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-discuss/attachments/20210218/3629bfa3/attachment-0001.html>