Do not leak credentials on leader-set failure

This will also give us more insights into the leader-set failure
happening in the linked bug.

Also updated project files from latest release-tools templates.

Also blacklisted libjuju 2.8.3 which causes spurious
JujuAPIError's.

Change-Id: I51b890098df6d918c1d84adba272559ef45411bb
Partial-Bug: #1890256

hooks/keystone_utils.py

@@ -1462,7 +1462,7 @@ def set_admin_passwd(passwd, user=None):
     if user is None:
         user = config('admin-user')
 
-    leader_set({'{}_passwd'.format(user): passwd})
+    _leader_set_secret({'{}_passwd'.format(user): passwd})
 
 
 def get_api_version():
@@ -1620,7 +1620,7 @@ def _migrate_admin_password():
     if is_leader() and os.path.exists(STORED_PASSWD):
         log('Migrating on-disk stored passwords to leader storage')
         with open(STORED_PASSWD) as fd:
-            leader_set({"admin_passwd": fd.readline().strip('\n')})
+            _leader_set_secret({"admin_passwd": fd.readline().strip('\n')})
 
         os.unlink(STORED_PASSWD)
 
@@ -1630,8 +1630,8 @@ def _migrate_service_passwords():
     if is_leader() and os.path.exists(SERVICE_PASSWD_PATH):
         log('Migrating on-disk stored passwords to leader storage')
         creds = load_stored_passwords()
-        for k, v in creds.items():
-            leader_set({"{}_passwd".format(k): v})
+        _leader_set_secret({"{}_passwd".format(k): v
+                            for k, v in creds.items()})
         os.unlink(SERVICE_PASSWD_PATH)
 
 
@@ -1645,7 +1645,7 @@ def get_service_password(service_username):
 
 
 def set_service_password(passwd, user):
-    leader_set({"{}_passwd".format(user): passwd})
+    _leader_set_secret({"{}_passwd".format(user): passwd})
 
 
 def is_password_changed(username, passwd):
@@ -2535,7 +2535,7 @@ def key_leader_set():
     `CREDENTIAL_KEY_REPOSITORY` directories.  Note that this function will fail
     if it is called on the unit that is not the leader.
 
