Merge "Partially address concerns in Couchbase restore strategy"

6 files changed, 92 insertions, 32 deletions

diff --git a/trove/guestagent/datastore/couchbase/service.py b/trove/guestagent/datastore/couchbase/service.py
index ff4ba8b8..abc7dc0c 100644
--- a/trove/guestagent/datastore/couchbase/service.py
+++ b/trove/guestagent/datastore/couchbase/service.py
@@ -16,7 +16,9 @@
 import json
 import pexpect
 import os
+import stat
 import subprocess
+import tempfile
 
 from trove.common import cfg
 from trove.common import exception
@@ -279,16 +281,25 @@ class CouchbaseRootAccess(object):
         self.write_password_to_file(root_password)
 
     def write_password_to_file(self, root_password):
-        utils.execute_with_timeout('mkdir',
-                                   '-p',
-                                   system.COUCHBASE_CONF_DIR,
-                                   run_as_root=True,
-                                   root_helper='sudo')
-        utils.execute_with_timeout("sudo sh -c 'echo " +
-                                   root_password +
-                                   ' > ' +
-                                   system.pwd_file + "'",
-                                   shell=True)
+        utils.execute_with_timeout('mkdir', '-p', system.COUCHBASE_CONF_DIR,
+                                   run_as_root=True, root_helper='sudo')
+
+        try:
+            tempfd, tempname = tempfile.mkstemp()
+            os.fchmod(tempfd, stat.S_IRUSR | stat.S_IWUSR)
+            os.write(tempfd, root_password)
+            os.fchmod(tempfd, stat.S_IRUSR)
+            os.close(tempfd)
+        except OSError as err:
+            message = _("An error occurred in saving password "
+                        "(%(errno)s). %(strerror)s.") % {
+                            "errno": err.errno,
+                            "strerror": err.strerror}
+            LOG.exception(message)
+            raise RuntimeError(message)
+
+        utils.execute_with_timeout('mv', tempname, system.pwd_file,
+                                   run_as_root=True, root_helper='sudo')
 
     @staticmethod
     def get_password():
diff --git a/trove/guestagent/strategies/backup/couchbase_impl.py b/trove/guestagent/strategies/backup/couchbase_impl.py
index 97359655..9156ee29 100644
--- a/trove/guestagent/strategies/backup/couchbase_impl.py
+++ b/trove/guestagent/strategies/backup/couchbase_impl.py
@@ -36,13 +36,12 @@ class CbBackup(base.BackupRunner):
     __strategy_name__ = 'cbbackup'
 
     pre_backup_commands = [
-        'rm -rf ' + system.COUCHBASE_DUMP_DIR,
-
-        'mkdir -p ' + system.COUCHBASE_DUMP_DIR,
+        ['rm', '-rf', system.COUCHBASE_DUMP_DIR],
+        ['mkdir', '-p', system.COUCHBASE_DUMP_DIR],
     ]
 
     post_backup_commands = [
-        'rm -rf ' + system.COUCHBASE_DUMP_DIR,
+        ['rm', '-rf', system.COUCHBASE_DUMP_DIR],
     ]
 
     @property
@@ -50,7 +49,7 @@ class CbBackup(base.BackupRunner):
         """
         Creates backup dump dir, tars it up, and encrypts it.
         """
-        cmd = 'tar cPf - ' + system.COUCHBASE_DUMP_DIR
+        cmd = 'tar cpPf - ' + system.COUCHBASE_DUMP_DIR
         return cmd + self.zip_cmd + self.encrypt_cmd
 
     def _save_buckets_config(self, password):
@@ -60,16 +59,16 @@ class CbBackup(base.BackupRunner):
                                    shell=True, timeout=300)
 
     def _backup(self, password):
-        utils.execute_with_timeout('/opt/couchbase/bin/cbbackup ' +
-                                   system.COUCHBASE_REST_API + ' ' +
-                                   system.COUCHBASE_DUMP_DIR +
-                                   ' -u root -p ' + password,
-                                   shell=True, timeout=600)
+        utils.execute_with_timeout(['/opt/couchbase/bin/cbbackup',
+                                    system.COUCHBASE_REST_API,
+                                    system.COUCHBASE_DUMP_DIR,
+                                    '-u', 'root', '-p', password],
+                                   timeout=600)
 
     def _run_pre_backup(self):
         try:
             for cmd in self.pre_backup_commands:
-                utils.execute_with_timeout(cmd, shell=True)
+                utils.execute_with_timeout(cmd)
             root = service.CouchbaseRootAccess()
             pw = root.get_password()
             self._save_buckets_config(pw)
@@ -88,14 +87,15 @@ class CbBackup(base.BackupRunner):
                     else:
                         LOG.info(_("All buckets are memcached.  "
                                    "Skipping backup."))
-            utils.execute_with_timeout("mv " + OUTFILE + " " +
-                                       system.COUCHBASE_DUMP_DIR,
-                                       shell=True)
+            utils.execute_with_timeout(['mv', OUTFILE,
+                                        system.COUCHBASE_DUMP_DIR])
             if pw != "password":
                 # Not default password, backup generated root password
-                utils.execute_with_timeout('sudo cp ' + system.pwd_file +
-                                           ' ' + system.COUCHBASE_DUMP_DIR,
-                                           shell=True)
+                utils.execute_with_timeout(['cp', '-p',
+                                            system.pwd_file,
+                                            system.COUCHBASE_DUMP_DIR],
+                                           run_as_root=True,
+                                           root_helper='sudo')
         except exception.ProcessExecutionError as p:
             LOG.error(p)
             raise p
