diff options
51 files changed, 311 insertions, 650 deletions
diff --git a/.rubocop_todo/layout/line_length.yml b/.rubocop_todo/layout/line_length.yml index 162afb96a09..a2ea6e44a24 100644 --- a/.rubocop_todo/layout/line_length.yml +++ b/.rubocop_todo/layout/line_length.yml @@ -4724,7 +4724,6 @@ Layout/LineLength: - 'spec/services/metrics/users_starred_dashboards/delete_service_spec.rb' - 'spec/services/milestones/transfer_service_spec.rb' - 'spec/services/namespace_settings/update_service_spec.rb' - - 'spec/services/namespaces/in_product_marketing_emails_service_spec.rb' - 'spec/services/notes/build_service_spec.rb' - 'spec/services/notes/copy_service_spec.rb' - 'spec/services/notes/create_service_spec.rb' diff --git a/.rubocop_todo/lint/redundant_cop_disable_directive.yml b/.rubocop_todo/lint/redundant_cop_disable_directive.yml index 7a07242ab29..38ea0ea12e6 100644 --- a/.rubocop_todo/lint/redundant_cop_disable_directive.yml +++ b/.rubocop_todo/lint/redundant_cop_disable_directive.yml @@ -123,7 +123,6 @@ Lint/RedundantCopDisableDirective: - 'ee/app/services/ee/search_service.rb' - 'ee/app/services/security/token_revocation_service.rb' - 'ee/app/workers/ee/issuable_export_csv_worker.rb' - - 'ee/app/workers/ee/namespaces/in_product_marketing_emails_worker.rb' - 'ee/app/workers/geo/design_repository_shard_sync_worker.rb' - 'ee/app/workers/geo/repository_shard_sync_worker.rb' - 'ee/app/workers/geo/repository_verification/secondary/shard_worker.rb' diff --git a/.rubocop_todo/rspec/before_all_role_assignment.yml b/.rubocop_todo/rspec/before_all_role_assignment.yml index c74c0d93813..b4f2eae6148 100644 --- a/.rubocop_todo/rspec/before_all_role_assignment.yml +++ b/.rubocop_todo/rspec/before_all_role_assignment.yml @@ -1505,7 +1505,6 @@ RSpec/BeforeAllRoleAssignment: - 'spec/services/metrics/dashboard/system_dashboard_service_spec.rb' - 'spec/services/metrics/dashboard/transient_embed_service_spec.rb' - 'spec/services/metrics/dashboard/update_dashboard_service_spec.rb' - - 'spec/services/namespaces/in_product_marketing_emails_service_spec.rb' - 'spec/services/notes/build_service_spec.rb' - 'spec/services/notes/create_service_spec.rb' - 'spec/services/notes/quick_actions_service_spec.rb' diff --git a/.rubocop_todo/rspec/verified_doubles.yml b/.rubocop_todo/rspec/verified_doubles.yml index 6f2e884fc55..0408eb97119 100644 --- a/.rubocop_todo/rspec/verified_doubles.yml +++ b/.rubocop_todo/rspec/verified_doubles.yml @@ -162,7 +162,6 @@ RSpec/VerifiedDoubles: - 'ee/spec/services/merge_requests/approval_service_spec.rb' - 'ee/spec/services/merge_requests/build_service_spec.rb' - 'ee/spec/services/merge_requests/reset_approvals_service_spec.rb' - - 'ee/spec/services/namespaces/in_product_marketing_emails_service_spec.rb' - 'ee/spec/services/projects/update_mirror_service_spec.rb' - 'ee/spec/services/security/ingestion/ingest_report_slice_service_spec.rb' - 'ee/spec/services/security/orchestration/assign_service_spec.rb' @@ -891,7 +890,6 @@ RSpec/VerifiedDoubles: - 'spec/services/metrics/dashboard/update_dashboard_service_spec.rb' - 'spec/services/metrics/users_starred_dashboards/create_service_spec.rb' - 'spec/services/milestones/update_service_spec.rb' - - 'spec/services/namespaces/in_product_marketing_emails_service_spec.rb' - 'spec/services/notes/create_service_spec.rb' - 'spec/services/notes/render_service_spec.rb' - 'spec/services/notification_service_spec.rb' diff --git a/.rubocop_todo/sidekiq_load_balancing/worker_data_consistency.yml b/.rubocop_todo/sidekiq_load_balancing/worker_data_consistency.yml index 9eed58acdc7..1c61aa893a2 100644 --- a/.rubocop_todo/sidekiq_load_balancing/worker_data_consistency.yml +++ b/.rubocop_todo/sidekiq_load_balancing/worker_data_consistency.yml @@ -181,7 +181,6 @@ SidekiqLoadBalancing/WorkerDataConsistency: - 'app/workers/metrics/dashboard/schedule_annotations_prune_worker.rb' - 'app/workers/metrics/dashboard/sync_dashboards_worker.rb' - 'app/workers/migrate_external_diffs_worker.rb' - - 'app/workers/namespaces/in_product_marketing_emails_worker.rb' - 'app/workers/namespaces/process_sync_events_worker.rb' - 'app/workers/namespaces/prune_aggregation_schedules_worker.rb' - 'app/workers/namespaces/schedule_aggregation_worker.rb' diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index e89fa01f7b8..d168c233202 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -3c844703332ab78c394328a9eb0250aac4e9eb93 +270d3c6bb726d59c56923c7b5de7abe93da1d5e9 diff --git a/app/mailers/emails/in_product_marketing.rb b/app/mailers/emails/in_product_marketing.rb index 972c1da065a..92743dc1926 100644 --- a/app/mailers/emails/in_product_marketing.rb +++ b/app/mailers/emails/in_product_marketing.rb @@ -12,15 +12,6 @@ module Emails 'X-Mailgun-Tag' => 'marketing' }.freeze - def in_product_marketing_email(recipient_id, group_id, track, series) - group = Group.find(group_id) - user = User.find(recipient_id) - email = user.notification_email_for(group) - @message = Gitlab::Email::Message::InProductMarketing.for(track).new(group: group, user: user, series: series) - - mail_to(to: email, subject: @message.subject_line) - end - def build_ios_app_guide_email(recipient_email) @message = ::Gitlab::Email::Message::BuildIosAppGuide.new diff --git a/app/services/namespaces/in_product_marketing_emails_service.rb b/app/services/namespaces/in_product_marketing_emails_service.rb index 1ce7e4cae16..14e670126c6 100644 --- a/app/services/namespaces/in_product_marketing_emails_service.rb +++ b/app/services/namespaces/in_product_marketing_emails_service.rb @@ -44,108 +44,6 @@ module Namespaces interval_days = TRACKS.dig(track.to_sym, :interval_days) interval_days&.count || 0 end - - def self.send_for_all_tracks_and_intervals - TRACKS.each_key do |track| - TRACKS[track][:interval_days].each do |interval| - new(track, interval).execute - end - end - end - - def initialize(track, interval) - @track = track - @interval = interval - @sent_email_records = ::Users::InProductMarketingEmailRecords.new - end - - def execute - raise ArgumentError, "Track #{track} not defined" unless TRACKS.key?(track) - - groups_for_track.each_batch do |groups| - groups.each do |group| - send_email_for_group(group) - end - end - end - - private - - attr_reader :track, :interval, :sent_email_records - - def send_email(user, group) - NotificationService.new.in_product_marketing(user.id, group.id, track, series) - end - - def send_email_for_group(group) - users_for_group(group).each do |user| - if can_perform_action?(user, group) - send_email(user, group) - sent_email_records.add(user, track: track, series: series) - end - end - - sent_email_records.save! - end - - def groups_for_track - onboarding_progress_scope = Onboarding::Progress - .completed_actions_with_latest_in_range(completed_actions, range) - .incomplete_actions(incomplete_actions) - - # Filtering out sub-groups is a temporary fix to prevent calling - # `.root_ancestor` on groups that are not root groups. - # See https://gitlab.com/groups/gitlab-org/-/epics/5594 for more information. - Group - .top_most - .with_onboarding_progress - .merge(onboarding_progress_scope) - .merge(subscription_scope) - end - - def subscription_scope - {} - end - - # rubocop: disable CodeReuse/ActiveRecord - def users_for_group(group) - group.users - .where(email_opted_in: true) - .merge(Users::InProductMarketingEmail.without_track_and_series(track, series)) - end - # rubocop: enable CodeReuse/ActiveRecord - - def can_perform_action?(user, group) - case track - when :create, :verify - user.can?(:create_projects, group) - when :trial, :trial_short - user.can?(:start_trial, group) - when :team, :team_short - user.can?(:admin_group_member, group) - when :admin_verify - user.can?(:admin_group, group) - when :experience - true - end - end - - def completed_actions - TRACKS[track][:completed_actions] - end - - def range - date = (interval + 1).days.ago - date.beginning_of_day..date.end_of_day - end - - def incomplete_actions - TRACKS[track][:incomplete_actions] - end - - def series - TRACKS[track][:interval_days].index(interval) - end end end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index d24f1ce399e..2832dfc340f 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -756,10 +756,6 @@ class NotificationService end end - def in_product_marketing(user_id, group_id, track, series) - mailer.in_product_marketing_email(user_id, group_id, track, series).deliver_later - end - def approve_mr(merge_request, current_user) approve_mr_email(merge_request, merge_request.target_project, current_user) end diff --git a/app/views/notify/in_product_marketing_email.html.haml b/app/views/notify/in_product_marketing_email.html.haml deleted file mode 100644 index a88d581c5de..00000000000 --- a/app/views/notify/in_product_marketing_email.html.haml +++ /dev/null @@ -1,51 +0,0 @@ -- if @message.series? - %tr{ style: "background-color: #ffffff;" } - %td{ style: "color: #424242; padding: 10px 30px; text-align: center; font-family: 'Source Sans Pro', helvetica, arial, sans-serif;font-size: 16px; line-height: 22px; border: 1px solid #dddddd" } - %p - = @message.progress.html_safe -%tr - %td{ bgcolor: "#ffffff", height: "auto", style: "max-width: 600px; width: 100%; text-align: center; height: 200px; padding: 25px 15px; mso-line-height-rule: exactly; min-height: 40px; font-family: 'Source Sans Pro', helvetica, arial, sans-serif;", valign: "middle", width: "100%" } - = inline_image_link(@message.logo_path, { width: '150', style: 'width: 150px;' }) - %h1{ style: "font-size: 40px; line-height: 46x; color: #000000; padding: 20px 0 0 0; font-weight: normal;" } - = @message.title - %h2{ style: "font-size: 28px; line-height: 34px; color: #000000; padding: 0; font-weight: 400;" } - = @message.subtitle -%tr - %td{ style: "padding: 10px 20px 30px 20px; font-family: 'Source Sans Pro', helvetica, arial, sans-serif; color:#000000; font-size: 18px; line-height: 24px;" } - %p{ style: "margin: 0 0 20px 0;" } - = @message.body_line1.html_safe - - @message.body_line2&.tap do |line| - %p{ style: "margin: 0 0 20px 0;" } - = line.html_safe -- if @message.cta_text - %tr - %td{ align: "center", style: "padding: 10px 20px 80px 20px; font-family: 'Source Sans Pro', helvetica, arial, sans-serif;" } - .cta_link.cta_link_primary= @message.cta_link -- else - %tr - %td{ style: "padding: 10px 20px 10px 20px; font-family: 'Source Sans Pro', helvetica, arial, sans-serif; color:#000000; font-size: 16px; line-height: 20px;" } - %table{ border: "0", cellpadding: "0", cellspacing: "0", width: "100%", style: "width: 100%; min-width: 100%;" } - %tr - %td{ width: "50%", style: "width: 50%; min-width: 50%; color: #000000; font-family: 'Source Sans Pro', helvetica, arial, sans-serif; font-size: 16px; line-height: 100%; padding-bottom: 16px; text-align: left;", align: "left" } - = @message.feedback_ratings(1) - %td{ width: "50%", style: "width: 50%; min-width: 50%; color: #000000; font-family: 'Source Sans Pro', helvetica, arial, sans-serif; font-size: 16px; line-height: 100%; padding-bottom: 16px; text-align: right;", align: "right" } - = @message.feedback_ratings(5) - %tr - %td{ align: "center", style: "padding: 10px 1px 30px 1px;" } - %table{ align: "center", cellpadding: "5", cellspacing: "0", width: "100%", style: "width: 100%; min-width: 100%; border: 1px solid #dae0ea; border-radius: 0; min-width: 100%; text-align: center; font-family: 'Source Sans Pro', helvetica, arial, sans-serif; font-size: 16px;" } - %tr - - (1..5).each do |rating| - %td{ height: "54", style: "border-left: 1px solid #dae0ea; padding-bottom: 0; width: 9% !important;", width: "9%" } - %a{ href: @message.feedback_link(rating), style: "color: #424242; display: block; text-decoration: none;" } - %span{ height: "54", style: "display: block; font-size: 18px; height: 22px; line-height: 22px; padding: 16px 0; width: 100%; text-decoration: none;" } - = rating - %tr - %td{ style: "padding: 10px 20px 30px 20px; font-family: 'Source Sans Pro', helvetica, arial, sans-serif; color:#000000; font-size: 18px; line-height: 24px;" } - %p{ style: "margin: 0 0 50px 0;" } - = @message.feedback_thanks -- if @message.invite_members? - %tr - %td{ align: "center", style: "padding: 0 20px 80px 20px; font-family: 'Source Sans Pro', helvetica, arial, sans-serif;" } - = @message.invite_text - %br - = @message.invite_link diff --git a/app/views/notify/in_product_marketing_email.text.erb b/app/views/notify/in_product_marketing_email.text.erb deleted file mode 100644 index 79a366eb1cc..00000000000 --- a/app/views/notify/in_product_marketing_email.text.erb +++ /dev/null @@ -1,36 +0,0 @@ -<%= @message.tagline %> - -<%= @message.title %> -<%= @message.subtitle %> - - -<%= @message.body_line1 %> - -<%= @message.body_line2 %> - -<% if @message.cta_text %> -<%= @message.cta_link %> - - - -<% else %> -<% (1..5).each do |rating| %> -<%= "#{rating} - #{@message.feedback_ratings(rating).upcase} - #{@message.feedback_link(rating)}" %> -<% end %> - - -<%= @message.feedback_thanks %> -<% end %> -<% if @message.invite_members? %> -<%= @message.invite_text %> -<%= @message.invite_link %> -<% end %> - - - - -<%= @message.footer_links %> - -<%= @message.address %> - -<%= @message.unsubscribe %> diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index fc73a3b0cbc..94b9acbc48b 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -579,15 +579,6 @@ :weight: 1 :idempotent: true :tags: [] -- :name: cronjob:namespaces_in_product_marketing_emails - :worker_name: Namespaces::InProductMarketingEmailsWorker - :feature_category: :experimentation_activation - :has_external_dependencies: false - :urgency: :low - :resource_boundary: :unknown - :weight: 1 - :idempotent: false - :tags: [] - :name: cronjob:namespaces_prune_aggregation_schedules :worker_name: Namespaces::PruneAggregationSchedulesWorker :feature_category: :source_code_management diff --git a/app/workers/background_migration/single_database_worker.rb b/app/workers/background_migration/single_database_worker.rb index 2f797a24468..56800c03bbb 100644 --- a/app/workers/background_migration/single_database_worker.rb +++ b/app/workers/background_migration/single_database_worker.rb @@ -45,7 +45,10 @@ module BackgroundMigration # lease on the class before giving up. See MR for more discussion. # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45298#note_434304956 def perform(class_name, arguments = [], lease_attempts = MAX_LEASE_ATTEMPTS) - unless Feature.enabled?(:execute_background_migrations, type: :ops) + should_skip = Feature.enabled?(:disallow_database_ddl_feature_flags, type: :ops) || + Feature.disabled?(:execute_background_migrations, type: :ops) + + if should_skip # Delay execution of background migrations self.class.perform_in(BACKGROUND_MIGRATIONS_DELAY, class_name, arguments, lease_attempts) diff --git a/app/workers/database/batched_background_migration/execution_worker.rb b/app/workers/database/batched_background_migration/execution_worker.rb index 1bdc829418a..75798f0ab73 100644 --- a/app/workers/database/batched_background_migration/execution_worker.rb +++ b/app/workers/database/batched_background_migration/execution_worker.rb @@ -64,6 +64,8 @@ module Database attr_accessor :database_name, :migration def enabled? + return false if Feature.enabled?(:disallow_database_ddl_feature_flags, type: :ops) + Feature.enabled?(:execute_batched_migrations_on_schedule, type: :ops) end diff --git a/app/workers/database/batched_background_migration/single_database_worker.rb b/app/workers/database/batched_background_migration/single_database_worker.rb index ebf63d34cbf..f73f8fd751b 100644 --- a/app/workers/database/batched_background_migration/single_database_worker.rb +++ b/app/workers/database/batched_background_migration/single_database_worker.rb @@ -27,6 +27,8 @@ module Database # :nocov: def enabled? + return false if Feature.enabled?(:disallow_database_ddl_feature_flags, type: :ops) + Feature.enabled?(:execute_batched_migrations_on_schedule, type: :ops) end diff --git a/app/workers/namespaces/in_product_marketing_emails_worker.rb b/app/workers/namespaces/in_product_marketing_emails_worker.rb deleted file mode 100644 index 470fba1227d..00000000000 --- a/app/workers/namespaces/in_product_marketing_emails_worker.rb +++ /dev/null @@ -1,33 +0,0 @@ -# frozen_string_literal: true - -module Namespaces - class InProductMarketingEmailsWorker # rubocop:disable Scalability/IdempotentWorker - include ApplicationWorker - - data_consistency :always - - include CronjobQueue # rubocop:disable Scalability/CronWorkerContext - - feature_category :experimentation_activation - urgency :low - - def perform - return if paid_self_managed_instance? - return if setting_disabled? - - Namespaces::InProductMarketingEmailsService.send_for_all_tracks_and_intervals - end - - private - - def paid_self_managed_instance? - false - end - - def setting_disabled? - !Gitlab::CurrentSettings.in_product_marketing_emails_enabled - end - end -end - -Namespaces::InProductMarketingEmailsWorker.prepend_mod_with('Namespaces::InProductMarketingEmailsWorker') diff --git a/config/feature_flags/ops/disallow_database_ddl_feature_flags.yml b/config/feature_flags/ops/disallow_database_ddl_feature_flags.yml new file mode 100644 index 00000000000..4591c6aaf63 --- /dev/null +++ b/config/feature_flags/ops/disallow_database_ddl_feature_flags.yml @@ -0,0 +1,8 @@ +--- +name: disallow_database_ddl_feature_flags +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/130554 +rollout_issue_url: +milestone: '16.4' +type: ops +group: group::database +default_enabled: false diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index c87d54b9bd4..3819f77d508 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -634,9 +634,6 @@ Settings.cron_jobs['user_status_cleanup_batch_worker']['job_class'] = 'UserStatu Settings.cron_jobs['ssh_keys_expired_notification_worker'] ||= {} Settings.cron_jobs['ssh_keys_expired_notification_worker']['cron'] ||= '0 2,14 * * *' Settings.cron_jobs['ssh_keys_expired_notification_worker']['job_class'] = 'SshKeys::ExpiredNotificationWorker' -Settings.cron_jobs['namespaces_in_product_marketing_emails_worker'] ||= {} -Settings.cron_jobs['namespaces_in_product_marketing_emails_worker']['cron'] ||= '0 16 * * *' -Settings.cron_jobs['namespaces_in_product_marketing_emails_worker']['job_class'] = 'Namespaces::InProductMarketingEmailsWorker' Settings.cron_jobs['ssh_keys_expiring_soon_notification_worker'] ||= {} Settings.cron_jobs['ssh_keys_expiring_soon_notification_worker']['cron'] ||= '0 1 * * *' Settings.cron_jobs['ssh_keys_expiring_soon_notification_worker']['job_class'] = 'SshKeys::ExpiringSoonNotificationWorker' diff --git a/db/migrate/20230824015840_add_finding_id_to_vulnerabilities.rb b/db/migrate/20230824015840_add_finding_id_to_vulnerabilities.rb new file mode 100644 index 00000000000..a8a8e8c8757 --- /dev/null +++ b/db/migrate/20230824015840_add_finding_id_to_vulnerabilities.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class AddFindingIdToVulnerabilities < Gitlab::Database::Migration[2.1] + def up + add_column :vulnerabilities, :finding_id, :bigint, if_not_exists: true + end + + def down + remove_column :vulnerabilities, :finding_id + end +end diff --git a/db/migrate/20230824022229_make_finding_id_on_vulnerabilities_invalid_foreign_key.rb b/db/migrate/20230824022229_make_finding_id_on_vulnerabilities_invalid_foreign_key.rb new file mode 100644 index 00000000000..e6ac5ea95ab --- /dev/null +++ b/db/migrate/20230824022229_make_finding_id_on_vulnerabilities_invalid_foreign_key.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class MakeFindingIdOnVulnerabilitiesInvalidForeignKey < Gitlab::Database::Migration[2.1] + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :vulnerabilities, :vulnerability_occurrences, + column: :finding_id, on_delete: :cascade, validate: false + end + + def down + remove_foreign_key_if_exists :vulnerabilities, column: :finding_id + end +end diff --git a/db/post_migrate/20230829120720_index_finding_id_for_vulnerabilities.rb b/db/post_migrate/20230829120720_index_finding_id_for_vulnerabilities.rb new file mode 100644 index 00000000000..2556134658f --- /dev/null +++ b/db/post_migrate/20230829120720_index_finding_id_for_vulnerabilities.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class IndexFindingIdForVulnerabilities < Gitlab::Database::Migration[2.1] + INDEX_NAME = 'index_vulnerabilities_on_finding_id' + + # TODO: Index to be created synchronously in https://gitlab.com/gitlab-org/gitlab/-/issues/423541 + def up + prepare_async_index :vulnerabilities, :finding_id, name: INDEX_NAME + end + + def down + unprepare_async_index :vulnerabilities, INDEX_NAME + end +end diff --git a/db/schema_migrations/20230824015840 b/db/schema_migrations/20230824015840 new file mode 100644 index 00000000000..934f3dffb55 --- /dev/null +++ b/db/schema_migrations/20230824015840 @@ -0,0 +1 @@ +14fee417b4fe7767f84068b2772e7fb9dd43df816af258d6b6d7e83bc3a2c176
\ No newline at end of file diff --git a/db/schema_migrations/20230824022229 b/db/schema_migrations/20230824022229 new file mode 100644 index 00000000000..ba7506b7cb4 --- /dev/null +++ b/db/schema_migrations/20230824022229 @@ -0,0 +1 @@ +cb2f7d874c5564c2aab48785043293abbd49803e39c466b1e1c2d47a3bc5cbfb
\ No newline at end of file diff --git a/db/schema_migrations/20230829120720 b/db/schema_migrations/20230829120720 new file mode 100644 index 00000000000..28155113a89 --- /dev/null +++ b/db/schema_migrations/20230829120720 @@ -0,0 +1 @@ +2d40c98a720cb5bd9518b7c9ede223e23178199733352301a8400d1d387230fe
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index ba55cd6f899..9c1fc3b5500 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -24404,7 +24404,8 @@ CREATE TABLE vulnerabilities ( dismissed_by_id bigint, resolved_on_default_branch boolean DEFAULT false NOT NULL, present_on_default_branch boolean DEFAULT true NOT NULL, - detected_at timestamp with time zone DEFAULT now() + detected_at timestamp with time zone DEFAULT now(), + finding_id bigint ); CREATE SEQUENCE vulnerabilities_id_seq @@ -36332,6 +36333,9 @@ ALTER TABLE ONLY sbom_occurrences ALTER TABLE ONLY namespace_commit_emails ADD CONSTRAINT fk_4d6ba63ba5 FOREIGN KEY (namespace_id) REFERENCES namespaces(id) ON DELETE CASCADE; +ALTER TABLE ONLY vulnerabilities + ADD CONSTRAINT fk_4e64972902 FOREIGN KEY (finding_id) REFERENCES vulnerability_occurrences(id) ON DELETE CASCADE NOT VALID; + ALTER TABLE ONLY ml_model_versions ADD CONSTRAINT fk_4e8b59e7a8 FOREIGN KEY (model_id) REFERENCES ml_models(id) ON DELETE CASCADE; diff --git a/doc/development/ai_features/index.md b/doc/development/ai_features/index.md index 6c1395b3b60..d95b6da1925 100644 --- a/doc/development/ai_features/index.md +++ b/doc/development/ai_features/index.md @@ -106,6 +106,11 @@ In order to obtain a GCP service key for local development, please follow the st - In the GCP console, go to `IAM & Admin` > `Service Accounts` and click on the "Create new service account" button - Name the service account something specific to what you're using it for. Select Create and Continue. Under `Grant this service account access to project`, select the role `Vertex AI User`. Select `Continue` then `Done` - Select your new service account and `Manage keys` > `Add Key` > `Create new key`. This will download the **private** JSON credentials for your service account. +- If you are using your own project, you may also need to enable the Vertex AI API: + 1. Go to **APIs & Services > Enabled APIs & services**. + 1. Select **+ Enable APIs and Services**. + 1. Search for `Vertex AI API`. + 1. Select **Vertex AI API**, then select **Enable**. - Open the Rails console. Update the settings to: ```ruby diff --git a/doc/integration/jenkins.md b/doc/integration/jenkins.md index a1f5f6a5234..462e9137c9d 100644 --- a/doc/integration/jenkins.md +++ b/doc/integration/jenkins.md @@ -33,8 +33,6 @@ The Jenkins integration requires configuration in both GitLab and Jenkins. ## Grant Jenkins access to the GitLab project -To grant Jenkins access to the GitLab project: - 1. Create a personal, project, or group access token. - [Create a personal access token](../user/profile/personal_access_tokens.md#create-a-personal-access-token) @@ -46,20 +44,22 @@ To grant Jenkins access to the GitLab project: to use the token for all Jenkins integrations in all projects of that group. 1. Set the access token scope to **API**. -1. Copy the access token value to [configure the Jenkins server](#configure-the-jenkins-server). +1. Copy the access token value to configure the Jenkins server. ## Configure the Jenkins server -Install and configure the Jenkins plugin. The plugin must be installed and configured to -authorize the connection to GitLab. +Install and configure the Jenkins plugin to authorize the connection to GitLab. 1. On the Jenkins server, select **Manage Jenkins > Manage Plugins**. -1. Install the [Jenkins GitLab Plugin](https://wiki.jenkins.io/display/JENKINS/GitLab+Plugin). +1. Select the **Available** tab. Search for `gitlab-plugin` and select it to install. + See the [Jenkins GitLab documentation](https://wiki.jenkins.io/display/JENKINS/GitLab+Plugin) + for other ways to install the plugin. 1. Select **Manage Jenkins > Configure System**. 1. In the **GitLab** section, select **Enable authentication for '/project' end-point**. 1. Select **Add**, then choose **Jenkins Credential Provider**. 1. Select **GitLab API token** as the token type. -1. In **API Token**, [paste the value you copied from GitLab](#grant-jenkins-access-to-the-gitlab-project) and select **Add**. +1. In **API Token**, [paste the access token value you copied from GitLab](#grant-jenkins-access-to-the-gitlab-project) + and select **Add**. 1. Enter the GitLab server's URL in **GitLab host URL**. 1. To test the connection, select **Test Connection**. @@ -72,21 +72,21 @@ For more information, see Set up the Jenkins project you intend to run your build on. -1. On your Jenkins instance, go to **New Item**. +1. On your Jenkins instance, select **New Item**. 1. Enter the project's name. 1. Select **Freestyle** or **Pipeline** and select **OK**. - We recommend a Freestyle project, because the Jenkins plugin updates the build status on - GitLab. In a Pipeline project, you must configure a script to update the status on GitLab. + You should select a freestyle project, because the Jenkins plugin updates the build status on + GitLab. In a pipeline project, you must configure a script to update the status on GitLab. 1. Choose your GitLab connection from the dropdown list. 1. Select **Build when a change is pushed to GitLab**. 1. Select the following checkboxes: - **Accepted Merge Request Events** - **Closed Merge Request Events** 1. Specify how the build status is reported to GitLab: - - If you created a **Freestyle** project, in the **Post-build Actions** section, choose - **Publish build status to GitLab**. - - If you created a **Pipeline** project, you must use a Jenkins Pipeline script to update the status on - GitLab. + - If you created a freestyle project, in the **Post-build Actions** section, + choose **Publish build status to GitLab**. + - If you created a pipeline project, you must use a Jenkins Pipeline script to + update the status on GitLab. Example Jenkins Pipeline script: @@ -106,7 +106,8 @@ Set up the Jenkins project you intend to run your build on. } ``` - For more Jenkins Pipeline script examples, go to the [Jenkins GitLab plugin repository on GitHub](https://github.com/jenkinsci/gitlab-plugin#scripted-pipeline-jobs). + For more Jenkins Pipeline script examples, see the + [Jenkins GitLab plugin repository on GitHub](https://github.com/jenkinsci/gitlab-plugin#scripted-pipeline-jobs). ## Configure the GitLab project diff --git a/doc/update/background_migrations.md b/doc/update/background_migrations.md index 6cbfae37c16..5a4b19016f8 100644 --- a/doc/update/background_migrations.md +++ b/doc/update/background_migrations.md @@ -276,6 +276,8 @@ use the information in the failure error logs or the database: sudo gitlab-rake gitlab:background_migrations:finalize[<job_class_name>,<table_name>,<column_name>,'<job_arguments>'] ``` + When dealing with multiple arguments, such as `[["id"],["id_convert_to_bigint"]]`, escape the + comma between each argument with a backslash <code>\</code> to prevent an invalid character error. For example, to finish the migration from the previous step: ```shell @@ -300,11 +302,13 @@ use the information in the failure error logs or the database: - `column_name`: `id` - `job_arguments`: `[["id"], ["id_convert_to_bigint"]]` - The command should be: + When dealing with multiple arguments, such as `[["id"],["id_convert_to_bigint"]]`, escape the + comma between each argument with a backslash <code>\</code> to prevent an invalid character error. + The command should be: - ```shell - sudo gitlab-rake gitlab:background_migrations:finalize[CopyColumnUsingBackgroundMigrationJob,events,id,'[["id"]\, ["id_convert_to_bigint"]]'] - ``` + ```shell + sudo gitlab-rake gitlab:background_migrations:finalize[CopyColumnUsingBackgroundMigrationJob,events,id,'[["id"]\, ["id_convert_to_bigint"]]'] + ``` ::EndTabs diff --git a/doc/user/clusters/agent/gitops/flux_tutorial.md b/doc/user/clusters/agent/gitops/flux_tutorial.md index 156123c7937..27724a95291 100644 --- a/doc/user/clusters/agent/gitops/flux_tutorial.md +++ b/doc/user/clusters/agent/gitops/flux_tutorial.md @@ -120,12 +120,10 @@ To install `agentk`: apiVersion: v1 kind: Secret metadata: - name: gitlab-agent-token-initial + name: gitlab-agent-token type: Opaque stringData: - values.yaml: |- - config: - token: "<your-token-here>" + token: "<your-token-here>" ``` 1. Apply `secret.yaml` to your cluster: @@ -173,13 +171,10 @@ To install `agentk`: values: config: kasAddress: "wss://kas.gitlab.com" - valuesFrom: - - kind: Secret - name: gitlab-agent-token-initial - valuesKey: values.yaml + secretName: gitlab-agent-token ``` - The Helm release creates a new secret with the name `gitlab-agent-token`, which is managed by Helm. + The Helm release uses the secret from the previous step. 1. To verify that `agentk` is installed and running in the cluster, run the following command: diff --git a/doc/user/packages/composer_repository/index.md b/doc/user/packages/composer_repository/index.md index 0fcc44e8780..40fef13e590 100644 --- a/doc/user/packages/composer_repository/index.md +++ b/doc/user/packages/composer_repository/index.md @@ -4,7 +4,7 @@ group: Package Registry info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments --- -# Composer packages in the Package Registry **(FREE ALL)** +# Composer packages in the Package Registry **(FREE ALL BETA)** > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/15886) in GitLab 13.2. > - [Moved](https://gitlab.com/gitlab-org/gitlab/-/issues/221259) from GitLab Premium to GitLab Free in 13.3. diff --git a/doc/user/packages/conan_repository/index.md b/doc/user/packages/conan_repository/index.md index 72c5a1980b5..74152515198 100644 --- a/doc/user/packages/conan_repository/index.md +++ b/doc/user/packages/conan_repository/index.md @@ -4,7 +4,7 @@ group: Package Registry info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments --- -# Conan packages in the Package Registry **(FREE ALL)** +# Conan packages in the Package Registry **(FREE ALL EXPERIMENT)** > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/8248) in GitLab 12.6. > - [Moved](https://gitlab.com/gitlab-org/gitlab/-/issues/221259) from GitLab Premium to GitLab Free in 13.3. diff --git a/doc/user/packages/debian_repository/index.md b/doc/user/packages/debian_repository/index.md index c3e35fc4f06..e42a75da8a7 100644 --- a/doc/user/packages/debian_repository/index.md +++ b/doc/user/packages/debian_repository/index.md @@ -4,7 +4,7 @@ group: Package Registry info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments --- -# Debian packages in the Package Registry **(FREE ALL)** +# Debian packages in the Package Registry **(FREE ALL EXPERIMENT)** > - Debian API [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/42670) in GitLab 13.5. > - Debian group API [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/66188) in GitLab 14.2. @@ -12,7 +12,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w WARNING: The Debian package registry for GitLab is under development and isn't ready for production use. This [epic](https://gitlab.com/groups/gitlab-org/-/epics/6057) details the remaining -work and timelines to make it production ready. +work and timelines to make it production ready. Support for [Debian packages is an experiment](../package_registry/supported_package_managers.md), and has known security vulnerabilities. Publish Debian packages in your project's Package Registry. Then install the packages whenever you need to use them as a dependency. diff --git a/doc/user/packages/go_proxy/index.md b/doc/user/packages/go_proxy/index.md index 52f98ecf4dc..6dffd7371b6 100644 --- a/doc/user/packages/go_proxy/index.md +++ b/doc/user/packages/go_proxy/index.md @@ -4,7 +4,7 @@ group: Package Registry info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments --- -# Go proxy for GitLab **(FREE ALL)** +# Go proxy for GitLab **(FREE ALL EXPERIMENT)** > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/27376) in GitLab 13.1. > - It's deployed behind a feature flag, disabled by default. diff --git a/doc/user/packages/helm_repository/index.md b/doc/user/packages/helm_repository/index.md index 57aee4fb4ca..b7888fe2d18 100644 --- a/doc/user/packages/helm_repository/index.md +++ b/doc/user/packages/helm_repository/index.md @@ -4,7 +4,7 @@ group: Package Registry info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments --- -# Helm charts in the Package Registry **(FREE ALL)** +# Helm charts in the Package Registry **(FREE ALL BETA)** > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/18997) in GitLab 14.1. diff --git a/doc/user/packages/rubygems_registry/index.md b/doc/user/packages/rubygems_registry/index.md index 1c898ee6d92..361114e6f9e 100644 --- a/doc/user/packages/rubygems_registry/index.md +++ b/doc/user/packages/rubygems_registry/index.md @@ -4,7 +4,7 @@ group: Package Registry info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments --- -# Ruby gems in the Package Registry **(FREE ALL)** +# Ruby gems in the Package Registry **(FREE ALL EXPERIMENT)** > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/803) in GitLab 13.10. diff --git a/lib/gitlab/database/partitioning.rb b/lib/gitlab/database/partitioning.rb index 48f58920d52..5e6f7024f00 100644 --- a/lib/gitlab/database/partitioning.rb +++ b/lib/gitlab/database/partitioning.rb @@ -27,6 +27,8 @@ module Gitlab end def sync_partitions(models_to_sync = registered_for_sync, only_on: nil) + return if Feature.enabled?(:disallow_database_ddl_feature_flags, type: :ops) + return unless Feature.enabled?(:partition_manager_sync_partitions, type: :ops) Gitlab::AppLogger.info(message: 'Syncing dynamic postgres partitions') @@ -60,6 +62,8 @@ module Gitlab end def drop_detached_partitions + return if Feature.enabled?(:disallow_database_ddl_feature_flags, type: :ops) + return unless Feature.enabled?(:partition_manager_sync_partitions, type: :ops) Gitlab::AppLogger.info(message: 'Dropping detached postgres partitions') diff --git a/lib/gitlab/database/reindexing.rb b/lib/gitlab/database/reindexing.rb index 4a1b0be848e..7eb07961345 100644 --- a/lib/gitlab/database/reindexing.rb +++ b/lib/gitlab/database/reindexing.rb @@ -16,6 +16,8 @@ module Gitlab REMOVE_INDEX_RETRY_CONFIG = [[1.minute, 9.minutes]] * 30 def self.enabled? + return false if Feature.enabled?(:disallow_database_ddl_feature_flags, type: :ops) + Feature.enabled?(:database_reindexing, type: :ops) end @@ -26,9 +28,15 @@ module Gitlab Gitlab::Database::SharedModel.logger = Logger.new($stdout) if Gitlab::Utils.to_boolean(ENV['LOG_QUERIES_TO_CONSOLE'], default: false) # Hack: Before we do actual reindexing work, create async indexes - Gitlab::Database::AsyncIndexes.create_pending_indexes! if Feature.enabled?(:database_async_index_creation, type: :ops) + if Feature.disabled?(:disallow_database_ddl_feature_flags, type: :ops) && Feature.enabled?(:database_async_index_creation, type: :ops) + Gitlab::Database::AsyncIndexes.create_pending_indexes! + end + Gitlab::Database::AsyncIndexes.drop_pending_indexes! - Gitlab::Database::AsyncConstraints.validate_pending_entries! if Feature.enabled?(:database_async_foreign_key_validation, type: :ops) + + if Feature.disabled?(:disallow_database_ddl_feature_flags, type: :ops) && Feature.enabled?(:database_async_foreign_key_validation, type: :ops) + Gitlab::Database::AsyncConstraints.validate_pending_entries! + end automatic_reindexing end diff --git a/lib/tasks/gitlab/db.rake b/lib/tasks/gitlab/db.rake index 546f5621515..cf52a219e83 100644 --- a/lib/tasks/gitlab/db.rake +++ b/lib/tasks/gitlab/db.rake @@ -258,6 +258,18 @@ namespace :gitlab do end end + def disabled_db_flags_note + return unless Feature.enabled?(:disallow_database_ddl_feature_flags, type: :ops) + + puts <<~NOTE.color(:yellow) + Note: disallow_database_ddl_feature_flags feature is currently enabled. Disable it to proceed. + + Disable with: Feature.disable(:disallow_database_ddl_feature_flags) + NOTE + + yield if block_given? + end + desc 'Enqueue an index for reindexing' task :enqueue_reindexing_action, [:index_name, :database] => :environment do |_, args| model = Gitlab::Database.database_base_models[args.fetch(:database, Gitlab::Database::PRIMARY_DATABASE_NAME)] @@ -269,7 +281,9 @@ namespace :gitlab do puts "There are #{Gitlab::Database::Reindexing::QueuedAction.queued.size} queued actions in total." end - unless Feature.enabled?(:database_reindexing, type: :ops) + disabled_db_flags_note + + if Feature.disabled?(:database_reindexing, type: :ops) puts <<~NOTE.color(:yellow) Note: database_reindexing feature is currently disabled. @@ -283,6 +297,8 @@ namespace :gitlab do task database_name, [:pick] => :environment do |_, args| args.with_defaults(pick: 2) + disabled_db_flags_note { exit } + if Feature.disabled?(:database_async_index_operations, type: :ops) puts <<~NOTE.color(:yellow) Note: database async index operations feature is currently disabled. @@ -313,6 +329,8 @@ namespace :gitlab do task database_name, [:pick] => :environment do |_, args| args.with_defaults(pick: 2) + disabled_db_flags_note { exit } + if Feature.disabled?(:database_async_foreign_key_validation, type: :ops) puts <<~NOTE.color(:yellow) Note: database async foreign key validation feature is currently disabled. diff --git a/spec/db/schema_spec.rb b/spec/db/schema_spec.rb index fbd9c4d32cc..04a83a1f6ab 100644 --- a/spec/db/schema_spec.rb +++ b/spec/db/schema_spec.rb @@ -14,7 +14,8 @@ RSpec.describe 'Database schema', feature_category: :database do # but in Search::NamespaceIndexAssignment model, only `search_index_id` is used as foreign key and indexed search_namespace_index_assignments: [%w[search_index_id index_type]], slack_integrations_scopes: [%w[slack_api_scope_id]], - notes: %w[namespace_id] # this index is added in an async manner, hence it needs to be ignored in the first phase. + notes: %w[namespace_id], # this index is added in an async manner, hence it needs to be ignored in the first phase. + vulnerabilities: [%w[finding_id]] # index will be created in https://gitlab.com/gitlab-org/gitlab/-/issues/423541 }.with_indifferent_access.freeze TABLE_PARTITIONS = %w[ci_builds_metadata].freeze diff --git a/spec/graphql/mutations/design_management/delete_spec.rb b/spec/graphql/mutations/design_management/delete_spec.rb index 2ec5a7abe87..9a2efb61e55 100644 --- a/spec/graphql/mutations/design_management/delete_spec.rb +++ b/spec/graphql/mutations/design_management/delete_spec.rb @@ -90,12 +90,12 @@ RSpec.describe Mutations::DesignManagement::Delete do allow(Gitlab::Tracking).to receive(:event) # rubocop:disable RSpec/ExpectGitlabTracking filenames.each(&:present?) # ignore setup - # Queries: as of 2022-06-15 + # Queries: as of 2022-08-30 # ------------- # 01. routing query - # 02. find project by id - # 03. project.project_features - # 04. find namespace by id and type + # 02. policy query: find namespace by type and id + # 03. policy query: find namespace by id + # 04. policy query: project.project_feature # 05,06. project.authorizations for user (same query twice) # 07. find issue by iid # 08. find project by id @@ -109,19 +109,20 @@ RSpec.describe Mutations::DesignManagement::Delete do # 16, 17 project.authorizations for user (same query as 5) # 18. find design_management_repository for project # 19. find route by id and source_type - # 20. find plan for standard context # ------------- our queries are below: - # 21. start transaction 1 - # 22. start transaction 2 - # 23. find version by sha and issue - # 24. exists version with sha and issue? - # 25. leave transaction 2 - # 26. create version with sha and issue - # 27. create design-version links - # 28. validate version.actions.present? - # 29. validate version.issue.present? - # 30. validate version.sha is unique - # 31. leave transaction 1 + # 20. start transaction + # 21. create version with sha and issue + # 22. create design-version links + # 23. validate version.actions.present? + # 24. validate version.sha is unique + # 25. validate version.issue.present? + # 26. leave transaction + # 27. find project by id (same query as 8) + # 28. find namespace by id (same query as 9) + # 29. find project by id (same query as 8) + # 30. find project by id (same query as 8) + # 31. create event + # 32. find plan for standard context # expect { run_mutation }.not_to exceed_query_limit(32) end diff --git a/spec/lib/gitlab/database/partitioning_spec.rb b/spec/lib/gitlab/database/partitioning_spec.rb index a1ae75ac916..b6b7d293778 100644 --- a/spec/lib/gitlab/database/partitioning_spec.rb +++ b/spec/lib/gitlab/database/partitioning_spec.rb @@ -8,6 +8,10 @@ RSpec.describe Gitlab::Database::Partitioning, feature_category: :database do let(:main_connection) { ApplicationRecord.connection } + before do + stub_feature_flags(disallow_database_ddl_feature_flags: false) + end + around do |example| previously_registered_models = described_class.registered_models.dup described_class.instance_variable_set(:@registered_models, Set.new) @@ -165,11 +169,11 @@ RSpec.describe Gitlab::Database::Partitioning, feature_category: :database do execute_on_each_database("DROP TABLE IF EXISTS #{table_name}") execute_on_each_database(<<~SQL) - CREATE TABLE #{table_name} ( - id serial not null, - created_at timestamptz not null, - PRIMARY KEY (id, created_at)) - PARTITION BY RANGE (created_at); + CREATE TABLE #{table_name} ( + id serial not null, + created_at timestamptz not null, + PRIMARY KEY (id, created_at)) + PARTITION BY RANGE (created_at); SQL end end @@ -204,6 +208,20 @@ RSpec.describe Gitlab::Database::Partitioning, feature_category: :database do described_class.sync_partitions(models) end end + + context 'when disallow_database_ddl_feature_flags feature flag is enabled' do + before do + described_class.register_models(models) + stub_feature_flags(disallow_database_ddl_feature_flags: true) + end + + it 'skips sync_partitions' do + expect(described_class::PartitionManager).not_to receive(:new) + expect(described_class).to receive(:sync_partitions).and_call_original + + described_class.sync_partitions(models) + end + end end describe '.report_metrics' do @@ -277,6 +295,18 @@ RSpec.describe Gitlab::Database::Partitioning, feature_category: :database do end end + context 'when the feature disallow DDL feature flags is enabled' do + before do + stub_feature_flags(disallow_database_ddl_feature_flags: true) + end + + it 'does not call the DetachedPartitionDropper' do + expect(Gitlab::Database::Partitioning::DetachedPartitionDropper).not_to receive(:new) + + described_class.drop_detached_partitions + end + end + def table_exists?(table_name) table_oid(table_name).present? end diff --git a/spec/lib/gitlab/database/reindexing_spec.rb b/spec/lib/gitlab/database/reindexing_spec.rb index 851fc7ea3cd..441f6476abe 100644 --- a/spec/lib/gitlab/database/reindexing_spec.rb +++ b/spec/lib/gitlab/database/reindexing_spec.rb @@ -6,6 +6,10 @@ RSpec.describe Gitlab::Database::Reindexing, feature_category: :database, time_t include ExclusiveLeaseHelpers include Database::DatabaseHelpers + before do + stub_feature_flags(disallow_database_ddl_feature_flags: false) + end + describe '.invoke' do let(:databases) { Gitlab::Database.database_base_models_with_gitlab_shared } let(:databases_count) { databases.count } @@ -44,6 +48,14 @@ RSpec.describe Gitlab::Database::Reindexing, feature_category: :database, time_t described_class.invoke end + + it 'does not execute async index creation when disable ddl flag is enabled' do + stub_feature_flags(disallow_database_ddl_feature_flags: true) + + expect(Gitlab::Database::AsyncIndexes).not_to receive(:create_pending_indexes!) + + described_class.invoke + end end it 'executes async index destruction prior to any reindexing actions' do @@ -86,6 +98,14 @@ RSpec.describe Gitlab::Database::Reindexing, feature_category: :database, time_t described_class.invoke end + + it 'does not execute async index creation when disable ddl flag is enabled' do + stub_feature_flags(disallow_database_ddl_feature_flags: true) + + expect(Gitlab::Database::AsyncIndexes).not_to receive(:validate_pending_entries!) + + described_class.invoke + end end end diff --git a/spec/mailers/emails/in_product_marketing_spec.rb b/spec/mailers/emails/in_product_marketing_spec.rb index 2d332dd99d6..93a06bfc881 100644 --- a/spec/mailers/emails/in_product_marketing_spec.rb +++ b/spec/mailers/emails/in_product_marketing_spec.rb @@ -25,75 +25,6 @@ RSpec.describe Emails::InProductMarketing do end end - describe '#in_product_marketing_email' do - let_it_be(:group) { create(:group) } - - let!(:onboarding_progress) { create(:onboarding_progress, namespace: group) } - - using RSpec::Parameterized::TableSyntax - - let(:track) { :create } - let(:series) { 0 } - - subject { Notify.in_product_marketing_email(user.id, group.id, track, series) } - - include_context 'gitlab email notification' - - it_behaves_like 'has custom headers when on gitlab.com' - - it 'sends to the right user with a link to unsubscribe' do - aggregate_failures do - expect(subject).to deliver_to(user.notification_email_or_default) - expect(subject).to have_body_text(profile_notifications_url) - end - end - - where(:track, :series) do - :create | 0 - :create | 1 - :create | 2 - :verify | 0 - :verify | 1 - :verify | 2 - :trial | 0 - :trial | 1 - :trial | 2 - :team | 0 - :team | 1 - :team | 2 - :team_short | 0 - :trial_short | 0 - :admin_verify | 0 - end - - with_them do - before do - group.add_owner(user) - end - - it 'has the correct subject and content' do - message = Gitlab::Email::Message::InProductMarketing.for(track).new(group: group, user: user, series: series) - - aggregate_failures do - is_expected.to have_subject(message.subject_line) - is_expected.to have_body_text(message.title) - is_expected.to have_body_text(message.subtitle) - is_expected.to have_body_text(CGI.unescapeHTML(message.cta_link)) - - if /create|verify/.match?(track) - is_expected.to have_body_text(message.invite_text) - is_expected.to have_body_text(CGI.unescapeHTML(message.invite_link)) - else - is_expected.not_to have_body_text(message.invite_text) - is_expected.not_to have_body_text(CGI.unescapeHTML(message.invite_link)) - end - - is_expected.to have_body_text(message.progress) - end - end - end - end - describe '#build_ios_app_guide_email' do subject { Notify.build_ios_app_guide_email(user.notification_email_or_default) } diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 79ba71b0830..a0de3013a87 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -2534,17 +2534,4 @@ RSpec.describe Notify do end end end - - describe 'in product marketing', :mailer do - let_it_be(:group) { create(:group) } - - let(:mail) { ActionMailer::Base.deliveries.last } - - it 'does not raise error' do - described_class.in_product_marketing_email(user.id, group.id, :trial, 0).deliver - - expect(mail.subject).to eq('Go farther with GitLab') - expect(mail.body.parts.first.to_s).to include('Start a GitLab Ultimate trial today in less than one minute, no credit card required.') - end - end end diff --git a/spec/services/namespaces/in_product_marketing_emails_service_spec.rb b/spec/services/namespaces/in_product_marketing_emails_service_spec.rb deleted file mode 100644 index 8a2ecd5c3e0..00000000000 --- a/spec/services/namespaces/in_product_marketing_emails_service_spec.rb +++ /dev/null @@ -1,216 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Namespaces::InProductMarketingEmailsService, '#execute', feature_category: :purchase do - subject(:execute_service) { described_class.new(track, interval).execute } - - let(:track) { :create } - let(:interval) { 1 } - - let(:frozen_time) { Time.zone.parse('23 Mar 2021 10:14:40 UTC') } - let(:previous_action_completed_at) { frozen_time - 2.days } - let(:current_action_completed_at) { nil } - let(:user_can_perform_current_track_action) { true } - let(:actions_completed) { { created_at: previous_action_completed_at, git_write_at: current_action_completed_at } } - - let_it_be(:group) { create(:group) } - let_it_be(:user) { create(:user, email_opted_in: true) } - - before do - travel_to(frozen_time) - create(:onboarding_progress, namespace: group, **actions_completed) - group.add_developer(user) - allow(Ability).to receive(:allowed?).with(user, anything, anything).and_return(user_can_perform_current_track_action) - allow(Notify).to receive(:in_product_marketing_email).and_return(double(deliver_later: nil)) - end - - RSpec::Matchers.define :send_in_product_marketing_email do |*args| - match do - expect(Notify).to have_received(:in_product_marketing_email).with(*args).once - end - - match_when_negated do - expect(Notify).not_to have_received(:in_product_marketing_email) - end - end - - context 'for each track and series with the right conditions' do - using RSpec::Parameterized::TableSyntax - - where(:track, :interval, :actions_completed) do - :create | 1 | { created_at: frozen_time - 2.days } - :create | 5 | { created_at: frozen_time - 6.days } - :create | 10 | { created_at: frozen_time - 11.days } - :team_short | 1 | { created_at: frozen_time - 2.days, git_write_at: frozen_time - 2.days } - :trial_short | 2 | { created_at: frozen_time - 3.days, git_write_at: frozen_time - 3.days } - :admin_verify | 3 | { created_at: frozen_time - 4.days, git_write_at: frozen_time - 4.days } - :verify | 4 | { created_at: frozen_time - 5.days, git_write_at: frozen_time - 5.days } - :verify | 8 | { created_at: frozen_time - 9.days, git_write_at: frozen_time - 9.days } - :verify | 13 | { created_at: frozen_time - 14.days, git_write_at: frozen_time - 14.days } - :trial | 1 | { created_at: frozen_time - 2.days, git_write_at: frozen_time - 2.days, pipeline_created_at: frozen_time - 2.days } - :trial | 5 | { created_at: frozen_time - 6.days, git_write_at: frozen_time - 6.days, pipeline_created_at: frozen_time - 6.days } - :trial | 10 | { created_at: frozen_time - 11.days, git_write_at: frozen_time - 11.days, pipeline_created_at: frozen_time - 11.days } - :team | 1 | { created_at: frozen_time - 2.days, git_write_at: frozen_time - 2.days, pipeline_created_at: frozen_time - 2.days, trial_started_at: frozen_time - 2.days } - :team | 5 | { created_at: frozen_time - 6.days, git_write_at: frozen_time - 6.days, pipeline_created_at: frozen_time - 6.days, trial_started_at: frozen_time - 6.days } - :team | 10 | { created_at: frozen_time - 11.days, git_write_at: frozen_time - 11.days, pipeline_created_at: frozen_time - 11.days, trial_started_at: frozen_time - 11.days } - end - - with_them do - it { is_expected.to send_in_product_marketing_email(user.id, group.id, track, described_class::TRACKS[track][:interval_days].index(interval)) } - end - end - - context 'when initialized with a different track' do - let(:track) { :team_short } - - it { is_expected.not_to send_in_product_marketing_email } - - context 'when the previous track actions have been completed' do - let(:current_action_completed_at) { frozen_time - 2.days } - - it { is_expected.to send_in_product_marketing_email(user.id, group.id, track, 0) } - end - end - - context 'when initialized with a different interval' do - let(:interval) { 5 } - - it { is_expected.not_to send_in_product_marketing_email } - - context 'when the previous track action was completed within the intervals range' do - let(:previous_action_completed_at) { frozen_time - 6.days } - - it { is_expected.to send_in_product_marketing_email(user.id, group.id, :create, 1) } - end - end - - context 'when the previous track action is not yet completed' do - let(:previous_action_completed_at) { nil } - - it { is_expected.not_to send_in_product_marketing_email } - end - - context 'when the previous track action is completed outside the intervals range' do - let(:previous_action_completed_at) { frozen_time - 3.days } - - it { is_expected.not_to send_in_product_marketing_email } - end - - context 'when the current track action is completed' do - let(:current_action_completed_at) { frozen_time } - - it { is_expected.not_to send_in_product_marketing_email } - end - - context "when the user cannot perform the current track's action" do - let(:user_can_perform_current_track_action) { false } - - it { is_expected.not_to send_in_product_marketing_email } - end - - context 'when the user has not opted into marketing emails' do - let(:user) { create(:user, email_opted_in: false) } - - it { is_expected.not_to send_in_product_marketing_email } - end - - describe 'do not send emails twice' do - subject { described_class.send_for_all_tracks_and_intervals } - - let(:user) { create(:user, email_opted_in: true) } - - context 'when user already got a specific email' do - before do - create(:in_product_marketing_email, user: user, track: track, series: 0) - end - - it { is_expected.not_to send_in_product_marketing_email(user.id, anything, track, 0) } - end - - context 'when user already got sent the whole track' do - before do - 0.upto(2) do |series| - create(:in_product_marketing_email, user: user, track: track, series: series) - end - end - - it 'does not send any of the emails anymore', :aggregate_failures do - 0.upto(2) do |series| - expect(subject).not_to send_in_product_marketing_email(user.id, anything, track, series) - end - end - end - - context 'when user is in two groups' do - let(:other_group) { create(:group) } - - before do - other_group.add_developer(user) - end - - context 'when both groups would get the same email' do - before do - create(:onboarding_progress, namespace: other_group, **actions_completed) - end - - it 'does not send the same email twice' do - subject - - expect(Notify).to have_received(:in_product_marketing_email).with(user.id, anything, :create, 0).once - end - end - - context 'when other group gets a different email' do - before do - create(:onboarding_progress, namespace: other_group, created_at: previous_action_completed_at, git_write_at: frozen_time - 2.days) - end - - it 'sends both emails' do - subject - - expect(Notify).to have_received(:in_product_marketing_email).with(user.id, group.id, :create, 0) - expect(Notify).to have_received(:in_product_marketing_email).with(user.id, other_group.id, :team_short, 0) - end - end - end - end - - it 'records sent emails' do - expect { subject }.to change { Users::InProductMarketingEmail.count }.by(1) - - expect( - Users::InProductMarketingEmail.where( - user: user, - track: Users::InProductMarketingEmail::ACTIVE_TRACKS[:create], - series: 0 - ) - ).to exist - end - - context 'when invoked with a non existing track' do - let(:track) { :foo } - - before do - stub_const("#{described_class}::TRACKS", { bar: {} }) - end - - it { expect { subject }.to raise_error(ArgumentError, 'Track foo not defined') } - end - - context 'when group is a sub-group' do - let(:root_group) { create(:group) } - let(:group) { create(:group) } - - before do - group.parent = root_group - group.save! - - allow(Ability).to receive(:allowed?).and_call_original - end - - it 'does not raise an exception' do - expect { execute_service }.not_to raise_error - end - end -end diff --git a/spec/support/rspec_order_todo.yml b/spec/support/rspec_order_todo.yml index e1c146aa677..5d82d42c1c1 100644 --- a/spec/support/rspec_order_todo.yml +++ b/spec/support/rspec_order_todo.yml @@ -2827,7 +2827,6 @@ - './ee/spec/services/milestones/destroy_service_spec.rb' - './ee/spec/services/milestones/promote_service_spec.rb' - './ee/spec/services/milestones/update_service_spec.rb' -- './ee/spec/services/namespaces/in_product_marketing_emails_service_spec.rb' - './ee/spec/services/namespaces/storage/email_notification_service_spec.rb' - './ee/spec/services/path_locks/lock_service_spec.rb' - './ee/spec/services/path_locks/unlock_service_spec.rb' @@ -3132,7 +3131,6 @@ - './ee/spec/workers/ee/arkose/blocked_users_report_worker_spec.rb' - './ee/spec/workers/ee/ci/build_finished_worker_spec.rb' - './ee/spec/workers/ee/issuable_export_csv_worker_spec.rb' -- './ee/spec/workers/ee/namespaces/in_product_marketing_emails_worker_spec.rb' - './ee/spec/workers/ee/namespaces/root_statistics_worker_spec.rb' - './ee/spec/workers/ee/projects/inactive_projects_deletion_cron_worker_spec.rb' - './ee/spec/workers/ee/repository_check/batch_worker_spec.rb' @@ -9322,7 +9320,6 @@ - './spec/services/milestones/transfer_service_spec.rb' - './spec/services/milestones/update_service_spec.rb' - './spec/services/namespace_settings/update_service_spec.rb' -- './spec/services/namespaces/in_product_marketing_emails_service_spec.rb' - './spec/services/namespaces/package_settings/update_service_spec.rb' - './spec/services/namespaces/statistics_refresher_service_spec.rb' - './spec/services/notes/build_service_spec.rb' @@ -10125,7 +10122,6 @@ - './spec/workers/metrics/dashboard/schedule_annotations_prune_worker_spec.rb' - './spec/workers/metrics/dashboard/sync_dashboards_worker_spec.rb' - './spec/workers/migrate_external_diffs_worker_spec.rb' -- './spec/workers/namespaces/in_product_marketing_emails_worker_spec.rb' - './spec/workers/namespaces/process_sync_events_worker_spec.rb' - './spec/workers/namespaces/prune_aggregation_schedules_worker_spec.rb' - './spec/workers/namespaces/root_statistics_worker_spec.rb' diff --git a/spec/support/shared_examples/workers/background_migration_worker_shared_examples.rb b/spec/support/shared_examples/workers/background_migration_worker_shared_examples.rb index 8ecb04bfdd6..1ea5eb6fd2e 100644 --- a/spec/support/shared_examples/workers/background_migration_worker_shared_examples.rb +++ b/spec/support/shared_examples/workers/background_migration_worker_shared_examples.rb @@ -1,6 +1,10 @@ # frozen_string_literal: true RSpec.shared_examples 'it runs background migration jobs' do |tracking_database| + before do + stub_feature_flags(disallow_database_ddl_feature_flags: false) + end + describe 'defining the job attributes' do it 'defines the data_consistency as always' do expect(described_class.get_data_consistency).to eq(:always) @@ -74,6 +78,38 @@ RSpec.shared_examples 'it runs background migration jobs' do |tracking_database| end end + context 'when disallow_database_ddl_feature_flags feature flag is disabled' do + before do + stub_feature_flags(disallow_database_ddl_feature_flags: true) + end + + it 'does not perform the job, reschedules it in the future, and logs a message' do + expect(worker).not_to receive(:perform_with_connection) + + expect(Sidekiq.logger).to receive(:info) do |payload| + expect(payload[:class]).to eq(described_class.name) + expect(payload[:database]).to eq(tracking_database) + expect(payload[:message]).to match(/skipping execution, migration rescheduled/) + end + + lease_attempts = 3 + delay = described_class::BACKGROUND_MIGRATIONS_DELAY + job_args = [10, 20] + + freeze_time do + worker.perform('Foo', job_args, lease_attempts) + + job = described_class.jobs.find { |job| job['args'] == ['Foo', job_args, lease_attempts] } + expect(job).to be, "Expected the job to be rescheduled with (#{job_args}, #{lease_attempts}), but it was not." + + expected_time = delay.to_i + Time.now.to_i + expect(job['at']).to eq(expected_time), + "Expected the job to be rescheduled in #{expected_time} seconds, " \ + "but it was rescheduled in #{job['at']} seconds." + end + end + end + context 'when execute_background_migrations feature flag is enabled' do before do stub_feature_flags(execute_background_migrations: true) diff --git a/spec/support/shared_examples/workers/batched_background_migration_execution_worker_shared_example.rb b/spec/support/shared_examples/workers/batched_background_migration_execution_worker_shared_example.rb index 8fdd59d1d8c..cf488a4d753 100644 --- a/spec/support/shared_examples/workers/batched_background_migration_execution_worker_shared_example.rb +++ b/spec/support/shared_examples/workers/batched_background_migration_execution_worker_shared_example.rb @@ -3,6 +3,10 @@ RSpec.shared_examples 'batched background migrations execution worker' do include ExclusiveLeaseHelpers + before do + stub_feature_flags(disallow_database_ddl_feature_flags: false) + end + it 'is a limited capacity worker' do expect(described_class.new).to be_a(LimitedCapacity::Worker) end @@ -100,6 +104,23 @@ RSpec.shared_examples 'batched background migrations execution worker' do end end + context 'when disable ddl flag is enabled' do + let(:migration) do + create(:batched_background_migration, :active, interval: job_interval, table_name: table_name) + end + + before do + stub_feature_flags(disallow_database_ddl_feature_flags: true) + end + + it 'does nothing' do + expect(Gitlab::Database::BackgroundMigration::BatchedMigration).not_to receive(:find_executable) + expect(worker).not_to receive(:run_migration_job) + + worker.perform_work(database_name, migration.id) + end + end + context 'when the feature flag is enabled' do before do stub_feature_flags(execute_batched_migrations_on_schedule: true) diff --git a/spec/support/shared_examples/workers/batched_background_migration_worker_shared_examples.rb b/spec/support/shared_examples/workers/batched_background_migration_worker_shared_examples.rb index e7385f9abb6..003b8d07819 100644 --- a/spec/support/shared_examples/workers/batched_background_migration_worker_shared_examples.rb +++ b/spec/support/shared_examples/workers/batched_background_migration_worker_shared_examples.rb @@ -3,6 +3,10 @@ RSpec.shared_examples 'it runs batched background migration jobs' do |tracking_database, table_name| include ExclusiveLeaseHelpers + before do + stub_feature_flags(disallow_database_ddl_feature_flags: false) + end + describe 'defining the job attributes' do it 'defines the data_consistency as always' do expect(described_class.get_data_consistency).to eq(:always) @@ -51,6 +55,12 @@ RSpec.shared_examples 'it runs batched background migration jobs' do |tracking_d expect(described_class.enabled?).to be_falsey end + + it 'returns false when disallow_database_ddl_feature_flags feature flag is enabled' do + stub_feature_flags(disallow_database_ddl_feature_flags: true) + + expect(described_class.enabled?).to be_falsey + end end describe '#perform' do @@ -116,6 +126,18 @@ RSpec.shared_examples 'it runs batched background migration jobs' do |tracking_d end end + context 'when the disallow_database_ddl_feature_flags feature flag is enabled' do + before do + stub_feature_flags(disallow_database_ddl_feature_flags: true) + end + + it 'does nothing' do + expect(worker).not_to receive(:queue_migrations_for_execution) + + worker.perform + end + end + context 'when the execute_batched_migrations_on_schedule feature flag is enabled' do let(:base_model) { Gitlab::Database.database_base_models[tracking_database] } let(:connection) { base_model.connection } diff --git a/spec/tasks/gitlab/db_rake_spec.rb b/spec/tasks/gitlab/db_rake_spec.rb index 344429dc6ec..3f4cc740954 100644 --- a/spec/tasks/gitlab/db_rake_spec.rb +++ b/spec/tasks/gitlab/db_rake_spec.rb @@ -18,6 +18,7 @@ RSpec.describe 'gitlab:db namespace rake task', :silence_stdout, feature_categor allow(Rake::Task['db:migrate']).to receive(:invoke).and_return(true) allow(Rake::Task['db:schema:load']).to receive(:invoke).and_return(true) allow(Rake::Task['db:seed_fu']).to receive(:invoke).and_return(true) + stub_feature_flags(disallow_database_ddl_feature_flags: false) end describe 'mark_migration_complete' do @@ -882,6 +883,16 @@ RSpec.describe 'gitlab:db namespace rake task', :silence_stdout, feature_categor end end + context 'when database ddl feature flag is enabled' do + it 'is a no-op' do + stub_feature_flags(disallow_database_ddl_feature_flags: true) + + expect(Gitlab::Database::AsyncIndexes).not_to receive(:execute_pending_actions!) + + expect { run_rake_task('gitlab:db:execute_async_index_operations:main') }.to raise_error(SystemExit) + end + end + context 'with geo configured' do before do skip_unless_geo_configured @@ -956,6 +967,16 @@ RSpec.describe 'gitlab:db namespace rake task', :silence_stdout, feature_categor end end + context 'when database ddl feature flag is enabled' do + it 'is a no-op' do + stub_feature_flags(disallow_database_ddl_feature_flags: true) + + expect(Gitlab::Database::AsyncConstraints).not_to receive(:validate_pending_entries!) + + expect { run_rake_task('gitlab:db:validate_async_constraints:main') }.to raise_error(SystemExit) + end + end + context 'with geo configured' do before do skip_unless_geo_configured diff --git a/spec/workers/namespaces/in_product_marketing_emails_worker_spec.rb b/spec/workers/namespaces/in_product_marketing_emails_worker_spec.rb deleted file mode 100644 index 237b5081bb1..00000000000 --- a/spec/workers/namespaces/in_product_marketing_emails_worker_spec.rb +++ /dev/null @@ -1,32 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Namespaces::InProductMarketingEmailsWorker, '#perform', unless: Gitlab.ee?, feature_category: :experimentation_activation do - # Running this in EE would call the overridden method, which can't be tested in CE. - # The EE code is covered in a separate EE spec. - - context 'when the in_product_marketing_emails_enabled setting is disabled' do - before do - stub_application_setting(in_product_marketing_emails_enabled: false) - end - - it 'does not execute the email service' do - expect(Namespaces::InProductMarketingEmailsService).not_to receive(:send_for_all_tracks_and_intervals) - - subject.perform - end - end - - context 'when the in_product_marketing_emails_enabled setting is enabled' do - before do - stub_application_setting(in_product_marketing_emails_enabled: true) - end - - it 'executes the email service' do - expect(Namespaces::InProductMarketingEmailsService).to receive(:send_for_all_tracks_and_intervals) - - subject.perform - end - end -end |