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.

Sign up for GitHub

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

Fix/ssm bucket auth #854

Closed
wants to merge 7 commits into from
Closed

Fix/ssm bucket auth #854

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b357a24
update how regions are determined
Jun 29, 2021
834c8f8
Add some integration tests
gillg Jan 8, 2022
aa47fb8
Add changelog
gillg Jan 8, 2022
5d4f1c5
Fix versioning
gillg Jan 8, 2022
6c7c9b0
Shorten test bucket name
gillg Jan 8, 2022
3f29fd3
Improve logic test
gillg Jan 9, 2022
4780af1
Improve documentation
gillg Jan 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelogs/fragments/854-ssm-bucket-region.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
minor_changes:
- aws_ssm connection plugin - added support for an independent bucket region (https://github.com/ansible-collections/community.aws/pull/854).
21 changes: 17 additions & 4 deletions plugins/connection/aws_ssm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@
description: The name of the S3 bucket used for file transfers.
vars:
- name: ansible_aws_ssm_bucket_name
bucket_region:
description: The region of the S3 bucket used for file transfers. By default inherit "region" or "us-east-1" if undefined.
vars:
- name: ansible_aws_ssm_bucket_region
version_added: 3.0.0
plugin:
description: This defines the location of the session-manager-plugin binary.
vars:
Expand Down Expand Up @@ -131,18 +136,22 @@
#plugin: aws_ec2
#regions:
# - us-east-1
# - eu-west-1
#hostnames:
# - instance-id
#filters:
# tag:SSMTag: ssmlinux
#compose:
# ansible_aws_ssm_instance_id: instance_id
# ansible_aws_ssm_region: location.region
# -----------------------
- name: install aws-cli
hosts: all
gather_facts: false
vars:
ansible_connection: aws_ssm
ansible_aws_ssm_bucket_name: nameofthebucket
ansible_aws_ssm_region: us-east-1
ansible_aws_ssm_bucket_region: eu-west-1
tasks:
- name: aws-cli
raw: yum install -y awscli
Expand All @@ -154,10 +163,14 @@
#plugin: aws_ec2
#regions:
# - us-east-1
# - eu-west-1
#hostnames:
# - instance-id
#filters:
# tag:SSMTag: ssmwindows
#compose:
# ansible_aws_ssm_instance_id: instance_id
# ansible_aws_ssm_region: location.region
# -----------------------
- name: Create a dir.
hosts: all
Expand All @@ -166,7 +179,7 @@
ansible_connection: aws_ssm
ansible_shell_type: powershell
ansible_aws_ssm_bucket_name: nameofthebucket
ansible_aws_ssm_region: us-east-1
ansible_aws_ssm_bucket_region: eu-west-1
tasks:
- name: Create the directory
win_file:
Expand All @@ -180,7 +193,7 @@
vars:
ansible_connection: aws_ssm
ansible_aws_ssm_bucket_name: nameofthebucket
ansible_aws_ssm_region: us-west-2
ansible_aws_ssm_bucket_region: us-west-2
ansible_aws_ssm_bucket_sse_mode: 'aws:kms'
ansible_aws_ssm_bucket_sse_kms_key_id: alias/kms-key-alias
tasks:
Expand Down Expand Up @@ -534,7 +547,7 @@ def _flush_stderr(self, subprocess):

def _get_url(self, client_method, bucket_name, out_path, http_method, profile_name, extra_args=None):
''' Generate URL for get_object / put_object '''
region_name = self.get_option('region') or 'us-east-1'
region_name = self.get_option('bucket_region') or self.get_option('region') or 'us-east-1'
client = self._get_boto_client('s3', region_name=region_name, profile_name=profile_name)
params = {'Bucket': bucket_name, 'Key': out_path}
if extra_args is not None:
Expand Down
3 changes: 2 additions & 1 deletion ...s/integration/targets/connection_aws_ssm/aws_ssm_integration_test_setup/defaults/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@ windows_ami_name: Windows_Server-2019-English-Full-Base-*
# see:
# - https://github.com/mattclay/aws-terminator/pull/181
# - https://github.com/ansible-collections/community.aws/pull/763
s3_bucket_name: ssm-encrypted-test-bucket
s3_bucket_name: "{{ tiny_prefix }}-ssm-encrypted-test-bucket"
Copy link
Contributor

@alinabuzachis alinabuzachis Jan 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@gillg Thank you for working on this. ssm-encrypted-test-bucket has been added in this PR #763 (comment) since It is an encrypted bucket requiring up to ~24hour to be created (we have to use a permanent one). At the moment, the integration tests of the aws_ssm connection plugin have been also disabled because of other issues. Please have a look at this PR #763 for a history.
@jillr can you confirm please?

Copy link
Author

Choose a reason for hiding this comment

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

24h to create a bucket 😨 !? I'm surprised but why not.
Don't you think it could be more revelent to create an unencrypted bucket by default, but add a resource policy to ensure "PUT" commands forces encryption ? Because if you enable encryption in bucket properties that means you object will be encrypted by default event if you don't ask for it. But it seems to don't be the use case of SSM plugin where we ask explicitely to encrypt new objects.

If there is no workaround what would be the next steps ? Do we assume to skip integration tests for this plugin for now ?
Just in case... I tested it localy in my context 😅

In the meantime I try to work on other PRs because I use this plugin for the first time and I prefer make it stable and close all pending PRs since almost 1yr.

s3_bucket_region: us-west-2
19 changes: 10 additions & 9 deletions tests/integration/targets/connection_aws_ssm/aws_ssm_integration_test_setup/tasks/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,11 @@
tags:
ansible-test: '{{ resource_prefix }}'

# - name: Create S3 bucket
# s3_bucket:
# name: "{{resource_prefix}}-aws-ssm-s3"
# register: s3_output
- name: Create S3 bucket
s3_bucket:
name: "{{ s3_bucket_name }}"
region: "{{ s3_bucket_region }}"
register: s3_output

- name: Create Inventory file
template:
Expand All @@ -138,11 +139,11 @@
src: ec2_windows_vars_to_delete.yml.j2
ignore_errors: yes

# - name: Create S3 vars_to_delete.yml
# template:
# dest: "{{playbook_dir}}/s3_vars_to_delete.yml"
# src: s3_vars_to_delete.yml.j2
# ignore_errors: yes
- name: Create S3 vars_to_delete.yml
template:
dest: "{{playbook_dir}}/s3_vars_to_delete.yml"
src: s3_vars_to_delete.yml.j2
ignore_errors: yes

- name: Create IAM Role vars_to_delete.yml
template:
Expand Down
1 change: 1 addition & 0 deletions ...connection_aws_ssm/aws_ssm_integration_test_setup/templates/inventory-combined.aws_ssm.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ aws_ssm_linux
[aws_ssm:vars]
ansible_connection=community.aws.aws_ssm
ansible_aws_ssm_bucket_name={{s3_bucket_name}}
ansible_aws_ssm_bucket_region={{s3_bucket_region}}
Copy link
Author

Choose a reason for hiding this comment

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

Most important part for integration test

ansible_aws_ssm_plugin=/usr/local/sessionmanagerplugin/bin/session-manager-plugin
ansible_python_interpreter=/usr/bin/env python
local_tmp=/tmp/ansible-local-
Expand Down
2 changes: 1 addition & 1 deletion ...gets/connection_aws_ssm/aws_ssm_integration_test_setup/templates/s3_vars_to_delete.yml.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
---
#bucket_name: {{s3_output.name}}
bucket_name: {{s3_output.name}}