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

Addressing Style S3 (ssm_connection) - choose between path, virtual or auto. #1633

Merged
merged 6 commits into from
Jan 20, 2023

Conversation

adsanz
Copy link
Contributor

@adsanz adsanz commented Dec 30, 2022

SUMMARY

Added the chance of setting up the addressing style for S3 URLs, this fixes #637

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

aws_ssm

ADDITIONAL INFORMATION

I came across this issue when I was trying to connect and execute tasks on a fresh AWS set-up (new S3 bucket + EC2), I'm using the latest (5.1.0) release and Ansible 5.10 (as per pip show).

The issue itself is the same as #637 and the last comment is the fix, there's another PR addressing this issue #786 but is quiet since May

Also AWS seems to be deprecating PATH addressing style: https://docs.aws.amazon.com/AmazonS3/latest/userguide/VirtualHosting.html#path-style-access

Thanks to timburnet-systematica and james-masson for their initial work on this.

@adsanz
Copy link
Contributor Author

adsanz commented Dec 30, 2022

Note for clarity: Commit made from my work acc PR made from personal acc

@github-actions
Copy link

github-actions bot commented Dec 30, 2022

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and your docs changes will be incorporated when they are next published.

@adsanz
Copy link
Contributor Author

adsanz commented Dec 30, 2022

Also adding how I tested this change:

> cp plugins/connection/aws_ssm.py /home/adsanz/.ansible/collections/ansible_collections/community/aws/plugins/connection/aws_ssm.py
> AWS_PROFILE=cc-int ansible-playbook -i ../env/int/ansible deploy.yml
PLAY [front] ******************************************************************************************************************************************************************************************************

TASK [Just a test hello there] ************************************************************************************************************************************************************************************
fatal: [i-xxxxxxxxxxxx]: FAILED! => {"changed": false, "module_stderr": "", "module_stdout": "  File \"/home/ssm-user/.ansible/tmp/ansible-tmp-1672409003.9969342-21608-195798520622054/AnsiballZ_command.py\", line 1\r\r\n    <?xml version=\"1.0\" encoding=\"UTF-8\"?>\r\r\n    ^\r\r\nSyntaxError: invalid syntax\r\r", "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error", "rc": 1}

PLAY RECAP ********************************************************************************************************************************************************************************************************
i-xxxxxxxxxxxx        : ok=0    changed=0    unreachable=0    failed=1    skipped=0    rescued=0    ignored=0   

> AWS_PROFILE=cc-int ansible-playbook -i ../env/int/ansible deploy.yml
> vim deploy.yml
> AWS_PROFILE=cc-int ansible-playbook -i ../env/int/ansible deploy.yml
PLAY [front] ******************************************************************************************************************************************************************************************************

TASK [Just a test hello there] ************************************************************************************************************************************************************************************
changed: [i-xxxxxxxxxxxx]

PLAY RECAP ********************************************************************************************************************************************************************************************************
i-xxxxxxxxxxxx        : ok=1    changed=1    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0  

The playbook is fairly simple:

- hosts: front
  become: true
  vars:
    ansible_connection: aws_ssm
    ansible_aws_ssm_bucket_name: my-newly-created-bucket
    ansible_aws_ssm_region: eu-west-1
    # edited this part on "vim deploy.yml" and added this variable
    ansible_aws_ssm_s3_addressing_style: virtual
  gather_facts: no
  tasks:
    - name: Just a test hello there
      ansible.builtin.shell:
        cmd: whoami

@ansibullbot ansibullbot added community_review connection connection plugin feature This issue/PR relates to a feature request needs_triage new_contributor Help guide this first time contributor plugins plugin (any type) labels Dec 30, 2022
@softwarefactory-project-zuul

This comment was marked as resolved.

@softwarefactory-project-zuul

This comment was marked as resolved.

@softwarefactory-project-zuul

This comment was marked as resolved.

@adsanz
Copy link
Contributor Author

adsanz commented Jan 2, 2023

There's a broken check: https://ansible.softwarefactory-project.io/zuul/build/b47386cb65304135b3cb84f0978a88d5 but test are passing now, let me know if anything else is needed!

@tremble
Copy link
Contributor

tremble commented Jan 2, 2023

recheck (this forces a retry on the hopefully no-longer-broken test)

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

