Revert "Enforcing scope with SRBAC breaks heat"

[SeanMooney]

Reverts #395

this is incorrect for 2 reasons

first we have an explicit design goal to not add feature flags to CRD for config options.
this patch violates that design goal

second disabling srbac is something that has to be done on all services together and its not planned ot be confiruable in any service.

we intend to require the new policies and scope enforcement and enable them for all adopted and greenfield deployments. #395 is not inline with that goal.

Related: OSPRH-1492

[SeanMooney]

note we need to review how to proceed with this so ill set a hold on this until that happens

[rabi]

first we have an explicit design goal to not add feature flags to CRD for config options.

Well, though both the features (a. new policy defaults b. enforce scope) are part of SRBAC, they're not only config options. It should be possible to enable/disable scope enforcement, as many NFV customers won't be using that.

second disabling srbac is something that has to be done on all services together and its not planned ot be confiruable in any service.

This is not about disabling SRBAC for a service. Heat srbac can be enabled as it does honor scope enforcement at all. However, heat has an existing design where domain admins should be able to list roles and domain users should be able to manage credentials, hence scope enforcement in keystone won't work. We've some workarounds https://review.opendev.org/c/openstack/keystone/+/914759 and https://review.opendev.org/c/openstack/keystone/+/916130 now merged in keystone to allow that (which IMO orthogonal to scope enforcement), but have to be backported to antelope for scope enforcement to work in OSP18.

we intend to require the new policies and scope enforcement and enable them for all adopted and greenfield deployments. #395 is not inline with that goal.

Why is this a goal before it's the default upstream[1]? Have we evaluated the impact across openstack projects before defining the goal for OSP18? I would also like to understand what is driving scope enforcement from customer requirement pov.

[1] https://github.com/openstack/oslo.policy/blob/master/oslo_policy/opts.py#L28

[vyzigold]

If I remember correctly, this prevented heat from working, so we can't just revert it unless we want to end up with a not working service. The PR was a requirement for Autoscaling to work (which uses heat).

[dmendiza]

It should be possible to enable/disable scope enforcement

I don't think this is true for Keystone. All the new default policies for Keystone were written with scope in mind, and Keystone requires that either both enforce_new_defaults and enforce_scope both be ture, or both false. Keystone does not test or support any other combinations.

Disabling enforce_scope in keystone changes the meaning of a lot of policies, not just the policies for the APIs that Heat uses with domain-scope tokens. The change that is being reverted here causes keystone tempest tests for Secure RBAC to fail, so this patch is needed to enable the correct policy enforcement in Keystone.

[rabi]

The change that is being reverted here causes keystone tempest tests for Secure RBAC to fail, so this patch is needed to enable the correct policy enforcement in Keystone.

Are we saying that someone can't use a custom config to toggle enforce_scope if they want with keystone? The failing tempest tests you mentioned could be assuming that either both are enabled or disabled which could be just a test assumption.

Having said that, as I mentioned earlier we should ensure that every service that is using keystone should work as before with downstream testing, before we enable this for OSP18 or wait for it to be the default upstream in 2024.2 release[1] which means it won't be the default for OSP18.

[1] https://review.opendev.org/c/openstack/governance/+/915179

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: d34dh0r53, SeanMooney
Once this PR has been reviewed and has the lgtm label, please assign olliewalsh for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[xek]

/lgtm

[dmendiza]

Are we saying that someone can't use a custom config to toggle enforce_scope if they want with keystone?

Yes, that is what we are saying. RHOS 18 will only support having SRBAC turned on, which for keystone means both enforce_scope and enforce_new_defaults set to true. We will not allow customers to choose to disable either option.

The failing tempest tests you mentioned could be assuming that either both are enabled or disabled which could be just a test assumption.

No, it's not just a test assumption. The policies were changed when we implemented Phase 1 of the SRBAC community goal in Keystone, which made enforce_scope = true a required setting for SRBAC to be enabled. The Keystone team does not support mixed true/false settings in the options that control SRBAC.

we should ensure that every service that is using keystone should work as before with downstream testing, before we enable this for OSP18

The decision to enable SRBAC by default in RHOS 18 was made a long time ago. I think we were clear with the requirements in the JIRA epic: https://issues.redhat.com/browse/OSPRH-1513

[rabi]

The decision to enable SRBAC by default in RHOS 18 was made a long time ago. I think we were clear with the requirements in the JIRA epic: https://issues.redhat.com/browse/OSPRH-1513

As asked earlier did we evaluate the impact on other services before making the decision and discussed with other teams, as this is something that we're doing before it's done upstream?

This revert can't be merged without back-porting the kesytone patches I mentioned earlier to OSP18.

[rabi]

Yes, that is what we are saying. RHOS 18 will only support having SRBAC turned on, which for keystone means both enforce_scope and enforce_new_defaults set to true. We will not allow customers to choose to disable either option.

How do you ensure that? I don't see anything in keystone-operator handling of customServiceConfig that can ensure it.

[dmendiza]

did we evaluate the impact on other services before making the decision and discussed with other teams

Yes, the feature epic is here: https://issues.redhat.com/browse/OSPRH-1492 ... requirements were communicated in the weekly program call as well as the weekly podified control plane meetings. For heat specifically the epic is here: https://issues.redhat.com/browse/OSPRH-6888

It's up to each team to evaluate impact and fix anything that needs fixing, which in this case meant updating Keystone policies needed by Heat, not turning off scope enforcement.

[rabi]

requirements were communicated in the weekly program call as well as the weekly podified control plane meetings.

I don't think everyone attends these calls/meetings. Did we communicate teams to evaluate the impact asynchronously?

For heat specifically the epic is here: https://issues.redhat.com/browse/OSPRH-6888

This epic is not related to what we're discussing here. You can enable enforce_new_defaults, enforce_scope in for heat. The issue is specific to keystone policies and what heat expects from keystone.

[dmendiza]

I don't see anything in keystone-operator handling of customServiceConfig that can ensure it

Currently keystone-operator does not prevent someone from adding unsupported settings in customServiceConfig. If a customer really wanted to, they could set those configuration settings to unsupported values, but they will likely run into unexpected behaviors.

[d34dh0r53]

I've removed my LGTM label from this for the time being, we need to get the backports of these two issues 1 and 2 down to 2023.1 before we can merge this so at to not break Heat.

[bshephar]

Just to clarify things since my Epic was mentioned here. That Epic only relates to enabling a CRD interface to enable SRBAC - although I echo Sean's concerns in that I didn't think we were doing this. So my Epic wont fix this particular problem, since it can already be enabled in Heat without the related PR:
openstack-k8s-operators/heat-operator#362

Just clarifying that I don't think my Epic is relevant to this particular discussion to avoid unnecessary confusion.

[dmendiza]

The policy changes to allow domain-scoped tokens required by Heat -> Keystone integration have merged into the stable/2023.1 branch upstream:

• https://opendev.org/openstack/keystone/commit/17da229f0d05bf0298ab5661d695015e63d7b10c
• https://opendev.org/openstack/keystone/commit/b6c20d912bf15158ee6580bfee4fc1ad4f0c2458

I think we can go ahead and merge this patch now?

[rabi]

I can see a new tag in https://quay.io/repository/podified-antelope-centos9/openstack-keystone?tab=tags after the keystone changes merged. Though it would be wise to run heat tempest tests to see if nothing else is broken before merging this, I'll remove the block.

Relevant keystone patches backported.