[PATCHv4 0/2] Repository related fixes
From: Royce Lv <lvroyce@linux.vnet.ibm.com> Royce Lv (2): Fix: Avoid passing unexpected config params to repository manager Fix: Add rollback to update repository src/kimchi/API.json | 2 +- src/kimchi/i18n.py | 1 + src/kimchi/repositories.py | 40 +++++++++++++++++++++++++--------------- tests/test_model.py | 10 ++++++++-- 4 files changed, 35 insertions(+), 18 deletions(-) -- 1.8.3.2
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 | 13 ++++++++++++- tests/test_model.py | 11 +++++++++-- 4 files changed, 23 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.", "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..ca4e9d1 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,10 @@ 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)}) mirrorlist = config.get('mirrorlist', '') if not baseurl and not mirrorlist: raise MissingParameter("KCHREPOS0013E") @@ -318,6 +323,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 +419,11 @@ 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..8f70d95 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,20 @@ 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': -- 1.8.3.2
From: Royce Lv <lvroyce@linux.vnet.ibm.com> When update repository, we remove a old repository first then add a new one, if error occurs when adding new one, old one was removed which will cause problem. Fix by recovering removed old repository after failure of adding new one. Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/repositories.py | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/kimchi/repositories.py b/src/kimchi/repositories.py index ca4e9d1..23324c2 100644 --- a/src/kimchi/repositories.py +++ b/src/kimchi/repositories.py @@ -17,6 +17,7 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +import copy import os import time import urlparse @@ -447,7 +448,6 @@ class AptRepo(object): kimchiLock.release() raise OperationFailed("KCHREPOS0026E", {'err': e.message}) kimchiLock.release() - return self._get_repo_id(source_entry) def toggleRepo(self, repo_id, enable): @@ -491,24 +491,23 @@ class AptRepo(object): """ Update a given repository in repositories.Repositories() format """ - r = self._get_source_entry(repo_id) - if r is None: - raise NotFoundError("KCHREPOS0012E", {'repo_id': repo_id}) - - info = {'enabled': not r.disabled, - 'baseurl': params.get('baseurl', r.uri), - 'config': {'type': 'deb', 'dist': r.dist, - 'comps': r.comps}} - - validate_repo_url(info['baseurl']) + old_info = self.getRepo(repo_id) + updated_info = copy.deepcopy(old_info) + updated_info['baseurl'] = params.get('baseurl', updated_info['baseurl']) if 'config' in params.keys(): config = params['config'] - info['config']['dist'] = config.get('dist', r.dist) - info['config']['comps'] = config.get('comps', r.comps) + updated_info['config']['dist'] = config.get( + 'dist', old_info['config']['dist']) + updated_info['config']['comps'] = config.get( + 'comps', old_info['config']['comps']) self.removeRepo(repo_id) - return self.addRepo(info) + try: + return self.addRepo(updated_info) + except: + self.addRepo(old_info) + raise def removeRepo(self, repo_id): """ -- 1.8.3.2
participants (1)
-
lvroyce＠linux.vnet.ibm.com