airship/armada
Code Issues Proposed changes

fix: Fix controller enforcing a nonexistent policy

Browse Source 
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] 9fad5cff0a/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
This commit is contained in:
Felipe Monteiro
2018-10-12 18:03:19 +01:00
parent 9fad5cff0a
commit aa07ae72d5
2 changed files with 7 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
armada/api/controller/test.py
View File

@@ -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.

12
armada/tests/unit/api/test_test_controller.py
View File

@@ -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)