Fix restore encrypted backup

For the backups created prior to Victoria which may be encrypted, the restore
function in Victoria or later release should correctly decrypt the backup data.

Backup encryption from Victoria is disabled.

Bump the backup docker image tag to 1.1.0.

Change-Id: I7abf5563b22ab1914fa355c089a3793da07f2215

8 files changed, 76 insertions, 38 deletions

diff --git a/backup/drivers/base.py b/backup/drivers/base.py
index 20ed75cf..4367f88e 100644
--- a/backup/drivers/base.py
+++ b/backup/drivers/base.py
@@ -47,7 +47,16 @@ class BaseRunner(object):
         self.restore_content_length = 0
 
         self.command = self.cmd % kwargs
-        self.restore_command = (self.decrypt_cmd +
+
+        if self.location.endswith('.enc') and not self.encrypt_key:
+            raise Exception("Encryption key not provided with an encrypted "
+                            "backup.")
+
+        self.restore_command = ''
+        # Only decrypt if the object name ends with .enc
+        if self.location.endswith('.enc'):
+            self.restore_command = self.decrypt_cmd
+        self.restore_command = (self.restore_command +
                                 self.unzip_cmd +
                                 (self.restore_cmd % kwargs))
         self.prepare_command = self.prepare_cmd % kwargs
@@ -78,12 +87,21 @@ class BaseRunner(object):
 
     @property
     def encrypt_cmd(self):
-        return (' | openssl enc -aes-256-cbc -md sha512 -pbkdf2 -iter 10000 '
-                '-salt -pass pass:%s' %
-                self.encrypt_key) if self.encrypt_key else ''
+        """Encryption command.
+
+        Since Victoria, trove no longer encrypts the backup data for the end
+        user. This could be improved by giving users the capability to specify
+        password when creating the backups.
+        """
+        return ""
 
     @property
     def decrypt_cmd(self):
+        """Decryption command.
+
+        Since Victoria, trove no longer encrypts the backup data for the end
+        user. This command is only for backward compatibility.
+        """
         if self.encrypt_key:
             return ('openssl enc -d -aes-256-cbc -md sha512 -pbkdf2 -iter '
                     '10000 -salt -pass pass:%s | '
diff --git a/backup/main.py b/backup/main.py
index a42dc4ba..b777ce31 100644
--- a/backup/main.py
+++ b/backup/main.py
@@ -39,7 +39,12 @@ cli_opts = [
         choices=['innobackupex', 'mariabackup', 'pg_basebackup']
     ),
     cfg.BoolOpt('backup'),
-    cfg.StrOpt('backup-encryption-key'),
+    cfg.StrOpt(
+        'backup-encryption-key',
+        help='This is only for backward compatibility. The backups '
+             'created prior to Victoria may be encrypted. Trove guest '
+             'agent is responsible for passing the key.'
+    ),
     cfg.StrOpt('db-user'),
     cfg.StrOpt('db-password'),
     cfg.StrOpt('db-host'),
diff --git a/trove/common/cfg.py b/trove/common/cfg.py
index 4d8d6078..8558893d 100644
--- a/trove/common/cfg.py
+++ b/trove/common/cfg.py
@@ -25,6 +25,7 @@ from oslo_config.cfg import NoSuchOptError
 from oslo_log import log as logging
 from oslo_middleware import cors
 from osprofiler import opts as profiler
+from oslo_log import versionutils
 
 from trove.common.i18n import _
 from trove.version import version_info as version
@@ -325,13 +326,36 @@ common_opts = [
     cfg.StrOpt('backup_swift_container', default='database_backups',
                help='Swift container to put backups in.'),
     cfg.BoolOpt('backup_use_gzip_compression', default=True,
-                help='Compress backups using gzip.'),
-    cfg.BoolOpt('backup_use_openssl_encryption', default=True,
-                help='Encrypt backups using OpenSSL.'),
-    cfg.StrOpt('backup_aes_cbc_key', default='default_aes_cbc_key',
-               help='Default OpenSSL aes_cbc key.'),
-    cfg.BoolOpt('backup_use_snet', default=False,
-                help='Send backup files over snet.'),
+                help='Compress backups using gzip.',
+                deprecated_for_removal=True,
+                deprecated_since=versionutils.deprecated.VICTORIA,
+                deprecated_reason='Backup data compression is enabled by '
+                                  'default. This option is ignored.'),
+    cfg.BoolOpt(
+        'backup_use_openssl_encryption', default=True,
+        help='Encrypt backups using OpenSSL.',
+        deprecated_for_removal=True,
+        deprecated_since=versionutils.deprecated.VICTORIA,
+        deprecated_reason='Trove should not encrypt backup data on '
+                          'behalf of the user. This option is ignored.'
+    ),
+    cfg.StrOpt(
+        'backup_aes_cbc_key', default='',
+        help='Default OpenSSL aes_cbc key for decrypting backup data created '
+             'prior to Victoria.',
+        deprecated_for_removal=True,
+        deprecated_since=versionutils.deprecated.VICTORIA,
+        deprecated_reason='This option is only for backward compatibility. '
+                          'Backups created after Victoria are not encrypted '
+                          'any more.'
+    ),
+    cfg.BoolOpt(
+        'backup_use_snet', default=False,
+        help='Send backup files over snet.',
+        deprecated_for_removal=True,
+        deprecated_since=versionutils.deprecated.VICTORIA,
+        deprecated_reason='This option is not supported any more.'
+    ),
     cfg.IntOpt('backup_chunk_size', default=2 ** 16,
                help='Chunk size (in bytes) to stream to the Swift container. '
                'This should be in multiples of 128 bytes, since this is the '
@@ -616,7 +640,7 @@ mysql_opts = [
         help='Database docker image.'
     ),
     cfg.StrOpt(
-        'backup_docker_image', default='openstacktrove/db-backup-mysql:1.0.0',
+        'backup_docker_image', default='openstacktrove/db-backup-mysql:1.1.0',
         help='The docker image used for backup and restore.'
     ),
 ]
@@ -1061,7 +1085,7 @@ postgresql_opts = [
     ),
     cfg.StrOpt(
         'backup_docker_image',
-        default='openstacktrove/db-backup-postgresql:1.0.0',
+        default='openstacktrove/db-backup-postgresql:1.1.0',
         help='The docker image used for backup and restore.'
     ),
     cfg.BoolOpt('icmp', default=False,
@@ -1382,7 +1406,7 @@ mariadb_opts = [
     ),
     cfg.StrOpt(
         'backup_docker_image',
-        default='openstacktrove/db-backup-mariadb:1.0.0',
+        default='openstacktrove/db-backup-mariadb:1.1.0',
         help='The docker image used for backup and restore.'
     ),
 ]
diff --git a/trove/guestagent/datastore/manager.py b/trove/guestagent/datastore/manager.py
index 9d9604dd..65281510 100644
--- a/trove/guestagent/datastore/manager.py
+++ b/trove/guestagent/datastore/manager.py
@@ -760,6 +760,12 @@ class Manager(periodic_task.PeriodicTasks):
         LOG.info("Starting to restore database from backup %s, "
                  "backup_info: %s", backup_info['id'], backup_info)
 
+        if (backup_info["location"].endswith('.enc') and
+                not CONF.backup_aes_cbc_key):
+            self.status.set_status(service_status.ServiceStatuses.FAILED)
+            raise exception.TroveError('Decryption key not configured for '
+                                       'encrypted backup.')
+
         try:
             self.app.restore_backup(context, backup_info, restore_location)
         except Exception:
diff --git a/trove/guestagent/datastore/mysql_common/service.py b/trove/guestagent/datastore/mysql_common/service.py
index 2bba511a..a20aa574 100644
--- a/trove/guestagent/datastore/mysql_common/service.py
+++ b/trove/guestagent/datastore/mysql_common/service.py
@@ -677,6 +677,9 @@ class BaseMySqlApp(service.BaseDbApp):
             f'--restore-from={backup_info["location"]} '
             f'--restore-checksum={backup_info["checksum"]}'
         )
+        if CONF.backup_aes_cbc_key:
+            command = (f"{command} "
+                       f"--backup-encryption-key={CONF.backup_aes_cbc_key}")
 
         LOG.debug('Stop the database and clean up the data before restore '
                   'from %s', backup_id)
diff --git a/trove/guestagent/datastore/postgres/service.py b/trove/guestagent/datastore/postgres/service.py
index 24e818f1..404db303 100644
--- a/trove/guestagent/datastore/postgres/service.py
+++ b/trove/guestagent/datastore/postgres/service.py
@@ -267,6 +267,9 @@ class PgSqlApp(service.BaseDbApp):
             f'--restore-checksum={backup_info["checksum"]} '
             f'--pg-wal-archive-dir {WAL_ARCHIVE_DIR}'
         )
