Use oslo.context's policy dict

[Jamie Lennox]

This is a cross project goal to standardize the values available to policy writers and to improve the basic oslo.context object. It is part of the follow up work to bug #1577996 and bug #968696.

There has been an ongoing problem for how we define the 'admin' role. Because tokens are project scoped having the 'admin' role on any project granted you the 'admin' role on all of OpenStack. As a solution to this keystone defined an is_admin_project field so that keystone defines a single project that your token must be scoped to to perform admin operations. This has been implemented.

The next phase of this is to make all the projects understand the X-Is-Admin-Project header from keystonemiddleware and pass it to oslo_policy. However this pattern of keystone changes something and then goes to every project to fix it has been repeated a number of times now and we would like to make it much more automatic.

Ongoing work has enhanced the base oslo.context object to include both the load_from_environ and to_policy_values methods. The load_from_environ classmethod takes an environment dict with all the standard auth_token and oslo middleware headers and loads them into their standard place on the context object.

The to_policy_values() then creates a standard credentials dictionary with all the information that should be required to enforce policy from the context. The combination of these two methods means in future when authentication information needs to be passed to policy it can be handled entirely by oslo.context and does not require changes in each individual service.

Note that in future a similar pattern will hopefully be employed to simplify passing authentication information over RPC to solve the timeout issues. This is a prerequisite for that work.

There are a few common problems in services that are required to make this work:

1. Most service context.__init__ functions take and discard **kwargs. This is so if the context.from_dict receives arguments it doesn't know how to handle (possibly because new things have been added to the base to_dict) it ignores them. Unfortunately to make the load_from_environ method work we need to pass parameters to __init__ that are handled by the base class.

To make this work we simply have to do a better job of using from_dict. Instead of passing everything to __init__ and ignoring what we don't know we have from_dict extract only the parameters that context knows how to use and call __init__ with those.

2. The parameters passed to the base context.__init__ are old. Typically they are user and tenant where most services expect user_id and project_id. There is ongoing work to improve this in oslo.context but for now we have to ensure that the subclass correctly sets and uses the right variable names.

3. Some services provide additional information to the policy enforcement method. To continue to make this function we will simply override the to_policy_values method in the subclasses.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/314889
Committed: https://git.openstack.org/cgit/openstack/glance/commit/?id=ca501cba92960d0d9cffc346ebd47d39fbce32e8
Submitter: Jenkins
Branch: master

commit ca501cba92960d0d9cffc346ebd47d39fbce32e8
Author: Jamie Lennox <email address hidden>
Date: Wed May 4 17:10:40 2016 +1000

    Use oslo.context features

    In an effort to standardize policy and authentication values
    oslo.context has new features such as from_environ which constructs a
    standard oslo.context object from the environment variables created by
    auth_token middleware and to_policy_values which emit a standard
    credentials target for writing common policy files across services.

    Use these standard functions when dealing with contexts and policy in
    glance.

    Closes-Bug: #1602081
    Change-Id: I40582cb34818b980d6c6914b2c9346a17a0ed489

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/341895

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/341896

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/341905

[Davanum Srinivas (DIMS) (dims-v)]

This issue was fixed in the openstack/glance 13.0.0.0b2 development milestone.

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/342604

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/335303
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=528945425e43c9c3d62c4d367b002ec02913900c
Submitter: Jenkins
Branch: master

commit 528945425e43c9c3d62c4d367b002ec02913900c
Author: Jamie Lennox <email address hidden>
Date: Wed Jun 29 13:38:39 2016 +1000

    Use to_policy_values from context for policy

    The oslo.context to_policy_values provide the standard arguments that
    should be passed to oslo.policy for enforcement. By using these values
    heat will automatically gain support for new things like
    is_admin_project as they are supported by oslo_context.

    Because previously the whole to_dict was passed to policy enforcement we
    are actually removing a whole bunch of options that could be used in
    policy enforcement - however from a practical perspective i'm not sure
    anyone would have used them.

    Closes-Bug: #1602081
    Change-Id: I244ed767e2077cf43d55104779484b64bd28c85f

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/341895
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=2a610741b5a1d02f984a6af132131b8838d8c156
Submitter: Jenkins
Branch: master

