diff options
| -rw-r--r-- | app/models/project.rb | 3 | ||||
| -rw-r--r-- | app/models/upload.rb | 2 | ||||
| -rw-r--r-- | app/uploaders/lfs_object_uploader.rb | 5 | ||||
| -rw-r--r-- | config/gitlab.yml.example | 26 | ||||
| -rw-r--r-- | config/sidekiq_queues.yml | 1 | ||||
| -rw-r--r-- | db/migrate/20170601163708_add_artifacts_store_to_ci_build.rb | 13 | ||||
| -rw-r--r-- | ee/spec/requests/api/jobs_spec.rb | 72 | ||||
| -rw-r--r-- | lib/gitlab/ci/trace/http_io.rb | 4 | ||||
| -rw-r--r-- | spec/controllers/concerns/send_file_upload_spec.rb | 8 | ||||
| -rw-r--r-- | spec/features/projects/import_export/test_project_export.tar.gz | bin | 343092 -> 341299 bytes | |||
| -rw-r--r-- | spec/lib/gitlab/ci/trace/http_io_spec.rb | 26 | ||||
| -rw-r--r-- | spec/requests/api/jobs_spec.rb | 47 | 
12 files changed, 147 insertions, 60 deletions
| diff --git a/app/models/project.rb b/app/models/project.rb index 0cef4d335dd..452c6784bd4 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -188,6 +188,8 @@ class Project < ActiveRecord::Base    has_many :todos    has_many :notification_settings, as: :source, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent +  has_many :internal_ids +    has_one :import_data, class_name: 'ProjectImportData', inverse_of: :project, autosave: true    has_one :project_feature, inverse_of: :project    has_one :statistics, class_name: 'ProjectStatistics' @@ -290,7 +292,6 @@ class Project < ActiveRecord::Base    scope :non_archived, -> { where(archived: false) }    scope :for_milestones, ->(ids) { joins(:milestones).where('milestones.id' => ids).distinct }    scope :with_push, -> { joins(:events).where('events.action = ?', Event::PUSHED) } -    scope :with_project_feature, -> { joins('LEFT JOIN project_features ON projects.id = project_features.project_id') }    scope :with_statistics, -> { includes(:statistics) }    scope :with_shared_runners, -> { where(shared_runners_enabled: true) } diff --git a/app/models/upload.rb b/app/models/upload.rb index cf71a7b76fc..e94a81c8770 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -1,4 +1,6 @@  class Upload < ActiveRecord::Base +  prepend EE::Upload +    # Upper limit for foreground checksum processing    CHECKSUM_THRESHOLD = 100.megabytes diff --git a/app/uploaders/lfs_object_uploader.rb b/app/uploaders/lfs_object_uploader.rb index e7cce1bbb0a..eb521a22ebc 100644 --- a/app/uploaders/lfs_object_uploader.rb +++ b/app/uploaders/lfs_object_uploader.rb @@ -2,11 +2,6 @@ class LfsObjectUploader < GitlabUploader    extend Workhorse::UploadPath    include ObjectStorage::Concern -  # LfsObject are in `tmp/upload` instead of `tmp/uploads` -  def self.workhorse_upload_path -    File.join(root, 'tmp/upload') -  end -    storage_options Gitlab.config.lfs    def filename diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 075ac022674..05299adfa93 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -182,19 +182,18 @@ production: &base      # storage_path: public/      # base_dir: uploads/-/system      object_store: -      enabled: true -      remote_directory: uploads # Bucket name -      # background_upload: false # Temporary option to limit automatic upload (Default: true) -      # proxy_download: false # Passthrough all downloads via GitLab instead of using Redirects to Object Storage -      connection: -        provider: AWS -        aws_access_key_id: AWS_ACCESS_KEY_ID -        aws_secret_access_key: AWS_SECRET_ACCESS_KEY -        region: eu-central-1 -        # Use the following options to configure an AWS compatible host -        # host: 'localhost' # default: s3.amazonaws.com -        # endpoint: 'http://127.0.0.1:9000' # default: nil -        # path_style: true # Use 'host/bucket_name/object' instead of 'bucket_name.host/object' +      enabled: false +    # remote_directory: uploads # Bucket name +    # background_upload: false # Temporary option to limit automatic upload (Default: true) +    # proxy_download: false # Passthrough all downloads via GitLab instead of using Redirects to Object Storage +    # connection: +    #   provider: AWS +    #   aws_access_key_id: AWS_ACCESS_KEY_ID +    #   aws_secret_access_key: AWS_SECRET_ACCESS_KEY +    #   region: eu-central-1 +    #   host: 'localhost' # default: s3.amazonaws.com +    #   endpoint: 'http://127.0.0.1:9000' # default: nil +    #   path_style: true # Use 'host/bucket_name/object' instead of 'bucket_name.host/object'    ## GitLab Pages    pages: @@ -719,7 +718,6 @@ test:          region: eu-central-1    uploads:      storage_path: tmp/tests/public -    enabled: true      object_store:        enabled: false        connection: diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 1c9c1d67669..c811034b29d 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -64,7 +64,6 @@    - [update_user_activity, 1]    - [propagate_service_template, 1]    - [background_migration, 1] -  - [object_storage_upload, 1]    - [gcp_cluster, 1]    - [project_migrate_hashed_storage, 1]    - [storage_migrator, 1] diff --git a/db/migrate/20170601163708_add_artifacts_store_to_ci_build.rb b/db/migrate/20170601163708_add_artifacts_store_to_ci_build.rb index deba890a478..e82109190a7 100644 --- a/db/migrate/20170601163708_add_artifacts_store_to_ci_build.rb +++ b/db/migrate/20170601163708_add_artifacts_store_to_ci_build.rb @@ -3,15 +3,8 @@ class AddArtifactsStoreToCiBuild < ActiveRecord::Migration    DOWNTIME = false -  disable_ddl_transaction! - -  def up -    add_column_with_default(:ci_builds, :artifacts_file_store, :integer, default: 1) -    add_column_with_default(:ci_builds, :artifacts_metadata_store, :integer, default: 1) -  end - -  def down -    remove_column(:ci_builds, :artifacts_file_store) -    remove_column(:ci_builds, :artifacts_metadata_store) +  def change +    add_column(:ci_builds, :artifacts_file_store, :integer) +    add_column(:ci_builds, :artifacts_metadata_store, :integer)    end  end diff --git a/ee/spec/requests/api/jobs_spec.rb b/ee/spec/requests/api/jobs_spec.rb new file mode 100644 index 00000000000..9f8b502867d --- /dev/null +++ b/ee/spec/requests/api/jobs_spec.rb @@ -0,0 +1,72 @@ +require 'spec_helper' + +describe API::Jobs do +  set(:project) do +    create(:project, :repository, public_builds: false) +  end + +  set(:pipeline) do +    create(:ci_empty_pipeline, project: project, +                               sha: project.commit.id, +                               ref: project.default_branch) +  end + +  let!(:job) { create(:ci_build, :success, pipeline: pipeline) } + +  let(:user) { create(:user) } +  let(:api_user) { user } +  let(:reporter) { create(:project_member, :reporter, project: project).user } +  let(:cross_project_pipeline_enabled) { true } + +  before do +    stub_licensed_features(cross_project_pipelines: cross_project_pipeline_enabled) +    project.add_developer(user) +  end + +  describe 'GET /projects/:id/jobs/:job_id/artifacts' do +    shared_examples 'downloads artifact' do +      let(:download_headers) do +        { 'Content-Transfer-Encoding' => 'binary', +          'Content-Disposition' => 'attachment; filename=ci_build_artifacts.zip' } +      end + +      it 'returns specific job artifacts' do +        expect(response).to have_gitlab_http_status(200) +        expect(response.headers).to include(download_headers) +        expect(response.body).to match_file(job.artifacts_file.file.file) +      end +    end + +    context 'authorized by job_token' do +      let(:job) { create(:ci_build, :artifacts, pipeline: pipeline, user: api_user) } + +      before do +        get api("/projects/#{project.id}/jobs/#{job.id}/artifacts"), job_token: job.token +      end + +      context 'user is developer' do +        let(:api_user) { user } + +        it_behaves_like 'downloads artifact' +      end + +      context 'when anonymous user is accessing private artifacts' do +        let(:api_user) { nil } + +        it 'hides artifacts and rejects request' do +          expect(project).to be_private +          expect(response).to have_gitlab_http_status(404) +        end +      end + +      context 'feature is disabled for EES' do +        let(:api_user) { user } +        let(:cross_project_pipeline_enabled) { false } + +        it 'disallows access to the artifacts' do +          expect(response).to have_gitlab_http_status(404) +        end +      end +    end +  end +end diff --git a/lib/gitlab/ci/trace/http_io.rb b/lib/gitlab/ci/trace/http_io.rb index 5256f7999c1..ac4308f4e2c 100644 --- a/lib/gitlab/ci/trace/http_io.rb +++ b/lib/gitlab/ci/trace/http_io.rb @@ -37,6 +37,10 @@ module Gitlab          end          def path +          nil +        end + +        def url            @uri.to_s          end diff --git a/spec/controllers/concerns/send_file_upload_spec.rb b/spec/controllers/concerns/send_file_upload_spec.rb index da4e955f6a3..f4c99ea4064 100644 --- a/spec/controllers/concerns/send_file_upload_spec.rb +++ b/spec/controllers/concerns/send_file_upload_spec.rb @@ -64,10 +64,12 @@ describe SendFileUpload do          end          it 'sends a file' do -          subject +          headers = double +          expect(headers).to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-url:/) +          expect(controller).to receive(:headers) { headers } +          expect(controller).to receive(:head).with(:ok) -          is_expected.to start_with(Gitlab::Workhorse::SEND_DATA_HEADER) -          is_expected.to end_with(/^send-url:/) +          subject          end        end diff --git a/spec/features/projects/import_export/test_project_export.tar.gz b/spec/features/projects/import_export/test_project_export.tar.gzBinary files differ index 0cc68aff494..ecb7651acad 100644 --- a/spec/features/projects/import_export/test_project_export.tar.gz +++ b/spec/features/projects/import_export/test_project_export.tar.gz diff --git a/spec/lib/gitlab/ci/trace/http_io_spec.rb b/spec/lib/gitlab/ci/trace/http_io_spec.rb index b839ef7ce36..5474e2f518c 100644 --- a/spec/lib/gitlab/ci/trace/http_io_spec.rb +++ b/spec/lib/gitlab/ci/trace/http_io_spec.rb @@ -7,26 +7,6 @@ describe Gitlab::Ci::Trace::HttpIO do    let(:url) { remote_trace_url }    let(:size) { remote_trace_size } -  describe 'Interchangeability between IO and HttpIO' do -    EXCEPT_METHODS = %i[read_nonblock raw raw! cooked cooked! getch echo= echo? -                        winsize winsize= iflush oflush ioflush beep goto cursor cursor= pressed? -                        getpass write_nonblock stat pathconf wait_readable wait_writable getbyte << -                        wait lines bytes chars codepoints getc readpartial set_encoding printf print -                        putc puts readlines gets each each_byte each_char each_codepoint to_io reopen -                        syswrite to_i fileno sysread fdatasync fsync sync= sync lineno= lineno readchar -                        ungetbyte readbyte ungetc nonblock= nread rewind pos= eof close_on_exec? -                        close_on_exec= closed? close_read close_write isatty tty? binmode? sysseek -                        advise ioctl fcntl pid external_encoding internal_encoding autoclose? autoclose= -                        posix_fileno nonblock? ready? noecho nonblock].freeze - -    it 'HttpIO covers core interfaces in IO' do -      expected_interfaces = ::IO.instance_methods(false) -      expected_interfaces -= EXCEPT_METHODS - -      expect(expected_interfaces - described_class.instance_methods).to be_empty -    end -  end -    describe '#close' do      subject { http_io.close } @@ -48,6 +28,12 @@ describe Gitlab::Ci::Trace::HttpIO do    describe '#path' do      subject { http_io.path } +    it { is_expected.to be_nil } +  end + +  describe '#url' do +    subject { http_io.url } +      it { is_expected.to eq(url) }    end diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb index 62ed9fd00a1..81335e72350 100644 --- a/spec/requests/api/jobs_spec.rb +++ b/spec/requests/api/jobs_spec.rb @@ -21,6 +21,7 @@ describe API::Jobs do    let(:guest) { create(:project_member, :guest, project: project).user }    before do +    stub_licensed_features(cross_project_pipelines: true)      project.add_developer(user)    end @@ -316,11 +317,6 @@ describe API::Jobs do        end      end -    before do -      stub_artifacts_object_storage -      get api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user) -    end -      context 'normal authentication' do        context 'job with artifacts' do          context 'when artifacts are stored locally' do @@ -344,8 +340,10 @@ describe API::Jobs do          end          context 'when artifacts are stored remotely' do +          let(:proxy_download) { false } +            before do -            stub_artifacts_object_storage +            stub_artifacts_object_storage(proxy_download: proxy_download)            end            let(:job) { create(:ci_build, pipeline: pipeline) } @@ -357,6 +355,20 @@ describe API::Jobs do              get api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user)            end +          context 'when proxy download is enabled' do +            let(:proxy_download) { true } + +            it 'responds with the workhorse send-url' do +              expect(response.headers[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("send-url:") +            end +          end + +          context 'when proxy download is disabled' do +            it 'returns location redirect' do +              expect(response).to have_gitlab_http_status(302) +            end +          end +            context 'authorized user' do              it 'returns the file remote URL' do                expect(response).to redirect_to(artifact.file.url) @@ -495,6 +507,29 @@ describe API::Jobs do          it_behaves_like 'a valid file'        end + +      context 'when using job_token to authenticate' do +        before do +          pipeline.reload +          pipeline.update(ref: 'master', +                          sha: project.commit('master').sha) + +          get api("/projects/#{project.id}/jobs/artifacts/master/download"), job: job.name, job_token: job.token +        end + +        context 'when user is reporter' do +          it_behaves_like 'a valid file' +        end + +        context 'when user is admin, but not member' do +          let(:api_user) { create(:admin) } +          let(:job) { create(:ci_build, :artifacts, pipeline: pipeline, user: api_user) } + +          it 'does not allow to see that artfiact is present' do +            expect(response).to have_gitlab_http_status(404) +          end +        end +      end      end    end | 
