[Kimchi-devel] [PATCHv2 1/3] Fix: Avoid passing unexpected config params to repository manager
Aline Manera
alinefm at linux.vnet.ibm.com
Mon Sep 22 15:12:51 UTC 2014
On 09/22/2014 06:32 AM, lvroyce at linux.vnet.ibm.com wrote:
> From: Royce Lv <lvroyce at 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.
> Also fix gpgkey handling for yum.
>
> Signed-off-by: Royce Lv <lvroyce at linux.vnet.ibm.com>
> ---
> 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)
From jsonschema, gpgcheck is boolean, ie, true or false, and the yum
repo config expects 1 or 0
> + gpgkey = config.get('gpgkey', list())
From jsonschema, gpgkey is a string and the yum repo config expects a list
I know I have commented that gpgcheck and gpgkey were missing on
docs/API.md but they are supposed to be
like that as during the repo creation we assume the defaut values and
they can be changed on PUT request
> 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)
You are restricting the test for yum repos.
You should create the repo params according to repo type to test deb and yum
for url in invalid_urls:
if repo_type == 'yum':
repo = {}
elif repo_type == 'deb':
repo = {}
self.assertRaises(...)
> for repo in test_repos:
> system_host_repos = len(inst.repositories_get_list())
More information about the Kimchi-devel
mailing list