diff --git a/etc/nova/policy.json b/etc/nova/policy.json index 51c9bc1445ba..0ae809273f2d 100644 --- a/etc/nova/policy.json +++ b/etc/nova/policy.json @@ -272,6 +272,8 @@ "os_compute_api:os-flavor-manage": "rule:admin_api", "os_compute_api:os-floating-ip-dns": "", "os_compute_api:os-floating-ip-dns:discoverable": "", + "os_compute_api:os-floating-ip-dns:domain:update": "rule:admin_api", + "os_compute_api:os-floating-ip-dns:domain:delete": "rule:admin_api", "os_compute_api:os-floating-ip-pools": "", "os_compute_api:os-floating-ip-pools:discoverable": "", "os_compute_api:os-floating-ips": "", diff --git a/nova/api/openstack/compute/contrib/floating_ip_dns.py b/nova/api/openstack/compute/contrib/floating_ip_dns.py index d8745a63fced..e46254e1f9eb 100644 --- a/nova/api/openstack/compute/contrib/floating_ip_dns.py +++ b/nova/api/openstack/compute/contrib/floating_ip_dns.py @@ -19,6 +19,7 @@ import webob from nova.api.openstack import extensions from nova.api.openstack import wsgi +from nova import context as nova_context from nova import exception from nova.i18n import _ from nova import network @@ -105,6 +106,9 @@ class FloatingIPDNSDomainController(object): """Add or modify domain entry.""" context = req.environ['nova.context'] authorize(context) + # NOTE(shaohe-feng): back-compatible with db layer hard-code + # admin permission checks. + nova_context.require_admin_context(context) fqdomain = _unquote_domain(id) try: entry = body['domain_entry'] @@ -138,6 +142,9 @@ class FloatingIPDNSDomainController(object): """Delete the domain identified by id.""" context = req.environ['nova.context'] authorize(context) + # NOTE(shaohe-feng): back-compatible with db layer hard-code + # admin permission checks. + nova_context.require_admin_context(context) domain = _unquote_domain(id) # Delete the whole domain diff --git a/nova/api/openstack/compute/plugins/v3/floating_ip_dns.py b/nova/api/openstack/compute/plugins/v3/floating_ip_dns.py index df65df1aab11..9b22e404ba85 100644 --- a/nova/api/openstack/compute/plugins/v3/floating_ip_dns.py +++ b/nova/api/openstack/compute/plugins/v3/floating_ip_dns.py @@ -111,7 +111,7 @@ class FloatingIPDNSDomainController(wsgi.Controller): def update(self, req, id, body): """Add or modify domain entry.""" context = req.environ['nova.context'] - authorize(context) + authorize(context, action="domain:update") fqdomain = _unquote_domain(id) entry = body['domain_entry'] scope = entry['scope'] @@ -146,7 +146,7 @@ class FloatingIPDNSDomainController(wsgi.Controller): def delete(self, req, id): """Delete the domain identified by id.""" context = req.environ['nova.context'] - authorize(context) + authorize(context, action="domain:delete") domain = _unquote_domain(id) # Delete the whole domain diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 4eb12a8fe44c..2a69016c80f7 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -1072,7 +1072,6 @@ def _dnsdomain_get_or_create(context, session, fqdomain): return domain_ref -@require_admin_context def dnsdomain_register_for_zone(context, fqdomain, zone): session = get_session() with session.begin(): @@ -1082,7 +1081,6 @@ def dnsdomain_register_for_zone(context, fqdomain, zone): session.add(domain_ref) -@require_admin_context def dnsdomain_register_for_project(context, fqdomain, project): session = get_session() with session.begin(): @@ -1092,7 +1090,6 @@ def dnsdomain_register_for_project(context, fqdomain, project): session.add(domain_ref) -@require_admin_context def dnsdomain_unregister(context, fqdomain): model_query(context, models.DNSDomain).\ filter_by(domain=fqdomain).\ diff --git a/nova/tests/unit/api/openstack/compute/contrib/test_floating_ip_dns.py b/nova/tests/unit/api/openstack/compute/contrib/test_floating_ip_dns.py index 347fccdd2620..2190be4ed87b 100644 --- a/nova/tests/unit/api/openstack/compute/contrib/test_floating_ip_dns.py +++ b/nova/tests/unit/api/openstack/compute/contrib/test_floating_ip_dns.py @@ -143,6 +143,7 @@ class FloatingIpDNSTestV21(test.TestCase): self.domain_controller = temp self.entry_controller = self.floating_ip_dns.\ FloatingIPDNSEntryController() + self.admin_req = fakes.HTTPRequest.blank('', use_admin_context=True) self.req = fakes.HTTPRequest.blank('') def tearDown(self): @@ -211,25 +212,28 @@ class FloatingIpDNSTestV21(test.TestCase): self.assertEqual(entry['dns_entry']['ip'], test_ipv4_address) def test_create_domain(self): + self._test_create_domain(self.req) + + def _test_create_domain(self, req): body = {'domain_entry': {'scope': 'private', 'project': 'testproject'}} self.assertRaises(self._bad_request(), - self.domain_controller.update, - self.req, _quote_domain(domain), body=body) + self.domain_controller.update, req, + _quote_domain(domain), body=body) body = {'domain_entry': {'scope': 'public', 'availability_zone': 'zone1'}} self.assertRaises(self._bad_request(), - self.domain_controller.update, - self.req, _quote_domain(domain), body=body) + self.domain_controller.update, req, + _quote_domain(domain), body=body) body = {'domain_entry': {'scope': 'public', 'project': 'testproject'}} - entry = self.domain_controller.update(self.req, _quote_domain(domain), - body=body) + entry = self.domain_controller.update(req, + _quote_domain(domain), body=body) self.assertEqual(entry['domain_entry']['domain'], domain) self.assertEqual(entry['domain_entry']['scope'], 'public') self.assertEqual(entry['domain_entry']['project'], 'testproject') @@ -237,8 +241,8 @@ class FloatingIpDNSTestV21(test.TestCase): body = {'domain_entry': {'scope': 'private', 'availability_zone': 'zone1'}} - entry = self.domain_controller.update(self.req, _quote_domain(domain), - body=body) + entry = self.domain_controller.update(req, + _quote_domain(domain), body=body) self.assertEqual(entry['domain_entry']['domain'], domain) self.assertEqual(entry['domain_entry']['scope'], 'private') self.assertEqual(entry['domain_entry']['availability_zone'], 'zone1') @@ -270,6 +274,9 @@ class FloatingIpDNSTestV21(test.TestCase): name) def test_delete_domain(self): + self._test_delete_domain(self.req) + + def _test_delete_domain(self, req): calls = [] def network_delete_dns_domain(fakeself, context, fqdomain): @@ -278,20 +285,25 @@ class FloatingIpDNSTestV21(test.TestCase): self.stubs.Set(network.api.API, "delete_dns_domain", network_delete_dns_domain) - res = self.domain_controller.delete(self.req, _quote_domain(domain)) + res = self.domain_controller.delete(req, + _quote_domain(domain)) self._check_status(202, res, self.domain_controller.delete) self.assertEqual([domain], calls) def test_delete_domain_notfound(self): + self._test_delete_domain_notfound(self.req) + + def _test_delete_domain_notfound(self, req): def delete_dns_domain_notfound(fakeself, context, fqdomain): raise exception.NotFound self.stubs.Set(network.api.API, "delete_dns_domain", delete_dns_domain_notfound) - self.assertRaises(webob.exc.HTTPNotFound, - self.domain_controller.delete, self.req, _quote_domain(domain)) + self.assertRaises( + webob.exc.HTTPNotFound, self.domain_controller.delete, + req, _quote_domain(domain)) def test_modify(self): body = {'dns_entry': @@ -329,7 +341,7 @@ class FloatingIpDNSTestV21(test.TestCase): with mock.patch.object(network.api.API, 'delete_dns_domain', side_effect=NotImplementedError()): self.assertRaises(webob.exc.HTTPNotImplemented, - self.domain_controller.delete, self.req, + self.domain_controller.delete, self.admin_req, _quote_domain(domain)) def test_not_implemented_create_domain(self): @@ -339,8 +351,8 @@ class FloatingIpDNSTestV21(test.TestCase): with mock.patch.object(network.api.API, 'create_private_dns_domain', side_effect=NotImplementedError()): self.assertRaises(webob.exc.HTTPNotImplemented, - self.domain_controller.update, - self.req, _quote_domain(domain), body=body) + self.domain_controller.update, self.admin_req, + _quote_domain(domain), body=body) def test_not_implemented_dns_domains_list(self): with mock.patch.object(network.api.API, 'get_dns_domains', @@ -358,6 +370,28 @@ class FloatingIpDNSTestV2(FloatingIpDNSTestV21): def _bad_request(self): return webob.exc.HTTPUnprocessableEntity + def test_update_dns_domain_with_non_admin(self): + body = {'domain_entry': + {'scope': 'private', + 'project': 'testproject'}} + self.assertRaises(exception.AdminRequired, + self.domain_controller.update, + self.req, _quote_domain(domain), body=body) + + def test_delete_dns_domain_with_non_admin(self): + self.assertRaises(exception.AdminRequired, + self.domain_controller.delete, + self.req, _quote_domain(domain)) + + def test_create_domain(self): + self._test_create_domain(self.admin_req) + + def test_delete_domain(self): + self._test_delete_domain(self.admin_req) + + def test_delete_domain_notfound(self): + self._test_delete_domain_notfound(self.admin_req) + class FloatingIPDNSDomainPolicyEnforcementV21(test.NoDBTestCase): @@ -369,14 +403,18 @@ class FloatingIPDNSDomainPolicyEnforcementV21(test.NoDBTestCase): self.req = fakes.HTTPRequest.blank('') def test_get_floating_ip_dns_policy_failed(self): + rule_name = "os_compute_api:os-floating-ip-dns" + self.policy.set_rules({rule_name: "project:non_fake"}) exc = self.assertRaises( exception.PolicyNotAuthorized, self.controller.index, self.req) self.assertEqual( - "Policy doesn't allow %s to be performed." % self.rule_name, + "Policy doesn't allow %s to be performed." % rule_name, exc.format_message()) def test_update_floating_ip_dns_policy_failed(self): + rule_name = "os_compute_api:os-floating-ip-dns:domain:update" + self.policy.set_rules({rule_name: "project:non_fake"}) body = {'domain_entry': {'scope': 'public', 'project': 'testproject'}} @@ -384,15 +422,17 @@ class FloatingIPDNSDomainPolicyEnforcementV21(test.NoDBTestCase): exception.PolicyNotAuthorized, self.controller.update, self.req, _quote_domain(domain), body=body) self.assertEqual( - "Policy doesn't allow %s to be performed." % self.rule_name, + "Policy doesn't allow %s to be performed." % rule_name, exc.format_message()) def test_delete_floating_ip_dns_policy_failed(self): + rule_name = "os_compute_api:os-floating-ip-dns:domain:delete" + self.policy.set_rules({rule_name: "project:non_fake"}) exc = self.assertRaises( exception.PolicyNotAuthorized, self.controller.delete, self.req, _quote_domain(domain)) self.assertEqual( - "Policy doesn't allow %s to be performed." % self.rule_name, + "Policy doesn't allow %s to be performed." % rule_name, exc.format_message()) diff --git a/nova/tests/unit/fake_policy.py b/nova/tests/unit/fake_policy.py index 2219fd5cb0f8..12609e5c899e 100644 --- a/nova/tests/unit/fake_policy.py +++ b/nova/tests/unit/fake_policy.py @@ -237,6 +237,8 @@ policy_data = """ "os_compute_api:os-flavors:discoverable": "", "compute_extension:floating_ip_dns": "", "os_compute_api:os-floating-ip-dns": "", + "os_compute_api:os-floating-ip-dns:domain:update": "", + "os_compute_api:os-floating-ip-dns:domain:delete": "", "compute_extension:floating_ip_pools": "", "os_compute_api:os-floating-ip-pools": "", "compute_extension:floating_ips": "", diff --git a/nova/tests/unit/network/test_manager.py b/nova/tests/unit/network/test_manager.py index 7f3610ddce3b..c9dfc1851e5f 100644 --- a/nova/tests/unit/network/test_manager.py +++ b/nova/tests/unit/network/test_manager.py @@ -3188,21 +3188,14 @@ class FloatingIPTestCase(test.TestCase): name1, zone) def test_floating_dns_domains_public(self): - zone1 = "testzone" domain1 = "example.org" domain2 = "example.com" address1 = '10.10.10.10' entryname = 'testentry' - context_admin = context.RequestContext('testuser', 'testproject', - is_admin=True) - - self.assertRaises(exception.AdminRequired, - self.network.create_public_dns_domain, self.context, - domain1, zone1) - self.network.create_public_dns_domain(context_admin, domain1, + self.network.create_public_dns_domain(self.context, domain1, 'testproject') - self.network.create_public_dns_domain(context_admin, domain2, + self.network.create_public_dns_domain(self.context, domain2, 'fakeproject') domains = self.network.get_dns_domains(self.context) @@ -3219,11 +3212,8 @@ class FloatingIPTestCase(test.TestCase): self.assertEqual(len(entries), 1) self.assertEqual(entries[0], address1) - self.assertRaises(exception.AdminRequired, - self.network.delete_dns_domain, self.context, - domain1) - self.network.delete_dns_domain(context_admin, domain1) - self.network.delete_dns_domain(context_admin, domain2) + self.network.delete_dns_domain(self.context, domain1) + self.network.delete_dns_domain(self.context, domain2) # Verify that deleting the domain deleted the associated entry entries = self.network.get_dns_entries_by_name(self.context, @@ -3436,23 +3426,13 @@ class InstanceDNSTestCase(test.TestCase): zone1 = 'testzone' domain1 = 'example.org' - context_admin = context.RequestContext('testuser', 'testproject', - is_admin=True) - - self.assertRaises(exception.AdminRequired, - self.network.create_private_dns_domain, self.context, - domain1, zone1) - - self.network.create_private_dns_domain(context_admin, domain1, zone1) + self.network.create_private_dns_domain(self.context, domain1, zone1) domains = self.network.get_dns_domains(self.context) self.assertEqual(len(domains), 1) self.assertEqual(domains[0]['domain'], domain1) self.assertEqual(domains[0]['availability_zone'], zone1) - self.assertRaises(exception.AdminRequired, - self.network.delete_dns_domain, self.context, - domain1) - self.network.delete_dns_domain(context_admin, domain1) + self.network.delete_dns_domain(self.context, domain1) domain1 = "example.org"