From b2ac5b6702220f46fd92a51e1c8031bde577e11f Mon Sep 17 00:00:00 2001 From: nitesh vanarase Date: Tue, 23 Jan 2018 17:32:26 +0530 Subject: [PATCH] VIM should accept valid boolean values for is_default In case of create VIM API, is_default_parameter accepts any string values like "abc" or "1234" and set the value as True. It should strictly check for valid boolean values. Fixed this issue by using convert_to_boolean validator. so now, if you pass any non-boolean value, it will return 400 error to the user. NOTE: The below test case fails if we inherit the test class '..tests.unit.base.TestCase' from '..tests.base.BaseTestCase' with KeyError: tacker.tests.unit.nfvo.test_nfvo_plugin.TestNfvoPlugin. test_create_ns_workflow_no_task_exception The reason behind the failure is, the raised exception '..extensions.nfvo.NoTasksException' expects that the 'action' and 'resource' should be passed as arguments to the exception while raising it to form the exception message. If you don't pass these two arguments then it fails to form the exception message and raises KeyError. This happens because the '..tests.base.BaseTestCase' test class uses exception fixture and sets 'use_fatal_exceptions' to True always which results into not raising the raised exception again and it tries to form the exception message and pass it to the base class __init__ for further processing. It fails and raises KeyError while forming that exception message because in 'test_create_ns_workflow_no_task_exception' test case these arguments are not passed to the exception while raising. To fix this test case, passed the required 'action' and 'resource' arguments to the exception while raising it. DocImpact Closes-Bug: #1746538 Change-Id: Ib533b38a48d31cb26eecdc546b20b0f99beeaa34 --- lower-constraints.txt | 1 + tacker/extensions/nfvo.py | 1 + tacker/tests/base.py | 4 + tacker/tests/unit/api/__init__.py | 0 tacker/tests/unit/api/v1/__init__.py | 0 tacker/tests/unit/api/v1/test_vim.py | 128 +++++++++++++++++++++ tacker/tests/unit/base.py | 5 +- tacker/tests/unit/nfvo/test_nfvo_plugin.py | 8 +- tacker/tests/unit/test_attributes.py | 9 ++ test-requirements.txt | 1 + 10 files changed, 153 insertions(+), 4 deletions(-) create mode 100644 tacker/tests/unit/api/__init__.py create mode 100644 tacker/tests/unit/api/v1/__init__.py create mode 100644 tacker/tests/unit/api/v1/test_vim.py diff --git a/lower-constraints.txt b/lower-constraints.txt index 0bccdf4fd..a276bff37 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -16,6 +16,7 @@ cmd2==0.8.2 contextlib2==0.5.5 coverage==4.0 cryptography==2.1 +ddt===1.0.1 debtcollector==1.19.0 decorator==4.2.1 deprecation==2.0 diff --git a/tacker/extensions/nfvo.py b/tacker/extensions/nfvo.py index 2d5689fec..ca2b247c4 100644 --- a/tacker/extensions/nfvo.py +++ b/tacker/extensions/nfvo.py @@ -351,6 +351,7 @@ RESOURCE_ATTRIBUTE_MAP = { 'allow_post': True, 'allow_put': True, 'is_visible': True, + 'validate': {'type:boolean': None}, 'default': False }, 'created_at': { diff --git a/tacker/tests/base.py b/tacker/tests/base.py index ddfc2738a..f64108336 100644 --- a/tacker/tests/base.py +++ b/tacker/tests/base.py @@ -32,6 +32,7 @@ import testtools from tacker.common import config from tacker.common import rpc as n_rpc +from tacker import context from tacker import manager from tacker.tests import fake_notifier from tacker.tests import post_mortem_debug @@ -181,6 +182,9 @@ class BaseTestCase(testtools.TestCase): if sys.version_info < (2, 7) and getattr(self, 'fmt', '') == 'xml': raise self.skipException('XML Testing Skipped in Py26') + def fake_admin_context(self): + return context.get_admin_context() + def config(self, **kw): """Override some configuration values. diff --git a/tacker/tests/unit/api/__init__.py b/tacker/tests/unit/api/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tacker/tests/unit/api/v1/__init__.py b/tacker/tests/unit/api/v1/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tacker/tests/unit/api/v1/test_vim.py b/tacker/tests/unit/api/v1/test_vim.py new file mode 100644 index 000000000..4bf64ed0a --- /dev/null +++ b/tacker/tests/unit/api/v1/test_vim.py @@ -0,0 +1,128 @@ +# Copyright (C) 2018 NTT DATA +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import ddt +import mock +import six +from webob import exc + +from tacker.api.v1 import base as v1_base +from tacker.extensions import nfvo +from tacker.nfvo import nfvo_plugin +from tacker.tests.unit import base +from tacker import wsgi + + +def get_vim_config(): + return { + "vim": { + "tenant_id": 'test-project', + "type": "openstack", + "auth_url": 'http://localhost:5000', + "auth_cred": { + "username": "test_user", + "user_domain_name": "Default", + "password": "password" + }, + "vim_project": { + "name": "test_project", + "project_domain_name": "Default" + }, + "name": "VIM1", + "description": "Additional site", + "is_default": False + } + } + + +@ddt.ddt +class VIMCreateTestCase(base.TestCase): + + def setUp(self): + super(VIMCreateTestCase, self).setUp() + plugin = nfvo_plugin.NfvoPlugin() + resource_name = 'vim' + collection_name = resource_name + "s" + attribute_info = nfvo.RESOURCE_ATTRIBUTE_MAP[collection_name] + self.controller = v1_base.Controller(plugin, collection_name, + resource_name, attribute_info) + + def _vim_create_response(self): + return { + 'auth_cred': { + 'auth_url': 'http://localhost:5000', + 'cert_verify': 'False', + 'key_type': 'barbican_key', + 'password': '***', + 'project_domain_name': 'Default', + 'project_id': None, + 'project_name': 'nfv', + 'secret_uuid': '***', + 'user_domain_name': 'Default', + 'username': 'test_user' + }, + 'auth_url': 'http://localhost:5000', + 'created_at': None, + 'description': 'Additional site', + 'id': '73493efe-3616-414c-bf87-bf450d0b3650', + 'is_default': False, + 'name': 'VIM1', + 'placement_attr': { + 'regions': [ + 'RegionOne' + ] + }, + 'shared': False, + 'status': 'PENDING', + 'tenant_id': 'test-project', + 'type': 'openstack', + 'updated_at': None, + 'vim_project': { + 'name': 'test_project' + } + } + + @mock.patch.object(nfvo_plugin.NfvoPlugin, 'create_vim') + def test_create_vim(self, mock_create_vim): + vim_dict = get_vim_config() + request = wsgi.Request.blank( + "/vims.json", method='POST', + headers={'Content-Type': "application/json"}) + request.environ['tacker.context'] = self.fake_admin_context() + mock_create_vim.return_value = self._vim_create_response() + + result = self.controller.create(request, vim_dict) + # End API response doesn't contain the 'shared' attribute so pop it + # from dict + resp_dict = self._vim_create_response() + resp_dict.pop('shared') + # Check whether VIM is created with the provided vim_details + self.assertEqual(resp_dict, result['vim']) + + @ddt.data({'is_default': 'ABC'}, + {'is_default': 123}, + {'is_default': ''}) + def test_create_vim_with_invalid_is_default(self, value): + vim_dict = get_vim_config() + vim_dict['vim']['is_default'] = value + request = wsgi.Request.blank("/vims.json", method='POST', + headers={'Content-Type': "application/json"}) + request.environ['tacker.context'] = self.fake_admin_context() + msg = ("Invalid input for is_default. Reason: '%s' is not a " + "valid boolean value." % vim_dict['vim']['is_default']) + exp = self.assertRaises(exc.HTTPBadRequest, + self.controller.create, + request, vim_dict) + self.assertEqual(msg, six.text_type(exp)) diff --git a/tacker/tests/unit/base.py b/tacker/tests/unit/base.py index 44a618309..d4f7a18bf 100644 --- a/tacker/tests/unit/base.py +++ b/tacker/tests/unit/base.py @@ -13,11 +13,11 @@ # License for the specific language governing permissions and limitations # under the License. - import mock from oslo_config import cfg from oslo_config import fixture as config_fixture -from oslotest import base + +from tacker.tests import base CONF = cfg.CONF @@ -27,6 +27,7 @@ class TestCase(base.BaseTestCase): def setUp(self): super(TestCase, self).setUp() self.config_fixture = self.useFixture(config_fixture.Config(CONF)) + CONF([], default_config_files=[]) def _mock(self, target, new=mock.DEFAULT): patcher = mock.patch(target, new) diff --git a/tacker/tests/unit/nfvo/test_nfvo_plugin.py b/tacker/tests/unit/nfvo/test_nfvo_plugin.py index 50a80efaf..f8283671a 100644 --- a/tacker/tests/unit/nfvo/test_nfvo_plugin.py +++ b/tacker/tests/unit/nfvo/test_nfvo_plugin.py @@ -65,11 +65,14 @@ class FakeDriverManager(mock.Mock): elif ('prepare_and_create_workflow' in args and 'delete' == kwargs['action'] and DUMMY_NS_2 == kwargs['kwargs']['ns']['id']): - raise nfvo.NoTasksException() + raise nfvo.NoTasksException(action=kwargs['action'], + resource=kwargs['kwargs']['ns']['id']) elif ('prepare_and_create_workflow' in args and 'create' == kwargs['action'] and utils.DUMMY_NS_2_NAME == kwargs['kwargs']['ns']['ns']['name']): - raise nfvo.NoTasksException() + raise nfvo.NoTasksException( + action=kwargs['action'], + resource=kwargs['kwargs']['ns']['ns']['name']) def get_by_name(): @@ -298,6 +301,7 @@ class TestNfvoPlugin(db_base.SqlTestCase): self.assertIn('placement_attr', res) self.assertIn('created_at', res) self.assertIn('updated_at', res) + self.assertEqual(False, res['is_default']) def test_delete_vim(self): self._insert_dummy_vim() diff --git a/tacker/tests/unit/test_attributes.py b/tacker/tests/unit/test_attributes.py index a95273892..e233aa925 100644 --- a/tacker/tests/unit/test_attributes.py +++ b/tacker/tests/unit/test_attributes.py @@ -713,6 +713,15 @@ class TestConvertToBoolean(base.BaseTestCase): self.assertRaises(n_exc.InvalidInput, attributes.convert_to_boolean, '7') + self.assertRaises(n_exc.InvalidInput, + attributes.convert_to_boolean, + "") + self.assertRaises(n_exc.InvalidInput, + attributes.convert_to_boolean, + "abc") + self.assertRaises(n_exc.InvalidInput, + attributes.convert_to_boolean, + 123) class TestConvertToInt(base.BaseTestCase): diff --git a/test-requirements.txt b/test-requirements.txt index ddce0e14a..0c61823d1 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -5,6 +5,7 @@ # Despite above warning added by global sync process, please use # ascii betical order. coverage!=4.4,>=4.0 # Apache-2.0 +ddt>=1.0.1 # MIT doc8>=0.6.0 # Apache-2.0 fixtures>=3.0.0 # Apache-2.0/BSD hacking!=0.13.0,<0.14,>=0.12.0 # Apache-2.0