pr feedback

cleanup

awscli/customizations/overridesslcommonname.py

@@ -11,83 +11,74 @@
 # ANY KIND, either express or implied. See the License for the specific
 # language governing permissions and limitations under the License.
 
-import os
 
-DEFAULT_DNS_SUFFIX = 'amazonaws.com'
-CHINA_DNS_SUFFIX = 'amazonaws.com.cn'
-GOV_DNS_SUFFIX = 'sc2s.sgov.gov'
-
-common_regions = [
-    "af-south-1",
-    "ap-east-1",
-    "ap-northeast-1",
-    "ap-northeast-2",
-    "ap-northeast-3",
-    "ap-south-1",
-    "ap-southeast-1",
-    "ap-southeast-2",
-    "ap-southeast-3",
-    "ca-central-1",
-    "eu-north-1",
-    "eu-south-1",
-    "eu-west-1",
-    "eu-west-2",
-    "eu-west-3",
-    "fips-us-east-1",
-    "fips-us-east-2",
-    "fips-us-west-1",
-    "fips-us-west-2",
-    "me-south-1",
-    "sa-east-1",
-    "us-east-2",
-    "us-west-1",
-    "us-west-2",
-]
-
-common_regions_with_dns_suffix = {
-    region: DEFAULT_DNS_SUFFIX for region in common_regions
-}
-
-sslCommonNameServiceRegions = {
+ssl_common_names = {
     "sqs": {
-        "us-east-1": DEFAULT_DNS_SUFFIX,
-        "eu-central-1": DEFAULT_DNS_SUFFIX,
-        "cn-north-1": CHINA_DNS_SUFFIX,
-        "cn-northwest-1": CHINA_DNS_SUFFIX,
-        "us-gov-west-1": DEFAULT_DNS_SUFFIX,
-        "us-isob-east-1": GOV_DNS_SUFFIX,
-        **common_regions_with_dns_suffix,
+        "af-south-1": "af-south-1.queue.amazonaws.com",
+        "ap-east-1": "ap-east-1.queue.amazonaws.com",
+        "ap-northeast-1": "ap-northeast-1.queue.amazonaws.com",
+        "ap-northeast-2": "ap-northeast-2.queue.amazonaws.com",
+        "ap-northeast-3": "ap-northeast-3.queue.amazonaws.com",
+        "ap-south-1": "ap-south-1.queue.amazonaws.com",
+        "ap-southeast-1": "ap-southeast-1.queue.amazonaws.com",
+        "ap-southeast-2": "ap-southeast-2.queue.amazonaws.com",
+        "ap-southeast-3": "ap-southeast-3.queue.amazonaws.com",
+        "ca-central-1": "ca-central-1.queue.amazonaws.com",
+        "eu-central-1": "eu-central-1.queue.amazonaws.com",
+        "eu-north-1": "eu-north-1.queue.amazonaws.com",
+        "eu-south-1": "eu-south-1.queue.amazonaws.com",
+        "eu-west-1": "eu-west-1.queue.amazonaws.com",
+        "eu-west-2": "eu-west-2.queue.amazonaws.com",
+        "eu-west-3": "eu-west-3.queue.amazonaws.com",
+        "me-south-1": "me-south-1.queue.amazonaws.com",
+        "sa-east-1": "sa-east-1.queue.amazonaws.com",
+        "us-east-1": "queue.amazonaws.com",
+        "us-east-2": "us-east-2.queue.amazonaws.com",
+        "us-west-1": "us-west-1.queue.amazonaws.com",
+        "us-west-2": "us-west-2.queue.amazonaws.com",
+        "cn-north-1": "cn-north-1.queue.amazonaws.com.cn",
+        "cn-northwest-1": "cn-northwest-1.queue.amazonaws.com.cn",
+        "us-gov-west-1": "us-gov-west-1.queue.amazonaws.com",
+        "us-isob-east-1": "us-isob-east-1.queue.sc2s.sgov.gov",
     },
     "emr": {
-        "fips-ca-central-1": DEFAULT_DNS_SUFFIX,
-        **common_regions_with_dns_suffix,
-    }
+        "af-south-1": "af-south-1.elasticmapreduce.amazonaws.com",
+        "ap-east-1": "ap-east-1.elasticmapreduce.amazonaws.com",
+        "ap-northeast-1": "ap-northeast-1.elasticmapreduce.amazonaws.com",
+        "ap-northeast-2": "ap-northeast-2.elasticmapreduce.amazonaws.com",
+        "ap-northeast-3": "ap-northeast-3.elasticmapreduce.amazonaws.com",
+        "ap-south-1": "ap-south-1.elasticmapreduce.amazonaws.com",
+        "ap-southeast-1": "ap-southeast-1.elasticmapreduce.amazonaws.com",
+        "ap-southeast-2": "ap-southeast-2.elasticmapreduce.amazonaws.com",
+        "ap-southeast-3": "ap-southeast-3.elasticmapreduce.amazonaws.com",
+        "ca-central-1": "ca-central-1.elasticmapreduce.amazonaws.com",
+        "eu-north-1": "eu-north-1.elasticmapreduce.amazonaws.com",
+        "eu-south-1": "eu-south-1.elasticmapreduce.amazonaws.com",
+        "eu-west-1": "eu-west-1.elasticmapreduce.amazonaws.com",
+        "eu-west-2": "eu-west-2.elasticmapreduce.amazonaws.com",
+        "eu-west-3": "eu-west-3.elasticmapreduce.amazonaws.com",
+        "me-south-1": "me-south-1.elasticmapreduce.amazonaws.com",
+        "sa-east-1": "sa-east-1.elasticmapreduce.amazonaws.com",
+        "us-east-2": "us-east-2.elasticmapreduce.amazonaws.com",
+        "us-west-1": "us-west-1.elasticmapreduce.amazonaws.com",
+        "us-west-2": "us-west-2.elasticmapreduce.amazonaws.com",
+    },
+    "rds": {
+        "us-east-1": "rds.amazonaws.com",
+    },
 }
 
 
 def register_override_ssl_common_name(cli):
