Commit 8cc3d5a2 authored by Marc Shaw's avatar Marc Shaw

Merge branch 'secure-files-read-only-feature-flag' into 'master'

Adding ci_secure_files_read_only Feature Flag

See merge request gitlab-org/gitlab!84089
parents 153a5710 b47fb9e9
---
name: ci_secure_files_read_only
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/84089
rollout_issue_url:
milestone: '14.10'
type: ops
group: group::incubation
default_enabled: false
\ No newline at end of file
...@@ -54,6 +54,7 @@ module API ...@@ -54,6 +54,7 @@ module API
resource do resource do
before do before do
read_only_feature_flag_enabled?
authorize! :admin_secure_files, user_project authorize! :admin_secure_files, user_project
end end
...@@ -97,6 +98,10 @@ module API ...@@ -97,6 +98,10 @@ module API
def feature_flag_enabled? def feature_flag_enabled?
service_unavailable! unless Feature.enabled?(:ci_secure_files, user_project, default_enabled: :yaml) service_unavailable! unless Feature.enabled?(:ci_secure_files, user_project, default_enabled: :yaml)
end end
def read_only_feature_flag_enabled?
service_unavailable! if Feature.enabled?(:ci_secure_files_read_only, user_project, type: :ops, default_enabled: :yaml)
end
end end
end end
end end
......
...@@ -6,6 +6,7 @@ RSpec.describe API::Ci::SecureFiles do ...@@ -6,6 +6,7 @@ RSpec.describe API::Ci::SecureFiles do
before do before do
stub_ci_secure_file_object_storage stub_ci_secure_file_object_storage
stub_feature_flags(ci_secure_files: true) stub_feature_flags(ci_secure_files: true)
stub_feature_flags(ci_secure_files_read_only: false)
end end
let_it_be(:maintainer) { create(:user) } let_it_be(:maintainer) { create(:user) }
...@@ -16,6 +17,13 @@ RSpec.describe API::Ci::SecureFiles do ...@@ -16,6 +17,13 @@ RSpec.describe API::Ci::SecureFiles do
let_it_be(:project) { create(:project, creator_id: maintainer.id) } let_it_be(:project) { create(:project, creator_id: maintainer.id) }
let_it_be(:secure_file) { create(:ci_secure_file, project: project) } let_it_be(:secure_file) { create(:ci_secure_file, project: project) }
let(:file_params) do
{
file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks'),
name: 'upload-keystore.jks'
}
end
before_all do before_all do
project.add_maintainer(maintainer) project.add_maintainer(maintainer)
project.add_developer(developer) project.add_developer(developer)
...@@ -40,6 +48,43 @@ RSpec.describe API::Ci::SecureFiles do ...@@ -40,6 +48,43 @@ RSpec.describe API::Ci::SecureFiles do
end end
end end
context 'ci_secure_files_read_only feature flag' do
context 'when the flag is enabled' do
before do
stub_feature_flags(ci_secure_files_read_only: true)
end
it 'returns a 503 when attempting to upload a file' do
stub_feature_flags(ci_secure_files_read_only: true)
expect do
post api("/projects/#{project.id}/secure_files", maintainer), params: file_params
end.not_to change {project.secure_files.count}
expect(response).to have_gitlab_http_status(:service_unavailable)
end
it 'returns a 200 when downloading a file' do
stub_feature_flags(ci_secure_files_read_only: true)
get api("/projects/#{project.id}/secure_files", developer)
expect(response).to have_gitlab_http_status(:ok)
expect(json_response).to be_a(Array)
end
end
context 'when the flag is disabled' do
it 'returns a 201 when uploading a file when the ci_secure_files_read_only feature flag is disabled' do
expect do
post api("/projects/#{project.id}/secure_files", maintainer), params: file_params
end.to change {project.secure_files.count}.by(1)
expect(response).to have_gitlab_http_status(:created)
end
end
end
context 'authenticated user with admin permissions' do context 'authenticated user with admin permissions' do
it 'returns project secure files' do it 'returns project secure files' do
get api("/projects/#{project.id}/secure_files", maintainer) get api("/projects/#{project.id}/secure_files", maintainer)
...@@ -204,14 +249,8 @@ RSpec.describe API::Ci::SecureFiles do ...@@ -204,14 +249,8 @@ RSpec.describe API::Ci::SecureFiles do
describe 'POST /projects/:id/secure_files' do describe 'POST /projects/:id/secure_files' do
context 'authenticated user with admin permissions' do context 'authenticated user with admin permissions' do
it 'creates a secure file' do it 'creates a secure file' do
params = {
file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks'),
name: 'upload-keystore.jks',
permissions: 'execute'
}
expect do expect do
post api("/projects/#{project.id}/secure_files", maintainer), params: params post api("/projects/#{project.id}/secure_files", maintainer), params: file_params.merge(permissions: 'execute')
end.to change {project.secure_files.count}.by(1) end.to change {project.secure_files.count}.by(1)
expect(response).to have_gitlab_http_status(:created) expect(response).to have_gitlab_http_status(:created)
...@@ -229,26 +268,15 @@ RSpec.describe API::Ci::SecureFiles do ...@@ -229,26 +268,15 @@ RSpec.describe API::Ci::SecureFiles do
end end
it 'creates a secure file with read_only permissions by default' do it 'creates a secure file with read_only permissions by default' do
params = {
file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks'),
name: 'upload-keystore.jks'
}
expect do expect do
post api("/projects/#{project.id}/secure_files", maintainer), params: params post api("/projects/#{project.id}/secure_files", maintainer), params: file_params
end.to change {project.secure_files.count}.by(1) end.to change {project.secure_files.count}.by(1)
expect(json_response['permissions']).to eq('read_only') expect(json_response['permissions']).to eq('read_only')
end end
it 'uploads and downloads a secure file' do it 'uploads and downloads a secure file' do
post_params = { post api("/projects/#{project.id}/secure_files", maintainer), params: file_params
file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks'),
name: 'upload-keystore.jks',
permissions: 'read_write'
}
post api("/projects/#{project.id}/secure_files", maintainer), params: post_params
secure_file_id = json_response['id'] secure_file_id = json_response['id']
...@@ -268,12 +296,8 @@ RSpec.describe API::Ci::SecureFiles do ...@@ -268,12 +296,8 @@ RSpec.describe API::Ci::SecureFiles do
end end
it 'returns an error when no file is uploaded' do it 'returns an error when no file is uploaded' do
post_params = {
name: 'upload-keystore.jks'
}
expect do expect do
post api("/projects/#{project.id}/secure_files", maintainer), params: post_params post api("/projects/#{project.id}/secure_files", maintainer), params: { name: 'upload-keystore.jks' }
end.not_to change { project.secure_files.count } end.not_to change { project.secure_files.count }
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
...@@ -281,12 +305,8 @@ RSpec.describe API::Ci::SecureFiles do ...@@ -281,12 +305,8 @@ RSpec.describe API::Ci::SecureFiles do
end end
it 'returns an error when the file name is missing' do it 'returns an error when the file name is missing' do
post_params = {
file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks')
}
expect do expect do
post api("/projects/#{project.id}/secure_files", maintainer), params: post_params post api("/projects/#{project.id}/secure_files", maintainer), params: { file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks') }
end.not_to change { project.secure_files.count } end.not_to change { project.secure_files.count }
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
...@@ -308,14 +328,8 @@ RSpec.describe API::Ci::SecureFiles do ...@@ -308,14 +328,8 @@ RSpec.describe API::Ci::SecureFiles do
end end
it 'returns an error when an unexpected permission is supplied' do it 'returns an error when an unexpected permission is supplied' do
post_params = {
file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks'),
name: 'upload-keystore.jks',
permissions: 'foo'
}
expect do expect do
post api("/projects/#{project.id}/secure_files", maintainer), params: post_params post api("/projects/#{project.id}/secure_files", maintainer), params: file_params.merge(permissions: 'foo')
end.not_to change { project.secure_files.count } end.not_to change { project.secure_files.count }
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
...@@ -329,13 +343,8 @@ RSpec.describe API::Ci::SecureFiles do ...@@ -329,13 +343,8 @@ RSpec.describe API::Ci::SecureFiles do
allow(instance).to receive_message_chain(:errors, :messages).and_return(['Error 1', 'Error 2']) allow(instance).to receive_message_chain(:errors, :messages).and_return(['Error 1', 'Error 2'])
end end
post_params = {
file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks'),
name: 'upload-keystore.jks'
}
expect do expect do
post api("/projects/#{project.id}/secure_files", maintainer), params: post_params post api("/projects/#{project.id}/secure_files", maintainer), params: file_params
end.not_to change { project.secure_files.count } end.not_to change { project.secure_files.count }
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
...@@ -346,13 +355,8 @@ RSpec.describe API::Ci::SecureFiles do ...@@ -346,13 +355,8 @@ RSpec.describe API::Ci::SecureFiles do
allow(instance).to receive_message_chain(:file, :size).and_return(6.megabytes.to_i) allow(instance).to receive_message_chain(:file, :size).and_return(6.megabytes.to_i)
end end
post_params = {
file: fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks'),
name: 'upload-keystore.jks'
}
expect do expect do
post api("/projects/#{project.id}/secure_files", maintainer), params: post_params post api("/projects/#{project.id}/secure_files", maintainer), params: file_params
end.not_to change { project.secure_files.count } end.not_to change { project.secure_files.count }
expect(response).to have_gitlab_http_status(:payload_too_large) expect(response).to have_gitlab_http_status(:payload_too_large)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment