diff --git a/app/controllers/projects/hooks_controller.rb b/app/controllers/projects/hooks_controller.rb index 22b6bf6faf0..4dcc9a3a43f 100644 --- a/app/controllers/projects/hooks_controller.rb +++ b/app/controllers/projects/hooks_controller.rb @@ -4,7 +4,8 @@ class Projects::HooksController < Projects::ApplicationController include ::WebHooks::HookActions # Authorize - before_action :authorize_admin_project! + before_action :authorize_admin_project!, except: :destroy + before_action :authorize_destroy_project_hook!, only: :destroy before_action :hook_logs, only: :edit before_action -> { check_rate_limit!(:project_testing_hook, scope: [@project, current_user]) }, only: :test @@ -41,4 +42,8 @@ class Projects::HooksController < Projects::ApplicationController def trigger_values ProjectHook.triggers.values end + + def authorize_destroy_project_hook! + render_404 unless can?(current_user, :destroy_web_hook, hook) + end end diff --git a/app/models/import_failure.rb b/app/models/import_failure.rb index 109c0c82487..0ca99faeb71 100644 --- a/app/models/import_failure.rb +++ b/app/models/import_failure.rb @@ -6,6 +6,7 @@ class ImportFailure < ApplicationRecord validates :project, presence: true, unless: :group validates :group, presence: true, unless: :project + validates :external_identifiers, json_schema: { filename: "import_failure_external_identifiers" } # Returns any `import_failures` for relations that were unrecoverable errors or failed after # several retries. An import can be successful even if some relations failed to import correctly. @@ -13,4 +14,8 @@ class ImportFailure < ApplicationRecord scope :hard_failures_by_correlation_id, ->(correlation_id) { where(correlation_id_value: correlation_id, retry_count: 0).order(created_at: :desc) } + + scope :failures_by_correlation_id, ->(correlation_id) { + where(correlation_id_value: correlation_id).order(created_at: :desc) + } end diff --git a/app/policies/project_hook_policy.rb b/app/policies/project_hook_policy.rb index c177fabb1ba..b4590c13670 100644 --- a/app/policies/project_hook_policy.rb +++ b/app/policies/project_hook_policy.rb @@ -1,10 +1,9 @@ # frozen_string_literal: true class ProjectHookPolicy < ::BasePolicy - delegate(:project) + delegate { @subject.project } rule { can?(:admin_project) }.policy do - enable :read_web_hook enable :destroy_web_hook end end diff --git a/app/validators/json_schemas/import_failure_external_identifiers.json b/app/validators/json_schemas/import_failure_external_identifiers.json new file mode 100644 index 00000000000..3756e712de5 --- /dev/null +++ b/app/validators/json_schemas/import_failure_external_identifiers.json @@ -0,0 +1,18 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "description": "Import failure external identifiers", + "type": "object", + "maxProperties": 3, + "patternProperties": { + ".*": { + "oneOf": [ + { + "type": "string" + }, + { + "type": "integer" + } + ] + } + } +} diff --git a/app/workers/concerns/gitlab/github_import/object_importer.rb b/app/workers/concerns/gitlab/github_import/object_importer.rb index 7a6f30b8cc7..7e488862696 100644 --- a/app/workers/concerns/gitlab/github_import/object_importer.rb +++ b/app/workers/concerns/gitlab/github_import/object_importer.rb @@ -13,10 +13,23 @@ module Gitlab sidekiq_options retry: 3 include GithubImport::Queue include ReschedulingMethods - include Gitlab::NotifyUponDeath feature_category :importers worker_has_external_dependencies! + + sidekiq_retries_exhausted do |msg| + args = msg['args'] + correlation_id = msg['correlation_id'] + jid = msg['jid'] + + new.perform_failure(args[0], args[1], correlation_id) + + # If a job is being exhausted we still want to notify the + # Gitlab::Import::AdvanceStageWorker to prevent the entire import from getting stuck + if args.length == 3 && (key = args.last) && key.is_a?(String) + JobWaiter.notify(key, jid) + end + end end NotRetriableError = Class.new(StandardError) @@ -51,11 +64,25 @@ module Gitlab track_and_raise_exception(project, e, fail_import: true) rescue ActiveRecord::RecordInvalid, NotRetriableError => e # We do not raise exception to prevent job retry - track_exception(project, e) + failure = track_exception(project, e) + add_identifiers_to_failure(failure, object.github_identifiers) rescue StandardError => e track_and_raise_exception(project, e) end + # hash - A Hash containing the details of the object to import. + def perform_failure(project_id, hash, correlation_id) + project = Project.find_by_id(project_id) + return unless project + + failure = project.import_failures.failures_by_correlation_id(correlation_id).first + return unless failure + + object = representation_class.from_json_hash(hash) + + add_identifiers_to_failure(failure, object.github_identifiers) + end + def increment_object_counter?(_object) true end @@ -105,6 +132,10 @@ module Gitlab raise(exception) end + + def add_identifiers_to_failure(failure, external_identifiers) + failure.update_column(:external_identifiers, external_identifiers) + end end end end diff --git a/config/metrics/counts_28d/20210216175101_merge_requests_users.yml b/config/metrics/counts_28d/20210216175101_merge_requests_users.yml index 07ad35e45bf..3029047f4ad 100644 --- a/config/metrics/counts_28d/20210216175101_merge_requests_users.yml +++ b/config/metrics/counts_28d/20210216175101_merge_requests_users.yml @@ -17,7 +17,5 @@ tier: - free - premium - ultimate -performance_indicator_type: -- gmau -- paid_gmau +performance_indicator_type: [] milestone: "<13.9" diff --git a/config/metrics/counts_28d/20210216175132_i_code_review_user_create_mr_monthly.yml b/config/metrics/counts_28d/20210216175132_i_code_review_user_create_mr_monthly.yml index a6bfeed7059..d4411c5e153 100644 --- a/config/metrics/counts_28d/20210216175132_i_code_review_user_create_mr_monthly.yml +++ b/config/metrics/counts_28d/20210216175132_i_code_review_user_create_mr_monthly.yml @@ -1,5 +1,5 @@ --- -data_category: optional +data_category: operational key_path: redis_hll_counters.code_review.i_code_review_user_create_mr_monthly description: Count of unique users per month who created a MR product_section: dev diff --git a/config/metrics/counts_28d/20210216175552_ci_pipeline_schedules.yml b/config/metrics/counts_28d/20210216175552_ci_pipeline_schedules.yml index 8a57e5989c4..ac55cb82f8f 100644 --- a/config/metrics/counts_28d/20210216175552_ci_pipeline_schedules.yml +++ b/config/metrics/counts_28d/20210216175552_ci_pipeline_schedules.yml @@ -1,5 +1,5 @@ --- -data_category: optional +data_category: operational key_path: usage_activity_by_stage_monthly.verify.ci_pipeline_schedules description: Distinct users creating pipeline schedules in a month product_section: ops diff --git a/config/metrics/counts_28d/20210216180312_snippets.yml b/config/metrics/counts_28d/20210216180312_snippets.yml index dfe24039f52..616368ec513 100644 --- a/config/metrics/counts_28d/20210216180312_snippets.yml +++ b/config/metrics/counts_28d/20210216180312_snippets.yml @@ -1,5 +1,5 @@ --- -data_category: optional +data_category: operational key_path: counts_monthly.snippets description: Monthly count of All Snippets product_section: dev diff --git a/config/metrics/counts_28d/20210216180334_g_edit_by_sfe_monthly.yml b/config/metrics/counts_28d/20210216180334_g_edit_by_sfe_monthly.yml index 78622d0fc18..1eda8cb2467 100644 --- a/config/metrics/counts_28d/20210216180334_g_edit_by_sfe_monthly.yml +++ b/config/metrics/counts_28d/20210216180334_g_edit_by_sfe_monthly.yml @@ -1,5 +1,5 @@ --- -data_category: optional +data_category: operational key_path: redis_hll_counters.ide_edit.g_edit_by_sfe_monthly description: Number of users editing a file from the single file editor product_section: dev diff --git a/config/metrics/counts_28d/20210216184458_p_ci_templates_implicit_auto_devops_monthly.yml b/config/metrics/counts_28d/20210216184458_p_ci_templates_implicit_auto_devops_monthly.yml index 43a8a2340eb..c219ea4dbfd 100644 --- a/config/metrics/counts_28d/20210216184458_p_ci_templates_implicit_auto_devops_monthly.yml +++ b/config/metrics/counts_28d/20210216184458_p_ci_templates_implicit_auto_devops_monthly.yml @@ -1,5 +1,5 @@ --- -data_category: optional +data_category: operational key_path: redis_hll_counters.ci_templates.p_ci_templates_implicit_auto_devops_monthly description: Count of pipelines with implicit Auto DevOps runs product_section: ops diff --git a/config/metrics/counts_28d/20210427102618_code_review_category_monthly_active_users.yml b/config/metrics/counts_28d/20210427102618_code_review_category_monthly_active_users.yml index 8a0005a5c5c..802fc03bee4 100644 --- a/config/metrics/counts_28d/20210427102618_code_review_category_monthly_active_users.yml +++ b/config/metrics/counts_28d/20210427102618_code_review_category_monthly_active_users.yml @@ -16,7 +16,9 @@ tier: - free - premium - ultimate -performance_indicator_type: [] +performance_indicator_type: +- gmau +- paid_gmau time_frame: 28d instrumentation_class: AggregatedMetric data_source: redis_hll diff --git a/config/metrics/counts_28d/20221213182900_i_code_review_create_mr_monthly.yml b/config/metrics/counts_28d/20221213182900_i_code_review_create_mr_monthly.yml index dca8545691a..e081e74b967 100644 --- a/config/metrics/counts_28d/20221213182900_i_code_review_create_mr_monthly.yml +++ b/config/metrics/counts_28d/20221213182900_i_code_review_create_mr_monthly.yml @@ -11,7 +11,7 @@ milestone: "15.7" introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/106869 time_frame: 28d data_source: redis_hll -data_category: optional +data_category: operational instrumentation_class: RedisHLLMetric performance_indicator_type: [] options: diff --git a/config/metrics/counts_28d/20230217215050_ci_internal_pipelines.yml b/config/metrics/counts_28d/20230217215050_ci_internal_pipelines.yml index 8c482d8fe49..5d927562f42 100644 --- a/config/metrics/counts_28d/20230217215050_ci_internal_pipelines.yml +++ b/config/metrics/counts_28d/20230217215050_ci_internal_pipelines.yml @@ -11,7 +11,7 @@ milestone: "15.10" introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/112370 time_frame: 28d data_source: database -data_category: optional +data_category: operational instrumentation_class: CountCiInternalPipelinesMetric performance_indicator_type: [] distribution: diff --git a/config/metrics/counts_7d/20221213183300_i_code_review_create_mr_weekly.yml b/config/metrics/counts_7d/20221213183300_i_code_review_create_mr_weekly.yml index 43405d5bd2c..0ab553a7b5c 100644 --- a/config/metrics/counts_7d/20221213183300_i_code_review_create_mr_weekly.yml +++ b/config/metrics/counts_7d/20221213183300_i_code_review_create_mr_weekly.yml @@ -11,7 +11,7 @@ milestone: "15.7" introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/106869 time_frame: 7d data_source: redis_hll -data_category: optional +data_category: operational instrumentation_class: RedisHLLMetric performance_indicator_type: [] options: diff --git a/config/metrics/counts_all/20210216180242_web_ide_commits.yml b/config/metrics/counts_all/20210216180242_web_ide_commits.yml index f86b5bd5f84..44585ed6916 100644 --- a/config/metrics/counts_all/20210216180242_web_ide_commits.yml +++ b/config/metrics/counts_all/20210216180242_web_ide_commits.yml @@ -1,5 +1,5 @@ --- -data_category: optional +data_category: operational key_path: counts.web_ide_commits description: Count of commits made from the Web IDE product_section: dev diff --git a/db/migrate/20230309000957_add_external_identifiers_to_import_failures.rb b/db/migrate/20230309000957_add_external_identifiers_to_import_failures.rb new file mode 100644 index 00000000000..f95cf0035d4 --- /dev/null +++ b/db/migrate/20230309000957_add_external_identifiers_to_import_failures.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddExternalIdentifiersToImportFailures < Gitlab::Database::Migration[2.1] + enable_lock_retries! + + def change + add_column :import_failures, :external_identifiers, :jsonb, default: {}, null: false + end +end diff --git a/db/schema_migrations/20230309000957 b/db/schema_migrations/20230309000957 new file mode 100644 index 00000000000..679d37f153b --- /dev/null +++ b/db/schema_migrations/20230309000957 @@ -0,0 +1 @@ +902e921099ed27cc1c8fd36eac192879ff6c68e4aa7ef4a0764381c0a01fd76e \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 5126a322387..5fc3ebcb69c 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -16753,7 +16753,8 @@ CREATE TABLE import_failures ( exception_message character varying(255), retry_count integer, group_id integer, - source character varying(128) + source character varying(128), + external_identifiers jsonb DEFAULT '{}'::jsonb NOT NULL ); CREATE SEQUENCE import_failures_id_seq diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index ae174fe2d7e..783e2f539f4 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -6365,6 +6365,7 @@ Input type: `VulnerabilityResolveInput` | Name | Type | Description | | ---- | ---- | ----------- | | `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| `comment` | [`String`](#string) | Comment why vulnerability was reverted to detected (max. 50 000 characters). | | `id` | [`VulnerabilityID!`](#vulnerabilityid) | ID of the vulnerability to be resolved. | #### Fields @@ -21619,6 +21620,19 @@ Represents the vulnerability details location within a file in the project. | `name` | [`String`](#string) | Name of the field. | | `offset` | [`Int!`](#int) | Offset of the module location. | +### `VulnerabilityDetailRow` + +Represents an individual row in a table. + +#### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `description` | [`String`](#string) | Description of the field. | +| `fieldName` | [`String`](#string) | Name of the field. | +| `name` | [`String`](#string) | Name of the field. | +| `row` | [`[VulnerabilityDetail!]!`](#vulnerabilitydetail) | Value of the field. | + ### `VulnerabilityDetailTable` Represents the vulnerability details table value. @@ -21631,7 +21645,7 @@ Represents the vulnerability details table value. | `fieldName` | [`String`](#string) | Name of the field. | | `headers` | [`[VulnerabilityDetail!]!`](#vulnerabilitydetail) | Table headers. | | `name` | [`String`](#string) | Name of the field. | -| `rows` | [`[VulnerabilityDetail!]!`](#vulnerabilitydetail) | Table rows. | +| `rows` | [`[VulnerabilityDetailRow!]!`](#vulnerabilitydetailrow) | Table rows. | ### `VulnerabilityDetailText` diff --git a/doc/development/service_ping/implement.md b/doc/development/service_ping/implement.md index 306ffa5d6e7..080aead9776 100644 --- a/doc/development/service_ping/implement.md +++ b/doc/development/service_ping/implement.md @@ -496,24 +496,12 @@ We have the following recommendations for [adding new events](#add-new-events): - Event aggregation: weekly. - When adding new metrics, use a [feature flag](../../operations/feature_flags.md) to control the impact. -- For feature flags triggered by another service, set `default_enabled: false`, - - Events can be triggered using the `UsageData` API, which helps when there are > 10 events per change +It's recommended to disable the new feature flag by default (set `default_enabled: false`). +- Events can be triggered using the `UsageData` API, which helps when there are > 10 events per change ##### Enable or disable Redis HLL tracking -Events are tracked behind optional [feature flags](../feature_flags/index.md) due to concerns for Redis performance and scalability. - -For a full list of events and corresponding feature flags, see the [`known_events/`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/usage_data_counters/known_events/) files. - -To enable or disable tracking for specific event in or , run commands such as the following to -[enable or disable the corresponding feature](../feature_flags/index.md). - -```shell -/chatops run feature set true -/chatops run feature set false -``` - -We can also disable tracking completely by using the global flag: +We can disable tracking completely by using the global flag: ```shell /chatops run feature set redis_hll_tracking true diff --git a/lib/gitlab/import/import_failure_service.rb b/lib/gitlab/import/import_failure_service.rb index 2f6ffc4c844..8257f9f5db4 100644 --- a/lib/gitlab/import/import_failure_service.rb +++ b/lib/gitlab/import/import_failure_service.rb @@ -50,10 +50,10 @@ module Gitlab def execute track_exception - persist_failure - - import_state.mark_as_failed(exception.message) if fail_import - track_metrics if metrics + persist_failure.tap do + import_state.mark_as_failed(exception.message) if fail_import + track_metrics if metrics + end end private diff --git a/lib/gitlab/usage_data_counters/hll_redis_counter.rb b/lib/gitlab/usage_data_counters/hll_redis_counter.rb index 8d170a9412a..4c54aff73c0 100644 --- a/lib/gitlab/usage_data_counters/hll_redis_counter.rb +++ b/lib/gitlab/usage_data_counters/hll_redis_counter.rb @@ -27,7 +27,6 @@ module Gitlab # - name: g_compliance_dashboard # Unique event name # redis_slot: compliance # Optional slot name, if not defined it will use name as a slot, used for totals # aggregation: daily # Aggregation level, keys are stored daily or weekly - # feature_flag: # The event feature flag # # Usage: # @@ -97,7 +96,7 @@ module Gitlab Gitlab::ErrorTracking.track_and_raise_for_dev_exception(UnknownEvent.new("Unknown event #{event_name}")) unless event.present? return if event.blank? - return unless feature_enabled?(event) + return unless Feature.enabled?(:redis_hll_tracking, type: :ops) Gitlab::Redis::HLL.add(key: redis_key(event, time, context), value: values, expiry: expiry(event)) rescue StandardError => e @@ -125,12 +124,6 @@ module Gitlab redis_usage_data { Gitlab::Redis::HLL.count(keys: keys) } end - def feature_enabled?(event) - return true if event[:feature_flag].blank? - - Feature.enabled?(event[:feature_flag]) && Feature.enabled?(:redis_hll_tracking, type: :ops) - end - def keys_for_aggregation(aggregation, events:, start_date:, end_date:, context: '') if aggregation.to_sym == :daily daily_redis_keys(events: events, start_date: start_date, end_date: end_date, context: context) diff --git a/rubocop/cop/gitlab/mark_used_feature_flags.rb b/rubocop/cop/gitlab/mark_used_feature_flags.rb index ffd59c8fffc..f466ab87aa0 100644 --- a/rubocop/cop/gitlab/mark_used_feature_flags.rb +++ b/rubocop/cop/gitlab/mark_used_feature_flags.rb @@ -32,12 +32,6 @@ module RuboCop RESTRICT_ON_SEND = FEATURE_METHODS + SELF_METHODS - USAGE_DATA_COUNTERS_EVENTS_YAML_GLOBS = [ - File.expand_path("../../../config/metrics/aggregates/*.yml", __dir__), - File.expand_path("../../../lib/gitlab/usage_data_counters/known_events/*.yml", __dir__), - File.expand_path("../../../ee/lib/ee/gitlab/usage_data_counters/known_events/*.yml", __dir__) - ].freeze - class << self # We track feature flags in `on_new_investigation` only once per # rubocop whole run instead once per file. @@ -52,8 +46,6 @@ module RuboCop return if self.class.feature_flags_already_tracked self.class.feature_flags_already_tracked = true - - track_usage_data_counters_known_events! end def on_casgn(node) @@ -184,22 +176,6 @@ module RuboCop feature_method?(node) || self_method?(node) end - # Marking all event's feature flags as used as Gitlab::UsageDataCounters::HLLRedisCounter.track_event{,context} - # is mostly used with dynamic event name. - def track_usage_data_counters_known_events! - usage_data_counters_known_event_feature_flags.each { |feature_flag_name| save_used_feature_flag(feature_flag_name) } - end - - def usage_data_counters_known_event_feature_flags - USAGE_DATA_COUNTERS_EVENTS_YAML_GLOBS.each_with_object(Set.new) do |glob, memo| - Dir.glob(glob).each do |path| - YAML.safe_load(File.read(path))&.each do |hash| - memo << hash['feature_flag'] if hash['feature_flag'] - end - end - end - end - def defined_feature_flags @defined_feature_flags ||= begin flags_paths = [ diff --git a/spec/controllers/projects/hooks_controller_spec.rb b/spec/controllers/projects/hooks_controller_spec.rb index 92a7be18c28..c056e7a33aa 100644 --- a/spec/controllers/projects/hooks_controller_spec.rb +++ b/spec/controllers/projects/hooks_controller_spec.rb @@ -173,6 +173,16 @@ RSpec.describe Projects::HooksController, feature_category: :integrations do let(:params) { { namespace_id: project.namespace, project_id: project, id: hook } } it_behaves_like 'Web hook destroyer' + + context 'when user does not have permission' do + let(:user) { create(:user, developer_projects: [project]) } + + it 'renders a 404' do + delete :destroy, params: params + + expect(response).to have_gitlab_http_status(:not_found) + end + end end describe '#test' do diff --git a/spec/lib/gitlab/database/tables_locker_spec.rb b/spec/lib/gitlab/database/tables_locker_spec.rb index 1c4d6ddbf3c..30f0f9376c8 100644 --- a/spec/lib/gitlab/database/tables_locker_spec.rb +++ b/spec/lib/gitlab/database/tables_locker_spec.rb @@ -33,7 +33,7 @@ RSpec.describe Gitlab::Database::TablesLocker, :suppress_gitlab_schemas_validate Ci::ApplicationRecord.connection.execute(create_partition_sql) create_detached_partition_sql = <<~SQL - CREATE TABLE IF NOT EXISTS gitlab_partitions_dynamic._test_gitlab_main_part_202201 ( + CREATE TABLE IF NOT EXISTS #{Gitlab::Database::DYNAMIC_PARTITIONS_SCHEMA}._test_gitlab_main_part_202201 ( id bigserial primary key not null ) SQL @@ -47,6 +47,12 @@ RSpec.describe Gitlab::Database::TablesLocker, :suppress_gitlab_schemas_validate drop_after: Time.current ) end + Gitlab::Database::SharedModel.using_connection(Ci::ApplicationRecord.connection) do + Postgresql::DetachedPartition.create!( + table_name: '_test_gitlab_main_part_20220101', + drop_after: Time.current + ) + end end shared_examples "lock tables" do |gitlab_schema, database_name| @@ -106,7 +112,7 @@ RSpec.describe Gitlab::Database::TablesLocker, :suppress_gitlab_schemas_validate end end - shared_examples "lock attached partitions" do |partition_identifier, database_name| + shared_examples "lock partitions" do |partition_identifier, database_name| let(:connection) { Gitlab::Database.database_base_models[database_name].connection } it 'locks the partition' do @@ -126,7 +132,7 @@ RSpec.describe Gitlab::Database::TablesLocker, :suppress_gitlab_schemas_validate end end - shared_examples "unlock attached partitions" do |partition_identifier, database_name| + shared_examples "unlock partitions" do |partition_identifier, database_name| let(:connection) { Gitlab::Database.database_base_models[database_name].connection } it 'unlocks the partition' do @@ -204,8 +210,12 @@ RSpec.describe Gitlab::Database::TablesLocker, :suppress_gitlab_schemas_validate it_behaves_like 'unlock tables', :gitlab_internal, 'ci' gitlab_main_partition = "#{Gitlab::Database::DYNAMIC_PARTITIONS_SCHEMA}.security_findings_test_partition" - it_behaves_like 'unlock attached partitions', gitlab_main_partition, 'main' - it_behaves_like 'lock attached partitions', gitlab_main_partition, 'ci' + it_behaves_like 'unlock partitions', gitlab_main_partition, 'main' + it_behaves_like 'lock partitions', gitlab_main_partition, 'ci' + + gitlab_main_detached_partition = "#{Gitlab::Database::DYNAMIC_PARTITIONS_SCHEMA}._test_gitlab_main_part_20220101" + it_behaves_like 'unlock partitions', gitlab_main_detached_partition, 'main' + it_behaves_like 'lock partitions', gitlab_main_detached_partition, 'ci' end describe '#unlock_writes' do @@ -221,8 +231,12 @@ RSpec.describe Gitlab::Database::TablesLocker, :suppress_gitlab_schemas_validate it_behaves_like "unlock tables", :gitlab_internal, 'ci' gitlab_main_partition = "#{Gitlab::Database::DYNAMIC_PARTITIONS_SCHEMA}.security_findings_test_partition" - it_behaves_like 'unlock attached partitions', gitlab_main_partition, 'main' - it_behaves_like 'unlock attached partitions', gitlab_main_partition, 'ci' + it_behaves_like 'unlock partitions', gitlab_main_partition, 'main' + it_behaves_like 'unlock partitions', gitlab_main_partition, 'ci' + + gitlab_main_detached_partition = "#{Gitlab::Database::DYNAMIC_PARTITIONS_SCHEMA}._test_gitlab_main_part_20220101" + it_behaves_like 'unlock partitions', gitlab_main_detached_partition, 'main' + it_behaves_like 'unlock partitions', gitlab_main_detached_partition, 'ci' end context 'when running in dry_run mode' do diff --git a/spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb b/spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb index b7be257246f..e35184544eb 100644 --- a/spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb +++ b/spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb @@ -50,8 +50,6 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s end describe 'known_events' do - let(:feature) { 'test_hll_redis_counter_ff_check' } - let(:weekly_event) { 'g_analytics_contribution' } let(:daily_event) { 'g_analytics_search' } let(:analytics_slot_event) { 'g_analytics_contribution' } @@ -71,7 +69,7 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s let(:known_events) do [ - { name: weekly_event, redis_slot: "analytics", aggregation: "weekly", feature_flag: feature }, + { name: weekly_event, redis_slot: "analytics", aggregation: "weekly" }, { name: daily_event, redis_slot: "analytics", aggregation: "daily" }, { name: category_productivity_event, redis_slot: "analytics", aggregation: "weekly" }, { name: compliance_slot_event, redis_slot: "compliance", aggregation: "weekly" }, @@ -106,32 +104,6 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s end end - context 'with event feature flag set' do - it 'tracks the event when feature enabled' do - stub_feature_flags(feature => true) - - expect(Gitlab::Redis::HLL).to receive(:add) - - described_class.track_event(weekly_event, values: 1) - end - - it 'does not track the event with feature flag disabled' do - stub_feature_flags(feature => false) - - expect(Gitlab::Redis::HLL).not_to receive(:add) - - described_class.track_event(weekly_event, values: 1) - end - end - - context 'with no event feature flag set' do - it 'tracks the event' do - expect(Gitlab::Redis::HLL).to receive(:add) - - described_class.track_event(daily_event, values: 1) - end - end - context 'when usage_ping is disabled' do it 'does not track the event' do allow(::ServicePing::ServicePingSettings).to receive(:enabled?).and_return(false) diff --git a/spec/models/import_failure_spec.rb b/spec/models/import_failure_spec.rb index 9fee1b0ae7b..0bdcb6dde31 100644 --- a/spec/models/import_failure_spec.rb +++ b/spec/models/import_failure_spec.rb @@ -10,7 +10,11 @@ RSpec.describe ImportFailure do let_it_be(:soft_failure) { create(:import_failure, :soft_failure, project: project, correlation_id_value: correlation_id) } let_it_be(:unrelated_failure) { create(:import_failure, project: project) } - it 'returns hard failures given a correlation ID' do + it 'returns failures for the given correlation ID' do + expect(ImportFailure.failures_by_correlation_id(correlation_id)).to match_array([hard_failure, soft_failure]) + end + + it 'returns hard failures for the given correlation ID' do expect(ImportFailure.hard_failures_by_correlation_id(correlation_id)).to eq([hard_failure]) end @@ -45,5 +49,15 @@ RSpec.describe ImportFailure do it { is_expected.to validate_presence_of(:group) } end + + describe '#external_identifiers' do + it { is_expected.to allow_value({ note_id: 234, noteable_id: 345, noteable_type: 'MergeRequest' }).for(:external_identifiers) } + it { is_expected.not_to allow_value(nil).for(:external_identifiers) } + it { is_expected.not_to allow_value({ ids: [123] }).for(:external_identifiers) } + + it 'allows up to 3 fields' do + is_expected.not_to allow_value({ note_id: 234, noteable_id: 345, noteable_type: 'MergeRequest', extra_attribute: 'abc' }).for(:external_identifiers) + end + end end end diff --git a/spec/policies/project_hook_policy_spec.rb b/spec/policies/project_hook_policy_spec.rb index 60b296e131a..a71940c319e 100644 --- a/spec/policies/project_hook_policy_spec.rb +++ b/spec/policies/project_hook_policy_spec.rb @@ -15,7 +15,7 @@ RSpec.describe ProjectHookPolicy, feature_category: :integrations do end it "cannot read and destroy web-hooks" do - expect(policy).to be_disallowed(:read_web_hook, :destroy_web_hook) + expect(policy).to be_disallowed(:destroy_web_hook) end end @@ -25,7 +25,7 @@ RSpec.describe ProjectHookPolicy, feature_category: :integrations do end it "can read and destroy web-hooks" do - expect(policy).to be_allowed(:read_web_hook, :destroy_web_hook) + expect(policy).to be_allowed(:destroy_web_hook) end end end diff --git a/spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb b/spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb index bfc0cebe203..9d550d9c56e 100644 --- a/spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb +++ b/spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb @@ -205,14 +205,4 @@ RSpec.describe RuboCop::Cop::Gitlab::MarkUsedFeatureFlags do include_examples 'sets flag as used', 'deduplicate :delayed, feature_flag: :foo', 'foo' include_examples 'does not set any flags as used', 'deduplicate :delayed' end - - describe "tracking of usage data metrics known events happens at the beginning of inspection" do - let(:usage_data_counters_known_event_feature_flags) { ['an_event_feature_flag'] } - - before do - allow(cop).to receive(:usage_data_counters_known_event_feature_flags).and_return(usage_data_counters_known_event_feature_flags) - end - - include_examples 'sets flag as used', "FEATURE_FLAG = :foo", %w[foo an_event_feature_flag] - end end diff --git a/spec/workers/concerns/gitlab/github_import/object_importer_spec.rb b/spec/workers/concerns/gitlab/github_import/object_importer_spec.rb index 02190201986..b9de8341852 100644 --- a/spec/workers/concerns/gitlab/github_import/object_importer_spec.rb +++ b/spec/workers/concerns/gitlab/github_import/object_importer_spec.rb @@ -196,6 +196,19 @@ RSpec.describe Gitlab::GithubImport::ObjectImporter, :aggregate_failures do end context 'when the record is invalid' do + let(:exception) { ActiveRecord::RecordInvalid.new } + + before do + expect(importer_class) + .to receive(:new) + .with(instance_of(MockRepresantation), project, client) + .and_return(importer_instance) + + expect(importer_instance) + .to receive(:execute) + .and_raise(exception) + end + it 'logs an error' do expect(Gitlab::GithubImport::Logger) .to receive(:info) @@ -208,16 +221,6 @@ RSpec.describe Gitlab::GithubImport::ObjectImporter, :aggregate_failures do } ) - expect(importer_class) - .to receive(:new) - .with(instance_of(MockRepresantation), project, client) - .and_return(importer_instance) - - exception = ActiveRecord::RecordInvalid.new - expect(importer_instance) - .to receive(:execute) - .and_raise(exception) - expect(Gitlab::Import::ImportFailureService) .to receive(:track) .with( @@ -230,6 +233,15 @@ RSpec.describe Gitlab::GithubImport::ObjectImporter, :aggregate_failures do worker.import(project, client, { 'number' => 10, 'github_id' => 1 }) end + + it 'updates external_identifiers of the correct failure' do + worker.import(project, client, { 'number' => 10, 'github_id' => 1 }) + + import_failures = project.import_failures + + expect(import_failures.count).to eq(1) + expect(import_failures.first.external_identifiers).to eq(github_identifiers.with_indifferent_access) + end end end @@ -240,4 +252,56 @@ RSpec.describe Gitlab::GithubImport::ObjectImporter, :aggregate_failures do expect(worker).to be_increment_object_counter(issue) end end + + describe '.sidekiq_retries_exhausted' do + let(:correlation_id) { 'abc' } + let(:job) do + { + 'args' => [project.id, { number: 123, state: 'open' }, '123abc'], + 'jid' => '123', + 'correlation_id' => correlation_id + } + end + + subject(:sidekiq_retries_exhausted) { worker.class.sidekiq_retries_exhausted_block.call(job, StandardError.new) } + + context 'when all arguments are given' do + it 'notifies the JobWaiter' do + expect(Gitlab::JobWaiter) + .to receive(:notify) + .with( + job['args'].last, + job['jid'] + ) + + sidekiq_retries_exhausted + end + end + + context 'when not all arguments are given' do + let(:job) do + { + 'args' => [project.id, { number: 123, state: 'open' }], + 'jid' => '123', + 'correlation_id' => correlation_id + } + end + + it 'does not notify the JobWaiter' do + expect(Gitlab::JobWaiter).not_to receive(:notify) + + sidekiq_retries_exhausted + end + end + + it 'updates external_identifiers of the correct failure' do + failure_1, failure_2 = create_list(:import_failure, 2, project: project) + failure_2.update_column(:correlation_id_value, correlation_id) + + sidekiq_retries_exhausted + + expect(failure_1.reload.external_identifiers).to be_empty + expect(failure_2.reload.external_identifiers).to eq(github_identifiers.with_indifferent_access) + end + end end