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

Fix detection of ssm connection bucket region #1428

Conversation

phene
Copy link
Contributor

@phene phene commented Aug 30, 2022

Fix detection of ssm connection bucket region by ensuring that the boto client is created normally and able to use supported credential sources

SUMMARY

PR #1176 introduced detection of an S3 bucket's region to handle cases where the bucket is in a different region than the SSM connection itself. This change did not use the preferred mechanism for creating client objects, which caused it to not have access to credentials from all supported sources. It also broke the ability to use this plugin in partitions other than aws. (e.g. aws-us-gov).

This change fixes this by building the bucket location client using _get_boto_client and the region for the connection to ensure it is both getting the proper credentials and starting in a region from the same partition as the client itself. From the default global region (or a hard-coded region), it will detect the bucket's region and continue S3 API calls using the bucket's own region.

Fixes bug introduced from #1176
Fixes #1413

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

aws_ssm connection plugin

@phene phene force-pushed the fix-bucket-region-detection branch from 929e956 to 509611a Compare August 30, 2022 00:22
@ansibullbot ansibullbot added bug This issue/PR relates to a bug community_review connection connection plugin needs_triage plugins plugin (any type) labels Aug 30, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

✔️ ansible-galaxy-importer SUCCESS in 7m 40s
✔️ build-ansible-collection SUCCESS in 13m 25s
✔️ ansible-test-sanity-docker-devel SUCCESS in 16m 23s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 16m 40s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 15m 28s
✔️ ansible-test-sanity-docker-stable-2.13 SUCCESS in 17m 25s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 11m 25s
✔️ ansible-test-units-community-aws-python39 SUCCESS in 13m 18s
✔️ ansible-test-splitter SUCCESS in 6m 41s
⚠️ 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

@phene phene force-pushed the fix-bucket-region-detection branch from 8221576 to ee2e080 Compare September 13, 2022 20:36
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

✔️ ansible-galaxy-importer SUCCESS in 5m 31s
✔️ build-ansible-collection SUCCESS in 5m 15s
✔️ ansible-test-sanity-docker-devel SUCCESS in 10m 29s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 9m 04s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 11m 14s
✔️ ansible-test-sanity-docker-stable-2.13 SUCCESS in 9m 58s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 6m 22s
✔️ ansible-test-units-community-aws-python39 SUCCESS in 5m 56s
✔️ ansible-test-splitter SUCCESS in 2m 28s
⚠️ 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

Geoffrey Hichborn added 2 commits September 15, 2022 09:22
…to client is created normally and able to use supported credential sources
@phene phene force-pushed the fix-bucket-region-detection branch from ee2e080 to 782752f Compare September 15, 2022 16:22
@softwarefactory-project-zuul
Copy link
Contributor

@phene
Copy link
Contributor Author

phene commented Sep 21, 2022

cc @jillr @markuman @s-hertel @tremble

@phene
Copy link
Contributor Author

phene commented Oct 17, 2022

Bump

@markuman markuman added backport-4 PR should be backported to the stable-4 branch backport-5 PR should be backported to the stable-5 branch labels Oct 18, 2022
Co-authored-by: Markus Bergholz <[email protected]>
@github-actions
Copy link

github-actions bot commented Oct 18, 2022

Docs Build 📝

Thank you for contribution!✨

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

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

✔️ ansible-galaxy-importer SUCCESS in 4m 11s
✔️ build-ansible-collection SUCCESS in 4m 54s
ansible-test-sanity-docker-devel FAILURE in 8m 04s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 9m 57s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 9m 27s
✔️ ansible-test-sanity-docker-stable-2.13 SUCCESS in 10m 15s
ansible-test-sanity-docker-stable-2.14 FAILURE in 8m 14s (non-voting)
✔️ ansible-test-units-amazon-aws-python36 SUCCESS in 6m 38s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 5m 47s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 6m 08s
✔️ ansible-test-splitter SUCCESS in 2m 36s
⚠️ 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
✔️ ansible-test-changelog SUCCESS in 2m 34s

@tremble tremble added the mergeit Merge the PR (SoftwareFactory) label Oct 18, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

✔️ ansible-galaxy-importer SUCCESS in 4m 12s
✔️ build-ansible-collection SUCCESS in 4m 55s
ansible-test-sanity-docker-devel FAILURE in 9m 02s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 9m 32s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 10m 32s
✔️ ansible-test-sanity-docker-stable-2.13 SUCCESS in 9m 32s
ansible-test-sanity-docker-stable-2.14 FAILURE in 8m 49s (non-voting)
✔️ ansible-test-units-amazon-aws-python36 SUCCESS in 6m 05s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 5m 47s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 5m 48s
✔️ ansible-test-splitter SUCCESS in 2m 34s
⚠️ 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
✔️ ansible-test-changelog SUCCESS in 2m 16s

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit fa58965 into ansible-collections:main Oct 18, 2022
@patchback
Copy link

