Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added cloud/Amazon module for SSM #19868

Closed
wants to merge 4 commits into from

Conversation

woznij
Copy link

@woznij woznij commented Jan 4, 2017

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

ssm_send_command

ANSIBLE VERSION
ansible 2.2.0 (devel 29fda4be1e) last updated 2016/09/16 18:26:39 (GMT +000)
  lib/ansible/modules/core: (detached HEAD 2e1e3562b9) last updated 2016/09/16 18:26:41 (GMT +000)
  lib/ansible/modules/extras: (detached HEAD 9b5c64e240) last updated 2016/09/16 18:26:42 (GMT +000)
  config file =
  configured module search path = Default w/o overrides

SUMMARY

This module allows you to interface with Amazon Simple Systems Manager (SSM) to manage the configuration of your Amazon EC2 instances. Specifically this module allows you to use the Run Command functionality to run system commands against your hosts. E.g. you can run PowerShell commands against Windows hosts or bash commands against a Linux host. The specific advantage is that you do not need to log into those hosts as the SSM service takes care of the execution. For more information see http://docs.aws.amazon.com/ssm/latest/APIReference/Welcome.html

This module allows you to interface with Amazon Simple Systems Manager (SSM) to manage the configuration of your Amazon EC2 instances.  Specifically this module allows you to use the Run Command functionality to run system commands against your hosts. E.g. you can run PowerShell commands against Windows hosts or bash commands against a Linux host. The specific advantage is that you do not need to log into those hosts as the SSM service takes care of the execution. For more information see http://docs.aws.amazon.com/ssm/latest/APIReference/Welcome.html
@woznij
Copy link
Author

woznij commented Jan 4, 2017

Migrated from ansible/ansible-modules-extras#3294

@ansibot ansibot added affects_2.3 This issue/PR affects Ansible v2.3 aws cloud module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. new_plugin This PR includes a new plugin. plugin labels Jan 4, 2017
@jimi-c jimi-c removed the plugin label Jan 4, 2017
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. new_module This PR includes a new module. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jan 5, 2017
@nitzmahone nitzmahone removed the needs_triage Needs a first human triage before being processed. label Jan 5, 2017
@nitzmahone
Copy link
Member

nitzmahone commented Jan 6, 2017

Rackspace has been teasing a connection plugin they wrote that uses SSM as well- I've reached out to see if we should expect it anytime soon. I'm not opposed to overlapping functionality here either, but a connection plugin really is the "ansible-y" way to do this sort of thing...

@ansibot ansibot added the community_review In order to be merged, this PR must follow the community review workflow. label Jan 7, 2017
@ansibot
Copy link
Contributor

ansibot commented Jan 31, 2017

sample: Success
'''

# import base64
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove these if not required

if not HAS_BOTO3:
module.fail_json(msg='Python module "boto3" is missing, please install it')

if not (document_name and instance_ids):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why aren't these required in the argument_spec?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still unanswered

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These parameters are set to required=True however I found that if you specify the parameter in a playbook but leave it blank, it still gets past that check. I put this in as a more friendly way of handling the exception than letting the exception generated by the the boto call to bubble up. If there is a more "Ansible-y" way of handling this, let me know. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, that makes sense, was just curious why it was required.

module.exit_json(changed=True, result=results)

# import module snippets
from ansible.module_utils.basic import *
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to use new style module imports

from ansible.module_utils.basic import AnsibleModule
from ansible.module_utils.ec2 import boto3_conn, ec2_argument_spec, get_aws_connection_info

preferably at the top of the file to avoid flake8 complaints.

short_description: Execute commands through Simple System Manager (SSM) a.k.a. Run Command
description:
- This module allows you to execute commands through SSM/Run Command.
version_added: "2.2"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This regressed back in your most recent commit

sample: Success
'''

from ansible.module_utils.basic import *
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just import AnsibleModule here

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Feb 1, 2017
@willthames
Copy link
Contributor

!needs_revision