-    cli.register_last(
-        'before-building-argument-table-parser',
-        update_endpoint_url
-    )
+    cli.register_last("before-building-argument-table-parser", update_endpoint_url)
+
 
 def update_endpoint_url(session, parsed_globals, **kwargs):
     service = parsed_globals.command
-    service_regions = sslCommonNameServiceRegions.get(service)
+    endpoints = ssl_common_names.get(service)
     # only change url if user has not overrided already themselves
-    if (
-        service_regions is not None
-        and parsed_globals.endpoint_url is None
-    ):
-        region = parsed_globals.region
-        # only resolve region if user has not provided it in the command
-        if region is None:
-            region = (
-                session.get_config_variable('region')
-                or os.environ.get('AWS_DEFAULT_REGION')
-            )
-        dnsSuffix = service_regions.get(region)
-        if dnsSuffix is not None:
-            service_data = session.get_service_data(service)
-            endpointPrefix = service_data['metadata']['endpointPrefix']
-            parsed_globals.endpoint_url = f"https://{endpointPrefix}.{region}.{dnsSuffix}"
+    if endpoints is not None and parsed_globals.endpoint_url is None:
+        region = session.get_config_variable("region")
+        endpoint_url = endpoints.get(region)
+        if endpoint_url is not None:
+            parsed_globals.endpoint_url = f"https://{endpoint_url}"

awscli/customizations/rds.py