patchback bot commented Oct 18, 2022

Backport to stable-4: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-4/fa58965fceb8613d734242e552313199892c96d1/pr-1428

Backported as #1563

🤖 @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 Oct 18, 2022
Fix detection of ssm connection bucket region

Fix detection of ssm connection bucket region by ensuring that the boto client is created normally and able to use supported credential sources
SUMMARY
PR #1176 introduced detection of an S3 bucket's region to handle cases where the bucket is in a different region than the SSM connection itself. This change did not use the preferred mechanism for creating client objects, which caused it to not have access to credentials from all supported sources. It also broke the ability to use this plugin in partitions other than aws. (e.g. aws-us-gov).
This change fixes this by building the bucket location client using _get_boto_client and the region for the connection to ensure it is both getting the proper credentials and starting in a region from the same partition as the client itself. From the default global region (or a hard-coded region), it will detect the bucket's region and continue S3 API calls using the bucket's own region.
Fixes bug introduced from #1176
Fixes #1413
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
aws_ssm connection plugin

Reviewed-by: Markus Bergholz <[email protected]>
Reviewed-by: Alina Buzachis <None>
Reviewed-by: Mark Chappell <None>
(cherry picked from commit fa58965)
@patchback
Copy link

patchback bot commented Oct 18, 2022

Backport to stable-5: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-5/fa58965fceb8613d734242e552313199892c96d1/pr-1428

Backported as #1564

🤖 @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 Oct 18, 2022
Fix detection of ssm connection bucket region

Fix detection of ssm connection bucket region by ensuring that the boto client is created normally and able to use supported credential sources
SUMMARY
PR #1176 introduced detection of an S3 bucket's region to handle cases where the bucket is in a different region than the SSM connection itself. This change did not use the preferred mechanism for creating client objects, which caused it to not have access to credentials from all supported sources. It also broke the ability to use this plugin in partitions other than aws. (e.g. aws-us-gov).
This change fixes this by building the bucket location client using _get_boto_client and the region for the connection to ensure it is both getting the proper credentials and starting in a region from the same partition as the client itself. From the default global region (or a hard-coded region), it will detect the bucket's region and continue S3 API calls using the bucket's own region.
Fixes bug introduced from #1176
Fixes #1413
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
aws_ssm connection plugin

Reviewed-by: Markus Bergholz <[email protected]>
Reviewed-by: Alina Buzachis <None>
Reviewed-by: Mark Chappell <None>
(cherry picked from commit fa58965)
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Nov 20, 2022
[PR #1428/fa58965f backport][stable-5] Fix detection of ssm connection bucket region

This is a backport of PR #1428 as merged into main (fa58965).
Fix detection of ssm connection bucket region by ensuring that the boto client is created normally and able to use supported credential sources
SUMMARY
PR #1176 introduced detection of an S3 bucket's region to handle cases where the bucket is in a different region than the SSM connection itself. This change did not use the preferred mechanism for creating client objects, which caused it to not have access to credentials from all supported sources. It also broke the ability to use this plugin in partitions other than aws. (e.g. aws-us-gov).
This change fixes this by building the bucket location client using _get_boto_client and the region for the connection to ensure it is both getting the proper credentials and starting in a region from the same partition as the client itself. From the default global region (or a hard-coded region), it will detect the bucket's region and continue S3 API calls using the bucket's own region.
Fixes bug introduced from #1176
Fixes #1413
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
aws_ssm connection plugin

Reviewed-by: Mark Chappell <None>
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Dec 2, 2022
[PR #1428/fa58965f backport][stable-4] Fix detection of ssm connection bucket region

This is a backport of PR #1428 as merged into main (fa58965).
Fix detection of ssm connection bucket region by ensuring that the boto client is created normally and able to use supported credential sources
SUMMARY
PR #1176 introduced detection of an S3 bucket's region to handle cases where the bucket is in a different region than the SSM connection itself. This change did not use the preferred mechanism for creating client objects, which caused it to not have access to credentials from all supported sources. It also broke the ability to use this plugin in partitions other than aws. (e.g. aws-us-gov).
This change fixes this by building the bucket location client using _get_boto_client and the region for the connection to ensure it is both getting the proper credentials and starting in a region from the same partition as the client itself. From the default global region (or a hard-coded region), it will detect the bucket's region and continue S3 API calls using the bucket's own region.
Fixes bug introduced from #1176
Fixes #1413
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
aws_ssm connection plugin

Reviewed-by: Mark Chappell <None>
@tremble tremble mentioned this pull request Jan 20, 2023
abikouo added a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-4 PR should be backported to the stable-4 branch backport-5 PR should be backported to the stable-5 branch bug This issue/PR relates to a bug community_review connection connection plugin has_issue mergeit Merge the PR (SoftwareFactory) needs_triage plugins plugin (any type)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ssm connection caught exception(Unable to locate credentials)
5 participants