From 61f22621d6924cb8043ef58738380237c21f0e3e Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 6 Oct 2017 13:54:49 -0400 Subject: [PATCH] Fix unbound variable in new cloud fronted storage Fixes https://sentry.io/coreos/backend-staging/issues/363196446/ --- storage/cloud.py | 12 ++++++++---- storage/test/test_cloudfront.py | 14 ++++++++++++++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/storage/cloud.py b/storage/cloud.py index 5116d2aae..4acebe5e3 100644 --- a/storage/cloud.py +++ b/storage/cloud.py @@ -611,7 +611,9 @@ class CloudFrontedS3Storage(S3Storage): self.cloudfront_key_id = cloudfront_key_id self.cloudfront_privatekey = self._load_private_key(cloudfront_privatekey_filename) - def get_direct_download_url(self, path, request_ip=None, expires_in=60, requires_cors=False, head=False): + def get_direct_download_url(self, path, request_ip=None, expires_in=60, requires_cors=False, + head=False): + resolved_ip_info = None logger.debug('Got direct download request for path "%s" with IP "%s"', path, request_ip) if request_ip is not None: # Lookup the IP address in our resolution table and determine whether it is under AWS. If it is, @@ -619,14 +621,16 @@ class CloudFrontedS3Storage(S3Storage): resolved_ip_info = self._context.ip_resolver.resolve_ip(request_ip) logger.debug('Resolved IP information for IP %s: %s', request_ip, resolved_ip_info) if resolved_ip_info and resolved_ip_info.provider == 'aws': - return super(CloudFrontedS3Storage, self).get_direct_download_url(path, request_ip, expires_in, requires_cors, + return super(CloudFrontedS3Storage, self).get_direct_download_url(path, request_ip, + expires_in, requires_cors, head) - + url = 'https://%s/%s' % (self.cloudfront_distribution_domain, path) expire_date = datetime.now() + timedelta(seconds=expires_in) signer = self._get_cloudfront_signer() signed_url = signer.generate_presigned_url(url, date_less_than=expire_date) - logger.debug('Returning CloudFront URL for path "%s" with IP "%s": %s', path, resolved_ip_info, signed_url) + logger.debug('Returning CloudFront URL for path "%s" with IP "%s": %s', path, resolved_ip_info, + signed_url) return signed_url @lru_cache(maxsize=1) diff --git a/storage/test/test_cloudfront.py b/storage/test/test_cloudfront.py index 69c3ddbdc..d03fc39be 100644 --- a/storage/test/test_cloudfront.py +++ b/storage/test/test_cloudfront.py @@ -52,3 +52,17 @@ def test_direct_download(test_aws_ip, aws_ip_range_data, ipranges_populated, app # get back an S3 URL. assert 's3.amazonaws.com' in engine.get_direct_download_url(_TEST_PATH, '1.2.3.4') +@mock_s3 +def test_direct_download_no_ip(test_aws_ip, aws_ip_range_data, ipranges_populated, app): + ipresolver = IPResolver(app) + context = StorageContext('nyc', None, None, config_provider, ipresolver) + + # Create a test bucket and put some test content. + boto.connect_s3().create_bucket(_TEST_BUCKET) + + engine = CloudFrontedS3Storage(context, 'cloudfrontdomain', 'keyid', 'test/data/test.pem', 'some/path', + _TEST_BUCKET, _TEST_USER, _TEST_PASSWORD) + engine.put_content(_TEST_PATH, _TEST_CONTENT) + assert engine.exists(_TEST_PATH) + + assert 'cloudfrontdomain' in engine.get_direct_download_url(_TEST_PATH)