On 09/25/2014 08:06 AM, lvroyce@linux.vnet.ibm.com wrote:
From: Royce Lv <lvroyce@linux.vnet.ibm.com> We allow config params for both yum and apt in API.json, while we need to filter out parmas not proper for each kind. So add this logic to repository model. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/API.json | 2 +- src/kimchi/i18n.py | 1 + src/kimchi/repositories.py | 15 ++++++++++++++- tests/test_model.py | 13 +++++++++++-- 4 files changed, 27 insertions(+), 4 deletions(-) diff --git a/src/kimchi/API.json b/src/kimchi/API.json index 5b752dc..d115c3f 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -707,7 +707,7 @@ "error": "KCHREPOS0007E" }, "gpgcheck": { - "description": "Indicates if a GPG signature check on the packages gotten from repository should be performed.", + "description": "Indicates if a GPG signature check on the packages got from repository should be performed.",
I think the previous message is right.
ACK
"type": "boolean", "error": "KCHREPOS0009E" }, diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 4466d23..17d94f9 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -287,4 +287,5 @@ messages = { "KCHREPOS0025E": _("Unable to retrieve repository information. Details: '%(err)s'"), "KCHREPOS0026E": _("Unable to add repository. Details: '%(err)s'"), "KCHREPOS0027E": _("Unable to remove repository. Details: '%(err)s'"), + "KCHREPOS0028E": _("Configuration items: '%(items)s' are not supported by repository manager"), } diff --git a/src/kimchi/repositories.py b/src/kimchi/repositories.py index 1d19fb7..b5a4ee1 100644 --- a/src/kimchi/repositories.py +++ b/src/kimchi/repositories.py @@ -25,7 +25,7 @@ from ConfigParser import ConfigParser from kimchi.basemodel import Singleton from kimchi.config import kimchiLock -from kimchi.exception import InvalidOperation +from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import OperationFailed, NotFoundError, MissingParameter from kimchi.utils import validate_repo_url @@ -104,6 +104,7 @@ class YumRepo(object): """ TYPE = 'yum' DEFAULT_CONF_DIR = "/etc/yum.repos.d" + CONFIG_ENTRY = ('repo_name', 'mirrorlist') def __init__(self): self._yb = getattr(__import__('yum'), 'YumBase') @@ -174,6 +175,11 @@ class YumRepo(object): baseurl = params.get('baseurl', '')
config = params.get('config', {}) + extra_keys = list( + set(config.keys()).difference(set(self.CONFIG_ENTRY))) + if len(extra_keys) > 0: + raise InvalidParameter("KCHREPOS0028E", + {'items': ",".join(extra_keys)})
To avoid duplicating code on YumRepo and AptRepo I suggest moving this verification to Repositories().addRepository - there you already know what type of repo you are dealing with.
Something like:
config = params.get('config', {})
extra_keys = set(config.keys()).difference(set(self._pkg_mnger,CONFIG_ENTRY)))
if len(extra_keys) > 0:
raise
mirrorlist = config.get('mirrorlist', '') if not baseurl and not mirrorlist: raise MissingParameter("KCHREPOS0013E") @@ -318,6 +324,7 @@ class AptRepo(object): """ TYPE = 'deb' KIMCHI_LIST = "kimchi-source.list" + CONFIG_ENTRY = ('dist', 'comps') def __init__(self): getattr(__import__('apt_pkg'), 'init_config')() @@ -413,6 +420,12 @@ class AptRepo(object): # To create a APT repository the dist is a required parameter # (in addition to baseurl, verified on controller through API.json) config = params.get('config', None) + extra_keys = list( + set(config.keys()).difference(set(self.CONFIG_ENTRY))) + if len(extra_keys) > 0: + raise InvalidParameter("KCHREPOS0028E", + {'items': ",".join(extra_keys)}) + if config is None: raise MissingParameter("KCHREPOS0019E") diff --git a/tests/test_model.py b/tests/test_model.py index 9c70c5d..fa0491d 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -1310,8 +1310,7 @@ class ModelTests(unittest.TestCase): 'baseurl': 'http://www.fedora.org'}, {'repo_id': 'fedora-updates-fake', 'config': - {'mirrorlist': 'http://www.fedoraproject.org', - 'gpgkey': 'file:///tmp/KEY-fedora-updates-fake-19'}}] + {'mirrorlist': 'http://www.fedoraproject.org'}}] deb_repos = [{'baseurl': 'http://archive.ubuntu.com/ubuntu/', 'config': {'dist': 'quantal'}}, @@ -1326,12 +1325,22 @@ class ModelTests(unittest.TestCase): wrong_mirrorlist = {'repo_id': 'wrong-id', 'baseurl': 'www.example.com', 'config': {'mirrorlist': url}} + wrong_config_item = { + 'repo_id': 'wrong-id', + 'baseurl': 'www.example.com', + 'config': { + 'gpgkey': 'file:///tmp/KEY-fedora-updates-fake-19'}} yum_invalid_repos.append(wrong_baseurl) yum_invalid_repos.append(wrong_mirrorlist) + yum_invalid_repos.append(wrong_config_item) wrong_baseurl['config'] = {'dist': 'tasty'} + wrong_config = {'baseurl': deb_repos[0]['baseurl'], + 'config': { + 'unsupported_item': "a_unsupported_item"}} deb_invalid_repos.append(wrong_baseurl) + deb_invalid_repos.append(wrong_config) repo_type = inst.capabilities_lookup()['repo_mngt_tool'] if repo_type == 'yum':