@willthames
Copy link
Contributor

shipit

@willthames
Copy link
Contributor

@nitzmahone the SSM connection plugin does sound like it might be simpler for some use cases - guess it depends on what else you're doing in the rest of your playbook.

Copy link
Contributor

@wimnat wimnat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs_revision

- A comment about this particular invocation.
required: false
default: NONE
instanceIds:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would change this to ansible preferred snake_case. instance_ids

def main():
argument_spec = ec2_argument_spec()
argument_spec.update(dict(
name = dict(required=True),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should try to follow pep8. Therefore, the spacing here isn't required. Also, applies to the rest of the dict

supports_check_mode=False
)

document_name = module.params.get('name') # Needs to be an existing SSM document name
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, pep8 means no spaces here

"the document name is correct and your profile has "
"permissions to execute SSM.",
exception=traceback.format_exc(ce))
module.fail_json(msg="Client-side error when invoking SSM, check inputs and specific error",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

module.fail_json(msg="Client-side error when invoking SSM, check inputs and specific error",
exception=traceback.format_exc(ce))
except botocore.exceptions.ParamValidationError as ve:
module.fail_json(msg="Parameters to `invoke` failed to validate",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while checking:
try:
invoke_response = client.list_command_invocations(**list_params)
except Exception as e:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only capture the specific exception type here and handle appropriately https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/cloud/amazon/GUIDELINES.md#boto3-2

@nitzmahone
Copy link
Member

Discussed offline with the author of the SSM connection plugin, and decided that both would be good to have. Once everyone's happy with implementation on this one, I'm good to merge.

@ansibot
Copy link
Contributor

ansibot commented Feb 16, 2017

@woznij This PR contains @ mentions in at least one commit message. Those mentions can cause cascading notifications through GitHub and need to be removed. Please squash or amend your commits to remove the mentions.

click here for bot help

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Apr 11, 2017
@ryansb
Copy link
Contributor

ryansb commented May 16, 2017

@woznij can you please rebase this and change the "version_added" docs property?

@ansibot
Copy link
Contributor

ansibot commented May 16, 2017

The test ansible-test sanity --test pep8 failed with the following errors:

lib/ansible/modules/cloud/amazon/ssm_send_command.py:112:13: E221 multiple spaces before operator
lib/ansible/modules/cloud/amazon/ssm_send_command.py:112:13: E251 unexpected spaces around keyword / parameter equals
lib/ansible/modules/cloud/amazon/ssm_send_command.py:112:31: E251 unexpected spaces around keyword / parameter equals
lib/ansible/modules/cloud/amazon/ssm_send_command.py:113:13: E221 multiple spaces before operator
lib/ansible/modules/cloud/amazon/ssm_send_command.py:113:13: E251 unexpected spaces around keyword / parameter equals
lib/ansible/modules/cloud/amazon/ssm_send_command.py:113:31: E251 unexpected spaces around keyword / parameter equals
lib/ansible/modules/cloud/amazon/ssm_send_command.py:114:16: E221 multiple spaces before operator
lib/ansible/modules/cloud/amazon/ssm_send_command.py:114:16: E251 unexpected spaces around keyword / parameter equals
lib/ansible/modules/cloud/amazon/ssm_send_command.py:114:31: E251 unexpected spaces around keyword / parameter equals
lib/ansible/modules/cloud/amazon/ssm_send_command.py:115:20: E221 multiple spaces before operator
lib/ansible/modules/cloud/amazon/ssm_send_command.py:115:20: E251 unexpected spaces around keyword / parameter equals
lib/ansible/modules/cloud/amazon/ssm_send_command.py:115:31: E251 unexpected spaces around keyword / parameter equals
lib/ansible/modules/cloud/amazon/ssm_send_command.py:116:19: E221 multiple spaces before operator
lib/ansible/modules/cloud/amazon/ssm_send_command.py:116:19: E251 unexpected spaces around keyword / parameter equals
lib/ansible/modules/cloud/amazon/ssm_send_command.py:116:31: E251 unexpected spaces around keyword / parameter equals
lib/ansible/modules/cloud/amazon/ssm_send_command.py:198:18: E225 missing whitespace around operator
lib/ansible/modules/cloud/amazon/ssm_send_command.py:203:18: E225 missing whitespace around operator

The test ansible-test sanity --test validate-modules failed with the following errors:

lib/ansible/modules/cloud/amazon/ssm_send_command.py:0:0: E307 version_added should be 2.4. Currently 2.3
lib/ansible/modules/cloud/amazon/ssm_send_command.py:0:0: E314 No ANSIBLE_METADATA provided
lib/ansible/modules/cloud/amazon/ssm_send_command.py:0:0: E319 RETURN.status.returned: required key not provided @ data['returned']. Got None
lib/ansible/modules/cloud/amazon/ssm_send_command.py:74:15: E311 EXAMPLES is not valid YAML

click here for bot help

@ansibot ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label May 16, 2017
@mattclay mattclay added the ci_verified Changes made in this PR are causing tests to fail. label May 16, 2017
@ansibot ansibot added stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:community This issue/PR relates to code supported by the Ansible community. labels Jun 24, 2017
@destroy-everything
Copy link

what is going on here? it has been a while since there was any progress? what can I do to help?

@woznij
Copy link
Author

woznij commented Sep 29, 2017

@destroy-everything It HAS been a while! I got pulled away on assignment at work and have not had time to come back to this. It has turned into a bit of a black hole it seems. I fix one thing and another thing is found, lots of small differentiating changes. With my workload as of recent it has made it difficult to keep up. This is really my first contribution to a large public repo so following the rules is a bit of a learning curve too, and it seems the rules have changed a few times since I started this. All that is just an excuse really and I really should just sit down and do it. Other than time, my blockers are reading up on how to "rebase" properly, read up on what "ANSIBLE_METADATA" is, read up on "RETURN.status.returned" should be. The other outstanding things like PEP8 tweaks should be pretty easy. I will note that a good chunk of this code was reused from another approved and merged module so some of the things that were caught must be because of new checks that are being performed.

@willthames
Copy link
Contributor

@woznij it is a bit of a moving target I'm afraid but a slow moving one that we're trying to better document. Some of it is that the project's coding standards are improving so we can do more checking of the obvious stuff, some of it is just evolution in action.

We should also just document a 'perfect' module skeleton that people can just copy and paste the relevant sections from into their work.

@willthames
Copy link
Contributor

willthames commented Oct 9, 2017

Metadata documentation

RETURN documentation

 E319 RETURN.status.returned: required key not provided @ data['returned']. Got None

just means that in your RETURN section, you have a status block that does not have a setting for returned.

@ansibot ansibot added the new_contributor This PR is the first contribution by a new community member. label Oct 18, 2017
@ansibot ansibot removed the new_contributor This PR is the first contribution by a new community member. label Nov 3, 2017
@ansibot ansibot added the new_contributor This PR is the first contribution by a new community member. label Jan 22, 2018
@RobReus
Copy link
Contributor

RobReus commented Jul 25, 2018

This PR has been inactive for over half a year now. Any activities going on?

I have written the same module for internal use with the same functionality. Should we try to fix this PR or should I open a fresh PR with my changes?

@gundalow
Copy link
Contributor

I'll close this and move to #54931

@woznij @RobReus

@gundalow gundalow closed this Sep 19, 2019
@ansible ansible locked and limited conversation to collaborators Oct 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.3 This issue/PR affects Ansible v2.3 aws ci_verified Changes made in this PR are causing tests to fail. cloud module This issue/PR relates to a module. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. new_contributor This PR is the first contribution by a new community member. new_module This PR includes a new module. new_plugin This PR includes a new plugin. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:community This issue/PR relates to code supported by the Ansible community.
Projects
None yet
Development

Successfully merging this pull request may close these issues.