Bumping this email. Spec is merged for Ussuri cycle now.
Please refer to the below review guide for this BP work.
-gmann
---- On Thu, 15 Aug 2019 08:18:52 -0500 Ghanshyam Mann gmann@ghanshyammann.com wrote ----
Hello Everyone,
As many of you might know that in Train, we are doing Nova policy changes to adopt the keystone's new defaults and scope type[1]. There are multiple changes required per policy as mentioned in spec. I am writing this review guide for the patch sequence and at the end how each policy will look like.
I have prepared the first set of patches. I would like to get the feedback on those so that we can modify the other policy also on the same line. My plan is to start the other policy work after we merge the first set of policy changes.
Patch sequence: Example: os-services API policy:
- Cover/Improve the test coverage for existing policies:
This will be the first patch. We do not have good test coverage of the policy, current tests are not at all useful and do not perform the real checks. Idea is to add the actual tests coverage for each policy as the first patch. new tests try to access the API with all possible context and check for positive and negative cases.
- Introduce scope_types:
This will add the scope_type for policy. It will be either 'system', 'project' or 'system and project'. In the same patch, along with existing test working as it is, new tests of scope type will be added which will run with [oslo_policy] enforce_scope=True so that we can capture the real scope checks.
- Add new default roles:
This will add new defaults which can be SYSTEM_ADMIN, SYSTEM_READER, PROJECT_MEMBER_OR_SYSTEM_ADMIN, PROJECT_READER_OR_SYSTEM_READER etc depends on Policy. Test coverage of new defaults, as well as deprecated defaults, are covered in same patch. This patch will add the granularity in policy if needed. Without policy granularity, we cannot add new defaults per rule.
- https://review.opendev.org/#/c/648480/ (I need to add more tests for deprecated rules)
- Pass actual Targets in policy:
This is to pass the actual targets in context.can(). Main goal is to remove the defaults targets which is nothing but context'user_id,project_id only. It will be {} if no actual target data needed in check_str.
Patch sequence: Example: Admin Action API policy:
- https://review.opendev.org/#/c/657698/
- https://review.opendev.org/#/c/657823/
- https://review.opendev.org/#/c/676682/
- https://review.opendev.org/#/c/663095/
There are other patches I have posted in between for common changes or fix or framework etc.
[1] https://specs.openstack.org/openstack/nova-specs/specs/train/approved/policy...
-gmann