[Kimchi-devel] [PATCH 1/2] Fix: Avoid passing unexpected config params to repository manager

[Aline Manera]

On 09/22/2014 04:41 AM, Royce Lv wrote:
> On 2014年09月22日 15:23, Royce Lv wrote:
>> On 2014年09月19日 11:00, Aline Manera wrote:
>>>
>>> On 09/17/2014 07:35 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/i18n.py | 1 +
>>>> src/kimchi/repositories.py | 14 +++++++++++---
>>>> 2 files changed, 12 insertions(+), 3 deletions(-)
>>>>
>>>> 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..e883f8c 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', 'gpgcheck', 'gpgkey', 'mirrorlist')
>>>
>>> Seems docs/API.md is missing gpgcheck and gpgkey on POST request.
>>> Could you also update it, please?
>> I took another look into repositories.py,
>> seems we ignore these two keys and just handled 'repo_name' and 
>> 'mirrorlist', so I'll fix my patch to ignore 'gpgcheck' and 'gpgkey' 
>> too:
>> repo_name = config.get('repo_name', repo_id)
>> repo = {'baseurl': baseurl, 'mirrorlist': mirrorlist,
>> 'name': repo_name, 'gpgcheck': 1,
>> 'gpgkey': [], 'enabled': 1}
> After investigation, hard code to ignore pgpkey is not the right way 
> to go, we still need to allow users to assign gpgkey so that 
> installation will not fail or throw error.
> So implementation needs to change.

Are we ignoring gpgcheck and gpgkey?
AFAIK, those parameters have default values while creating a new repo, 
but user can edit it to properly change them.

>>
>>>
>>>> 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")
>>>> @@ -318,6 +322,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 +418,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', [])
>>>
>>
>> _______________________________________________
>> Kimchi-devel mailing list
>> Kimchi-devel at ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>