✔️ ansible-galaxy-importer SUCCESS in 4m 01s
✔️ build-ansible-collection SUCCESS in 5m 01s
✔️ ansible-test-sanity-docker-devel SUCCESS in 8m 50s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 9m 14s (non-voting)
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 9m 59s
✔️ ansible-test-sanity-docker-stable-2.13 SUCCESS in 9m 16s
✔️ ansible-test-sanity-docker-stable-2.14 SUCCESS in 12m 23s
✔️ ansible-test-units-amazon-aws-python36 SUCCESS in 6m 36s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 6m 16s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 5m 44s
✔️ ansible-test-units-amazon-aws-python310 SUCCESS in 6m 04s
✔️ ansible-test-changelog SUCCESS in 2m 27s
✔️ ansible-test-splitter SUCCESS in 3m 37s
⚠️ integration-community.aws-1 SKIPPED
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED
⚠️ integration-community.aws-14 SKIPPED
⚠️ integration-community.aws-15 SKIPPED
⚠️ integration-community.aws-16 SKIPPED
⚠️ integration-community.aws-17 SKIPPED
⚠️ integration-community.aws-18 SKIPPED
⚠️ integration-community.aws-19 SKIPPED
⚠️ integration-community.aws-20 SKIPPED
⚠️ integration-community.aws-21 SKIPPED
⚠️ integration-community.aws-22 SKIPPED

@adsanz
Copy link
Contributor Author

adsanz commented Jan 9, 2023

I'll add this for the record, let me know if should I add this somewhere else, but aliases still don't work, I just tested it and works with the bucket name, but not with the alias. Not sure why

@tremble tremble added the backport-5 PR should be backported to the stable-5 branch label Jan 20, 2023
@softwarefactory-project-zuul

This comment was marked as resolved.

@tremble tremble added the mergeit Merge the PR (SoftwareFactory) label Jan 20, 2023
@softwarefactory-project-zuul

This comment was marked as resolved.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

✔️ ansible-galaxy-importer SUCCESS in 4m 17s
✔️ build-ansible-collection SUCCESS in 5m 12s
✔️ ansible-test-sanity-docker-devel SUCCESS in 10m 46s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 13m 47s (non-voting)
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 10m 28s
✔️ ansible-test-sanity-docker-stable-2.13 SUCCESS in 9m 18s
✔️ ansible-test-sanity-docker-stable-2.14 SUCCESS in 11m 28s
✔️ ansible-test-units-amazon-aws-python36 SUCCESS in 6m 32s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 7m 50s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 7m 08s
✔️ ansible-test-units-amazon-aws-python310 SUCCESS in 6m 47s
✔️ ansible-test-changelog SUCCESS in 2m 41s
✔️ ansible-test-splitter SUCCESS in 2m 49s
integration-community.aws-1 FAILURE in 10m 47s
integration-community.aws-2 FAILURE in 11m 33s
✔️ integration-community.aws-3 SUCCESS in 12m 32s
✔️ integration-community.aws-4 SUCCESS in 6m 22s
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED
⚠️ integration-community.aws-14 SKIPPED
⚠️ integration-community.aws-15 SKIPPED
⚠️ integration-community.aws-16 SKIPPED
⚠️ integration-community.aws-17 SKIPPED
⚠️ integration-community.aws-18 SKIPPED
⚠️ integration-community.aws-19 SKIPPED
⚠️ integration-community.aws-20 SKIPPED
⚠️ integration-community.aws-21 SKIPPED
⚠️ integration-community.aws-22 SKIPPED

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

✔️ ansible-galaxy-importer SUCCESS in 3m 54s
✔️ build-ansible-collection SUCCESS in 5m 18s
✔️ ansible-test-sanity-docker-devel SUCCESS in 10m 42s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 10m 11s (non-voting)
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 9m 18s
✔️ ansible-test-sanity-docker-stable-2.13 SUCCESS in 9m 52s
✔️ ansible-test-sanity-docker-stable-2.14 SUCCESS in 9m 22s
✔️ ansible-test-units-amazon-aws-python36 SUCCESS in 7m 05s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 5m 51s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 5m 35s
✔️ ansible-test-units-amazon-aws-python310 SUCCESS in 6m 59s
✔️ ansible-test-changelog SUCCESS in 2m 32s
✔️ ansible-test-splitter SUCCESS in 2m 59s
✔️ integration-community.aws-1 SUCCESS in 11m 45s
✔️ integration-community.aws-2 SUCCESS in 11m 48s
✔️ integration-community.aws-3 SUCCESS in 11m 55s
✔️ integration-community.aws-4 SUCCESS in 5m 50s
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED
⚠️ integration-community.aws-14 SKIPPED
⚠️ integration-community.aws-15 SKIPPED
⚠️ integration-community.aws-16 SKIPPED
⚠️ integration-community.aws-17 SKIPPED
⚠️ integration-community.aws-18 SKIPPED
⚠️ integration-community.aws-19 SKIPPED
⚠️ integration-community.aws-20 SKIPPED
⚠️ integration-community.aws-21 SKIPPED
⚠️ integration-community.aws-22 SKIPPED

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

