Checkbox

Merge lp:~zyga/checkbox/fix-1303955 into lp:checkbox

  1. fix-1303955
  2. Merge into trunk
Proposed by Zygmunt Krynicki
Status: Merged
Approved by: Brendan Donegan
Approved revision: 2991
Merged at revision: 2990
Proposed branch: lp:~zyga/checkbox/fix-1303955
Merge into: lp:checkbox
Diff against target: 218 lines (+42/-20)
7 files modified
plainbox/plainbox/abc.py (+8/-0)
plainbox/plainbox/impl/ctrl.py (+6/-6)
plainbox/plainbox/impl/secure/job.py (+9/-0)
plainbox/plainbox/impl/secure/launcher1.py (+2/-2)
plainbox/plainbox/impl/secure/test_job.py (+4/-0)
plainbox/plainbox/impl/secure/test_launcher1.py (+8/-7)
plainbox/plainbox/impl/test_ctrl.py (+5/-5)
To merge this branch: bzr merge lp:~zyga/checkbox/fix-1303955
Reviewer Review Type Date Requested Status
Brendan Donegan (community) Approve
Review via email: mp+219297@code.launchpad.net

Description of the change

1812357 plainbox:secure:job: add job.shell property
2a9184f plainbox:abc: add IJobDefinition.shell
38aa014 plainbox:ctrl: use job.shell instead of hardcoded 'bash'
717f10b plainbox:secure:launcher: use job.shell instead of hardcoded 'bash'

To post a comment you must log in.
Revision history for this message
Brendan Donegan (brendan-donegan) wrote :

