diff options
94 files changed, 678 insertions, 634 deletions
diff --git a/.gitignore b/.gitignore index 323741575de..9d9730bf406 100644 --- a/.gitignore +++ b/.gitignore @@ -42,7 +42,6 @@ eslint-report.html /config/redis.cache.yml /config/redis.queues.yml /config/redis.shared_state.yml -/config/redis.trace_chunks.yml /config/unicorn.rb /config/puma.rb /config/puma_actioncable.rb diff --git a/.gitlab/CODEOWNERS b/.gitlab/CODEOWNERS index eee9b638d78..56fe593c94d 100644 --- a/.gitlab/CODEOWNERS +++ b/.gitlab/CODEOWNERS @@ -201,7 +201,8 @@ Dangerfile @gl-quality/eng-prod [Templates] /lib/gitlab/ci/templates/ @nolith @shinya.maeda @matteeyah /lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml @DylanGriffith @mayra-cabrera @tkuah -/lib/gitlab/ci/templates/Security/ @plafoucriere @gonzoyumo @twoodham @sethgitlab +/lib/gitlab/ci/templates/Security/ @gonzoyumo @twoodham @sethgitlab @thiagocsf +/lib/gitlab/ci/templates/Security/Container-Scanning.*.yml @gitlab-org/protect/container-security-backend [Project Alias] /ee/app/models/project_alias.rb @patrickbajao diff --git a/.gitlab/ci/review.gitlab-ci.yml b/.gitlab/ci/review.gitlab-ci.yml index 28515917eb1..64d6f49576f 100644 --- a/.gitlab/ci/review.gitlab-ci.yml +++ b/.gitlab/ci/review.gitlab-ci.yml @@ -66,7 +66,7 @@ review-deploy: - *base-before_script script: - check_kube_domain - - ensure_namespace + - "ensure_namespace ${KUBE_NAMESPACE}" - install_external_dns - download_chart - date diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index f7879c2b570..49615757b04 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -102,6 +102,7 @@ - ".gitlab/ci/build-images.gitlab-ci.yml" - ".gitlab/ci/review.gitlab-ci.yml" - "scripts/review_apps/base-config.yaml" + - "scripts/review_apps/review-apps.sh" - "scripts/trigger-build" .ci-qa-patterns: &ci-qa-patterns @@ -489,9 +489,9 @@ gem 'google-protobuf', '~> 3.17.1' gem 'toml-rb', '~> 1.0.0' # Feature toggles -gem 'flipper', '~> 0.17.1' -gem 'flipper-active_record', '~> 0.17.1' -gem 'flipper-active_support_cache_store', '~> 0.17.1' +gem 'flipper', '~> 0.21.0' +gem 'flipper-active_record', '~> 0.21.0' +gem 'flipper-active_support_cache_store', '~> 0.21.0' gem 'unleash', '~> 0.1.5' gem 'gitlab-experiment', '~> 0.5.4' diff --git a/Gemfile.lock b/Gemfile.lock index 59cbd84a3e6..06dfaccac3d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -376,13 +376,13 @@ GEM rake ffi-yajl (2.3.4) libyajl2 (~> 1.2) - flipper (0.17.1) - flipper-active_record (0.17.1) - activerecord (>= 4.2, < 7) - flipper (~> 0.17.1) - flipper-active_support_cache_store (0.17.1) - activesupport (>= 4.2, < 7) - flipper (~> 0.17.1) + flipper (0.21.0) + flipper-active_record (0.21.0) + activerecord (>= 5.0, < 7) + flipper (~> 0.21.0) + flipper-active_support_cache_store (0.21.0) + activesupport (>= 5.0, < 7) + flipper (~> 0.21.0) flowdock (0.7.1) httparty (~> 0.7) multi_json @@ -1453,9 +1453,9 @@ DEPENDENCIES faraday_middleware-aws-sigv4 (~> 0.3.0) fast_blank ffaker (~> 2.10) - flipper (~> 0.17.1) - flipper-active_record (~> 0.17.1) - flipper-active_support_cache_store (~> 0.17.1) + flipper (~> 0.21.0) + flipper-active_record (~> 0.21.0) + flipper-active_support_cache_store (~> 0.21.0) flowdock (~> 0.7) fog-aliyun (~> 0.3) fog-aws (~> 3.9) diff --git a/app/assets/javascripts/batch_comments/components/preview_item.vue b/app/assets/javascripts/batch_comments/components/preview_item.vue index 753608cf6f7..0eb4e6e7709 100644 --- a/app/assets/javascripts/batch_comments/components/preview_item.vue +++ b/app/assets/javascripts/batch_comments/components/preview_item.vue @@ -46,9 +46,13 @@ export default { } if (this.discussion) { - return sprintf(__("%{authorsName}'s thread"), { - authorsName: this.discussion.notes.find((note) => !note.system).author.name, - }); + return sprintf( + __("%{authorsName}'s thread"), + { + authorsName: this.discussion.notes.find((note) => !note.system).author.name, + }, + false, + ); } return __('Your new comment'); diff --git a/app/assets/javascripts/content_editor/services/track_input_rules_and_shortcuts.js b/app/assets/javascripts/content_editor/services/track_input_rules_and_shortcuts.js index 860e5372bc2..d26f32a7e7a 100644 --- a/app/assets/javascripts/content_editor/services/track_input_rules_and_shortcuts.js +++ b/app/assets/javascripts/content_editor/services/track_input_rules_and_shortcuts.js @@ -1,4 +1,4 @@ -import { mapValues, omit } from 'lodash'; +import { mapValues } from 'lodash'; import { InputRule } from 'prosemirror-inputrules'; import { ENTER_KEY, BACKSPACE_KEY } from '~/lib/utils/keys'; import Tracking from '~/tracking'; @@ -36,15 +36,16 @@ const trackInputRulesAndShortcuts = (tiptapExtension) => { addKeyboardShortcuts() { const shortcuts = this.parent?.() || {}; const { name } = this; - /** * We don’t want to track keyboard shortcuts * that are not deliberately executed to create * new types of content */ - const withoutEnterShortcut = omit(shortcuts, [ENTER_KEY, BACKSPACE_KEY]); - const decorated = mapValues(withoutEnterShortcut, (commandFn, shortcut) => - trackKeyboardShortcut(name, commandFn, shortcut), + const dotNotTrackKeys = [ENTER_KEY, BACKSPACE_KEY]; + const decorated = mapValues(shortcuts, (commandFn, shortcut) => + dotNotTrackKeys.includes(shortcut) + ? commandFn + : trackKeyboardShortcut(name, commandFn, shortcut), ); return decorated; diff --git a/app/assets/stylesheets/framework/layout.scss b/app/assets/stylesheets/framework/layout.scss index 4f9896dd58a..e00bb83362a 100644 --- a/app/assets/stylesheets/framework/layout.scss +++ b/app/assets/stylesheets/framework/layout.scss @@ -38,8 +38,11 @@ body { } } -.content-wrapper { +.content-wrapper-margin { margin-top: $header-height; +} + +.content-wrapper { padding-bottom: 100px; } @@ -166,15 +169,8 @@ body { } .content-wrapper { - margin-top: 0; padding-bottom: 0; flex: 1; min-height: 0; } - - &.flash-shown { - .content-wrapper { - margin-top: 0; - } - } } diff --git a/app/assets/stylesheets/framework/system_messages.scss b/app/assets/stylesheets/framework/system_messages.scss index 10796f319bf..437915d5034 100644 --- a/app/assets/stylesheets/framework/system_messages.scss +++ b/app/assets/stylesheets/framework/system_messages.scss @@ -52,7 +52,7 @@ top: $system-header-height + $header-height; } - .content-wrapper { + .content-wrapper-margin { margin-top: $system-header-height + $header-height; } @@ -90,7 +90,7 @@ bottom: $system-footer-height; } - .content-wrapper { + .content-wrapper-margin { margin-bottom: 16px; } diff --git a/app/assets/stylesheets/print.scss b/app/assets/stylesheets/print.scss index 1d0333d1e2f..ab86a2f69dd 100644 --- a/app/assets/stylesheets/print.scss +++ b/app/assets/stylesheets/print.scss @@ -65,6 +65,6 @@ a[href]::after { margin-top: 0; } -.content-wrapper { +.content-wrapper-margin { margin-top: 0; } diff --git a/app/controllers/admin/dashboard_controller.rb b/app/controllers/admin/dashboard_controller.rb index ba24e3e619b..46e5a508a1b 100644 --- a/app/controllers/admin/dashboard_controller.rb +++ b/app/controllers/admin/dashboard_controller.rb @@ -15,7 +15,7 @@ class Admin::DashboardController < Admin::ApplicationController @groups = Group.order_id_desc.with_route.limit(10) @notices = Gitlab::ConfigChecker::PumaRuggedChecker.check @notices += Gitlab::ConfigChecker::ExternalDatabaseChecker.check - @redis_versions = [Gitlab::Redis::Queues, Gitlab::Redis::SharedState, Gitlab::Redis::Cache, Gitlab::Redis::TraceChunks].map(&:version).uniq + @redis_versions = [Gitlab::Redis::Queues, Gitlab::Redis::SharedState, Gitlab::Redis::Cache].map(&:version).uniq end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/controllers/groups/registry/repositories_controller.rb b/app/controllers/groups/registry/repositories_controller.rb index d914e0bffc6..3aaaf6ade6b 100644 --- a/app/controllers/groups/registry/repositories_controller.rb +++ b/app/controllers/groups/registry/repositories_controller.rb @@ -17,7 +17,7 @@ module Groups .execute .with_api_entity_associations - track_package_event(:list_repositories, :container) + track_package_event(:list_repositories, :container, user: current_user, namespace: group) serializer = ContainerRepositoriesSerializer .new(current_user: current_user) diff --git a/app/controllers/health_controller.rb b/app/controllers/health_controller.rb index 99b0b775217..c6a02250896 100644 --- a/app/controllers/health_controller.rb +++ b/app/controllers/health_controller.rb @@ -15,7 +15,6 @@ class HealthController < ActionController::Base Gitlab::HealthChecks::Redis::CacheCheck, Gitlab::HealthChecks::Redis::QueuesCheck, Gitlab::HealthChecks::Redis::SharedStateCheck, - Gitlab::HealthChecks::Redis::TraceChunksCheck, Gitlab::HealthChecks::GitalyCheck ].freeze diff --git a/app/mailers/emails/service_desk.rb b/app/mailers/emails/service_desk.rb index 66eb2c646a9..e8034ef9b57 100644 --- a/app/mailers/emails/service_desk.rb +++ b/app/mailers/emails/service_desk.rb @@ -20,7 +20,9 @@ module Emails options = service_desk_options(email_sender, 'thank_you', @issue.external_author) .merge(subject: "Re: #{subject_base}") - mail_new_thread(@issue, options) + mail_new_thread(@issue, options).tap do + Gitlab::Metrics::BackgroundTransaction.current&.add_event(:service_desk_thank_you_email) + end end def service_desk_new_note_email(issue_id, note_id, recipient) @@ -31,7 +33,9 @@ module Emails options = service_desk_options(email_sender, 'new_note', recipient) .merge(subject: subject_base) - mail_answer_thread(@issue, options) + mail_answer_thread(@issue, options).tap do + Gitlab::Metrics::BackgroundTransaction.current&.add_event(:service_desk_new_note_email) + end end private diff --git a/app/models/key.rb b/app/models/key.rb index 15b3c460b52..b8b9bd53a63 100644 --- a/app/models/key.rb +++ b/app/models/key.rb @@ -7,6 +7,7 @@ class Key < ApplicationRecord include Sortable include Sha256Attribute include Expirable + include EachBatch sha256_attribute :fingerprint_sha256 @@ -43,7 +44,9 @@ class Key < ApplicationRecord scope :preload_users, -> { preload(:user) } scope :for_user, -> (user) { where(user: user) } scope :order_last_used_at_desc, -> { reorder(::Gitlab::Database.nulls_last_order('last_used_at', 'DESC')) } - scope :expired_today_and_not_notified, -> { where(["date(expires_at AT TIME ZONE 'UTC') = CURRENT_DATE AND expiry_notification_delivered_at IS NULL"]) } + + # Date is set specifically in this scope to improve query time. + scope :expired_and_not_notified, -> { where(["date(expires_at AT TIME ZONE 'UTC') BETWEEN '2000-01-01' AND CURRENT_DATE AND expiry_notification_delivered_at IS NULL"]) } scope :expiring_soon_and_not_notified, -> { where(["date(expires_at AT TIME ZONE 'UTC') > CURRENT_DATE AND date(expires_at AT TIME ZONE 'UTC') < ? AND before_expiry_notification_delivered_at IS NULL", DAYS_TO_EXPIRE.days.from_now.to_date]) } def self.regular_keys diff --git a/app/models/user.rb b/app/models/user.rb index a7964cf0001..5eab59f657f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -108,7 +108,7 @@ class User < ApplicationRecord # Profile has_many :keys, -> { regular_keys }, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent - has_many :expired_today_and_unnotified_keys, -> { expired_today_and_not_notified }, class_name: 'Key' + has_many :expired_and_unnotified_keys, -> { expired_and_not_notified }, class_name: 'Key' has_many :expiring_soon_and_unnotified_keys, -> { expiring_soon_and_not_notified }, class_name: 'Key' has_many :deploy_keys, -> { where(type: 'DeployKey') }, dependent: :nullify # rubocop:disable Cop/ActiveRecordDependent has_many :group_deploy_keys @@ -411,14 +411,7 @@ class User < ApplicationRecord .without_impersonation .expired_today_and_not_notified) end - scope :with_ssh_key_expired_today, -> do - includes(:expired_today_and_unnotified_keys) - .where('EXISTS (?)', - ::Key - .select(1) - .where('keys.user_id = users.id') - .expired_today_and_not_notified) - end + scope :with_ssh_key_expiring_soon, -> do includes(:expiring_soon_and_unnotified_keys) .where('EXISTS (?)', diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 5e9633d1c8a..df06f0a95fe 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -238,6 +238,7 @@ class ProjectPolicy < BasePolicy enable :admin_issue_board enable :download_code enable :read_statistics + enable :daily_statistics enable :download_wiki_code enable :create_snippet enable :update_issue @@ -346,7 +347,6 @@ class ProjectPolicy < BasePolicy enable :update_deployment enable :create_release enable :update_release - enable :daily_statistics enable :create_metrics_dashboard_annotation enable :delete_metrics_dashboard_annotation enable :update_metrics_dashboard_annotation diff --git a/app/services/users/refresh_authorized_projects_service.rb b/app/services/users/refresh_authorized_projects_service.rb index d28ff45bfdf..1850fa9747d 100644 --- a/app/services/users/refresh_authorized_projects_service.rb +++ b/app/services/users/refresh_authorized_projects_service.rb @@ -24,11 +24,6 @@ module Users @source = source @incorrect_auth_found_callback = incorrect_auth_found_callback @missing_auth_found_callback = missing_auth_found_callback - - # We need an up to date User object that has access to all relations that - # may have been created earlier. The only way to ensure this is to reload - # the User object. - user.reset end def execute @@ -43,6 +38,10 @@ module Users end begin + # We need an up to date User object that has access to all relations that + # may have been created earlier. The only way to ensure this is to reload + # the User object. + user.reset execute_without_lease ensure Gitlab::ExclusiveLease.cancel(lease_key, uuid) diff --git a/app/views/layouts/_page.html.haml b/app/views/layouts/_page.html.haml index c91d27e3ed1..61f03c0540d 100644 --- a/app/views/layouts/_page.html.haml +++ b/app/views/layouts/_page.html.haml @@ -1,7 +1,7 @@ .layout-page{ class: page_with_sidebar_class } - if defined?(nav) && nav = render "layouts/nav/sidebar/#{nav}" - .content-wrapper{ class: "#{@content_wrapper_class}" } + .content-wrapper.content-wrapper-margin{ class: "#{@content_wrapper_class}" } .mobile-overlay = yield :group_invite_members_banner .alert-wrapper.gl-force-block-formatting-context diff --git a/app/views/layouts/fullscreen.html.haml b/app/views/layouts/fullscreen.html.haml index 63bb9f8fff5..f46c58f96ee 100644 --- a/app/views/layouts/fullscreen.html.haml +++ b/app/views/layouts/fullscreen.html.haml @@ -11,6 +11,6 @@ = render "layouts/broadcast" = yield :flash_message = render "layouts/flash" - .content-wrapper{ id: "content-body", class: "d-flex flex-column align-items-stretch mt-0" } + .content-wrapper{ id: "content-body", class: "d-flex flex-column align-items-stretch" } = yield = footer_message diff --git a/app/views/layouts/terms.html.haml b/app/views/layouts/terms.html.haml index e39cb5ee0a2..4d5c354388f 100644 --- a/app/views/layouts/terms.html.haml +++ b/app/views/layouts/terms.html.haml @@ -5,7 +5,7 @@ %body{ data: { page: body_data_page } } .layout-page.terms{ class: page_class } - .content-wrapper.gl-mt-0 + .content-wrapper .mobile-overlay .alert-wrapper = render "layouts/broadcast" diff --git a/app/workers/ssh_keys/expired_notification_worker.rb b/app/workers/ssh_keys/expired_notification_worker.rb index 9d5143fe655..95c9601a04c 100644 --- a/app/workers/ssh_keys/expired_notification_worker.rb +++ b/app/workers/ssh_keys/expired_notification_worker.rb @@ -15,16 +15,16 @@ module SshKeys return unless ::Feature.enabled?(:ssh_key_expiration_email_notification, default_enabled: :yaml) # rubocop:disable CodeReuse/ActiveRecord - User.with_ssh_key_expired_today.find_each(batch_size: 10_000) do |user| - with_context(user: user) do - Gitlab::AppLogger.info "#{self.class}: Notifying User #{user.id} about expired ssh key(s)" + Key.expired_and_not_notified.each_batch(of: 1000) do |relation| # rubocop:disable Cop/InBatches + users = User.where(id: relation.select(:user_id)) - keys = user.expired_today_and_unnotified_keys - - Keys::ExpiryNotificationService.new(user, { keys: keys, expiring_soon: false }).execute + users.each do |user| + with_context(user: user) do + Keys::ExpiryNotificationService.new(user, { keys: user.expired_and_unnotified_keys, expiring_soon: false }).execute + end end - # rubocop:enable CodeReuse/ActiveRecord end + # rubocop:enable CodeReuse/ActiveRecord end end end diff --git a/config/README.md b/config/README.md index be5bd442fd8..7f3125cefd2 100644 --- a/config/README.md +++ b/config/README.md @@ -147,34 +147,3 @@ searched): 3. the configuration file pointed to by the `GITLAB_REDIS_CONFIG_FILE` environment variable 4. the configuration file `resque.yml` - -## redis.trace_chunks.yml - -If configured, `redis.trace_chunks.yml` overrides the -`resque.yml` settings to configure the Redis database instance -used for clients of `::Gitlab::Redis::TraceChunks` which stores CI trace chunks. - -Settings here can be overridden by the environment variable -`GITLAB_REDIS_TRACE_CHUNKS_CONFIG_FILE` which provides -an alternate location for configuration settings. - -The order of precedence for the URL used to connect to the Redis instance -used for `trace_chunks` is: -1. URL from a configuration file pointed to by the -`GITLAB_REDIS_TRACE_CHUNKS_CONFIG_FILE` environment variable -2. URL from `redis.trace_chunks.yml` -3. URL from a configuration file pointed to by the -`GITLAB_REDIS_CONFIG_FILE` environment variable -4. URL from `resque.yml` -5. `redis://localhost:6383` - -The order of precedence for all other configuration settings for `trace_chunks` -are selected from only the first of the following files found (if a setting -is not provided in an earlier file, the remainder of the files are not -searched): -1. the configuration file pointed to by the -`GITLAB_REDIS_TRACE_CHUNKS_CONFIG_FILE` environment variable -2. the configuration file `redis.trace_chunks.yml` -3. the configuration file pointed to by the -`GITLAB_REDIS_CONFIG_FILE` environment variable -4. the configuration file `resque.yml` diff --git a/config/feature_flags/development/usage_data_track_ecosystem_jira_service.yml b/config/feature_flags/development/usage_data_track_ecosystem_jira_service.yml deleted file mode 100644 index adf850aa8bc..00000000000 --- a/config/feature_flags/development/usage_data_track_ecosystem_jira_service.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: usage_data_track_ecosystem_jira_service -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/52110 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/300447 -milestone: '13.9' -type: development -group: group::ecosystem -default_enabled: false diff --git a/config/initializers/7_redis.rb b/config/initializers/7_redis.rb index fe37dfd7579..a6025a6dbf0 100644 --- a/config/initializers/7_redis.rb +++ b/config/initializers/7_redis.rb @@ -8,4 +8,3 @@ Gitlab::Redis::Cache.with { nil } Gitlab::Redis::Queues.with { nil } Gitlab::Redis::SharedState.with { nil } -Gitlab::Redis::TraceChunks.with { nil } diff --git a/config/initializers/flipper.rb b/config/initializers/flipper.rb new file mode 100644 index 00000000000..be4f01f537d --- /dev/null +++ b/config/initializers/flipper.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +Rails.application.configure do + config.flipper.preload = false + config.flipper.memoizer = false +end diff --git a/config/redis.trace_chunks.yml.example b/config/redis.trace_chunks.yml.example deleted file mode 100644 index d38b9ba4966..00000000000 --- a/config/redis.trace_chunks.yml.example +++ /dev/null @@ -1,38 +0,0 @@ -# If you change this file in a merge request, please also create -# a merge request on https://gitlab.com/gitlab-org/omnibus-gitlab/merge_requests -# -development: - url: redis://localhost:6379/13 - # - # url: redis://localhost:6382 - # sentinels: - # - - # host: localhost - # port: 26382 # point to sentinel, not to redis port - # - - # host: replica2 - # port: 26382 # point to sentinel, not to redis port -test: - url: redis://localhost:6379/13 - # - # url: redis://localhost:6382 -production: - # Redis (single instance) - url: unix:/var/run/redis/redis.trace_chunks.sock - ## - # Redis + Sentinel (for HA) - # - # Please read instructions carefully before using it as you may lose data: - # http://redis.io/topics/sentinel - # - # You must specify a list of a few sentinels that will handle client connection - # please read here for more information: https://docs.gitlab.com/ee/administration/redis/index.html - ## - # url: redis://master:6382 - # sentinels: - # - - # host: replica1 - # port: 26382 # point to sentinel, not to redis port - # - - # host: replica2 - # port: 26382 # point to sentinel, not to redis port diff --git a/doc/administration/monitoring/prometheus/gitlab_metrics.md b/doc/administration/monitoring/prometheus/gitlab_metrics.md index faa24e76a0e..9a168b84869 100644 --- a/doc/administration/monitoring/prometheus/gitlab_metrics.md +++ b/doc/administration/monitoring/prometheus/gitlab_metrics.md @@ -130,6 +130,8 @@ The following metrics are available: | `gitlab_ci_pipeline_security_orchestration_policy_processing_duration_seconds` | Histogram | 13.12 | Time in seconds it takes to process Security Policies in CI/CD pipeline | | | `gitlab_ci_difference_live_vs_actual_minutes` | Histogram | 13.12 | Difference between CI minute consumption counted while jobs were running (live) vs when jobs are complete (actual). Used to enforce CI minute consumption limits on long running jobs. | `plan` | | `gitlab_spamcheck_request_duration_seconds` | Histogram | 13.12 | The duration for requests between Rails and the anti-spam engine | | +| `service_desk_thank_you_email` | Counter | 14.0 | Total number of email responses to new service desk emails | | +| `service_desk_new_note_email` | Counter | 14.0 | Total number of email notifications on new service desk comment | | ## Metrics controlled by a feature flag diff --git a/doc/development/changelog.md b/doc/development/changelog.md index 616b175c9eb..5e659e368da 100644 --- a/doc/development/changelog.md +++ b/doc/development/changelog.md @@ -20,14 +20,14 @@ automatically. The `Changelog` trailer accepts the following values: -- added -- fixed -- changed -- deprecated -- removed -- security -- performance -- other +- `added`: New feature +- `fixed`: Bug fix +- `changed`: Feature change +- `deprecated`: New deprecation +- `removed`: Feature removal +- `security`: Security fix +- `performance`: Performance improvement +- `other`: Other An example of a Git commit to include in the changelog is the following: diff --git a/doc/development/feature_flags/controls.md b/doc/development/feature_flags/controls.md index 76b5bb78573..a9ebcfc9fba 100644 --- a/doc/development/feature_flags/controls.md +++ b/doc/development/feature_flags/controls.md @@ -223,6 +223,23 @@ you should fully roll out the feature by enabling the flag **globally** by runni This changes the feature flag state to be **enabled** always, which overrides the existing gates (e.g. `--group=gitlab-org`) in the above processes. +Note, that if an actor based feature gate is present, switching the +`default_enabled` attribute of the YAML definition from `false` to `true` +will not have any effect. The feature gate must be deleted first. + +For example, a feature flag is set via chatops: + +```shell +/chatops run feature set --project=gitlab-org/gitlab some_feature true +``` + +When the `default_enabled` attribute in the YAML definition is switched to +`true`, the feature gate must be deleted to have the desired effect: + +```shell +/chatops run feature delete some_feature +``` + ##### Percentage of actors vs percentage of time rollouts If you want to make sure a feature is always on or off for users, use a **Percentage of actors** diff --git a/doc/development/redis.md b/doc/development/redis.md index c7111db0cdc..1dde9eaeea2 100644 --- a/doc/development/redis.md +++ b/doc/development/redis.md @@ -18,8 +18,9 @@ Redis instance. On GitLab.com, we use [separate Redis instances](../administration/redis/replication_and_failover.md#running-multiple-redis-clusters). -(We do not currently use [ActionCable on -GitLab.com](https://gitlab.com/groups/gitlab-com/gl-infra/-/epics/228)). +See the [Redis SRE guide](https://gitlab.com/gitlab-com/runbooks/-/blob/master/docs/redis/redis-survival-guide-for-sres.md) +for more details on our setup. +We do not currently use [ActionCable on GitLab.com](https://gitlab.com/groups/gitlab-com/gl-infra/-/epics/228). Every application process is configured to use the same Redis servers, so they can be used for inter-process communication in cases where [PostgreSQL](sql.md) diff --git a/doc/development/testing_guide/review_apps.md b/doc/development/testing_guide/review_apps.md index 4fd0003795d..29cd0e7d5f6 100644 --- a/doc/development/testing_guide/review_apps.md +++ b/doc/development/testing_guide/review_apps.md @@ -81,6 +81,8 @@ subgraph "CNG-mirror pipeline" - Since we're using [the official GitLab Helm chart](https://gitlab.com/gitlab-org/charts/gitlab/), this means you get a dedicated environment for your branch that's very close to what it would look in production. + - Each review app is deployed to its own Kubernetes namespace. The namespace is based on the Review App slug that is + unique to each branch. 1. Once the [`review-deploy`](https://gitlab.com/gitlab-org/gitlab/-/jobs/467724810) job succeeds, you should be able to use your Review App thanks to the direct link to it from the MR widget. To log into the Review App, see "Log into my Review App?" below. @@ -203,7 +205,7 @@ the GitLab handbook information for the [shared 1Password account](https://about 1. Click on the `KUBECTL` dropdown, then `Exec` -> `task-runner`. 1. Replace `-c task-runner -- ls` with `-it -- gitlab-rails console` from the default command or - - Run `kubectl exec --namespace review-apps review-qa-raise-e-12chm0-task-runner-d5455cc8-2lsvz -it -- gitlab-rails console` and + - Run `kubectl exec --namespace review-qa-raise-e-12chm0 review-qa-raise-e-12chm0-task-runner-d5455cc8-2lsvz -it -- gitlab-rails console` and - Replace `review-qa-raise-e-12chm0-task-runner-d5455cc8-2lsvz` with your Pod's name. @@ -221,7 +223,7 @@ the GitLab handbook information for the [shared 1Password account](https://about ## Diagnosing unhealthy Review App releases If [Review App Stability](https://app.periscopedata.com/app/gitlab/496118/Engineering-Productivity-Sandbox?widget=6690556&udv=785399) -dips this may be a signal that the `review-apps-ce/ee` cluster is unhealthy. +dips this may be a signal that the `review-apps` cluster is unhealthy. Leading indicators may be health check failures leading to restarts or majority failure for Review App deployments. The [Review Apps Overview dashboard](https://console.cloud.google.com/monitoring/classic/dashboards/6798952013815386466?project=gitlab-review-apps&timeDomain=1d) diff --git a/doc/user/permissions.md b/doc/user/permissions.md index d69d1a4b1b3..79b3b2692d2 100644 --- a/doc/user/permissions.md +++ b/doc/user/permissions.md @@ -101,6 +101,7 @@ The following table lists project permissions available for each role: | Move [test case](../ci/test_cases/index.md) | | ✓ | ✓ | ✓ | ✓ | | Reopen [test case](../ci/test_cases/index.md) | | ✓ | ✓ | ✓ | ✓ | | Pull [packages](packages/index.md) | | ✓ | ✓ | ✓ | ✓ | +| View project statistics | | ✓ | ✓ | ✓ | ✓ | | Publish [packages](packages/index.md) | | | ✓ | ✓ | ✓ | | Create/edit/delete a Cleanup policy | | | ✓ | ✓ | ✓ | | Upload [Design Management](project/issues/design_management.md) files | | | ✓ | ✓ | ✓ | @@ -117,7 +118,6 @@ The following table lists project permissions available for each role: | Lock merge request threads | | | ✓ | ✓ | ✓ | | Approve merge requests (*9*) | | | ✓ | ✓ | ✓ | | Manage/Accept merge requests | | | ✓ | ✓ | ✓ | -| View project statistics | | | ✓ | ✓ | ✓ | | Create new environments | | | ✓ | ✓ | ✓ | | Stop environments | | | ✓ | ✓ | ✓ | | Enable Review Apps | | | ✓ | ✓ | ✓ | diff --git a/lib/api/composer_packages.rb b/lib/api/composer_packages.rb index 115a6b8ac4f..7b3750b37ee 100644 --- a/lib/api/composer_packages.rb +++ b/lib/api/composer_packages.rb @@ -137,7 +137,7 @@ module API bad_request! end - track_package_event('push_package', :composer) + track_package_event('push_package', :composer, project: authorized_user_project, user: current_user, namespace: authorized_user_project.namespace) ::Packages::Composer::CreatePackageService .new(authorized_user_project, current_user, declared_params.merge(build: current_authenticated_job)) @@ -161,7 +161,7 @@ module API not_found! unless metadata - track_package_event('pull_package', :composer) + track_package_event('pull_package', :composer, project: unauthorized_user_project, namespace: unauthorized_user_project.namespace) send_git_archive unauthorized_user_project.repository, ref: metadata.target_sha, format: 'zip', append_sha: true end diff --git a/lib/api/concerns/packages/nuget_endpoints.rb b/lib/api/concerns/packages/nuget_endpoints.rb index 5364eeb1880..208daeb3037 100644 --- a/lib/api/concerns/packages/nuget_endpoints.rb +++ b/lib/api/concerns/packages/nuget_endpoints.rb @@ -58,7 +58,8 @@ module API end get 'index', format: :json do authorize_read_package!(project_or_group) - track_package_event('cli_metadata', :nuget, category: 'API::NugetPackages') + + track_package_event('cli_metadata', :nuget, **snowplow_gitlab_standard_context.merge(category: 'API::NugetPackages')) present ::Packages::Nuget::ServiceIndexPresenter.new(project_or_group), with: ::API::Entities::Nuget::ServiceIndex @@ -117,7 +118,7 @@ module API results = search_packages(params[:q], search_options) - track_package_event('search_package', :nuget, category: 'API::NugetPackages') + track_package_event('search_package', :nuget, **snowplow_gitlab_standard_context.merge(category: 'API::NugetPackages')) present ::Packages::Nuget::SearchResultsPresenter.new(results), with: ::API::Entities::Nuget::SearchResults diff --git a/lib/api/debian_project_packages.rb b/lib/api/debian_project_packages.rb index feb83b52695..eadb0646a67 100644 --- a/lib/api/debian_project_packages.rb +++ b/lib/api/debian_project_packages.rb @@ -35,7 +35,7 @@ module API authorize_upload!(authorized_user_project) bad_request!('File is too large') if authorized_user_project.actual_limits.exceeded?(:debian_max_file_size, params[:file].size) - track_package_event('push_package', :debian) + track_package_event('push_package', :debian, user: current_user, project: authorized_user_project, namespace: authorized_user_project.namespace) file_params = { file: params['file'], diff --git a/lib/api/group_container_repositories.rb b/lib/api/group_container_repositories.rb index 4fede0ad583..96175f31696 100644 --- a/lib/api/group_container_repositories.rb +++ b/lib/api/group_container_repositories.rb @@ -31,7 +31,7 @@ module API user: current_user, subject: user_group ).execute - track_package_event('list_repositories', :container) + track_package_event('list_repositories', :container, user: current_user, namespace: user_group) present paginate(repositories), with: Entities::ContainerRegistry::Repository, tags: params[:tags], tags_count: params[:tags_count] end diff --git a/lib/api/helm_packages.rb b/lib/api/helm_packages.rb index 446dcaf1e3a..dc5630a1395 100644 --- a/lib/api/helm_packages.rb +++ b/lib/api/helm_packages.rb @@ -46,7 +46,7 @@ module API package_file = Packages::Helm::PackageFilesFinder.new(authorized_user_project, params[:channel], file_name: "#{params[:file_name]}.tgz").execute.last! - track_package_event('pull_package', :helm) + track_package_event('pull_package', :helm, project: authorized_user_project, namespace: authorized_user_project.namespace) present_carrierwave_file!(package_file.file) end diff --git a/lib/api/maven_packages.rb b/lib/api/maven_packages.rb index cdce92d5c46..9e5705abe88 100644 --- a/lib/api/maven_packages.rb +++ b/lib/api/maven_packages.rb @@ -130,7 +130,7 @@ module API when 'sha1' package_file.file_sha1 else - track_package_event('pull_package', :maven) if jar_file?(format) + track_package_event('pull_package', :maven, project: project, namespace: project.namespace) if jar_file?(format) present_carrierwave_file_with_head_support!(package_file.file) end end @@ -170,7 +170,7 @@ module API when 'sha1' package_file.file_sha1 else - track_package_event('pull_package', :maven) if jar_file?(format) + track_package_event('pull_package', :maven, project: package.project, namespace: package.project.namespace) if jar_file?(format) present_carrierwave_file_with_head_support!(package_file.file) end @@ -208,7 +208,7 @@ module API when 'sha1' package_file.file_sha1 else - track_package_event('pull_package', :maven) if jar_file?(format) + track_package_event('pull_package', :maven, project: user_project, namespace: user_project.namespace) if jar_file?(format) present_carrierwave_file_with_head_support!(package_file.file) end @@ -264,7 +264,7 @@ module API when 'md5' '' else - track_package_event('push_package', :maven) if jar_file?(format) + track_package_event('push_package', :maven, user: current_user, project: user_project, namespace: user_project.namespace) if jar_file?(format) file_params = { file: params[:file], diff --git a/lib/api/npm_project_packages.rb b/lib/api/npm_project_packages.rb index 887084dc9ae..7ff4439ce04 100644 --- a/lib/api/npm_project_packages.rb +++ b/lib/api/npm_project_packages.rb @@ -32,7 +32,7 @@ module API package_file = ::Packages::PackageFileFinder .new(package, params[:file_name]).execute! - track_package_event('pull_package', package, category: 'API::NpmPackages') + track_package_event('pull_package', package, category: 'API::NpmPackages', project: project, namespace: project.namespace) present_carrierwave_file!(package_file.file) end @@ -48,7 +48,7 @@ module API put ':package_name', requirements: ::API::Helpers::Packages::Npm::NPM_ENDPOINT_REQUIREMENTS do authorize_create_package!(project) - track_package_event('push_package', :npm, category: 'API::NpmPackages') + track_package_event('push_package', :npm, category: 'API::NpmPackages', project: project, user: current_user, namespace: project.namespace) created_package = ::Packages::Npm::CreatePackageService .new(project, current_user, params.merge(build: current_authenticated_job)).execute diff --git a/lib/api/nuget_group_packages.rb b/lib/api/nuget_group_packages.rb index a80de06d6b0..eb55e4cbf70 100644 --- a/lib/api/nuget_group_packages.rb +++ b/lib/api/nuget_group_packages.rb @@ -38,6 +38,10 @@ module API def require_authenticated! unauthorized! unless current_user end + + def snowplow_gitlab_standard_context + { namespace: find_authorized_group! } + end end params do diff --git a/lib/api/nuget_project_packages.rb b/lib/api/nuget_project_packages.rb index 73ecc140959..5bae08d4dae 100644 --- a/lib/api/nuget_project_packages.rb +++ b/lib/api/nuget_project_packages.rb @@ -36,6 +36,10 @@ module API def project_or_group authorized_user_project end + + def snowplow_gitlab_standard_context + { project: authorized_user_project, namespace: authorized_user_project.namespace } + end end params do @@ -69,7 +73,7 @@ module API package_file = ::Packages::CreatePackageFileService.new(package, file_params.merge(build: current_authenticated_job)) .execute - track_package_event('push_package', :nuget, category: 'API::NugetPackages') + track_package_event('push_package', :nuget, category: 'API::NugetPackages', user: current_user, project: package.project, namespace: package.project.namespace) ::Packages::Nuget::ExtractionWorker.perform_async(package_file.id) # rubocop:disable CodeReuse/Worker @@ -118,7 +122,7 @@ module API not_found!('Package') unless package_file - track_package_event('pull_package', :nuget, category: 'API::NugetPackages') + track_package_event('pull_package', :nuget, category: 'API::NugetPackages', project: package_file.project, namespace: package_file.project.namespace) # nuget and dotnet don't support 302 Moved status codes, supports_direct_download has to be set to false present_carrierwave_file!(package_file.file, supports_direct_download: false) diff --git a/lib/api/project_container_repositories.rb b/lib/api/project_container_repositories.rb index 2580f7adbc9..28cfa9e3ae0 100644 --- a/lib/api/project_container_repositories.rb +++ b/lib/api/project_container_repositories.rb @@ -31,7 +31,7 @@ module API user: current_user, subject: user_project ).execute - track_package_event('list_repositories', :container) + track_package_event('list_repositories', :container, user: current_user, project: user_project, namespace: user_project.namespace) present paginate(repositories), with: Entities::ContainerRegistry::Repository, tags: params[:tags], tags_count: params[:tags_count] end @@ -46,7 +46,7 @@ module API authorize_admin_container_image! DeleteContainerRepositoryWorker.perform_async(current_user.id, repository.id) # rubocop:disable CodeReuse/Worker - track_package_event('delete_repository', :container) + track_package_event('delete_repository', :container, user: current_user, project: user_project, namespace: user_project.namespace) status :accepted end @@ -63,7 +63,7 @@ module API authorize_read_container_image! tags = Kaminari.paginate_array(repository.tags) - track_package_event('list_tags', :container) + track_package_event('list_tags', :container, user: current_user, project: user_project, namespace: user_project.namespace) present paginate(tags), with: Entities::ContainerRegistry::Tag end @@ -92,7 +92,7 @@ module API declared_params.except(:repository_id).merge(container_expiration_policy: false)) # rubocop:enable CodeReuse/Worker - track_package_event('delete_tag_bulk', :container) + track_package_event('delete_tag_bulk', :container, user: current_user, project: user_project, namespace: user_project.namespace) status :accepted end @@ -128,7 +128,7 @@ module API .execute(repository) if result[:status] == :success - track_package_event('delete_tag', :container) + track_package_event('delete_tag', :container, user: current_user, project: user_project, namespace: user_project.namespace) status :ok else diff --git a/lib/api/pypi_packages.rb b/lib/api/pypi_packages.rb index 969b619c1cd..7c5f8bb4d99 100644 --- a/lib/api/pypi_packages.rb +++ b/lib/api/pypi_packages.rb @@ -121,7 +121,7 @@ module API package = Packages::Pypi::PackageFinder.new(current_user, project, { filename: filename, sha256: params[:sha256] }).execute package_file = ::Packages::PackageFileFinder.new(package, filename, with_file_name_like: false).execute - track_package_event('pull_package', :pypi) + track_package_event('pull_package', :pypi, project: project, namespace: project.namespace) present_carrierwave_file!(package_file.file, supports_direct_download: true) end @@ -140,7 +140,7 @@ module API get 'simple/*package_name', format: :txt do authorize_read_package!(authorized_user_project) - track_package_event('list_package', :pypi) + track_package_event('list_package', :pypi, project: authorized_user_project, namespace: authorized_user_project.namespace) packages = Packages::Pypi::PackagesFinder.new(current_user, authorized_user_project, { package_name: params[:package_name] }).execute! presenter = ::Packages::Pypi::PackagePresenter.new(packages, authorized_user_project) @@ -171,7 +171,7 @@ module API authorize_upload!(authorized_user_project) bad_request!('File is too large') if authorized_user_project.actual_limits.exceeded?(:pypi_max_file_size, params[:content].size) - track_package_event('push_package', :pypi) + track_package_event('push_package', :pypi, project: authorized_user_project, user: current_user, namespace: authorized_user_project.namespace) ::Packages::Pypi::CreatePackageService .new(authorized_user_project, current_user, declared_params.merge(build: current_authenticated_job)) diff --git a/lib/api/rubygem_packages.rb b/lib/api/rubygem_packages.rb index 1d17148e0df..d7f9c584c67 100644 --- a/lib/api/rubygem_packages.rb +++ b/lib/api/rubygem_packages.rb @@ -70,7 +70,7 @@ module API user_project, params[:file_name] ).last! - track_package_event('pull_package', :rubygems) + track_package_event('pull_package', :rubygems, project: user_project, namespace: user_project.namespace) present_carrierwave_file!(package_file.file) end @@ -97,7 +97,7 @@ module API authorize_upload!(user_project) bad_request!('File is too large') if user_project.actual_limits.exceeded?(:rubygems_max_file_size, params[:file].size) - track_package_event('push_package', :rubygems) + track_package_event('push_package', :rubygems, user: current_user, project: user_project, namespace: user_project.namespace) package_file = nil diff --git a/lib/api/terraform/modules/v1/packages.rb b/lib/api/terraform/modules/v1/packages.rb index 34e77e09800..aa59b6a4fee 100644 --- a/lib/api/terraform/modules/v1/packages.rb +++ b/lib/api/terraform/modules/v1/packages.rb @@ -124,7 +124,7 @@ module API end get do - track_package_event('pull_package', :terraform_module) + track_package_event('pull_package', :terraform_module, project: package.project, namespace: module_namespace, user: current_user) present_carrierwave_file!(package_file.file) end @@ -183,7 +183,7 @@ module API render_api_error!(result[:message], result[:http_status]) if result[:status] == :error - track_package_event('push_package', :terraform_module) + track_package_event('push_package', :terraform_module, project: authorized_user_project, user: current_user, namespace: authorized_user_project.namespace) created! rescue ObjectStorage::RemoteStoreError => e diff --git a/lib/feature.rb b/lib/feature.rb index 87abd2689d0..453ecc8255a 100644 --- a/lib/feature.rb +++ b/lib/feature.rb @@ -18,6 +18,10 @@ class Feature superclass.table_name = 'feature_gates' end + # To enable EE overrides + class ActiveSupportCacheStoreAdapter < Flipper::Adapters::ActiveSupportCacheStore + end + InvalidFeatureFlagError = Class.new(Exception) # rubocop:disable Lint/InheritException class << self @@ -167,7 +171,8 @@ class Feature ActiveSupportCacheStoreAdapter.new( active_record_adapter, l2_cache_backend, - expires_in: 1.hour) + expires_in: 1.hour, + write_through: true) # Thread-local L1 cache: use a short timeout since we don't have a # way to expire this cache all at once diff --git a/lib/feature/active_support_cache_store_adapter.rb b/lib/feature/active_support_cache_store_adapter.rb deleted file mode 100644 index 431f1169a86..00000000000 --- a/lib/feature/active_support_cache_store_adapter.rb +++ /dev/null @@ -1,36 +0,0 @@ -# frozen_string_literal: true - -# rubocop:disable Gitlab/NamespacedClass -# This class was already nested this way before moving to a separate file -class Feature - class ActiveSupportCacheStoreAdapter < Flipper::Adapters::ActiveSupportCacheStore - # This patch represents https://github.com/jnunemaker/flipper/pull/512. In - # Flipper 0.21.0 and later, we can remove this and just pass `write_through: - # true` to the constructor in `Feature.build_flipper_instance`. - - extend ::Gitlab::Utils::Override - - override :enable - def enable(feature, gate, thing) - result = @adapter.enable(feature, gate, thing) - @cache.write(key_for(feature.key), @adapter.get(feature), @write_options) - result - end - - override :disable - def disable(feature, gate, thing) - result = @adapter.disable(feature, gate, thing) - @cache.write(key_for(feature.key), @adapter.get(feature), @write_options) - result - end - - override :remove - def remove(feature) - result = @adapter.remove(feature) - @cache.delete(FeaturesKey) - @cache.write(key_for(feature.key), {}, @write_options) - result - end - end -end -# rubocop:disable Gitlab/NamespacedClass diff --git a/lib/gitlab/emoji.rb b/lib/gitlab/emoji.rb index e6f71e3ad3c..2b5f465d3c5 100644 --- a/lib/gitlab/emoji.rb +++ b/lib/gitlab/emoji.rb @@ -41,7 +41,17 @@ module Gitlab end def emoji_image_tag(name, src) - "<img class='emoji' title=':#{name}:' alt=':#{name}:' src='#{src}' height='20' width='20' align='absmiddle' />" + image_options = { + class: 'emoji', + src: src, + title: ":#{name}:", + alt: ":#{name}:", + height: 20, + width: 20, + align: 'absmiddle' + } + + ActionController::Base.helpers.tag(:img, image_options) end def emoji_exists?(name) diff --git a/lib/gitlab/health_checks/redis/redis_check.rb b/lib/gitlab/health_checks/redis/redis_check.rb index 44b85bf886e..f7e46fce134 100644 --- a/lib/gitlab/health_checks/redis/redis_check.rb +++ b/lib/gitlab/health_checks/redis/redis_check.rb @@ -20,8 +20,7 @@ module Gitlab def check ::Gitlab::HealthChecks::Redis::CacheCheck.check_up && ::Gitlab::HealthChecks::Redis::QueuesCheck.check_up && - ::Gitlab::HealthChecks::Redis::SharedStateCheck.check_up && - ::Gitlab::HealthChecks::Redis::TraceChunksCheck.check_up + ::Gitlab::HealthChecks::Redis::SharedStateCheck.check_up end end end diff --git a/lib/gitlab/health_checks/redis/trace_chunks_check.rb b/lib/gitlab/health_checks/redis/trace_chunks_check.rb deleted file mode 100644 index cf9fa700b0a..00000000000 --- a/lib/gitlab/health_checks/redis/trace_chunks_check.rb +++ /dev/null @@ -1,35 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module HealthChecks - module Redis - class TraceChunksCheck - extend SimpleAbstractCheck - - class << self - def check_up - check - end - - private - - def metric_prefix - 'redis_trace_chunks_ping' - end - - def successful?(result) - result == 'PONG' - end - - # rubocop: disable CodeReuse/ActiveRecord - def check - catch_timeout 10.seconds do - Gitlab::Redis::TraceChunks.with(&:ping) - end - end - # rubocop: enable CodeReuse/ActiveRecord - end - end - end - end -end diff --git a/lib/gitlab/instrumentation/redis.rb b/lib/gitlab/instrumentation/redis.rb index ab0e56adc32..9a9d3a866b1 100644 --- a/lib/gitlab/instrumentation/redis.rb +++ b/lib/gitlab/instrumentation/redis.rb @@ -8,9 +8,8 @@ module Gitlab Cache = Class.new(RedisBase).enable_redis_cluster_validation Queues = Class.new(RedisBase) SharedState = Class.new(RedisBase).enable_redis_cluster_validation - TraceChunks = Class.new(RedisBase).enable_redis_cluster_validation - STORAGES = [ActionCable, Cache, Queues, SharedState, TraceChunks].freeze + STORAGES = [ActionCable, Cache, Queues, SharedState].freeze # Milliseconds represented in seconds (from 1 millisecond to 2 seconds). QUERY_TIME_BUCKETS = [0.001, 0.0025, 0.005, 0.01, 0.025, 0.05, 0.1, 0.25, 0.5, 1, 2].freeze diff --git a/lib/gitlab/redis/trace_chunks.rb b/lib/gitlab/redis/trace_chunks.rb deleted file mode 100644 index a2e77cb5df5..00000000000 --- a/lib/gitlab/redis/trace_chunks.rb +++ /dev/null @@ -1,12 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Redis - class TraceChunks < ::Gitlab::Redis::Wrapper - # The data we store on TraceChunks used to be stored on SharedState. - def self.config_fallback - SharedState - end - end - end -end diff --git a/lib/gitlab/redis/wrapper.rb b/lib/gitlab/redis/wrapper.rb index fa4e7f387ed..32447d39c02 100644 --- a/lib/gitlab/redis/wrapper.rb +++ b/lib/gitlab/redis/wrapper.rb @@ -64,19 +64,8 @@ module Gitlab def config_file_name [ - # Instance specific config sources: ENV["GITLAB_REDIS_#{store_name.underscore.upcase}_CONFIG_FILE"], config_file_path("redis.#{store_name.underscore}.yml"), - - # The current Redis instance may have been split off from another one - # (e.g. TraceChunks was split off from SharedState). There are - # installations out there where the lowest priority config source - # (resque.yml) contains bogus values. In those cases, config_file_name - # should resolve to the instance we originated from (the - # "config_fallback") rather than resque.yml. - config_fallback&.config_file_name, - - # Global config sources: ENV['GITLAB_REDIS_CONFIG_FILE'], config_file_path('resque.yml') ].compact.first @@ -86,10 +75,6 @@ module Gitlab name.demodulize end - def config_fallback - nil - end - def instrumentation_class "::Gitlab::Instrumentation::Redis::#{store_name}".constantize end diff --git a/lib/gitlab/usage/metrics/instrumentations/database_metric.rb b/lib/gitlab/usage/metrics/instrumentations/database_metric.rb index 012727dd475..69a288e5b6e 100644 --- a/lib/gitlab/usage/metrics/instrumentations/database_metric.rb +++ b/lib/gitlab/usage/metrics/instrumentations/database_metric.rb @@ -47,6 +47,14 @@ module Gitlab Gitlab::Usage::Metrics::Query.for(self.class.metric_operation, relation, self.class.column) end + def suggested_name + Gitlab::Usage::Metrics::NameSuggestion.for( + self.class.metric_operation, + relation: relation, + column: self.class.column + ) + end + private def relation diff --git a/lib/gitlab/usage/metrics/instrumentations/generic_metric.rb b/lib/gitlab/usage/metrics/instrumentations/generic_metric.rb index 7c97cc37d17..1849773e33d 100644 --- a/lib/gitlab/usage/metrics/instrumentations/generic_metric.rb +++ b/lib/gitlab/usage/metrics/instrumentations/generic_metric.rb @@ -13,6 +13,9 @@ module Gitlab # end # end class << self + attr_reader :metric_operation + @metric_operation = :alt + def value(&block) @metric_value = block end @@ -25,6 +28,12 @@ module Gitlab self.class.metric_value.call end end + + def suggested_name + Gitlab::Usage::Metrics::NameSuggestion.for( + self.class.metric_operation + ) + end end end end diff --git a/lib/gitlab/usage/metrics/instrumentations/redis_hll_metric.rb b/lib/gitlab/usage/metrics/instrumentations/redis_hll_metric.rb index 502a8147473..a36e612a1cb 100644 --- a/lib/gitlab/usage/metrics/instrumentations/redis_hll_metric.rb +++ b/lib/gitlab/usage/metrics/instrumentations/redis_hll_metric.rb @@ -12,6 +12,11 @@ module Gitlab # events: # - g_analytics_valuestream # end + class << self + attr_reader :metric_operation + @metric_operation = :redis + end + def initialize(time_frame:, options: {}) super @@ -30,6 +35,12 @@ module Gitlab end end + def suggested_name + Gitlab::Usage::Metrics::NameSuggestion.for( + self.class.metric_operation + ) + end + private def time_constraints diff --git a/lib/gitlab/usage/metrics/name_suggestion.rb b/lib/gitlab/usage/metrics/name_suggestion.rb new file mode 100644 index 00000000000..0728af9e2ca --- /dev/null +++ b/lib/gitlab/usage/metrics/name_suggestion.rb @@ -0,0 +1,200 @@ +# frozen_string_literal: true + +module Gitlab + module Usage + module Metrics + class NameSuggestion + FREE_TEXT_METRIC_NAME = "<please fill metric name>" + REDIS_EVENT_METRIC_NAME = "<please fill metric name, suggested format is: {subject}_{verb}{ing|ed}_{object} eg: users_creating_epics or merge_requests_viewed_in_single_file_mode>" + CONSTRAINTS_PROMPT_TEMPLATE = "<adjective describing: '%{constraints}'>" + + class << self + def for(operation, relation: nil, column: nil) + case operation + when :count + name_suggestion(column: column, relation: relation, prefix: 'count') + when :distinct_count + name_suggestion(column: column, relation: relation, prefix: 'count_distinct', distinct: :distinct) + when :estimate_batch_distinct_count + name_suggestion(column: column, relation: relation, prefix: 'estimate_distinct_count') + when :sum + name_suggestion(column: column, relation: relation, prefix: 'sum') + when :redis + REDIS_EVENT_METRIC_NAME + when :alt + FREE_TEXT_METRIC_NAME + else + raise ArgumentError, "#{operation} operation not supported" + end + end + + private + + def name_suggestion(relation:, column: nil, prefix: nil, distinct: nil) + # rubocop: disable CodeReuse/ActiveRecord + relation = relation.unscope(where: :created_at) + # rubocop: enable CodeReuse/ActiveRecord + + parts = [prefix] + arel_column = arelize_column(relation, column) + + # nil as column indicates that the counting would use fallback value of primary key. + # Because counting primary key from relation is the conceptual equal to counting all + # records from given relation, in order to keep name suggestion more condensed + # primary key column is skipped. + # eg: SELECT COUNT(id) FROM issues would translate as count_issues and not + # as count_id_from_issues since it does not add more information to the name suggestion + if arel_column != Arel::Table.new(relation.table_name)[relation.primary_key] + parts << arel_column.name + parts << 'from' + end + + arel = arel_query(relation: relation, column: arel_column, distinct: distinct) + constraints = parse_constraints(relation: relation, arel: arel) + + # In some cases due to performance reasons metrics are instrumented with joined relations + # where relation listed in FROM statement is not the one that includes counted attribute + # in such situations to make name suggestion more intuitive source should be inferred based + # on the relation that provide counted attribute + # EG: SELECT COUNT(deployments.environment_id) FROM clusters + # JOIN deployments ON deployments.cluster_id = cluster.id + # should be translated into: + # count_environment_id_from_deployments_with_clusters + # instead of + # count_environment_id_from_clusters_with_deployments + actual_source = parse_source(relation, arel_column) + + append_constraints_prompt(actual_source, [constraints], parts) + + parts << actual_source + parts += process_joined_relations(actual_source, arel, relation, constraints) + parts.compact.join('_').delete('"') + end + + def append_constraints_prompt(target, constraints, parts) + applicable_constraints = constraints.select { |constraint| constraint.include?(target) } + return unless applicable_constraints.any? + + parts << CONSTRAINTS_PROMPT_TEMPLATE % { constraints: applicable_constraints.join(' AND ') } + end + + def parse_constraints(relation:, arel:) + connection = relation.connection + ::Gitlab::Usage::Metrics::NamesSuggestions::RelationParsers::Constraints + .new(connection) + .accept(arel, collector(connection)) + .value + end + + # TODO: joins with `USING` keyword + def process_joined_relations(actual_source, arel, relation, where_constraints) + joins = parse_joins(connection: relation.connection, arel: arel) + return [] unless joins.any? + + sources = [relation.table_name, *joins.map { |join| join[:source] }] + joins = extract_joins_targets(joins, sources) + + relations = if actual_source != relation.table_name + build_relations_tree(joins + [{ source: relation.table_name }], actual_source) + else + # in case where counter attribute comes from joined relations, the relations + # diagram has to be built bottom up, thus source and target are reverted + build_relations_tree(joins + [{ source: relation.table_name }], actual_source, source_key: :target, target_key: :source) + end + + collect_join_parts(relations: relations[actual_source], joins: joins, wheres: where_constraints) + end + + def parse_joins(connection:, arel:) + ::Gitlab::Usage::Metrics::NamesSuggestions::RelationParsers::Joins + .new(connection) + .accept(arel) + end + + def extract_joins_targets(joins, sources) + joins.map do |join| + source_regex = /(#{join[:source]})\.(\w+_)*id/i + + tables_except_src = (sources - [join[:source]]).join('|') + target_regex = /(?<target>#{tables_except_src})\.(\w+_)*id/i + + join_cond_regex = /(#{source_regex}\s+=\s+#{target_regex})|(#{target_regex}\s+=\s+#{source_regex})/i + matched = join_cond_regex.match(join[:constraints]) + + if matched + join[:target] = matched[:target] + join[:constraints].gsub!(/#{join_cond_regex}(\s+(and|or))*/i, '') + end + + join + end + end + + def build_relations_tree(joins, parent, source_key: :source, target_key: :target) + return [] if joins.blank? + + tree = {} + tree[parent] = [] + + joins.each do |join| + if join[source_key] == parent + tree[parent] << build_relations_tree(joins - [join], join[target_key], source_key: source_key, target_key: target_key) + end + end + tree + end + + def collect_join_parts(relations:, joins:, wheres:, parts: [], conjunctions: %w[with having including].cycle) + conjunction = conjunctions.next + relations.each do |subtree| + subtree.each do |parent, children| + parts << "<#{conjunction}>" + join_constraints = joins.find { |join| join[:source] == parent }&.dig(:constraints) + append_constraints_prompt(parent, [wheres, join_constraints].compact, parts) + parts << parent + collect_join_parts(relations: children, joins: joins, wheres: wheres, parts: parts, conjunctions: conjunctions) + end + end + parts + end + + def arelize_column(relation, column) + case column + when Arel::Attribute + column + when NilClass + Arel::Table.new(relation.table_name)[relation.primary_key] + when String + if column.include?('.') + table, col = column.split('.') + Arel::Table.new(table)[col] + else + Arel::Table.new(relation.table_name)[column] + end + when Symbol + arelize_column(relation, column.to_s) + end + end + + def parse_source(relation, column) + column.relation.name || relation.table_name + end + + def collector(connection) + Arel::Collectors::SubstituteBinds.new(connection, Arel::Collectors::SQLString.new) + end + + def arel_query(relation:, column: nil, distinct: nil) + column ||= relation.primary_key + + if column.is_a?(Arel::Attribute) + relation.select(column.count(distinct)).arel + else + relation.select(relation.all.table[column].count(distinct)).arel + end + end + end + end + end + end +end diff --git a/lib/gitlab/usage/metrics/names_suggestions/generator.rb b/lib/gitlab/usage/metrics/names_suggestions/generator.rb index 49581169452..a669b43f395 100644 --- a/lib/gitlab/usage/metrics/names_suggestions/generator.rb +++ b/lib/gitlab/usage/metrics/names_suggestions/generator.rb @@ -5,10 +5,6 @@ module Gitlab module Metrics module NamesSuggestions class Generator < ::Gitlab::UsageData - FREE_TEXT_METRIC_NAME = "<please fill metric name>" - REDIS_EVENT_METRIC_NAME = "<please fill metric name, suggested format is: {subject}_{verb}{ing|ed}_{object} eg: users_creating_epics or merge_requests_viewed_in_single_file_mode>" - CONSTRAINTS_PROMPT_TEMPLATE = "<adjective describing: '%{constraints}'>" - class << self def generate(key_path) uncached_data.deep_stringify_keys.dig(*key_path.split('.')) @@ -17,200 +13,36 @@ module Gitlab private def count(relation, column = nil, batch: true, batch_size: nil, start: nil, finish: nil) - name_suggestion(column: column, relation: relation, prefix: 'count') + Gitlab::Usage::Metrics::NameSuggestion.for(:count, column: column, relation: relation) end def distinct_count(relation, column = nil, batch: true, batch_size: nil, start: nil, finish: nil) - name_suggestion(column: column, relation: relation, prefix: 'count_distinct', distinct: :distinct) + Gitlab::Usage::Metrics::NameSuggestion.for(:distinct_count, column: column, relation: relation) end def redis_usage_counter - REDIS_EVENT_METRIC_NAME + Gitlab::Usage::Metrics::NameSuggestion.for(:redis) end def alt_usage_data(*) - FREE_TEXT_METRIC_NAME + Gitlab::Usage::Metrics::NameSuggestion.for(:alt) end def redis_usage_data_totals(counter) - counter.fallback_totals.transform_values { |_| REDIS_EVENT_METRIC_NAME } + counter.fallback_totals.transform_values { |_| Gitlab::Usage::Metrics::NameSuggestion.for(:redis) } end def sum(relation, column, *rest) - name_suggestion(column: column, relation: relation, prefix: 'sum') + Gitlab::Usage::Metrics::NameSuggestion.for(:sum, column: column, relation: relation) end def estimate_batch_distinct_count(relation, column = nil, *rest) - name_suggestion(column: column, relation: relation, prefix: 'estimate_distinct_count') + Gitlab::Usage::Metrics::NameSuggestion.for(:estimate_batch_distinct_count, column: column, relation: relation) end def add(*args) "add_#{args.join('_and_')}" end - - def name_suggestion(relation:, column: nil, prefix: nil, distinct: nil) - # rubocop: disable CodeReuse/ActiveRecord - relation = relation.unscope(where: :created_at) - # rubocop: enable CodeReuse/ActiveRecord - - parts = [prefix] - arel_column = arelize_column(relation, column) - - # nil as column indicates that the counting would use fallback value of primary key. - # Because counting primary key from relation is the conceptual equal to counting all - # records from given relation, in order to keep name suggestion more condensed - # primary key column is skipped. - # eg: SELECT COUNT(id) FROM issues would translate as count_issues and not - # as count_id_from_issues since it does not add more information to the name suggestion - if arel_column != Arel::Table.new(relation.table_name)[relation.primary_key] - parts << arel_column.name - parts << 'from' - end - - arel = arel_query(relation: relation, column: arel_column, distinct: distinct) - constraints = parse_constraints(relation: relation, arel: arel) - - # In some cases due to performance reasons metrics are instrumented with joined relations - # where relation listed in FROM statement is not the one that includes counted attribute - # in such situations to make name suggestion more intuitive source should be inferred based - # on the relation that provide counted attribute - # EG: SELECT COUNT(deployments.environment_id) FROM clusters - # JOIN deployments ON deployments.cluster_id = cluster.id - # should be translated into: - # count_environment_id_from_deployments_with_clusters - # instead of - # count_environment_id_from_clusters_with_deployments - actual_source = parse_source(relation, arel_column) - - append_constraints_prompt(actual_source, [constraints], parts) - - parts << actual_source - parts += process_joined_relations(actual_source, arel, relation, constraints) - parts.compact.join('_').delete('"') - end - - def append_constraints_prompt(target, constraints, parts) - applicable_constraints = constraints.select { |constraint| constraint.include?(target) } - return unless applicable_constraints.any? - - parts << CONSTRAINTS_PROMPT_TEMPLATE % { constraints: applicable_constraints.join(' AND ') } - end - - def parse_constraints(relation:, arel:) - connection = relation.connection - ::Gitlab::Usage::Metrics::NamesSuggestions::RelationParsers::Constraints - .new(connection) - .accept(arel, collector(connection)) - .value - end - - # TODO: joins with `USING` keyword - def process_joined_relations(actual_source, arel, relation, where_constraints) - joins = parse_joins(connection: relation.connection, arel: arel) - return [] unless joins.any? - - sources = [relation.table_name, *joins.map { |join| join[:source] }] - joins = extract_joins_targets(joins, sources) - - relations = if actual_source != relation.table_name - build_relations_tree(joins + [{ source: relation.table_name }], actual_source) - else - # in case where counter attribute comes from joined relations, the relations - # diagram has to be built bottom up, thus source and target are reverted - build_relations_tree(joins + [{ source: relation.table_name }], actual_source, source_key: :target, target_key: :source) - end - - collect_join_parts(relations: relations[actual_source], joins: joins, wheres: where_constraints) - end - - def parse_joins(connection:, arel:) - ::Gitlab::Usage::Metrics::NamesSuggestions::RelationParsers::Joins - .new(connection) - .accept(arel) - end - - def extract_joins_targets(joins, sources) - joins.map do |join| - source_regex = /(#{join[:source]})\.(\w+_)*id/i - - tables_except_src = (sources - [join[:source]]).join('|') - target_regex = /(?<target>#{tables_except_src})\.(\w+_)*id/i - - join_cond_regex = /(#{source_regex}\s+=\s+#{target_regex})|(#{target_regex}\s+=\s+#{source_regex})/i - matched = join_cond_regex.match(join[:constraints]) - - if matched - join[:target] = matched[:target] - join[:constraints].gsub!(/#{join_cond_regex}(\s+(and|or))*/i, '') - end - - join - end - end - - def build_relations_tree(joins, parent, source_key: :source, target_key: :target) - return [] if joins.blank? - - tree = {} - tree[parent] = [] - - joins.each do |join| - if join[source_key] == parent - tree[parent] << build_relations_tree(joins - [join], join[target_key], source_key: source_key, target_key: target_key) - end - end - tree - end - - def collect_join_parts(relations:, joins:, wheres:, parts: [], conjunctions: %w[with having including].cycle) - conjunction = conjunctions.next - relations.each do |subtree| - subtree.each do |parent, children| - parts << "<#{conjunction}>" - join_constraints = joins.find { |join| join[:source] == parent }&.dig(:constraints) - append_constraints_prompt(parent, [wheres, join_constraints].compact, parts) - parts << parent - collect_join_parts(relations: children, joins: joins, wheres: wheres, parts: parts, conjunctions: conjunctions) - end - end - parts - end - - def arelize_column(relation, column) - case column - when Arel::Attribute - column - when NilClass - Arel::Table.new(relation.table_name)[relation.primary_key] - when String - if column.include?('.') - table, col = column.split('.') - Arel::Table.new(table)[col] - else - Arel::Table.new(relation.table_name)[column] - end - when Symbol - arelize_column(relation, column.to_s) - end - end - - def parse_source(relation, column) - column.relation.name || relation.table_name - end - - def collector(connection) - Arel::Collectors::SubstituteBinds.new(connection, Arel::Collectors::SQLString.new) - end - - def arel_query(relation:, column: nil, distinct: nil) - column ||= relation.primary_key - - if column.is_a?(Arel::Attribute) - relation.select(column.count(distinct)).arel - else - relation.select(relation.all.table[column].count(distinct)).arel - end - end end end end diff --git a/lib/gitlab/usage_data_counters/known_events/ecosystem.yml b/lib/gitlab/usage_data_counters/known_events/ecosystem.yml index adc5ba36ad7..f594c6a1b7c 100644 --- a/lib/gitlab/usage_data_counters/known_events/ecosystem.yml +++ b/lib/gitlab/usage_data_counters/known_events/ecosystem.yml @@ -4,22 +4,18 @@ category: ecosystem redis_slot: ecosystem aggregation: weekly - feature_flag: usage_data_track_ecosystem_jira_service - name: i_ecosystem_jira_service_cross_reference category: ecosystem redis_slot: ecosystem aggregation: weekly - feature_flag: usage_data_track_ecosystem_jira_service - name: i_ecosystem_jira_service_list_issues category: ecosystem redis_slot: ecosystem aggregation: weekly - feature_flag: usage_data_track_ecosystem_jira_service - name: i_ecosystem_jira_service_create_issue category: ecosystem redis_slot: ecosystem aggregation: weekly - feature_flag: usage_data_track_ecosystem_jira_service - name: i_ecosystem_slack_service_issue_notification category: ecosystem redis_slot: ecosystem diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 8bf3d62dd3e..717a74b3a89 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -10012,9 +10012,6 @@ msgstr "" msgid "DAST Scans" msgstr "" -msgid "DAST Settings" -msgstr "" - msgid "DNS" msgstr "" @@ -10081,6 +10078,15 @@ msgstr "" msgid "Dashboard|Unable to add %{invalidProjects}. This dashboard is available for public projects, and private projects in groups with a Premium plan." msgstr "" +msgid "DastConfig|Customize DAST settings to suit your requirements. Configuration changes made here override those provided by GitLab and are excluded from updates. For details of more advanced configuration options, see the %{docsLinkStart}GitLab DAST documentation%{docsLinkEnd}." +msgstr "" + +msgid "DastConfig|DAST Settings" +msgstr "" + +msgid "DastConfig|Scan Configuration" +msgstr "" + msgid "DastProfiles|A passive scan monitors all HTTP messages (requests and responses) sent to the target. An active scan attacks the target to find potential vulnerabilities." msgstr "" diff --git a/scripts/prepare_build.sh b/scripts/prepare_build.sh index e0be80d429f..5753a0af4f8 100644 --- a/scripts/prepare_build.sh +++ b/scripts/prepare_build.sh @@ -42,9 +42,6 @@ sed -i 's|url:.*$|url: redis://redis:6379/11|g' config/redis.queues.yml cp config/redis.shared_state.yml.example config/redis.shared_state.yml sed -i 's|url:.*$|url: redis://redis:6379/12|g' config/redis.shared_state.yml -cp config/redis.trace_chunks.yml.example config/redis.trace_chunks.yml -sed -i 's|url:.*$|url: redis://redis:6379/13|g' config/redis.trace_chunks.yml - if [ "$SETUP_DB" != "false" ]; then setup_db elif getent hosts postgres; then diff --git a/scripts/review_apps/base-config.yaml b/scripts/review_apps/base-config.yaml index bb4d5392b3b..29c8e5dc2ba 100644 --- a/scripts/review_apps/base-config.yaml +++ b/scripts/review_apps/base-config.yaml @@ -5,9 +5,9 @@ global: ingress: annotations: external-dns.alpha.kubernetes.io/ttl: 10 + cert-manager.io/cluster-issuer: review-apps-route53-dns01-wildcard-cluster-issuer + kubernetes.io/tls-acme: true configureCertmanager: false - tls: - secretName: review-apps-tls initialRootPassword: secret: shared-gitlab-initial-root-password certmanager: diff --git a/scripts/review_apps/review-apps.sh b/scripts/review_apps/review-apps.sh index 74c04ab2b7c..c1d274af56d 100755 --- a/scripts/review_apps/review-apps.sh +++ b/scripts/review_apps/review-apps.sh @@ -40,7 +40,7 @@ function previous_deploy_failed() { } function delete_release() { - local namespace="${KUBE_NAMESPACE}" + local namespace="${CI_ENVIRONMENT_SLUG}" local release="${CI_ENVIRONMENT_SLUG}" if [ -z "${release}" ]; then @@ -48,39 +48,11 @@ function delete_release() { return fi - # Check if helm release exists before attempting to delete - # There may be situation where k8s resources exist, but helm release does not, - # for example, following a failed helm install. - # In such cases, we still want to continue to clean up k8s resources. - if deploy_exists "${namespace}" "${release}"; then - helm_delete_release "${namespace}" "${release}" - fi - kubectl_cleanup_release "${namespace}" "${release}" -} - -function helm_delete_release() { - local namespace="${1}" - local release="${2}" - - echoinfo "Deleting Helm release '${release}'..." true - - helm uninstall --namespace "${namespace}" "${release}" -} - -function kubectl_cleanup_release() { - local namespace="${1}" - local release="${2}" - - echoinfo "Deleting all K8s resources matching '${release}'..." true - kubectl --namespace "${namespace}" get ingress,svc,pdb,hpa,deploy,statefulset,job,pod,secret,configmap,pvc,clusterrole,clusterrolebinding,role,rolebinding,sa,crd 2>&1 \ - | grep "${release}" \ - | awk '{print $1}' \ - | xargs kubectl --namespace "${namespace}" delete --ignore-not-found \ - || true + delete_k8s_release_namespace } function delete_failed_release() { - local namespace="${KUBE_NAMESPACE}" + local namespace="${CI_ENVIRONMENT_SLUG}" local release="${CI_ENVIRONMENT_SLUG}" if [ -z "${release}" ]; then @@ -93,7 +65,7 @@ function delete_failed_release() { else # Cleanup and previous installs, as FAILED and PENDING_UPGRADE will cause errors with `upgrade` if previous_deploy_failed "${namespace}" "${release}" ; then - echoinfo "Review App deployment in bad state, cleaning up ${release}" + echoinfo "Review App deployment in bad state, cleaning up namespace ${release}" delete_release else echoinfo "Review App deployment in good state" @@ -101,8 +73,14 @@ function delete_failed_release() { fi } +function delete_k8s_release_namespace() { + local namespace="${CI_ENVIRONMENT_SLUG}" + + kubectl delete namespace "${namespace}" --wait +} + function get_pod() { - local namespace="${KUBE_NAMESPACE}" + local namespace="${CI_ENVIRONMENT_SLUG}" local release="${CI_ENVIRONMENT_SLUG}" local app_name="${1}" local status="${2-Running}" @@ -133,7 +111,7 @@ function get_pod() { } function run_task() { - local namespace="${KUBE_NAMESPACE}" + local namespace="${CI_ENVIRONMENT_SLUG}" local ruby_cmd="${1}" local task_runner_pod=$(get_pod "task-runner") @@ -177,7 +155,7 @@ function check_kube_domain() { } function ensure_namespace() { - local namespace="${KUBE_NAMESPACE}" + local namespace="${1}" echoinfo "Ensuring the ${namespace} namespace exists..." true @@ -245,7 +223,7 @@ function install_certmanager() { } function create_application_secret() { - local namespace="${KUBE_NAMESPACE}" + local namespace="${CI_ENVIRONMENT_SLUG}" local release="${CI_ENVIRONMENT_SLUG}" local initial_root_password_shared_secret local gitlab_license_shared_secret @@ -306,7 +284,7 @@ function parse_gitaly_image_tag() { } function deploy() { - local namespace="${KUBE_NAMESPACE}" + local namespace="${CI_ENVIRONMENT_SLUG}" local release="${CI_ENVIRONMENT_SLUG}" local base_config_file_ref="${CI_DEFAULT_BRANCH}" if [[ "$(base_config_changed)" == "true" ]]; then base_config_file_ref="${CI_COMMIT_SHA}"; fi @@ -324,11 +302,14 @@ function deploy() { gitlab_shell_image_repository="${IMAGE_REPOSITORY}/gitlab-shell" gitlab_workhorse_image_repository="${IMAGE_REPOSITORY}/gitlab-workhorse-ee" + ensure_namespace "${namespace}" + create_application_secret HELM_CMD=$(cat << EOF helm upgrade \ --namespace="${namespace}" \ + --create-namespace \ --install \ --wait \ --timeout "${HELM_INSTALL_TIMEOUT:-20m}" \ @@ -339,6 +320,9 @@ HELM_CMD=$(cat << EOF --set releaseOverride="${release}" \ --set global.hosts.hostSuffix="${HOST_SUFFIX}" \ --set global.hosts.domain="${REVIEW_APPS_DOMAIN}" \ + --set gitlab.webservice.ingress.tls.secretName="${release}-gitlab-tls" \ + --set registry.ingress.tls.secretName="${release}-registry-tls" \ + --set minio.ingress.tls.secretName="${release}-minio-tls" \ --set gitlab.migrations.image.repository="${gitlab_migrations_image_repository}" \ --set gitlab.migrations.image.tag="${CI_COMMIT_REF_SLUG}" \ --set gitlab.gitaly.image.repository="${gitlab_gitaly_image_repository}" \ @@ -382,7 +366,7 @@ EOF } function display_deployment_debug() { - local namespace="${KUBE_NAMESPACE}" + local namespace="${CI_ENVIRONMENT_SLUG}" local release="${CI_ENVIRONMENT_SLUG}" # Get all pods for this release diff --git a/spec/controllers/groups/registry/repositories_controller_spec.rb b/spec/controllers/groups/registry/repositories_controller_spec.rb index 35c9a80266e..f4541eda293 100644 --- a/spec/controllers/groups/registry/repositories_controller_spec.rb +++ b/spec/controllers/groups/registry/repositories_controller_spec.rb @@ -75,6 +75,8 @@ RSpec.describe Groups::Registry::RepositoriesController do context 'json format' do let(:format) { :json } + let(:namespace) { group } + let(:snowplow_gitlab_standard_context) { { user: user, namespace: group } } it 'has the correct response schema' do subject diff --git a/spec/frontend/batch_comments/components/preview_item_spec.js b/spec/frontend/batch_comments/components/preview_item_spec.js index 03a28ce8001..cb71edd1238 100644 --- a/spec/frontend/batch_comments/components/preview_item_spec.js +++ b/spec/frontend/batch_comments/components/preview_item_spec.js @@ -104,7 +104,7 @@ describe('Batch comments draft preview item component', () => { notes: [ { author: { - name: 'Author Name', + name: "Author 'Nick' Name", }, }, ], @@ -114,7 +114,7 @@ describe('Batch comments draft preview item component', () => { it('renders title', () => { expect(vm.$el.querySelector('.review-preview-item-header-text').textContent).toContain( - "Author Name's thread", + "Author 'Nick' Name's thread", ); }); diff --git a/spec/frontend/content_editor/services/track_input_rules_and_shortcuts_spec.js b/spec/frontend/content_editor/services/track_input_rules_and_shortcuts_spec.js index 437714ba938..cf74b5c56c9 100644 --- a/spec/frontend/content_editor/services/track_input_rules_and_shortcuts_spec.js +++ b/spec/frontend/content_editor/services/track_input_rules_and_shortcuts_spec.js @@ -5,11 +5,8 @@ import { Heading } from '@tiptap/extension-heading'; import { ListItem } from '@tiptap/extension-list-item'; import { Paragraph } from '@tiptap/extension-paragraph'; import { Text } from '@tiptap/extension-text'; -import { Editor, EditorContent } from '@tiptap/vue-2'; -import { mount } from '@vue/test-utils'; -import { nextTick } from 'vue'; +import { Editor } from '@tiptap/vue-2'; import { mockTracking } from 'helpers/tracking_helper'; -import { extendedWrapper } from 'helpers/vue_test_utils_helper'; import { KEYBOARD_SHORTCUT_TRACKING_ACTION, INPUT_RULE_TRACKING_ACTION, @@ -19,47 +16,33 @@ import trackInputRulesAndShortcuts from '~/content_editor/services/track_input_r import { ENTER_KEY, BACKSPACE_KEY } from '~/lib/utils/keys'; describe('content_editor/services/track_input_rules_and_shortcuts', () => { - let wrapper; let trackingSpy; let editor; + let trackedExtensions; const HEADING_TEXT = 'Heading text'; - - const buildWrapper = () => { - wrapper = extendedWrapper( - mount(EditorContent, { - propsData: { - editor, - }, - }), - ); - }; + const extensions = [Document, Paragraph, Text, Heading, CodeBlockLowlight, BulletList, ListItem]; beforeEach(() => { trackingSpy = mockTracking(undefined, null, jest.spyOn); }); - afterEach(() => { - wrapper.destroy(); - }); - describe('given the heading extension is instrumented', () => { beforeEach(() => { + trackedExtensions = extensions.map(trackInputRulesAndShortcuts); editor = new Editor({ - extensions: [ - Document, - Paragraph, - Text, - Heading, - CodeBlockLowlight, - BulletList, - ListItem, - ].map(trackInputRulesAndShortcuts), + extensions: extensions.map(trackInputRulesAndShortcuts), }); }); - beforeEach(async () => { - buildWrapper(); - await nextTick(); + it('does not remove existing keyboard shortcuts', () => { + extensions.forEach((extension, index) => { + const originalShortcuts = Object.keys(extension.addKeyboardShortcuts?.() || {}); + const trackedShortcuts = Object.keys( + trackedExtensions[index].addKeyboardShortcuts?.() || {}, + ); + + expect(originalShortcuts).toEqual(trackedShortcuts); + }); }); describe('when creating a heading using an keyboard shortcut', () => { diff --git a/spec/lib/gitlab/emoji_spec.rb b/spec/lib/gitlab/emoji_spec.rb index ada37f25d1e..8f855489c12 100644 --- a/spec/lib/gitlab/emoji_spec.rb +++ b/spec/lib/gitlab/emoji_spec.rb @@ -91,7 +91,16 @@ RSpec.describe Gitlab::Emoji do it 'returns emoji image tag' do emoji_image = described_class.emoji_image_tag('emoji_one', 'src_url') - expect(emoji_image).to eq( "<img class='emoji' title=':emoji_one:' alt=':emoji_one:' src='src_url' height='20' width='20' align='absmiddle' />") + expect(emoji_image).to eq("<img class=\"emoji\" src=\"src_url\" title=\":emoji_one:\" alt=\":emoji_one:\" height=\"20\" width=\"20\" align=\"absmiddle\" />") + end + + it 'escapes emoji image attrs to prevent XSS' do + xss_payload = "<script>alert(1)</script>" + escaped_xss_payload = html_escape(xss_payload) + + emoji_image = described_class.emoji_image_tag(xss_payload, 'http://aaa#' + xss_payload) + + expect(emoji_image).to eq("<img class=\"emoji\" src=\"http://aaa##{escaped_xss_payload}\" title=\":#{escaped_xss_payload}:\" alt=\":#{escaped_xss_payload}:\" height=\"20\" width=\"20\" align=\"absmiddle\" />") end end diff --git a/spec/lib/gitlab/health_checks/redis/trace_chunks_check_spec.rb b/spec/lib/gitlab/health_checks/redis/trace_chunks_check_spec.rb deleted file mode 100644 index 5fb5232a4dd..00000000000 --- a/spec/lib/gitlab/health_checks/redis/trace_chunks_check_spec.rb +++ /dev/null @@ -1,8 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' -require_relative '../simple_check_shared' - -RSpec.describe Gitlab::HealthChecks::Redis::TraceChunksCheck do - include_examples 'simple_check', 'redis_trace_chunks_ping', 'RedisTraceChunks', 'PONG' -end diff --git a/spec/lib/gitlab/instrumentation/redis_spec.rb b/spec/lib/gitlab/instrumentation/redis_spec.rb index 6cddf958f2a..d712d09bdd8 100644 --- a/spec/lib/gitlab/instrumentation/redis_spec.rb +++ b/spec/lib/gitlab/instrumentation/redis_spec.rb @@ -68,8 +68,7 @@ RSpec.describe Gitlab::Instrumentation::Redis do .to contain_exactly(details_row.merge(storage: 'ActionCable'), details_row.merge(storage: 'Cache'), details_row.merge(storage: 'Queues'), - details_row.merge(storage: 'SharedState'), - details_row.merge(storage: 'TraceChunks')) + details_row.merge(storage: 'SharedState')) end end end diff --git a/spec/lib/gitlab/redis/trace_chunks_spec.rb b/spec/lib/gitlab/redis/trace_chunks_spec.rb deleted file mode 100644 index e974dc519d6..00000000000 --- a/spec/lib/gitlab/redis/trace_chunks_spec.rb +++ /dev/null @@ -1,55 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::Redis::TraceChunks do - let(:instance_specific_config_file) { "config/redis.trace_chunks.yml" } - let(:environment_config_file_name) { "GITLAB_REDIS_TRACE_CHUNKS_CONFIG_FILE" } - let(:shared_state_config_file) { nil } - - before do - allow(Gitlab::Redis::SharedState).to receive(:config_file_name).and_return(shared_state_config_file) - end - - include_examples "redis_shared_examples" - - describe '.config_file_name' do - subject { described_class.config_file_name } - - let(:rails_root) { Dir.mktmpdir('redis_shared_examples') } - - before do - # Undo top-level stub of config_file_name because we are testing that method now. - allow(described_class).to receive(:config_file_name).and_call_original - - allow(described_class).to receive(:rails_root).and_return(rails_root) - FileUtils.mkdir_p(File.join(rails_root, 'config')) - end - - after do - FileUtils.rm_rf(rails_root) - end - - context 'when there is only a resque.yml' do - before do - FileUtils.touch(File.join(rails_root, 'config/resque.yml')) - end - - it { expect(subject).to eq("#{rails_root}/config/resque.yml") } - - context 'and there is a global env override' do - before do - stub_env('GITLAB_REDIS_CONFIG_FILE', 'global override') - end - - it { expect(subject).to eq('global override') } - - context 'and SharedState has a different config file' do - let(:shared_state_config_file) { 'shared state config file' } - - it { expect(subject).to eq('shared state config file') } - end - end - end - end -end diff --git a/spec/lib/gitlab/usage/metrics/name_suggestion_spec.rb b/spec/lib/gitlab/usage/metrics/name_suggestion_spec.rb new file mode 100644 index 00000000000..2da0e7df72f --- /dev/null +++ b/spec/lib/gitlab/usage/metrics/name_suggestion_spec.rb @@ -0,0 +1,110 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Usage::Metrics::NameSuggestion do + describe '#for' do + shared_examples 'name suggestion' do + it 'return correct name' do + expect(described_class.for(operation, relation: relation, column: column)).to match name_suggestion + end + end + + context 'for count with nil column' do + it_behaves_like 'name suggestion' do + let(:operation) { :count } + let(:relation) { Board } + let(:column) { nil } + let(:name_suggestion) { /count_boards/ } + end + end + + context 'for count with column :id' do + it_behaves_like 'name suggestion' do + let(:operation) { :count } + let(:relation) { Board } + let(:column) { :id } + let(:name_suggestion) { /count_boards/ } + end + end + + context 'for count distinct with column defined metrics' do + it_behaves_like 'name suggestion' do + let(:operation) { :distinct_count } + let(:relation) { ZoomMeeting } + let(:column) { :issue_id } + let(:name_suggestion) { /count_distinct_issue_id_from_zoom_meetings/ } + end + end + + context 'joined relations' do + context 'counted attribute comes from joined relation' do + it_behaves_like 'name suggestion' do + let(:operation) { :distinct_count } + let(:column) { ::Deployment.arel_table[:environment_id] } + let(:relation) do + ::Clusters::Applications::Ingress.modsecurity_enabled.logging + .joins(cluster: :deployments) + .merge(::Clusters::Cluster.enabled) + .merge(Deployment.success) + end + + let(:constraints) { /'\(clusters_applications_ingress\.modsecurity_enabled = TRUE AND clusters_applications_ingress\.modsecurity_mode = \d+ AND clusters.enabled = TRUE AND deployments.status = \d+\)'/ } + let(:name_suggestion) { /count_distinct_environment_id_from_<adjective describing\: #{constraints}>_deployments_<with>_<adjective describing\: #{constraints}>_clusters_<having>_<adjective describing\: #{constraints}>_clusters_applications_ingress/ } + end + end + + context 'counted attribute comes from source relation' do + it_behaves_like 'name suggestion' do + # corresponding metric is collected with count(Issue.with_alert_management_alerts.not_authored_by(::User.alert_bot), start: issue_minimum_id, finish: issue_maximum_id) + let(:operation) { :count } + let(:relation) { Issue.with_alert_management_alerts.not_authored_by(::User.alert_bot) } + let(:column) { nil } + let(:name_suggestion) { /count_<adjective describing\: '\(issues\.author_id != \d+\)'>_issues_<with>_alert_management_alerts/ } + end + end + end + + context 'strips off time period constraint' do + it_behaves_like 'name suggestion' do + # corresponding metric is collected with distinct_count(::Clusters::Cluster.aws_installed.enabled.where(time_period), :user_id) + let(:operation) { :distinct_count } + let(:relation) { ::Clusters::Cluster.aws_installed.enabled.where(created_at: 30.days.ago..2.days.ago ) } + let(:column) { :user_id } + let(:constraints) { /<adjective describing\: '\(clusters.provider_type = \d+ AND \(cluster_providers_aws\.status IN \(\d+\)\) AND clusters\.enabled = TRUE\)'>/ } + let(:name_suggestion) { /count_distinct_user_id_from_#{constraints}_clusters_<with>_#{constraints}_cluster_providers_aws/ } + end + end + + context 'for sum metrics' do + it_behaves_like 'name suggestion' do + # corresponding metric is collected with sum(JiraImportState.finished, :imported_issues_count) + let(:key_path) { 'counts.jira_imports_total_imported_issues_count' } + let(:operation) { :sum } + let(:relation) { JiraImportState.finished } + let(:column) { :imported_issues_count} + let(:name_suggestion) { /sum_imported_issues_count_from_<adjective describing\: '\(jira_imports\.status = \d+\)'>_jira_imports/ } + end + end + + context 'for redis metrics' do + it_behaves_like 'name suggestion' do + # corresponding metric is collected with redis_usage_data { unique_visit_service.unique_visits_for(targets: :analytics) } + let(:operation) { :redis } + let(:column) { nil } + let(:relation) { nil } + let(:name_suggestion) { /<please fill metric name, suggested format is: {subject}_{verb}{ing|ed}_{object} eg: users_creating_epics or merge_requests_viewed_in_single_file_mode>/ } + end + end + + context 'for alt_usage_data metrics' do + it_behaves_like 'name suggestion' do + # corresponding metric is collected with alt_usage_data(fallback: nil) { operating_system } + let(:operation) { :alt } + let(:column) { nil } + let(:relation) { nil } + let(:name_suggestion) { /<please fill metric name>/ } + end + end + end +end diff --git a/spec/mailers/emails/service_desk_spec.rb b/spec/mailers/emails/service_desk_spec.rb index 57fa990d399..995e6c006cd 100644 --- a/spec/mailers/emails/service_desk_spec.rb +++ b/spec/mailers/emails/service_desk_spec.rb @@ -115,6 +115,16 @@ RSpec.describe Emails::ServiceDesk do end end + shared_examples 'notification with metric event' do |event_type| + it 'adds metric event' do + metric_transaction = double('Gitlab::Metrics::WebTransaction', increment: true, observe: true) + allow(::Gitlab::Metrics::BackgroundTransaction).to receive(:current).and_return(metric_transaction) + expect(metric_transaction).to receive(:add_event).with(event_type) + + subject.content_type + end + end + describe '.service_desk_thank_you_email' do let_it_be(:reply_in_subject) { true } let_it_be(:default_text) do @@ -124,6 +134,7 @@ RSpec.describe Emails::ServiceDesk do subject { ServiceEmailClass.service_desk_thank_you_email(issue.id) } it_behaves_like 'read template from repository', 'thank_you' + it_behaves_like 'notification with metric event', :service_desk_thank_you_email context 'handling template markdown' do context 'with a simple text' do @@ -164,6 +175,7 @@ RSpec.describe Emails::ServiceDesk do subject { ServiceEmailClass.service_desk_new_note_email(issue.id, note.id, email) } it_behaves_like 'read template from repository', 'new_note' + it_behaves_like 'notification with metric event', :service_desk_new_note_email context 'handling template markdown' do context 'with a simple text' do diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index 73d0f273504..7468c1b9f0a 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -85,9 +85,9 @@ RSpec.describe Key, :mailer do let_it_be(:expiring_soon_notified) { create(:key, expires_at: 4.days.from_now, user: user, before_expiry_notification_delivered_at: Time.current) } let_it_be(:future_expiry) { create(:key, expires_at: 1.month.from_now, user: user) } - describe '.expired_today_and_not_notified' do - it 'returns keys that expire today' do - expect(described_class.expired_today_and_not_notified).to contain_exactly(expired_today_not_notified) + describe '.expired_and_not_notified' do + it 'returns keys that expire today and in the past' do + expect(described_class.expired_and_not_notified).to contain_exactly(expired_today_not_notified, expired_yesterday) end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 6f19f3d44b1..bf2d2473eeb 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -92,7 +92,7 @@ RSpec.describe User do it { is_expected.to have_many(:group_members) } it { is_expected.to have_many(:groups) } it { is_expected.to have_many(:keys).dependent(:destroy) } - it { is_expected.to have_many(:expired_today_and_unnotified_keys) } + it { is_expected.to have_many(:expired_and_unnotified_keys) } it { is_expected.to have_many(:deploy_keys).dependent(:nullify) } it { is_expected.to have_many(:group_deploy_keys) } it { is_expected.to have_many(:events).dependent(:delete_all) } @@ -1027,12 +1027,6 @@ RSpec.describe User do let_it_be(:expiring_soon_not_notified) { create(:key, expires_at: 2.days.from_now, user: user2) } let_it_be(:expiring_soon_notified) { create(:key, expires_at: 2.days.from_now, user: user1, before_expiry_notification_delivered_at: Time.current) } - describe '.with_ssh_key_expired_today' do - it 'returns users whose key has expired today' do - expect(described_class.with_ssh_key_expired_today).to contain_exactly(user1) - end - end - describe '.with_ssh_key_expiring_soon' do it 'returns users whose keys will expire soon' do expect(described_class.with_ssh_key_expiring_soon).to contain_exactly(user2) diff --git a/spec/requests/api/composer_packages_spec.rb b/spec/requests/api/composer_packages_spec.rb index 0ff88cb41a8..4120edabea3 100644 --- a/spec/requests/api/composer_packages_spec.rb +++ b/spec/requests/api/composer_packages_spec.rb @@ -9,6 +9,7 @@ RSpec.describe API::ComposerPackages do let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } let_it_be(:package_name) { 'package-name' } let_it_be(:project, reload: true) { create(:project, :custom_repo, files: { 'composer.json' => { name: package_name }.to_json }, group: group) } + let(:snowplow_gitlab_standard_context) { { project: project, namespace: project.namespace, user: user } } let(:headers) { {} } using RSpec::Parameterized::TableSyntax @@ -428,6 +429,7 @@ RSpec.describe API::ComposerPackages do with_them do let(:token) { user_token ? personal_access_token.token : 'wrong' } let(:headers) { user_role == :anonymous ? {} : basic_auth_header(user.username, token) } + let(:snowplow_gitlab_standard_context) { { project: project, namespace: project.namespace } } before do project.update!(visibility_level: Gitlab::VisibilityLevel.const_get(project_visibility_level, false)) diff --git a/spec/requests/api/debian_project_packages_spec.rb b/spec/requests/api/debian_project_packages_spec.rb index c74c0ea1c2a..023904cb2b8 100644 --- a/spec/requests/api/debian_project_packages_spec.rb +++ b/spec/requests/api/debian_project_packages_spec.rb @@ -39,6 +39,7 @@ RSpec.describe API::DebianProjectPackages do describe 'PUT projects/:id/packages/debian/:file_name' do let(:method) { :put } let(:url) { "/projects/#{container.id}/packages/debian/#{file_name}" } + let(:snowplow_gitlab_standard_context) { { project: container, user: user, namespace: container.namespace } } context 'with a deb' do let(:file_name) { 'libsample0_1.2.3~alpha2_amd64.deb' } diff --git a/spec/requests/api/graphql/project_query_spec.rb b/spec/requests/api/graphql/project_query_spec.rb index b367bbaaf43..4160538b243 100644 --- a/spec/requests/api/graphql/project_query_spec.rb +++ b/spec/requests/api/graphql/project_query_spec.rb @@ -119,6 +119,29 @@ RSpec.describe 'getting project information' do end end + context 'when the user has reporter access to the project' do + let(:statistics_query) do + <<~GRAPHQL + { + project(fullPath: "#{project.full_path}") { + statistics { wikiSize } + } + } + GRAPHQL + end + + before do + project.add_reporter(current_user) + create(:project_statistics, project: project, wiki_size: 100) + end + + it 'allows fetching project statistics' do + post_graphql(statistics_query, current_user: current_user) + + expect(graphql_data.dig('project', 'statistics')).to include('wikiSize' => 100.0) + end + end + context 'when the user does not have access to the project' do it 'returns an empty field' do post_graphql(query, current_user: current_user) diff --git a/spec/requests/api/group_container_repositories_spec.rb b/spec/requests/api/group_container_repositories_spec.rb index 4584ef37bd0..fdbf910e4bc 100644 --- a/spec/requests/api/group_container_repositories_spec.rb +++ b/spec/requests/api/group_container_repositories_spec.rb @@ -33,6 +33,7 @@ RSpec.describe API::GroupContainerRepositories do describe 'GET /groups/:id/registry/repositories' do let(:url) { "/groups/#{group.id}/registry/repositories" } + let(:snowplow_gitlab_standard_context) { { user: api_user, namespace: group } } subject { get api(url, api_user) } diff --git a/spec/requests/api/helm_packages_spec.rb b/spec/requests/api/helm_packages_spec.rb index 01ee69d0a0e..5871c0a5d5b 100644 --- a/spec/requests/api/helm_packages_spec.rb +++ b/spec/requests/api/helm_packages_spec.rb @@ -44,6 +44,7 @@ RSpec.describe API::HelmPackages do with_them do let(:token) { user_token ? personal_access_token.token : 'wrong' } let(:headers) { user_role == :anonymous ? {} : basic_auth_header(user.username, token) } + let(:snowplow_gitlab_standard_context) { { project: project, namespace: project.namespace } } subject { get api(url), headers: headers } diff --git a/spec/requests/api/maven_packages_spec.rb b/spec/requests/api/maven_packages_spec.rb index de9f60a7929..2bb6d05f54b 100644 --- a/spec/requests/api/maven_packages_spec.rb +++ b/spec/requests/api/maven_packages_spec.rb @@ -21,6 +21,7 @@ RSpec.describe API::MavenPackages do let_it_be(:deploy_token_for_group) { create(:deploy_token, :group, read_package_registry: true, write_package_registry: true) } let_it_be(:group_deploy_token) { create(:group_deploy_token, deploy_token: deploy_token_for_group, group: group) } + let(:snowplow_gitlab_standard_context) { { project: project, namespace: project.namespace, user: user } } let(:package_name) { 'com/example/my-app' } let(:headers) { workhorse_headers } let(:headers_with_token) { headers.merge('Private-Token' => personal_access_token.token) } @@ -96,6 +97,8 @@ RSpec.describe API::MavenPackages do context 'with jar file' do let_it_be(:package_file) { jar_file } + let(:snowplow_gitlab_standard_context) { { project: project, namespace: project.namespace } } + it_behaves_like 'a package tracking event', described_class.name, 'pull_package' end end diff --git a/spec/requests/api/npm_project_packages_spec.rb b/spec/requests/api/npm_project_packages_spec.rb index 10271719a15..8230061546f 100644 --- a/spec/requests/api/npm_project_packages_spec.rb +++ b/spec/requests/api/npm_project_packages_spec.rb @@ -71,6 +71,8 @@ RSpec.describe API::NpmProjectPackages do end context 'a public project' do + let(:snowplow_gitlab_standard_context) { { project: project, namespace: project.namespace } } + it_behaves_like 'successfully downloads the file' it_behaves_like 'a package tracking event', 'API::NpmPackages', 'pull_package' @@ -161,6 +163,7 @@ RSpec.describe API::NpmProjectPackages do context 'valid package record' do let(:params) { upload_params(package_name: package_name) } + let(:snowplow_gitlab_standard_context) { { project: project, namespace: project.namespace, user: user } } shared_examples 'handling upload with different authentications' do context 'with access token' do diff --git a/spec/requests/api/nuget_group_packages_spec.rb b/spec/requests/api/nuget_group_packages_spec.rb index aefbc89dc3b..1b71f0f9de1 100644 --- a/spec/requests/api/nuget_group_packages_spec.rb +++ b/spec/requests/api/nuget_group_packages_spec.rb @@ -46,6 +46,7 @@ RSpec.describe API::NugetGroupPackages do let_it_be(:group_deploy_token) { create(:group_deploy_token, deploy_token: deploy_token, group: subgroup) } let(:target) { subgroup } + let(:snowplow_gitlab_standard_context) { { namespace: subgroup } } it_behaves_like 'handling all endpoints' @@ -57,6 +58,7 @@ RSpec.describe API::NugetGroupPackages do context 'a group' do let(:target) { group } + let(:snowplow_gitlab_standard_context) { { namespace: group } } it_behaves_like 'handling all endpoints' diff --git a/spec/requests/api/nuget_project_packages_spec.rb b/spec/requests/api/nuget_project_packages_spec.rb index 54fe0b985df..98458cb8dfa 100644 --- a/spec/requests/api/nuget_project_packages_spec.rb +++ b/spec/requests/api/nuget_project_packages_spec.rb @@ -16,6 +16,7 @@ RSpec.describe API::NugetProjectPackages do describe 'GET /api/v4/projects/:id/packages/nuget' do it_behaves_like 'handling nuget service requests' do let(:url) { "/projects/#{target.id}/packages/nuget/index.json" } + let(:snowplow_gitlab_standard_context) { { project: project, namespace: project.namespace } } end end @@ -34,6 +35,7 @@ RSpec.describe API::NugetProjectPackages do describe 'GET /api/v4/projects/:id/packages/nuget/query' do it_behaves_like 'handling nuget search requests' do let(:url) { "/projects/#{target.id}/packages/nuget/query?#{query_parameters.to_query}" } + let(:snowplow_gitlab_standard_context) { { project: project, namespace: project.namespace } } end end @@ -121,6 +123,7 @@ RSpec.describe API::NugetProjectPackages do with_them do let(:token) { user_token ? personal_access_token.token : 'wrong' } let(:headers) { user_role == :anonymous ? {} : basic_auth_header(user.username, token) } + let(:snowplow_gitlab_standard_context) { { project: project, namespace: project.namespace } } subject { get api(url), headers: headers } @@ -244,6 +247,7 @@ RSpec.describe API::NugetProjectPackages do let(:token) { user_token ? personal_access_token.token : 'wrong' } let(:user_headers) { user_role == :anonymous ? {} : basic_auth_header(user.username, token) } let(:headers) { user_headers.merge(workhorse_headers) } + let(:snowplow_gitlab_standard_context) { { project: project, user: user, namespace: project.namespace } } before do update_visibility_to(Gitlab::VisibilityLevel.const_get(visibility_level, false)) diff --git a/spec/requests/api/project_container_repositories_spec.rb b/spec/requests/api/project_container_repositories_spec.rb index f3da99573fe..695d2c3fe2c 100644 --- a/spec/requests/api/project_container_repositories_spec.rb +++ b/spec/requests/api/project_container_repositories_spec.rb @@ -32,6 +32,8 @@ RSpec.describe API::ProjectContainerRepositories do let(:method) { :get } let(:params) { {} } + let(:snowplow_gitlab_standard_context) { { user: api_user, project: project, namespace: project.namespace } } + before_all do project.add_maintainer(maintainer) project.add_developer(developer) @@ -405,7 +407,7 @@ RSpec.describe API::ProjectContainerRepositories do subject expect(response).to have_gitlab_http_status(:ok) - expect_snowplow_event(category: described_class.name, action: 'delete_tag') + expect_snowplow_event(category: described_class.name, action: 'delete_tag', project: project, user: api_user, namespace: project.namespace) end end @@ -421,7 +423,7 @@ RSpec.describe API::ProjectContainerRepositories do subject expect(response).to have_gitlab_http_status(:ok) - expect_snowplow_event(category: described_class.name, action: 'delete_tag') + expect_snowplow_event(category: described_class.name, action: 'delete_tag', project: project, user: api_user, namespace: project.namespace) end end end diff --git a/spec/requests/api/project_statistics_spec.rb b/spec/requests/api/project_statistics_spec.rb index 5f0cac403aa..d314af0746a 100644 --- a/spec/requests/api/project_statistics_spec.rb +++ b/spec/requests/api/project_statistics_spec.rb @@ -3,11 +3,11 @@ require 'spec_helper' RSpec.describe API::ProjectStatistics do - let_it_be(:developer) { create(:user) } + let_it_be(:reporter) { create(:user) } let_it_be(:public_project) { create(:project, :public) } before do - public_project.add_developer(developer) + public_project.add_reporter(reporter) end describe 'GET /projects/:id/statistics' do @@ -19,7 +19,7 @@ RSpec.describe API::ProjectStatistics do let_it_be(:fetch_statistics_other_project) { create(:project_daily_statistic, project: create(:project), fetch_count: 29, date: 29.days.ago) } it 'returns the fetch statistics of the last 30 days' do - get api("/projects/#{public_project.id}/statistics", developer) + get api("/projects/#{public_project.id}/statistics", reporter) expect(response).to have_gitlab_http_status(:ok) fetches = json_response['fetches'] @@ -32,7 +32,7 @@ RSpec.describe API::ProjectStatistics do it 'excludes the fetch statistics older than 30 days' do create(:project_daily_statistic, fetch_count: 31, project: public_project, date: 30.days.ago) - get api("/projects/#{public_project.id}/statistics", developer) + get api("/projects/#{public_project.id}/statistics", reporter) expect(response).to have_gitlab_http_status(:ok) fetches = json_response['fetches'] @@ -41,7 +41,7 @@ RSpec.describe API::ProjectStatistics do expect(fetches['days'].last).to eq({ 'count' => fetch_statistics1.fetch_count, 'date' => fetch_statistics1.date.to_s }) end - it 'responds with 403 when the user is not a developer of the repository' do + it 'responds with 403 when the user is not a reporter of the repository' do guest = create(:user) public_project.add_guest(guest) diff --git a/spec/requests/api/pypi_packages_spec.rb b/spec/requests/api/pypi_packages_spec.rb index 4e8c50fedb6..552ef2b2120 100644 --- a/spec/requests/api/pypi_packages_spec.rb +++ b/spec/requests/api/pypi_packages_spec.rb @@ -23,6 +23,7 @@ RSpec.describe API::PypiPackages do describe 'GET /api/v4/groups/:id/-/packages/pypi/simple/:package_name' do let(:url) { "/groups/#{group.id}/-/packages/pypi/simple/#{package.name}" } + let(:snowplow_gitlab_standard_context) { {} } it_behaves_like 'pypi simple API endpoint' it_behaves_like 'rejects PyPI access with unknown group id' @@ -53,6 +54,7 @@ RSpec.describe API::PypiPackages do describe 'GET /api/v4/projects/:id/packages/pypi/simple/:package_name' do let(:url) { "/projects/#{project.id}/packages/pypi/simple/#{package.name}" } + let(:snowplow_gitlab_standard_context) { { project: project, namespace: project.namespace } } it_behaves_like 'pypi simple API endpoint' it_behaves_like 'rejects PyPI access with unknown project id' @@ -121,6 +123,7 @@ RSpec.describe API::PypiPackages do let(:base_params) { { requires_python: requires_python, version: '1.0.0', name: 'sample-project', sha256_digest: '123' } } let(:params) { base_params.merge(content: temp_file(file_name)) } let(:send_rewritten_field) { true } + let(:snowplow_gitlab_standard_context) { { project: project, namespace: project.namespace, user: user } } subject do workhorse_finalize( @@ -221,6 +224,7 @@ RSpec.describe API::PypiPackages do describe 'GET /api/v4/groups/:id/-/packages/pypi/files/:sha256/*file_identifier' do let(:url) { "/groups/#{group.id}/-/packages/pypi/files/#{package.package_files.first.file_sha256}/#{package_name}-1.0.0.tar.gz" } + let(:snowplow_gitlab_standard_context) { {} } it_behaves_like 'pypi file download endpoint' it_behaves_like 'rejects PyPI access with unknown group id' @@ -229,6 +233,7 @@ RSpec.describe API::PypiPackages do describe 'GET /api/v4/projects/:id/packages/pypi/files/:sha256/*file_identifier' do let(:url) { "/projects/#{project.id}/packages/pypi/files/#{package.package_files.first.file_sha256}/#{package_name}-1.0.0.tar.gz" } + let(:snowplow_gitlab_standard_context) { { project: project, namespace: project.namespace } } it_behaves_like 'pypi file download endpoint' it_behaves_like 'rejects PyPI access with unknown project id' diff --git a/spec/requests/api/rubygem_packages_spec.rb b/spec/requests/api/rubygem_packages_spec.rb index d6ad8186063..09b63139e54 100644 --- a/spec/requests/api/rubygem_packages_spec.rb +++ b/spec/requests/api/rubygem_packages_spec.rb @@ -14,6 +14,7 @@ RSpec.describe API::RubygemPackages do let_it_be(:deploy_token) { create(:deploy_token, read_package_registry: true, write_package_registry: true) } let_it_be(:project_deploy_token) { create(:project_deploy_token, deploy_token: deploy_token, project: project) } let_it_be(:headers) { {} } + let(:snowplow_gitlab_standard_context) { { project: project, namespace: project.namespace, user: user } } let(:tokens) do { @@ -162,6 +163,7 @@ RSpec.describe API::RubygemPackages do with_them do let(:token) { valid_token ? tokens[token_type] : 'invalid-token123' } let(:headers) { user_role == :anonymous ? {} : { 'HTTP_AUTHORIZATION' => token } } + let(:snowplow_gitlab_standard_context) { { project: project, namespace: project.namespace } } before do project.update_column(:visibility_level, Gitlab::VisibilityLevel.level_value(visibility.to_s)) @@ -304,6 +306,16 @@ RSpec.describe API::RubygemPackages do let(:token) { valid_token ? tokens[token_type] : 'invalid-token123' } let(:user_headers) { user_role == :anonymous ? {} : { 'HTTP_AUTHORIZATION' => token } } let(:headers) { user_headers.merge(workhorse_headers) } + let(:snowplow_gitlab_standard_context) { { project: project, namespace: project.namespace, user: snowplow_user } } + let(:snowplow_user) do + if token_type == :deploy_token + deploy_token + elsif token_type == :job_token + job.user + else + user + end + end before do project.update_column(:visibility_level, Gitlab::VisibilityLevel.level_value(visibility.to_s)) diff --git a/spec/requests/api/terraform/modules/v1/packages_spec.rb b/spec/requests/api/terraform/modules/v1/packages_spec.rb index d318b22cf27..6803c09b8c2 100644 --- a/spec/requests/api/terraform/modules/v1/packages_spec.rb +++ b/spec/requests/api/terraform/modules/v1/packages_spec.rb @@ -188,6 +188,7 @@ RSpec.describe API::Terraform::Modules::V1::Packages do with_them do let(:token) { valid_token ? tokens[token_type] : 'invalid-token123' } let(:url) { api("/packages/terraform/modules/v1/#{group.path}/#{package.name}/#{package.version}/file?token=#{token}") } + let(:snowplow_gitlab_standard_context) { { project: project, user: user, namespace: project.namespace } } before do group.update!(visibility: visibility.to_s) @@ -330,6 +331,16 @@ RSpec.describe API::Terraform::Modules::V1::Packages do let(:token) { valid_token ? tokens[token_type] : 'invalid-token123' } let(:user_headers) { user_role == :anonymous ? {} : { token_header => token } } let(:headers) { user_headers.merge(workhorse_headers) } + let(:snowplow_gitlab_standard_context) { { project: project, namespace: project.namespace, user: snowplow_user } } + let(:snowplow_user) do + if token_type == :deploy_token + deploy_token + elsif token_type == :job_token + job.user + else + user + end + end before do project.update!(visibility: visibility.to_s) diff --git a/spec/support/shared_contexts/policies/project_policy_shared_context.rb b/spec/support/shared_contexts/policies/project_policy_shared_context.rb index 35dc709b5d9..d638ffcf8fa 100644 --- a/spec/support/shared_contexts/policies/project_policy_shared_context.rb +++ b/spec/support/shared_contexts/policies/project_policy_shared_context.rb @@ -26,7 +26,7 @@ RSpec.shared_context 'ProjectPolicy context' do let(:base_reporter_permissions) do %i[ admin_issue admin_issue_link admin_label admin_issue_board_list create_snippet - download_code download_wiki_code fork_project metrics_dashboard + daily_statistics download_code download_wiki_code fork_project metrics_dashboard read_build read_commit_status read_confidential_issues read_container_image read_deployment read_environment read_merge_request read_metrics_dashboard_annotation read_pipeline read_prometheus @@ -44,7 +44,7 @@ RSpec.shared_context 'ProjectPolicy context' do create_commit_status create_container_image create_deployment create_environment create_merge_request_from create_metrics_dashboard_annotation create_pipeline create_release - create_wiki daily_statistics delete_metrics_dashboard_annotation + create_wiki delete_metrics_dashboard_annotation destroy_container_image push_code read_pod_logs read_terraform_state resolve_note update_build update_commit_status update_container_image update_deployment update_environment update_merge_request diff --git a/spec/support/shared_examples/requests/api/packages_shared_examples.rb b/spec/support/shared_examples/requests/api/packages_shared_examples.rb index eb86b7c37d5..42c29084d7b 100644 --- a/spec/support/shared_examples/requests/api/packages_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/packages_shared_examples.rb @@ -146,6 +146,6 @@ RSpec.shared_examples 'a package tracking event' do |category, action| it "creates a gitlab tracking event #{action}", :snowplow do expect { subject }.to change { Packages::Event.count }.by(1) - expect_snowplow_event(category: category, action: action) + expect_snowplow_event(category: category, action: action, **snowplow_gitlab_standard_context) end end diff --git a/spec/support/shared_examples/requests/api/pypi_packages_shared_examples.rb b/spec/support/shared_examples/requests/api/pypi_packages_shared_examples.rb index cf749aab79a..8a351226123 100644 --- a/spec/support/shared_examples/requests/api/pypi_packages_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/pypi_packages_shared_examples.rb @@ -224,6 +224,7 @@ RSpec.shared_examples 'pypi simple API endpoint' do let(:url) { "/projects/#{project.id}/packages/pypi/simple/my-package" } let(:headers) { basic_auth_header(user.username, personal_access_token.token) } + let(:snowplow_gitlab_standard_context) { { project: project, namespace: project.namespace } } it_behaves_like 'PyPI package versions', :developer, :success end diff --git a/spec/workers/ssh_keys/expired_notification_worker_spec.rb b/spec/workers/ssh_keys/expired_notification_worker_spec.rb index 249ee404870..0fc999a81d6 100644 --- a/spec/workers/ssh_keys/expired_notification_worker_spec.rb +++ b/spec/workers/ssh_keys/expired_notification_worker_spec.rb @@ -50,8 +50,18 @@ RSpec.describe SshKeys::ExpiredNotificationWorker, type: :worker do context 'when key has expired in the past' do let_it_be(:expired_past) { create(:key, expires_at: 1.day.ago, user: user) } - it 'does not update notified column' do - expect { worker.perform }.not_to change { expired_past.reload.expiry_notification_delivered_at } + it 'does update notified column' do + expect { worker.perform }.to change { expired_past.reload.expiry_notification_delivered_at } + end + + context 'when key has already been notified of expiration' do + before do + expired_past.update!(expiry_notification_delivered_at: 1.day.ago) + end + + it 'does not update notified column' do + expect { worker.perform }.not_to change { expired_past.reload.expiry_notification_delivered_at } + end end end end |