✔️ ansible-galaxy-importer SUCCESS in 4m 01s
✔️ build-ansible-collection SUCCESS in 6m 08s
✔️ ansible-test-sanity-docker-devel SUCCESS in 9m 42s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 10m 12s (non-voting)
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 10m 11s
✔️ ansible-test-sanity-docker-stable-2.13 SUCCESS in 9m 57s
✔️ ansible-test-sanity-docker-stable-2.14 SUCCESS in 9m 03s
✔️ ansible-test-units-amazon-aws-python36 SUCCESS in 6m 10s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 5m 53s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 5m 36s
✔️ ansible-test-units-amazon-aws-python310 SUCCESS in 6m 11s
✔️ ansible-test-changelog SUCCESS in 2m 15s
✔️ ansible-test-splitter SUCCESS in 2m 44s
✔️ integration-community.aws-1 SUCCESS in 11m 31s
✔️ integration-community.aws-2 SUCCESS in 11m 21s
✔️ integration-community.aws-3 SUCCESS in 13m 33s
✔️ integration-community.aws-4 SUCCESS in 5m 24s
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED
⚠️ integration-community.aws-14 SKIPPED
⚠️ integration-community.aws-15 SKIPPED
⚠️ integration-community.aws-16 SKIPPED
⚠️ integration-community.aws-17 SKIPPED
⚠️ integration-community.aws-18 SKIPPED
⚠️ integration-community.aws-19 SKIPPED
⚠️ integration-community.aws-20 SKIPPED
⚠️ integration-community.aws-21 SKIPPED
⚠️ integration-community.aws-22 SKIPPED

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 8503d80 into ansible-collections:main Jan 20, 2023
@patchback
Copy link

patchback bot commented Jan 20, 2023

Backport to stable-5: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-5/8503d80d2c8477bd1601d419bb73e51bb691498f/pr-1633

Backported as #1669

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Jan 20, 2023
…r auto. (#1633)

Addressing Style S3 (ssm_connection) - choose between path, virtual or auto.

SUMMARY
Added the chance of setting up the addressing style for S3 URLs, this fixes #637
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
aws_ssm
ADDITIONAL INFORMATION
I came across this issue when I was trying to connect and execute tasks on a fresh AWS set-up (new S3 bucket + EC2), I'm using the latest (5.1.0) release and Ansible 5.10 (as per pip show).
The issue itself is the same as #637 and the last comment is the fix, there's another PR addressing this issue #786 but is quiet since May
Also AWS seems to be deprecating PATH addressing style: https://docs.aws.amazon.com/AmazonS3/latest/userguide/VirtualHosting.html#path-style-access
Thanks to timburnet-systematica and james-masson for their initial work on this.

Reviewed-by: Mark Chappell <None>
(cherry picked from commit 8503d80)
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Jan 20, 2023
…r auto. (#1633) (#1669)

[PR #1633/8503d80d backport][stable-5] Addressing Style S3 (ssm_connection) - choose between path, virtual or auto.

This is a backport of PR #1633 as merged into main (8503d80).
SUMMARY
Added the chance of setting up the addressing style for S3 URLs, this fixes #637
ISSUE TYPE


Feature Pull Request

COMPONENT NAME
aws_ssm
ADDITIONAL INFORMATION
I came across this issue when I was trying to connect and execute tasks on a fresh AWS set-up (new S3 bucket + EC2), I'm using the latest (5.1.0) release and Ansible 5.10 (as per pip show).
The issue itself is the same as #637 and the last comment is the fix, there's another PR addressing this issue #786 but is quiet since May
Also AWS seems to be deprecating PATH addressing style: https://docs.aws.amazon.com/AmazonS3/latest/userguide/VirtualHosting.html#path-style-access
Thanks to timburnet-systematica and james-masson for their initial work on this.

Reviewed-by: Mark Chappell <None>
abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
…-collections#1633)

Document and validate backup_selection conditions suboptions

SUMMARY
Adds documentation and validation for all conditions suboptions in backup_selection module. Fixes ansible-collections#1613
Additionally fixes a bug in module_utils.backup that caused an empty list to be returned from get_selection_details() when multiple backup selections exist for a given backup plan.
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
backup_selection
module_utils.backup
ADDITIONAL INFORMATION
See ansible-collections#1613 for detailed description of related issue.

Reviewed-by: Jill R
Reviewed-by: Alina Buzachis
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-5 PR should be backported to the stable-5 branch community_review connection connection plugin feature This issue/PR relates to a feature request integration tests/integration mergeit Merge the PR (SoftwareFactory) needs_triage new_contributor Help guide this first time contributor plugins plugin (any type) tests tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

aws_ssm connection plugin: S3 Signed Url invalid for newly created S3 Bucket
4 participants