+        if CONF.backup_aes_cbc_key:
+            command = (f"{command} "
+                       f"--backup-encryption-key={CONF.backup_aes_cbc_key}")
 
         LOG.debug('Stop the database and clean up the data before restore '
                   'from %s', backup_id)
diff --git a/trove/tests/api/configurations.py b/trove/tests/api/configurations.py
index 92a61fa6..cc719c58 100644
--- a/trove/tests/api/configurations.py
+++ b/trove/tests/api/configurations.py
@@ -470,8 +470,7 @@ class ListConfigurations(ConfigurationsTestBase):
 
     @test
     def test_changing_configuration_with_nondynamic_parameter(self):
-        # test that changing a non-dynamic parameter is applied to instance
-        # and show that the instance requires a restart
+        """test_changing_configuration_with_nondynamic_parameter"""
         expected_configs = self.expected_default_datastore_configs()
         values = json.dumps(expected_configs.get('nondynamic_parameter'))
         instance_info.dbaas.configurations.update(configuration_info.id,
@@ -486,6 +485,7 @@ class ListConfigurations(ConfigurationsTestBase):
     @test(depends_on=[test_changing_configuration_with_nondynamic_parameter])
     @time_out(20)
     def test_waiting_for_instance_in_restart_required(self):
+        """test_waiting_for_instance_in_restart_required"""
         def result_is_not_active():
             instance = instance_info.dbaas.instances.get(
                 instance_info.id)
@@ -733,26 +733,6 @@ class DeleteConfigurations(ConfigurationsTestBase):
 
     @test(depends_on=[test_unassign_configuration_from_instances])
     @time_out(120)
-    def test_restart_service_after_unassign_return_active(self):
-        """test_restart_service_after_unassign_return_active"""
-        def result_is_not_active():
-            instance = instance_info.dbaas.instances.get(
-                instance_info.id)
-            if instance.status in CONFIG.running_status:
-                return False
-            else:
-                return True
-        poll_until(result_is_not_active)
-
-        config = instance_info.dbaas.configurations.list()
-        print(config)
-        instance = instance_info.dbaas.instances.get(instance_info.id)
-        resp, body = instance_info.dbaas.client.last_response
-        assert_equal(resp.status, 200)
-        assert_equal('RESTART_REQUIRED', instance.status)
-
-    @test(depends_on=[test_restart_service_after_unassign_return_active])
-    @time_out(120)
     def test_restart_service_should_return_active(self):
         """test that after restarting the instance it becomes active"""
         instance_info.dbaas.instances.restart(instance_info.id)
diff --git a/trove/tests/config.py b/trove/tests/config.py
index 85181075..49d97c3e 100644
--- a/trove/tests/config.py
+++ b/trove/tests/config.py
@@ -98,7 +98,6 @@ class TestConfig(object):
                     "valid_values": {
                         "connect_timeout": 120,
                         "local_infile": 0,
-                        "collation_server": "latin1_swedish_ci"
                     },
                     "appending_values": {
                         "join_buffer_size": 1048576,