diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index b0f5d4a9933aa33002d4cd656a46a01798d4d071..d807e6263ee1bcd77cf3fc306f8d31657a6bb63e 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -83,6 +83,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController :akismet_api_key, :akismet_enabled, :container_registry_token_expire_delay, + :default_artifacts_expire_in, :default_branch_protection, :default_group_visibility, :default_project_visibility, diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 4212f1247ccbb7225102baed26c05b01eb812dee..dc36c7544388d9ccbbedd36abc8f744876902222 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -76,6 +76,12 @@ class ApplicationSetting < ActiveRecord::Base presence: true, numericality: { only_integer: true, greater_than: 0 } + validates :max_artifacts_size, + presence: true, + numericality: { only_integer: true, greater_than: 0 } + + validates :default_artifacts_expire_in, presence: true, duration: true + validates :container_registry_token_expire_delay, presence: true, numericality: { only_integer: true, greater_than: 0 } @@ -168,6 +174,7 @@ class ApplicationSetting < ActiveRecord::Base after_sign_up_text: nil, akismet_enabled: false, container_registry_token_expire_delay: 5, + default_artifacts_expire_in: '30 days', default_branch_protection: Settings.gitlab['default_branch_protection'], default_project_visibility: Settings.gitlab.default_projects_features['visibility_level'], default_projects_limit: Settings.gitlab['default_projects_limit'], @@ -201,9 +208,9 @@ class ApplicationSetting < ActiveRecord::Base sign_in_text: nil, signin_enabled: Settings.gitlab['signin_enabled'], signup_enabled: Settings.gitlab['signup_enabled'], + terminal_max_session_time: 0, two_factor_grace_period: 48, - user_default_external: false, - terminal_max_session_time: 0 + user_default_external: false } end @@ -215,6 +222,14 @@ class ApplicationSetting < ActiveRecord::Base create(defaults) end + def self.human_attribute_name(attr, _options = {}) + if attr == :default_artifacts_expire_in + 'Default artifacts expiration' + else + super + end + end + def home_page_url_column_exist ActiveRecord::Base.connection.column_exists?(:application_settings, :home_page_url) end diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 16d4f3b4f1b1da200ccf7151ed26b5ede73f51d7..77aba91f65cd12c96f30599ac38be8e05a9a29b9 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -484,7 +484,7 @@ module Ci def artifacts_expire_in=(value) self.artifacts_expire_at = if value - Time.now + ChronicDuration.parse(value) + ChronicDuration.parse(value)&.seconds&.from_now end end diff --git a/app/validators/duration_validator.rb b/app/validators/duration_validator.rb new file mode 100644 index 0000000000000000000000000000000000000000..10ff44031c6616101a1e0ae7ad85e368ef606360 --- /dev/null +++ b/app/validators/duration_validator.rb @@ -0,0 +1,17 @@ +# DurationValidator +# +# Validate the format conforms with ChronicDuration +# +# Example: +# +# class ApplicationSetting < ActiveRecord::Base +# validates :default_artifacts_expire_in, presence: true, duration: true +# end +# +class DurationValidator < ActiveModel::EachValidator + def validate_each(record, attribute, value) + ChronicDuration.parse(value) + rescue ChronicDuration::DurationParseError + record.errors.add(attribute, "is not a correct duration") + end +end diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index 749c74b811022abb714a9234120f5ec1acccbfc4..057b584e1bca457b199066b2a8d3baf197a67e83 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -212,8 +212,16 @@ .col-sm-10 = f.number_field :max_artifacts_size, class: 'form-control' .help-block - Set the maximum file size each jobs's artifacts can have - = link_to "(?)", help_page_path("user/admin_area/settings/continuous_integration", anchor: "maximum-artifacts-size") + Set the maximum file size for each job's artifacts + = link_to icon('question-circle'), help_page_path('user/admin_area/settings/continuous_integration', anchor: 'maximum-artifacts-size') + .form-group + = f.label :default_artifacts_expire_in, 'Default artifacts expiration', class: 'control-label col-sm-2' + .col-sm-10 + = f.text_field :default_artifacts_expire_in, class: 'form-control' + .help-block + Set the default expiration time for each job's artifacts. + 0 for unlimited. + = link_to icon('question-circle'), help_page_path('user/admin_area/settings/continuous_integration', anchor: 'default-artifacts-expiration') - if Gitlab.config.registry.enabled %fieldset diff --git a/changelogs/unreleased/27762-add-default-artifacts-expiration.yml b/changelogs/unreleased/27762-add-default-artifacts-expiration.yml new file mode 100644 index 0000000000000000000000000000000000000000..27fa77ed04dd161b65927fe8ca8a48cd33acbe05 --- /dev/null +++ b/changelogs/unreleased/27762-add-default-artifacts-expiration.yml @@ -0,0 +1,4 @@ +--- +title: Add admin setting for default artifacts expiration +merge_request: 9219 +author: diff --git a/db/migrate/20170214084746_add_default_artifacts_expiration_to_application_settings.rb b/db/migrate/20170214084746_add_default_artifacts_expiration_to_application_settings.rb new file mode 100644 index 0000000000000000000000000000000000000000..e0e3ff8957a05f8c4f1b3c6fdbc436294b0bd844 --- /dev/null +++ b/db/migrate/20170214084746_add_default_artifacts_expiration_to_application_settings.rb @@ -0,0 +1,11 @@ +class AddDefaultArtifactsExpirationToApplicationSettings < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + add_column :application_settings, + :default_artifacts_expire_in, :string, + null: false, default: '0' + end +end diff --git a/db/schema.rb b/db/schema.rb index 532103b8216a425fce6b0e9cf80626c89ef30069..82bccda580a058ea164635e695312e8cdd25fa19 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -111,6 +111,7 @@ ActiveRecord::Schema.define(version: 20170216141440) do t.boolean "plantuml_enabled" t.integer "max_pages_size", default: 100, null: false t.integer "terminal_max_session_time", default: 0, null: false + t.string "default_artifacts_expire_in", default: '0', null: false end create_table "audit_events", force: :cascade do |t| diff --git a/doc/user/admin_area/settings/continuous_integration.md b/doc/user/admin_area/settings/continuous_integration.md index b8d24cb2d3b6b916a592b82dfe018fdec1bdec71..64df7ee48bda342a8901e4c06278350b53b5994f 100644 --- a/doc/user/admin_area/settings/continuous_integration.md +++ b/doc/user/admin_area/settings/continuous_integration.md @@ -3,18 +3,38 @@ ## Maximum artifacts size The maximum size of the [job artifacts][art-yml] can be set in the Admin area -of your GitLab instance. The value is in MB and the default is 100MB. Note that -this setting is set for each job. +of your GitLab instance. The value is in *MB* and the default is 100MB. Note +that this setting is set for each job. 1. Go to **Admin area > Settings** (`/admin/application_settings`). ![Admin area settings button](img/admin_area_settings_button.png) -1. Change the value of the maximum artifacts size (in MB): +1. Change the value of maximum artifacts size (in MB): ![Admin area maximum artifacts size](img/admin_area_maximum_artifacts_size.png) 1. Hit **Save** for the changes to take effect. +[art-yml]: ../../../administration/build_artifacts -[art-yml]: ../../../administration/job_artifacts.md +## Default artifacts expiration + +The default expiration time of the [job artifacts][art-yml] can be set in +the Admin area of your GitLab instance. The syntax of duration is described +in [artifacts:expire_in][duration-syntax]. The default is `30 days`. Note that +this setting is set for each job. Set it to 0 if you don't want default +expiration. + +1. Go to **Admin area > Settings** (`/admin/application_settings`). + + ![Admin area settings button](img/admin_area_settings_button.png) + +1. Change the value of default expiration time ([syntax][duration-syntax]): + + ![Admin area default artifacts expiration](img/admin_area_default_artifacts_expiration.png) + +1. Hit **Save** for the changes to take effect. + +[art-yml]: ../../../administration/job_artifacts +[duration-syntax]: ../../../ci/yaml/README#artifactsexpire_in diff --git a/doc/user/admin_area/settings/img/admin_area_default_artifacts_expiration.png b/doc/user/admin_area/settings/img/admin_area_default_artifacts_expiration.png new file mode 100644 index 0000000000000000000000000000000000000000..50a86ede56ba0a2ca1268bdd1e748c84f51e8c7a Binary files /dev/null and b/doc/user/admin_area/settings/img/admin_area_default_artifacts_expiration.png differ diff --git a/doc/user/admin_area/settings/img/admin_area_maximum_artifacts_size.png b/doc/user/admin_area/settings/img/admin_area_maximum_artifacts_size.png index b7d6671902ad980d5eb7497626fffa08d3fe68e0..33fd29e2039a20adb0f2e49b3c7313fe4b957e87 100644 Binary files a/doc/user/admin_area/settings/img/admin_area_maximum_artifacts_size.png and b/doc/user/admin_area/settings/img/admin_area_maximum_artifacts_size.png differ diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 85aa6932f81d7b6ce56452c23252dca25797ca46..0e37f40a887d79d9ef309c32af5cc3c7bac5247e 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -557,6 +557,7 @@ module API expose :default_project_visibility expose :default_snippet_visibility expose :default_group_visibility + expose :default_artifacts_expire_in expose :domain_whitelist expose :domain_blacklist_enabled expose :domain_blacklist diff --git a/lib/api/settings.rb b/lib/api/settings.rb index 747ceb4e3e02d86ce9b7ea434660b6488d8042ff..936c7e0930b62c07bfda1311e19ff9f7bcfa883d 100644 --- a/lib/api/settings.rb +++ b/lib/api/settings.rb @@ -56,7 +56,8 @@ module API given shared_runners_enabled: ->(val) { val } do requires :shared_runners_text, type: String, desc: 'Shared runners text ' end - optional :max_artifacts_size, type: Integer, desc: "Set the maximum file size each build's artifacts can have" + optional :max_artifacts_size, type: Integer, desc: "Set the maximum file size for each job's artifacts" + optional :default_artifacts_expire_in, type: String, desc: "Set the default expiration time for each job's artifacts" optional :max_pages_size, type: Integer, desc: 'Maximum size of pages in MB' optional :container_registry_token_expire_delay, type: Integer, desc: 'Authorization token duration (minutes)' optional :metrics_enabled, type: Boolean, desc: 'Enable the InfluxDB metrics' @@ -117,7 +118,9 @@ module API :send_user_confirmation_email, :domain_whitelist, :domain_blacklist_enabled, :after_sign_up_text, :signin_enabled, :require_two_factor_authentication, :home_page_url, :after_sign_out_path, :sign_in_text, :help_page_text, - :shared_runners_enabled, :max_artifacts_size, :max_pages_size, :container_registry_token_expire_delay, + :shared_runners_enabled, :max_artifacts_size, + :default_artifacts_expire_in, :max_pages_size, + :container_registry_token_expire_delay, :metrics_enabled, :sidekiq_throttling_enabled, :recaptcha_enabled, :akismet_enabled, :admin_notification_email, :sentry_enabled, :repository_storage, :repository_checks_enabled, :koding_enabled, :plantuml_enabled, diff --git a/lib/ci/api/builds.rb b/lib/ci/api/builds.rb index 8b939663ffd972360a4cdda9dceb273515120ea8..0e17ac24d5a957fb07fd9dc46c0129de3199400b 100644 --- a/lib/ci/api/builds.rb +++ b/lib/ci/api/builds.rb @@ -167,7 +167,10 @@ module Ci build.artifacts_file = artifacts build.artifacts_metadata = metadata - build.artifacts_expire_in = params['expire_in'] + build.artifacts_expire_in = + params['expire_in'] || + Gitlab::CurrentSettings.current_application_settings + .default_artifacts_expire_in if build.save present(build, with: Entities::BuildDetails) diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 4086e00e363f6bbab344a056defc616f0600c1d8..01ca1584ed2eac1173df507c48752e6bf787d902 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -29,6 +29,40 @@ describe ApplicationSetting, models: true do it { is_expected.not_to allow_value(['test']).for(:disabled_oauth_sign_in_sources) } end + describe 'default_artifacts_expire_in' do + it 'sets an error if it cannot parse' do + setting.update(default_artifacts_expire_in: 'a') + + expect_invalid + end + + it 'sets an error if it is blank' do + setting.update(default_artifacts_expire_in: ' ') + + expect_invalid + end + + it 'sets the value if it is valid' do + setting.update(default_artifacts_expire_in: '30 days') + + expect(setting).to be_valid + expect(setting.default_artifacts_expire_in).to eq('30 days') + end + + it 'sets the value if it is 0' do + setting.update(default_artifacts_expire_in: '0') + + expect(setting).to be_valid + expect(setting.default_artifacts_expire_in).to eq('0') + end + + def expect_invalid + expect(setting).to be_invalid + expect(setting.errors.messages) + .to have_key(:default_artifacts_expire_in) + end + end + it { is_expected.to validate_presence_of(:max_attachment_size) } it do diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 63b6c3c65a6efb3cbffcc189812158cb2c048f86..4251cb06a2d41a6db3e485236253206de51fbb5e 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -162,11 +162,17 @@ describe Ci::Build, :models do is_expected.to be_nil end - it 'when resseting value' do + it 'when resetting value' do build.artifacts_expire_in = nil is_expected.to be_nil end + + it 'when setting to 0' do + build.artifacts_expire_in = '0' + + is_expected.to be_nil + end end describe '#commit' do diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index 91e3c333a02e63fa8402e582cb7d2c7d68086910..411905edb49c4a9f4d98f5bc318052e6d74cfca3 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -30,8 +30,14 @@ describe API::Settings, 'Settings', api: true do it "updates application settings" do put api("/application/settings", admin), - default_projects_limit: 3, signin_enabled: false, repository_storage: 'custom', koding_enabled: true, koding_url: 'http://koding.example.com', - plantuml_enabled: true, plantuml_url: 'http://plantuml.example.com' + default_projects_limit: 3, + signin_enabled: false, + repository_storage: 'custom', + koding_enabled: true, + koding_url: 'http://koding.example.com', + plantuml_enabled: true, + plantuml_url: 'http://plantuml.example.com', + default_artifacts_expire_in: '2 days' expect(response).to have_http_status(200) expect(json_response['default_projects_limit']).to eq(3) expect(json_response['signin_enabled']).to be_falsey @@ -41,6 +47,7 @@ describe API::Settings, 'Settings', api: true do expect(json_response['koding_url']).to eq('http://koding.example.com') expect(json_response['plantuml_enabled']).to be_truthy expect(json_response['plantuml_url']).to eq('http://plantuml.example.com') + expect(json_response['default_artifacts_expire_in']).to eq('2 days') end end diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb index 444258e312dac34c6928670e2ba09f6103e22186..9948d1a9ea021d43b932f5e7171f7655647da378 100644 --- a/spec/requests/ci/api/builds_spec.rb +++ b/spec/requests/ci/api/builds_spec.rb @@ -630,6 +630,7 @@ describe Ci::API::Builds do context 'with an expire date' do let!(:artifacts) { file_upload } + let(:default_artifacts_expire_in) {} let(:post_data) do { 'file.path' => artifacts.path, @@ -638,6 +639,9 @@ describe Ci::API::Builds do end before do + stub_application_setting( + default_artifacts_expire_in: default_artifacts_expire_in) + post(post_url, post_data, headers_with_token) end @@ -648,7 +652,8 @@ describe Ci::API::Builds do build.reload expect(response).to have_http_status(201) expect(json_response['artifacts_expire_at']).not_to be_empty - expect(build.artifacts_expire_at).to be_within(5.minutes).of(Time.now + 7.days) + expect(build.artifacts_expire_at). + to be_within(5.minutes).of(7.days.from_now) end end @@ -661,6 +666,32 @@ describe Ci::API::Builds do expect(json_response['artifacts_expire_at']).to be_nil expect(build.artifacts_expire_at).to be_nil end + + context 'with application default' do + context 'default to 5 days' do + let(:default_artifacts_expire_in) { '5 days' } + + it 'sets to application default' do + build.reload + expect(response).to have_http_status(201) + expect(json_response['artifacts_expire_at']) + .not_to be_empty + expect(build.artifacts_expire_at) + .to be_within(5.minutes).of(5.days.from_now) + end + end + + context 'default to 0' do + let(:default_artifacts_expire_in) { '0' } + + it 'does not set expire_in' do + build.reload + expect(response).to have_http_status(201) + expect(json_response['artifacts_expire_at']).to be_nil + expect(build.artifacts_expire_at).to be_nil + end + end + end end end