diff --git a/.rubocop_todo/layout/hash_alignment.yml b/.rubocop_todo/layout/hash_alignment.yml index 264619830f4..5d103dae210 100644 --- a/.rubocop_todo/layout/hash_alignment.yml +++ b/.rubocop_todo/layout/hash_alignment.yml @@ -233,58 +233,8 @@ Layout/HashAlignment: - 'ee/app/graphql/mutations/projects/set_locked.rb' - 'ee/app/graphql/resolvers/iterations/cadences_resolver.rb' - 'ee/app/graphql/resolvers/vulnerabilities_count_per_day_resolver.rb' - - 'ee/app/graphql/types/admin/cloud_licenses/current_license_type.rb' - - 'ee/app/graphql/types/admin/cloud_licenses/license_type.rb' - - 'ee/app/graphql/types/admin/cloud_licenses/subscription_future_entry_type.rb' - - 'ee/app/graphql/types/analytics/devops_adoption/enabled_namespace_type.rb' - - 'ee/app/graphql/types/analytics/devops_adoption/snapshot_type.rb' - - 'ee/app/graphql/types/app_sec/fuzzing/api/ci_configuration_type.rb' - - 'ee/app/graphql/types/app_sec/fuzzing/api/scan_profile_type.rb' - - 'ee/app/graphql/types/app_sec/fuzzing/coverage/corpus_type.rb' - - 'ee/app/graphql/types/boards/board_epic_type.rb' - - 'ee/app/graphql/types/boards/epic_board_type.rb' - - 'ee/app/graphql/types/boards/epic_list_metadata_type.rb' - - 'ee/app/graphql/types/boards/epic_list_type.rb' - - 'ee/app/graphql/types/boards/epic_user_preferences_type.rb' - - 'ee/app/graphql/types/burnup_chart_daily_totals_type.rb' - - 'ee/app/graphql/types/ci/code_coverage_activity_type.rb' - - 'ee/app/graphql/types/ci/code_coverage_summary_type.rb' - - 'ee/app/graphql/types/ci/code_quality_degradation_type.rb' - - 'ee/app/graphql/types/ci/minutes/namespace_monthly_usage_type.rb' - - 'ee/app/graphql/types/ci/minutes/project_monthly_usage_type.rb' - - 'ee/app/graphql/types/compliance_management/merge_requests/compliance_violation_type.rb' - - 'ee/app/graphql/types/dast/profile_branch_type.rb' - - 'ee/app/graphql/types/dast/profile_schedule_type.rb' - - 'ee/app/graphql/types/dast/profile_type.rb' - - 'ee/app/graphql/types/dast_scanner_profile_type.rb' - - 'ee/app/graphql/types/dast_site_profile_type.rb' - - 'ee/app/graphql/types/dast_site_validation_type.rb' - - 'ee/app/graphql/types/dora_metric_type.rb' - - 'ee/app/graphql/types/dora_type.rb' - - 'ee/app/graphql/types/epic_descendant_weight_sum_type.rb' - - 'ee/app/graphql/types/epic_issue_type.rb' - - 'ee/app/graphql/types/epic_type.rb' - - 'ee/app/graphql/types/external_issue_type.rb' - - 'ee/app/graphql/types/group_release_stats_type.rb' - - 'ee/app/graphql/types/instance_security_dashboard_type.rb' - - 'ee/app/graphql/types/iteration_type.rb' - - 'ee/app/graphql/types/iterations/cadence_type.rb' - - 'ee/app/graphql/types/merge_requests/approval_state_type.rb' - - 'ee/app/graphql/types/metric_image_type.rb' - - 'ee/app/graphql/types/path_lock_type.rb' - - 'ee/app/graphql/types/requirements_management/requirement_type.rb' - - 'ee/app/graphql/types/requirements_management/test_report_type.rb' - - 'ee/app/graphql/types/security/training_type.rb' - - 'ee/app/graphql/types/security/training_url_type.rb' - - 'ee/app/graphql/types/security_report_summary_type.rb' - - 'ee/app/graphql/types/security_scanners.rb' - - 'ee/app/graphql/types/time_report_stats_type.rb' - - 'ee/app/graphql/types/timebox_metrics_type.rb' - - 'ee/app/graphql/types/timebox_report_interface.rb' - - 'ee/app/graphql/types/timebox_report_type.rb' - 'ee/app/graphql/types/vulnerabilities/asset_type.rb' - 'ee/app/graphql/types/vulnerabilities/link_type.rb' - - 'ee/app/graphql/types/vulnerabilities_count_by_day_type.rb' - 'ee/app/graphql/types/vulnerability/external_issue_link_type.rb' - 'ee/app/graphql/types/vulnerability/issue_link_type.rb' - 'ee/app/graphql/types/vulnerability_details/base_type.rb' @@ -300,10 +250,6 @@ Layout/HashAlignment: - 'ee/app/graphql/types/vulnerability_details/table_type.rb' - 'ee/app/graphql/types/vulnerability_details/text_type.rb' - 'ee/app/graphql/types/vulnerability_details/url_type.rb' - - 'ee/app/graphql/types/vulnerability_evidence_source_type.rb' - - 'ee/app/graphql/types/vulnerability_evidence_supporting_message_type.rb' - - 'ee/app/graphql/types/vulnerability_evidence_type.rb' - - 'ee/app/graphql/types/vulnerability_identifier_type.rb' - 'ee/app/graphql/types/vulnerability_location/cluster_image_scanning_type.rb' - 'ee/app/graphql/types/vulnerability_location/container_scanning_type.rb' - 'ee/app/graphql/types/vulnerability_location/coverage_fuzzing_type.rb' @@ -313,15 +259,6 @@ Layout/HashAlignment: - 'ee/app/graphql/types/vulnerability_location/sast_type.rb' - 'ee/app/graphql/types/vulnerability_location/secret_detection_type.rb' - 'ee/app/graphql/types/vulnerability_request_response_header_type.rb' - - 'ee/app/graphql/types/vulnerability_request_type.rb' - - 'ee/app/graphql/types/vulnerability_response_type.rb' - - 'ee/app/graphql/types/vulnerability_scanner_type.rb' - - 'ee/app/graphql/types/vulnerability_severities_count_type.rb' - - 'ee/app/graphql/types/vulnerability_type.rb' - - 'ee/app/graphql/types/vulnerable_dependency_type.rb' - - 'ee/app/graphql/types/vulnerable_kubernetes_resource_type.rb' - - 'ee/app/graphql/types/vulnerable_package_type.rb' - - 'ee/app/graphql/types/vulnerable_projects_by_grade_type.rb' - 'ee/app/helpers/ee/feature_flags_helper.rb' - 'ee/app/helpers/ee/sorting_helper.rb' - 'ee/app/models/allowed_email_domain.rb' diff --git a/app/assets/javascripts/header_search/components/app.vue b/app/assets/javascripts/header_search/components/app.vue index 0c4f9640972..72fec17ac9d 100644 --- a/app/assets/javascripts/header_search/components/app.vue +++ b/app/assets/javascripts/header_search/components/app.vue @@ -262,9 +262,9 @@ export default {
-
+
+ element.addEventListener('keyup', EmailFormatValidator.eventHandler.bind(this)), + ); + } + + static eventHandler(event) { + const inputDomElement = event.target; + + EmailFormatValidator.setMessageVisibility(inputDomElement, hintMessageSelector); + EmailFormatValidator.setMessageVisibility(inputDomElement, warningMessageSelector); + EmailFormatValidator.validateEmailInput(inputDomElement); + } + + static validateEmailInput(inputDomElement) { + const validEmail = inputDomElement.checkValidity(); + const validPattern = inputDomElement.value.match(emailRegexPattern); + + EmailFormatValidator.setMessageVisibility( + inputDomElement, + warningMessageSelector, + validEmail && !validPattern, + ); + } + + static setMessageVisibility(inputDomElement, messageSelector, isVisible = false) { + const messageElement = inputDomElement.parentElement.querySelector(messageSelector); + messageElement.classList.toggle('hide', !isVisible); + } +} diff --git a/app/assets/stylesheets/pages/search.scss b/app/assets/stylesheets/pages/search.scss index f1865a7dc40..012407cf09c 100644 --- a/app/assets/stylesheets/pages/search.scss +++ b/app/assets/stylesheets/pages/search.scss @@ -121,11 +121,7 @@ input[type='checkbox']:hover { .header-search-dropdown-menu { max-height: $dropdown-max-height; - top: $header-height; -} - -.header-search-dropdown-content { - max-height: $dropdown-max-height; + top: 100%; } .search { diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index bb16c2d2098..33d2c482795 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -21,6 +21,7 @@ class RegistrationsController < Devise::RegistrationsController before_action only: [:new] do push_frontend_feature_flag(:gitlab_gtm_datalayer, type: :ops) + push_frontend_feature_flag(:trial_email_validation, type: :development) end feature_category :authentication_and_authorization diff --git a/app/models/commit_signatures/ssh_signature.rb b/app/models/commit_signatures/ssh_signature.rb index dbfbe0c3889..7a8d0653fcd 100644 --- a/app/models/commit_signatures/ssh_signature.rb +++ b/app/models/commit_signatures/ssh_signature.rb @@ -4,6 +4,6 @@ module CommitSignatures class SshSignature < ApplicationRecord include CommitSignature - belongs_to :key, optional: false + belongs_to :key, optional: true end end diff --git a/app/services/concerns/alert_management/alert_processing.rb b/app/services/concerns/alert_management/alert_processing.rb index f10ff4e6f19..347963ff642 100644 --- a/app/services/concerns/alert_management/alert_processing.rb +++ b/app/services/concerns/alert_management/alert_processing.rb @@ -39,12 +39,6 @@ module AlertManagement SystemNoteService.change_alert_status(alert, User.alert_bot) close_issue(alert.issue_id) if auto_close_incident? - else - logger.warn( - message: 'Unable to update AlertManagement::Alert status to resolved', - project_id: project.id, - alert_id: alert.id - ) end end @@ -66,9 +60,10 @@ module AlertManagement SystemNoteService.create_new_alert(alert, alert_source) else logger.warn( - message: "Unable to create AlertManagement::Alert from #{alert_source}", + message: "Unable to create AlertManagement::Alert", project_id: project.id, - alert_errors: alert.errors.messages + alert_errors: alert.errors.messages, + alert_source: alert_source ) end end diff --git a/app/views/devise/shared/_signup_box.html.haml b/app/views/devise/shared/_signup_box.html.haml index 1868cfa06e9..22581f3d0c4 100644 --- a/app/views/devise/shared/_signup_box.html.haml +++ b/app/views/devise/shared/_signup_box.html.haml @@ -49,7 +49,8 @@ data: { qa_selector: 'new_user_email_field' }, required: true, title: _('Please provide a valid email address.') - %p.gl-field-hint.text-secondary= _('We recommend a work email address.') + %p.validation-hint.gl-field-hint.text-secondary= _('We recommend a work email address.') + %p.validation-warning.gl-field-error-ignore.text-secondary.hide= _('This email address does not look right, are you sure you typed it correctly?') -# This is used for providing entry to Jihu on email verification = render_if_exists 'devise/shared/signup_email_additional_info' .form-group.gl-mb-5#password-strength diff --git a/app/views/projects/pages/_no_domains.html.haml b/app/views/projects/pages/_no_domains.html.haml index a537bd80d30..eee7d062d00 100644 --- a/app/views/projects/pages/_no_domains.html.haml +++ b/app/views/projects/pages/_no_domains.html.haml @@ -1,6 +1,6 @@ - if can?(current_user, :update_pages, @project) - .card - .card-header + = render Pajamas::CardComponent.new(card_options: { class: 'gl-mb-5'}, body_options: { class: 'gl-text-center nothing-here-block' }) do |c| + - c.header do = s_('GitLabPages|Domains') - .nothing-here-block + - c.body do = s_("GitLabPages|Support for domains and certificates is disabled. Ask your system's administrator to enable it.") diff --git a/app/views/shared/admin/_admin_note.html.haml b/app/views/shared/admin/_admin_note.html.haml index 82407705885..9dcf181a118 100644 --- a/app/views/shared/admin/_admin_note.html.haml +++ b/app/views/shared/admin/_admin_note.html.haml @@ -1,7 +1,7 @@ - if @group.admin_note.present? - text = @group.admin_note.note - .card.border-info - .card-header.bg-info.gl-text-white + = render Pajamas::CardComponent.new(card_options: { class: 'gl-border-blue-500 gl-mb-5' }, header_options: { class: 'gl-bg-blue-500 gl-text-white' }) do |c| + - c.header do = s_('Admin|Admin notes') - .card-body + - c.body do %p= text diff --git a/config/feature_flags/development/trial_email_validation.yml b/config/feature_flags/development/trial_email_validation.yml new file mode 100644 index 00000000000..c658a49f195 --- /dev/null +++ b/config/feature_flags/development/trial_email_validation.yml @@ -0,0 +1,8 @@ +--- +name: trial_email_validation +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/92762 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/368999 +milestone: '15.3' +type: development +group: group::acquisition +default_enabled: false diff --git a/config/initializers/memory_watchdog.rb b/config/initializers/memory_watchdog.rb index 72779a18b10..82ad615ce25 100644 --- a/config/initializers/memory_watchdog.rb +++ b/config/initializers/memory_watchdog.rb @@ -13,7 +13,8 @@ Gitlab::Cluster::LifecycleEvents.on_worker_start do Gitlab::Memory::Watchdog::NullHandler.instance end - Gitlab::Memory::Watchdog.new( + watchdog = Gitlab::Memory::Watchdog.new( handler: handler, logger: Gitlab::AppLogger - ).start + ) + Gitlab::BackgroundTask.new(watchdog).start end diff --git a/db/migrate/20220728114136_make_ssh_signature_key_nullable.rb b/db/migrate/20220728114136_make_ssh_signature_key_nullable.rb new file mode 100644 index 00000000000..5d724e9f406 --- /dev/null +++ b/db/migrate/20220728114136_make_ssh_signature_key_nullable.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class MakeSshSignatureKeyNullable < Gitlab::Database::Migration[2.0] + enable_lock_retries! + + def change + change_column_null :ssh_signatures, :key_id, true + end +end diff --git a/db/schema_migrations/20220728114136 b/db/schema_migrations/20220728114136 new file mode 100644 index 00000000000..f5bd9962aa3 --- /dev/null +++ b/db/schema_migrations/20220728114136 @@ -0,0 +1 @@ +eb0a6cff006f54f3b5fe12ab566dabfbefa1af46fafbfadde1b292b46e9d17c9 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 0c6b484174d..109c519e02f 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -21133,7 +21133,7 @@ CREATE TABLE ssh_signatures ( created_at timestamp with time zone NOT NULL, updated_at timestamp with time zone NOT NULL, project_id bigint NOT NULL, - key_id bigint NOT NULL, + key_id bigint, verification_status smallint DEFAULT 0 NOT NULL, commit_sha bytea NOT NULL ); diff --git a/doc/user/project/issues/managing_issues.md b/doc/user/project/issues/managing_issues.md index 4c1f6614b1a..40e5a6d6a92 100644 --- a/doc/user/project/issues/managing_issues.md +++ b/doc/user/project/issues/managing_issues.md @@ -731,4 +731,4 @@ You can do the following **only by using quick actions**: - [Publish an issue](#publish-an-issue) (`/publish`). - Clone an issue to the same or another project (`/clone`). - Close an issue and mark as a duplicate of another issue (`/duplicate`). -- Copy labels and milestone from another merge request in the project (`/copy_metadata`). +- Copy labels and milestone from another merge request or issue in the project (`/copy_metadata`). diff --git a/lib/gitlab/memory/watchdog.rb b/lib/gitlab/memory/watchdog.rb index 722c8b5a001..91edb68ad66 100644 --- a/lib/gitlab/memory/watchdog.rb +++ b/lib/gitlab/memory/watchdog.rb @@ -15,7 +15,7 @@ module Gitlab # # The duration for which a process may be above a given fragmentation # threshold is computed as `max_strikes * sleep_time_seconds`. - class Watchdog < Daemon + class Watchdog DEFAULT_SLEEP_TIME_SECONDS = 60 DEFAULT_HEAP_FRAG_THRESHOLD = 0.5 DEFAULT_MAX_STRIKES = 5 @@ -91,7 +91,7 @@ module Gitlab attr_reader :strikes, :max_heap_fragmentation, :max_strikes, :sleep_time_seconds - def run_thread + def call @logger.info(log_labels.merge(message: 'started')) while @alive @@ -103,6 +103,10 @@ module Gitlab @logger.info(log_labels.merge(message: 'stopped')) end + def stop + @alive = false + end + private def monitor_heap_fragmentation @@ -141,10 +145,6 @@ module Gitlab @handler end - def stop_working - @alive = false - end - def log_labels { pid: $$, diff --git a/lib/gitlab/ssh/commit.rb b/lib/gitlab/ssh/commit.rb new file mode 100644 index 00000000000..bfeefc47f13 --- /dev/null +++ b/lib/gitlab/ssh/commit.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module Gitlab + module Ssh + class Commit < Gitlab::SignedCommit + private + + def signature_class + CommitSignatures::SshSignature + end + + def attributes + signature = ::Gitlab::Ssh::Signature.new(signature_text, signed_text, @commit.committer_email) + + { + commit_sha: @commit.sha, + project: @commit.project, + key_id: signature.signed_by_key&.id, + verification_status: signature.verification_status + } + end + end + end +end diff --git a/lib/gitlab/ssh/signature.rb b/lib/gitlab/ssh/signature.rb index 1a236e1a70c..3b4df9a8d0c 100644 --- a/lib/gitlab/ssh/signature.rb +++ b/lib/gitlab/ssh/signature.rb @@ -26,6 +26,14 @@ module Gitlab end end + def signed_by_key + strong_memoize(:signed_by_key) do + next unless key_fingerprint + + Key.find_by_fingerprint_sha256(key_fingerprint) + end + end + private def all_attributes_present? @@ -61,14 +69,6 @@ module Gitlab def key_fingerprint strong_memoize(:key_fingerprint) { signature&.public_key&.fingerprint } end - - def signed_by_key - strong_memoize(:signed_by_key) do - next unless key_fingerprint - - Key.find_by_fingerprint_sha256(key_fingerprint) - end - end end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 74785240606..9b7ae62b370 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -39861,6 +39861,9 @@ msgstr "" msgid "This domain is not verified. You will need to verify ownership before access is enabled." msgstr "" +msgid "This email address does not look right, are you sure you typed it correctly?" +msgstr "" + msgid "This email supersedes any previous emails about scheduled deletion you may have received for %{project_link}." msgstr "" diff --git a/package.json b/package.json index 2d368f66b93..881282e3741 100644 --- a/package.json +++ b/package.json @@ -229,7 +229,6 @@ "jest-junit": "^12.0.0", "jest-raw-loader": "^1.0.1", "jest-transform-graphql": "^2.1.0", - "jest-util": "^26.5.2", "jest-util": "^27.5.1", "markdownlint-cli": "0.31.0", "miragejs": "^0.1.40", diff --git a/spec/features/users/signup_spec.rb b/spec/features/users/signup_spec.rb index 30441dac7b6..132a3f557e4 100644 --- a/spec/features/users/signup_spec.rb +++ b/spec/features/users/signup_spec.rb @@ -283,6 +283,12 @@ RSpec.describe 'Signup' do expect(page).to have_current_path user_registration_path, ignore_query: true expect(page.body).not_to match(/#{new_user.password}/) end + + context 'with invalid email', :saas, :js do + it_behaves_like 'user email validation' do + let(:path) { new_user_registration_path } + end + end end context 'when terms are enforced' do diff --git a/spec/initializers/memory_watchdog_spec.rb b/spec/initializers/memory_watchdog_spec.rb new file mode 100644 index 00000000000..56f995b5cd3 --- /dev/null +++ b/spec/initializers/memory_watchdog_spec.rb @@ -0,0 +1,116 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +RSpec.describe 'memory watchdog' do + subject(:run_initializer) do + load Rails.root.join('config/initializers/memory_watchdog.rb') + end + + context 'when GITLAB_MEMORY_WATCHDOG_ENABLED is truthy' do + let(:env_switch) { 'true' } + + before do + stub_env('GITLAB_MEMORY_WATCHDOG_ENABLED', env_switch) + end + + context 'when runtime is an application' do + let(:watchdog) { instance_double(Gitlab::Memory::Watchdog) } + let(:background_task) { instance_double(Gitlab::BackgroundTask) } + + before do + allow(Gitlab::Runtime).to receive(:application?).and_return(true) + end + + it 'registers a life-cycle hook' do + expect(Gitlab::Cluster::LifecycleEvents).to receive(:on_worker_start) + + run_initializer + end + + shared_examples 'starts watchdog with handler' do |handler_class| + it "uses the #{handler_class} and starts the watchdog" do + expect(Gitlab::Memory::Watchdog).to receive(:new).with( + handler: an_instance_of(handler_class), + logger: Gitlab::AppLogger).and_return(watchdog) + expect(Gitlab::BackgroundTask).to receive(:new).with(watchdog).and_return(background_task) + expect(background_task).to receive(:start) + expect(Gitlab::Cluster::LifecycleEvents).to receive(:on_worker_start).and_yield + + run_initializer + end + end + + # In tests, the Puma constant does not exist so we cannot use a verified double. + # rubocop: disable RSpec/VerifiedDoubles + context 'when puma' do + let(:puma) do + Class.new do + def self.cli_config + Struct.new(:options).new + end + end + end + + before do + stub_const('Puma', puma) + stub_const('Puma::Cluster::WorkerHandle', double.as_null_object) + + allow(Gitlab::Runtime).to receive(:puma?).and_return(true) + end + + it_behaves_like 'starts watchdog with handler', Gitlab::Memory::Watchdog::PumaHandler + end + # rubocop: enable RSpec/VerifiedDoubles + + context 'when sidekiq' do + before do + allow(Gitlab::Runtime).to receive(:sidekiq?).and_return(true) + end + + it_behaves_like 'starts watchdog with handler', Gitlab::Memory::Watchdog::TermProcessHandler + end + + context 'when other runtime' do + it_behaves_like 'starts watchdog with handler', Gitlab::Memory::Watchdog::NullHandler + end + end + + context 'when runtime is unsupported' do + it 'does not register life-cycle hook' do + expect(Gitlab::Cluster::LifecycleEvents).not_to receive(:on_worker_start) + + run_initializer + end + end + end + + context 'when GITLAB_MEMORY_WATCHDOG_ENABLED is false' do + let(:env_switch) { 'false' } + + before do + stub_env('GITLAB_MEMORY_WATCHDOG_ENABLED', env_switch) + # To rule out we return early due to this being false. + allow(Gitlab::Runtime).to receive(:application?).and_return(true) + end + + it 'does not register life-cycle hook' do + expect(Gitlab::Cluster::LifecycleEvents).not_to receive(:on_worker_start) + + run_initializer + end + end + + context 'when GITLAB_MEMORY_WATCHDOG_ENABLED is not set' do + before do + # To rule out we return early due to this being false. + allow(Gitlab::Runtime).to receive(:application?).and_return(true) + end + + it 'does not register life-cycle hook' do + expect(Gitlab::Cluster::LifecycleEvents).not_to receive(:on_worker_start) + + run_initializer + end + end +end diff --git a/spec/lib/gitlab/database/load_balancing/rack_middleware_spec.rb b/spec/lib/gitlab/database/load_balancing/rack_middleware_spec.rb index b768d4ecea3..a1c141af537 100644 --- a/spec/lib/gitlab/database/load_balancing/rack_middleware_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/rack_middleware_spec.rb @@ -30,6 +30,8 @@ RSpec.describe Gitlab::Database::LoadBalancing::RackMiddleware, :redis do expect(app).to receive(:call).with(env).and_return(10) + allow(ActiveSupport::Notifications).to receive(:instrument).and_call_original + expect(ActiveSupport::Notifications) .to receive(:instrument) .with('web_transaction_completed.load_balancing') diff --git a/spec/lib/gitlab/database/load_balancing/sticking_spec.rb b/spec/lib/gitlab/database/load_balancing/sticking_spec.rb index f3139bb1b4f..2ffb2c32c32 100644 --- a/spec/lib/gitlab/database/load_balancing/sticking_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/sticking_spec.rb @@ -77,6 +77,8 @@ RSpec.describe Gitlab::Database::LoadBalancing::Sticking, :redis do let(:last_write_location) { 'foo' } before do + allow(ActiveSupport::Notifications).to receive(:instrument).and_call_original + allow(sticking) .to receive(:last_write_location_for) .with(:user, 42) diff --git a/spec/lib/gitlab/memory/watchdog_spec.rb b/spec/lib/gitlab/memory/watchdog_spec.rb index f9e6b698cda..010f6884df3 100644 --- a/spec/lib/gitlab/memory/watchdog_spec.rb +++ b/spec/lib/gitlab/memory/watchdog_spec.rb @@ -14,12 +14,39 @@ RSpec.describe Gitlab::Memory::Watchdog, :aggregate_failures, :prometheus do let(:sleep_time) { 0.1 } let(:max_heap_fragmentation) { 0.2 } + # Tests should set this to control the number of loop iterations in `call`. + let(:watchdog_iterations) { 1 } + subject(:watchdog) do described_class.new(handler: handler, logger: logger, sleep_time_seconds: sleep_time, - max_strikes: max_strikes, max_heap_fragmentation: max_heap_fragmentation) + max_strikes: max_strikes, max_heap_fragmentation: max_heap_fragmentation).tap do |instance| + # We need to defuse `sleep` and stop the internal loop after N iterations. + iterations = 0 + expect(instance).to receive(:sleep) do + instance.stop if (iterations += 1) >= watchdog_iterations + end.at_most(watchdog_iterations) + end + end + + def stub_prometheus_metrics + allow(Gitlab::Metrics).to receive(:gauge) + .with(:gitlab_memwd_heap_frag_limit, anything) + .and_return(heap_frag_limit_gauge) + allow(Gitlab::Metrics).to receive(:counter) + .with(:gitlab_memwd_heap_frag_violations_total, anything, anything) + .and_return(heap_frag_violations_counter) + allow(Gitlab::Metrics).to receive(:counter) + .with(:gitlab_memwd_heap_frag_violations_handled_total, anything, anything) + .and_return(heap_frag_violations_handled_counter) + + allow(heap_frag_limit_gauge).to receive(:set) + allow(heap_frag_violations_counter).to receive(:increment) + allow(heap_frag_violations_handled_counter).to receive(:increment) end before do + stub_prometheus_metrics + allow(handler).to receive(:on_high_heap_fragmentation).and_return(true) allow(logger).to receive(:warn) @@ -30,18 +57,14 @@ RSpec.describe Gitlab::Memory::Watchdog, :aggregate_failures, :prometheus do allow(::Prometheus::PidProvider).to receive(:worker_id).and_return('worker_1') end - after do - watchdog.stop - end - - context 'when starting up' do + context 'when created' do let(:fragmentation) { 0 } let(:max_strikes) { 0 } it 'sets the heap fragmentation limit gauge' do - allow(Gitlab::Metrics).to receive(:gauge).with(anything, anything).and_return(heap_frag_limit_gauge) - expect(heap_frag_limit_gauge).to receive(:set).with({}, max_heap_fragmentation) + + watchdog end context 'when no settings are set in the environment' do @@ -78,72 +101,49 @@ RSpec.describe Gitlab::Memory::Watchdog, :aggregate_failures, :prometheus do it 'does not signal the handler' do expect(handler).not_to receive(:on_high_heap_fragmentation) - watchdog.start - - sleep sleep_time * 3 + watchdog.call end end context 'when process exceeds heap fragmentation threshold permanently' do let(:fragmentation) { max_heap_fragmentation + 0.1 } - - before do - expected_labels = { pid: 'worker_1' } - allow(Gitlab::Metrics).to receive(:counter) - .with(:gitlab_memwd_heap_frag_violations_total, anything, expected_labels) - .and_return(heap_frag_violations_counter) - allow(Gitlab::Metrics).to receive(:counter) - .with(:gitlab_memwd_heap_frag_violations_handled_total, anything, expected_labels) - .and_return(heap_frag_violations_handled_counter) - allow(heap_frag_violations_counter).to receive(:increment) - allow(heap_frag_violations_handled_counter).to receive(:increment) - end + let(:max_strikes) { 3 } context 'when process has not exceeded allowed number of strikes' do - let(:max_strikes) { 10 } + let(:watchdog_iterations) { max_strikes } it 'does not signal the handler' do expect(handler).not_to receive(:on_high_heap_fragmentation) - watchdog.start - - sleep sleep_time * 3 + watchdog.call end it 'does not log any events' do expect(logger).not_to receive(:warn) - watchdog.start - - sleep sleep_time * 3 + watchdog.call end it 'increments the violations counter' do - expect(heap_frag_violations_counter).to receive(:increment) + expect(heap_frag_violations_counter).to receive(:increment).exactly(watchdog_iterations) - watchdog.start - - sleep sleep_time * 3 + watchdog.call end it 'does not increment violations handled counter' do expect(heap_frag_violations_handled_counter).not_to receive(:increment) - watchdog.start - - sleep sleep_time * 3 + watchdog.call end end context 'when process exceeds the allowed number of strikes' do - let(:max_strikes) { 1 } + let(:watchdog_iterations) { max_strikes + 1 } it 'signals the handler and resets strike counter' do expect(handler).to receive(:on_high_heap_fragmentation).and_return(true) - watchdog.start - - sleep sleep_time * 3 + watchdog.call expect(watchdog.strikes).to eq(0) end @@ -163,18 +163,14 @@ RSpec.describe Gitlab::Memory::Watchdog, :aggregate_failures, :prometheus do memwd_rss_bytes: 1024 }) - watchdog.start - - sleep sleep_time * 3 + watchdog.call end it 'increments both the violations and violations handled counters' do - expect(heap_frag_violations_counter).to receive(:increment) + expect(heap_frag_violations_counter).to receive(:increment).exactly(watchdog_iterations) expect(heap_frag_violations_handled_counter).to receive(:increment) - watchdog.start - - sleep sleep_time * 3 + watchdog.call end context 'when enforce_memory_watchdog ops toggle is off' do @@ -188,35 +184,31 @@ RSpec.describe Gitlab::Memory::Watchdog, :aggregate_failures, :prometheus do receive(:on_high_heap_fragmentation).with(fragmentation).and_return(true) ) - watchdog.start - - sleep sleep_time * 3 + watchdog.call end end - end - context 'when handler result is true' do - let(:max_strikes) { 1 } + context 'when handler result is true' do + it 'considers the event handled and stops itself' do + expect(handler).to receive(:on_high_heap_fragmentation).once.and_return(true) + expect(logger).to receive(:info).with(hash_including(message: 'stopped')) - it 'considers the event handled and stops itself' do - expect(handler).to receive(:on_high_heap_fragmentation).once.and_return(true) - - watchdog.start - - sleep sleep_time * 3 + watchdog.call + end end - end - context 'when handler result is false' do - let(:max_strikes) { 1 } + context 'when handler result is false' do + let(:max_strikes) { 0 } # to make sure the handler fires each iteration + let(:watchdog_iterations) { 3 } - it 'keeps running' do - # Return true the third time to terminate the daemon. - expect(handler).to receive(:on_high_heap_fragmentation).and_return(false, false, true) + it 'keeps running' do + expect(heap_frag_violations_counter).to receive(:increment).exactly(watchdog_iterations) + expect(heap_frag_violations_handled_counter).to receive(:increment).exactly(watchdog_iterations) + # Return true the third time to terminate the daemon. + expect(handler).to receive(:on_high_heap_fragmentation).and_return(false, false, true) - watchdog.start - - sleep sleep_time * 4 + watchdog.call + end end end end @@ -224,6 +216,7 @@ RSpec.describe Gitlab::Memory::Watchdog, :aggregate_failures, :prometheus do context 'when process exceeds heap fragmentation threshold temporarily' do let(:fragmentation) { max_heap_fragmentation } let(:max_strikes) { 1 } + let(:watchdog_iterations) { 4 } before do allow(Gitlab::Metrics::Memory).to receive(:gc_heap_fragmentation).and_return( @@ -237,9 +230,7 @@ RSpec.describe Gitlab::Memory::Watchdog, :aggregate_failures, :prometheus do it 'does not signal the handler' do expect(handler).not_to receive(:on_high_heap_fragmentation) - watchdog.start - - sleep sleep_time * 4 + watchdog.call end end @@ -254,9 +245,7 @@ RSpec.describe Gitlab::Memory::Watchdog, :aggregate_failures, :prometheus do it 'does not monitor heap fragmentation' do expect(Gitlab::Metrics::Memory).not_to receive(:gc_heap_fragmentation) - watchdog.start - - sleep sleep_time * 3 + watchdog.call end end end diff --git a/spec/lib/gitlab/ssh/commit_spec.rb b/spec/lib/gitlab/ssh/commit_spec.rb new file mode 100644 index 00000000000..cc977a80f95 --- /dev/null +++ b/spec/lib/gitlab/ssh/commit_spec.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe Gitlab::Ssh::Commit do + let_it_be(:project) { create(:project, :repository) } + let_it_be(:signed_by_key) { create(:key) } + + let(:commit) { create(:commit, project: project) } + let(:signature_text) { 'signature_text' } + let(:signed_text) { 'signed_text' } + let(:signature_data) { [signature_text, signed_text] } + let(:verifier) { instance_double('Gitlab::Ssh::Signature') } + let(:verification_status) { :verified } + + subject(:signature) { described_class.new(commit).signature } + + before do + allow(Gitlab::Git::Commit).to receive(:extract_signature_lazily) + .with(Gitlab::Git::Repository, commit.sha) + .and_return(signature_data) + + allow(verifier).to receive(:verification_status).and_return(verification_status) + allow(verifier).to receive(:signed_by_key).and_return(signed_by_key) + + allow(Gitlab::Ssh::Signature).to receive(:new) + .with(signature_text, signed_text, commit.committer_email) + .and_return(verifier) + end + + describe '#signature' do + it 'returns the cached signature on multiple calls' do + ssh_commit = described_class.new(commit) + + expect(ssh_commit).to receive(:create_cached_signature!).and_call_original + ssh_commit.signature + + expect(ssh_commit).not_to receive(:create_cached_signature!) + ssh_commit.signature + end + + context 'when all expected data is present' do + it 'calls signature verifier and uses returned attributes' do + expect(signature).to have_attributes( + commit_sha: commit.sha, + project: project, + key_id: signed_by_key.id, + verification_status: 'verified' + ) + end + end + + context 'when signed_by_key is nil' do + let_it_be(:signed_by_key) { nil } + + let(:verification_status) { :unknown_key } + + it 'creates signature without a key_id' do + expect(signature).to have_attributes( + commit_sha: commit.sha, + project: project, + key_id: nil, + verification_status: 'unknown_key' + ) + end + end + end + + describe '#update_signature!' do + it 'updates verification status' do + allow(verifier).to receive(:verification_status).and_return(:unverified) + signature + + stored_signature = CommitSignatures::SshSignature.find_by_commit_sha(commit.sha) + + allow(verifier).to receive(:verification_status).and_return(:verified) + + expect { described_class.new(commit).update_signature!(stored_signature) }.to( + change { signature.reload.verification_status }.from('unverified').to('verified') + ) + end + end +end diff --git a/spec/models/commit_signatures/ssh_signature_spec.rb b/spec/models/commit_signatures/ssh_signature_spec.rb index ac4496e9d8c..64d95fe3a71 100644 --- a/spec/models/commit_signatures/ssh_signature_spec.rb +++ b/spec/models/commit_signatures/ssh_signature_spec.rb @@ -22,7 +22,7 @@ RSpec.describe CommitSignatures::SshSignature do it_behaves_like 'commit signature' describe 'associations' do - it { is_expected.to belong_to(:key).required } + it { is_expected.to belong_to(:key).optional } end describe '.by_commit_sha scope' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 921d6503099..091627c8a38 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -506,3 +506,16 @@ module TouchRackUploadedFile end Rack::Test::UploadedFile.prepend(TouchRackUploadedFile) + +# Monkey-patch to enable ActiveSupport::Notifications for Redis commands +module RedisCommands + module Instrumentation + def process(commands, &block) + ActiveSupport::Notifications.instrument('redis.process_commands', commands: commands) do + super(commands, &block) + end + end + end +end + +Redis::Client.prepend(RedisCommands::Instrumentation) diff --git a/spec/support/helpers/redis_commands/recorder.rb b/spec/support/helpers/redis_commands/recorder.rb new file mode 100644 index 00000000000..05a1aa67853 --- /dev/null +++ b/spec/support/helpers/redis_commands/recorder.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module RedisCommands + class Recorder + def initialize(pattern: nil, &block) + @log = [] + @pattern = pattern + + record(&block) if block + end + + attr_reader :log + + def record(&block) + ActiveSupport::Notifications.subscribed(method(:callback), 'redis.process_commands', &block) + end + + def by_command(command) + @log.select { |record| record.include?(command) } + end + + def count + @count ||= @log.count + end + + private + + def callback(name, start, finish, message_id, values) + commands = values[:commands] + + @log << commands.flatten if @pattern.nil? || commands.to_s.include?(@pattern) + end + end +end diff --git a/spec/support/shared_examples/features/trial_email_validation_shared_example.rb b/spec/support/shared_examples/features/trial_email_validation_shared_example.rb new file mode 100644 index 00000000000..8304a91af86 --- /dev/null +++ b/spec/support/shared_examples/features/trial_email_validation_shared_example.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'user email validation' do + let(:email_hint_message) { 'We recommend a work email address.' } + let(:email_error_message) { 'Please provide a valid email address.' } + + let(:email_warning_message) do + 'This email address does not look right, are you sure you typed it correctly?' + end + + context 'with trial_email_validation flag enabled' do + it 'shows an error message until a correct email is entered' do + visit path + expect(page).to have_content(email_hint_message) + expect(page).not_to have_content(email_error_message) + expect(page).not_to have_content(email_warning_message) + + fill_in 'new_user_email', with: 'foo@' + fill_in 'new_user_first_name', with: '' + + expect(page).not_to have_content(email_hint_message) + expect(page).to have_content(email_error_message) + expect(page).not_to have_content(email_warning_message) + + fill_in 'new_user_email', with: 'foo@bar' + fill_in 'new_user_first_name', with: '' + + expect(page).not_to have_content(email_hint_message) + expect(page).not_to have_content(email_error_message) + expect(page).to have_content(email_warning_message) + + fill_in 'new_user_email', with: 'foo@gitlab.com' + fill_in 'new_user_first_name', with: '' + + expect(page).not_to have_content(email_hint_message) + expect(page).not_to have_content(email_error_message) + expect(page).not_to have_content(email_warning_message) + end + end + + context 'when trial_email_validation flag disabled' do + before do + stub_feature_flags trial_email_validation: false + end + + it 'does not show an error message' do + visit path + expect(page).to have_content(email_hint_message) + expect(page).not_to have_content(email_error_message) + expect(page).not_to have_content(email_warning_message) + + fill_in 'new_user_email', with: 'foo@' + + expect(page).to have_content(email_hint_message) + expect(page).not_to have_content(email_error_message) + expect(page).not_to have_content(email_warning_message) + end + end +end diff --git a/spec/support/shared_examples/services/alert_management/alert_processing/alert_firing_shared_examples.rb b/spec/support/shared_examples/services/alert_management/alert_processing/alert_firing_shared_examples.rb index ca86cb082a7..df11fa2dbdb 100644 --- a/spec/support/shared_examples/services/alert_management/alert_processing/alert_firing_shared_examples.rb +++ b/spec/support/shared_examples/services/alert_management/alert_processing/alert_firing_shared_examples.rb @@ -35,9 +35,10 @@ RSpec.shared_examples 'creates an alert management alert or errors' do it 'writes a warning to the log' do expect(Gitlab::AppLogger).to receive(:warn).with( - message: "Unable to create AlertManagement::Alert from #{source}", + message: "Unable to create AlertManagement::Alert", project_id: project.id, - alert_errors: { hosts: ['hosts array is over 255 chars'] } + alert_errors: { hosts: ['hosts array is over 255 chars'] }, + alert_source: source ) subject diff --git a/spec/support/shared_examples/services/alert_management/alert_processing/alert_recovery_shared_examples.rb b/spec/support/shared_examples/services/alert_management/alert_processing/alert_recovery_shared_examples.rb index f8e096297d3..eb9f76d8626 100644 --- a/spec/support/shared_examples/services/alert_management/alert_processing/alert_recovery_shared_examples.rb +++ b/spec/support/shared_examples/services/alert_management/alert_processing/alert_recovery_shared_examples.rb @@ -4,8 +4,6 @@ # - `alert`, the alert to be resolved RSpec.shared_examples 'resolves an existing alert management alert' do it 'sets the end time and status' do - expect(Gitlab::AppLogger).not_to receive(:warn) - expect { subject } .to change { alert.reload.resolved? }.to(true) .and change { alert.ended_at.present? }.to(true) @@ -22,36 +20,6 @@ RSpec.shared_examples 'does not change the alert end time' do end end -# This shared_example requires the following variables: -# - `project`, expected project for an incoming alert -# - `service`, a service which includes AlertManagement::AlertProcessing -# - `alert` (optional), the alert which should fail to resolve. If not -# included, the log is expected to correspond to a new alert -RSpec.shared_examples 'writes a warning to the log for a failed alert status update' do - before do - allow(service).to receive(:alert).and_call_original - allow(service).to receive_message_chain(:alert, :resolve).and_return(false) - end - - specify do - expect(Gitlab::AppLogger).to receive(:warn).with( - message: 'Unable to update AlertManagement::Alert status to resolved', - project_id: project.id, - alert_id: alert ? alert.id : (last_alert_id + 1) - ) - - # Failure to resolve a recovery alert is not a critical failure - expect(subject).to be_success - end - - private - - def last_alert_id - AlertManagement::Alert.connection - .select_value("SELECT nextval('#{AlertManagement::Alert.sequence_name}')") - end -end - RSpec.shared_examples 'processes recovery alert' do context 'seen for the first time' do let(:alert) { AlertManagement::Alert.last } @@ -69,7 +37,6 @@ RSpec.shared_examples 'processes recovery alert' do it_behaves_like 'creates expected system notes for alert', :recovery_alert, :resolve_alert it_behaves_like 'sends alert notification emails if enabled' it_behaves_like 'closes related incident if enabled' - it_behaves_like 'writes a warning to the log for a failed alert status update' it_behaves_like 'does not create an alert management alert' it_behaves_like 'does not process incident issues' @@ -83,7 +50,6 @@ RSpec.shared_examples 'processes recovery alert' do it_behaves_like 'creates expected system notes for alert', :recovery_alert, :resolve_alert it_behaves_like 'sends alert notification emails if enabled' it_behaves_like 'closes related incident if enabled' - it_behaves_like 'writes a warning to the log for a failed alert status update' it_behaves_like 'does not create an alert management alert' it_behaves_like 'does not process incident issues' @@ -97,7 +63,6 @@ RSpec.shared_examples 'processes recovery alert' do it_behaves_like 'creates expected system notes for alert', :recovery_alert, :resolve_alert it_behaves_like 'sends alert notification emails if enabled' it_behaves_like 'closes related incident if enabled' - it_behaves_like 'writes a warning to the log for a failed alert status update' it_behaves_like 'does not create an alert management alert' it_behaves_like 'does not process incident issues' diff --git a/spec/support_specs/helpers/redis_commands/recorder_spec.rb b/spec/support_specs/helpers/redis_commands/recorder_spec.rb new file mode 100644 index 00000000000..6f93ed2fcf0 --- /dev/null +++ b/spec/support_specs/helpers/redis_commands/recorder_spec.rb @@ -0,0 +1,128 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe RedisCommands::Recorder, :use_clean_rails_redis_caching do + subject(:recorder) { described_class.new(pattern: pattern) } + + let(:cache) { Rails.cache } + let(:pattern) { nil } + + describe '#initialize' do + context 'with a block' do + it 'records Redis commands' do + recorder = described_class.new { cache.read('key1') } + + expect(recorder.log).to include([:get, 'cache:gitlab:key1']) + end + end + + context 'without block' do + it 'only initializes the recorder' do + recorder = described_class.new + + expect(recorder.log).to eq([]) + end + end + end + + describe '#record' do + it 'records Redis commands' do + recorder.record do + cache.write('key1', '1') + cache.read('key1') + cache.read('key2') + cache.delete('key1') + end + + expect(recorder.log).to include([:set, 'cache:gitlab:key1', anything]) + expect(recorder.log).to include([:get, 'cache:gitlab:key1']) + expect(recorder.log).to include([:get, 'cache:gitlab:key2']) + expect(recorder.log).to include([:del, 'cache:gitlab:key1']) + end + + it 'does not record commands before the call' do + cache.write('key1', 1) + + recorder.record do + cache.read('key1') + end + + expect(recorder.log).not_to include([:set, anything, anything]) + expect(recorder.log).to include([:get, 'cache:gitlab:key1']) + end + + it 'refreshes recording after reinitialization' do + cache.read('key1') + + recorder1 = described_class.new + recorder1.record do + cache.read('key2') + end + + recorder2 = described_class.new + + cache.read('key3') + + recorder2.record do + cache.read('key4') + end + + expect(recorder1.log).to include([:get, 'cache:gitlab:key2']) + expect(recorder1.log).not_to include([:get, 'cache:gitlab:key1']) + expect(recorder1.log).not_to include([:get, 'cache:gitlab:key3']) + expect(recorder1.log).not_to include([:get, 'cache:gitlab:key4']) + + expect(recorder2.log).to include([:get, 'cache:gitlab:key4']) + expect(recorder2.log).not_to include([:get, 'cache:gitlab:key1']) + expect(recorder2.log).not_to include([:get, 'cache:gitlab:key2']) + expect(recorder2.log).not_to include([:get, 'cache:gitlab:key3']) + end + end + + describe 'Pattern recording' do + let(:pattern) { 'key1' } + + it 'records only matching keys' do + recorder.record do + cache.write('key1', '1') + cache.read('key2') + cache.read('key1') + cache.delete('key2') + end + + expect(recorder.log).to include([:set, 'cache:gitlab:key1', anything]) + expect(recorder.log).to include([:get, 'cache:gitlab:key1']) + expect(recorder.log).not_to include([:get, 'cache:gitlab:key2']) + expect(recorder.log).not_to include([:del, 'cache:gitlab:key2']) + end + end + + describe '#by_command' do + it 'returns only matching commands' do + recorder.record do + cache.write('key1', '1') + cache.read('key2') + cache.read('key1') + cache.delete('key2') + end + + expect(recorder.by_command(:del)).to match_array([[:del, 'cache:gitlab:key2']]) + end + end + + describe '#count' do + it 'returns the number of recorded commands' do + cache.read 'warmup' + + recorder.record do + cache.write('key1', '1') + cache.read('key2') + cache.read('key1') + cache.delete('key2') + end + + expect(recorder.count).to eq(4) + end + end +end