Looks good, +1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'plainbox/plainbox/abc.py'
--- plainbox/plainbox/abc.py 2014-04-30 18:34:21 +0000
+++ plainbox/plainbox/abc.py 2014-05-13 08:09:49 +0000
@@ -94,6 +94,14 @@
94 """94 """
9595
96 @abstractproperty96 @abstractproperty
97 def shell(self):
98 """
99 Shell that is used to interpret the command
100
101 Defaults to 'bash' for checkbox compatibility.
102 """
103
104 @abstractproperty
97 def command(self):105 def command(self):
98 """106 """
99 The shell command to execute to perform the job.107 The shell command to execute to perform the job.
100108
=== modified file 'plainbox/plainbox/impl/ctrl.py'
--- plainbox/plainbox/impl/ctrl.py 2014-03-26 20:01:03 +0000
+++ plainbox/plainbox/impl/ctrl.py 2014-05-13 08:09:49 +0000
@@ -580,9 +580,9 @@
580 :returns:580 :returns:
581 List of command arguments581 List of command arguments
582582
583 This basically returns ['bash', '-c', job.command]583 This basically returns [job.shell, '-c', job.command]
584 """584 """
585 return ['bash', '-c', job.command]585 return [job.shell, '-c', job.command]
586586
587 def get_checkbox_score(self, job):587 def get_checkbox_score(self, job):
588 """588 """
@@ -795,8 +795,8 @@
795 job, config, nest_dir)795 job, config, nest_dir)
796 cmd += ["{key}={value}".format(key=key, value=value)796 cmd += ["{key}={value}".format(key=key, value=value)
797 for key, value in sorted(env.items())]797 for key, value in sorted(env.items())]
798 # Lastly use bash -c, to run our command798 # Lastly use job.shell -c, to run our command
799 cmd += ['bash', '-c', job.command]799 cmd += [job.shell, '-c', job.command]
800 return cmd800 return cmd
801801
802 def get_checkbox_score(self, job):802 def get_checkbox_score(self, job):
@@ -880,8 +880,8 @@
880 job, config, nest_dir)880 job, config, nest_dir)
881 cmd += ["{key}={value}".format(key=key, value=value)881 cmd += ["{key}={value}".format(key=key, value=value)
882 for key, value in sorted(env.items())]882 for key, value in sorted(env.items())]
883 # Lastly use bash -c, to run our command883 # Lastly use job.shell -c, to run our command
884 cmd += ['bash', '-c', job.command]884 cmd += [job.shell, '-c', job.command]
885 return cmd885 return cmd
886886
887 def get_checkbox_score(self, job):887 def get_checkbox_score(self, job):
888888
=== modified file 'plainbox/plainbox/impl/secure/job.py'
--- plainbox/plainbox/impl/secure/job.py 2014-03-21 21:33:59 +0000
+++ plainbox/plainbox/impl/secure/job.py 2014-05-13 08:09:49 +0000
@@ -103,6 +103,15 @@
103 return self.get_record_value('user')103 return self.get_record_value('user')
104104
105 @property105 @property
106 def shell(self):
107 """
108 Shell that is used to interpret the command
109
110 Defaults to 'bash' for checkbox compatibility.
111 """
112 return self.get_record_value('shell', 'bash')
113
114 @property
106 def checksum(self):115 def checksum(self):
107 """116 """
108 Checksum of the job definition.117 Checksum of the job definition.
109118
=== modified file 'plainbox/plainbox/impl/secure/launcher1.py'
--- plainbox/plainbox/impl/secure/launcher1.py 2014-03-21 11:18:35 +0000
+++ plainbox/plainbox/impl/secure/launcher1.py 2014-05-13 08:09:49 +0000
@@ -89,7 +89,7 @@
89 If the checksum does not match any known job89 If the checksum does not match any known job
90 """90 """
91 job = self.find_job(checksum)91 job = self.find_job(checksum)
92 cmd = ['bash', '-c', job.command]92 cmd = [job.shell, '-c', job.command]
93 return subprocess.call(cmd, env=self.modify_execution_environment(env))93 return subprocess.call(cmd, env=self.modify_execution_environment(env))
9494
95 def run_local_job(self, checksum, env):95 def run_local_job(self, checksum, env):
@@ -106,7 +106,7 @@
106 If the checksum does not match any known job106 If the checksum does not match any known job
107 """107 """
108 job = self.find_job(checksum)108 job = self.find_job(checksum)
109 cmd = ['bash', '-c', job.command]109 cmd = [job.shell, '-c', job.command]
110 output = subprocess.check_output(110 output = subprocess.check_output(
111 cmd, universal_newlines=True,111 cmd, universal_newlines=True,
112 env=self.modify_execution_environment(env))112 env=self.modify_execution_environment(env))
113113
=== modified file 'plainbox/plainbox/impl/secure/test_job.py'
--- plainbox/plainbox/impl/secure/test_job.py 2014-03-21 21:33:59 +0000
+++ plainbox/plainbox/impl/secure/test_job.py 2014-05-13 08:09:49 +0000
@@ -59,16 +59,19 @@
59 'command': 'command-value',59 'command': 'command-value',
60 'environ': 'environ-value',60 'environ': 'environ-value',
61 'user': 'user-value',61 'user': 'user-value',
62 'shell': 'shell-value',
62 }, {63 }, {
63 'plugin': 'plugin-raw',64 'plugin': 'plugin-raw',
64 'command': 'command-raw',65 'command': 'command-raw',
65 'environ': 'environ-raw',66 'environ': 'environ-raw',
66 'user': 'user-raw',67 'user': 'user-raw',
68 'shell': 'shell-raw',
67 })69 })
68 self.assertEqual(job.plugin, "plugin-value")70 self.assertEqual(job.plugin, "plugin-value")
69 self.assertEqual(job.command, "command-value")71 self.assertEqual(job.command, "command-value")
70 self.assertEqual(job.environ, "environ-value")72 self.assertEqual(job.environ, "environ-value")
71 self.assertEqual(job.user, "user-value")73 self.assertEqual(job.user, "user-value")
74 self.assertEqual(job.shell, "shell-value")
7275
73 def test_properties_default_values(self):76 def test_properties_default_values(self):
74 """77 """
@@ -79,6 +82,7 @@
79 self.assertEqual(job.command, None)82 self.assertEqual(job.command, None)
80 self.assertEqual(job.environ, None)83 self.assertEqual(job.environ, None)
81 self.assertEqual(job.user, None)84 self.assertEqual(job.user, None)
85 self.assertEqual(job.shell, 'bash')
8286
83 def test_checksum_smoke(self):87 def test_checksum_smoke(self):
84 job1 = BaseJob({'plugin': 'plugin', 'user': 'root'})88 job1 = BaseJob({'plugin': 'plugin', 'user': 'root'})
8589
=== modified file 'plainbox/plainbox/impl/secure/test_launcher1.py'
--- plainbox/plainbox/impl/secure/test_launcher1.py 2014-03-06 16:53:30 +0000
+++ plainbox/plainbox/impl/secure/test_launcher1.py 2014-05-13 08:09:49 +0000
@@ -85,8 +85,9 @@
85 env = {'key': 'value'}85 env = {'key': 'value'}
86 # Run the tested method86 # Run the tested method
87 retval = self.launcher.run_shell_from_job(job.checksum, env)87 retval = self.launcher.run_shell_from_job(job.checksum, env)
88 # Ensure that we run the job command via bash88 # Ensure that we run the job command via job.shell
89 mock_call.assert_called_once_with(['bash', '-c', job.command], env=env)89 mock_call.assert_called_once_with(
90 [job.shell, '-c', job.command], env=env)
90 # Ensure that the return value of subprocess.call() is returned91 # Ensure that the return value of subprocess.call() is returned
91 self.assertEqual(retval, mock_call())92 self.assertEqual(retval, mock_call())
9293
@@ -100,11 +101,11 @@
100 env = {'key': 'value'}101 env = {'key': 'value'}
101 # Run the tested method102 # Run the tested method
102 retval = self.launcher.run_shell_from_job(job.checksum, env)103 retval = self.launcher.run_shell_from_job(job.checksum, env)
103 # Ensure that we run the job command via bash with a preserved env104 # Ensure that we run the job command via job.shell with a preserved env
104 expected_env = dict(os.environ)105 expected_env = dict(os.environ)
105 expected_env.update(env)106 expected_env.update(env)
106 mock_call.assert_called_once_with(['bash', '-c', job.command],107 mock_call.assert_called_once_with(
107 env=expected_env)108 [job.shell, '-c', job.command], env=expected_env)
108 # Ensure that the return value of subprocess.call() is returned109 # Ensure that the return value of subprocess.call() is returned
109 self.assertEqual(retval, mock_call())110 self.assertEqual(retval, mock_call())
110111
@@ -124,9 +125,9 @@
124 mock_load_rfc822_records.return_value = [record1, record2]125 mock_load_rfc822_records.return_value = [record1, record2]
125 # Run the tested method126 # Run the tested method
126 job_list = self.launcher.run_local_job(job.checksum, None)127 job_list = self.launcher.run_local_job(job.checksum, None)
127 # Ensure that we run the job command via bash128 # Ensure that we run the job command via job.shell
128 mock_check_output.assert_called_with(129 mock_check_output.assert_called_with(
129 ['bash', '-c', job.command], env={}, universal_newlines=True)130 [job.shell, '-c', job.command], env={}, universal_newlines=True)
130 # Ensure that we parse all of the output131 # Ensure that we parse all of the output
131 mock_load_rfc822_records.assert_called_with(132 mock_load_rfc822_records.assert_called_with(
132 mock_check_output(), source=JobOutputTextSource(job))133 mock_check_output(), source=JobOutputTextSource(job))
133134
=== modified file 'plainbox/plainbox/impl/test_ctrl.py'
--- plainbox/plainbox/impl/test_ctrl.py 2014-03-26 20:01:03 +0000
+++ plainbox/plainbox/impl/test_ctrl.py 2014-05-13 08:09:49 +0000
@@ -617,12 +617,12 @@
617617
618 def test_get_command(self):618 def test_get_command(self):
619 """619 """
620 verify that we simply execute the command via bash620 verify that we simply execute the command via job.shell
621 """621 """
622 self.assertEqual(622 self.assertEqual(
623 self.ctrl.get_execution_command(623 self.ctrl.get_execution_command(
624 self.job, self.config, self.NEST_DIR),624 self.job, self.config, self.NEST_DIR),
625 ['bash', '-c', self.job.command])625 [self.job.shell, '-c', self.job.command])
626626
627 def test_get_checkbox_score_for_jobs_without_user(self):627 def test_get_checkbox_score_for_jobs_without_user(self):
628 """628 """
@@ -888,7 +888,7 @@
888 @mock.patch.dict('os.environ', clear=True, PATH='vanilla-path')888 @mock.patch.dict('os.environ', clear=True, PATH='vanilla-path')
889 def test_get_command(self):889 def test_get_command(self):
890 """890 """
891 verify that we run env(1) + bash(1) as the target user891 verify that we run env(1) + job.shell as the target user
892 """892 """
893 self.job.get_environ_settings.return_value = []893 self.job.get_environ_settings.return_value = []
894 self.assertEqual(894 self.assertEqual(
@@ -903,7 +903,7 @@
903 os.pathsep.join([self.NEST_DIR, 'vanilla-path'])),903 os.pathsep.join([self.NEST_DIR, 'vanilla-path'])),
904 'PLAINBOX_PROVIDER_DATA=data_dir',904 'PLAINBOX_PROVIDER_DATA=data_dir',
905 'PLAINBOX_SESSION_SHARE=session-dir/CHECKBOX_DATA',905 'PLAINBOX_SESSION_SHARE=session-dir/CHECKBOX_DATA',
906 'bash', '-c', self.job.command])906 self.job.shell, '-c', self.job.command])
907907
908 def test_get_checkbox_score_for_user_jobs(self):908 def test_get_checkbox_score_for_user_jobs(self):
909 # Assume that the job runs as the current user909 # Assume that the job runs as the current user
@@ -943,7 +943,7 @@
943 os.pathsep.join([self.NEST_DIR, 'vanilla-path'])),943 os.pathsep.join([self.NEST_DIR, 'vanilla-path'])),
944 'PLAINBOX_PROVIDER_DATA=data_dir',944 'PLAINBOX_PROVIDER_DATA=data_dir',
945 'PLAINBOX_SESSION_SHARE=session-dir/CHECKBOX_DATA',945 'PLAINBOX_SESSION_SHARE=session-dir/CHECKBOX_DATA',
946 'bash', '-c', self.job.command])946 self.job.shell, '-c', self.job.command])
947947
948 SUDO, ADMIN = range(2)948 SUDO, ADMIN = range(2)
949949

Subscribers

People subscribed via source and target branches