-    :raises: :class:`subprocess.CalledProcessError` if the leader_set fails.
+    :raises: :class:`RuntimeError` if the leader_set fails.
     """
     disk_keys = {}
     for key_repository in [FERNET_KEY_REPOSITORY, CREDENTIAL_KEY_REPOSITORY]:
@@ -2544,7 +2544,7 @@ def key_leader_set():
             with open(os.path.join(key_repository, key_number),
                       'r') as f:
                 disk_keys[key_repository][key_number] = f.read()
-    leader_set({'key_repository': json.dumps(disk_keys)})
+    _leader_set_secret({'key_repository': json.dumps(disk_keys)})
 
 
 def key_write():
@@ -2702,3 +2702,18 @@ def endpoints_dict(settings):
     }
 
     return endpoints
+
+
+def _leader_set_secret(secret_dict):
+    """Wrapper around leader_set doing its best to prevent leaking secrets."""
+    if not is_leader():
+        raise RuntimeError("This unit isn't the leader and therefore can't "
+                           "call leader_set() with the given secrets")
+    try:
+        leader_set(secret_dict)
+    except subprocess.CalledProcessError as e:
+        # Do not log 'e' or 'e.cmd' as this would leak secrets. Raise a
+        # different exception to make sure that no one above will leak the
+        # secrets by accident.
+        raise RuntimeError('leader-set failed with {}: {}'.format(e.returncode,
+                                                                  e.output))

requirements.txt

@@ -7,6 +7,7 @@
 #       requirements.  They are intertwined.  Also, Zaza itself should specify
 #       all of its own requirements and if it doesn't, fix it there.
 #
+setuptools<50.0.0  # https://github.com/pypa/setuptools/commit/04e3df22df840c6bb244e9b27bc56750c44b7c85
 pbr>=1.8.0,<1.9.0
 simplejson>=2.2.0
 netifaces>=0.10.4

test-requirements.txt

@@ -7,12 +7,14 @@
 #       requirements.  They are intertwined.  Also, Zaza itself should specify
 #       all of its own requirements and if it doesn't, fix it there.
 #
+setuptools<50.0.0  # https://github.com/pypa/setuptools/commit/04e3df22df840c6bb244e9b27bc56750c44b7c85
 charm-tools>=2.4.4
 requests>=2.18.4
 mock>=1.2
-flake8>=2.2.4,<=2.4.1
+flake8>=2.2.4
 stestr>=2.2.0
 coverage>=4.5.2
 pyudev              # for ceph-* charm unit tests (need to fix the ceph-* charm unit tests/mocking)
+juju!=2.8.3         # this version causes spurious JujuAPIError's
 git+https://github.com/openstack-charmers/zaza.git#egg=zaza;python_version>='3.0'
 git+https://github.com/openstack-charmers/zaza-openstack-tests.git#egg=zaza.openstack

tox.ini

@@ -116,5 +116,5 @@ commands =
     functest-run-suite --keep-model --bundle {posargs}
 
 [flake8]
-ignore = E402,E226
+ignore = E402,E226,W503,W504
 exclude = */charmhelpers

unit_tests/test_keystone_utils.py

@@ -367,6 +367,7 @@ class TestKeystoneUtils(CharmTestCase):
                                                    **relation_data)
 
     @patch.object(utils, 'get_real_role_names')
+    @patch.object(utils, 'is_leader')
     @patch.object(utils, 'leader_set')
     @patch.object(utils, 'leader_get')
     @patch.object(utils, 'get_api_version')
@@ -378,10 +379,11 @@ class TestKeystoneUtils(CharmTestCase):
     def test_add_service_to_keystone_no_clustered_no_https_complete_values(
             self, KeystoneManager, add_endpoint, ensure_valid_service,
             _resolve_address, create_user, get_api_version, leader_get,
-            leader_set, _get_real_role_names, test_api_version=2):
+            leader_set, is_leader, _get_real_role_names, test_api_version=2):
         _get_real_role_names.return_value = ['Member', 'SpecialRole']
         get_api_version.return_value = test_api_version
         leader_get.return_value = None
+        is_leader.return_value = True
         relation_id = 'identity-service:0'
         remote_unit = 'unit/0'
         self.get_service_password.return_value = 'password'
@@ -1099,19 +1101,25 @@ class TestKeystoneUtils(CharmTestCase):
         self.assertEqual(utils.get_admin_passwd(), 'admin')
         leader_get.assert_called_with('test_passwd')
 
+    @patch.object(utils, 'is_leader')
     @patch.object(utils, 'leader_set')
-    def test_set_admin_password(self, leader_set):
+    def test_set_admin_password(self, leader_set, is_leader):
+        is_leader.return_value = True
         utils.set_admin_passwd('secret')
         leader_set.assert_called_once_with({'admin_passwd': 'secret'})
 
+    @patch.object(utils, 'is_leader')
     @patch.object(utils, 'leader_set')
-    def test_set_admin_password_config_username(self, leader_set):
+    def test_set_admin_password_config_username(self, leader_set, is_leader):
+        is_leader.return_value = True
         self.test_config.set('admin-user', 'username')
         utils.set_admin_passwd('secret')
         leader_set.assert_called_once_with({'username_passwd': 'secret'})
 
+    @patch.object(utils, 'is_leader')
     @patch.object(utils, 'leader_set')
-    def test_set_admin_password_username(self, leader_set):
+    def test_set_admin_password_username(self, leader_set, is_leader):
+        is_leader.return_value = True
         utils.set_admin_passwd('secret', user='username')
         leader_set.assert_called_once_with({'username_passwd': 'secret'})
 
@@ -1688,10 +1696,12 @@ class TestKeystoneUtils(CharmTestCase):
         utils.fernet_rotate()
         self.subprocess.check_output.called_with(cmd)
 
+    @patch.object(utils, 'is_leader')
     @patch.object(utils, 'leader_set')
     @patch('os.listdir')
-    def test_key_leader_set(self, listdir, leader_set):
+    def test_key_leader_set(self, listdir, leader_set, is_leader):
         listdir.return_value = ['0', '1']
+        is_leader.return_value = True
         self.time.time.return_value = "the-time"
         with patch.object(builtins, 'open', mock_open(
                 read_data="some_data")):
@@ -1881,6 +1891,7 @@ class TestKeystoneUtils(CharmTestCase):
         mock_leader_get.assert_called_with('_charm-keystone-admin_passwd')
 
     @patch.object(utils, 'get_manager')
+    @patch.object(utils, 'is_leader')
     @patch.object(utils, 'leader_set')
     @patch.object(utils, 'resolve_address')
     @patch.object(utils, 'endpoint_url')
@@ -1895,6 +1906,7 @@ class TestKeystoneUtils(CharmTestCase):
             mock_endpoint_url,
             mock_resolve_address,
             mock_leader_set,
+            mock_is_leader,
             mock_get_manager):
         configs = MagicMock()
         mock_get_api_suffix.return_value = 'suffix'
@@ -1902,6 +1914,7 @@ class TestKeystoneUtils(CharmTestCase):
         mock_endpoint_url.side_effect = (
             lambda x, y, z: 'http://{}:{}/{}'.format(x, y, z))
         mock_leader_get.return_value = 'fakepassword'
+        mock_is_leader.return_value = True
         mock_get_manager().resolve_user_id.return_value = 'fakeid'
         self.os_release.return_value = 'queens'
         utils.bootstrap_keystone(configs=configs)