Blame SOURCES/subscription-manager-1.10.14-7-to-subscription-manager-1.10.14-8.patch

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