Skip to content

Commit 392613e

Browse files
vladakvladak
committed
Allow argument substitution using environment variables.
Also overhaul the command structure.
1 parent f9a29f5 commit 392613e

File tree

5 files changed

+98
-60
lines changed

5 files changed

+98
-60
lines changed

‎tools/src/main/python/opengrok_tools/sync.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
#
2020

2121
#
22-
# Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved.
22+
# Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved.
2323
#
2424

2525
"""

‎tools/src/main/python/opengrok_tools/utils/command.py‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -344,10 +344,14 @@ def fill_arg(self, args_append=None, args_subst=None):
344344
newarg = cmdarg
345345
for pattern in args_subst.keys():
346346
if pattern in newarg and args_subst[pattern]:
347+
value = args_subst[pattern]
348+
if value.startswith("$"):
349+
self.logger.debug(f"treating {value} as environment variable")
350+
value = os.environ.get(value[1:], "")
347351
self.logger.debug("replacing '{}' in '{}' with '{}'".
348352
format(pattern, newarg,
349-
args_subst[pattern]))
350-
newarg = newarg.replace(pattern, args_subst[pattern])
353+
value))
354+
newarg = newarg.replace(pattern, value)
351355
self.logger.debug("replaced argument with {}".
352356
format(newarg))
353357
subst_done = i

‎tools/src/main/python/opengrok_tools/utils/commandsequence.py‎

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
#
1919

2020
#
21-
# Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved.
21+
# Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved.
2222
#
2323

2424
import logging
@@ -41,6 +41,10 @@
4141
HEADERS_PROPERTY = "headers"
4242
METHOD_PROPERTY = "method"
4343
URI_PROPERTY = "uri"
44+
ARGS_SUBST_PROPERTY = "args_subst"
45+
LIMITS_PROPERTY = "limits"
46+
ENV_PROPERTY = "env"
47+
ARGS_PROPERTY = "args"
4448

4549

4650
class CommandConfigurationException(Exception):
@@ -97,8 +101,8 @@ def check_command_property(command):
97101
if command_value is None and call_value is None:
98102
raise CommandConfigurationException(f"command dictionary has unknown key: {command}")
99103

100-
if command_value and not isinstance(command_value, list):
101-
raise CommandConfigurationException("command value not a list: {}".
104+
if command_value and not isinstance(command_value, dict):
105+
raise CommandConfigurationException("command value not a dictionary: {}".
102106
format(command_value))
103107
if call_value:
104108
check_call_config(call_value)
@@ -148,7 +152,7 @@ class CommandSequenceBase:
148152

149153
def __init__(self, name, commands, loglevel=logging.INFO, cleanup=None,
150154
driveon=False, url=None, env=None, http_headers=None,
151-
api_timeout=None, async_api_timeout=None, args_subst={}):
155+
api_timeout=None, async_api_timeout=None):
152156
self.name = name
153157

154158
if commands is None:
@@ -181,7 +185,6 @@ def __init__(self, name, commands, loglevel=logging.INFO, cleanup=None,
181185

182186
self.args_subst = {PROJECT_SUBST: self.name,
183187
URL_SUBST: self.url}
184-
self.args_subst.extend(args_subst)
185188

186189
def __str__(self):
187190
return str(self.name)
@@ -249,8 +252,7 @@ def run(self):
249252
if command.get(CALL_PROPERTY):
250253
try:
251254
call_rest_api(ApiCall(command.get(CALL_PROPERTY)),
252-
{PROJECT_SUBST: self.name,
253-
URL_SUBST: self.url},
255+
self.args_subst,
254256
self.http_headers,
255257
self.api_timeout,
256258
self.async_api_timeout)
@@ -262,12 +264,16 @@ def run(self):
262264

263265
break
264266
elif command.get(COMMAND_PROPERTY):
265-
command_args = command.get(COMMAND_PROPERTY)
267+
command = command.get(COMMAND_PROPERTY)
268+
command_args = command.get(ARGS_PROPERTY)
269+
args_subst = self.args_subst
270+
if command.get(ARGS_SUBST_PROPERTY):
271+
args_subst.update(command.get(ARGS_SUBST_PROPERTY))
266272
command = Command(command_args,
267-
env_vars=command.get("env"),
273+
env_vars=command.get(ENV_PROPERTY),
268274
logger=self.logger,
269-
resource_limits=command.get("limits"),
270-
args_subst=self.args_subst,
275+
resource_limits=command.get(LIMITS_PROPERTY),
276+
args_subst=args_subst,
271277
args_append=[self.name], excl_subst=True)
272278
ret_code = self.run_command(command)
273279

@@ -319,13 +325,13 @@ def run_cleanup(self):
319325
self.logger.error("API call {} failed: {}".
320326
format(cleanup_cmd, e))
321327
elif cleanup_cmd.get(COMMAND_PROPERTY):
322-
command_args = cleanup_cmd.get(COMMAND_PROPERTY)
328+
cleanup_cmd = cleanup_cmd.get(COMMAND_PROPERTY)
329+
command_args = cleanup_cmd.get(ARGS_PROPERTY)
323330
self.logger.debug("Running cleanup command '{}'".
324331
format(command_args))
325332
cmd = Command(command_args,
326333
logger=self.logger,
327-
args_subst={PROJECT_SUBST: self.name,
328-
URL_SUBST: self.url},
334+
args_subst=self.args_subst,
329335
args_append=[self.name], excl_subst=True)
330336
cmd.execute()
331337
if cmd.getretcode() != SUCCESS_EXITVAL:

‎tools/src/test/python/test_command.py‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
#
2121

2222
#
23-
# Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved.
23+
# Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved.
2424
# Portions Copyright (c) 2020, Krystof Tulinger <[email protected]>
2525
#
2626

@@ -75,6 +75,17 @@ def test_subst_multiple():
7575
assert cmd.cmd == ['foo', 'aaablahbbbxyzccc', 'bar']
7676

7777

78+
def test_subst_env():
79+
"""
80+
Test substitution with value treated as environment variable.
81+
"""
82+
os.environ["FOO"] = "foo"
83+
cmd = Command(['foo', 'aaa%ARG%bbb', 'bar'],
84+
args_subst={"%ARG%": "$FOO"})
85+
assert cmd.cmd == ['foo', 'aaafoobbb', 'bar']
86+
os.environ.pop("FOO")
87+
88+
7889
# On Windows the return code is actually 1.
7990
@pytest.mark.skipif(platform.system() == 'Windows',
8091
reason="broken on Windows")

‎tools/src/test/python/test_command_sequence.py‎

Lines changed: 59 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
#
2121

2222
#
23-
# Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved.
23+
# Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved.
2424
#
2525

2626
import os
@@ -38,8 +38,8 @@
3838

3939
def test_str():
4040
cmds = CommandSequence(CommandSequenceBase("opengrok-master",
41-
[{"command": ['foo']},
42-
{"command": ["bar"]}]))
41+
[{"command": {"args": ['foo']}},
42+
{"command": {"args": ["bar"]}}]))
4343
assert str(cmds) == "opengrok-master"
4444

4545

@@ -50,32 +50,25 @@ def test_invalid_configuration_commands_none():
5050
assert str(exc_info.value) == "commands is None"
5151

5252

53-
def test_invalid_configuration_commands_not_list():
54-
with pytest.raises(CommandConfigurationException) as exc_info:
55-
CommandSequence(CommandSequenceBase("foo", {"foo": "bar"}))
56-
57-
assert str(exc_info.value) == "commands is not a list"
58-
59-
6053
def test_invalid_configuration_commands_no_command():
6154
with pytest.raises(CommandConfigurationException) as exc_info:
62-
CommandSequence(CommandSequenceBase("foo", [{"command": ['foo']},
55+
CommandSequence(CommandSequenceBase("foo", [{"command": {"args": ['foo']}},
6356
{"foo": "bar"}]))
6457

6558
assert str(exc_info.value).startswith("command dictionary has unknown key")
6659

6760

68-
def test_invalid_configuration_commands_no_list():
61+
def test_invalid_configuration_commands_no_dict1():
6962
with pytest.raises(CommandConfigurationException) as exc_info:
70-
CommandSequence(CommandSequenceBase("foo", [{"command": ['foo']},
71-
{"command": "bar"}]))
63+
CommandSequence(CommandSequenceBase("foo", [{"command": {"args": ['foo']}},
64+
{"command": ["bar"]}]))
7265

73-
assert str(exc_info.value).startswith("command value not a list")
66+
assert str(exc_info.value).startswith("command value not a dictionary")
7467

7568

76-
def test_invalid_configuration_commands_no_dict():
69+
def test_invalid_configuration_commands_no_dict2():
7770
with pytest.raises(CommandConfigurationException) as exc_info:
78-
CommandSequence(CommandSequenceBase("foo", [{"command": ['foo']},
71+
CommandSequence(CommandSequenceBase("foo", [{"command": {"args": ['foo']}},
7972
"command"]))
8073

8174
assert str(exc_info.value).find("is not a dictionary") != -1
@@ -87,7 +80,7 @@ def test_timeout_propagation():
8780
"""
8881
expected_timeout = 11
8982
expected_api_timeout = 22
90-
cmd_seq_base = CommandSequenceBase("foo", [{"command": ['foo']}],
83+
cmd_seq_base = CommandSequenceBase("foo", [{"command": {"args": ['foo']}}],
9184
api_timeout=expected_timeout,
9285
async_api_timeout=expected_api_timeout)
9386
cmd_seq = CommandSequence(cmd_seq_base)
@@ -99,11 +92,11 @@ def test_timeout_propagation():
9992
or not os.path.exists('/bin/echo'),
10093
reason="requires Unix")
10194
def test_run_retcodes():
102-
cmd_list = [{"command": ["/bin/echo"]},
103-
{"command": ["/bin/sh", "-c",
104-
"echo " + PROJECT_SUBST + "; exit 0"]},
105-
{"command": ["/bin/sh", "-c",
106-
"echo " + PROJECT_SUBST + "; exit 1"]}]
95+
cmd_list = [{"command": {"args": ["/bin/echo"]}},
96+
{"command": {"args": ["/bin/sh", "-c",
97+
"echo " + PROJECT_SUBST + "; exit 0"]}},
98+
{"command": {"args": ["/bin/sh", "-c",
99+
"echo " + PROJECT_SUBST + "; exit 1"]}}]
107100
cmds = CommandSequence(CommandSequenceBase("opengrok-master", cmd_list))
108101
cmds.run()
109102
assert cmds.retcodes == {
@@ -117,9 +110,9 @@ def test_run_retcodes():
117110
or not os.path.exists('/bin/echo'),
118111
reason="requires Unix")
119112
def test_terminate_after_non_zero_code():
120-
cmd_list = [{"command": ["/bin/sh", "-c",
121-
"echo " + PROJECT_SUBST + "; exit 255"]},
122-
{"command": ["/bin/echo"]}]
113+
cmd_list = [{"command": {"args": ["/bin/sh", "-c",
114+
"echo " + PROJECT_SUBST + "; exit 255"]}},
115+
{"command": {"args": ["/bin/echo"]}}]
123116
cmds = CommandSequence(CommandSequenceBase("opengrok-master", cmd_list))
124117
cmds.run()
125118
assert cmds.retcodes == {
@@ -131,9 +124,9 @@ def test_terminate_after_non_zero_code():
131124
or not os.path.exists('/bin/echo'),
132125
reason="requires Unix")
133126
def test_exit_2_handling():
134-
cmd_list = [{"command": ["/bin/sh", "-c",
135-
"echo " + PROJECT_SUBST + "; exit 2"]},
136-
{"command": ["/bin/echo"]}]
127+
cmd_list = [{"command": {"args": ["/bin/sh", "-c",
128+
"echo " + PROJECT_SUBST + "; exit 2"]}},
129+
{"command": {"args": ["/bin/echo"]}}]
137130
cmds = CommandSequence(CommandSequenceBase("opengrok-master", cmd_list))
138131
cmds.run()
139132
assert cmds.retcodes == {
@@ -146,14 +139,14 @@ def test_exit_2_handling():
146139
or not os.path.exists('/bin/echo'),
147140
reason="requires Unix")
148141
def test_driveon_flag():
149-
cmd_list = [{"command": ["/bin/sh", "-c",
150-
"echo " + PROJECT_SUBST + "; exit 2"]},
151-
{"command": ["/bin/echo"]},
152-
{"command": ["/bin/sh", "-c",
153-
"echo " + PROJECT_SUBST +
154-
"; exit 1"]},
155-
{"command": ["/bin/sh", "-c",
156-
"echo " + PROJECT_SUBST]}]
142+
cmd_list = [{"command": {"args": ["/bin/sh", "-c",
143+
"echo " + PROJECT_SUBST + "; exit 2"]}},
144+
{"command": {"args": ["/bin/echo"]}},
145+
{"command": {"args": ["/bin/sh", "-c",
146+
"echo " + PROJECT_SUBST +
147+
"; exit 1"]}},
148+
{"command": {"args": ["/bin/sh", "-c",
149+
"echo " + PROJECT_SUBST]}}]
157150
cmds = CommandSequence(CommandSequenceBase("opengrok-master",
158151
cmd_list, driveon=True))
159152
cmds.run()
@@ -168,13 +161,37 @@ def test_driveon_flag():
168161
@pytest.mark.skipif(not os.path.exists('/bin/echo'),
169162
reason="requires Unix")
170163
def test_project_subst():
171-
cmd_list = [{"command": ["/bin/echo", PROJECT_SUBST]}]
164+
cmd_list = [{"command": {"args": ["/bin/echo", PROJECT_SUBST]}}]
172165
cmds = CommandSequence(CommandSequenceBase("test-subst", cmd_list))
173166
cmds.run()
174167

