From 41fe927c8067ed2f1a1075c7fdf7799b91af95ef Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Fri, 22 Jun 2018 17:32:21 +0100 Subject: [PATCH] Add missing policy actions to policy.json file This patchset adds missing policy actions to the policy.json file for several reasons: 1) It signals to operators all the policy actions that are enforced in the system. With the governance spec [0] urging projects toward policy in code documentation, it makes sense to document all policy actions in the policy.json as Neutron doesn't have policy in code. 2) It is consistent with Neutron's policy enforcement documentation [1]: "For each attribute which has been explicitly specified in the request create a rule matching policy names in the form _: rule" So it makes sense to capture each policy that is enforced, including all those with these special attributes. 3) Why include "update_router:external_gateway_info" but not "create_router:external_gateway_info"? This is inconsistent. 4) It makes it difficult to validate Neutron's policy via Patrole if the policies aren't contained in the policy.json -- how else is it possible to determine which policies to expect if they aren't documented anywhere? [0] https://governance.openstack.org/tc/goals/queens/policy-in-code.html [1] https://docs.openstack.org/neutron/pike/contributor/internals/policy.html#authorization-workflow Change-Id: I40f84134f0b56cfd574dfd69e5ebbf6a3fc2b3df --- etc/policy.json | 4 ++++ neutron/tests/etc/policy.json | 4 ++++ neutron/tests/unit/extensions/test_l3.py | 7 +++---- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/etc/policy.json b/etc/policy.json index 9c55f01157e..0e8fe5bd04c 100644 --- a/etc/policy.json +++ b/etc/policy.json @@ -73,6 +73,7 @@ "create_port": "", "create_port:device_owner": "not rule:network_device or rule:context_is_advsvc or rule:admin_or_network_owner", "create_port:mac_address": "rule:context_is_advsvc or rule:admin_or_network_owner", + "create_port:fixed_ips": "rule:context_is_advsvc or rule:admin_or_network_owner", "create_port:fixed_ips:ip_address": "rule:context_is_advsvc or rule:admin_or_network_owner", "create_port:fixed_ips:subnet_id": "rule:context_is_advsvc or rule:admin_or_network_owner or rule:shared", "create_port:port_security_enabled": "rule:context_is_advsvc or rule:admin_or_network_owner", @@ -89,6 +90,7 @@ "update_port": "rule:admin_or_owner or rule:context_is_advsvc", "update_port:device_owner": "not rule:network_device or rule:context_is_advsvc or rule:admin_or_network_owner", "update_port:mac_address": "rule:admin_only or rule:context_is_advsvc", + "update_port:fixed_ips": "rule:context_is_advsvc or rule:admin_or_network_owner", "update_port:fixed_ips:ip_address": "rule:context_is_advsvc or rule:admin_or_network_owner", "update_port:fixed_ips:subnet_id": "rule:context_is_advsvc or rule:admin_or_network_owner or rule:shared", "update_port:port_security_enabled": "rule:context_is_advsvc or rule:admin_or_network_owner", @@ -100,6 +102,8 @@ "delete_port": "rule:context_is_advsvc or rule:admin_owner_or_network_owner", "create_router": "rule:regular_user", + "create_router:external_gateway_info": "rule:admin_or_owner", + "create_router:external_gateway_info:network_id": "rule:admin_or_owner", "create_router:external_gateway_info:enable_snat": "rule:admin_only", "create_router:external_gateway_info:external_fixed_ips": "rule:admin_only", "create_router:distributed": "rule:admin_only", diff --git a/neutron/tests/etc/policy.json b/neutron/tests/etc/policy.json index 9c55f01157e..0e8fe5bd04c 100644 --- a/neutron/tests/etc/policy.json +++ b/neutron/tests/etc/policy.json @@ -73,6 +73,7 @@ "create_port": "", "create_port:device_owner": "not rule:network_device or rule:context_is_advsvc or rule:admin_or_network_owner", "create_port:mac_address": "rule:context_is_advsvc or rule:admin_or_network_owner", + "create_port:fixed_ips": "rule:context_is_advsvc or rule:admin_or_network_owner", "create_port:fixed_ips:ip_address": "rule:context_is_advsvc or rule:admin_or_network_owner", "create_port:fixed_ips:subnet_id": "rule:context_is_advsvc or rule:admin_or_network_owner or rule:shared", "create_port:port_security_enabled": "rule:context_is_advsvc or rule:admin_or_network_owner", @@ -89,6 +90,7 @@ "update_port": "rule:admin_or_owner or rule:context_is_advsvc", "update_port:device_owner": "not rule:network_device or rule:context_is_advsvc or rule:admin_or_network_owner", "update_port:mac_address": "rule:admin_only or rule:context_is_advsvc", + "update_port:fixed_ips": "rule:context_is_advsvc or rule:admin_or_network_owner", "update_port:fixed_ips:ip_address": "rule:context_is_advsvc or rule:admin_or_network_owner", "update_port:fixed_ips:subnet_id": "rule:context_is_advsvc or rule:admin_or_network_owner or rule:shared", "update_port:port_security_enabled": "rule:context_is_advsvc or rule:admin_or_network_owner", @@ -100,6 +102,8 @@ "delete_port": "rule:context_is_advsvc or rule:admin_owner_or_network_owner", "create_router": "rule:regular_user", + "create_router:external_gateway_info": "rule:admin_or_owner", + "create_router:external_gateway_info:network_id": "rule:admin_or_owner", "create_router:external_gateway_info:enable_snat": "rule:admin_only", "create_router:external_gateway_info:external_fixed_ips": "rule:admin_only", "create_router:distributed": "rule:admin_only", diff --git a/neutron/tests/unit/extensions/test_l3.py b/neutron/tests/unit/extensions/test_l3.py index e6a949ce315..6c54fa79787 100644 --- a/neutron/tests/unit/extensions/test_l3.py +++ b/neutron/tests/unit/extensions/test_l3.py @@ -1298,8 +1298,7 @@ class L3NatTestCaseBase(L3NatTestCaseMixin): 'ip_address': s2['subnet']['gateway_ip']} with self.port(subnet=s1, fixed_ips=fixed_ips, - tenant_id=router_tenant_id, - set_context=True) as p: + tenant_id=router_tenant_id) as p: kwargs = {'expected_code': expected_code} if not router_action_as_admin: kwargs['tenant_id'] = router_tenant_id @@ -1800,7 +1799,7 @@ class L3NatTestCaseBase(L3NatTestCaseMixin): gw_info = body['router']['external_gateway_info'] self.assertIsNone(gw_info) - def test_create_router_port_with_device_id_of_other_teants_router(self): + def test_create_router_port_with_device_id_of_other_tenants_router(self): with self.router() as admin_router: with self.network(tenant_id='tenant_a', set_context=True) as n: @@ -1814,7 +1813,7 @@ class L3NatTestCaseBase(L3NatTestCaseMixin): set_context=True, expected_res_status=exc.HTTPConflict.code) - def test_create_non_router_port_device_id_of_other_teants_router_update( + def test_create_non_router_port_device_id_of_other_tenants_router_update( self): # This tests that HTTPConflict is raised if we create a non-router # port that matches the device_id of another tenants router and then