From 34873f5fe72ca244df63307b4a19efea42f2dac3 Mon Sep 17 00:00:00 2001 From: Joseph Snyder Date: Wed, 22 Dec 2021 10:15:49 -0500 Subject: [PATCH] Use consistent method of checking for presence of info in connection (#27694) Fixes #27652 Ensure that mirror's to_dict function returns a syaml_dict object for all code paths. Switch to using the .get function for accessing the potential information from the S3 mirror objects. If the key is not there, it will gracefully return None instead of failing with a KeyError Additionally, check that the connection object is a dictionary before trying to "get" from it. Add a test for the capturing of the new S3 information. --- lib/spack/spack/mirror.py | 26 ++++++++++++----------- lib/spack/spack/test/web.py | 23 ++++++++++++++++++++ lib/spack/spack/util/s3.py | 42 ++++++++++++++++++++++--------------- 3 files changed, 62 insertions(+), 29 deletions(-) diff --git a/lib/spack/spack/mirror.py b/lib/spack/spack/mirror.py index f6e5d38091..9b6fd5ba3c 100644 --- a/lib/spack/spack/mirror.py +++ b/lib/spack/spack/mirror.py @@ -90,7 +90,9 @@ def from_json(stream, name=None): def to_dict(self): if self._push_url is None: - return self._fetch_url + return syaml_dict([ + ('fetch', self._fetch_url), + ('push', self._fetch_url)]) else: return syaml_dict([ ('fetch', self._fetch_url), @@ -105,12 +107,12 @@ def from_dict(d, name=None): def display(self, max_len=0): if self._push_url is None: - _display_mirror_entry(max_len, self._name, self._fetch_url) + _display_mirror_entry(max_len, self._name, self.fetch_url) else: _display_mirror_entry( - max_len, self._name, self._fetch_url, "fetch") + max_len, self._name, self.fetch_url, "fetch") _display_mirror_entry( - max_len, self._name, self._push_url, "push") + max_len, self._name, self.push_url, "push") def __str__(self): name = self._name @@ -145,8 +147,8 @@ def name(self): def get_profile(self, url_type): if isinstance(self._fetch_url, dict): if url_type == "push": - return self._push_url['profile'] - return self._fetch_url['profile'] + return self._push_url.get('profile', None) + return self._fetch_url.get('profile', None) else: return None @@ -159,8 +161,8 @@ def set_profile(self, url_type, profile): def get_access_pair(self, url_type): if isinstance(self._fetch_url, dict): if url_type == "push": - return self._push_url['access_pair'] - return self._fetch_url['access_pair'] + return self._push_url.get('access_pair', None) + return self._fetch_url.get('access_pair', None) else: return None @@ -173,8 +175,8 @@ def set_access_pair(self, url_type, connection_tuple): def get_endpoint_url(self, url_type): if isinstance(self._fetch_url, dict): if url_type == "push": - return self._push_url['endpoint_url'] - return self._fetch_url['endpoint_url'] + return self._push_url.get('endpoint_url', None) + return self._fetch_url.get('endpoint_url', None) else: return None @@ -187,8 +189,8 @@ def set_endpoint_url(self, url_type, url): def get_access_token(self, url_type): if isinstance(self._fetch_url, dict): if url_type == "push": - return self._push_url['access_token'] - return self._fetch_url['access_token'] + return self._push_url.get('access_token', None) + return self._fetch_url.get('access_token', None) else: return None diff --git a/lib/spack/spack/test/web.py b/lib/spack/spack/test/web.py index 4ac0da162b..41aa1e6121 100644 --- a/lib/spack/spack/test/web.py +++ b/lib/spack/spack/test/web.py @@ -246,6 +246,29 @@ def get_object(self, Bucket=None, Key=None): raise self.ClientError +def test_gather_s3_information(monkeypatch, capfd): + mock_connection_data = {"access_token": "AAAAAAA", + "profile": "SPacKDeV", + "access_pair": ("SPA", "CK"), + "endpoint_url": "https://127.0.0.1:8888"} + + session_args, client_args = spack.util.s3.get_mirror_s3_connection_info(mock_connection_data) # noqa: E501 + + # Session args are used to create the S3 Session object + assert "aws_session_token" in session_args + assert session_args.get("aws_session_token") == "AAAAAAA" + assert "aws_access_key_id" in session_args + assert session_args.get("aws_access_key_id") == "SPA" + assert "aws_secret_access_key" in session_args + assert session_args.get("aws_secret_access_key") == "CK" + assert "profile_name" in session_args + assert session_args.get("profile_name") == "SPacKDeV" + + # In addition to the session object, use the client_args to create the s3 + # Client object + assert "endpoint_url" in client_args + + def test_remove_s3_url(monkeypatch, capfd): fake_s3_url = 's3://my-bucket/subdirectory/mirror' diff --git a/lib/spack/spack/util/s3.py b/lib/spack/spack/util/s3.py index c120b4d5c5..82d51eb72e 100644 --- a/lib/spack/spack/util/s3.py +++ b/lib/spack/spack/util/s3.py @@ -27,6 +27,30 @@ def _parse_s3_endpoint_url(endpoint_url): return endpoint_url +def get_mirror_s3_connection_info(connection): + s3_connection = {} + + s3_connection_is_dict = connection and isinstance(connection, dict) + if s3_connection_is_dict: + if connection.get("access_token"): + s3_connection["aws_session_token"] = connection["access_token"] + if connection.get("access_pair"): + s3_connection["aws_access_key_id"] = connection["access_pair"][0] + s3_connection["aws_secret_access_key"] = connection["access_pair"][1] + if connection.get("profile"): + s3_connection["profile_name"] = connection["profile"] + + s3_client_args = {"use_ssl": spack.config.get('config:verify_ssl')} + + endpoint_url = os.environ.get('S3_ENDPOINT_URL') + if endpoint_url: + s3_client_args['endpoint_url'] = _parse_s3_endpoint_url(endpoint_url) + elif s3_connection_is_dict and connection.get("endpoint_url"): + s3_client_args["endpoint_url"] = _parse_s3_endpoint_url(connection["endpoint_url"]) # noqa: E501 + + return (s3_connection, s3_client_args) + + def create_s3_session(url, connection={}): url = url_util.parse(url) if url.scheme != 's3': @@ -40,25 +64,9 @@ def create_s3_session(url, connection={}): from boto3 import Session from botocore.exceptions import ClientError - s3_connection = {} - - if connection: - if connection['access_token']: - s3_connection["aws_session_token"] = connection["access_token"] - if connection["access_pair"][0]: - s3_connection["aws_access_key_id"] = connection["access_pair"][0] - s3_connection["aws_secret_access_key"] = connection["access_pair"][1] - if connection["profile"]: - s3_connection["profile_name"] = connection["profile"] + s3_connection, s3_client_args = get_mirror_s3_connection_info(connection) session = Session(**s3_connection) - s3_client_args = {"use_ssl": spack.config.get('config:verify_ssl')} - - endpoint_url = os.environ.get('S3_ENDPOINT_URL') - if endpoint_url: - s3_client_args['endpoint_url'] = _parse_s3_endpoint_url(endpoint_url) - elif connection and 'endpoint_url' in connection: - s3_client_args["endpoint_url"] = _parse_s3_endpoint_url(connection["endpoint_url"]) # noqa: E501 # if no access credentials provided above, then access anonymously if not session.get_credentials(): from botocore import UNSIGNED