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.gz Binary files differindex 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 |