From b331e1e71c0e3fbca5a1f0b3001009912fe19dca Mon Sep 17 00:00:00 2001 From: tengqm Date: Tue, 17 Nov 2015 19:41:03 -0500 Subject: [PATCH] Disallow profile update for spec Previously we support creating new profiles when doing profile-update, if a new 'spec' is provided. However, this looks like an abuse of the 'update' operation. It creates more confusion than convenience. This patch proposes removing 'spec' from the update interface. More patches will come to remove this completely. This is only about the API and the RPC client layer. Change-Id: Id8edf1d1bcb33f85dbdad614dd4db27285f17d0a --- senlin/api/openstack/v1/profiles.py | 6 ++-- senlin/rpc/client.py | 6 ++-- senlin/tests/unit/apiv1/test_profiles.py | 35 +----------------------- senlin/tests/unit/test_rpc_client.py | 1 - 4 files changed, 5 insertions(+), 43 deletions(-) diff --git a/senlin/api/openstack/v1/profiles.py b/senlin/api/openstack/v1/profiles.py index 9a4ad92ff..862fdc15a 100644 --- a/senlin/api/openstack/v1/profiles.py +++ b/senlin/api/openstack/v1/profiles.py @@ -132,15 +132,13 @@ class ProfileController(object): if profile_data is None: raise exc.HTTPBadRequest(_("Malformed request data, missing " "'profile' key in request body.")) - # spec can be empty in an update request + # We ignore the 'spec' property even if it is specified. name = profile_data.get(consts.PROFILE_NAME, None) - spec = profile_data.get(consts.PROFILE_SPEC, None) permission = profile_data.get(consts.PROFILE_PERMISSION, None) metadata = profile_data.get(consts.PROFILE_METADATA, None) # We don't check if type is specified or not profile = self.rpc_client.profile_update(req.context, profile_id, - name, spec, permission, - metadata) + name, permission, metadata) return {'profile': profile} diff --git a/senlin/rpc/client.py b/senlin/rpc/client.py index 0e4ed3faf..f0eb6e01f 100644 --- a/senlin/rpc/client.py +++ b/senlin/rpc/client.py @@ -97,13 +97,11 @@ class EngineClient(object): return self.call(ctxt, self.make_msg('profile_get', identity=identity)) - def profile_update(self, ctxt, profile_id, name, spec, permission, - metadata): + def profile_update(self, ctxt, profile_id, name, permission, metadata): return self.call(ctxt, self.make_msg('profile_update', profile_id=profile_id, - name=name, spec=spec, - permission=permission, + name=name, permission=permission, metadata=metadata)) def profile_delete(self, ctxt, identity, cast=True): diff --git a/senlin/tests/unit/apiv1/test_profiles.py b/senlin/tests/unit/apiv1/test_profiles.py index 0bb830345..0c150d309 100644 --- a/senlin/tests/unit/apiv1/test_profiles.py +++ b/senlin/tests/unit/apiv1/test_profiles.py @@ -488,6 +488,7 @@ class ProfileControllerTest(shared.ControllerTest, base.SenlinTestCase): args = copy.deepcopy(body['profile']) args['profile_id'] = pid args['permission'] = None + del args['spec'] mock_call.assert_called_with(req.context, ('profile_update', args)) expected = {'profile': engine_resp} @@ -558,7 +559,6 @@ class ProfileControllerTest(shared.ControllerTest, base.SenlinTestCase): args = copy.deepcopy(body['profile']) args['profile_id'] = pid - args['spec'] = None mock_call.assert_called_with(req.context, ('profile_update', args)) expected = {'profile': engine_response} @@ -590,39 +590,6 @@ class ProfileControllerTest(shared.ControllerTest, base.SenlinTestCase): self.assertEqual(404, resp.json['code']) self.assertEqual('ProfileNotFound', resp.json['error']['type']) - def test_profile_update_with_spec_validation_failed(self, mock_enforce): - self._mock_enforce_setup(mock_enforce, 'update', True) - pid = 'aaaa-bbbb-cccc' - body = { - 'profile': { - 'name': 'test_profile', - 'spec': {'param4': 'value4'}, - } - } - req = self._put('/profiles/%(profile_id)s' % {'profile_id': pid}, - json.dumps(body)) - - msg = 'Spec validation error (param): value' - error = senlin_exc.SpecValidationFailed(message=msg) - mock_call = self.patchobject(rpc_client.EngineClient, 'call', - side_effect=error) - - resp = shared.request_with_middleware(fault.FaultWrapper, - self.controller.update, - req, tenant_id=self.project, - profile_id=pid, - body=body) - - expected_args = body['profile'] - expected_args['permission'] = None - expected_args['metadata'] = None - expected_args['profile_id'] = pid - mock_call.assert_called_once_with(req.context, - ('profile_update', expected_args)) - self.assertEqual(400, resp.json['code']) - self.assertEqual('SpecValidationFailed', resp.json['error']['type']) - self.assertIsNone(resp.json['error']['traceback']) - def test_profile_update_denied_policy(self, mock_enforce): self._mock_enforce_setup(mock_enforce, 'update', False) pid = 'aaaa-bbbb-cccc' diff --git a/senlin/tests/unit/test_rpc_client.py b/senlin/tests/unit/test_rpc_client.py index 2086fafe8..44ed54ecd 100644 --- a/senlin/tests/unit/test_rpc_client.py +++ b/senlin/tests/unit/test_rpc_client.py @@ -185,7 +185,6 @@ class EngineRpcAPITestCase(base.SenlinTestCase): default_args = { 'profile_id': 'aaaa-bbbb-cccc', 'name': mock.ANY, - 'spec': mock.ANY, 'permission': mock.ANY, 'metadata': mock.ANY, }