Re: [Kimchi-devel] [PATCHv2 1/3] Fix: Avoid passing unexpected config params to repository manager

From: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; 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. Also fix gpgkey handling for yum. Signed-off-by: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; --- docs/API.md | 2 ++ src/kimchi/API.json | 12 +++++++++++- src/kimchi/i18n.py | 1 + src/kimchi/repositories.py | 20 +++++++++++++++----- tests/test_model.py | 22 ++++++++++++++++++---- 5 files changed, 47 insertions(+), 10 deletions(-) diff --git a/docs/API.md b/docs/API.md index b679ce7..a6a2bcc 100644 --- a/docs/API.md +++ b/docs/API.md @@ -963,6 +963,8 @@ http://, ftp:// or file:// URL. list of baseurls for YUM repository * dist: Distribution to DEB repository * comps *(optional)*: List of components to DEB repository + * gpgcheck *(optional)*: Indicates if a GPG signature check on the packages got from repository should be performed. + * gpgkey *(optional)*: URL pointing to the ASCII-armored GPG key file for the repository. ### Resource: Repository diff --git a/src/kimchi/API.json b/src/kimchi/API.json index b8604d2..43d731e 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -666,6 +666,16 @@ "description": "URL to a file containing a list of baseurls", "type": "string", "error": "KCHREPOS0007E" + }, + "gpgcheck": { + "description": "Indicates if a GPG signature check on the packages got from repository should be performed.", + "type": "boolean", + "error": "KCHREPOS0009E" + }, + "gpgkey": { + "description": "URL pointing to the ASCII-armored GPG key file for the repository.", + "type": "string", + "error": "KCHREPOS0010E" } } } @@ -713,7 +723,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.", "type": "boolean", "error": "KCHREPOS0009E" }, diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 9e66c68..e83843c 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 676b7c2..3ac0e9c 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', 'gpgcheck', 'gpgkey') def __init__(self): self._yb = getattr(__import__('yum'), 'YumBase') @@ -172,8 +173,11 @@ class YumRepo(object): """ # At least one base url, or one mirror, must be given. 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)}) mirrorlist = config.get('mirrorlist', '') if not baseurl and not mirrorlist: raise MissingParameter("KCHREPOS0013E") @@ -195,9 +199,11 @@ class YumRepo(object): raise InvalidOperation("KCHREPOS0022E", {'repo_id': repo_id}) repo_name = config.get('repo_name', repo_id)

+ gpgcheck = config.get('gpgcheck', 1)

+ gpgkey = config.get('gpgkey', list())

repo = {'baseurl': baseurl, 'mirrorlist': mirrorlist, - 'name': repo_name, 'gpgcheck': 1, - 'gpgkey': [], 'enabled': 1} + 'name': repo_name, 'gpgcheck': gpgcheck, + 'gpgkey': gpgkey, 'enabled': 1} # write a repo file in the system with repo{} information. parser = ConfigParser() @@ -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,12 +420,15 @@ 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") if 'dist' not in config.keys(): raise MissingParameter("KCHREPOS0019E") - uri = params['baseurl'] dist = config['dist'] comps = config.get('comps', []) diff --git a/tests/test_model.py b/tests/test_model.py index ceedc6f..56a8e12 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -1295,11 +1295,21 @@ class ModelTests(unittest.TestCase): {'baseurl': 'http://br.archive.kimchi.com/ubuntu/fake', 'config': {'dist': 'quantal', 'comps': ['main']}}] + invalid_deb_repo = {'baseurl': 'http://br.archive.ubuntu.com/ubuntu/', + 'config': {'gpgkey': 'file:///fake-key-file'}} + + invalid_yum_repo = { + 'repo_id': 'fedora-fake2', + 'baseurl': 'http://www.fedora.org', + 'config': {'dist': 'quantal', 'comps': ['main']}} + repo_type = inst.capabilities_lookup()['repo_mngt_tool'] if repo_type == 'yum': test_repos = yum_repos + invalid_repo = invalid_deb_repo elif repo_type == 'deb': test_repos = deb_repos + invalid_repo = invalid_yum_repo else: # repository management tool was not recognized by Kimchi # skip test case @@ -1317,11 +1327,15 @@ class ModelTests(unittest.TestCase): 'config': {'dist': 'quantal'}} self.assertRaises(InvalidParameter, inst.repositories_create, repo) + # Create repo with invalid config + self.assertRaises(InvalidParameter, inst.repositories_create, repo) + # create repositories with invalid mirrorlist - for url in invalid_urls: - repo = {'repo_id': 'repo_fake', - 'config': {'mirrorlist': url, 'dist': 'quantal'}} - self.assertRaises(InvalidParameter, inst.repositories_create, repo)

+ if repo_type == 'yum': + for url in invalid_urls: + repo = {'repo_id': 'repo_fake', + 'config': {'mirrorlist': url, 'dist': 'quantal'}} + self.assertRaises(InvalidParameter, inst.repositories_create, repo)

for repo in test_repos: system_host_repos = len(inst.repositories_get_list())