From 9b457ec51e69ce305f93973e79454c9ec4625194 Mon Sep 17 00:00:00 2001 From: Alexander Mancevice Date: Tue, 20 Mar 2018 16:46:00 -0400 Subject: [PATCH] Upgraded use of AWS Ruby SDK for S3 When S3DownloadStrategy is detected in DownloadStrategyDetector an attempt is made to install & require the aws-sdk-s3 gem. The AWS S3 SDK is used to sign the URL and handed off to curl_download. --- Library/Homebrew/download_strategy.rb | 26 +++++++-------- .../Homebrew/test/download_strategies_spec.rb | 32 ++++++++++++++++++- 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb index 6e750806f0..2b594c9081 100644 --- a/Library/Homebrew/download_strategy.rb +++ b/Library/Homebrew/download_strategy.rb @@ -473,16 +473,6 @@ end # distribution. (It will work for public buckets as well.) class S3DownloadStrategy < CurlDownloadStrategy def _fetch - # Put the aws gem requirement here (vs top of file) so it's only - # a dependency of S3 users, not all Homebrew users - require "rubygems" - begin - require "aws-sdk-v1" - rescue LoadError - onoe "Install the aws-sdk gem into the gem repo used by brew." - raise - end - if @url !~ %r{^https?://([^.].*)\.s3\.amazonaws\.com/(.+)$} raise "Bad S3 URL: " + @url end @@ -492,12 +482,12 @@ class S3DownloadStrategy < CurlDownloadStrategy ENV["AWS_ACCESS_KEY_ID"] = ENV["HOMEBREW_AWS_ACCESS_KEY_ID"] ENV["AWS_SECRET_ACCESS_KEY"] = ENV["HOMEBREW_AWS_SECRET_ACCESS_KEY"] - obj = AWS::S3.new.buckets[bucket].objects[key] begin - s3url = obj.url_for(:get) - rescue AWS::Errors::MissingCredentialsError + signer = Aws::S3::Presigner.new + s3url = signer.presigned_url :get_object, bucket: bucket, key: key + rescue Aws::Sigv4::Errors::MissingCredentialsError ohai "AWS credentials missing, trying public URL instead." - s3url = obj.public_url + s3url = @url end curl_download s3url, to: temporary_path @@ -1116,6 +1106,9 @@ class DownloadStrategyDetector def self.detect(url, strategy = nil) if strategy.nil? detect_from_url(url) + elsif strategy == S3DownloadStrategy + require_aws_sdk + strategy elsif strategy.is_a?(Class) && strategy < AbstractDownloadStrategy strategy elsif strategy.is_a?(Symbol) @@ -1169,4 +1162,9 @@ class DownloadStrategyDetector raise "Unknown download strategy #{symbol} was requested." end end + + def self.require_aws_sdk + Homebrew.install_gem! "aws-sdk-s3", "~> 1.8" + require "aws-sdk-s3" + end end diff --git a/Library/Homebrew/test/download_strategies_spec.rb b/Library/Homebrew/test/download_strategies_spec.rb index f1c64db717..b8b975e52a 100644 --- a/Library/Homebrew/test/download_strategies_spec.rb +++ b/Library/Homebrew/test/download_strategies_spec.rb @@ -200,6 +200,26 @@ describe GitDownloadStrategy do end end +describe S3DownloadStrategy do + subject { described_class.new(name, resource) } + let(:name) { "foo" } + let(:url) { "http://bucket.s3.amazonaws.com/foo.tar.gz" } + let(:resource) { double(Resource, url: url, mirrors: [], specs: {}, version: nil) } + + describe "#_fetch" do + subject { described_class.new(name, resource)._fetch } + + context "when given Bad S3 URL" do + let(:url) { "http://example.com/foo.tar.gz" } + it "should raise Bad S3 URL error" do + expect { + subject._fetch + }.to raise_error(RuntimeError) + end + end + end +end + describe CurlDownloadStrategy do subject { described_class.new(name, resource) } let(:name) { "foo" } @@ -226,8 +246,9 @@ end describe DownloadStrategyDetector do describe "::detect" do - subject { described_class.detect(url) } + subject { described_class.detect(url, strategy) } let(:url) { Object.new } + let(:strategy) { nil } context "when given Git URL" do let(:url) { "git://example.com/foo.git" } @@ -239,6 +260,15 @@ describe DownloadStrategyDetector do it { is_expected.to eq(GitHubGitDownloadStrategy) } end + context "when given strategy = S3DownloadStrategy" do + let(:url) { "https://bkt.s3.amazonaws.com/key.tar.gz" } + let(:strategy) { S3DownloadStrategy } + it "requires aws-sdk-s3" do + allow(DownloadStrategyDetector).to receive(:require_aws_sdk).and_return(true) + is_expected.to eq(S3DownloadStrategy) + end + end + it "defaults to cURL" do expect(subject).to eq(CurlDownloadStrategy) end