@@ -96,8 +96,10 @@ class GenerateDBAuthTokenCommand(BasicCommand):
 
     def _run_main(self, parsed_args, parsed_globals):
         rds = self._session.create_client(
-            'rds', parsed_globals.region, parsed_globals.endpoint_url,
-            parsed_globals.verify_ssl
+            'rds',
+            region_name=parsed_globals.region,
+            endpoint_url=parsed_globals.endpoint_url,
+            verify=parsed_globals.verify_ssl
         )
         token = rds.generate_db_auth_token(
             DBHostname=parsed_args.hostname,

tests/functional/test_override_ssl_common_name.py

@@ -0,0 +1,52 @@
+# Copyright 2022 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"). You
+# may not use this file except in compliance with the License. A copy of
+# the License is located at
+#
+#     http://aws.amazon.com/apache2.0/
+#
+# or in the "license" file accompanying this file. This file is
+# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF
+# ANY KIND, either express or implied. See the License for the specific
+# language governing permissions and limitations under the License.
+
+from awscli.testutils import mock, create_clidriver
+from awscli.customizations.overridesslcommonname import ssl_common_names
+
+import pytest
+
+
+
+@pytest.fixture
+def driver():
+    return create_clidriver()
+
+
+@pytest.fixture
+def invoke():
+    patch = mock.patch("awscli.clidriver.CLIOperationCaller.invoke", return_value=0)
+    return patch.start()
+
+
+def common_name_test_cases():
+
+    service_ops = {
+        "sqs": "list-queues",
+        "emr": "list-clusters",
+        "rds": "describe-db-clusters",
+    }
+    for service in ssl_common_names:
+        for region in ssl_common_names[service]:
+            yield (service, service_ops[service], region)
+
+
+@pytest.mark.parametrize("service,operation,region", common_name_test_cases())
+@mock.patch("awscli.clidriver.CLIOperationCaller.invoke", return_value=0)
+def test_set_endpoint_url_arg(invoke, driver, service, operation, region):
+    driver.main(f"{service} {operation} --region {region}".split())
+    formatted_op = "".join([part.lower() for part in operation.split("-")])
+    call_args = invoke.call_args[0]
+    assert call_args[0] == service
+    assert call_args[1].lower() == formatted_op
+    assert call_args[3].endpoint_url == f"https://{ssl_common_names[service][region]}"

tests/integration/customizations/test_override_ssl_common_name.py

@@ -0,0 +1,72 @@
+# Copyright 2022 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"). You
+# may not use this file except in compliance with the License. A copy of
+# the License is located at
+#
+#     http://aws.amazon.com/apache2.0/
+#
+# or in the "license" file accompanying this file. This file is
+# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF
+# ANY KIND, either express or implied. See the License for the specific
+# language governing permissions and limitations under the License.
+
+from awscli.testutils import aws
+from awscli.customizations.overridesslcommonname import ssl_common_names
+
+import pytest
+
+
+def common_name_test_cases():
+
+    regions_to_exclude = [
+        "af-south-1",
+        "ap-east-1",
+        "ap-southeast-3",
+        "eu-south-1",
+        "me-south-1",
+        "cn-north-1",
+        "cn-northwest-1",
+        "us-gov-west-1",
+        "us-isob-east-1",
+    ]
+
+    service_ops = {
+        "sqs": {
+            "op": "create-queue --queue-name test",
+            "return_key": "QueueUrl",
+        },
+        "emr": {"op": "list-clusters", "return_key": "Clusters"},
+        "rds": {
+            "op": "describe-db-clusters",
+            "return_key": "DBClusters",
+        },
+    }
+    for service in ssl_common_names:
+        for region in ssl_common_names[service]:
+            if region not in regions_to_exclude:
+                yield (
+                    service,
+                    service_ops[service]["op"],
+                    service_ops[service]["return_key"],
+                    region,
+                )
+
+
+def cleanup(service, json, region):
+    if service == "sqs":
+        p = aws(
+            f"{service} delete-queue --queue-url {json['QueueUrl']} --region {region}"
+        )
+        assert p.rc == 0
+
+
+@pytest.mark.parametrize(
+    "service,operation,return_key,region", common_name_test_cases()
+)
+def test_update_endpoint_url(service, operation, return_key, region):
+
+    p = aws(f"{service} {operation} --region {region}")
+    assert p.rc == 0
+    assert return_key in p.json
+    cleanup(service, p.json, region)

tests/unit/customizations/test_overridesslcommonname.py

@@ -10,65 +10,62 @@
 # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF
 # ANY KIND, either express or implied. See the License for the specific
 # language governing permissions and limitations under the License.
-from awscli.testutils import mock, unittest
-from awscli.testutils import BaseAWSCommandParamsTest
-from awscli.customizations.overridesslcommonname import update_endpoint_url
 
+from awscli.testutils import mock, create_clidriver
+from awscli.customizations.overridesslcommonname import (
+    update_endpoint_url,
+    ssl_common_names,
+)
 
-class TestOverrideSslCommonName(BaseAWSCommandParamsTest):
-    prefix = 'sqs list-queues'
+import pytest
 
-    def setUp(self):
-        super().setUp()
-        self.parsed_globals = mock.Mock()
-        self.parsed_globals.endpoint_url = None
-        self.parsed_globals.region = None
-        self.parsed_globals.command = 'sqs'
 
-    def test_update_endpoint_url(self):
-        update_endpoint_url(self.driver.session, self.parsed_globals)
-        self.assertEqual(
-            self.parsed_globals.endpoint_url,
-            'https://sqs.us-east-1.amazonaws.com'
-        )
+test_cases = [
+    (service, region)
+    for service, regions in ssl_common_names.items()
+    for region in regions
+]
 
-    def test_dont_modify_provided_region_url(self):
-        self.parsed_globals.endpoint_url = 'https://test.com'
-        self.parsed_globals.region = 'us-west-1'
-        update_endpoint_url(self.driver.session, self.parsed_globals)
-        self.assertEqual(self.parsed_globals.endpoint_url, 'https://test.com')
-        self.assertEqual(self.parsed_globals.region, 'us-west-1')
 
-    def test_other_dns_suffixes(self):
-        self.parsed_globals.region = 'cn-north-1'
-        update_endpoint_url(self.driver.session, self.parsed_globals)
-        self.assertEqual(self.parsed_globals.endpoint_url, 'https://sqs.cn-north-1.amazonaws.com.cn')
+@pytest.fixture
+def parsed_globals():
+    pg = mock.Mock()
+    pg.endpoint_url = None
+    pg.region = None
+    return pg
 
-        self.parsed_globals.endpoint_url = None
-        self.parsed_globals.region = 'us-isob-east-1'
-        update_endpoint_url(self.driver.session, self.parsed_globals)
-        self.assertEqual(self.parsed_globals.endpoint_url, 'https://sqs.us-isob-east-1.sc2s.sgov.gov')
 
-    def test_url_modified_from_event(self):
-        self.driver.session.emit(
-            'before-building-argument-table-parser.sqs',
-            args=[],
-            session=self.driver.session,
-            argument_table=mock.Mock(),
-            parsed_globals=self.parsed_globals
-        )
-        self.assertEqual(
-            self.parsed_globals.endpoint_url,
-            'https://sqs.us-east-1.amazonaws.com'
-        )
+@pytest.fixture
+def session():
+    driver = create_clidriver()
+    return driver.session
 
-    @mock.patch('awscli.clidriver.CLIOperationCaller.invoke', return_value=0)
-    def test_set_endpoint_url_arg(self, invoke):
-        self.run_cmd(self.prefix.split())
-        call_args = invoke.call_args[0]
-        self.assertEqual(call_args[0], 'sqs')
-        self.assertEqual(call_args[1], 'ListQueues')
-        self.assertEqual(call_args[3].endpoint_url, 'https://sqs.us-east-1.amazonaws.com')
 
-if __name__ == "__main__":
-    unittest.main()
+@pytest.mark.parametrize("service,region", test_cases)
+def test_update_endpoint_url(parsed_globals, session, service, region):
+    parsed_globals.command = service
+    session.set_config_variable("region", region)
+    update_endpoint_url(session, parsed_globals)
+    assert parsed_globals.endpoint_url == f"https://{ssl_common_names[service][region]}"
+
+
+@pytest.mark.parametrize("service,region", test_cases)
+def test_url_modified_from_event(parsed_globals, session, service, region):
+    assert parsed_globals.endpoint_url is None
+    parsed_globals.command = service
+    session.set_config_variable("region", region)
+    session.emit(
+        "before-building-argument-table-parser.sqs",
+        args=[],
+        session=session,
+        argument_table=mock.Mock(),
+        parsed_globals=parsed_globals,
+    )
+    assert parsed_globals.endpoint_url == f"https://{ssl_common_names[service][region]}"
+
+
+def test_dont_modify_provided_url(parsed_globals, session):
+    parsed_globals.endpoint_url = "http://test.com"
+    parsed_globals.command = "sqs"
+    update_endpoint_url(session, parsed_globals)
+    assert parsed_globals.endpoint_url == "http://test.com"