commit 2a610741b5a1d02f984a6af132131b8838d8c156
Author: Jamie Lennox <email address hidden>
Date: Thu Jul 14 11:21:27 2016 +1000

    Pass kwargs through to base context

    The unknown keyword arguments that were passed to
    RequestContext.__init__ were being dropped so that unknown parameters
    coming back from from_dict didn't cause errors. This means however that
    newer properties like RequestContext.from_environ might be passing
    values to this context and nova's context drop them.

    To work around this instead of dropping values at __init__ parse the
    incoming values in from_dict so that only known arguments are passed
    into the constructor. We can then assume that all values that context
    doesn't handle are intended for the base class.

    Change-Id: I393c51e5319773420f944bb85fd215270565fcbb
    Related-Bug: #1602081

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/345633

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/345643

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/341896
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=13b5ab26861e4d6f6b5d99052004d5d1da8ccab5
Submitter: Jenkins
Branch: master

commit 13b5ab26861e4d6f6b5d99052004d5d1da8ccab5
Author: Jamie Lennox <email address hidden>
Date: Thu Jul 14 11:25:09 2016 +1000

    Use from_environ when creating a context

    The from_environ method is designed to pick up all the parameters set
    from auth_token middleware and other oslo middlewares and create a
    context with them.

    By doing this there will be information available to libraries like
    oslo.policy and oslo.logging without nova having to track each change to
    the base library.

    There is ongoing work here to move more values to the base class that
    will be cleaned up in future.

    Change-Id: I6b61028fcecb86cc6c25fb69977774e266a8ea5b
    Related-Bug: #1602081

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/345633
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=9f99f6c212c38f8ae26c6aeee4b4521cbf9207a4
Submitter: Jenkins
Branch: master

commit 9f99f6c212c38f8ae26c6aeee4b4521cbf9207a4
Author: Jamie Lennox <email address hidden>
Date: Fri Jul 22 05:21:23 2016 +1000

    Don't assert exact to_dict output

    The context.to_dict in turn calls the oslo.context to_dict. By enforcing
    the exact dictionary output of the to_dict tests will fail any time that
    oslo.context changes the output of to_dict. This happened with the
    release of oslo.context 2.6.0.

    For nova testing purposes we should only assert properties of the
    dictionary output that were added by nova and rely on oslo.context doing
    the correct thing regarding deserializing its own options.

    Related-Bug: #1602081
    Change-Id: Ib91a73a017e02b48ec01bbf8c28f946fb5120ba5

[OpenStack Infra (hudson-openstack)]

Change abandoned by Jamie Lennox (<email address hidden>) on branch: master
Review: https://review.openstack.org/345643

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: stable/mitaka
Review: https://review.openstack.org/348174

[OpenStack Infra (hudson-openstack)]

Change abandoned by ChangBo Guo(gcb) (<email address hidden>) on branch: stable/mitaka
Review: https://review.openstack.org/348174

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/342604
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=80738df83f4cdd7a3143b157b73d1a9e6b3b2c40
Submitter: Jenkins
Branch: master

commit 80738df83f4cdd7a3143b157b73d1a9e6b3b2c40
Author: Tony Breeds <email address hidden>
Date: Fri Jul 15 14:49:54 2016 +1000

    Add support for oslo.context 2.6.0

    The 2.6.0 release of oslo.context adds a new attribute
    'is_admin_project'. Always include that attribute when serializing the
    object, and expect it in our tests.

    Change-Id: I563cf810385e84ab30d49ef079b75df279006f0d
    Related-Bug: 1602081

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/340193
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=842d95bb6849058267b67965d1a6379acfe8e38c
Submitter: Jenkins
Branch: master

