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
1=== modified file 'plainbox/plainbox/abc.py'
2--- plainbox/plainbox/abc.py 2014-04-30 18:34:21 +0000
3+++ plainbox/plainbox/abc.py 2014-05-13 08:09:49 +0000
4@@ -94,6 +94,14 @@
5 """
6
7 @abstractproperty
8+ def shell(self):
9+ """
10+ Shell that is used to interpret the command
11+
12+ Defaults to 'bash' for checkbox compatibility.
13+ """
14+
15+ @abstractproperty
16 def command(self):
17 """
18 The shell command to execute to perform the job.
19
20=== modified file 'plainbox/plainbox/impl/ctrl.py'
21--- plainbox/plainbox/impl/ctrl.py 2014-03-26 20:01:03 +0000
22+++ plainbox/plainbox/impl/ctrl.py 2014-05-13 08:09:49 +0000
23@@ -580,9 +580,9 @@
24 :returns:
25 List of command arguments
26
27- This basically returns ['bash', '-c', job.command]
28+ This basically returns [job.shell, '-c', job.command]
29 """
30- return ['bash', '-c', job.command]
31+ return [job.shell, '-c', job.command]
32
33 def get_checkbox_score(self, job):
34 """
35@@ -795,8 +795,8 @@
36 job, config, nest_dir)
37 cmd += ["{key}={value}".format(key=key, value=value)
38 for key, value in sorted(env.items())]
39- # Lastly use bash -c, to run our command
40- cmd += ['bash', '-c', job.command]
41+ # Lastly use job.shell -c, to run our command
42+ cmd += [job.shell, '-c', job.command]
43 return cmd
44
45 def get_checkbox_score(self, job):
46@@ -880,8 +880,8 @@
47 job, config, nest_dir)
48 cmd += ["{key}={value}".format(key=key, value=value)
49 for key, value in sorted(env.items())]
50- # Lastly use bash -c, to run our command
51- cmd += ['bash', '-c', job.command]
52+ # Lastly use job.shell -c, to run our command
53+ cmd += [job.shell, '-c', job.command]
54 return cmd
55
56 def get_checkbox_score(self, job):
57
58=== modified file 'plainbox/plainbox/impl/secure/job.py'
59--- plainbox/plainbox/impl/secure/job.py 2014-03-21 21:33:59 +0000
60+++ plainbox/plainbox/impl/secure/job.py 2014-05-13 08:09:49 +0000
61@@ -103,6 +103,15 @@
62 return self.get_record_value('user')
63
64 @property
65+ def shell(self):
66+ """
67+ Shell that is used to interpret the command
68+
69+ Defaults to 'bash' for checkbox compatibility.
70+ """
71+ return self.get_record_value('shell', 'bash')
72+
73+ @property
74 def checksum(self):
75 """
76 Checksum of the job definition.
77
78=== modified file 'plainbox/plainbox/impl/secure/launcher1.py'
79--- plainbox/plainbox/impl/secure/launcher1.py 2014-03-21 11:18:35 +0000
80+++ plainbox/plainbox/impl/secure/launcher1.py 2014-05-13 08:09:49 +0000
81@@ -89,7 +89,7 @@
82 If the checksum does not match any known job
83 """
84 job = self.find_job(checksum)
85- cmd = ['bash', '-c', job.command]
86+ cmd = [job.shell, '-c', job.command]
87 return subprocess.call(cmd, env=self.modify_execution_environment(env))
88
89 def run_local_job(self, checksum, env):
90@@ -106,7 +106,7 @@
91 If the checksum does not match any known job
92 """
93 job = self.find_job(checksum)
94- cmd = ['bash', '-c', job.command]
95+ cmd = [job.shell, '-c', job.command]
96 output = subprocess.check_output(
97 cmd, universal_newlines=True,
98 env=self.modify_execution_environment(env))
99
100=== modified file 'plainbox/plainbox/impl/secure/test_job.py'
101--- plainbox/plainbox/impl/secure/test_job.py 2014-03-21 21:33:59 +0000
102+++ plainbox/plainbox/impl/secure/test_job.py 2014-05-13 08:09:49 +0000
103@@ -59,16 +59,19 @@
104 'command': 'command-value',
105 'environ': 'environ-value',
106 'user': 'user-value',
107+ 'shell': 'shell-value',
108 }, {
109 'plugin': 'plugin-raw',
110 'command': 'command-raw',
111 'environ': 'environ-raw',
112 'user': 'user-raw',
113+ 'shell': 'shell-raw',
114 })
115 self.assertEqual(job.plugin, "plugin-value")
116 self.assertEqual(job.command, "command-value")
117 self.assertEqual(job.environ, "environ-value")
118 self.assertEqual(job.user, "user-value")
119+ self.assertEqual(job.shell, "shell-value")
120
121 def test_properties_default_values(self):
122 """
123@@ -79,6 +82,7 @@
124 self.assertEqual(job.command, None)
125 self.assertEqual(job.environ, None)
126 self.assertEqual(job.user, None)
127+ self.assertEqual(job.shell, 'bash')
128
129 def test_checksum_smoke(self):
130 job1 = BaseJob({'plugin': 'plugin', 'user': 'root'})
131
132=== modified file 'plainbox/plainbox/impl/secure/test_launcher1.py'
133--- plainbox/plainbox/impl/secure/test_launcher1.py 2014-03-06 16:53:30 +0000
134+++ plainbox/plainbox/impl/secure/test_launcher1.py 2014-05-13 08:09:49 +0000
135@@ -85,8 +85,9 @@
136 env = {'key': 'value'}
137 # Run the tested method
138 retval = self.launcher.run_shell_from_job(job.checksum, env)
139- # Ensure that we run the job command via bash
140- mock_call.assert_called_once_with(['bash', '-c', job.command], env=env)
141+ # Ensure that we run the job command via job.shell
142+ mock_call.assert_called_once_with(
143+ [job.shell, '-c', job.command], env=env)
144 # Ensure that the return value of subprocess.call() is returned
145 self.assertEqual(retval, mock_call())
146
147@@ -100,11 +101,11 @@
148 env = {'key': 'value'}
149 # Run the tested method
150 retval = self.launcher.run_shell_from_job(job.checksum, env)
151- # Ensure that we run the job command via bash with a preserved env
152+ # Ensure that we run the job command via job.shell with a preserved env
153 expected_env = dict(os.environ)
154 expected_env.update(env)
155- mock_call.assert_called_once_with(['bash', '-c', job.command],
156- env=expected_env)
157+ mock_call.assert_called_once_with(
158+ [job.shell, '-c', job.command], env=expected_env)
159 # Ensure that the return value of subprocess.call() is returned
160 self.assertEqual(retval, mock_call())
161
162@@ -124,9 +125,9 @@
163 mock_load_rfc822_records.return_value = [record1, record2]
164 # Run the tested method
165 job_list = self.launcher.run_local_job(job.checksum, None)
166- # Ensure that we run the job command via bash
167+ # Ensure that we run the job command via job.shell
168 mock_check_output.assert_called_with(
169- ['bash', '-c', job.command], env={}, universal_newlines=True)
170+ [job.shell, '-c', job.command], env={}, universal_newlines=True)
171 # Ensure that we parse all of the output
172 mock_load_rfc822_records.assert_called_with(
173 mock_check_output(), source=JobOutputTextSource(job))
174
175=== modified file 'plainbox/plainbox/impl/test_ctrl.py'
176--- plainbox/plainbox/impl/test_ctrl.py 2014-03-26 20:01:03 +0000
177+++ plainbox/plainbox/impl/test_ctrl.py 2014-05-13 08:09:49 +0000
178@@ -617,12 +617,12 @@
179
180 def test_get_command(self):
181 """
182- verify that we simply execute the command via bash
183+ verify that we simply execute the command via job.shell
184 """
185 self.assertEqual(
186 self.ctrl.get_execution_command(
187 self.job, self.config, self.NEST_DIR),
188- ['bash', '-c', self.job.command])
189+ [self.job.shell, '-c', self.job.command])
190
191 def test_get_checkbox_score_for_jobs_without_user(self):
192 """
193@@ -888,7 +888,7 @@
194 @mock.patch.dict('os.environ', clear=True, PATH='vanilla-path')
195 def test_get_command(self):
196 """
197- verify that we run env(1) + bash(1) as the target user
198+ verify that we run env(1) + job.shell as the target user
199 """
200 self.job.get_environ_settings.return_value = []
201 self.assertEqual(
202@@ -903,7 +903,7 @@
203 os.pathsep.join([self.NEST_DIR, 'vanilla-path'])),
204 'PLAINBOX_PROVIDER_DATA=data_dir',
205 'PLAINBOX_SESSION_SHARE=session-dir/CHECKBOX_DATA',
206- 'bash', '-c', self.job.command])
207+ self.job.shell, '-c', self.job.command])
208
209 def test_get_checkbox_score_for_user_jobs(self):
210 # Assume that the job runs as the current user
211@@ -943,7 +943,7 @@
212 os.pathsep.join([self.NEST_DIR, 'vanilla-path'])),
213 'PLAINBOX_PROVIDER_DATA=data_dir',
214 'PLAINBOX_SESSION_SHARE=session-dir/CHECKBOX_DATA',
215- 'bash', '-c', self.job.command])
216+ self.job.shell, '-c', self.job.command])
217
218 SUDO, ADMIN = range(2)
219

Subscribers

People subscribed via source and target branches