Skip to content

Commit

Permalink
AWS S3 fixes for us-east-2.
Browse files Browse the repository at this point in the history
  • Loading branch information
jmchilton committed May 6, 2024
1 parent 594537d commit 18c1376
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 2 deletions.
24 changes: 22 additions & 2 deletions lib/galaxy/objectstore/s3.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ def parse_config_xml(config_xml):
multipart = string_as_bool(cn_xml.get("multipart", "True"))
is_secure = string_as_bool(cn_xml.get("is_secure", "True"))
conn_path = cn_xml.get("conn_path", "/")
region = cn_xml.get("region", None)

cache_dict = parse_caching_config_dict_from_xml(config_xml)

Expand Down Expand Up @@ -114,6 +115,7 @@ def parse_config_xml(config_xml):
"multipart": multipart,
"is_secure": is_secure,
"conn_path": conn_path,
"region": region,
},
"cache": cache_dict,
"extra_dirs": extra_dirs,
Expand Down Expand Up @@ -142,6 +144,7 @@ def _config_to_dict(self):
"multipart": self.multipart,
"is_secure": self.is_secure,
"conn_path": self.conn_path,
"region": self.region,
},
"cache": {
"size": self.cache_size,
Expand Down Expand Up @@ -185,6 +188,7 @@ def __init__(self, config, config_dict):
self.multipart = connection_dict.get("multipart", True)
self.is_secure = connection_dict.get("is_secure", True)
self.conn_path = connection_dict.get("conn_path", "/")
self.region = connection_dict.get("region", None)

self.cache_size = cache_dict.get("size") or self.config.object_store_cache_size
self.staging_path = cache_dict.get("path") or self.config.object_store_cache_path
Expand Down Expand Up @@ -228,7 +232,23 @@ def _configure_connection(self):
log.debug("Configuring S3 Connection")
# If access_key is empty use default credential chain
if self.access_key:
self.conn = S3Connection(self.access_key, self.secret_key)
if self.region:
# If specify a region we can infer a host and turn on SIGV4.
# https://stackoverflow.com/questions/26744712/s3-using-boto-and-sigv4-missing-host-parameter

# Turning on SIGV4 is needed for AWS regions created after 2014... from
# https://docs.aws.amazon.com/AmazonS3/latest/API/sig-v4-authenticating-requests.html:
#
# "Amazon S3 supports Signature Version 4, a protocol for authenticating inbound API requests to AWS services,
# in all AWS Regions. At this time, AWS Regions created before January 30, 2014 will continue to support the
# previous protocol, Signature Version 2. Any new Regions after January 30, 2014 will support only Signature
# Version 4 and therefore all requests to those Regions must be made with Signature Version 4."
os.environ["S3_USE_SIGV4"] = "True"
self.conn = S3Connection(self.access_key, self.secret_key, host=f"s3.{self.region}.amazonaws.com")
else:
# See notes above, this path through the code will not work for
# newer regions.
self.conn = S3Connection(self.access_key, self.secret_key)
else:
self.conn = S3Connection()

Expand Down Expand Up @@ -581,7 +601,7 @@ def _create(self, obj, **kwargs):

def _empty(self, obj, **kwargs):
if self._exists(obj, **kwargs):
return bool(self._size(obj, **kwargs) > 0)
return bool(self._size(obj, **kwargs) == 0)
else:
raise ObjectNotFound(f"objectstore.empty, object does not exist: {obj}, kwargs: {kwargs}")

Expand Down
5 changes: 5 additions & 0 deletions test/unit/objectstore/test_objectstore.py
Original file line number Diff line number Diff line change
Expand Up @@ -1518,6 +1518,9 @@ def test_real_azure_blob_store_in_hierarchical(tmp_path):
bucket:
name: ${bucket}
connection:
region: ${region}
extra_dirs:
- type: job_work
path: database/job_working_directory_azure
Expand All @@ -1529,11 +1532,13 @@ def test_real_azure_blob_store_in_hierarchical(tmp_path):
@skip_unless_environ("GALAXY_TEST_AWS_ACCESS_KEY")
@skip_unless_environ("GALAXY_TEST_AWS_SECRET_KEY")
@skip_unless_environ("GALAXY_TEST_AWS_BUCKET")
@skip_unless_environ("GALAXY_TEST_AWS_REGION")
def test_real_aws_s3_store(tmp_path):
template_vars = {
"access_key": os.environ["GALAXY_TEST_AWS_ACCESS_KEY"],
"secret_key": os.environ["GALAXY_TEST_AWS_SECRET_KEY"],
"bucket": os.environ["GALAXY_TEST_AWS_BUCKET"],
"region": os.environ["GALAXY_TEST_AWS_REGION"],
}
with TestConfig(AMAZON_S3_SIMPLE_TEMPLATE_TEST_CONFIG_YAML, template_vars=template_vars) as (_, object_store):
verify_caching_object_store_functionality(tmp_path, object_store)
Expand Down

0 comments on commit 18c1376

Please sign in to comment.