[Kimchi-devel] [PATCHv5 1/2] Fix: Avoid passing unexpected config params to repository manager
Royce Lv
lvroyce at linux.vnet.ibm.com
Fri Sep 26 06:57:01 UTC 2014
On 2014?09?25? 21:25, Aline Manera wrote:
>
> On 09/25/2014 08:06 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.
>>
>> Signed-off-by: Royce Lv<lvroyce at 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.
>
>> "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
ACK
>
>> 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':
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20140926/ca145a5b/attachment.html>
More information about the Kimchi-devel
mailing list