Open Stack

On Thu, Dec 20, 2018 at 3:23 PM Brian Rosmaita <rosmaita.fossdev at gmail.com>
wrote:

> Update: I've revised the patch documenting how to configure Cinder
> policies to enable a read-only administrator [0] to take into account
> the ongoing Keystone work that Lance discussed earlier in this thread.
>
> At this point in the development cycle, I think it makes the most sense
> to continue with the plan to "fix" this via documentation, especially
> since the workaround documented immediately applies to the stable branches.
>
> The work Yikun Jiang is doing now to get "protection" tests into Cinder
> [1] will give us a base to work from if we decide to refactor the code
> to make this easier (or to implement default policies that recognize a
> 'reader' role out of the box, as Keystone is doing this cycle).
>
>
++

I like this idea, too. Increasing test coverage of what cinder's policy
supports today will make it easier to improve things later. For example,
adding formal read-only support by changing the default policies, or even
refactoring the contextual admin checks out of the cinder database layer
and into a formal validation layer closer to the API.

Thanks for the summary, Brian!


> In any case, reviews of [0] will be appreciated, especially during this
> festive holiday season.
>
> cheers,
> brian
>
> [0] https://review.openstack.org/#/c/624424/
> [1]
>
> https://review.openstack.org/#/q/status:open+project:openstack/cinder+branch:master+topic:policy_in_code_test
>
>
> On 12/13/18 12:58 PM, Lance Bragstad wrote:
> > Thanks for looping keystone into this, Ben.
> >
> > This sounds very similar, and related, to the default roles work
> > keystone implemented in Rocky [0]. TL;DR, keystone ensures the
> > ``member`` and ``reader`` roles are available during ``keystone-manage
> > bootstrap``, which is typically done during installation and sometimes
> > on upgrade. Pre-Rocky, only the ``admin`` role existed in keystone.
> >
> > The idea is to make it easier for other services, like cinder, to rely
> > on these roles by default. As opposed to cinder changing a policy
> > default to something like ``role:reader`` and another service using
> > something like ``role:auditor``, but they both mean the same thing. We
> > want to avoid the case where operators would need to make sure both
> > roles exist in keystone and users have both if they're expected to
> > perform read-only operations from each service.
> >
> > The work keystone did in Rocky made it possible for cinder to start
> > making those assumptions about the existence of roles called ``member``
> > and ``reader``.
> >
> > As far as testing goes, I remember there being a cinder session in
> > Denver about policy changes, regression, and making sure we can protect
> > against it happening, especially late in the release. Ideally, it would
> > be fantastic to have protection tests in each service that assert the
> > default policies work as expected for protecting APIs. Unfortunately,
> > testing this in its entirety makes it an integration test that requires
> > keystone. Luckily, there are some ways for services to expand protection
> > testing without having that dependency, which falls within the
> > parameters of unit tests. I proposed a couple of changes to cinder that
> > show how to do this [1], and it requires modeling an API request similar
> > to what keystonemiddleware/oslo.context do and ensuring the actual
> > defaults are tested by removing overridden test policies [2] that leave
> > APIs unprotected. We attempted to document a generalized approach to
> > this sort of testing in oslo.policy [3].
> >
> > [0]
> http://specs.openstack.org/openstack/keystone-specs/specs/keystone/rocky/define-default-roles.html
> > [1] https://review.openstack.org/#/c/602489/
> > [2] https://review.openstack.org/#/c/609498/
> > [3]
> https://docs.openstack.org/oslo.policy/latest/user/usage.html#testing-default-policies
> >
> > On Thu, Dec 13, 2018 at 8:10 AM Ben Nemec <openstack at nemebean.com
> > <mailto:openstack at nemebean.com>> wrote:
> >
> >     cc Keystone. It would be interesting to know how this fits in with
> the
> >     new default roles work.
> >
> >     On 12/12/18 8:21 PM, Brian Rosmaita wrote:
> >     > At last week's cinder meeting (5 December) we had a discussion
> about
> >     > policy checks at the database layer.  While these checks seem to
> >     make it
> >     > difficult to perform some policy configurations, there are too
> many of
> >     > them to simply remove them without impacting stability given our
> >     current
> >     > test coverage (at least that's my feeling).  Additionally, it's
> >     possible
> >     > to handle the proposed use case (a read-only administrator) without
> >     > making any code changes.  So we decided to fix this issue by
> >     documenting
> >     > how this could work.
> >     >
> >     > I've got a patch up for the documentation.  I've flagged this
> >     email for
> >     > cinder people to read for accuracy, operators to read to make sure
> the
> >     > instructions are clear and detailed, and docs people to read for
> >     > felicity of expression.  Please leave comments on the patch:
> >     >
> >     > https://review.openstack.org/#/c/624424/
> >     >
> >     > cheers,
> >     > brian
> >     >
> >
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-discuss/attachments/20181220/577cf71c/attachment-0001.html>