175168
assert cmds.outputs['/bin/echo test-subst'] == ['test-subst\n']
176169

177170

171+
@pytest.mark.skipif(not os.path.exists('/bin/echo'),
172+
reason="requires Unix")
173+
def test_args_subst():
174+
cmd_list = [{"command": {"args": ["/bin/echo", "%PATTERN%"],
175+
"args_subst": {"%PATTERN%": "foo"}}}]
176+
cmds = CommandSequence(CommandSequenceBase("test-subst", cmd_list))
177+
cmds.run()
178+
179+
assert cmds.outputs['/bin/echo foo'] == ['foo\n']
180+
181+
182+
@pytest.mark.skipif(not os.path.exists('/bin/echo'),
183+
reason="requires Unix")
184+
def test_args_subst_env():
185+
cmd_list = [{"command": {"args": ["/bin/echo", "%PATTERN%"],
186+
"args_subst": {"%PATTERN%": "$FOO"}}}]
187+
os.environ["FOO"] = "bar"
188+
cmds = CommandSequence(CommandSequenceBase("test-subst", cmd_list))
189+
cmds.run()
190+
os.environ.pop("FOO")
191+
192+
assert cmds.outputs['/bin/echo bar'] == ['bar\n']
193+
194+
178195
def test_cleanup_exception():
179196
"""
180197
If cleanup is not a list, exception should be thrown when initializing
@@ -195,12 +212,12 @@ def test_cleanup():
195212
with tempfile.TemporaryDirectory() as tmpdir:
196213
file_foo = os.path.join(tmpdir, "foo")
197214
file_bar = os.path.join(tmpdir, "bar")
198-
cleanup_list = [{"command": ["/usr/bin/touch", file_foo]},
199-
{"command": ["/bin/cat", "/totallynonexistent"]},
200-
{"command": ["/usr/bin/touch", file_bar]}]
215+
cleanup_list = [{"command": {"args": ["/usr/bin/touch", file_foo]}},
216+
{"command": {"args": ["/bin/cat", "/totallynonexistent"]}},
217+
{"command": {"args": ["/usr/bin/touch", file_bar]}}]
201218
# Running 'cat' on non-existing entry causes it to return 1.
202219
cmd = ["/bin/cat", "/foobar"]
203-
cmd_list = [{"command": cmd}]
220+
cmd_list = [{"command": {"args": cmd}}]
204221
commands = CommandSequence(CommandSequenceBase("test-cleanup-list",
205222
cmd_list,
206223
cleanup=cleanup_list))

0 commit comments

Comments
 (0)