Added warnings to command module

Generate warnings when users are shelling out to commands
rather than using modules

Can be turned off on a per-action line with the documented
warn=False flag. Can be turned off globally using
command_warnings = False in ansible config file.

Print out warnings using the standard playbook callbacks.

Created some additional tests in TestRunner.test_command
and also a demonstration playbook.
This commit is contained in:
Will Thames 2013-12-02 19:54:15 +10:00 committed by Michael DeHaan
parent f54665aa51
commit ab8490d003
5 changed files with 68 additions and 17 deletions

View file

@ -535,6 +535,9 @@ class PlaybookRunnerCallbacks(DefaultRunnerCallbacks):
display(msg, color='green', runner=self.runner) display(msg, color='green', runner=self.runner)
else: else:
display(msg, color='yellow', runner=self.runner) display(msg, color='yellow', runner=self.runner)
if constants.COMMAND_WARNINGS and 'warnings' in host_result2 and host_result2['warnings']:
for warning in host_result2['warnings']:
display("warn: %s" % warning, color='purple', runner=self.runner)
super(PlaybookRunnerCallbacks, self).on_ok(host, host_result) super(PlaybookRunnerCallbacks, self).on_ok(host, host_result)
def on_skipped(self, host, item=None): def on_skipped(self, host, item=None):

View file

