From 97fffede9ed3037469be62961d45d5bfe474a4ac Mon Sep 17 00:00:00 2001 From: Rick Bartra Date: Thu, 12 Sep 2019 13:35:10 -0400 Subject: [PATCH] fix: admin, member, and reader gates broken Recent changes in Keystone to move trust enforcement [0] to default policies is currently breaking several voting gates in Patrole. This commit updates the trusts_rbac tests to account for these changes. Additionally, 'test_list_trusts' is updated so that it does indeed test 'identity:list_trusts'. If a 'trustor_user_id' or 'trustee_user_id' is passed into list_trusts() then a different policy action will be enforced. A future commit will add tests for the actions added here [1]. Added new feature flag called ``keystone_policy_enforcement_train`` under the configuration group ``[policy-feature-enabled]`` to make ``test_list_trusts`` test backwards compatible, test the current release, and test the correct policy action. The Keystone Trust API is enforced differently depending on passed arguments. The new feature flag is needed so that all the voting gates pass, otherwise the 'test_list_trusts' is not backwards compatible and would not test the correct policy action in the current release. [0] https://review.opendev.org/#/q/topic:trust-policies+(status:open+OR+status:merged) [1] https://review.opendev.org/#/c/675807/10/keystone/common/policies/trust.py Change-Id: Ia5661e12977b26e1c16f09a074d1a805263c6c22 --- devstack/plugin.sh | 26 ++++++++++++++++ patrole_tempest_plugin/config.py | 7 ++++- .../tests/api/identity/v3/test_trusts_rbac.py | 31 +++++++++++++++---- ...cy_enforcement_rocky-b52fb471ac31189b.yaml | 7 +++++ 4 files changed, 64 insertions(+), 7 deletions(-) create mode 100644 releasenotes/notes/keystone_policy_enforcement_rocky-b52fb471ac31189b.yaml diff --git a/devstack/plugin.sh b/devstack/plugin.sh index 6b95182c..f60f0f45 100644 --- a/devstack/plugin.sh +++ b/devstack/plugin.sh @@ -37,6 +37,12 @@ function install_patrole_tempest_plugin { iniset $TEMPEST_CONFIG policy-feature-enabled removed_nova_policies_stein False iniset $TEMPEST_CONFIG policy-feature-enabled removed_keystone_policies_stein False iniset $TEMPEST_CONFIG policy-feature-enabled added_cinder_policies_stein False + + # TODO(rb560u): Remove this once stable/pike becomes EOL. + # Make the 'test_list_trusts' test backwards compatible. + # The Keystone Trust API is enforced differently depending on passed + # arguments + iniset $TEMPEST_CONFIG policy-feature-enabled keystone_policy_enforcement_train False fi if [[ ${DEVSTACK_SERIES} == 'queens' ]]; then @@ -54,12 +60,32 @@ function install_patrole_tempest_plugin { iniset $TEMPEST_CONFIG policy-feature-enabled removed_nova_policies_stein False iniset $TEMPEST_CONFIG policy-feature-enabled removed_keystone_policies_stein False iniset $TEMPEST_CONFIG policy-feature-enabled added_cinder_policies_stein False + + # TODO(rb560u): Remove this once stable/queens becomes EOL. + # Make the 'test_list_trusts' test backwards compatible. + # The Keystone Trust API is enforced differently depending on passed + # arguments + iniset $TEMPEST_CONFIG policy-feature-enabled keystone_policy_enforcement_train False fi if [[ ${DEVSTACK_SERIES} == 'rocky' ]]; then # TODO(cl566n): Policies used by Patrole testing. Remove these once stable/rocky becomes EOL. iniset $TEMPEST_CONFIG policy-feature-enabled added_cinder_policies_stein False iniset $TEMPEST_CONFIG policy-feature-enabled removed_keystone_policies_stein False + + # TODO(rb560u): Remove this once stable/rocky becomes EOL. + # Make the 'test_list_trusts' test backwards compatible. + # The Keystone Trust API is enforced differently depending on passed + # arguments + iniset $TEMPEST_CONFIG policy-feature-enabled keystone_policy_enforcement_train False + fi + + if [[ ${DEVSTACK_SERIES} == 'stein' ]]; then + # TODO(rb560u): Remove this once stable/stein becomes EOL. + # Make the 'test_list_trusts' test backwards compatible. + # The Keystone Trust API is enforced differently depending on passed + # arguments + iniset $TEMPEST_CONFIG policy-feature-enabled keystone_policy_enforcement_train False fi iniset $TEMPEST_CONFIG patrole rbac_test_roles $RBAC_TEST_ROLES diff --git a/patrole_tempest_plugin/config.py b/patrole_tempest_plugin/config.py index 63f0a8a9..5eab0e3b 100644 --- a/patrole_tempest_plugin/config.py +++ b/patrole_tempest_plugin/config.py @@ -184,7 +184,12 @@ were removed in Stein."""), default=True, help="""Are the Cinder Stein policies available in the cloud (e.g. [create|update|get|delete]_encryption_policy)? These policies are added -in Stein.""") +in Stein."""), + cfg.BoolOpt('keystone_policy_enforcement_train', + default=True, + help="""Is the cloud running the Train release or newer? If +so, the Keystone Trust API is enforced differently depending on passed +arguments""") ] diff --git a/patrole_tempest_plugin/tests/api/identity/v3/test_trusts_rbac.py b/patrole_tempest_plugin/tests/api/identity/v3/test_trusts_rbac.py index c8db04e0..41c9bf58 100644 --- a/patrole_tempest_plugin/tests/api/identity/v3/test_trusts_rbac.py +++ b/patrole_tempest_plugin/tests/api/identity/v3/test_trusts_rbac.py @@ -94,7 +94,10 @@ class IdentityTrustV3RbacTest(rbac_base.BaseIdentityV3RbacTest): @decorators.idempotent_id('d9a6fd06-08f6-462c-a86c-ce009adf1230') @rbac_rule_validation.action( service="keystone", - rules=["identity:delete_trust"]) + rules=["identity:delete_trust"], + extra_target_data={ + "target.trust.trustor_user_id": "os_primary.credentials.user_id" + }) def test_delete_trust(self): trust = self.setup_test_trust(trustor_user_id=self.trustor_user_id, trustee_user_id=self.trustee_user_id) @@ -107,14 +110,24 @@ class IdentityTrustV3RbacTest(rbac_base.BaseIdentityV3RbacTest): service="keystone", rules=["identity:list_trusts"]) def test_list_trusts(self): + # Depending on the passed arguments to the list trusts API, different + # policy actions are enforced. + feature_flag = \ + CONF.policy_feature_enabled.keystone_policy_enforcement_train with self.override_role(): - self.trusts_client.list_trusts( - trustor_user_id=self.trustor_user_id) + if feature_flag: + self.trusts_client.list_trusts() + else: + self.trusts_client.list_trusts( + trustor_user_id=self.trustor_user_id) @decorators.idempotent_id('3c9ff92f-a73e-4f9b-8865-e017f38c70f5') @rbac_rule_validation.action( service="keystone", - rules=["identity:list_roles_for_trust"]) + rules=["identity:list_roles_for_trust"], + extra_target_data={ + "target.trust.trustor_user_id": "os_primary.credentials.user_id" + }) def test_list_roles_for_trust(self): with self.override_role(): self.trusts_client.list_trust_roles(self.trust['id']) @@ -122,7 +135,10 @@ class IdentityTrustV3RbacTest(rbac_base.BaseIdentityV3RbacTest): @decorators.idempotent_id('3bb4f97b-cecd-4c7d-ad10-b88ee6c5d573') @rbac_rule_validation.action( service="keystone", - rules=["identity:get_role_for_trust"]) + rules=["identity:get_role_for_trust"], + extra_target_data={ + "target.trust.trustor_user_id": "os_primary.credentials.user_id" + }) def test_show_trust_role(self): with self.override_role(): self.trusts_client.show_trust_role( @@ -131,7 +147,10 @@ class IdentityTrustV3RbacTest(rbac_base.BaseIdentityV3RbacTest): @decorators.idempotent_id('0184e0fb-641e-4b52-ab73-81c1ce6ca5c1') @rbac_rule_validation.action( service="keystone", - rules=["identity:get_trust"]) + rules=["identity:get_trust"], + extra_target_data={ + "target.trust.trustor_user_id": "os_primary.credentials.user_id" + }) def test_show_trust(self): with self.override_role(): self.trusts_client.show_trust(self.trust['id']) diff --git a/releasenotes/notes/keystone_policy_enforcement_rocky-b52fb471ac31189b.yaml b/releasenotes/notes/keystone_policy_enforcement_rocky-b52fb471ac31189b.yaml new file mode 100644 index 00000000..1520cab1 --- /dev/null +++ b/releasenotes/notes/keystone_policy_enforcement_rocky-b52fb471ac31189b.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + Added new feature flag called ``keystone_policy_enforcement_train`` under + the configuration group ``[policy-feature-enabled]`` to make ``test_list_trusts`` + test backwards compatible, test the current release, and test the correct policy + action. The Keystone Trust API is enforced differently depending on passed arguments