commit 842d95bb6849058267b67965d1a6379acfe8e38c
Author: Jamie Lennox <email address hidden>
Date: Mon Jul 11 09:28:26 2016 +1000

    Use context.from_dict to determine available arguments

    RequestContext.__init__ takes and ignores arbitrary keyword arguments.
    This leads to mistakes such as those changed in tests and will prevent
    cinder from passing those keyword arguments to the base oslo_context
    class.

    The ignored arguments are generally made up of things that oslo_context
    added to the to_dict that cinder doesn't handle. Instead of accepting
    and ignoring those arguments make from_dict smart enough to construct
    the correct arguments to the class.

    Related-Bug: #1602081
    Change-Id: Iaf37e38ddc368a6f504bab17163d7f0ba21f5029

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/340194
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=abf91a8a4e4af01ff34737386d775cd45d7d528b
Submitter: Jenkins
Branch: master

commit abf91a8a4e4af01ff34737386d775cd45d7d528b
Author: Jamie Lennox <email address hidden>
Date: Mon Jul 11 11:17:24 2016 +1000

    Use from_environ to load context

    Oslo.context provides a from_environ method that loads all the headers
    from the auth_token middleware into the correct parameters and will let
    oslo context provide new features in future.

    Related-Bug: #1602081
    Change-Id: Ie79ee8d5f40032777bf12b8798d64e9dc8141c9e

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/340205
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=8ad224d4e6bf40aeb895b526ddcaa5c00011fc46
Submitter: Jenkins
Branch: master

commit 8ad224d4e6bf40aeb895b526ddcaa5c00011fc46
Author: Jamie Lennox <email address hidden>
Date: Mon Jul 11 12:06:41 2016 +1000

    Use from_dict to load context params

    The context objects accepts and ignores unknown keyword arguments. This
    was to allow it to handle the deserialization of parameters it didn't
    understand from the base class' to_dict method. This will make it
    difficult to pass unknown attributes to the base class so fix the
    from_dict method to only accept params it knows about.

    Related-Bug: #1602081
    Change-Id: Ic58a2025680e8e1ba4f8a177d898be457e2c3160

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/340206
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=11257a31c1a5d4cf309515f8c9523990386f5ab5
Submitter: Jenkins
Branch: master

commit 11257a31c1a5d4cf309515f8c9523990386f5ab5
Author: Jamie Lennox <email address hidden>
Date: Mon Jul 11 12:36:56 2016 +1000

    Use context from_environ to load contexts

    The from_environ method loads a context object with all the appropriate
    values that are retrieved from auth_token middleware and other oslo
    middlewares. This will let oslo add new things to policy enforcement in
    future without having to manually change all services.

    To do this we need to pass keyword arguments for the context through to
    the base class.

    Related-Bug: #1602081
    Change-Id: I0533c8aee3893449b757f1e35fc89a451ae1720c

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/heat 7.0.0.0b3 development milestone.

[Ihar Hrachyshka (ihar-hrachyshka)]

Is there anything more to do in Neutron?

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/340195
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=bc5a2d9741dfea75b7be0448f7322bb1ef6f028c
Submitter: Jenkins
Branch: master

commit bc5a2d9741dfea75b7be0448f7322bb1ef6f028c
Author: Jamie Lennox <email address hidden>
Date: Mon Jul 11 11:25:46 2016 +1000

    Use to_policy_values for enforcing policy

    oslo_context's to_policy_values provides a standard list of parameters
    that policy should be able to be enforced upon. The combination of this
    and from_environ lets oslo.context handle adding new values to policy
    enforcement.

    Closes-Bug: #1602081
    Change-Id: I8f70580e7209412800aa7b948602b003392ef238

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/370499

[Ihar Hrachyshka (ihar-hrachyshka)]

It's honestly not clear to me why we track it for RC2 inclusion for neutron. Is anything broken? I doubt it.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/341905
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=304bc201c004d549de408c75cfe731eb65fde78d
Submitter: Jenkins
Branch: master

commit 304bc201c004d549de408c75cfe731eb65fde78d
Author: Adam Young <email address hidden>
Date: Mon Sep 12 21:39:45 2016 -0400

    Use to_policy_values for policy credentials

    The base oslo.context defines to_policy_values with all the information
    that it expects a service to require to enforce policy. Use that instead
    of throwing everything in to_dict at policy enforcement.

    Change-Id: I0a42b4425e9dd1bd062c48792c4d116dd370afe3
    Closes-Bug: #1602081

[Steve Martinelli (stevemar)]

Keystone patch for related work: https://review.openstack.org/#/c/371856/

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/cinder 9.0.0.0rc1 release candidate.