@@ -103,7 +103,7 @@ class CbBackup(base.BackupRunner):
     def _run_post_backup(self):
         try:
             for cmd in self.post_backup_commands:
-                utils.execute_with_timeout(cmd, shell=True)
+                utils.execute_with_timeout(cmd)
         except exception.ProcessExecutionError as p:
             LOG.error(p)
             raise p
diff --git a/trove/guestagent/strategies/restore/couchbase_impl.py b/trove/guestagent/strategies/restore/couchbase_impl.py
index f3daddb8..185fd3fe 100644
--- a/trove/guestagent/strategies/restore/couchbase_impl.py
+++ b/trove/guestagent/strategies/restore/couchbase_impl.py
@@ -35,7 +35,7 @@ class CbBackup(base.RestoreRunner):
     Implementation of Restore Strategy for Couchbase.
     """
     __strategy_name__ = 'cbbackup'
-    base_restore_cmd = 'sudo tar xPf -'
+    base_restore_cmd = 'sudo tar xpPf -'
 
     def __init__(self, *args, **kwargs):
         super(CbBackup, self).__init__(*args, **kwargs)
diff --git a/trove/tests/unittests/backup/test_backupagent.py b/trove/tests/unittests/backup/test_backupagent.py
index b8f83a91..6cec405d 100644
--- a/trove/tests/unittests/backup/test_backupagent.py
+++ b/trove/tests/unittests/backup/test_backupagent.py
@@ -217,7 +217,7 @@ class BackupAgentTest(testtools.TestCase):
         utils.execute_with_timeout = Mock(return_value=None)
         cbbackup = couchbase_impl.CbBackup('cbbackup', extra_opts='')
         self.assertIsNotNone(cbbackup)
-        str_cbbackup_cmd = ("tar cPf - /tmp/backups | "
+        str_cbbackup_cmd = ("tar cpPf - /tmp/backups | "
                             "gzip | openssl enc -aes-256-cbc -salt -pass "
                             "pass:default_aes_cbc_key")
         self.assertEqual(str_cbbackup_cmd, cbbackup.cmd)
diff --git a/trove/tests/unittests/guestagent/test_backups.py b/trove/tests/unittests/guestagent/test_backups.py
index e70671c7..2cd38b25 100644
--- a/trove/tests/unittests/guestagent/test_backups.py
+++ b/trove/tests/unittests/guestagent/test_backups.py
@@ -69,9 +69,9 @@ PREPARE = ("sudo innobackupex --apply-log /var/lib/mysql "
            "--ibbackup xtrabackup 2>/tmp/innoprepare.log")
 CRYPTO_KEY = "default_aes_cbc_key"
 
-CBBACKUP_CMD = "tar cPf - /tmp/backups"
+CBBACKUP_CMD = "tar cpPf - /tmp/backups"
 
-CBBACKUP_RESTORE = "sudo tar xPf -"
+CBBACKUP_RESTORE = "sudo tar xpPf -"
 
 
 class GuestAgentBackupTest(testtools.TestCase):
diff --git a/trove/tests/unittests/guestagent/test_couchbase_manager.py b/trove/tests/unittests/guestagent/test_couchbase_manager.py
index a1235f08..e86eeac8 100644
--- a/trove/tests/unittests/guestagent/test_couchbase_manager.py
+++ b/trove/tests/unittests/guestagent/test_couchbase_manager.py
@@ -12,8 +12,14 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import mock
+import os
+import stat
+import tempfile
 import testtools
 from mock import MagicMock
+from mock import Mock
+from trove.common import utils
 from trove.common.context import TroveContext
 from trove.guestagent import volume
 from trove.guestagent import backup
@@ -127,3 +133,46 @@ class GuestAgentCouchbaseManagerTest(testtools.TestCase):
         #verification/assertion
         couch_service.CouchbaseApp.stop_db.assert_any_call(
             do_not_start_on_reboot=False)
+
+    def __fake_mkstemp(self):
+        self.tempfd, self.tempname = self.original_mkstemp()
+        return self.tempfd, self.tempname
+
+    def __fake_mkstemp_raise(self):
+        raise OSError(11, 'Resource temporarily unavailable')
+
+    def __cleanup_tempfile(self):
+        if self.tempname:
+            os.unlink(self.tempname)
+
+    @mock.patch.object(utils, 'execute_with_timeout', Mock(return_value=0))
+    def test_write_password_to_file1(self):
+        self.original_mkstemp = tempfile.mkstemp
+        self.tempname = None
+
+        with mock.patch.object(tempfile,
+                               'mkstemp',
+                               self.__fake_mkstemp):
+            self.addCleanup(self.__cleanup_tempfile)
+
+            rootaccess = couch_service.CouchbaseRootAccess()
+            rootaccess.write_password_to_file('mypassword')
+
+            filepermissions = os.stat(self.tempname).st_mode
+            self.assertEqual(
+                filepermissions & 0o777, stat.S_IRUSR)
+
+    @mock.patch.object(utils, 'execute_with_timeout', Mock(return_value=0))
+    def test_write_password_to_file2(self):
+        self.original_mkstemp = tempfile.mkstemp
+        self.tempname = None
+
+        with mock.patch.object(tempfile,
+                               'mkstemp',
+                               self.__fake_mkstemp_raise):
+
+            rootaccess = couch_service.CouchbaseRootAccess()
+
+            self.assertRaises(RuntimeError,
+                              rootaccess.write_password_to_file,
+                              'mypassword')