From aa07ae72d50e7ed755ea25fbd3de3ae795eb8a2c Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Fri, 12 Oct 2018 18:03:19 +0100 Subject: [PATCH] fix: Fix controller enforcing a nonexistent policy This patch set corrects in an issue in Armada's "TestReleasesManifestController" in which a nonexistent policy "armada:tests_manifest" is getting enforced; note that it is nowhere to be found among Armada's actual policies: [0] http://codesearch.openstack.org/?q=tests_manifest&i=nope&files=&repos=airship-armada [1] https://github.com/openstack/airship-armada/blob/master/armada/common/policies/service.py [2] https://github.com/openstack/airship-armada/blob/9fad5cff0a840a18beab666d1a9787cadd366fa4/etc/armada/policy.yaml#L28 As [2] demonstrates, the policy is actually called "armada:test_manifest" NOT "armada:tests_manifest" (with an "s"). The root cause is related to how Armada is calling oslo.policy; this will be fixed in a follow up (by root cause, the fact that this issue is allowed to exist in the first place without an error getting thrown somehwere). Change-Id: I3e424e657e7d11e7968359d7a9ed13fa5d3e0896 --- armada/api/controller/test.py | 2 +- armada/tests/unit/api/test_test_controller.py | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/armada/api/controller/test.py b/armada/api/controller/test.py index 24436f75..0a0c63bc 100644 --- a/armada/api/controller/test.py +++ b/armada/api/controller/test.py @@ -116,7 +116,7 @@ class TestReleasesManifestController(api.BaseResource): result, details = validate.validate_armada_documents(documents) return self._format_validation_response(req, resp, result, details) - @policy.enforce('armada:tests_manifest') + @policy.enforce('armada:test_manifest') def on_post(self, req, resp): # TODO(fmontei): Validation Content-Type is application/x-yaml. diff --git a/armada/tests/unit/api/test_test_controller.py b/armada/tests/unit/api/test_test_controller.py index 739e85d8..8cc8d785 100644 --- a/armada/tests/unit/api/test_test_controller.py +++ b/armada/tests/unit/api/test_test_controller.py @@ -30,7 +30,7 @@ class TestReleasesManifestControllerTest(base.BaseControllerTest): @mock.patch.object(test, 'Manifest') @mock.patch.object(test, 'Tiller') def test_test_controller_with_manifest(self, mock_tiller, mock_manifest): - rules = {'armada:tests_manifest': '@'} + rules = {'armada:test_manifest': '@'} self.policy.set_rules(rules) # TODO: Don't use example charts in tests. @@ -111,7 +111,7 @@ class TestReleasesManifestControllerNegativeTest(base.BaseControllerTest): @mock.patch.object(test, 'test_release_for_success') def test_test_controller_tiller_exc_returns_500( self, mock_test_release_for_success, mock_tiller, _): - rules = {'armada:tests_manifest': '@'} + rules = {'armada:test_manifest': '@'} self.policy.set_rules(rules) mock_tiller.side_effect = Exception @@ -123,7 +123,7 @@ class TestReleasesManifestControllerNegativeTest(base.BaseControllerTest): @mock.patch.object(test, 'Manifest') @mock.patch.object(test, 'Tiller') def test_test_controller_validation_failure_returns_400(self, *_): - rules = {'armada:tests_manifest': '@'} + rules = {'armada:test_manifest': '@'} self.policy.set_rules(rules) manifest_path = os.path.join(os.getcwd(), 'examples', @@ -163,7 +163,7 @@ class TestReleasesManifestControllerNegativeTest(base.BaseControllerTest): @mock.patch.object(test, 'Tiller') def test_test_controller_manifest_failure_returns_400( self, _, mock_manifest): - rules = {'armada:tests_manifest': '@'} + rules = {'armada:test_manifest': '@'} self.policy.set_rules(rules) mock_manifest.return_value.get_manifest.side_effect = ( @@ -231,11 +231,11 @@ class TestReleasesReleaseNameControllerNegativeRbacTest( class TestReleasesManifestControllerNegativeRbacTest(base.BaseControllerTest): @test_utils.attr(type=['negative']) - def test_tests_manifest_insufficient_permissions(self): + def test_test_manifest_insufficient_permissions(self): """Tests the POST /api/v1.0/tests endpoint returns 403 following failed authorization. """ - rules = {'armada:tests_manifest': policy_base.RULE_ADMIN_REQUIRED} + rules = {'armada:test_manifest': policy_base.RULE_ADMIN_REQUIRED} self.policy.set_rules(rules) resp = self.app.simulate_post('/api/v1.0/tests') self.assertEqual(403, resp.status_code)