[OpenStack Infra (hudson-openstack)]

Change abandoned by Jamie Lennox (<email address hidden>) on branch: master
Review: https://review.openstack.org/379919
Reason: In favor of https://review.openstack.org/#/c/295371/

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/371856
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=ca73d296bd5a16435dd35cd0818f4b0a16bc2d02
Submitter: Jenkins
Branch: master

commit ca73d296bd5a16435dd35cd0818f4b0a16bc2d02
Author: Jamie Lennox <email address hidden>
Date: Sat Sep 17 12:01:26 2016 +1000

    Add is_admin_project to policy dict

    Fixing keystone's policy dict is going to be a big effort. Until then we
    can just add the is_admin_project flag from the context so that at least
    we can enforce policy on it as other projects do.

    Change-Id: I2f6731f0bfe00ae77a20a5c3015948b9ba2a191e
    Related-Bug: #1602081

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/cinder 9.0.0.0rc1 release candidate.

[OpenStack Infra (hudson-openstack)]

Change abandoned by Armando Migliaccio (<email address hidden>) on branch: master
Review: https://review.openstack.org/370499
Reason: This review is > 4 weeks without comment, and failed Jenkins the last time it was checked. We are abandoning this for now. Feel free to reactivate the review by pressing the restore button and leaving a 'recheck' comment to get fresh test results.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/nova 15.0.0.0b1 development milestone.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/295371
Committed: https://git.openstack.org/cgit/openstack/ironic/commit/?id=3eba764be3dcbafa01e1e4298bdf1810925a6f49
Submitter: Jenkins
Branch: master

commit 3eba764be3dcbafa01e1e4298bdf1810925a6f49
Author: Vladyslav Drok <email address hidden>
Date: Mon Mar 21 17:46:55 2016 +0200

    Add user and project domains to ironic context

    This change also removes most of the logic from ironic's
    RequestContext to reuse the oslo_context as much as possible.
    Usage of domain_id and domain_name in policy files is deprecated
    and their support will be removed in the Pike release. domain_id
    field was removed from the context class completely, domain_name
    value now mathces the oslo_context expectations.

    ContextHook is changed too so as not to duplicate from_environ
    functional from oslo_context.

    to_dict method left as is, so that we don't break an older service
    receiving the context over RPC. It will be changed in Pike release
    to reuse the base oslo_context class' to_dict.

    Closes-Bug: #1602081
    Closes-Bug: #1627173
    Closes-Bug: #1641972
    Co-Authored-By: Jamie Lennox <email address hidden>
    Co-Authored-By: Devananda van der Veen <email address hidden>
    Change-Id: I9afe89bc6aee282ee4b7579d661e3fa83cc0ce84

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/370499
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=2e621eeb1cdfae5ceb3c83eb6befcb954f0b6cec
Submitter: Jenkins
Branch: master

commit 2e621eeb1cdfae5ceb3c83eb6befcb954f0b6cec
Author: Jamie Lennox <email address hidden>
Date: Thu Sep 15 10:07:18 2016 +1000

    Use to_policy_values for policy enforcement

    Use the common policy values for all services when enforcing policy
    decisions. We add all possibly used policy values to maintain backwards
    compatibility.

    Change-Id: Ie1d0739ab4dfb0654d8767693dbdba5cd52a30b2
    Closes-Bug: #1602081

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron 10.0.0.0b2 development milestone.

[David Stanek (dstanek)]

Is there any keystone work that still needs to be completed?

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/ironic 7.0.0 release.

[Lance Bragstad (lbragstad)]

This seems done to me from a keystone perspective, but I'll give Jamie and opportunity to confirm.

[Lance Bragstad (lbragstad)]

Automatically unassigning due to inactivity.

[Adam Young (ayoung)]

Fixed in Keystone by f71a78db86632dccb391782e62da69a4627c7cad
https://review.openstack.org/#/c/523650/

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/zun 1.0.0 release.

[Goutham Pacha Ravi (gouthamr)]

Manila change: https://review.opendev.org/c/openstack/manila/+/775724

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/manila 12.0.0.0rc1 release candidate.