@ -170,6 +170,7 @@ HOST_KEY_CHECKING = get_config(p, DEFAULTS, 'host_key_checking', '
SYSTEM_WARNINGS = get_config(p, DEFAULTS, 'system_warnings', 'ANSIBLE_SYSTEM_WARNINGS', True, boolean=True) SYSTEM_WARNINGS = get_config(p, DEFAULTS, 'system_warnings', 'ANSIBLE_SYSTEM_WARNINGS', True, boolean=True)
DEPRECATION_WARNINGS = get_config(p, DEFAULTS, 'deprecation_warnings', 'ANSIBLE_DEPRECATION_WARNINGS', True, boolean=True) DEPRECATION_WARNINGS = get_config(p, DEFAULTS, 'deprecation_warnings', 'ANSIBLE_DEPRECATION_WARNINGS', True, boolean=True)
DEFAULT_CALLABLE_WHITELIST = get_config(p, DEFAULTS, 'callable_whitelist', 'ANSIBLE_CALLABLE_WHITELIST', [], islist=True) DEFAULT_CALLABLE_WHITELIST = get_config(p, DEFAULTS, 'callable_whitelist', 'ANSIBLE_CALLABLE_WHITELIST', [], islist=True)
COMMAND_WARNINGS = get_config(p, DEFAULTS, 'command_warnings', 'ANSIBLE_COMMAND_WARNINGS', True, boolean=True)
# CONNECTION RELATED # CONNECTION RELATED
ANSIBLE_SSH_ARGS = get_config(p, 'ssh_connection', 'ssh_args', 'ANSIBLE_SSH_ARGS', None) ANSIBLE_SSH_ARGS = get_config(p, 'ssh_connection', 'ssh_args', 'ANSIBLE_SSH_ARGS', None)

View file

@ -67,6 +67,12 @@ options:
required: false required: false
default: null default: null
version_added: "0.9" version_added: "0.9"
warn:
description:
- turn off warnings about running a command that is provided by an Ansible module.
required: false
default: True
version_added: "1.5"
notes: notes:
- If you want to run a command through the shell (say you are using C(<), - If you want to run a command through the shell (say you are using C(<),
C(>), C(|), etc), you actually want the M(shell) module instead. The C(>), C(|), etc), you actually want the M(shell) module instead. The
@ -108,7 +114,27 @@ EXAMPLES = '''
# that was matched in the first 'quote' is found, or the end of # that was matched in the first 'quote' is found, or the end of
# the line is reached # the line is reached
PARAM_REGEX = re.compile(r'(^|\s)(creates|removes|chdir|executable|NO_LOG)=(?P<quote>[\'"])?(.*?)(?(quote)(?<!\\)(?P=quote))((?<!\\)(?=\s)|$)') PARAM_REGEX = re.compile(r'(^|\s)(creates|removes|chdir|executable|NO_LOG|warn)=(?P<quote>[\'"])?(.*?)(?(quote)(?<!\\)(?P=quote))((?<!\\)(?=\s)|$)')
def check_command(commandline):
arguments = { 'chown': 'owner', 'chmod': 'mode', 'chgrp': 'group',
'ln': 'state=link', 'mkdir': 'state=directory',
'rmdir': 'state=absent', 'rm': 'state=absent', 'touch': 'state=touch' }
commands = { 'git': 'git', 'hg': 'hg', 'curl': 'get_url', 'wget': 'get_url',
'svn': 'subversion', 'service': 'service',
'mount': 'mount', 'rpm': 'yum', 'yum': 'yum', 'apt-get': 'apt-get',
'tar': 'unarchive', 'unzip': 'unarchive', 'sed': 'template or lineinfile',
'echo': 'template or lineinfile', 'cp': 'synchronize or copy',
'rsync': 'synchronize' }
warnings = list()
command = os.path.basename(commandline.split()[0])
if command in arguments:
warnings.append("Consider using file module with %s rather than running %s" % (arguments[command], command))
if command in commands:
warnings.append("Consider using %s module rather than running %s" % (commands[command], command))
return warnings
def main(): def main():
@ -122,6 +148,7 @@ def main():
args = module.params['args'] args = module.params['args']
creates = module.params['creates'] creates = module.params['creates']
removes = module.params['removes'] removes = module.params['removes']
warn = module.params.get('warn', True)
if args.strip() == '': if args.strip() == '':
module.fail_json(rc=256, msg="no command given") module.fail_json(rc=256, msg="no command given")
@ -157,6 +184,10 @@ def main():
rc=0 rc=0
) )
warnings = list()
if warn:
warnings = check_command(args)
if not shell: if not shell:
args = shlex.split(args) args = shlex.split(args)
startd = datetime.datetime.now() startd = datetime.datetime.now()
@ -172,14 +203,15 @@ def main():
err = '' err = ''
module.exit_json( module.exit_json(
cmd = args, cmd = args,
stdout = out.rstrip("\r\n"), stdout = out.rstrip("\r\n"),
stderr = err.rstrip("\r\n"), stderr = err.rstrip("\r\n"),
rc = rc, rc = rc,
start = str(startd), start = str(startd),
end = str(endd), end = str(endd),
delta = str(delta), delta = str(delta),
changed = True changed = True,
warnings = warnings
) )
# import module snippets # import module snippets
@ -206,6 +238,7 @@ class CommandModule(AnsibleModule):
params['removes'] = None params['removes'] = None
params['shell'] = False params['shell'] = False
params['executable'] = None params['executable'] = None
params['warn'] = True
if "#USE_SHELL" in args: if "#USE_SHELL" in args:
args = args.replace("#USE_SHELL", "") args = args.replace("#USE_SHELL", "")
params['shell'] = True params['shell'] = True
@ -236,6 +269,7 @@ class CommandModule(AnsibleModule):
# Remove any of the above k=v params from the args string # Remove any of the above k=v params from the args string
args = PARAM_REGEX.sub('', args) args = PARAM_REGEX.sub('', args)
params['args'] = args.strip() params['args'] = args.strip()
return (params, params['args']) return (params, params['args'])
main() main()

View file

@ -41,6 +41,12 @@ options:
required: false required: false
default: null default: null
version_added: "0.9" version_added: "0.9"
warn:
description:
- turn off warnings about running a command that is provided by an Ansible module.
required: false
default: True
version_added: "1.5"
notes: notes:
- If you want to execute a command securely and predictably, it may be - If you want to execute a command securely and predictably, it may be
better to use the M(command) module instead. Best practices when writing better to use the M(command) module instead. Best practices when writing

View file

@ -115,6 +115,7 @@
- "shell_result0.rc == 0" - "shell_result0.rc == 0"
- "shell_result0.stderr == ''" - "shell_result0.stderr == ''"
- "shell_result0.stdout == 'win'" - "shell_result0.stdout == 'win'"
- "not shell_result0.warnings"
# executable # executable
@ -143,6 +144,7 @@
- "shell_result2.rc == 0" - "shell_result2.rc == 0"
- "shell_result2.stderr == ''" - "shell_result2.stderr == ''"
- "shell_result2.stdout == 'win'" - "shell_result2.stdout == 'win'"
- "not shell_result2.warnings"
# creates # creates
@ -157,17 +159,23 @@
# removes # removes
- name: execute the test.sh script with chdir - name: remove afile.txt using rm
shell: rm {{output_dir_test | expanduser}}/afile.txt removes={{output_dir_test | expanduser}}/afile.txt shell: rm {{output_dir_test | expanduser}}/afile.txt removes={{output_dir_test | expanduser}}/afile.txt
register: shell_result4
- name: assert that using rm under shell causes a warning
assert:
that:
- "shell_result4.warnings"
- name: verify that afile.txt is absent - name: verify that afile.txt is absent
file: path={{output_dir_test}}/afile.txt state=absent file: path={{output_dir_test}}/afile.txt state=absent
register: shell_result4 register: shell_result5
- name: assert that the file was removed by the shell - name: assert that the file was removed by the shell
assert: assert:
that: that:
- "shell_result4.changed == False" - "shell_result5.changed == False"
- name: execute a shell command using a literal multiline block - name: execute a shell command using a literal multiline block
args: args:
@ -181,13 +189,12 @@
| tr -s ' ' \ | tr -s ' ' \
| cut -f1 -d ' ' | cut -f1 -d ' '
echo "this is a second line" echo "this is a second line"
register: shell_result5 register: shell_result6
- debug: var=shell_result5 - debug: var=shell_result6
- name: assert the multiline shell command ran as expected - name: assert the multiline shell command ran as expected
assert: assert:
that: that:
- "shell_result5.changed" - "shell_result6.changed"
- "shell_result5.stdout == '32f3cc201b69ed8afa3902b80f554ca8\nthis is a second line'" - "shell_result6.stdout == '32f3cc201b69ed8afa3902b80f554ca8\nthis is a second line'"