aws_ssm - split S3 region/endpoint discovery into dedicated function (#1674) (#1677)

[PR #1674/8237ebb7 backport][stable-5] aws_ssm - split S3 region/endpoint discovery into dedicated function

This is a backport of PR #1674 as merged into main (8237ebb).
Depends-On: #1670
SUMMARY
fixes: #1616
Newer AWS regions don't generate valid presigned URLs unless you explicitly pass the endpoint_url for the region (see also boto/boto3#3015)
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
aws_ssm
ADDITIONAL INFORMATION

Reviewed-by: Markus Bergholz <[email protected]>

Author: patchback[bot]

changelogs/fragments/1616-aws_ssm-new-regions.yml

@@ -0,0 +1,2 @@
+bugfixes:
+- aws_ssm - fixes bug with presigned S3 URLs in post-2019 AWS regions (https://github.com/ansible-collections/community.aws/issues/1616).

plugins/connection/aws_ssm.py

@@ -350,6 +350,32 @@ def _vvv(self, message):
     def _vvvv(self, message):
         self._display(display.vvvv, message)
 
+    def _get_bucket_endpoint(self):
+        # Fetch the correct S3 endpoint for use with our bucket.
+        # If we don't explicitly set the endpoint then some commands will use the global
+        # endpoint and fail
+        # (new AWS regions and new buckets in a region other than the one we're running in)
+
+        region_name = self.get_option('region') or 'us-east-1'
+        profile_name = self.get_option('profile') or ''
+        self._vvvv("_get_bucket_endpoint: S3 (global)")
+        tmp_s3_client = self._get_boto_client(
+            's3', region_name=region_name, profile_name=profile_name,
+        )
+        # Fetch the location of the bucket so we can open a client against the 'right' endpoint
+        # This /should/ always work
+        bucket_location = tmp_s3_client.get_bucket_location(
+            Bucket=(self.get_option('bucket_name')),
+        )
+        bucket_region = bucket_location['LocationConstraint']
+        # Create another client for the region the bucket lives in, so we can nab the endpoint URL
+        self._vvvv(f"_get_bucket_endpoint: S3 (bucket region) - {bucket_region}")
+        s3_bucket_client = self._get_boto_client(
+            's3', region_name=bucket_region, profile_name=profile_name,
+        )
+
+        return s3_bucket_client.meta.endpoint_url, s3_bucket_client.meta.region_name
+
     def _init_clients(self):
         self._vvvv("INITIALIZE BOTO3 CLIENTS")
         profile_name = self.get_option('profile') or ''
@@ -358,20 +384,17 @@ def _init_clients(self):
         # The SSM Boto client, currently used to initiate and manage the session
         # Note: does not handle the actual SSM session traffic
         self._vvvv("SETUP BOTO3 CLIENTS: SSM")
-        ssm_client = self._get_boto_client('ssm', region_name=region_name, profile_name=profile_name)
+        ssm_client = self._get_boto_client(
+            'ssm', region_name=region_name, profile_name=profile_name,
+        )
         self._client = ssm_client
 
-        region_name = self.get_option('region') or 'us-east-1'
-        self._vvvv("SETUP BOTO3 CLIENTS: S3 (tmp)")
-        tmp_s3_client = self._get_boto_client('s3', region_name=region_name, profile_name=profile_name)
-        # Fetch the location of the bucket so we can open a client against the 'right' endpoint
-        bucket_location = tmp_s3_client.get_bucket_location(
-            Bucket=(self.get_option('bucket_name')),
+        s3_endpoint_url, s3_region_name = self._get_bucket_endpoint()
+        self._vvvv(f"SETUP BOTO3 CLIENTS: S3 {s3_endpoint_url}")
+        s3_bucket_client = self._get_boto_client(
+            's3', region_name=s3_region_name, endpoint_url=s3_endpoint_url, profile_name=profile_name,
         )
-        bucket_region = bucket_location['LocationConstraint']
-        # This is the S3 client we'll really be using
-        self._vvvv(f"SETUP BOTO3 CLIENTS: S3 - {bucket_region}")
-        s3_bucket_client = self._get_boto_client('s3', region_name=bucket_region, profile_name=profile_name)
+
         self._s3_client = s3_bucket_client
 
     def __init__(self, *args, **kwargs):
@@ -706,7 +729,7 @@ def _get_url(self, client_method, bucket_name, out_path, http_method, extra_args
             params.update(extra_args)
         return client.generate_presigned_url(client_method, Params=params, ExpiresIn=3600, HttpMethod=http_method)
 
-    def _get_boto_client(self, service, region_name=None, profile_name=None):
+    def _get_boto_client(self, service, region_name=None, profile_name=None, endpoint_url=None):
         ''' Gets a boto3 client based on the STS token '''
 
         aws_access_key_id = self.get_option('access_key_id')
@@ -734,6 +757,7 @@ def _get_boto_client(self, service, region_name=None, profile_name=None):
 
         client = session.client(
             service,
+            endpoint_url=endpoint_url,
             config=Config(
                 signature_version="s3v4",
                 s3={'addressing_style': self.get_option('s3_addressing_style')}

tests/integration/targets/connection_aws_ssm_cross_region/aliases

@@ -0,0 +1,4 @@
+time=10m
+
+cloud/aws
+connection_aws_ssm

tests/integration/targets/connection_aws_ssm_cross_region/aws_ssm_integration_test_setup.yml

@@ -0,0 +1,10 @@
+- hosts: localhost
+  roles:
+    - role: ../setup_connection_aws_ssm
+      vars:
+        target_os: fedora
+        s3_bucket_region: 'eu-central-1'
+        # Post 2019 regions behave differently from other regions
+        # they're worth testing but it's not possible in CI today.
+        #s3_bucket_region: 'eu-south-1'
+        test_suffix: crossregion

tests/integration/targets/connection_aws_ssm_cross_region/aws_ssm_integration_test_teardown.yml

@@ -0,0 +1,5 @@
+- hosts: localhost
+  tasks:
+    - include_role:
+        name: ../setup_connection_aws_ssm
+        tasks_from: cleanup.yml

tests/integration/targets/connection_aws_ssm_cross_region/meta/main.yml

@@ -0,0 +1,3 @@
+dependencies:
+  - connection
+  - setup_connection_aws_ssm

tests/integration/targets/connection_aws_ssm_cross_region/runme.sh

@@ -0,0 +1,31 @@
+#!/usr/bin/env bash
+
+PLAYBOOK_DIR=$(pwd)
+set -eux
+
+CMD_ARGS=("$@")
+
+# Destroy Environment
+cleanup() {
+
+    cd "${PLAYBOOK_DIR}"
+    ansible-playbook -c local aws_ssm_integration_test_teardown.yml "${CMD_ARGS[@]}"
+
+}
+
+trap "cleanup" EXIT
+
+# Setup Environment
+ansible-playbook -c local aws_ssm_integration_test_setup.yml "$@"
+
+# Export the AWS Keys
+set +x
+. ./aws-env-vars.sh
+set -x
+
+cd ../connection
+
+# Execute Integration tests
+INVENTORY="${PLAYBOOK_DIR}/ssm_inventory" ./test.sh \
+    -e target_hosts=aws_ssm \
+    "$@"