From 9a2b93a416a8b418f650a6919f6c9faea4d61e01 Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Fri, 8 Oct 2021 13:23:12 -0400 Subject: [PATCH 1/4] ApplicationSetting: Add runner_token_expiration_interval field Add the new field so that runners can use it to determine their token expiration time. Changelog: added Issue: https://gitlab.com/gitlab-org/gitlab/-/issue/30942 --- app/models/application_setting.rb | 4 ++++ ...piration_interval_settings_to_application_settings.rb | 9 +++++++++ db/schema_migrations/20220118155846 | 1 + db/structure.sql | 3 +++ 4 files changed, 17 insertions(+) create mode 100644 db/migrate/20220118155846_add_runner_token_expiration_interval_settings_to_application_settings.rb create mode 100644 db/schema_migrations/20220118155846 diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index e6aa3d525a9eef..9d82224356356a 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -78,6 +78,10 @@ def self.kroki_formats_attributes chronic_duration_attr_writer :archive_builds_in_human_readable, :archive_builds_in_seconds + chronic_duration_attr :runner_token_expiration_interval_human_readable, :runner_token_expiration_interval + chronic_duration_attr :group_runner_token_expiration_interval_human_readable, :group_runner_token_expiration_interval + chronic_duration_attr :project_runner_token_expiration_interval_human_readable, :project_runner_token_expiration_interval + validates :grafana_url, system_hook_url: { blocked_message: "is blocked: %{exception_message}. " + GRAFANA_URL_ERROR_MESSAGE diff --git a/db/migrate/20220118155846_add_runner_token_expiration_interval_settings_to_application_settings.rb b/db/migrate/20220118155846_add_runner_token_expiration_interval_settings_to_application_settings.rb new file mode 100644 index 00000000000000..32ca8a5fb12cfa --- /dev/null +++ b/db/migrate/20220118155846_add_runner_token_expiration_interval_settings_to_application_settings.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddRunnerTokenExpirationIntervalSettingsToApplicationSettings < Gitlab::Database::Migration[1.0] + def change + [:runner_token_expiration_interval, :group_runner_token_expiration_interval, :project_runner_token_expiration_interval].each do |field| + add_column :application_settings, field, :integer + end + end +end diff --git a/db/schema_migrations/20220118155846 b/db/schema_migrations/20220118155846 new file mode 100644 index 00000000000000..b5247cb374318e --- /dev/null +++ b/db/schema_migrations/20220118155846 @@ -0,0 +1 @@ +88935eee0781ee1faaf52e3bc89dc5c490c2095d6ccf83e4fd4186641507c173 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 258597a21a4d51..314c06890cbe9b 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -10483,6 +10483,9 @@ CREATE TABLE application_settings ( future_subscriptions jsonb DEFAULT '[]'::jsonb NOT NULL, user_email_lookup_limit integer DEFAULT 60 NOT NULL, packages_cleanup_package_file_worker_capacity smallint DEFAULT 2 NOT NULL, + runner_token_expiration_interval integer, + group_runner_token_expiration_interval integer, + project_runner_token_expiration_interval integer, CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)), CONSTRAINT app_settings_dep_proxy_ttl_policies_worker_capacity_positive CHECK ((dependency_proxy_ttl_group_policy_worker_capacity >= 0)), CONSTRAINT app_settings_ext_pipeline_validation_service_url_text_limit CHECK ((char_length(external_pipeline_validation_service_url) <= 255)), -- GitLab From 2200280922db24e8d29bec631163adeb048decd4 Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Mon, 10 Jan 2022 11:39:15 -0500 Subject: [PATCH 2/4] Concerns: Add RunnerTokenExpirationInterval concern This concern de-duplicates code between Group and Project. --- .../runner_token_expiration_interval.rb | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 app/models/concerns/runner_token_expiration_interval.rb diff --git a/app/models/concerns/runner_token_expiration_interval.rb b/app/models/concerns/runner_token_expiration_interval.rb new file mode 100644 index 00000000000000..f84e69e7b7dded --- /dev/null +++ b/app/models/concerns/runner_token_expiration_interval.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module RunnerTokenExpirationInterval + extend ActiveSupport::Concern + + def enforced_runner_token_expiration_interval_human_readable + interval = enforced_runner_token_expiration_interval + ChronicDuration.output(interval, format: :short) if interval + end + + def effective_runner_token_expiration_interval + [ + enforced_runner_token_expiration_interval, + runner_token_expiration_interval&.seconds + ].compact.min + end + + def effective_runner_token_expiration_interval_human_readable + interval = effective_runner_token_expiration_interval + ChronicDuration.output(interval, format: :short) if interval + end +end -- GitLab From 69dcbd670516ee02a05950535624975baf53714c Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Fri, 8 Oct 2021 13:24:35 -0400 Subject: [PATCH 3/4] Group: Add runner token expiration interval settings These settings determine when group runners' tokens will expire. Changelog: added Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/30942 --- app/models/group.rb | 16 ++ app/models/namespace_setting.rb | 8 +- ...interval_settings_to_namespace_settings.rb | 11 + db/schema_migrations/20220118155847 | 1 + db/structure.sql | 3 + spec/models/group_spec.rb | 219 ++++++++++++++++++ 6 files changed, 257 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20220118155847_add_runner_token_expiration_interval_settings_to_namespace_settings.rb create mode 100644 db/schema_migrations/20220118155847 diff --git a/app/models/group.rb b/app/models/group.rb index 92d7fddf7369af..6c4d22cc43b1c0 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -17,6 +17,8 @@ class Group < Namespace include GroupAPICompatibility include EachBatch include BulkMemberAccessLoad + include ChronicDurationAttribute + include RunnerTokenExpirationInterval def self.sti_name 'Group' @@ -91,6 +93,9 @@ def self.sti_name has_many :group_callouts, class_name: 'Users::GroupCallout', foreign_key: :group_id delegate :prevent_sharing_groups_outside_hierarchy, :new_user_signups_cap, :setup_for_company, :jobs_to_be_done, to: :namespace_settings + delegate :runner_token_expiration_interval, :runner_token_expiration_interval=, :runner_token_expiration_interval_human_readable, :runner_token_expiration_interval_human_readable=, to: :namespace_settings, allow_nil: true + delegate :subgroup_runner_token_expiration_interval, :subgroup_runner_token_expiration_interval=, :subgroup_runner_token_expiration_interval_human_readable, :subgroup_runner_token_expiration_interval_human_readable=, to: :namespace_settings, allow_nil: true + delegate :project_runner_token_expiration_interval, :project_runner_token_expiration_interval=, :project_runner_token_expiration_interval_human_readable, :project_runner_token_expiration_interval_human_readable=, to: :namespace_settings, allow_nil: true has_one :crm_settings, class_name: 'Group::CrmSettings', inverse_of: :group @@ -784,6 +789,17 @@ def shared_with_group_links_visible_to_user(user) shared_with_group_links.preload_shared_with_groups.filter { |link| Ability.allowed?(user, :read_group, link.shared_with_group) } end + def enforced_runner_token_expiration_interval + all_parent_groups = Gitlab::ObjectHierarchy.new(Group.where(id: id)).ancestors + all_group_settings = NamespaceSetting.where(namespace_id: all_parent_groups) + group_interval = all_group_settings.where.not(subgroup_runner_token_expiration_interval: nil).minimum(:subgroup_runner_token_expiration_interval)&.seconds + + [ + Gitlab::CurrentSettings.group_runner_token_expiration_interval&.seconds, + group_interval + ].compact.min + end + private def max_member_access(user_ids) diff --git a/app/models/namespace_setting.rb b/app/models/namespace_setting.rb index ed0ac0929547c7..ef917c8a22e3a0 100644 --- a/app/models/namespace_setting.rb +++ b/app/models/namespace_setting.rb @@ -3,6 +3,7 @@ class NamespaceSetting < ApplicationRecord include CascadingNamespaceSettingAttribute include Sanitizable + include ChronicDurationAttribute cascading_attr :delayed_project_removal @@ -16,10 +17,15 @@ class NamespaceSetting < ApplicationRecord enum jobs_to_be_done: { basics: 0, move_repository: 1, code_storage: 2, exploring: 3, ci: 4, other: 5 }, _suffix: true + chronic_duration_attr :runner_token_expiration_interval_human_readable, :runner_token_expiration_interval + chronic_duration_attr :subgroup_runner_token_expiration_interval_human_readable, :subgroup_runner_token_expiration_interval + chronic_duration_attr :project_runner_token_expiration_interval_human_readable, :project_runner_token_expiration_interval + NAMESPACE_SETTINGS_PARAMS = [:default_branch_name, :delayed_project_removal, :lock_delayed_project_removal, :resource_access_token_creation_allowed, :prevent_sharing_groups_outside_hierarchy, :new_user_signups_cap, - :setup_for_company, :jobs_to_be_done].freeze + :setup_for_company, :jobs_to_be_done, :runner_token_expiration_interval, + :subgroup_runner_token_expiration_interval, :project_runner_token_expiration_interval].freeze self.primary_key = :namespace_id diff --git a/db/migrate/20220118155847_add_runner_token_expiration_interval_settings_to_namespace_settings.rb b/db/migrate/20220118155847_add_runner_token_expiration_interval_settings_to_namespace_settings.rb new file mode 100644 index 00000000000000..7b83cb2dd554f3 --- /dev/null +++ b/db/migrate/20220118155847_add_runner_token_expiration_interval_settings_to_namespace_settings.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class AddRunnerTokenExpirationIntervalSettingsToNamespaceSettings < Gitlab::Database::Migration[1.0] + enable_lock_retries! + + def change + [:runner_token_expiration_interval, :subgroup_runner_token_expiration_interval, :project_runner_token_expiration_interval].each do |field| + add_column :namespace_settings, field, :integer + end + end +end diff --git a/db/schema_migrations/20220118155847 b/db/schema_migrations/20220118155847 new file mode 100644 index 00000000000000..caa0dfc590146d --- /dev/null +++ b/db/schema_migrations/20220118155847 @@ -0,0 +1 @@ +aa73b04aa355111564cdc7adb036a2030a28fbb3b524c3b3dbb8248e27b845d7 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 314c06890cbe9b..99201039f28e6d 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -16507,6 +16507,9 @@ CREATE TABLE namespace_settings ( new_user_signups_cap integer, setup_for_company boolean, jobs_to_be_done smallint, + runner_token_expiration_interval integer, + subgroup_runner_token_expiration_interval integer, + project_runner_token_expiration_interval integer, CONSTRAINT check_0ba93c78c7 CHECK ((char_length(default_branch_name) <= 255)) ); diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 67db822a6e86fd..f29fce83da7d3e 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -4,6 +4,7 @@ RSpec.describe Group do include ReloadHelpers + include StubGitlabCalls let!(:group) { create(:group) } @@ -2910,4 +2911,222 @@ def setup_group_members(group) end end end + + describe '#enforced_runner_token_expiration_interval and #effective_runner_token_expiration_interval' do + shared_examples 'no enforced expiration interval' do + it { expect(subject.enforced_runner_token_expiration_interval).to be_nil } + end + + shared_examples 'enforced expiration interval' do |enforced_interval:| + it { expect(subject.enforced_runner_token_expiration_interval).to eq(enforced_interval) } + end + + shared_examples 'no effective expiration interval' do + it { expect(subject.effective_runner_token_expiration_interval).to be_nil } + end + + shared_examples 'effective expiration interval' do |effective_interval:| + it { expect(subject.effective_runner_token_expiration_interval).to eq(effective_interval) } + end + + context 'when there is no interval in group settings' do + let_it_be(:group) { create(:group) } + + subject { group } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'no effective expiration interval' + end + + context 'when there is a group interval' do + let(:group_settings) { create(:namespace_settings, runner_token_expiration_interval: 3.days.to_i) } + + subject { create(:group, namespace_settings: group_settings) } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'effective expiration interval', effective_interval: 3.days + end + + # runner_token_expiration_interval should not affect the expiration interval, only + # group_runner_token_expiration_interval should. + context 'when there is a site-wide enforced shared interval' do + before do + stub_application_setting(runner_token_expiration_interval: 5.days.to_i) + end + + let_it_be(:group) { create(:group) } + + subject { group } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'no effective expiration interval' + end + + context 'when there is a site-wide enforced group interval' do + before do + stub_application_setting(group_runner_token_expiration_interval: 5.days.to_i) + end + + let_it_be(:group) { create(:group) } + + subject { group } + + it_behaves_like 'enforced expiration interval', enforced_interval: 5.days + it_behaves_like 'effective expiration interval', effective_interval: 5.days + end + + # project_runner_token_expiration_interval should not affect the expiration interval, only + # group_runner_token_expiration_interval should. + context 'when there is a site-wide enforced project interval' do + before do + stub_application_setting(project_runner_token_expiration_interval: 5.days.to_i) + end + + let_it_be(:group) { create(:group) } + + subject { group } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'no effective expiration interval' + end + + # runner_token_expiration_interval should not affect the expiration interval, only + # subgroup_runner_token_expiration_interval should. + context 'when there is a grandparent group enforced group interval' do + let_it_be(:grandparent_group_settings) { create(:namespace_settings, runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:grandparent_group) { create(:group, namespace_settings: grandparent_group_settings) } + let_it_be(:parent_group) { create(:group, parent: grandparent_group) } + let_it_be(:subgroup) { create(:group, parent: parent_group) } + + subject { subgroup } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'no effective expiration interval' + end + + context 'when there is a grandparent group enforced subgroup interval' do + let_it_be(:grandparent_group_settings) { create(:namespace_settings, subgroup_runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:grandparent_group) { create(:group, namespace_settings: grandparent_group_settings) } + let_it_be(:parent_group) { create(:group, parent: grandparent_group) } + let_it_be(:subgroup) { create(:group, parent: parent_group) } + + subject { subgroup } + + it_behaves_like 'enforced expiration interval', enforced_interval: 4.days + it_behaves_like 'effective expiration interval', effective_interval: 4.days + end + + # project_runner_token_expiration_interval should not affect the expiration interval, only + # subgroup_runner_token_expiration_interval should. + context 'when there is a grandparent group enforced project interval' do + let_it_be(:grandparent_group_settings) { create(:namespace_settings, project_runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:grandparent_group) { create(:group, namespace_settings: grandparent_group_settings) } + let_it_be(:parent_group) { create(:group, parent: grandparent_group) } + let_it_be(:subgroup) { create(:group, parent: parent_group) } + + subject { subgroup } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'no effective expiration interval' + end + + context 'when there is a parent group enforced interval overridden by group interval' do + let_it_be(:parent_group_settings) { create(:namespace_settings, subgroup_runner_token_expiration_interval: 5.days.to_i) } + let_it_be(:parent_group) { create(:group, namespace_settings: parent_group_settings) } + let_it_be(:group_settings) { create(:namespace_settings, runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:subgroup_with_settings) { create(:group, parent: parent_group, namespace_settings: group_settings) } + + subject { subgroup_with_settings } + + it_behaves_like 'enforced expiration interval', enforced_interval: 5.days + it_behaves_like 'effective expiration interval', effective_interval: 4.days + + it 'has human-readable expiration intervals' do + expect(subject.enforced_runner_token_expiration_interval_human_readable).to eq('5d') + expect(subject.effective_runner_token_expiration_interval_human_readable).to eq('4d') + end + end + + context 'when site-wide enforced interval overrides group interval' do + before do + stub_application_setting(group_runner_token_expiration_interval: 3.days.to_i) + end + + let_it_be(:group_settings) { create(:namespace_settings, runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:group_with_settings) { create(:group, namespace_settings: group_settings) } + + subject { group_with_settings } + + it_behaves_like 'enforced expiration interval', enforced_interval: 3.days + it_behaves_like 'effective expiration interval', effective_interval: 3.days + end + + context 'when group interval overrides site-wide enforced interval' do + before do + stub_application_setting(group_runner_token_expiration_interval: 5.days.to_i) + end + + let_it_be(:group_settings) { create(:namespace_settings, runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:group_with_settings) { create(:group, namespace_settings: group_settings) } + + subject { group_with_settings } + + it_behaves_like 'enforced expiration interval', enforced_interval: 5.days + it_behaves_like 'effective expiration interval', effective_interval: 4.days + end + + context 'when site-wide enforced interval overrides parent group enforced interval' do + before do + stub_application_setting(group_runner_token_expiration_interval: 3.days.to_i) + end + + let_it_be(:parent_group_settings) { create(:namespace_settings, subgroup_runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:parent_group) { create(:group, namespace_settings: parent_group_settings) } + let_it_be(:subgroup) { create(:group, parent: parent_group) } + + subject { subgroup } + + it_behaves_like 'enforced expiration interval', enforced_interval: 3.days + it_behaves_like 'effective expiration interval', effective_interval: 3.days + end + + context 'when parent group enforced interval overrides site-wide enforced interval' do + before do + stub_application_setting(group_runner_token_expiration_interval: 5.days.to_i) + end + + let_it_be(:parent_group_settings) { create(:namespace_settings, subgroup_runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:parent_group) { create(:group, namespace_settings: parent_group_settings) } + let_it_be(:subgroup) { create(:group, parent: parent_group) } + + subject { subgroup } + + it_behaves_like 'enforced expiration interval', enforced_interval: 4.days + it_behaves_like 'effective expiration interval', effective_interval: 4.days + end + + # Unrelated groups should not affect the expiration interval. + context 'when there is an enforced group interval in an unrelated group' do + let_it_be(:unrelated_group_settings) { create(:namespace_settings, subgroup_runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:unrelated_group) { create(:group, namespace_settings: unrelated_group_settings) } + let_it_be(:group) { create(:group) } + + subject { group } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'no effective expiration interval' + end + + # Subgroups should not affect the parent group expiration interval. + context 'when there is an enforced group interval in a subgroup' do + let_it_be(:subgroup_settings) { create(:namespace_settings, subgroup_runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:subgroup) { create(:group, parent: group, namespace_settings: subgroup_settings) } + let_it_be(:group) { create(:group) } + + subject { group } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'no effective expiration interval' + end + end end -- GitLab From 8f301cfb5efa4e6930475df74eb52d4aebc05dba Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Fri, 8 Oct 2021 13:25:32 -0400 Subject: [PATCH 4/4] Project: Add runner token expiration interval settings These settings determine when project runners' tokens will expire. Changelog: added Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/30942 --- app/models/project.rb | 17 ++ app/models/project_ci_cd_setting.rb | 4 + ...n_interval_settings_to_project_settings.rb | 9 + db/schema_migrations/20220118155848 | 1 + db/structure.sql | 3 +- lib/api/entities/project.rb | 1 + .../import_export/project/import_export.yml | 1 + spec/factories/projects.rb | 4 + .../import_export/safe_model_attributes.yml | 1 + spec/models/project_spec.rb | 253 ++++++++++++++++++ 10 files changed, 293 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20220118155848_add_runner_token_expiration_interval_settings_to_project_settings.rb create mode 100644 db/schema_migrations/20220118155848 diff --git a/app/models/project.rb b/app/models/project.rb index 5aa4b4d99680e5..c59cac8ee85542 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -37,6 +37,7 @@ class Project < ApplicationRecord include EachBatch include GitlabRoutingHelper include BulkMemberAccessLoad + include RunnerTokenExpirationInterval extend Gitlab::Cache::RequestCache extend Gitlab::Utils::Override @@ -454,6 +455,7 @@ def self.integration_association_name(name) delegate :job_token_scope_enabled, :job_token_scope_enabled=, to: :ci_cd_settings, prefix: :ci, allow_nil: true delegate :keep_latest_artifact, :keep_latest_artifact=, to: :ci_cd_settings, allow_nil: true delegate :restrict_user_defined_variables, :restrict_user_defined_variables=, to: :ci_cd_settings, allow_nil: true + delegate :runner_token_expiration_interval, :runner_token_expiration_interval=, :runner_token_expiration_interval_human_readable, :runner_token_expiration_interval_human_readable=, to: :ci_cd_settings, allow_nil: true delegate :actual_limits, :actual_plan_name, :actual_plan, to: :namespace, allow_nil: true delegate :allow_merge_on_skipped_pipeline, :allow_merge_on_skipped_pipeline?, :allow_merge_on_skipped_pipeline=, :has_confluence?, :has_shimo?, @@ -2697,6 +2699,10 @@ def keep_latest_artifact? ci_cd_settings.keep_latest_artifact? end + def runner_token_expiration_interval + ci_cd_settings&.runner_token_expiration_interval + end + def group_runners_enabled? return false unless ci_cd_settings @@ -2728,6 +2734,17 @@ def remove_project_authorizations(user_ids, per_batch = 1000) end end + def enforced_runner_token_expiration_interval + all_parent_groups = Gitlab::ObjectHierarchy.new(Group.where(id: group)).base_and_ancestors + all_group_settings = NamespaceSetting.where(namespace_id: all_parent_groups) + group_interval = all_group_settings.where.not(project_runner_token_expiration_interval: nil).minimum(:project_runner_token_expiration_interval)&.seconds + + [ + Gitlab::CurrentSettings.project_runner_token_expiration_interval&.seconds, + group_interval + ].compact.min + end + private # overridden in EE diff --git a/app/models/project_ci_cd_setting.rb b/app/models/project_ci_cd_setting.rb index c8a0b247b59151..28a493cae33a44 100644 --- a/app/models/project_ci_cd_setting.rb +++ b/app/models/project_ci_cd_setting.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class ProjectCiCdSetting < ApplicationRecord + include ChronicDurationAttribute + belongs_to :project, inverse_of: :ci_cd_settings DEFAULT_GIT_DEPTH = 20 @@ -17,6 +19,8 @@ class ProjectCiCdSetting < ApplicationRecord default_value_for :forward_deployment_enabled, true + chronic_duration_attr :runner_token_expiration_interval_human_readable, :runner_token_expiration_interval + def forward_deployment_enabled? super && ::Feature.enabled?(:forward_deployment_enabled, project, default_enabled: true) end diff --git a/db/migrate/20220118155848_add_runner_token_expiration_interval_settings_to_project_settings.rb b/db/migrate/20220118155848_add_runner_token_expiration_interval_settings_to_project_settings.rb new file mode 100644 index 00000000000000..ef9591718283c2 --- /dev/null +++ b/db/migrate/20220118155848_add_runner_token_expiration_interval_settings_to_project_settings.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddRunnerTokenExpirationIntervalSettingsToProjectSettings < Gitlab::Database::Migration[1.0] + enable_lock_retries! + + def change + add_column :project_ci_cd_settings, :runner_token_expiration_interval, :integer + end +end diff --git a/db/schema_migrations/20220118155848 b/db/schema_migrations/20220118155848 new file mode 100644 index 00000000000000..bf3205b5ddad42 --- /dev/null +++ b/db/schema_migrations/20220118155848 @@ -0,0 +1 @@ +4d5adffe1a3e835d6d23f6bd9634993c778fef23d134552d54b003af2a3ff4da \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 99201039f28e6d..41c5b497ad847d 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -18174,7 +18174,8 @@ CREATE TABLE project_ci_cd_settings ( auto_rollback_enabled boolean DEFAULT false NOT NULL, keep_latest_artifact boolean DEFAULT true NOT NULL, restrict_user_defined_variables boolean DEFAULT false NOT NULL, - job_token_scope_enabled boolean DEFAULT false NOT NULL + job_token_scope_enabled boolean DEFAULT false NOT NULL, + runner_token_expiration_interval integer ); CREATE SEQUENCE project_ci_cd_settings_id_seq diff --git a/lib/api/entities/project.rb b/lib/api/entities/project.rb index 01bb35f89e3fae..74097dc2883179 100644 --- a/lib/api/entities/project.rb +++ b/lib/api/entities/project.rb @@ -132,6 +132,7 @@ class Project < BasicProjectDetails Ability.allowed?(options[:current_user], :change_repository_storage, project) } expose :keep_latest_artifacts_available?, as: :keep_latest_artifact + expose :runner_token_expiration_interval # rubocop: disable CodeReuse/ActiveRecord def self.preload_resource(project) diff --git a/lib/gitlab/import_export/project/import_export.yml b/lib/gitlab/import_export/project/import_export.yml index f65d166a995a33..059f6bd42e36de 100644 --- a/lib/gitlab/import_export/project/import_export.yml +++ b/lib/gitlab/import_export/project/import_export.yml @@ -120,6 +120,7 @@ included_attributes: - :name ci_cd_settings: - :group_runners_enabled + - :runner_token_expiration_interval metrics_setting: - :dashboard_timezone - :external_dashboard_url diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index f34f6b67ef88f2..c345fa0c8b46bf 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -49,6 +49,8 @@ forward_deployment_enabled { nil } restrict_user_defined_variables { nil } ci_job_token_scope_enabled { nil } + runner_token_expiration_interval { nil } + runner_token_expiration_interval_human_readable { nil } end after(:build) do |project, evaluator| @@ -92,6 +94,8 @@ project.keep_latest_artifact = evaluator.keep_latest_artifact unless evaluator.keep_latest_artifact.nil? project.restrict_user_defined_variables = evaluator.restrict_user_defined_variables unless evaluator.restrict_user_defined_variables.nil? project.ci_job_token_scope_enabled = evaluator.ci_job_token_scope_enabled unless evaluator.ci_job_token_scope_enabled.nil? + project.runner_token_expiration_interval = evaluator.runner_token_expiration_interval unless evaluator.runner_token_expiration_interval.nil? + project.runner_token_expiration_interval_human_readable = evaluator.runner_token_expiration_interval_human_readable unless evaluator.runner_token_expiration_interval_human_readable.nil? if evaluator.import_status import_state = project.import_state || project.build_import_state diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 6ffe2187466928..f019883a91edd0 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -692,6 +692,7 @@ Badge: - type ProjectCiCdSetting: - group_runners_enabled +- runner_token_expiration_interval ProjectSetting: - allow_merge_on_skipped_pipeline - has_confluence diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index f88d741c96e1e6..bc5d19fab929d3 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -7,6 +7,7 @@ include GitHelpers include ExternalAuthorizationServiceHelpers include ReloadHelpers + include StubGitlabCalls using RSpec::Parameterized::TableSyntax let_it_be(:namespace) { create_default(:namespace).freeze } @@ -7455,6 +7456,258 @@ def has_external_wiki end end + describe '#enforced_runner_token_expiration_interval and #effective_runner_token_expiration_interval' do + shared_examples 'no enforced expiration interval' do + it { expect(subject.enforced_runner_token_expiration_interval).to be_nil } + end + + shared_examples 'enforced expiration interval' do |enforced_interval:| + it { expect(subject.enforced_runner_token_expiration_interval).to eq(enforced_interval) } + end + + shared_examples 'no effective expiration interval' do + it { expect(subject.effective_runner_token_expiration_interval).to be_nil } + end + + shared_examples 'effective expiration interval' do |effective_interval:| + it { expect(subject.effective_runner_token_expiration_interval).to eq(effective_interval) } + end + + context 'when there is no interval' do + let_it_be(:project) { create(:project) } + + subject { project } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'no effective expiration interval' + end + + context 'when there is a project interval' do + let_it_be(:project) { create(:project, runner_token_expiration_interval: 3.days.to_i) } + + subject { project } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'effective expiration interval', effective_interval: 3.days + end + + # runner_token_expiration_interval should not affect the expiration interval, only + # project_runner_token_expiration_interval should. + context 'when there is a site-wide enforced shared interval' do + before do + stub_application_setting(runner_token_expiration_interval: 5.days.to_i) + end + + let_it_be(:project) { create(:project) } + + subject { project } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'no effective expiration interval' + end + + # group_runner_token_expiration_interval should not affect the expiration interval, only + # project_runner_token_expiration_interval should. + context 'when there is a site-wide enforced group interval' do + before do + stub_application_setting(group_runner_token_expiration_interval: 5.days.to_i) + end + + let_it_be(:project) { create(:project) } + + subject { project } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'no effective expiration interval' + end + + context 'when there is a site-wide enforced project interval' do + before do + stub_application_setting(project_runner_token_expiration_interval: 5.days.to_i) + end + + let_it_be(:project) { create(:project) } + + subject { project } + + it_behaves_like 'enforced expiration interval', enforced_interval: 5.days + it_behaves_like 'effective expiration interval', effective_interval: 5.days + end + + # runner_token_expiration_interval should not affect the expiration interval, only + # project_runner_token_expiration_interval should. + context 'when there is a group-enforced group interval' do + let_it_be(:group_settings) { create(:namespace_settings, runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:group) { create(:group, namespace_settings: group_settings) } + let_it_be(:project) { create(:project, group: group) } + + subject { project } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'no effective expiration interval' + end + + # subgroup_runner_token_expiration_interval should not affect the expiration interval, only + # project_runner_token_expiration_interval should. + context 'when there is a group-enforced subgroup interval' do + let_it_be(:group_settings) { create(:namespace_settings, subgroup_runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:group) { create(:group, namespace_settings: group_settings) } + let_it_be(:project) { create(:project, group: group) } + + subject { project } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'no effective expiration interval' + end + + context 'when there is an owner group-enforced project interval' do + let_it_be(:group_settings) { create(:namespace_settings, project_runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:group) { create(:group, namespace_settings: group_settings) } + let_it_be(:project) { create(:project, group: group) } + + subject { project } + + it_behaves_like 'enforced expiration interval', enforced_interval: 4.days + it_behaves_like 'effective expiration interval', effective_interval: 4.days + end + + context 'when there is a grandparent group-enforced interval' do + let_it_be(:grandparent_group_settings) { create(:namespace_settings, project_runner_token_expiration_interval: 3.days.to_i) } + let_it_be(:grandparent_group) { create(:group, namespace_settings: grandparent_group_settings) } + let_it_be(:parent_group_settings) { create(:namespace_settings) } + let_it_be(:parent_group) { create(:group, parent: grandparent_group, namespace_settings: parent_group_settings) } + let_it_be(:group_settings) { create(:namespace_settings, project_runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:group) { create(:group, parent: parent_group, namespace_settings: group_settings) } + let_it_be(:project) { create(:project, group: group) } + + subject { project } + + it_behaves_like 'enforced expiration interval', enforced_interval: 3.days + it_behaves_like 'effective expiration interval', effective_interval: 3.days + end + + context 'when there is a parent group-enforced interval overridden by group-enforced interval' do + let_it_be(:parent_group_settings) { create(:namespace_settings, project_runner_token_expiration_interval: 5.days.to_i) } + let_it_be(:parent_group) { create(:group, namespace_settings: parent_group_settings) } + let_it_be(:group_settings) { create(:namespace_settings, project_runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:group) { create(:group, parent: parent_group, namespace_settings: group_settings) } + let_it_be(:project) { create(:project, group: group) } + + subject { project } + + it_behaves_like 'enforced expiration interval', enforced_interval: 4.days + it_behaves_like 'effective expiration interval', effective_interval: 4.days + end + + context 'when site-wide enforced interval overrides project interval' do + before do + stub_application_setting(project_runner_token_expiration_interval: 3.days.to_i) + end + + let_it_be(:project) { create(:project, runner_token_expiration_interval: 4.days.to_i) } + + subject { project } + + it_behaves_like 'enforced expiration interval', enforced_interval: 3.days + it_behaves_like 'effective expiration interval', effective_interval: 3.days + end + + context 'when project interval overrides site-wide enforced interval' do + before do + stub_application_setting(project_runner_token_expiration_interval: 5.days.to_i) + end + + let_it_be(:project) { create(:project, runner_token_expiration_interval: 4.days.to_i) } + + subject { project } + + it_behaves_like 'enforced expiration interval', enforced_interval: 5.days + it_behaves_like 'effective expiration interval', effective_interval: 4.days + + it 'has human-readable expiration intervals' do + expect(subject.enforced_runner_token_expiration_interval_human_readable).to eq('5d') + expect(subject.effective_runner_token_expiration_interval_human_readable).to eq('4d') + end + end + + context 'when site-wide enforced interval overrides group-enforced interval' do + before do + stub_application_setting(project_runner_token_expiration_interval: 3.days.to_i) + end + + let_it_be(:group_settings) { create(:namespace_settings, project_runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:group) { create(:group, namespace_settings: group_settings) } + let_it_be(:project) { create(:project, group: group) } + + subject { project } + + it_behaves_like 'enforced expiration interval', enforced_interval: 3.days + it_behaves_like 'effective expiration interval', effective_interval: 3.days + end + + context 'when group-enforced interval overrides site-wide enforced interval' do + before do + stub_application_setting(project_runner_token_expiration_interval: 5.days.to_i) + end + + let_it_be(:group_settings) { create(:namespace_settings, project_runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:group) { create(:group, namespace_settings: group_settings) } + let_it_be(:project) { create(:project, group: group) } + + subject { project } + + it_behaves_like 'enforced expiration interval', enforced_interval: 4.days + it_behaves_like 'effective expiration interval', effective_interval: 4.days + end + + context 'when group-enforced interval overrides project interval' do + let_it_be(:group_settings) { create(:namespace_settings, project_runner_token_expiration_interval: 3.days.to_i) } + let_it_be(:group) { create(:group, namespace_settings: group_settings) } + let_it_be(:project) { create(:project, group: group, runner_token_expiration_interval: 4.days.to_i) } + + subject { project } + + it_behaves_like 'enforced expiration interval', enforced_interval: 3.days + it_behaves_like 'effective expiration interval', effective_interval: 3.days + end + + context 'when project interval overrides group-enforced interval' do + let_it_be(:group_settings) { create(:namespace_settings, project_runner_token_expiration_interval: 5.days.to_i) } + let_it_be(:group) { create(:group, namespace_settings: group_settings) } + let_it_be(:project) { create(:project, group: group, runner_token_expiration_interval: 4.days.to_i) } + + subject { project } + + it_behaves_like 'enforced expiration interval', enforced_interval: 5.days + it_behaves_like 'effective expiration interval', effective_interval: 4.days + end + + # Unrelated groups should not affect the expiration interval. + context 'when there is an enforced project interval in an unrelated group' do + let_it_be(:unrelated_group_settings) { create(:namespace_settings, project_runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:unrelated_group) { create(:group, namespace_settings: unrelated_group_settings) } + let_it_be(:project) { create(:project) } + + subject { project } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'no effective expiration interval' + end + + # Subgroups should not affect the parent group expiration interval. + context 'when there is an enforced project interval in a subgroup' do + let_it_be(:group) { create(:group) } + let_it_be(:subgroup_settings) { create(:namespace_settings, project_runner_token_expiration_interval: 4.days.to_i) } + let_it_be(:subgroup) { create(:group, parent: group, namespace_settings: subgroup_settings) } + let_it_be(:project) { create(:project, group: group) } + + subject { project } + + it_behaves_like 'no enforced expiration interval' + it_behaves_like 'no effective expiration interval' + end + end + it_behaves_like 'it has loose foreign keys' do let(:factory_name) { :project } end -- GitLab