diff options
63 files changed, 523 insertions, 285 deletions
diff --git a/Gemfile.lock b/Gemfile.lock index 41794b552ca..960422fc3b0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -413,7 +413,6 @@ GEM fog-core (~> 2.1) fog-json (~> 1.1) fog-xml (~> 0.1) - ipaddress (~> 0.8) fog-core (2.1.0) builder excon (~> 0.58) diff --git a/app/assets/javascripts/incidents/components/incidents_list.vue b/app/assets/javascripts/incidents/components/incidents_list.vue index 922e870caa7..dbd2225167a 100644 --- a/app/assets/javascripts/incidents/components/incidents_list.vue +++ b/app/assets/javascripts/incidents/components/incidents_list.vue @@ -144,7 +144,6 @@ export default { 'assigneeUsernameQuery', 'slaFeatureAvailable', 'canCreateIncident', - 'incidentEscalationsAvailable', ], apollo: { incidents: { @@ -238,7 +237,6 @@ export default { const isHidden = { published: !this.publishedAvailable, incidentSla: !this.slaFeatureAvailable, - escalationStatus: !this.incidentEscalationsAvailable, }; return this.$options.fields.filter(({ key }) => !isHidden[key]); @@ -421,7 +419,7 @@ export default { </div> </template> - <template v-if="incidentEscalationsAvailable" #cell(escalationStatus)="{ item }"> + <template #cell(escalationStatus)="{ item }"> <tooltip-on-truncate :title="getEscalationStatus(item.escalationStatus)" data-testid="incident-escalation-status" diff --git a/app/assets/javascripts/incidents/list.js b/app/assets/javascripts/incidents/list.js index c0f16a43d5c..1d40f1093a4 100644 --- a/app/assets/javascripts/incidents/list.js +++ b/app/assets/javascripts/incidents/list.js @@ -46,7 +46,6 @@ export default () => { assigneeUsernameQuery, slaFeatureAvailable: parseBoolean(slaFeatureAvailable), canCreateIncident: parseBoolean(canCreateIncident), - incidentEscalationsAvailable: parseBoolean(gon?.features?.incidentEscalations), }, apolloProvider, render(createElement) { diff --git a/app/controllers/projects/incidents_controller.rb b/app/controllers/projects/incidents_controller.rb index fd7ba7b5460..70eab792b40 100644 --- a/app/controllers/projects/incidents_controller.rb +++ b/app/controllers/projects/incidents_controller.rb @@ -7,7 +7,6 @@ class Projects::IncidentsController < Projects::ApplicationController before_action :authorize_read_issue! before_action :load_incident, only: [:show] before_action do - push_frontend_feature_flag(:incident_escalations, @project) push_frontend_feature_flag(:incident_timeline, @project) end diff --git a/app/graphql/mutations/terraform/state/delete.rb b/app/graphql/mutations/terraform/state/delete.rb index f08219cb395..f52ace07393 100644 --- a/app/graphql/mutations/terraform/state/delete.rb +++ b/app/graphql/mutations/terraform/state/delete.rb @@ -8,9 +8,9 @@ module Mutations def resolve(id:) state = authorized_find!(id: id) - state.destroy + response = ::Terraform::States::TriggerDestroyService.new(state, current_user: current_user).execute - { errors: errors_on_object(state) } + { errors: response.errors } end end end diff --git a/app/graphql/mutations/user_preferences/update.rb b/app/graphql/mutations/user_preferences/update.rb index b71c952b0f2..c92c6d725b7 100644 --- a/app/graphql/mutations/user_preferences/update.rb +++ b/app/graphql/mutations/user_preferences/update.rb @@ -14,15 +14,6 @@ module Mutations null: true, description: 'User preferences after mutation.' - def ready?(**args) - if disabled_sort_value?(args) - raise Gitlab::Graphql::Errors::ArgumentError, - 'Feature flag `incident_escalations` must be enabled to use this sort order.' - end - - super - end - def resolve(**attributes) user_preferences = current_user.user_preference user_preferences.update(attributes) @@ -32,14 +23,6 @@ module Mutations errors: errors_on_object(user_preferences) } end - - private - - def disabled_sort_value?(args) - return false unless [:escalation_status_asc, :escalation_status_desc].include?(args[:issues_sort]) - - Feature.disabled?(:incident_escalations) - end end end end diff --git a/app/graphql/resolvers/base_issues_resolver.rb b/app/graphql/resolvers/base_issues_resolver.rb index a1fda976876..ec47a8996eb 100644 --- a/app/graphql/resolvers/base_issues_resolver.rb +++ b/app/graphql/resolvers/base_issues_resolver.rb @@ -33,13 +33,6 @@ module Resolvers end end - def prepare_params(args, parent) - return unless [:escalation_status_asc, :escalation_status_desc].include?(args[:sort]) - return if Feature.enabled?(:incident_escalations, parent) - - args[:sort] = :created_desc # default for sort argument - end - private def unconditional_includes diff --git a/app/graphql/resolvers/concerns/issue_resolver_arguments.rb b/app/graphql/resolvers/concerns/issue_resolver_arguments.rb index ea737f54a95..fe213936f55 100644 --- a/app/graphql/resolvers/concerns/issue_resolver_arguments.rb +++ b/app/graphql/resolvers/concerns/issue_resolver_arguments.rb @@ -90,7 +90,6 @@ module IssueResolverArguments prepare_assignee_username_params(args) prepare_release_tag_params(args) - prepare_params(args, parent) if defined?(prepare_params) finder = IssuesFinder.new(current_user, args) diff --git a/app/graphql/types/issue_sort_enum.rb b/app/graphql/types/issue_sort_enum.rb index db51e491d4e..23d1d549d59 100644 --- a/app/graphql/types/issue_sort_enum.rb +++ b/app/graphql/types/issue_sort_enum.rb @@ -14,8 +14,8 @@ module Types value 'TITLE_DESC', 'Title by descending order.', value: :title_desc value 'POPULARITY_ASC', 'Number of upvotes (awarded "thumbs up" emoji) by ascending order.', value: :popularity_asc value 'POPULARITY_DESC', 'Number of upvotes (awarded "thumbs up" emoji) by descending order.', value: :popularity_desc - value 'ESCALATION_STATUS_ASC', 'Status from triggered to resolved. Defaults to `CREATED_DESC` if `incident_escalations` feature flag is disabled.', value: :escalation_status_asc - value 'ESCALATION_STATUS_DESC', 'Status from resolved to triggered. Defaults to `CREATED_DESC` if `incident_escalations` feature flag is disabled.', value: :escalation_status_desc + value 'ESCALATION_STATUS_ASC', 'Status from triggered to resolved.', value: :escalation_status_asc + value 'ESCALATION_STATUS_DESC', 'Status from resolved to triggered.', value: :escalation_status_desc end end diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 713a4386fee..81650e3d84a 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -195,8 +195,6 @@ module Issuable end def supports_escalation? - return false unless ::Feature.enabled?(:incident_escalations, project) - incident? end diff --git a/app/models/merge_request/cleanup_schedule.rb b/app/models/merge_request/cleanup_schedule.rb index 35194b2b318..7f52a110da1 100644 --- a/app/models/merge_request/cleanup_schedule.rb +++ b/app/models/merge_request/cleanup_schedule.rb @@ -8,6 +8,9 @@ class MergeRequest::CleanupSchedule < ApplicationRecord failed: 3 }.freeze + # NOTE: Limit the number of stuck schedule jobs to retry just in case it becomes too big. + STUCK_RETRY_LIMIT = 5 + belongs_to :merge_request, inverse_of: :cleanup_schedule validates :scheduled_at, presence: true @@ -48,6 +51,11 @@ class MergeRequest::CleanupSchedule < ApplicationRecord .order('scheduled_at DESC') } + # NOTE: It is considered stuck as it is unusual to take more than 6 hours to finish the cleanup task. + scope :stuck, -> { + where('updated_at <= NOW() - interval \'6 hours\' AND status = ?', STATUSES[:running]) + } + def self.start_next MergeRequest::CleanupSchedule.transaction do cleanup_schedule = scheduled_and_unstarted.lock('FOR UPDATE SKIP LOCKED').first @@ -58,4 +66,8 @@ class MergeRequest::CleanupSchedule < ApplicationRecord cleanup_schedule end end + + def self.stuck_retry! + self.stuck.limit(STUCK_RETRY_LIMIT).map(&:retry!) + end end diff --git a/app/models/terraform/state.rb b/app/models/terraform/state.rb index 8c3b85ac4c3..4d17a4d332c 100644 --- a/app/models/terraform/state.rb +++ b/app/models/terraform/state.rb @@ -23,13 +23,10 @@ module Terraform scope :ordered_by_name, -> { order(:name) } scope :with_name, -> (name) { where(name: name) } - validates :name, presence: true, uniqueness: { scope: :project_id } - validates :project_id, presence: true + validates :project_id, :name, presence: true validates :uuid, presence: true, uniqueness: true, length: { is: UUID_LENGTH }, format: { with: HEX_REGEXP, message: 'only allows hex characters' } - before_destroy :ensure_state_is_unlocked - default_value_for(:uuid, allows_nil: false) { SecureRandom.hex(UUID_LENGTH / 2) } def latest_file @@ -90,13 +87,6 @@ module Terraform new_version.save! end - def ensure_state_is_unlocked - return unless locked? - - errors.add(:base, s_("Terraform|You cannot remove the State file because it's locked. Unlock the State file first before removing it.")) - throw :abort # rubocop:disable Cop/BanCatchThrow - end - def parse_serial(file) Gitlab::Json.parse(file)["serial"] rescue JSON::ParserError diff --git a/app/models/terraform/state_version.rb b/app/models/terraform/state_version.rb index 31ff7e4c27d..c50eaa66860 100644 --- a/app/models/terraform/state_version.rb +++ b/app/models/terraform/state_version.rb @@ -2,6 +2,7 @@ module Terraform class StateVersion < ApplicationRecord + include EachBatch include FileStoreMounter belongs_to :terraform_state, class_name: 'Terraform::State', optional: false diff --git a/app/services/terraform/remote_state_handler.rb b/app/services/terraform/remote_state_handler.rb index f13477b8b34..849afaddec6 100644 --- a/app/services/terraform/remote_state_handler.rb +++ b/app/services/terraform/remote_state_handler.rb @@ -3,6 +3,7 @@ module Terraform class RemoteStateHandler < BaseService StateLockedError = Class.new(StandardError) + StateDeletedError = Class.new(StandardError) UnauthorizedError = Class.new(StandardError) def find_with_lock @@ -66,14 +67,15 @@ module Terraform find_params = { project: project, name: params[:name] } - return find_state!(find_params) if find_only + state = if find_only + find_state!(find_params) + else + Terraform::State.create_or_find_by(find_params) + end - state = Terraform::State.create_or_find_by(find_params) + raise StateDeletedError if state.deleted_at? - # https://github.com/rails/rails/issues/36027 - return state unless state.errors.of_kind? :name, :taken - - find_state(find_params) + state end def lock_matches?(state) diff --git a/app/services/terraform/states/destroy_service.rb b/app/services/terraform/states/destroy_service.rb new file mode 100644 index 00000000000..76fd0f9433d --- /dev/null +++ b/app/services/terraform/states/destroy_service.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Terraform + module States + class DestroyService + def initialize(state) + @state = state + end + + def execute + return unless state.deleted_at? + + state.versions.each_batch(column: :version) do |batch| + batch.each do |version| + version.file.remove! + end + end + + state.destroy! + end + + private + + attr_reader :state + end + end +end diff --git a/app/services/terraform/states/trigger_destroy_service.rb b/app/services/terraform/states/trigger_destroy_service.rb new file mode 100644 index 00000000000..3669bdcf716 --- /dev/null +++ b/app/services/terraform/states/trigger_destroy_service.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +module Terraform + module States + class TriggerDestroyService + def initialize(state, current_user:) + @state = state + @current_user = current_user + end + + def execute + return unauthorized_response unless can_destroy_state? + return state_locked_response if state.locked? + + state.update!(deleted_at: Time.current) + + Terraform::States::DestroyWorker.perform_async(state.id) + + ServiceResponse.success + end + + private + + attr_reader :state, :current_user + + def can_destroy_state? + current_user.can?(:admin_terraform_state, state.project) + end + + def unauthorized_response + error_response(s_('Terraform|You have insufficient permissions to delete this state')) + end + + def state_locked_response + error_response(s_('Terraform|Cannot remove a locked state')) + end + + def error_response(message) + ServiceResponse.error(message: message) + end + end + end +end diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index fe321c598aa..9822182cc70 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -1866,6 +1866,15 @@ :weight: 1 :idempotent: :tags: [] +- :name: terraform:terraform_states_destroy + :worker_name: Terraform::States::DestroyWorker + :feature_category: :infrastructure_as_code + :has_external_dependencies: + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] - :name: todos_destroyer:todos_destroyer_confidential_issue :worker_name: TodosDestroyer::ConfidentialIssueWorker :feature_category: :team_planning diff --git a/app/workers/schedule_merge_request_cleanup_refs_worker.rb b/app/workers/schedule_merge_request_cleanup_refs_worker.rb index b3d0067471a..8099c3d56b6 100644 --- a/app/workers/schedule_merge_request_cleanup_refs_worker.rb +++ b/app/workers/schedule_merge_request_cleanup_refs_worker.rb @@ -14,6 +14,7 @@ class ScheduleMergeRequestCleanupRefsWorker return if Gitlab::Database.read_only? return unless Feature.enabled?(:merge_request_refs_cleanup) + MergeRequest::CleanupSchedule.stuck_retry! MergeRequestCleanupRefsWorker.perform_with_capacity end end diff --git a/app/workers/terraform/states/destroy_worker.rb b/app/workers/terraform/states/destroy_worker.rb new file mode 100644 index 00000000000..48abf0f22b8 --- /dev/null +++ b/app/workers/terraform/states/destroy_worker.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module Terraform + module States + class DestroyWorker + include ApplicationWorker + + queue_namespace :terraform + feature_category :infrastructure_as_code + + deduplicate :until_executed + idempotent! + urgency :low + data_consistency :always + + def perform(terraform_state_id) + if state = Terraform::State.find_by_id(terraform_state_id) + Terraform::States::DestroyService.new(state).execute + end + end + end + end +end diff --git a/config/feature_flags/development/incident_escalations.yml b/config/feature_flags/development/incident_escalations.yml deleted file mode 100644 index 61ae0092dc9..00000000000 --- a/config/feature_flags/development/incident_escalations.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: incident_escalations -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74337 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/345769 -milestone: '14.6' -type: development -group: group::respond -default_enabled: true diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 1ffc9d97a0e..c8211d8607f 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -433,6 +433,8 @@ - 1 - - tasks_to_be_done_create - 1 +- - terraform + - 1 - - todos_destroyer - 1 - - unassign_issuables diff --git a/db/migrate/20220524021855_add_deleted_at_to_terraform_states.rb b/db/migrate/20220524021855_add_deleted_at_to_terraform_states.rb new file mode 100644 index 00000000000..01bbeb17db7 --- /dev/null +++ b/db/migrate/20220524021855_add_deleted_at_to_terraform_states.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddDeletedAtToTerraformStates < Gitlab::Database::Migration[2.0] + enable_lock_retries! + + def change + add_column :terraform_states, :deleted_at, :datetime_with_timezone + end +end diff --git a/db/schema_migrations/20220524021855 b/db/schema_migrations/20220524021855 new file mode 100644 index 00000000000..2738ba4fb41 --- /dev/null +++ b/db/schema_migrations/20220524021855 @@ -0,0 +1 @@ +549bca1a8f6f33b4044da0ff453cf3e55615697be98366ecdcdc7bbbac2533ef
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 8dce81b5e1c..1183e7d329a 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -21122,7 +21122,8 @@ CREATE TABLE terraform_states ( locked_by_user_id bigint, uuid character varying(32) NOT NULL, name character varying(255) NOT NULL, - versioning_enabled boolean DEFAULT true NOT NULL + versioning_enabled boolean DEFAULT true NOT NULL, + deleted_at timestamp with time zone ); CREATE SEQUENCE terraform_states_id_seq diff --git a/doc/administration/auth/index.md b/doc/administration/auth/index.md index fca4f163075..98babfb9e7a 100644 --- a/doc/administration/auth/index.md +++ b/doc/administration/auth/index.md @@ -8,31 +8,14 @@ info: To determine the technical writer assigned to the Stage/Group associated w # GitLab authentication and authorization **(FREE SELF)** -GitLab integrates with the following external authentication and authorization -providers: - -- [Atlassian](atlassian.md) -- [Auth0](../../integration/auth0.md) -- [Authentiq](authentiq.md) -- [AWS Cognito](cognito.md) -- [Azure](../../integration/azure.md) -- [Bitbucket Cloud](../../integration/bitbucket.md) -- [CAS](../../integration/cas.md) -- [Crowd](crowd.md) -- [Facebook](../../integration/facebook.md) -- [GitHub](../../integration/github.md) -- [GitLab.com](../../integration/gitlab.md) -- [Google OAuth](../../integration/google.md) -- [JWT](jwt.md) -- [Kerberos](../../integration/kerberos.md) +GitLab integrates with a number of [OmniAuth providers](../../integration/omniauth.md#supported-providers), +and the following external authentication and authorization providers: + - [LDAP](ldap/index.md): Includes Active Directory, Apple Open Directory, Open LDAP, and 389 Server. - [Google Secure LDAP](ldap/google_secure_ldap.md) -- [Salesforce](../../integration/salesforce.md) -- [SAML](../../integration/saml.md) - [SAML for GitLab.com groups](../../user/group/saml_sso/index.md) **(PREMIUM SAAS)** - [Smartcard](smartcard.md) **(PREMIUM SELF)** -- [Twitter](../../integration/twitter.md) NOTE: UltraAuth has removed their software which supports OmniAuth integration. We have therefore removed all references to UltraAuth integration. diff --git a/doc/administration/packages/container_registry.md b/doc/administration/packages/container_registry.md index 54584d98bb9..e106ff2bebd 100644 --- a/doc/administration/packages/container_registry.md +++ b/doc/administration/packages/container_registry.md @@ -1297,7 +1297,10 @@ openssl rsa -noout -modulus -in /var/opt/gitlab/gitlab-rails/etc/gitlab-registry ``` If the two pieces of the certificate do not align, remove the files and run `gitlab-ctl reconfigure` -to regenerate the pair. If you have overridden the automatically generated self-signed pair with +to regenerate the pair. The pair is recreated using the existing values in `/etc/gitlab/gitlab-secrets.json` if they exist. To generate a new pair, +delete the `registry` section in your `/etc/gitlab/gitlab-secrets.json` before running `gitlab-ctl reconfigure`. + +If you have overridden the automatically generated self-signed pair with your own certificates and have made sure that their contents align, you can delete the 'registry' section in your `/etc/gitlab/gitlab-secrets.json` and run `gitlab-ctl reconfigure`. diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 9edd98a09e2..9b44fedf19c 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -16895,8 +16895,20 @@ Represents a historically accurate report about the timebox. | Name | Type | Description | | ---- | ---- | ----------- | | <a id="timeboxreportburnuptimeseries"></a>`burnupTimeSeries` | [`[BurnupChartDailyTotals!]`](#burnupchartdailytotals) | Daily scope and completed totals for burnup charts. | +| <a id="timeboxreporterror"></a>`error` | [`TimeboxReportError`](#timeboxreporterror) | If the report cannot be generated, information about why. | | <a id="timeboxreportstats"></a>`stats` | [`TimeReportStats`](#timereportstats) | Represents the time report stats for the timebox. | +### `TimeboxReportError` + +Explains why we could not generate a timebox report. + +#### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| <a id="timeboxreporterrorcode"></a>`code` | [`TimeboxReportErrorReason`](#timeboxreporterrorreason) | Machine readable code, categorizing the error. | +| <a id="timeboxreporterrormessage"></a>`message` | [`String`](#string) | Human readable message explaining what happened. | + ### `TimelineEventType` Describes an incident management timeline event. @@ -18711,8 +18723,8 @@ Values for sorting issues. | <a id="issuesortcreated_desc"></a>`CREATED_DESC` | Created at descending order. | | <a id="issuesortdue_date_asc"></a>`DUE_DATE_ASC` | Due date by ascending order. | | <a id="issuesortdue_date_desc"></a>`DUE_DATE_DESC` | Due date by descending order. | -| <a id="issuesortescalation_status_asc"></a>`ESCALATION_STATUS_ASC` | Status from triggered to resolved. Defaults to `CREATED_DESC` if `incident_escalations` feature flag is disabled. | -| <a id="issuesortescalation_status_desc"></a>`ESCALATION_STATUS_DESC` | Status from resolved to triggered. Defaults to `CREATED_DESC` if `incident_escalations` feature flag is disabled. | +| <a id="issuesortescalation_status_asc"></a>`ESCALATION_STATUS_ASC` | Status from triggered to resolved. | +| <a id="issuesortescalation_status_desc"></a>`ESCALATION_STATUS_DESC` | Status from resolved to triggered. | | <a id="issuesortlabel_priority_asc"></a>`LABEL_PRIORITY_ASC` | Label priority by ascending order. | | <a id="issuesortlabel_priority_desc"></a>`LABEL_PRIORITY_DESC` | Label priority by descending order. | | <a id="issuesortmilestone_due_asc"></a>`MILESTONE_DUE_ASC` | Milestone due date by ascending order. | @@ -19416,6 +19428,30 @@ State of a test report. | <a id="testreportstatefailed"></a>`FAILED` | Failed test report. | | <a id="testreportstatepassed"></a>`PASSED` | Passed test report. | +### `TimeboxReportErrorReason` + +Category of error. + +| Value | Description | +| ----- | ----------- | +| <a id="timeboxreporterrorreasoncreated_asc"></a>`CREATED_ASC` | Created at ascending order. | +| <a id="timeboxreporterrorreasoncreated_desc"></a>`CREATED_DESC` | Created at descending order. | +| <a id="timeboxreporterrorreasonlabel_priority_asc"></a>`LABEL_PRIORITY_ASC` | Label priority by ascending order. | +| <a id="timeboxreporterrorreasonlabel_priority_desc"></a>`LABEL_PRIORITY_DESC` | Label priority by descending order. | +| <a id="timeboxreporterrorreasonmilestone_due_asc"></a>`MILESTONE_DUE_ASC` | Milestone due date by ascending order. | +| <a id="timeboxreporterrorreasonmilestone_due_desc"></a>`MILESTONE_DUE_DESC` | Milestone due date by descending order. | +| <a id="timeboxreporterrorreasonmissing_dates"></a>`MISSING_DATES` | One or both of start_date and due_date is missing. | +| <a id="timeboxreporterrorreasonpriority_asc"></a>`PRIORITY_ASC` | Priority by ascending order. | +| <a id="timeboxreporterrorreasonpriority_desc"></a>`PRIORITY_DESC` | Priority by descending order. | +| <a id="timeboxreporterrorreasontoo_many_events"></a>`TOO_MANY_EVENTS` | There are too many events. | +| <a id="timeboxreporterrorreasonunsupported"></a>`UNSUPPORTED` | This type does not support timebox reports. | +| <a id="timeboxreporterrorreasonupdated_asc"></a>`UPDATED_ASC` | Updated at ascending order. | +| <a id="timeboxreporterrorreasonupdated_desc"></a>`UPDATED_DESC` | Updated at descending order. | +| <a id="timeboxreporterrorreasoncreated_asc"></a>`created_asc` **{warning-solid}** | **Deprecated** in 13.5. This was renamed. Use: `CREATED_ASC`. | +| <a id="timeboxreporterrorreasoncreated_desc"></a>`created_desc` **{warning-solid}** | **Deprecated** in 13.5. This was renamed. Use: `CREATED_DESC`. | +| <a id="timeboxreporterrorreasonupdated_asc"></a>`updated_asc` **{warning-solid}** | **Deprecated** in 13.5. This was renamed. Use: `UPDATED_ASC`. | +| <a id="timeboxreporterrorreasonupdated_desc"></a>`updated_desc` **{warning-solid}** | **Deprecated** in 13.5. This was renamed. Use: `UPDATED_DESC`. | + ### `TodoActionEnum` | Value | Description | diff --git a/doc/operations/incident_management/incidents.md b/doc/operations/incident_management/incidents.md index b4880e39db5..e48127b3415 100644 --- a/doc/operations/incident_management/incidents.md +++ b/doc/operations/incident_management/incidents.md @@ -257,11 +257,7 @@ Add a to-do for incidents that you want to track in your to-do list. Select > - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/5716) in GitLab 14.9 [with a flag](../../administration/feature_flags.md) named `incident_escalations`. Disabled by default. > - [Enabled on GitLab.com and self-managed](https://gitlab.com/gitlab-org/gitlab/-/issues/345769) in GitLab 14.10. - -FLAG: -This feature is available by default. To disable it per project or for your entire -instance, ask an administrator to [disable the feature flag](../../administration/feature_flags.md) -named `incident_escalations`. +> - [Feature flag `incident_escalations`](https://gitlab.com/gitlab-org/gitlab/-/issues/345769) removed in GitLab 15.1. For users with the Developer role or higher, select **Edit** in the **Status** section of the right-hand side bar of an incident, then select a status. **Triggered** is the default status for @@ -281,11 +277,7 @@ updating the incident status also updates the alert status. > - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/5716) in GitLab 14.9 [with a flag](../../administration/feature_flags.md) named `incident_escalations`. Disabled by default. > - [Enabled on GitLab.com and self-managed](https://gitlab.com/gitlab-org/gitlab/-/issues/345769) in GitLab 14.10. - -FLAG: -This feature is available by default. To disable it per project or for your entire -instance, ask an administrator to [disable the feature flag](../../administration/feature_flags.md) -named `incident_escalations`. +> - [Feature flag `incident_escalations`](https://gitlab.com/gitlab-org/gitlab/-/issues/345769) removed in GitLab 15.1. For users with the Developer role or higher, select **Edit** in the **Escalation policy** section of the right-hand side bar of an incident, then select a policy. By default, new incidents do not have diff --git a/doc/operations/incident_management/paging.md b/doc/operations/incident_management/paging.md index 420454d8d8a..936c297e555 100644 --- a/doc/operations/incident_management/paging.md +++ b/doc/operations/incident_management/paging.md @@ -52,11 +52,7 @@ or stop alert escalations by [updating the alert's status](alerts.md#update-an-a > - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/5716) in GitLab 14.9 [with a flag](../../administration/feature_flags.md) named `incident_escalations`. Disabled by default. > - [Enabled on GitLab.com and self-managed](https://gitlab.com/gitlab-org/gitlab/-/issues/345769) in GitLab 14.10. - -FLAG: -This feature is available by default. To disable it per project or for your entire -instance, ask an administrator to [disable the feature flag](../../administration/feature_flags.md) -named `incident_escalations`. +> - [Feature flag `incident_escalations`](https://gitlab.com/gitlab-org/gitlab/-/issues/345769) removed in GitLab 15.1. For incidents, paging on-call responders is optional for each individual incident. To begin escalating the incident, [set the incident's escalation policy](incidents.md#change-escalation-policy). diff --git a/lib/api/terraform/state.rb b/lib/api/terraform/state.rb index 35716fb0fae..b727fbd9f65 100644 --- a/lib/api/terraform/state.rb +++ b/lib/api/terraform/state.rb @@ -13,6 +13,7 @@ module API default_format :json rescue_from( + ::Terraform::RemoteStateHandler::StateDeletedError, ::ActiveRecord::RecordNotUnique, ::PG::UniqueViolation ) do |e| @@ -81,7 +82,7 @@ module API authorize! :admin_terraform_state, user_project remote_state_handler.handle_with_lock do |state| - state.destroy! + ::Terraform::States::TriggerDestroyService.new(state, current_user: current_user).execute end body false diff --git a/lib/gitlab/ci/templates/Jobs/DAST-Default-Branch-Deploy.gitlab-ci.yml b/lib/gitlab/ci/templates/Jobs/DAST-Default-Branch-Deploy.gitlab-ci.yml index 04b1c4a6f73..6a6fc2cb702 100644 --- a/lib/gitlab/ci/templates/Jobs/DAST-Default-Branch-Deploy.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Jobs/DAST-Default-Branch-Deploy.gitlab-ci.yml @@ -1,5 +1,5 @@ variables: - DAST_AUTO_DEPLOY_IMAGE_VERSION: 'v2.25.0' + DAST_AUTO_DEPLOY_IMAGE_VERSION: 'v2.28.2' .dast-auto-deploy: image: "registry.gitlab.com/gitlab-org/cluster-integration/auto-deploy-image:${DAST_AUTO_DEPLOY_IMAGE_VERSION}" diff --git a/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml b/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml index c29b5b74bfc..98c4216679f 100644 --- a/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml @@ -1,5 +1,5 @@ variables: - AUTO_DEPLOY_IMAGE_VERSION: 'v2.25.0' + AUTO_DEPLOY_IMAGE_VERSION: 'v2.28.2' .auto-deploy: image: "registry.gitlab.com/gitlab-org/cluster-integration/auto-deploy-image:${AUTO_DEPLOY_IMAGE_VERSION}" diff --git a/lib/gitlab/ci/templates/Jobs/Deploy.latest.gitlab-ci.yml b/lib/gitlab/ci/templates/Jobs/Deploy.latest.gitlab-ci.yml index d09bb53a5b1..603be5b1cdb 100644 --- a/lib/gitlab/ci/templates/Jobs/Deploy.latest.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Jobs/Deploy.latest.gitlab-ci.yml @@ -1,5 +1,5 @@ variables: - AUTO_DEPLOY_IMAGE_VERSION: 'v2.25.0' + AUTO_DEPLOY_IMAGE_VERSION: 'v2.28.2' .auto-deploy: image: "registry.gitlab.com/gitlab-org/cluster-integration/auto-deploy-image:${AUTO_DEPLOY_IMAGE_VERSION}" diff --git a/lib/gitlab/database/load_balancing/load_balancer.rb b/lib/gitlab/database/load_balancing/load_balancer.rb index d369376a6c4..40b76a1c028 100644 --- a/lib/gitlab/database/load_balancing/load_balancer.rb +++ b/lib/gitlab/database/load_balancing/load_balancer.rb @@ -104,12 +104,24 @@ module Gitlab # Yields a connection that can be used for both reads and writes. def read_write connection = nil + transaction_open = nil # In the event of a failover the primary may be briefly unavailable. # Instead of immediately grinding to a halt we'll retry the operation # a few times. retry_with_backoff do connection = pool.connection + transaction_open = connection.transaction_open? + yield connection + rescue StandardError => e + if transaction_open && connection_error?(e) + ::Gitlab::Database::LoadBalancing::Logger.warn( + event: :transaction_leak, + message: 'A write transaction has leaked during database fail-over' + ) + end + + raise e end end diff --git a/lib/tasks/gitlab/db.rake b/lib/tasks/gitlab/db.rake index 068dc463d16..5eb83346e90 100644 --- a/lib/tasks/gitlab/db.rake +++ b/lib/tasks/gitlab/db.rake @@ -131,14 +131,6 @@ namespace :gitlab do end end - desc 'GitLab | DB | Sets up EE specific database functionality' - - if Gitlab.ee? - task setup_ee: %w[db:drop:geo db:create:geo db:schema:load:geo db:migrate:geo] - else - task :setup_ee - end - desc 'This adjusts and cleans db/structure.sql - it runs after db:structure:dump' task :clean_structure_sql do |task_name| ActiveRecord::Base.configurations.configs_for(env_name: ActiveRecord::Tasks::DatabaseTasks.env).each do |db_config| diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 55cc184d89e..ce47193b965 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -37427,6 +37427,9 @@ msgstr "" msgid "Terraform|Cancel" msgstr "" +msgid "Terraform|Cannot remove a locked state" +msgstr "" + msgid "Terraform|Copy Terraform init command" msgstr "" @@ -37520,7 +37523,7 @@ msgstr "" msgid "Terraform|You are about to remove the state file %{name}. This will permanently delete all the State versions and history. The infrastructure provisioned previously will remain intact, and only the state file with all its versions will be removed. This action cannot be undone." msgstr "" -msgid "Terraform|You cannot remove the State file because it's locked. Unlock the State file first before removing it." +msgid "Terraform|You have insufficient permissions to delete this state" msgstr "" msgid "Terraform|Your project doesn't have any Terraform state files" diff --git a/scripts/utils.sh b/scripts/utils.sh index ae071b98b43..41a8234466e 100644 --- a/scripts/utils.sh +++ b/scripts/utils.sh @@ -67,7 +67,7 @@ function setup_db_praefect() { function setup_db() { run_timed_command "setup_db_user_only" - run_timed_command_with_metric "bundle exec rake db:drop db:create db:structure:load db:migrate gitlab:db:setup_ee" "setup_db" + run_timed_command_with_metric "bundle exec rake db:drop db:create db:schema:load db:migrate" "setup_db" run_timed_command "setup_db_praefect" } diff --git a/spec/controllers/projects/incidents_controller_spec.rb b/spec/controllers/projects/incidents_controller_spec.rb index 20cf0dcfd3a..460821634b0 100644 --- a/spec/controllers/projects/incidents_controller_spec.rb +++ b/spec/controllers/projects/incidents_controller_spec.rb @@ -43,7 +43,6 @@ RSpec.describe Projects::IncidentsController do expect(response).to have_gitlab_http_status(:ok) expect(response).to render_template(:index) - expect(Gon.features).to include('incidentEscalations' => true) end context 'when user is unauthorized' do diff --git a/spec/factories/terraform/state.rb b/spec/factories/terraform/state.rb index fb63c845073..c40fa14b4f8 100644 --- a/spec/factories/terraform/state.rb +++ b/spec/factories/terraform/state.rb @@ -12,6 +12,10 @@ FactoryBot.define do locked_by_user { association(:user) } end + trait :deletion_in_progress do + deleted_at { Time.current } + end + trait :with_version do after(:create) do |state| create(:terraform_state_version, terraform_state: state) diff --git a/spec/features/incidents/incidents_list_spec.rb b/spec/features/incidents/incidents_list_spec.rb index 789cc89e083..3241e71f537 100644 --- a/spec/features/incidents/incidents_list_spec.rb +++ b/spec/features/incidents/incidents_list_spec.rb @@ -44,18 +44,5 @@ RSpec.describe 'Incident Management index', :js do expect(table).to have_content('Date created') expect(table).to have_content('Assignees') end - - context 'when :incident_escalations feature is disabled' do - before do - stub_feature_flags(incident_escalations: false) - end - - it 'does not include the Status columns' do - visit project_incidents_path(project) - wait_for_requests - - expect(page.find('.gl-table')).not_to have_content('Status') - end - end end end diff --git a/spec/frontend/incidents/components/incidents_list_spec.js b/spec/frontend/incidents/components/incidents_list_spec.js index a556f3c17f3..356480f931e 100644 --- a/spec/frontend/incidents/components/incidents_list_spec.js +++ b/spec/frontend/incidents/components/incidents_list_spec.js @@ -85,7 +85,6 @@ describe('Incidents List', () => { assigneeUsernameQuery: '', slaFeatureAvailable: true, canCreateIncident: true, - incidentEscalationsAvailable: true, ...provide, }, stubs: { @@ -211,20 +210,6 @@ describe('Incidents List', () => { expect(status.classes('gl-text-truncate')).toBe(true); }); }); - - describe('when feature is disabled', () => { - beforeEach(() => { - mountComponent({ - data: { incidents: { list: mockIncidents }, incidentsCount }, - provide: { incidentEscalationsAvailable: false }, - loading: false, - }); - }); - - it('is absent if feature flag is disabled', () => { - expect(findEscalationStatus().length).toBe(0); - }); - }); }); it('contains a link to the incident details page', async () => { diff --git a/spec/graphql/mutations/issues/set_escalation_status_spec.rb b/spec/graphql/mutations/issues/set_escalation_status_spec.rb index d41118b1812..f04d396efb8 100644 --- a/spec/graphql/mutations/issues/set_escalation_status_spec.rb +++ b/spec/graphql/mutations/issues/set_escalation_status_spec.rb @@ -50,16 +50,6 @@ RSpec.describe Mutations::Issues::SetEscalationStatus do expect { result }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable, 'Feature unavailable for provided issue') end end - - context 'with feature disabled' do - before do - stub_feature_flags(incident_escalations: false) - end - - it 'raises an error' do - expect { result }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable, 'Feature unavailable for provided issue') - end - end end end end diff --git a/spec/graphql/mutations/terraform/state/delete_spec.rb b/spec/graphql/mutations/terraform/state/delete_spec.rb index 313a85a4bac..66d4b50741f 100644 --- a/spec/graphql/mutations/terraform/state/delete_spec.rb +++ b/spec/graphql/mutations/terraform/state/delete_spec.rb @@ -34,12 +34,12 @@ RSpec.describe Mutations::Terraform::State::Delete do state.project.add_maintainer(user) end - it 'deletes the state', :aggregate_failures do - expect do - expect(subject).to eq(errors: []) - end.to change { ::Terraform::State.count }.by(-1) + it 'schedules the state for deletion', :aggregate_failures do + expect_next_instance_of(Terraform::States::TriggerDestroyService, state, current_user: user) do |service| + expect(service).to receive(:execute).once.and_return(ServiceResponse.success) + end - expect { state.reload }.to raise_error(ActiveRecord::RecordNotFound) + subject end end diff --git a/spec/graphql/resolvers/issues_resolver_spec.rb b/spec/graphql/resolvers/issues_resolver_spec.rb index e6ec9d8c895..1f2e2aec023 100644 --- a/spec/graphql/resolvers/issues_resolver_spec.rb +++ b/spec/graphql/resolvers/issues_resolver_spec.rb @@ -573,22 +573,6 @@ RSpec.describe Resolvers::IssuesResolver do issues = resolve_issues(sort: :created_desc).to_a expect(issues).to eq([resolved_incident, issue_no_status, triggered_incident]) end - - context 'when incident_escalations feature flag is disabled' do - before do - stub_feature_flags(incident_escalations: false) - end - - it 'defaults ascending status sort to created_desc' do - issues = resolve_issues(sort: :escalation_status_asc).to_a - expect(issues).to eq([resolved_incident, issue_no_status, triggered_incident]) - end - - it 'defaults descending status sort to created_desc' do - issues = resolve_issues(sort: :escalation_status_desc).to_a - expect(issues).to eq([resolved_incident, issue_no_status, triggered_incident]) - end - end end context 'when sorting with non-stable cursors' do diff --git a/spec/graphql/types/issue_type_spec.rb b/spec/graphql/types/issue_type_spec.rb index 920aadd2faf..e7454b85357 100644 --- a/spec/graphql/types/issue_type_spec.rb +++ b/spec/graphql/types/issue_type_spec.rb @@ -291,14 +291,6 @@ RSpec.describe GitlabSchema.types['Issue'] do let!(:escalation_status) { create(:incident_management_issuable_escalation_status, issue: issue) } it { is_expected.to eq(escalation_status.status_name.to_s.upcase) } - - context 'with feature disabled' do - before do - stub_feature_flags(incident_escalations: false) - end - - it { is_expected.to be_nil } - end end end end diff --git a/spec/lib/gitlab/database/load_balancing/transaction_leaking_spec.rb b/spec/lib/gitlab/database/load_balancing/transaction_leaking_spec.rb new file mode 100644 index 00000000000..30e5fbbd803 --- /dev/null +++ b/spec/lib/gitlab/database/load_balancing/transaction_leaking_spec.rb @@ -0,0 +1,81 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Load balancer behavior with errors inside a transaction', :redis, :delete do + let(:model) { ApplicationRecord } + let(:db_host) { model.connection_pool.db_config.host } + + let(:test_table_name) { '_test_foo' } + + before do + # Patch in our load balancer config, simply pointing at the test database twice + allow(Gitlab::Database::LoadBalancing::Configuration).to receive(:for_model) do |base_model| + Gitlab::Database::LoadBalancing::Configuration.new(base_model, [db_host, db_host]) + end + + Gitlab::Database::LoadBalancing::Setup.new(ApplicationRecord).setup + + model.connection.execute(<<~SQL) + CREATE TABLE IF NOT EXISTS #{test_table_name} (id SERIAL PRIMARY KEY, value INTEGER) + SQL + end + + after do + model.connection.execute(<<~SQL) + DROP TABLE IF EXISTS #{test_table_name} + SQL + end + + def execute(conn) + conn.execute("INSERT INTO #{test_table_name} (value) VALUES (1)") + backend_pid = conn.execute("SELECT pg_backend_pid() AS pid").to_a.first['pid'] + + # This will result in a PG error, which is not raised. + # Instead, we retry the statement on a fresh connection (where the pid is different and it does nothing) + # and the load balancer continues with a fresh connection and no transaction if a transaction was open previously + conn.execute(<<~SQL) + SELECT CASE + WHEN pg_backend_pid() = #{backend_pid} THEN + pg_terminate_backend(#{backend_pid}) + END + SQL + + # This statement will execute on a new connection, and violate transaction semantics + # if we were in a transaction before + conn.execute("INSERT INTO #{test_table_name} (value) VALUES (2)") + end + + it 'logs a warning when violating transaction semantics with writes' do + conn = model.connection + + expect(::Gitlab::Database::LoadBalancing::Logger).to receive(:warn).with(hash_including(event: :transaction_leak)) + + conn.transaction do + expect(conn).to be_transaction_open + + execute(conn) + + expect(conn).not_to be_transaction_open + end + + values = conn.execute("SELECT value FROM #{test_table_name}").to_a.map { |row| row['value'] } + expect(values).to contain_exactly(2) # Does not include 1 because the transaction was aborted and leaked + end + + it 'does not log a warning when no transaction is open to be leaked' do + conn = model.connection + + expect(::Gitlab::Database::LoadBalancing::Logger) + .not_to receive(:warn).with(hash_including(event: :transaction_leak)) + + expect(conn).not_to be_transaction_open + + execute(conn) + + expect(conn).not_to be_transaction_open + + values = conn.execute("SELECT value FROM #{test_table_name}").to_a.map { |row| row['value'] } + expect(values).to contain_exactly(1, 2) # Includes both rows because there was no transaction to roll back + end +end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index e8e9c263d23..87821de3cf5 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -982,14 +982,6 @@ RSpec.describe Issuable do subject { issuable.supports_escalation? } it { is_expected.to eq(supports_escalation) } - - context 'with feature disabled' do - before do - stub_feature_flags(incident_escalations: false) - end - - it { is_expected.to eq(false) } - end end end diff --git a/spec/models/merge_request/cleanup_schedule_spec.rb b/spec/models/merge_request/cleanup_schedule_spec.rb index 85208f901fd..9c50b64f2bd 100644 --- a/spec/models/merge_request/cleanup_schedule_spec.rb +++ b/spec/models/merge_request/cleanup_schedule_spec.rb @@ -120,6 +120,41 @@ RSpec.describe MergeRequest::CleanupSchedule do end end + describe '.stuck' do + let!(:cleanup_schedule_1) { create(:merge_request_cleanup_schedule, updated_at: 1.day.ago) } + let!(:cleanup_schedule_2) { create(:merge_request_cleanup_schedule, :running, updated_at: 5.hours.ago) } + let!(:cleanup_schedule_3) { create(:merge_request_cleanup_schedule, :running, updated_at: 7.hours.ago) } + let!(:cleanup_schedule_4) { create(:merge_request_cleanup_schedule, :completed, updated_at: 1.day.ago) } + let!(:cleanup_schedule_5) { create(:merge_request_cleanup_schedule, :failed, updated_at: 1.day.ago) } + + it 'returns records that has been in running state for more than 6 hours' do + expect(described_class.stuck).to match_array([cleanup_schedule_3]) + end + end + + describe '.stuck_retry!' do + let!(:cleanup_schedule_1) { create(:merge_request_cleanup_schedule, :running, updated_at: 5.hours.ago) } + let!(:cleanup_schedule_2) { create(:merge_request_cleanup_schedule, :running, updated_at: 7.hours.ago) } + + it 'sets stuck records to unstarted' do + expect { described_class.stuck_retry! }.to change { cleanup_schedule_2.reload.unstarted? }.from(false).to(true) + end + + context 'when there are more than 5 stuck schedules' do + before do + create_list(:merge_request_cleanup_schedule, 5, :running, updated_at: 1.day.ago) + end + + it 'only retries 5 stuck schedules at once' do + expect(described_class.stuck.count).to eq 6 + + described_class.stuck_retry! + + expect(described_class.stuck.count).to eq 1 + end + end + end + describe '.start_next' do let!(:cleanup_schedule_1) { create(:merge_request_cleanup_schedule, :completed, scheduled_at: 1.day.ago) } let!(:cleanup_schedule_2) { create(:merge_request_cleanup_schedule, scheduled_at: 2.days.ago) } diff --git a/spec/models/terraform/state_spec.rb b/spec/models/terraform/state_spec.rb index a113ae37203..a484952bfe9 100644 --- a/spec/models/terraform/state_spec.rb +++ b/spec/models/terraform/state_spec.rb @@ -11,8 +11,6 @@ RSpec.describe Terraform::State do it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_presence_of(:project_id) } - it { is_expected.to validate_uniqueness_of(:name).scoped_to(:project_id) } - describe 'scopes' do describe '.ordered_by_name' do let_it_be(:project) { create(:project) } @@ -40,22 +38,6 @@ RSpec.describe Terraform::State do end end - describe '#destroy' do - let(:terraform_state) { create(:terraform_state) } - let(:user) { terraform_state.project.creator } - - it 'deletes when the state is unlocked' do - expect(terraform_state.destroy).to be_truthy - end - - it 'fails to delete when the state is locked', :aggregate_failures do - terraform_state.update!(lock_xid: SecureRandom.uuid, locked_by_user: user, locked_at: Time.current) - - expect(terraform_state.destroy).to be_falsey - expect(terraform_state.errors.full_messages).to eq(["You cannot remove the State file because it's locked. Unlock the State file first before removing it."]) - end - end - describe '#latest_file' do let(:terraform_state) { create(:terraform_state, :with_version) } let(:latest_version) { terraform_state.latest_version } diff --git a/spec/models/terraform/state_version_spec.rb b/spec/models/terraform/state_version_spec.rb index 7af9b7897ff..22b1397f30a 100644 --- a/spec/models/terraform/state_version_spec.rb +++ b/spec/models/terraform/state_version_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' RSpec.describe Terraform::StateVersion do it { is_expected.to be_a FileStoreMounter } + it { is_expected.to be_a EachBatch } it { is_expected.to belong_to(:terraform_state).required } it { is_expected.to belong_to(:created_by_user).class_name('User').optional } diff --git a/spec/requests/api/graphql/mutations/issues/set_escalation_status_spec.rb b/spec/requests/api/graphql/mutations/issues/set_escalation_status_spec.rb index 0166871502b..a81364d37b2 100644 --- a/spec/requests/api/graphql/mutations/issues/set_escalation_status_spec.rb +++ b/spec/requests/api/graphql/mutations/issues/set_escalation_status_spec.rb @@ -49,14 +49,6 @@ RSpec.describe 'Setting the escalation status of an incident' do it_behaves_like 'a mutation that returns top-level errors', errors: ['Feature unavailable for provided issue'] end - context 'with feature disabled' do - before do - stub_feature_flags(incident_escalations: false) - end - - it_behaves_like 'a mutation that returns top-level errors', errors: ['Feature unavailable for provided issue'] - end - it 'sets given escalation_policy to the escalation status for the issue' do post_graphql_mutation(mutation, current_user: current_user) diff --git a/spec/requests/api/graphql/mutations/user_preferences/update_spec.rb b/spec/requests/api/graphql/mutations/user_preferences/update_spec.rb index 85194e6eb20..e1c7fd9d60d 100644 --- a/spec/requests/api/graphql/mutations/user_preferences/update_spec.rb +++ b/spec/requests/api/graphql/mutations/user_preferences/update_spec.rb @@ -28,17 +28,6 @@ RSpec.describe Mutations::UserPreferences::Update do expect(current_user.user_preference.persisted?).to eq(true) expect(current_user.user_preference.issues_sort).to eq(Types::IssueSortEnum.values[sort_value].value.to_s) end - - context 'when incident_escalations feature flag is disabled' do - let(:sort_value) { 'ESCALATION_STATUS_ASC' } - - before do - stub_feature_flags(incident_escalations: false) - end - - it_behaves_like 'a mutation that returns top-level errors', - errors: ['Feature flag `incident_escalations` must be enabled to use this sort order.'] - end end context 'when user has existing preference' do @@ -56,16 +45,5 @@ RSpec.describe Mutations::UserPreferences::Update do expect(current_user.user_preference.issues_sort).to eq(Types::IssueSortEnum.values[sort_value].value.to_s) end - - context 'when incident_escalations feature flag is disabled' do - let(:sort_value) { 'ESCALATION_STATUS_DESC' } - - before do - stub_feature_flags(incident_escalations: false) - end - - it_behaves_like 'a mutation that returns top-level errors', - errors: ['Feature flag `incident_escalations` must be enabled to use this sort order.'] - end end end diff --git a/spec/requests/api/graphql/terraform/state/delete_spec.rb b/spec/requests/api/graphql/terraform/state/delete_spec.rb index 35927d03b49..ba0619ea611 100644 --- a/spec/requests/api/graphql/terraform/state/delete_spec.rb +++ b/spec/requests/api/graphql/terraform/state/delete_spec.rb @@ -12,12 +12,12 @@ RSpec.describe 'delete a terraform state' do let(:mutation) { graphql_mutation(:terraform_state_delete, id: state.to_global_id.to_s) } before do + expect_next_instance_of(Terraform::States::TriggerDestroyService, state, current_user: user) do |service| + expect(service).to receive(:execute).once.and_return(ServiceResponse.success) + end + post_graphql_mutation(mutation, current_user: user) end include_examples 'a working graphql query' - - it 'deletes the state' do - expect { state.reload }.to raise_error(ActiveRecord::RecordNotFound) - end end diff --git a/spec/requests/api/terraform/state_spec.rb b/spec/requests/api/terraform/state_spec.rb index f49e799cf5d..e8458db4a4d 100644 --- a/spec/requests/api/terraform/state_spec.rb +++ b/spec/requests/api/terraform/state_spec.rb @@ -71,6 +71,18 @@ RSpec.describe API::Terraform::State, :snowplow do end end + shared_context 'cannot access a state that is scheduled for deletion' do + before do + state.update!(deleted_at: Time.current) + end + + it 'returns unprocessable entity' do + request + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + end + end + describe 'GET /projects/:id/terraform/state/:name' do subject(:request) { get api(state_path), headers: auth_header } @@ -106,6 +118,8 @@ RSpec.describe API::Terraform::State, :snowplow do expect(response).to have_gitlab_http_status(:not_found) end end + + it_behaves_like 'cannot access a state that is scheduled for deletion' end context 'with developer permissions' do @@ -191,6 +205,8 @@ RSpec.describe API::Terraform::State, :snowplow do expect(response).to have_gitlab_http_status(:unprocessable_entity) end end + + it_behaves_like 'cannot access a state that is scheduled for deletion' end context 'without body' do @@ -269,13 +285,19 @@ RSpec.describe API::Terraform::State, :snowplow do context 'with maintainer permissions' do let(:current_user) { maintainer } + let(:deletion_service) { instance_double(Terraform::States::TriggerDestroyService) } - it 'deletes the state and returns empty body' do - expect { request }.to change { Terraform::State.count }.by(-1) + it 'schedules the state for deletion and returns empty body' do + expect(Terraform::States::TriggerDestroyService).to receive(:new).and_return(deletion_service) + expect(deletion_service).to receive(:execute).once + + request expect(response).to have_gitlab_http_status(:ok) expect(Gitlab::Json.parse(response.body)).to be_empty end + + it_behaves_like 'cannot access a state that is scheduled for deletion' end context 'with developer permissions' do @@ -305,6 +327,7 @@ RSpec.describe API::Terraform::State, :snowplow do subject(:request) { post api("#{state_path}/lock"), headers: auth_header, params: params } it_behaves_like 'endpoint with unique user tracking' + it_behaves_like 'cannot access a state that is scheduled for deletion' it 'locks the terraform state' do request @@ -359,6 +382,10 @@ RSpec.describe API::Terraform::State, :snowplow do let(:lock_id) { 'irrelevant to this test, just needs to be present' } end + it_behaves_like 'cannot access a state that is scheduled for deletion' do + let(:lock_id) { 'irrelevant to this test, just needs to be present' } + end + context 'with the correct lock id' do let(:lock_id) { '123-456' } diff --git a/spec/serializers/issue_sidebar_basic_entity_spec.rb b/spec/serializers/issue_sidebar_basic_entity_spec.rb index 564ffb1aea9..64a271e359a 100644 --- a/spec/serializers/issue_sidebar_basic_entity_spec.rb +++ b/spec/serializers/issue_sidebar_basic_entity_spec.rb @@ -59,16 +59,6 @@ RSpec.describe IssueSidebarBasicEntity do expect(entity[:current_user][:can_update_escalation_status]).to be(false) end end - - context 'with :incident_escalations feature flag disabled' do - before do - stub_feature_flags(incident_escalations: false) - end - - it 'is not present' do - expect(entity[:current_user]).not_to include(:can_update_escalation_status) - end - end end end end diff --git a/spec/services/alert_management/alerts/update_service_spec.rb b/spec/services/alert_management/alerts/update_service_spec.rb index f02607b8174..9bdc9970807 100644 --- a/spec/services/alert_management/alerts/update_service_spec.rb +++ b/spec/services/alert_management/alerts/update_service_spec.rb @@ -291,14 +291,6 @@ RSpec.describe AlertManagement::Alerts::UpdateService do it_behaves_like 'does not sync with the incident status' end - - context 'when feature flag is disabled' do - before do - stub_feature_flags(incident_escalations: false) - end - - it_behaves_like 'does not sync with the incident status' - end end end end diff --git a/spec/services/incident_management/issuable_escalation_statuses/prepare_update_service_spec.rb b/spec/services/incident_management/issuable_escalation_statuses/prepare_update_service_spec.rb index 25164df40ca..6c99631fcb0 100644 --- a/spec/services/incident_management/issuable_escalation_statuses/prepare_update_service_spec.rb +++ b/spec/services/incident_management/issuable_escalation_statuses/prepare_update_service_spec.rb @@ -42,14 +42,6 @@ RSpec.describe IncidentManagement::IssuableEscalationStatuses::PrepareUpdateServ it_behaves_like 'successful response', { status_event: :acknowledge } - context 'when feature flag is disabled' do - before do - stub_feature_flags(incident_escalations: false) - end - - it_behaves_like 'availability error response' - end - context 'when user is anonymous' do let(:current_user) { nil } diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index d496857bb25..d11fe772023 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -1230,14 +1230,6 @@ RSpec.describe Issues::UpdateService, :mailer do it_behaves_like 'updates the escalation status record', :acknowledged end - - context 'with :incident_escalations feature flag disabled' do - before do - stub_feature_flags(incident_escalations: false) - end - - it_behaves_like 'does not change the status record' - end end context 'when issue type is not incident' do diff --git a/spec/services/terraform/remote_state_handler_spec.rb b/spec/services/terraform/remote_state_handler_spec.rb index ca392849d49..19c1d4109e9 100644 --- a/spec/services/terraform/remote_state_handler_spec.rb +++ b/spec/services/terraform/remote_state_handler_spec.rb @@ -33,6 +33,14 @@ RSpec.describe Terraform::RemoteStateHandler do it 'returns the state' do expect(subject.find_with_lock).to eq(state) end + + context 'with a state scheduled for deletion' do + let!(:state) { create(:terraform_state, :deletion_in_progress, project: project, name: 'state') } + + it 'raises an exception' do + expect { subject.find_with_lock }.to raise_error(described_class::StateDeletedError) + end + end end end end @@ -84,6 +92,13 @@ RSpec.describe Terraform::RemoteStateHandler do .to raise_error(described_class::StateLockedError) end + it 'raises an exception if the state is scheduled for deletion' do + create(:terraform_state, :deletion_in_progress, project: project, name: 'new-state') + + expect { handler.handle_with_lock } + .to raise_error(described_class::StateDeletedError) + end + context 'user does not have permission to modify state' do let(:user) { developer } @@ -127,24 +142,28 @@ RSpec.describe Terraform::RemoteStateHandler do expect { handler.lock! }.to raise_error(described_class::StateLockedError) end + + it 'raises an exception when the state exists and is scheduled for deletion' do + create(:terraform_state, :deletion_in_progress, project: project, name: 'new-state') + + expect { handler.lock! }.to raise_error(described_class::StateDeletedError) + end end describe '#unlock!' do - let(:lock_id) { 'abc-abc' } + let_it_be(:state) { create(:terraform_state, :locked, project: project, name: 'new-state', lock_xid: 'abc-abc') } + + let(:lock_id) { state.lock_xid } subject(:handler) do described_class.new( project, user, - name: 'new-state', + name: state.name, lock_id: lock_id ) end - before do - create(:terraform_state, :locked, project: project, name: 'new-state', lock_xid: 'abc-abc') - end - it 'unlocks the state' do state = handler.unlock! @@ -169,6 +188,15 @@ RSpec.describe Terraform::RemoteStateHandler do .to raise_error(described_class::StateLockedError) end end + + context 'with a state scheduled for deletion' do + it 'raises an exception' do + state.update!(deleted_at: Time.current) + + expect { handler.unlock! } + .to raise_error(described_class::StateDeletedError) + end + end end end end diff --git a/spec/services/terraform/states/destroy_service_spec.rb b/spec/services/terraform/states/destroy_service_spec.rb new file mode 100644 index 00000000000..16fcc8f93db --- /dev/null +++ b/spec/services/terraform/states/destroy_service_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Terraform::States::DestroyService do + let_it_be(:state) { create(:terraform_state, :with_version, :deletion_in_progress) } + let_it_be(:file) { double } + + before do + allow_next_found_instance_of(Terraform::StateVersion) do |version| + allow(version).to receive(:file).and_return(file) + end + end + + describe '#execute' do + subject { described_class.new(state).execute } + + it 'removes version files from object storage, followed by the state record' do + expect(file).to receive(:remove!).once + expect(state).to receive(:destroy!) + + subject + end + + context 'state is not marked for deletion' do + let(:state) { create(:terraform_state) } + + it 'does not delete the state' do + expect(state).not_to receive(:destroy!) + + subject + end + end + end +end diff --git a/spec/services/terraform/states/trigger_destroy_service_spec.rb b/spec/services/terraform/states/trigger_destroy_service_spec.rb new file mode 100644 index 00000000000..2e96331779c --- /dev/null +++ b/spec/services/terraform/states/trigger_destroy_service_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Terraform::States::TriggerDestroyService do + let_it_be(:project) { create(:project) } + let_it_be(:user) { create(:user, maintainer_projects: [project]) } + + describe '#execute', :aggregate_failures do + let_it_be(:state) { create(:terraform_state, project: project) } + + subject { described_class.new(state, current_user: user).execute } + + it 'marks the state as deleted and schedules a cleanup worker' do + expect(Terraform::States::DestroyWorker).to receive(:perform_async).with(state.id).once + + expect(subject).to be_success + expect(state.deleted_at).to be_like_time(Time.current) + end + + shared_examples 'unable to delete state' do + it 'does not modify the state' do + expect(Terraform::States::DestroyWorker).not_to receive(:perform_async) + + expect { subject }.not_to change(state, :deleted_at) + expect(subject).to be_error + expect(subject.message).to eq(message) + end + end + + context 'user does not have permission' do + let(:user) { create(:user, developer_projects: [project]) } + let(:message) { 'You have insufficient permissions to delete this state' } + + include_examples 'unable to delete state' + end + + context 'state is locked' do + let(:state) { create(:terraform_state, :locked, project: project) } + let(:message) { 'Cannot remove a locked state' } + + include_examples 'unable to delete state' + end + end +end diff --git a/spec/workers/schedule_merge_request_cleanup_refs_worker_spec.rb b/spec/workers/schedule_merge_request_cleanup_refs_worker_spec.rb index ef515e43474..49730d9ab8c 100644 --- a/spec/workers/schedule_merge_request_cleanup_refs_worker_spec.rb +++ b/spec/workers/schedule_merge_request_cleanup_refs_worker_spec.rb @@ -25,6 +25,12 @@ RSpec.describe ScheduleMergeRequestCleanupRefsWorker do end end + it 'retries stuck cleanup schedules' do + expect(MergeRequest::CleanupSchedule).to receive(:stuck_retry!) + + worker.perform + end + include_examples 'an idempotent worker' do it 'schedules MergeRequestCleanupRefsWorker to be performed with capacity' do expect(MergeRequestCleanupRefsWorker).to receive(:perform_with_capacity).twice diff --git a/spec/workers/terraform/states/destroy_worker_spec.rb b/spec/workers/terraform/states/destroy_worker_spec.rb new file mode 100644 index 00000000000..02e79373279 --- /dev/null +++ b/spec/workers/terraform/states/destroy_worker_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Terraform::States::DestroyWorker do + let(:state) { create(:terraform_state) } + + describe '#perform' do + let(:state_id) { state.id } + let(:deletion_service) { instance_double(Terraform::States::DestroyService, execute: true) } + + subject { described_class.new.perform(state_id) } + + it 'calls the deletion service' do + expect(deletion_service).to receive(:execute).once + expect(Terraform::States::DestroyService).to receive(:new) + .with(state).and_return(deletion_service) + + subject + end + + context 'state no longer exists' do + let(:state_id) { -1 } + + it 'completes without error' do + expect { subject }.not_to raise_error + end + end + end +end |