diff --git a/rel-eng/packages/subscription-manager b/rel-eng/packages/subscription-manager index 1d1a8e1..f9393db 100644 --- a/rel-eng/packages/subscription-manager +++ b/rel-eng/packages/subscription-manager @@ -1 +1 @@ -1.10.14-7 ./ +1.10.14-8 ./ diff --git a/src/subscription_manager/cache.py b/src/subscription_manager/cache.py index d773741..298f64e 100644 --- a/src/subscription_manager/cache.py +++ b/src/subscription_manager/cache.py @@ -508,3 +508,30 @@ class PoolTypeCache(object): def clear(self): self.pooltype_map = {} + + +class WrittenOverrideCache(CacheManager): + ''' + Cache to keep track of the overrides used last time the a redhat.repo + was written. Doesn't track server status, we've got another cache for + that. + ''' + + CACHE_FILE = "/var/lib/rhsm/cache/written_overrides.json" + + def __init__(self, overrides=None): + self.overrides = overrides or {} + + def to_dict(self): + return self.overrides + + def _load_data(self, open_file): + try: + self.overrides = json.loads(open_file.read()) or {} + return self.overrides + except IOError: + log.error("Unable to read cache: %s" % self.CACHE_FILE) + except ValueError: + # ignore json file parse errors, we are going to generate + # a new as if it didn't exist + pass diff --git a/src/subscription_manager/repolib.py b/src/subscription_manager/repolib.py index ad0fc30..fa0913d 100644 --- a/src/subscription_manager/repolib.py +++ b/src/subscription_manager/repolib.py @@ -20,12 +20,11 @@ import logging import os import string import subscription_manager.injection as inj -from subscription_manager.cache import OverrideStatusCache +from subscription_manager.cache import OverrideStatusCache, WrittenOverrideCache from urllib import basejoin from rhsm.config import initConfig from rhsm.connection import RemoteServerException, RestlibException -from rhsm.utils import UnsupportedOperationException from certlib import ActionLock, DataLib from certdirectory import Path, ProductDirectory, EntitlementDirectory @@ -56,10 +55,7 @@ class RepoLib(DataLib): action = UpdateAction(self.uep, cache_only=self.cache_only, apply_overrides=apply_overrides, identity=self.identity) repos = action.get_unique_content() - if self.identity.is_valid() and action.override_supported: - return repos - # Otherwise we are in a disconnected case or dealing with an old server current = set() # Add the current repo data repo_file = RepoFile() @@ -83,6 +79,8 @@ class RepoLib(DataLib): repo_file = RepoFile() if os.path.exists(repo_file.path): os.unlink(repo_file.path) + # When the repo is removed, also remove the override tracker + WrittenOverrideCache.delete_cache() # WARNING: exact same name as another action in factlib and certlib. @@ -111,8 +109,9 @@ class UpdateAction: self.manage_repos = int(CFG.get('rhsm', 'manage_repos')) self.release = None - self.overrides = [] + self.overrides = {} self.override_supported = bool(self.uep and self.uep.supports_resource('content_overrides')) + self.written_overrides = WrittenOverrideCache() # If we are not registered, skip trying to refresh the # data from the server @@ -122,6 +121,7 @@ class UpdateAction: # Only attempt to update the overrides if they are supported # by the server. if self.override_supported: + self.written_overrides._read_cache() try: override_cache = inj.require(inj.OVERRIDE_STATUS_CACHE) except KeyError: @@ -131,8 +131,11 @@ class UpdateAction: else: status = override_cache.load_status(self.uep, self.identity.uuid) - if status is not None: - self.overrides = status + for item in status or []: + # Don't iterate through the list + if item['contentLabel'] not in self.overrides: + self.overrides[item['contentLabel']] = {} + self.overrides[item['contentLabel']][item['name']] = item['value'] message = "Release API is not supported by the server. Using default." try: @@ -174,14 +177,9 @@ class UpdateAction: repo_file.add(cont) updates += 1 else: - # In the non-disconnected case, destroy the old repo and replace it with - # what's in the entitlement cert plus any overrides. - if self.identity.is_valid() and self.override_supported: - repo_file.update(cont) - updates += 1 - else: - updates += self.update_repo(existing, cont) - repo_file.update(existing) + # Updates the existing repo with new content + updates += self.update_repo(existing, cont) + repo_file.update(existing) for section in repo_file.sections(): if section not in valid: @@ -190,6 +188,12 @@ class UpdateAction: # Write new RepoFile to disk: repo_file.write() + + if self.override_supported: + # Update with the values we just wrote + self.written_overrides.overrides = self.overrides + self.written_overrides.write_cache() + log.info("repos updated: %s" % updates) return updates @@ -295,9 +299,16 @@ class UpdateAction: def _set_override_info(self, repo): # In the disconnected case, self.overrides will be an empty list - for entry in self.overrides: - if entry['contentLabel'] == repo.id: - repo[entry['name']] = entry['value'] + for name, value in self.overrides.get(repo.id, {}).items(): + repo[name] = value + + def _is_overridden(self, repo, key): + return key in self.overrides.get(repo.id, {}) + + def _was_overridden(self, repo, key, value): + written_value = self.written_overrides.overrides.get(repo.id, {}).get(key) + # Compare values as strings to avoid casting problems from io + return written_value is not None and value is not None and str(written_value) == str(value) def _set_proxy_info(self, repo): proxy = "" @@ -328,29 +339,33 @@ class UpdateAction: url = url.lstrip('/') return basejoin(base, url) + def _build_props(self, old_repo, new_repo): + result = {} + all_keys = old_repo.keys() + new_repo.keys() + for key in all_keys: + result[key] = Repo.PROPERTIES.get(key, (1, None)) + return result + def update_repo(self, old_repo, new_repo): """ Checks an existing repo definition against a potentially updated version created from most recent entitlement certificates and configuration. Creates, updates, and removes properties as appropriate and returns the number of changes made. (if any) - - This method should only be used in disconnected cases! """ - if self.identity.is_valid() and self.override_supported: - log.error("Can not update repos when registered!") - raise UnsupportedOperationException() - changes_made = 0 - for key, mutable, default in Repo.PROPERTIES: + for key, (mutable, default) in self._build_props(old_repo, new_repo).items(): new_val = new_repo.get(key) # Mutable properties should be added if not currently defined, - # otherwise left alone. - if mutable: - if (new_val is not None) and (not old_repo[key]): - if old_repo[key] == new_val: + # otherwise left alone. However if we see that the property was overridden + # but that override has since been removed, we need to revert to the default + # value. + if mutable and not self._is_overridden(old_repo, key) \ + and not self._was_overridden(old_repo, key, old_repo.get(key)): + if (new_val is not None) and (not old_repo.get(key)): + if old_repo.get(key) == new_val: continue old_repo[key] = new_val changes_made += 1 @@ -358,7 +373,7 @@ class UpdateAction: # Immutable properties should be always be added/updated, # and removed if undefined in the new repo definition. else: - if new_val is None or (new_val.strip() == ""): + if new_val is None or (str(new_val).strip() == ""): # Immutable property should be removed: if key in old_repo.keys(): del old_repo[key] @@ -366,7 +381,7 @@ class UpdateAction: continue # Unchanged: - if old_repo[key] == new_val: + if old_repo.get(key) == new_val: continue old_repo[key] = new_val @@ -377,22 +392,21 @@ class UpdateAction: class Repo(dict): # (name, mutable, default) - The mutability information is only used in disconnected cases - PROPERTIES = ( - ('name', 0, None), - ('baseurl', 0, None), - ('enabled', 1, '1'), - ('gpgcheck', 1, '1'), - ('gpgkey', 0, None), - ('sslverify', 1, '1'), - ('sslcacert', 0, None), - ('sslclientkey', 0, None), - ('sslclientcert', 0, None), - ('metadata_expire', 1, None), - ('proxy', 0, None), - ('proxy_username', 0, None), - ('proxy_password', 0, None), - ('ui_repoid_vars', 0, None), - ) + PROPERTIES = { + 'name': (0, None), + 'baseurl': (0, None), + 'enabled': (1, '1'), + 'gpgcheck': (1, '1'), + 'gpgkey': (0, None), + 'sslverify': (1, '1'), + 'sslcacert': (0, None), + 'sslclientkey': (0, None), + 'sslclientcert': (0, None), + 'metadata_expire': (1, None), + 'proxy': (0, None), + 'proxy_username': (0, None), + 'proxy_password': (0, None), + 'ui_repoid_vars': (0, None)} def __init__(self, repo_id, existing_values=None): # existing_values is a list of 2-tuples @@ -412,7 +426,7 @@ class Repo(dict): # NOTE: This sets the above properties to the default values even if # they are not defined on disk. i.e. these properties will always # appear in this dict, but their values may be None. - for k, m, d in self.PROPERTIES: + for k, (m, d) in self.PROPERTIES.items(): if k not in self.keys(): self[k] = d diff --git a/subscription-manager.spec b/subscription-manager.spec index dea8402..04a660d 100644 --- a/subscription-manager.spec +++ b/subscription-manager.spec @@ -14,7 +14,7 @@ Name: subscription-manager Version: 1.10.14 -Release: 7%{?dist} +Release: 8%{?dist} Summary: Tools and libraries for subscription and repository management Group: System Environment/Base License: GPLv2 @@ -419,6 +419,10 @@ fi %endif %changelog +* Tue May 27 2014 ckozak 1.10.14-8 +- 1098891: Update repos, persisting local settings when possible + (ckozak@redhat.com) + * Tue Mar 25 2014 ckozak 1.10.14-7 - 1080531: Require newer python-rhsm to support branding (ckozak@redhat.com) diff --git a/test/test_repolib.py b/test/test_repolib.py index 57274e6..72505a3 100644 --- a/test/test_repolib.py +++ b/test/test_repolib.py @@ -18,8 +18,6 @@ import unittest from mock import Mock, patch from StringIO import StringIO -from rhsm.utils import UnsupportedOperationException - from fixture import SubManFixture from stubs import StubCertificateDirectory, StubProductCertificate, \ StubProduct, StubEntitlementCertificate, StubContent, \ @@ -111,24 +109,32 @@ class UpdateActionTests(SubManFixture): self.assertFalse(override_cache_mock.load_status.called) def test_overrides_trump_ent_cert(self): - self.update_action.overrides = [{ - 'contentLabel': 'x', - 'name': 'gpgcheck', - 'value': 'blah' - }] + self.update_action.overrides = {'x': {'gpgcheck': 'blah'}} r = Repo('x', [('gpgcheck', 'original'), ('gpgkey', 'some_key')]) self.assertEquals('original', r['gpgcheck']) self.update_action._set_override_info(r) self.assertEquals('blah', r['gpgcheck']) self.assertEquals('some_key', r['gpgkey']) + def test_overrides_trump_existing(self): + self.update_action.overrides = {'x': {'gpgcheck': 'blah'}} + values = [('gpgcheck', 'original'), ('gpgkey', 'some_key')] + old_repo = Repo('x', values) + new_repo = Repo(old_repo.id, values) + self.update_action._set_override_info(new_repo) + self.assertEquals('original', old_repo['gpgcheck']) + self.update_action.update_repo(old_repo, new_repo) + self.assertEquals('blah', old_repo['gpgcheck']) + self.assertEquals('some_key', old_repo['gpgkey']) + @patch("subscription_manager.repolib.RepoFile") def test_update_when_new_repo(self, mock_file): mock_file = mock_file.return_value mock_file.section.return_value = None def stub_content(): - return [Repo('x', [('gpgcheck', 'original'), ('gpgkey', 'some_key')])] + return [Repo('x', [('gpgcheck', 'original'), ('gpgkey', 'some_key'), ('name', 'some name')])] + self.update_action.get_unique_content = stub_content updates = self.update_action.perform() written_repo = mock_file.add.call_args[0][0] @@ -137,31 +143,13 @@ class UpdateActionTests(SubManFixture): self.assertEquals(1, updates) @patch("subscription_manager.repolib.RepoFile") - @patch("subscription_manager.repolib.ConsumerIdentity") - def test_update_when_registered_and_existing_repo(self, mock_ident, mock_file): - mock_ident.existsAndValid.return_value = True + def test_update_when_not_registered_and_existing_repo(self, mock_file): mock_file = mock_file.return_value mock_file.section.return_value = Repo('x', [('gpgcheck', 'original'), ('gpgkey', 'some_key')]) def stub_content(): - return [Repo('x', [('gpgcheck', 'new'), ('gpgkey', 'new_key')])] - self.update_action.get_unique_content = stub_content - self.update_action.override_supported = True - updates = self.update_action.perform() - written_repo = mock_file.update.call_args[0][0] - self.assertEquals('new', written_repo['gpgcheck']) - self.assertEquals('new_key', written_repo['gpgkey']) - self.assertEquals(1, updates) + return [Repo('x', [('gpgcheck', 'new'), ('gpgkey', 'new_key'), ('name', 'test')])] - @patch("subscription_manager.repolib.RepoFile") - @patch("subscription_manager.repolib.ConsumerIdentity") - def test_update_when_not_registered_and_existing_repo(self, mock_ident, mock_file): - mock_ident.existsAndValid.return_value = False - mock_file = mock_file.return_value - mock_file.section.return_value = Repo('x', [('gpgcheck', 'original'), ('gpgkey', 'some_key')]) - - def stub_content(): - return [Repo('x', [('gpgcheck', 'new'), ('gpgkey', 'new_key')])] self.update_action.get_unique_content = stub_content self.update_action.perform() written_repo = mock_file.update.call_args[0][0] @@ -369,14 +357,57 @@ class UpdateActionTests(SubManFixture): existing_repo['fake_prop'] = 'fake' self.assertTrue(('fake_prop', 'fake') in existing_repo.items()) - @patch("subscription_manager.repolib.ConsumerIdentity") - def test_repo_update_forbidden_when_registered(self, mock_ident): - mock_ident.existsAndValid.return_value = True - existing_repo = Repo('testrepo') - existing_repo['proxy_username'] = "blah" - incoming_repo = {'proxy_username': 'foo'} - self.update_action.override_supported = True - self.assertRaises(UnsupportedOperationException, self.update_action.update_repo, existing_repo, incoming_repo) + def test_overrides_removed_revert_to_default(self): + self.update_action.written_overrides.overrides = {'x': {'gpgcheck': 'blah'}} + self.update_action.overrides = {} + old_repo = Repo('x', [('gpgcheck', 'blah'), ('gpgkey', 'some_key')]) + new_repo = Repo(old_repo.id, [('gpgcheck', 'original'), ('gpgkey', 'some_key')]) + self.update_action._set_override_info(new_repo) + # The value from the current repo file (with the old override) should exist pre-update + self.assertEquals('blah', old_repo['gpgcheck']) + self.update_action.update_repo(old_repo, new_repo) + # Because the override has been removed, the value is reset to the default + self.assertEquals('original', old_repo['gpgcheck']) + self.assertEquals('some_key', old_repo['gpgkey']) + + def test_overrides_removed_and_edited(self): + self.update_action.written_overrides.overrides = {'x': {'gpgcheck': 'override_value'}} + self.update_action.overrides = {} + old_repo = Repo('x', [('gpgcheck', 'hand_edit'), ('gpgkey', 'some_key')]) + new_repo = Repo(old_repo.id, [('gpgcheck', 'original'), ('gpgkey', 'some_key')]) + self.update_action._set_override_info(new_repo) + # The value from the current repo file (with the old hand edit) should exist pre-update + self.assertEquals('hand_edit', old_repo['gpgcheck']) + self.update_action.update_repo(old_repo, new_repo) + # Because the current value doesn't match the override, we don't modify it + self.assertEquals('hand_edit', old_repo['gpgcheck']) + self.assertEquals('some_key', old_repo['gpgkey']) + + def test_non_default_overrides_added_to_existing(self): + ''' + Test that overrides for values that aren't found in Repo.PROPERTIES are written + to existing repos + ''' + self.update_action.written_overrides.overrides = {} + self.update_action.overrides = {'x': {'somekey': 'someval'}} + old_repo = Repo('x', []) + new_repo = Repo(old_repo.id, []) + self.update_action._set_override_info(new_repo) + self.update_action.update_repo(old_repo, new_repo) + self.assertEquals('someval', old_repo['somekey']) + + def test_non_default_override_removed_deleted(self): + ''' + Test that overrides for values that aren't found in Repo.PROPERTIES are + removed from redhat.repo once the override is removed + ''' + self.update_action.written_overrides.overrides = {'x': {'somekey': 'someval'}} + self.update_action.overrides = {} + old_repo = Repo('x', [('somekey', 'someval')]) + new_repo = Repo(old_repo.id, []) + self.update_action._set_override_info(new_repo) + self.update_action.update_repo(old_repo, new_repo) + self.assertFalse('somekey' in old_repo) class TidyWriterTests(unittest.TestCase):