diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-06-17 12:09:53 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-06-17 12:09:53 +0300 |
commit | 612bb6f624ea7fdf5fd20e3332d543191603db88 (patch) | |
tree | 0540ef31b097382b8fd974cc4c41d1938d8caae4 | |
parent | af4364040394d0261c8b1c5f78ca60cc1e68e43c (diff) |
Add latest changes from gitlab-org/gitlab@master
32 files changed, 972 insertions, 264 deletions
diff --git a/.gitlab/ci/rails.gitlab-ci.yml b/.gitlab/ci/rails.gitlab-ci.yml index b74c8ebf575..929f129e9ab 100644 --- a/.gitlab/ci/rails.gitlab-ci.yml +++ b/.gitlab/ci/rails.gitlab-ci.yml @@ -47,7 +47,6 @@ - rspec_profiling/ - tmp/capybara/ - tmp/memory_test/ - - tmp/feature_flags/ - log/*.log reports: junit: junit_rspec.xml @@ -149,13 +148,16 @@ setup-test-env: - .rails-job-base - .setup-test-env-cache - .rails:rules:code-backstage-qa - - .use-pg12 stage: prepare variables: GITLAB_TEST_EAGER_LOAD: "0" + SETUP_DB: "false" script: - - run_timed_command "bundle exec ruby -I. -e 'require \"config/environment\"; TestEnv.init'" + - run_timed_command "scripts/setup-test-env" + - echo -e "\e[0Ksection_start:`date +%s`:gitaly-test-build[collapsed=true]\r\e[0KCompiling Gitaly binaries" - run_timed_command "scripts/gitaly-test-build" # Do not use 'bundle exec' here + - echo -e "\e[0Ksection_end:`date +%s`:gitaly-test-build\r\e[0K" + artifacts: expire_in: 7d paths: @@ -237,6 +239,11 @@ static-analysis: script: - run_timed_command "retry yarn install --frozen-lockfile" - scripts/static-analysis + artifacts: + expire_in: 31d + when: always + paths: + - tmp/feature_flags/ static-analysis as-if-foss: extends: @@ -487,23 +494,7 @@ rspec:feature-flags: - .coverage-base - .rails:rules:rspec-feature-flags stage: post-test - # We cannot use needs since it would mean needing 84 jobs (since most are parallelized) - # so we use `dependencies` here. - dependencies: - - setup-test-env - - rspec migration pg12 - - rspec unit pg12 - - rspec integration pg12 - - rspec system pg12 - - rspec-ee migration pg12 - - rspec-ee unit pg12 - - rspec-ee integration pg12 - - rspec-ee system pg12 - - rspec-ee unit pg12 geo - - rspec-ee integration pg12 geo - - rspec-ee system pg12 geo - - memory-static - - memory-on-boot + needs: ["static-analysis"] script: - !reference [.minimal-bundle-install, script] - if [ "$CI_COMMIT_BRANCH" == "$CI_DEFAULT_BRANCH" ]; then diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index 8dd97c1fe69..e5b278f20b3 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -932,14 +932,6 @@ - <<: *if-merge-request-title-run-all-rspec when: always -.rails:rules:rspec-feature-flags: - rules: - - <<: *if-not-ee - when: never - - <<: *if-default-branch-schedule-2-hourly - allow_failure: true - - <<: *if-merge-request-title-run-all-rspec - .rails:rules:default-branch-schedule-nightly--code-backstage: rules: - <<: *if-default-branch-schedule-nightly @@ -954,6 +946,12 @@ - <<: *if-merge-request changes: [".gitlab/ci/rails.gitlab-ci.yml"] +.rails:rules:rspec-feature-flags: + rules: + - <<: *if-not-ee + when: never + - changes: *code-backstage-patterns + ######################### # Static analysis rules # ######################### diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 295213bd38c..ea473f9b1a2 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -50,10 +50,9 @@ class Projects::IssuesController < Projects::ApplicationController end before_action only: :show do - real_time_feature_flag = :real_time_issue_sidebar - real_time_enabled = Gitlab::ActionCable::Config.in_app? || Feature.enabled?(real_time_feature_flag, @project) + real_time_enabled = Gitlab::ActionCable::Config.in_app? || Feature.enabled?(:real_time_issue_sidebar, @project) - push_to_gon_attributes(:features, real_time_feature_flag, real_time_enabled) + push_to_gon_attributes(:features, :real_time_issue_sidebar, real_time_enabled) push_frontend_feature_flag(:confidential_notes, @project, default_enabled: :yaml) push_frontend_feature_flag(:issue_assignees_widget, @project, default_enabled: :yaml) push_frontend_feature_flag(:labels_widget, @project, default_enabled: :yaml) diff --git a/app/views/layouts/header/_registration_enabled_callout.html.haml b/app/views/layouts/header/_registration_enabled_callout.html.haml index 9266702e44e..25a7f7ba9d7 100644 --- a/app/views/layouts/header/_registration_enabled_callout.html.haml +++ b/app/views/layouts/header/_registration_enabled_callout.html.haml @@ -7,8 +7,8 @@ alert_data: { feature_id: UserCalloutsHelper::REGISTRATION_ENABLED_CALLOUT, dismiss_endpoint: user_callouts_path }, close_button_data: { testid: 'close-registration-enabled-callout' } do .gl-alert-body - = html_escape(_('%{anchorOpen}Learn more%{anchorClose} about how you can customize / disable registration on your instance.')) % { anchorOpen: "<a href=\"#{help_page_path('user/admin_area/settings/sign_up_restrictions')}\">".html_safe, anchorClose: '</a>'.html_safe } + = html_escape(_('%{anchorOpen}Learn more%{anchorClose} about how you can customize / disable registration on your instance.')) % { anchorOpen: "<a href=\"#{help_page_path('user/admin_area/settings/sign_up_restrictions')}\" class=\"gl-link\">".html_safe, anchorClose: '</a>'.html_safe } .gl-alert-actions - = link_to general_admin_application_settings_path(anchor: 'js-signup-settings'), class: 'btn gl-alert-action btn-info btn-md gl-button' do + = link_to general_admin_application_settings_path(anchor: 'js-signup-settings'), class: 'btn gl-alert-action btn-confirm btn-md gl-button' do %span.gl-button-text = _('View setting') diff --git a/app/views/shared/_auto_devops_implicitly_enabled_banner.html.haml b/app/views/shared/_auto_devops_implicitly_enabled_banner.html.haml index f788bf53a4c..35a3835a522 100644 --- a/app/views/shared/_auto_devops_implicitly_enabled_banner.html.haml +++ b/app/views/shared/_auto_devops_implicitly_enabled_banner.html.haml @@ -10,5 +10,5 @@ %div = _('Container registry is not enabled on this GitLab instance. Ask an administrator to enable it in order for Auto DevOps to work.') .gl-alert-actions - = link_to _('Settings'), project_settings_ci_cd_path(project), class: 'alert-link btn gl-button btn-info' - = link_to _('More information'), help_page_path('topics/autodevops/index.md'), target: '_blank', class: 'alert-link btn gl-button btn-default gl-ml-2' + = link_to _('Settings'), project_settings_ci_cd_path(project), class: 'alert-link btn gl-button btn-confirm' + = link_to _('More information'), help_page_path('topics/autodevops/index.md'), target: '_blank', class: 'alert-link btn gl-button btn-default gl-ml-3' diff --git a/app/views/shared/_global_alert.html.haml b/app/views/shared/_global_alert.html.haml index bebc72fe428..ea83f5c1656 100644 --- a/app/views/shared/_global_alert.html.haml +++ b/app/views/shared/_global_alert.html.haml @@ -2,19 +2,23 @@ - title = local_assigns.fetch(:title, nil) - variant = local_assigns.fetch(:variant, :info) +- dismissible = local_assigns.fetch(:dismissible, true) - alert_class = local_assigns.fetch(:alert_class, nil) - alert_data = local_assigns.fetch(:alert_data, nil) - close_button_class = local_assigns.fetch(:close_button_class, nil) - close_button_data = local_assigns.fetch(:close_button_data, nil) - icon = icons[variant] +- alert_root_class = 'gl-alert-layout-limited' if fluid_layout +- alert_container_class = [container_class, @content_class] unless fluid_layout || local_assigns.fetch(:is_contained, false) -%div{ role: 'alert', class: ["gl-alert-#{variant}", alert_class], data: alert_data } - %div{ class: [container_class, @content_class, 'gl-px-0!'] } - .gl-alert - = sprite_icon(icon, size: 16, css_class: "gl-alert-icon#{' gl-alert-icon-no-title' if title.nil?}") - %button.gl-alert-dismiss.js-close{ type: 'button', aria: { label: _('Close') }, class: close_button_class, data: close_button_data } +%div{ role: 'alert', class: [alert_root_class, 'gl-alert-max-content', 'gl-alert', "gl-alert-#{variant}", alert_class], data: alert_data } + .gl-alert-container{ class: alert_container_class } + = sprite_icon(icon, size: 16, css_class: "gl-alert-icon#{' gl-alert-icon-no-title' if title.nil?}") + - if dismissible + %button.btn.gl-dismiss-btn.btn-default.btn-sm.gl-button.btn-default-tertiary.btn-icon.js-close{ type: 'button', aria: { label: _('Dismiss') }, class: close_button_class, data: close_button_data } = sprite_icon('close', size: 16) + .gl-alert-content{ role: 'alert' } - if title - .gl-alert-title + %h4.gl-alert-title = title = yield diff --git a/doc/administration/pages/index.md b/doc/administration/pages/index.md index b9637f1b6f5..54af9950bb1 100644 --- a/doc/administration/pages/index.md +++ b/doc/administration/pages/index.md @@ -373,9 +373,13 @@ When adding a custom domain, users are required to prove they own it by adding a GitLab-controlled verification code to the DNS records for that domain. If your user base is private or otherwise trusted, you can disable the -verification requirement. Go to **Admin Area > Settings > Preferences** and -uncheck **Require users to prove ownership of custom domains** in the **Pages** section. -This setting is enabled by default. +verification requirement: + +1. On the top bar, select **Menu >** **{admin}** **Admin**. +1. On the left sidebar, select **Settings > Preferences**. +1. Expand **Pages**. +1. Clear the **Require users to prove ownership of custom domains** checkbox. + This setting is enabled by default. ### Let's Encrypt integration @@ -388,9 +392,11 @@ sites served under a custom domain. To enable it, you must: 1. Choose an email address on which you want to receive notifications about expiring domains. -1. Go to your instance's **Admin Area > Settings > Preferences** and expand **Pages** settings. +1. On the top bar, select **Menu >** **{admin}** **Admin**. +1. On the left sidebar, select **Settings > Preferences**. +1. Expand **Pages**. 1. Enter the email address for receiving notifications and accept Let's Encrypt's Terms of Service as shown below. -1. Click **Save changes**. +1. Select **Save changes**. ![Let's Encrypt settings](img/lets_encrypt_integration_v12_1.png) @@ -442,11 +448,12 @@ The scope to use for authentication must match the GitLab Pages OAuth applicatio pre-existing applications must modify the GitLab Pages OAuth application. Follow these steps to do this: -1. Go to your instance's **Admin Area > Settings > Applications** and expand **GitLab Pages** - settings. +1. On the top bar, select **Menu >** **{admin}** **Admin**. +1. On the left sidebar, select **Settings > Applications**. +1. Expand **GitLab Pages**. 1. Clear the `api` scope's checkbox and select the desired scope's checkbox (for example, `read_api`). -1. Click **Save changes**. +1. Select **Save changes**. #### Disabling public access to all Pages websites @@ -460,9 +467,11 @@ This can be useful to preserve information published with Pages websites to the of your instance only. To do that: -1. Go to your instance's **Admin Area > Settings > Preferences** and expand **Pages** settings. -1. Check the **Disable public access to Pages sites** checkbox. -1. Click **Save changes**. +1. On the top bar, select **Menu >** **{admin}** **Admin**. +1. On the left sidebar, select **Settings > Preferences**. +1. Expand **Pages**. +1. Select the **Disable public access to Pages sites** checkbox. +1. Select **Save changes**. WARNING: For self-managed installations, all public websites remain private until they are @@ -635,12 +644,6 @@ Follow the steps below to configure the proxy listener of GitLab Pages. 1. [Reconfigure GitLab](../restart_gitlab.md#omnibus-gitlab-reconfigure). -## Set maximum pages size - -You can configure the maximum size of the unpacked archive per project in -**Admin Area > Settings > Preferences > Pages**, in **Maximum size of pages (MB)**. -The default is 100MB. - ### Override maximum pages size per project or group **(PREMIUM SELF)** > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/16610) in GitLab 12.7. @@ -1256,9 +1259,14 @@ Upgrading to an [officially supported operating system](https://about.gitlab.com ### The requested scope is invalid, malformed, or unknown -This problem comes from the permissions of the GitLab Pages OAuth application. To fix it, go to -**Admin > Applications > GitLab Pages** and edit the application. Under **Scopes**, ensure that the -`api` scope is selected and save your changes. +This problem comes from the permissions of the GitLab Pages OAuth application. To fix it: + +1. On the top bar, select **Menu >** **{admin}** **Admin**. +1. On the left sidebar, select **Applications > GitLab Pages**. +1. Edit the application. +1. Under **Scopes**, ensure that the `api` scope is selected. +1. Save your changes. + When running a [separate Pages server](#running-gitlab-pages-on-a-separate-server), this setting needs to be configured on the main GitLab server. diff --git a/doc/administration/pages/source.md b/doc/administration/pages/source.md index f1c3b515f68..1427713b9a4 100644 --- a/doc/administration/pages/source.md +++ b/doc/administration/pages/source.md @@ -443,9 +443,14 @@ are stored. ## Set maximum Pages size -The maximum size of the unpacked archive per project can be configured in -**Admin Area > Settings > Preferences > Pages**, in **Maximum size of pages (MB)**. -The default is 100MB. +The default for the maximum size of unpacked archives per project is 100 MB. + +To change this value: + +1. On the top bar, select **Menu >** **{admin}** **Admin**. +1. On the left sidebar, select **Settings > Preferences**. +1. Expand **Pages**. +1. Update the value for **Maximum size of pages (MB)**. ## Backup diff --git a/doc/api/status_checks.md b/doc/api/status_checks.md index f4e384a2efb..75be51f5606 100644 --- a/doc/api/status_checks.md +++ b/doc/api/status_checks.md @@ -11,7 +11,7 @@ type: reference, api > - It's [deployed behind a feature flag](../user/feature_flags.md), disabled by default. > - It's disabled on GitLab.com. > - It's not recommended for production use. -> - To use it in GitLab self-managed instances, ask a GitLab administrator to [enable it](#enable-or-disable-status-checks). **(ULTIMATE SELF)** +> - To use it in GitLab self-managed instances, ask a GitLab administrator to [enable it](#enable-or-disable-external-status-checks). WARNING: This feature might not be available to you. Check the **version history** note above for details. @@ -67,13 +67,6 @@ POST /projects/:id/merge_requests/:merge_request_iid/status_check_responses NOTE: `sha` must be the SHA at the `HEAD` of the merge request's source branch. -## Enable or disable status checks **(ULTIMATE SELF)** - -Status checks are under development and not ready for production use. It is -deployed behind a feature flag that is **disabled by default**. -[GitLab administrators with access to the GitLab Rails console](../administration/feature_flags.md) -can enable it. - ## Get project external status checks **(ULTIMATE)** You can request information about a project's external status checks using the following endpoint: @@ -86,7 +79,7 @@ GET /projects/:id/external_status_checks | Attribute | Type | Required | Description | |---------------------|---------|----------|---------------------| -| `id` | integer | yes | The ID of a project | +| `id` | integer | yes | ID of a project | ```json [ @@ -109,7 +102,7 @@ GET /projects/:id/external_status_checks ] ``` -### Create external status check **(ULTIMATE)** +## Create external status check **(ULTIMATE)** You can create a new external status check for a project using the following endpoint: @@ -117,14 +110,14 @@ You can create a new external status check for a project using the following end POST /projects/:id/external_status_checks ``` -| Attribute | Type | Required | Description | -|------------------------|----------------|----------|----------------------------------------------------| -| `id` | integer | yes | The ID of a project | -| `name` | string | yes | Display name of status check | -| `external_url` | string | yes | URL of status check resource | -| `protected_branch_ids` | `array<Integer>` | no | The ids of protected branches to scope the rule by | +| Attribute | Type | Required | Description | +|------------------------|------------------|----------|------------------------------------------------| +| `id` | integer | yes | ID of a project | +| `name` | string | yes | Display name of status check | +| `external_url` | string | yes | URL of status check resource | +| `protected_branch_ids` | `array<Integer>` | no | IDs of protected branches to scope the rule by | -### Delete external status check **(ULTIMATE)** +## Delete external status check **(ULTIMATE)** You can delete an external status check for a project using the following endpoint: @@ -132,12 +125,12 @@ You can delete an external status check for a project using the following endpoi DELETE /projects/:id/external_status_checks/:check_id ``` -| Attribute | Type | Required | Description | -|------------------------|----------------|----------|----------------------------------------------------| -| `rule_id` | integer | yes | The ID of an status check | -| `id` | integer | yes | The ID of a project | +| Attribute | Type | Required | Description | +|------------------------|----------------|----------|-----------------------| +| `rule_id` | integer | yes | ID of an status check | +| `id` | integer | yes | ID of a project | -### Update external status check **(ULTIMATE)** +## Update external status check **(ULTIMATE)** You can update an existing external status check for a project using the following endpoint: @@ -145,18 +138,22 @@ You can update an existing external status check for a project using the followi PUT /projects/:id/external_status_checks/:check_id ``` -| Attribute | Type | Required | Description | -|------------------------|----------------|----------|----------------------------------------------------| -| `id` | integer | yes | The ID of a project | -| `rule_id` | integer | yes | The ID of an external status check | -| `name` | string | no | Display name of status check | -| `external_url` | string | no | URL of external status check resource | -| `protected_branch_ids` | `array<Integer>` | no | The ids of protected branches to scope the rule by | +| Attribute | Type | Required | Description | +|------------------------|------------------|----------|------------------------------------------------| +| `id` | integer | yes | ID of a project | +| `rule_id` | integer | yes | ID of an external status check | +| `name` | string | no | Display name of status check | +| `external_url` | string | no | URL of external status check resource | +| `protected_branch_ids` | `array<Integer>` | no | IDs of protected branches to scope the rule by | -### Enable or disable External Project-level MR status checks **(ULTIMATE SELF)** +## Enable or disable external status checks **(ULTIMATE SELF)** -Enable or disable External Project-level MR status checks is under development and not ready for production use. It is -deployed behind a feature flag that is **disabled by default**. +External status checks are: + +- Under development. +- Not ready for production use. + +The feature is deployed behind a feature flag that is **disabled by default**. [GitLab administrators with access to the GitLab Rails console](../user/feature_flags.md) can enable it. @@ -178,24 +175,6 @@ Feature.disable(:ff_compliance_approval_gates) Feature.disable(:ff_compliance_approval_gates, Project.find(<project id>)) ``` -To enable it: - -```ruby -# For the instance -Feature.enable(:ff_compliance_approval_gates) -# For a single project -Feature.enable(:ff_compliance_approval_gates, Project.find(<project id>)) -``` - -To disable it: - -```ruby -# For the instance -Feature.disable(:ff_compliance_approval_gates) -# For a single project -Feature.disable(:ff_compliance_approval_gates, Project.find(<project id>) -``` - ## Related links -- [External status checks](../user/project/merge_requests/status_checks.md) +- [External status checks](../user/project/merge_requests/status_checks.md). diff --git a/doc/ci/caching/index.md b/doc/ci/caching/index.md index 0778d598d32..3c9a796a9f2 100644 --- a/doc/ci/caching/index.md +++ b/doc/ci/caching/index.md @@ -19,62 +19,60 @@ Use cache for dependencies, like packages you download from the internet. Cache is stored where GitLab Runner is installed and uploaded to S3 if [distributed cache is enabled](https://docs.gitlab.com/runner/configuration/autoscale.html#distributed-runners-caching). -- You can define it per job by using the `cache:` keyword. Otherwise it is disabled. -- You can define it per job so that: - - Subsequent pipelines can use it. - - Subsequent jobs in the same pipeline can use it, if the dependencies are identical. -- You cannot share it between projects. - Use artifacts to pass intermediate build results between stages. Artifacts are generated by a job, stored in GitLab, and can be downloaded. -- You can define artifacts per job. Subsequent jobs in later stages of the same - pipeline can use them. -- You can't use the artifacts in a different pipeline. +Both artifacts and caches define their paths relative to the project directory, and +can't link to files outside it. + +### Cache + +- Define cache per job by using the `cache:` keyword. Otherwise it is disabled. +- Subsequent pipelines can use the cache. +- Subsequent jobs in the same pipeline can use the cache, if the dependencies are identical. +- Different projects cannot share the cache. + +### Artifacts + +- Define artifacts per job. +- Subsequent jobs in later stages of the same pipeline can use artifacts. +- Different projects cannot share artifacts. Artifacts expire after 30 days unless you define an [expiration time](../yaml/README.md#artifactsexpire_in). Use [dependencies](../yaml/README.md#dependencies) to control which jobs fetch the artifacts. -Both artifacts and caches define their paths relative to the project directory, and -can't link to files outside it. - ## Good caching practices -To ensure maximum availability of the cache, when you declare `cache` in your jobs, -use one or more of the following: +To ensure maximum availability of the cache, do one or more of the following: - [Tag your runners](../runners/configure_runners.md#use-tags-to-limit-the-number-of-jobs-using-the-runner) and use the tag on jobs - that share their cache. + that share the cache. - [Use runners that are only available to a particular project](../runners/runners_scope.md#prevent-a-specific-runner-from-being-enabled-for-other-projects). -- [Use a `key`](../yaml/README.md#cachekey) that fits your workflow (for example, - different caches on each branch). For that, you can take advantage of the - [predefined CI/CD variables](../variables/README.md#predefined-cicd-variables). +- [Use a `key`](../yaml/README.md#cachekey) that fits your workflow. For example, + you can configure a different cache for each branch. For runners to work with caches efficiently, you must do one of the following: - Use a single runner for all your jobs. -- Use multiple runners (in autoscale mode or not) that use +- Use multiple runners that have [distributed caching](https://docs.gitlab.com/runner/configuration/autoscale.html#distributed-runners-caching), - where the cache is stored in S3 buckets (like shared runners on GitLab.com). -- Use multiple runners (not in autoscale mode) of the same architecture that - share a common network-mounted directory (using NFS or something similar) - where the cache is stored. - -Read about the [availability of the cache](#availability-of-the-cache) -to learn more about the internals and get a better idea how cache works. + where the cache is stored in S3 buckets. Shared runners on GitLab.com behave this way. These runners can be in autoscale mode, + but they don't have to be. +- Use multiple runners with the same architecture and have these runners + share a common network-mounted directory to store the cache. This directory should use NFS or something similar. + These runners must be in autoscale mode. ### Share caches between jobs in the same branch -Define a cache with the `key: ${CI_COMMIT_REF_SLUG}` so that jobs of each -branch always use the same cache: +To have jobs for each branch use the same cache, define a cache with the `key: ${CI_COMMIT_REF_SLUG}`: ```yaml cache: key: ${CI_COMMIT_REF_SLUG} ``` -This configuration is safe from accidentally overwriting the cache, but merge requests -get slow first pipelines. The next time a new commit is pushed to the branch, the +This configuration prevents you from accidentally overwriting the cache. However, the +first pipeline for a merge request is slow. The next time a commit is pushed to the branch, the cache is re-used and jobs run faster. To enable per-job and per-branch caching: diff --git a/doc/user/project/deploy_keys/index.md b/doc/user/project/deploy_keys/index.md index c0bc97781b6..48959c0ba81 100644 --- a/doc/user/project/deploy_keys/index.md +++ b/doc/user/project/deploy_keys/index.md @@ -121,8 +121,9 @@ repositories to secure, shared services, such as CI/CD. Instance administrators can add public deploy keys: -1. Go to **Admin Area > Deploy Keys**. -1. Click on **New deploy key**. +1. On the top bar, select **Menu >** **{admin}** **Admin**. +1. On the left sidebar, select **Deploy Keys**. +1. Select **New deploy key**. Make sure your new key has a meaningful title, as it is the primary way for project maintainers and owners to identify the correct public deploy key to add. For example, diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index f4a89edecd1..2c26da037da 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -120,7 +120,7 @@ module Gitlab raise "storage #{storage.inspect} is missing a gitaly_address" end - unless URI(address).scheme.in?(%w(tcp unix tls)) + unless %w(tcp unix tls).include?(URI(address).scheme) raise "Unsupported Gitaly address: #{address.inspect} does not use URL scheme 'tcp' or 'unix' or 'tls'" end diff --git a/lib/gitlab/gitaly_client/storage_settings.rb b/lib/gitlab/gitaly_client/storage_settings.rb index f66dc3010ea..4cc0269673f 100644 --- a/lib/gitlab/gitaly_client/storage_settings.rb +++ b/lib/gitlab/gitaly_client/storage_settings.rb @@ -52,7 +52,7 @@ module Gitlab @legacy_disk_path = File.expand_path(storage['path'], Rails.root) if storage['path'] storage['path'] = Deprecated - @hash = storage.with_indifferent_access + @hash = ActiveSupport::HashWithIndifferentAccess.new(storage) end def gitaly_address diff --git a/lib/tasks/gitlab/helpers.rake b/lib/tasks/gitlab/helpers.rake index b61b1833c5a..b467aa3819d 100644 --- a/lib/tasks/gitlab/helpers.rake +++ b/lib/tasks/gitlab/helpers.rake @@ -3,6 +3,8 @@ # Prevent StateMachine warnings from outputting during a cron task StateMachines::Machine.ignore_method_conflicts = true if ENV['CRON'] -task gitlab_environment: :environment do +task :gitlab_environment do + Rake::Task[:environment].invoke unless ENV['SKIP_RAILS_ENV_IN_RAKE'] + extend SystemCheck::Helpers end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 28ca39dbdf9..5de25aa173b 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -21752,10 +21752,7 @@ msgstr "" msgid "NetworkPolicies|Namespace" msgstr "" -msgid "NetworkPolicies|Network Policy" -msgstr "" - -msgid "NetworkPolicies|Network policy" +msgid "NetworkPolicies|Network" msgstr "" msgid "NetworkPolicies|Network traffic" diff --git a/package.json b/package.json index 0a889f6f6a0..633cdeabadc 100644 --- a/package.json +++ b/package.json @@ -56,7 +56,7 @@ "@gitlab/favicon-overlay": "2.0.0", "@gitlab/svgs": "1.199.0", "@gitlab/tributejs": "1.0.0", - "@gitlab/ui": "29.35.0", + "@gitlab/ui": "29.36.0", "@gitlab/visual-review-tools": "1.6.1", "@rails/actioncable": "6.1.3-2", "@rails/ujs": "6.1.3-2", diff --git a/rubocop/code_reuse_helpers.rb b/rubocop/code_reuse_helpers.rb index 63019c43943..283c43de227 100644 --- a/rubocop/code_reuse_helpers.rb +++ b/rubocop/code_reuse_helpers.rb @@ -32,59 +32,79 @@ module RuboCop # Returns true if the given node resides in app/finders or ee/app/finders. def in_finder?(node) - in_directory?(node, 'finders') + in_app_directory?(node, 'finders') end # Returns true if the given node resides in app/models or ee/app/models. def in_model?(node) - in_directory?(node, 'models') + in_app_directory?(node, 'models') end # Returns true if the given node resides in app/services or ee/app/services. def in_service_class?(node) - in_directory?(node, 'services') + in_app_directory?(node, 'services') end # Returns true if the given node resides in app/presenters or # ee/app/presenters. def in_presenter?(node) - in_directory?(node, 'presenters') + in_app_directory?(node, 'presenters') end # Returns true if the given node resides in app/serializers or # ee/app/serializers. def in_serializer?(node) - in_directory?(node, 'serializers') + in_app_directory?(node, 'serializers') end # Returns true if the given node resides in app/workers or ee/app/workers. def in_worker?(node) - in_directory?(node, 'workers') + in_app_directory?(node, 'workers') end # Returns true if the given node resides in app/controllers or # ee/app/controllers. def in_controller?(node) - in_directory?(node, 'controllers') + in_app_directory?(node, 'controllers') + end + + # Returns true if the given node resides in app/graphql/types, + # ee/app/graphql/types, or ee/app/graphql/ee/types. + def in_graphql_types?(node) + in_app_directory?(node, 'graphql/types') || in_app_directory?(node, 'graphql/ee/types') end # Returns true if the given node resides in lib/api or ee/lib/api. def in_api?(node) + in_lib_directory?(node, 'api') + end + + # Returns true if the given node resides in spec or ee/spec. + def in_spec?(node) file_path_for_node(node).start_with?( - File.join(ce_lib_directory, 'api'), - File.join(ee_lib_directory, 'api') + ce_spec_directory, + ee_spec_directory ) end # Returns `true` if the given AST node resides in the given directory, # relative to app and/or ee/app. - def in_directory?(node, directory) + def in_app_directory?(node, directory) file_path_for_node(node).start_with?( File.join(ce_app_directory, directory), File.join(ee_app_directory, directory) ) end + # Returns `true` if the given AST node resides in the given directory, + # relative to lib and/or ee/lib. + def in_lib_directory?(node, directory) + file_path_for_node(node).start_with?( + File.join(ce_lib_directory, directory), + File.join(ee_lib_directory, directory) + ) + end + # Returns the receiver name of a send node. # # For the AST node `(send (const nil? :Foo) ...)` this would return @@ -149,6 +169,14 @@ module RuboCop File.join(rails_root, 'ee', 'lib') end + def ce_spec_directory + File.join(rails_root, 'spec') + end + + def ee_spec_directory + File.join(rails_root, 'ee', 'spec') + end + def rails_root File.expand_path('..', __dir__) end diff --git a/rubocop/cop/gitlab/mark_used_feature_flags.rb b/rubocop/cop/gitlab/mark_used_feature_flags.rb new file mode 100644 index 00000000000..2a020d6efb2 --- /dev/null +++ b/rubocop/cop/gitlab/mark_used_feature_flags.rb @@ -0,0 +1,265 @@ +# frozen_string_literal: true + +require_relative '../../code_reuse_helpers' + +module RuboCop + module Cop + module Gitlab + # This cop tracks the usage of feature flags among the codebase. + # + # The files set in `tmp/feature_flags/*.used` can then be used for verification purpose. + # + class MarkUsedFeatureFlags < RuboCop::Cop::Cop + include RuboCop::CodeReuseHelpers + + FEATURE_METHODS = %i[enabled? disabled?].freeze + EXPERIMENTATION_METHODS = %i[active?].freeze + EXPERIMENT_METHODS = %i[ + experiment + experiment_enabled? + push_frontend_experiment + ].freeze + RUGGED_METHODS = %i[ + use_rugged? + ].freeze + WORKER_METHODS = %i[ + data_consistency + deduplicate + ].freeze + GRAPHQL_METHODS = %i[ + field + ].freeze + SELF_METHODS = %i[ + push_frontend_feature_flag + limit_feature_flag= + ].freeze + EXPERIMENT_METHODS + RUGGED_METHODS + WORKER_METHODS + + RESTRICT_ON_SEND = FEATURE_METHODS + EXPERIMENTATION_METHODS + GRAPHQL_METHODS + SELF_METHODS + + USAGE_DATA_COUNTERS_EVENTS_YAML_GLOBS = [ + File.expand_path("../../../config/metrics/aggregates/*.yml", __dir__), + File.expand_path("../../../lib/gitlab/usage_data_counters/known_events/*.yml", __dir__) + ].freeze + + DYNAMIC_FEATURE_FLAGS = [ + :usage_data_static_site_editor_commits, # https://gitlab.com/gitlab-org/gitlab/-/issues/284082 + :usage_data_static_site_editor_merge_requests # https://gitlab.com/gitlab-org/gitlab/-/issues/284083 + ].freeze + + # Called before all on_... have been called + # When refining this method, always call `super` + def on_new_investigation + super + track_dynamic_feature_flags! + track_usage_data_counters_known_events! + end + + def on_casgn(node) + _, lhs_name, rhs = *node + + save_used_feature_flag(rhs.value) if lhs_name == :FEATURE_FLAG + end + + def on_send(node) + return if in_spec?(node) + return unless trackable_flag?(node) + + flag_arg = flag_arg(node) + flag_value = flag_value(node) + return unless flag_value + + if flag_arg_is_str_or_sym?(node) + if caller_is_feature_gitaly?(node) + save_used_feature_flag("gitaly_#{flag_value}") + else + save_used_feature_flag(flag_value) + end + + if experiment_method?(node) || experimentation_method?(node) + # Additionally, mark experiment-related feature flag as used as well + matching_feature_flags = defined_feature_flags.select { |flag| flag == "#{flag_value}_experiment_percentage" } + matching_feature_flags.each do |matching_feature_flag| + puts_if_ci(node, "The '#{matching_feature_flag}' feature flag tracks the #{flag_value} experiment, which is still in use, so we'll mark it as used.") + save_used_feature_flag(matching_feature_flag) + end + end + elsif flag_arg_is_send_type?(node) + puts_if_ci(node, "Feature flag is dynamic: '#{flag_value}.") + elsif flag_arg_is_dstr_or_dsym?(node) + str_prefix = flag_arg.children[0] + rest_children = flag_arg.children[1..] + + if rest_children.none? { |child| child.str_type? } + matching_feature_flags = defined_feature_flags.select { |flag| flag.start_with?(str_prefix.value) } + matching_feature_flags.each do |matching_feature_flag| + puts_if_ci(node, "The '#{matching_feature_flag}' feature flag starts with '#{str_prefix.value}', so we'll optimistically mark it as used.") + save_used_feature_flag(matching_feature_flag) + end + else + puts_if_ci(node, "Interpolated feature flag name has multiple static string parts, we won't track it.") + end + else + puts_if_ci(node, "Feature flag has an unknown type: #{flag_arg.type}.") + end + end + + private + + def puts_if_ci(node, text) + puts "#{text} (call: `#{node.source}`, source: #{node.location.expression.source_buffer.name})" if ENV['CI'] + end + + def save_used_feature_flag(feature_flag_name) + used_feature_flag_file = File.expand_path("../../../tmp/feature_flags/#{feature_flag_name}.used", __dir__) + return if File.exist?(used_feature_flag_file) + + FileUtils.touch(used_feature_flag_file) + end + + def class_caller(node) + node.children[0]&.const_name.to_s + end + + def method_name(node) + node.children[1] + end + + def flag_arg(node) + if worker_method?(node) + return unless node.children.size > 3 + + node.children[3].each_pair.find do |pair| + pair.key.value == :feature_flag + end&.value + elsif graphql_method?(node) + return unless node.children.size > 3 + + opts_index = node.children[3].hash_type? ? 3 : 4 + return unless node.children[opts_index] + + node.children[opts_index].each_pair.find do |pair| + pair.key.value == :feature_flag + end&.value + else + arg_index = rugged_method?(node) ? 3 : 2 + + node.children[arg_index] + end + end + + def flag_value(node) + flag_arg = flag_arg(node) + return unless flag_arg + + if flag_arg.respond_to?(:value) + flag_arg.value + else + flag_arg + end.to_s.tr("\n/", ' _') + end + + def flag_arg_is_str_or_sym?(node) + flag_arg = flag_arg(node) + flag_arg.str_type? || flag_arg.sym_type? + end + + def flag_arg_is_send_type?(node) + flag_arg(node).send_type? + end + + def flag_arg_is_dstr_or_dsym?(node) + flag = flag_arg(node) + (flag.dstr_type? || flag.dsym_type?) && flag.children[0].str_type? + end + + def caller_is_feature?(node) + class_caller(node) == "Feature" + end + + def caller_is_feature_gitaly?(node) + class_caller(node) == "Feature::Gitaly" + end + + def caller_is_experimentation?(node) + class_caller(node) == "Gitlab::Experimentation" + end + + def experiment_method?(node) + EXPERIMENT_METHODS.include?(method_name(node)) + end + + def rugged_method?(node) + RUGGED_METHODS.include?(method_name(node)) + end + + def feature_method?(node) + FEATURE_METHODS.include?(method_name(node)) && (caller_is_feature?(node) || caller_is_feature_gitaly?(node)) + end + + def experimentation_method?(node) + EXPERIMENTATION_METHODS.include?(method_name(node)) && caller_is_experimentation?(node) + end + + def worker_method?(node) + WORKER_METHODS.include?(method_name(node)) + end + + def graphql_method?(node) + GRAPHQL_METHODS.include?(method_name(node)) && in_graphql_types?(node) + end + + def self_method?(node) + SELF_METHODS.include?(method_name(node)) && class_caller(node).empty? + end + + def trackable_flag?(node) + feature_method?(node) || experimentation_method?(node) || graphql_method?(node) || self_method?(node) + end + + # Marking all event's feature flags as used as Gitlab::UsageDataCounters::HLLRedisCounter.track_event{,context} + # is mostly used with dynamic event name. + def track_dynamic_feature_flags! + DYNAMIC_FEATURE_FLAGS.each(&method(:save_used_feature_flag)) + end + + # Marking all event's feature flags as used as Gitlab::UsageDataCounters::HLLRedisCounter.track_event{,context} + # is mostly used with dynamic event name. + def track_usage_data_counters_known_events! + usage_data_counters_known_event_feature_flags.each(&method(:save_used_feature_flag)) + end + + def usage_data_counters_known_event_feature_flags + USAGE_DATA_COUNTERS_EVENTS_YAML_GLOBS.each_with_object(Set.new) do |glob, memo| + Dir.glob(glob).each do |path| + YAML.safe_load(File.read(path))&.each do |hash| + memo << hash['feature_flag'] if hash['feature_flag'] + end + end + end + end + + def defined_feature_flags + @defined_feature_flags ||= begin + flags_paths = [ + 'config/feature_flags/**/*.yml' + ] + + # For EE additionally process `ee/` feature flags + if File.exist?(File.expand_path('../../../ee/app/models/license.rb', __dir__)) && !%w[true 1].include?(ENV['FOSS_ONLY'].to_s) + flags_paths << 'ee/config/feature_flags/**/*.yml' + end + + flags_paths.each_with_object([]) do |flags_path, memo| + flags_path = File.expand_path("../../../#{flags_path}", __dir__) + Dir.glob(flags_path).each do |path| + feature_flag_name = File.basename(path, '.yml') + + memo << feature_flag_name + end + end + end + end + end + end + end +end diff --git a/scripts/setup-test-env b/scripts/setup-test-env new file mode 100755 index 00000000000..ebd3a48ae15 --- /dev/null +++ b/scripts/setup-test-env @@ -0,0 +1,68 @@ +#!/usr/bin/env ruby + +# frozen_string_literal: true + +require 'bundler/setup' + +require 'request_store' +require 'rake' +require 'active_support/dependencies' +require 'active_support/dependencies/autoload' +require 'active_support/core_ext/numeric' +require 'active_support/string_inquirer' + +ENV['SKIP_RAILS_ENV_IN_RAKE'] = 'true' + +module Rails + extend self + + def root + Pathname.new(File.expand_path('..', __dir__)) + end + + def env + @_env ||= ActiveSupport::StringInquirer.new(ENV["RAILS_ENV"] || ENV["RACK_ENV"] || "test") + end +end + +ActiveSupport::Dependencies.autoload_paths << 'lib' + +load File.expand_path('../lib/tasks/gitlab/helpers.rake', __dir__) +load File.expand_path('../lib/tasks/gitlab/gitaly.rake', __dir__) + +# Required for config/0_inject_enterprise_edition_module.rb, lib/gitlab/access.rb +require_dependency File.expand_path('../lib/gitlab', __dir__) + +require_dependency File.expand_path('../config/initializers/0_inject_enterprise_edition_module', __dir__) + +# Require for lib/gitlab/gitaly_client/storage_settings.rb and config/initializers/1_settings.rb +require 'active_support/hash_with_indifferent_access' + +# Required for lib/gitlab/visibility_level.rb and lib/gitlab/safe_request_store.rb +require 'active_support/concern' +require 'active_support/core_ext/module/delegation' + +# Required for lib/system_check/helpers.rb +require_dependency File.expand_path('../lib/gitlab/task_helpers', __dir__) + +# Required for lib/tasks/gitlab/helpers.rake +require_dependency File.expand_path('../lib/system_check/helpers', __dir__) + +# Required for config/initializers/1_settings.rb +require 'omniauth' +require 'omniauth-github' +require 'etc' +require_dependency File.expand_path('../lib/gitlab/access', __dir__) + +require_dependency File.expand_path('../config/initializers/1_settings', __dir__) + +Gitlab.ee do + load File.expand_path('../ee/lib/tasks/gitlab/indexer.rake', __dir__) + + require_dependency File.expand_path('../ee/lib/gitlab/elastic/indexer', __dir__) + require_dependency File.expand_path('../lib/gitlab/utils/override', __dir__) +end + +require_dependency File.expand_path('../spec/support/helpers/test_env', __dir__) + +TestEnv.init diff --git a/scripts/static-analysis b/scripts/static-analysis index 7aa2fbf1594..fc917f1b975 100755 --- a/scripts/static-analysis +++ b/scripts/static-analysis @@ -24,7 +24,10 @@ class StaticAnalysis (Gitlab.ee? ? %w[bin/rake gettext:updated_check] : nil) => 410, # Most of the time, RuboCop finishes in 30 seconds, but sometimes it can take around 1200 seconds so we set a # duration of 300 to lower the likelihood that it will run in the same job as another long task... - %w[bundle exec rubocop --parallel] => 300, + %w[bundle exec rubocop --parallel --except Gitlab/MarkUsedFeatureFlags] => 300, + # We need to disable the cache for this cop since it creates files under tmp/feature_flags/*.used, + # the cache would prevent these files from being created. + %w[bundle exec rubocop --only Gitlab/MarkUsedFeatureFlags --cache false] => 600, %w[yarn run lint:eslint:all] => 264, %w[yarn run lint:prettier] => 134, %w[bin/rake gettext:lint] => 81, diff --git a/scripts/used-feature-flags b/scripts/used-feature-flags index aebd007dda9..07c022a4c1a 100755 --- a/scripts/used-feature-flags +++ b/scripts/used-feature-flags @@ -28,6 +28,16 @@ flags_paths = [ # For EE additionally process `ee/` feature flags if File.exist?('ee/app/models/license.rb') && !%w[true 1].include?(ENV['FOSS_ONLY'].to_s) flags_paths << 'ee/config/feature_flags/**/*.yml' + + # Geo feature flags are constructed dynamically and there's no explicit checks in the codebase so we mark all + # the replicators' derived feature flags as used. + # See https://gitlab.com/gitlab-org/gitlab/-/blob/54e802e8fe76b6f93656d75ef9b566bf57b60f41/ee/lib/gitlab/geo/replicator.rb#L183-185 + Dir.glob('ee/app/replicators/geo/*_replicator.rb').each_with_object(Set.new) do |path, memo| + replicator_name = File.basename(path, '.rb') + feature_flag_name = "geo_#{replicator_name.delete_suffix('_replicator')}_replication" + + FileUtils.touch(File.join('tmp', 'feature_flags', "#{feature_flag_name}.used")) + end end all_flags = {} @@ -41,7 +51,17 @@ flags_paths.each do |flags_path| feature_flag_name = File.basename(path, '.yml') # TODO: we need a better way of tracking use of Gitaly FF across Gitaly and GitLab - next if feature_flag_name.start_with?('gitaly_') + if feature_flag_name.start_with?('gitaly_') + puts "Skipping the #{feature_flag_name} feature flag since it starts with 'gitaly_'." + next + end + + # Dynamic feature flag names for redirect to latest CI templates + # See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/63144/diffs#fa2193ace3f6a02f7ef9995ef9bc519eca92c4ee_57_84 + if feature_flag_name.start_with?('redirect_to_latest_template_') + puts "Skipping the #{feature_flag_name} feature flag since it starts with 'redirect_to_latest_template_'." + next + end all_flags[feature_flag_name] = File.exist?(File.join('tmp', 'feature_flags', feature_flag_name + '.used')) end diff --git a/spec/experiments/application_experiment_spec.rb b/spec/experiments/application_experiment_spec.rb index 22c436e4159..2d2b911749b 100644 --- a/spec/experiments/application_experiment_spec.rb +++ b/spec/experiments/application_experiment_spec.rb @@ -19,12 +19,6 @@ RSpec.describe ApplicationExperiment, :experiment do allow(subject).to receive(:enabled?).and_return(true) end - it "naively assumes a 1x1 relationship to feature flags for tests" do - expect(Feature).to receive(:persist_used!).with('namespaced_stub') - - described_class.new('namespaced/stub') - end - it "doesn't raise an exception without a defined control" do # because we have a default behavior defined diff --git a/spec/fast_spec_helper.rb b/spec/fast_spec_helper.rb index 2f0bcd318d9..469c29cd2e0 100644 --- a/spec/fast_spec_helper.rb +++ b/spec/fast_spec_helper.rb @@ -22,3 +22,8 @@ ActiveSupport::Dependencies.autoload_paths << 'lib' ActiveSupport::Dependencies.autoload_paths << 'ee/lib' ActiveSupport::XmlMini.backend = 'Nokogiri' + +RSpec.configure do |config| + config.filter_run focus: true + config.run_all_when_everything_filtered = true +end diff --git a/spec/rubocop/code_reuse_helpers_spec.rb b/spec/rubocop/code_reuse_helpers_spec.rb index 9337df368e3..695c152e3db 100644 --- a/spec/rubocop/code_reuse_helpers_spec.rb +++ b/spec/rubocop/code_reuse_helpers_spec.rb @@ -150,6 +150,31 @@ RSpec.describe RuboCop::CodeReuseHelpers do end end + describe '#in_graphql_types?' do + %w[ + app/graphql/types + ee/app/graphql/ee/types + ee/app/graphql/types + ].each do |path| + it "returns true for a node in #{path}" do + node = build_and_parse_source('10', rails_root_join(path, 'foo.rb')) + + expect(cop.in_graphql_types?(node)).to eq(true) + end + end + + %w[ + app/graphql/resolvers + app/foo + ].each do |path| + it "returns true for a node in #{path}" do + node = build_and_parse_source('10', rails_root_join(path, 'foo.rb')) + + expect(cop.in_graphql_types?(node)).to eq(false) + end + end + end + describe '#in_api?' do it 'returns true for a node in the API directory' do node = build_and_parse_source('10', rails_root_join('lib', 'api', 'foo.rb')) @@ -164,25 +189,67 @@ RSpec.describe RuboCop::CodeReuseHelpers do end end - describe '#in_directory?' do + describe '#in_spec?' do + it 'returns true for a node in the spec directory' do + node = build_and_parse_source('10', rails_root_join('spec', 'foo.rb')) + + expect(cop.in_spec?(node)).to eq(true) + end + + it 'returns true for a node in the ee/spec directory' do + node = build_and_parse_source('10', rails_root_join('ee', 'spec', 'foo.rb')) + + expect(cop.in_spec?(node)).to eq(true) + end + + it 'returns false for a node outside the spec directory' do + node = build_and_parse_source('10', rails_root_join('lib', 'foo.rb')) + + expect(cop.in_spec?(node)).to eq(false) + end + end + + describe '#in_app_directory?' do it 'returns true for a directory in the CE app/ directory' do node = build_and_parse_source('10', rails_root_join('app', 'models', 'foo.rb')) - expect(cop.in_directory?(node, 'models')).to eq(true) + expect(cop.in_app_directory?(node, 'models')).to eq(true) end it 'returns true for a directory in the EE app/ directory' do node = build_and_parse_source('10', rails_root_join('ee', 'app', 'models', 'foo.rb')) - expect(cop.in_directory?(node, 'models')).to eq(true) + expect(cop.in_app_directory?(node, 'models')).to eq(true) end it 'returns false for a directory in the lib/ directory' do node = build_and_parse_source('10', rails_root_join('lib', 'models', 'foo.rb')) - expect(cop.in_directory?(node, 'models')).to eq(false) + expect(cop.in_app_directory?(node, 'models')).to eq(false) + end + end + + describe '#in_lib_directory?' do + it 'returns true for a directory in the CE lib/ directory' do + node = build_and_parse_source('10', rails_root_join('lib', 'models', 'foo.rb')) + + expect(cop.in_lib_directory?(node, 'models')).to eq(true) + end + + it 'returns true for a directory in the EE lib/ directory' do + node = + build_and_parse_source('10', rails_root_join('ee', 'lib', 'models', 'foo.rb')) + + expect(cop.in_lib_directory?(node, 'models')).to eq(true) + end + + it 'returns false for a directory in the app/ directory' do + node = + build_and_parse_source('10', rails_root_join('app', 'models', 'foo.rb')) + + expect(cop.in_lib_directory?(node, 'models')).to eq(false) end end diff --git a/spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb b/spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb new file mode 100644 index 00000000000..968cafc57d4 --- /dev/null +++ b/spec/rubocop/cop/gitlab/mark_used_feature_flags_spec.rb @@ -0,0 +1,233 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rubocop' +require 'rubocop/rspec/support' +require_relative '../../../../rubocop/cop/gitlab/mark_used_feature_flags' + +RSpec.describe RuboCop::Cop::Gitlab::MarkUsedFeatureFlags do + let(:defined_feature_flags) do + %w[a_feature_flag foo_hello foo_world baz_experiment_percentage bar_baz] + end + + subject(:cop) { described_class.new } + + before do + stub_const("#{described_class}::DYNAMIC_FEATURE_FLAGS", []) + allow(cop).to receive(:defined_feature_flags).and_return(defined_feature_flags) + allow(cop).to receive(:usage_data_counters_known_event_feature_flags).and_return([]) + end + + def feature_flag_path(feature_flag_name) + File.expand_path("../../../../tmp/feature_flags/#{feature_flag_name}.used", __dir__) + end + + shared_examples 'sets flag as used' do |method_call, flags_to_be_set| + it 'sets the flag as used' do + Array(flags_to_be_set).each do |flag_to_be_set| + expect(FileUtils).to receive(:touch).with(feature_flag_path(flag_to_be_set)) + end + + expect_no_offenses(<<~RUBY) + class Foo < ApplicationRecord + #{method_call} + end + RUBY + end + end + + shared_examples 'does not set any flags as used' do |method_call| + it 'sets the flag as used' do + expect(FileUtils).not_to receive(:touch) + + expect_no_offenses(method_call) + end + end + + %w[ + Feature.enabled? + Feature.disabled? + push_frontend_feature_flag + ].each do |feature_flag_method| + context "#{feature_flag_method} method" do + context 'a string feature flag' do + include_examples 'sets flag as used', %Q|#{feature_flag_method}("foo")|, 'foo' + end + + context 'a symbol feature flag' do + include_examples 'sets flag as used', %Q|#{feature_flag_method}(:foo)|, 'foo' + end + + context 'an interpolated string feature flag with a string prefix' do + include_examples 'sets flag as used', %Q|#{feature_flag_method}("foo_\#{bar}")|, %w[foo_hello foo_world] + end + + context 'an interpolated symbol feature flag with a string prefix' do + include_examples 'sets flag as used', %Q|#{feature_flag_method}(:"foo_\#{bar}")|, %w[foo_hello foo_world] + end + + context 'a string with a "/" in it' do + include_examples 'sets flag as used', %Q|#{feature_flag_method}("bar/baz")|, 'bar_baz' + end + + context 'an interpolated string feature flag with a string prefix and suffix' do + include_examples 'does not set any flags as used', %Q|#{feature_flag_method}(:"foo_\#{bar}_baz")| + end + + context 'a dynamic string feature flag as a variable' do + include_examples 'does not set any flags as used', %Q|#{feature_flag_method}(a_variable, an_arg)| + end + + context 'an integer feature flag' do + include_examples 'does not set any flags as used', %Q|#{feature_flag_method}(123)| + end + end + end + + %w[ + Feature::Gitaly.enabled? + Feature::Gitaly.disabled? + ].each do |feature_flag_method| + context "#{feature_flag_method} method" do + context 'a string feature flag' do + include_examples 'sets flag as used', %Q|#{feature_flag_method}("foo")|, 'gitaly_foo' + end + + context 'a symbol feature flag' do + include_examples 'sets flag as used', %Q|#{feature_flag_method}(:foo)|, 'gitaly_foo' + end + + context 'an interpolated string feature flag with a string prefix' do + include_examples 'sets flag as used', %Q|#{feature_flag_method}("foo_\#{bar}")|, %w[foo_hello foo_world] + end + + context 'an interpolated symbol feature flag with a string prefix' do + include_examples 'sets flag as used', %Q|#{feature_flag_method}(:"foo_\#{bar}")|, %w[foo_hello foo_world] + end + + context 'an interpolated string feature flag with a string prefix and suffix' do + include_examples 'does not set any flags as used', %Q|#{feature_flag_method}(:"foo_\#{bar}_baz")| + end + + context 'a dynamic string feature flag as a variable' do + include_examples 'does not set any flags as used', %Q|#{feature_flag_method}(a_variable, an_arg)| + end + + context 'an integer feature flag' do + include_examples 'does not set any flags as used', %Q|#{feature_flag_method}(123)| + end + end + end + + %w[ + experiment + experiment_enabled? + push_frontend_experiment + Gitlab::Experimentation.active? + ].each do |feature_flag_method| + context "#{feature_flag_method} method" do + context 'a string feature flag' do + include_examples 'sets flag as used', %Q|#{feature_flag_method}("baz")|, %w[baz baz_experiment_percentage] + end + + context 'a symbol feature flag' do + include_examples 'sets flag as used', %Q|#{feature_flag_method}(:baz)|, %w[baz baz_experiment_percentage] + end + + context 'an interpolated string feature flag with a string prefix' do + include_examples 'sets flag as used', %Q|#{feature_flag_method}("foo_\#{bar}")|, %w[foo_hello foo_world] + end + + context 'an interpolated symbol feature flag with a string prefix' do + include_examples 'sets flag as used', %Q|#{feature_flag_method}(:"foo_\#{bar}")|, %w[foo_hello foo_world] + end + + context 'an interpolated string feature flag with a string prefix and suffix' do + include_examples 'does not set any flags as used', %Q|#{feature_flag_method}(:"foo_\#{bar}_baz")| + end + + context 'a dynamic string feature flag as a variable' do + include_examples 'does not set any flags as used', %Q|#{feature_flag_method}(a_variable, an_arg)| + end + + context 'an integer feature flag' do + include_examples 'does not set any flags as used', %Q|#{feature_flag_method}(123)| + end + end + end + + %w[ + use_rugged? + ].each do |feature_flag_method| + context "#{feature_flag_method} method" do + context 'a string feature flag' do + include_examples 'sets flag as used', %Q|#{feature_flag_method}(arg, "baz")|, 'baz' + end + + context 'a symbol feature flag' do + include_examples 'sets flag as used', %Q|#{feature_flag_method}(arg, :baz)|, 'baz' + end + + context 'an interpolated string feature flag with a string prefix' do + include_examples 'sets flag as used', %Q|#{feature_flag_method}(arg, "foo_\#{bar}")|, %w[foo_hello foo_world] + end + + context 'an interpolated symbol feature flag with a string prefix' do + include_examples 'sets flag as used', %Q|#{feature_flag_method}(arg, :"foo_\#{bar}")|, %w[foo_hello foo_world] + end + + context 'an interpolated string feature flag with a string prefix and suffix' do + include_examples 'does not set any flags as used', %Q|#{feature_flag_method}(arg, :"foo_\#{bar}_baz")| + end + + context 'a dynamic string feature flag as a variable' do + include_examples 'does not set any flags as used', %Q|#{feature_flag_method}(a_variable, an_arg)| + end + + context 'an integer feature flag' do + include_examples 'does not set any flags as used', %Q|#{feature_flag_method}(arg, 123)| + end + end + end + + describe 'self.limit_feature_flag = :foo' do + include_examples 'sets flag as used', 'self.limit_feature_flag = :foo', 'foo' + end + + describe 'FEATURE_FLAG = :foo' do + include_examples 'sets flag as used', 'FEATURE_FLAG = :foo', 'foo' + end + + describe 'Worker `data_consistency` method' do + include_examples 'sets flag as used', 'data_consistency :delayed, feature_flag: :foo', 'foo' + include_examples 'does not set any flags as used', 'data_consistency :delayed' + end + + describe 'Worker `deduplicate` method' do + include_examples 'sets flag as used', 'deduplicate :delayed, feature_flag: :foo', 'foo' + include_examples 'does not set any flags as used', 'deduplicate :delayed' + end + + describe 'GraphQL `field` method' do + before do + allow(cop).to receive(:in_graphql_types?).and_return(true) + end + + include_examples 'sets flag as used', 'field :runners, Types::Ci::RunnerType.connection_type, null: true, feature_flag: :foo', 'foo' + include_examples 'sets flag as used', 'field :runners, null: true, feature_flag: :foo', 'foo' + include_examples 'does not set any flags as used', 'field :solution' + include_examples 'does not set any flags as used', 'field :runners, Types::Ci::RunnerType.connection_type' + include_examples 'does not set any flags as used', 'field :runners, Types::Ci::RunnerType.connection_type, null: true, description: "hello world"' + include_examples 'does not set any flags as used', 'field :solution, type: GraphQL::STRING_TYPE, null: true, description: "URL to the vulnerabilitys details page."' + end + + describe "tracking of usage data metrics known events happens at the beginning of inspection" do + let(:usage_data_counters_known_event_feature_flags) { ['an_event_feature_flag'] } + + before do + allow(cop).to receive(:usage_data_counters_known_event_feature_flags).and_return(usage_data_counters_known_event_feature_flags) + end + + include_examples 'sets flag as used', "FEATURE_FLAG = :foo", %w[foo an_event_feature_flag] + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 31ff619232c..492b9a478fe 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -230,6 +230,10 @@ RSpec.configure do |config| Gitlab::Database.set_open_transactions_baseline end + config.append_before do + Thread.current[:current_example_group] = ::RSpec.current_example.metadata[:example_group] + end + config.append_after do Gitlab::Database.reset_open_transactions_baseline end diff --git a/spec/support/gitlab_experiment.rb b/spec/support/gitlab_experiment.rb index b84adf82d29..3d099dc689c 100644 --- a/spec/support/gitlab_experiment.rb +++ b/spec/support/gitlab_experiment.rb @@ -4,16 +4,6 @@ require 'gitlab/experiment/rspec' require_relative 'stub_snowplow' -# This is a temporary fix until we have a larger discussion around the -# challenges raised in https://gitlab.com/gitlab-org/gitlab/-/issues/300104 -require Rails.root.join('app', 'experiments', 'application_experiment') -class ApplicationExperiment # rubocop:disable Gitlab/NamespacedClass - def initialize(...) - super(...) - Feature.persist_used!(feature_flag_name) - end -end - RSpec.configure do |config| config.include StubSnowplow, :experiment diff --git a/spec/support/helpers/stub_experiments.rb b/spec/support/helpers/stub_experiments.rb index 408d16a7c08..8995b8f5f7b 100644 --- a/spec/support/helpers/stub_experiments.rb +++ b/spec/support/helpers/stub_experiments.rb @@ -11,7 +11,6 @@ module StubExperiments allow(Gitlab::Experimentation).to receive(:active?).and_call_original experiments.each do |experiment_key, enabled| - Feature.persist_used!("#{experiment_key}#{feature_flag_suffix}") allow(Gitlab::Experimentation).to receive(:active?).with(experiment_key) { enabled } end end @@ -26,7 +25,6 @@ module StubExperiments allow(Gitlab::Experimentation).to receive(:in_experiment_group?).and_call_original experiments.each do |experiment_key, enabled| - Feature.persist_used!("#{experiment_key}#{feature_flag_suffix}") allow(Gitlab::Experimentation).to receive(:in_experiment_group?).with(experiment_key, anything) { enabled } end end diff --git a/spec/support/helpers/stubbed_feature.rb b/spec/support/helpers/stubbed_feature.rb index 67ceb7d9b35..4113a28182b 100644 --- a/spec/support/helpers/stubbed_feature.rb +++ b/spec/support/helpers/stubbed_feature.rb @@ -4,14 +4,6 @@ module StubbedFeature extend ActiveSupport::Concern - prepended do - cattr_reader(:persist_used) do - # persist feature flags in CI - # nil: indicates that we do not want to persist used feature flags - Gitlab::Utils.to_boolean(ENV['CI']) ? {} : nil - end - end - class_methods do # Turn stubbed feature flags on or off. def stub=(stub) @@ -41,8 +33,6 @@ module StubbedFeature feature_flag = super return feature_flag unless stub? - persist_used!(args.first) - # If feature flag is not persisted we mark the feature flag as enabled # We do `m.call` as we want to validate the execution of method arguments # and a feature flag state if it is not persisted @@ -52,17 +42,5 @@ module StubbedFeature feature_flag end - - # This method creates a temporary file in `tmp/feature_flags` - # if feature flag was touched during execution - def persist_used!(name) - return unless persist_used - return if persist_used[name] - - persist_used[name] = true - FileUtils.touch( - Rails.root.join('tmp', 'feature_flags', name.to_s + ".used") - ) - end end end diff --git a/spec/support/helpers/test_env.rb b/spec/support/helpers/test_env.rb index 40a3dbfbf25..8814d260fb3 100644 --- a/spec/support/helpers/test_env.rb +++ b/spec/support/helpers/test_env.rb @@ -1,7 +1,8 @@ # frozen_string_literal: true +require 'parallel' + module TestEnv - extend ActiveSupport::Concern extend self ComponentFailedToInstallError = Class.new(StandardError) @@ -94,50 +95,40 @@ module TestEnv TMP_TEST_PATH = Rails.root.join('tmp', 'tests').freeze REPOS_STORAGE = 'default' SECOND_STORAGE_PATH = Rails.root.join('tmp', 'tests', 'second_storage') + SETUP_METHODS = %i[setup_gitaly setup_gitlab_shell setup_workhorse setup_factory_repo setup_forked_repo].freeze + + # Can be overriden + def setup_methods + SETUP_METHODS + end # Test environment # # See gitlab.yml.example test section for paths # - def init(opts = {}) + def init unless Rails.env.test? puts "\nTestEnv.init can only be run if `RAILS_ENV` is set to 'test' not '#{Rails.env}'!\n" exit 1 end + start = Time.now # Disable mailer for spinach tests - disable_mailer if opts[:mailer] == false - clean_test_path - setup_gitlab_shell - - setup_gitaly - - # Feature specs are run through Workhorse - setup_workhorse - - # Create repository for FactoryBot.create(:project) - setup_factory_repo - - # Create repository for FactoryBot.create(:forked_project_with_submodules) - setup_forked_repo - end - - included do |config| - config.append_before do - set_current_example_group + # Install components in parallel as most of the setup is I/O. + Parallel.each(setup_methods) do |method| + public_send(method) end - end - def disable_mailer - allow_any_instance_of(NotificationService).to receive(:mailer) - .and_return(double.as_null_object) + post_init + + puts "\nTest environment set up in #{Time.now - start} seconds" end - def enable_mailer - allow_any_instance_of(NotificationService).to receive(:mailer) - .and_call_original + # Can be overriden + def post_init + start_gitaly(gitaly_dir) end # Clean /tmp/tests @@ -164,12 +155,11 @@ module TestEnv end def setup_gitaly - install_gitaly_args = [gitaly_dir, repos_path, gitaly_url].compact.join(',') - component_timed_setup('Gitaly', install_dir: gitaly_dir, version: Gitlab::GitalyClient.expected_server_version, - task: "gitlab:gitaly:install[#{install_gitaly_args}]") do + task: "gitlab:gitaly:install", + task_args: [gitaly_dir, repos_path, gitaly_url].compact) do Gitlab::SetupHelper::Gitaly.create_configuration( gitaly_dir, { 'default' => repos_path }, @@ -190,8 +180,6 @@ module TestEnv ) Gitlab::SetupHelper::Praefect.create_configuration(gitaly_dir, { 'praefect' => repos_path }, force: true) end - - start_gitaly(gitaly_dir) end def gitaly_socket_path @@ -273,19 +261,18 @@ module TestEnv raise "could not connect to #{service} at #{socket.inspect} after #{sleep_time} seconds" end + # Feature specs are run through Workhorse def setup_workhorse start = Time.now return if skip_compile_workhorse? - puts "\n==> Setting up GitLab Workhorse..." - FileUtils.rm_rf(workhorse_dir) Gitlab::SetupHelper::Workhorse.compile_into(workhorse_dir) Gitlab::SetupHelper::Workhorse.create_configuration(workhorse_dir, nil) File.write(workhorse_tree_file, workhorse_tree) if workhorse_source_clean? - puts " GitLab Workhorse set up in #{Time.now - start} seconds...\n" + puts "==> GitLab Workhorse set up in #{Time.now - start} seconds...\n" end def skip_compile_workhorse? @@ -349,10 +336,12 @@ module TestEnv ENV.fetch('GITLAB_WORKHORSE_URL', nil) end + # Create repository for FactoryBot.create(:project) def setup_factory_repo setup_repo(factory_repo_path, factory_repo_path_bare, factory_repo_name, BRANCH_SHA) end + # Create repository for FactoryBot.create(:forked_project_with_submodules) # This repo has a submodule commit that is not present in the main test # repository. def setup_forked_repo @@ -363,20 +352,18 @@ module TestEnv clone_url = "https://gitlab.com/gitlab-org/#{repo_name}.git" unless File.directory?(repo_path) - puts "\n==> Setting up #{repo_name} repository in #{repo_path}..." start = Time.now system(*%W(#{Gitlab.config.git.bin_path} clone --quiet -- #{clone_url} #{repo_path})) - puts " #{repo_path} set up in #{Time.now - start} seconds...\n" + puts "==> #{repo_path} set up in #{Time.now - start} seconds...\n" end set_repo_refs(repo_path, refs) unless File.directory?(repo_path_bare) - puts "\n==> Setting up #{repo_name} bare repository in #{repo_path_bare}..." start = Time.now # We must copy bare repositories because we will push to them. system(git_env, *%W(#{Gitlab.config.git.bin_path} clone --quiet --bare -- #{repo_path} #{repo_path_bare})) - puts " #{repo_path_bare} set up in #{Time.now - start} seconds...\n" + puts "==> #{repo_path_bare} set up in #{Time.now - start} seconds...\n" end end @@ -468,10 +455,6 @@ module TestEnv private - def set_current_example_group - Thread.current[:current_example_group] = ::RSpec.current_example.metadata[:example_group] - end - # These are directories that should be preserved at cleanup time def test_dirs @test_dirs ||= %w[ @@ -526,7 +509,7 @@ module TestEnv end end - def component_timed_setup(component, install_dir:, version:, task:) + def component_timed_setup(component, install_dir:, version:, task:, task_args: []) start = Time.now ensure_component_dir_name_is_correct!(component, install_dir) @@ -535,17 +518,22 @@ module TestEnv return if File.exist?(install_dir) && ci? if component_needs_update?(install_dir, version) - puts "\n==> Setting up #{component}..." # Cleanup the component entirely to ensure we start fresh FileUtils.rm_rf(install_dir) - unless system('rake', task) - raise ComponentFailedToInstallError + if ENV['SKIP_RAILS_ENV_IN_RAKE'] + # When we run `scripts/setup-test-env`, we take care of loading the necessary dependencies + # so we can run the rake task programmatically. + Rake::Task[task].invoke(*task_args) + else + # In other cases, we run the task via `rake` so that the environment + # and dependencies are automatically loaded. + raise ComponentFailedToInstallError unless system('rake', "#{task}[#{task_args.join(',')}]") end yield if block_given? - puts " #{component} set up in #{Time.now - start} seconds...\n" + puts "==> #{component} set up in #{Time.now - start} seconds...\n" end rescue ComponentFailedToInstallError puts "\n#{component} failed to install, cleaning up #{install_dir}!\n" diff --git a/spec/views/shared/_global_alert.html.haml_spec.rb b/spec/views/shared/_global_alert.html.haml_spec.rb new file mode 100644 index 00000000000..7eec068645a --- /dev/null +++ b/spec/views/shared/_global_alert.html.haml_spec.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe 'shared/_global_alert.html.haml' do + before do + allow(view).to receive(:sprite_icon).and_return('<span class="icon"></span>'.html_safe) + end + + it 'renders the title' do + title = "The alert's title" + render partial: 'shared/global_alert', locals: { title: title } + + expect(rendered).to have_text(title) + end + + context 'variants' do + it 'renders an info alert by default' do + render + + expect(rendered).to have_selector(".gl-alert-info") + end + + %w[warning success danger tip].each do |variant| + it "renders a #{variant} variant" do + allow(view).to receive(:variant).and_return(variant) + render partial: 'shared/global_alert', locals: { variant: variant } + + expect(rendered).to have_selector(".gl-alert-#{variant}") + end + end + end + + context 'dismissible option' do + it 'shows the dismiss button by default' do + render + + expect(rendered).to have_selector('.gl-dismiss-btn') + end + + it 'does not show the dismiss button when dismissible is false' do + render partial: 'shared/global_alert', locals: { dismissible: false } + + expect(rendered).not_to have_selector('.gl-dismiss-btn') + end + end + + context 'fixed layout' do + before do + allow(view).to receive(:fluid_layout).and_return(false) + end + + it 'does not add layout limited class' do + render + + expect(rendered).not_to have_selector('.gl-alert-layout-limited') + end + + it 'adds container classes' do + render + + expect(rendered).to have_selector('.container-fluid.container-limited') + end + + it 'does not add container classes if is_contained is true' do + render partial: 'shared/global_alert', locals: { is_contained: true } + + expect(rendered).not_to have_selector('.container-fluid.container-limited') + end + end + + context 'fluid layout' do + before do + allow(view).to receive(:fluid_layout).and_return(true) + render + end + + it 'adds layout limited class' do + expect(rendered).to have_selector('.gl-alert-layout-limited') + end + + it 'does not add container classes' do + expect(rendered).not_to have_selector('.container-fluid.container-limited') + end + end +end diff --git a/yarn.lock b/yarn.lock index cbbe7a1114e..f86c8d75736 100644 --- a/yarn.lock +++ b/yarn.lock @@ -908,10 +908,10 @@ resolved "https://registry.yarnpkg.com/@gitlab/tributejs/-/tributejs-1.0.0.tgz#672befa222aeffc83e7d799b0500a7a4418e59b8" integrity sha512-nmKw1+hB6MHvlmPz63yPwVs1qQkycHwsKgxpEbzmky16Y6mL4EJMk3w1b8QlOAF/AIAzjCERPhe/R4MJiohbZw== -"@gitlab/ui@29.35.0": - version "29.35.0" - resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-29.35.0.tgz#bb04d1e4f8796134bc406adaa869c1b5b1fdcaf2" - integrity sha512-Fso++QXqxZSfIgSmPGlfQC3mdFI6oh/AOL/9cGn4t/3kfxwHd1GCMjUNAFeHsgyIwKIr1hwksipapwuuOIFSCw== +"@gitlab/ui@29.36.0": + version "29.36.0" + resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-29.36.0.tgz#a418c34c7ef768552b551807fa2a65deeaeba0bf" + integrity sha512-ZsaYpbp5cFN9hxVCf19E7avS9AmMaAyS4/Zwkwu2reHJUOkwyOY24eLr44u/Kbaq6SkFarQ2y+zU8vuhzXwQjQ== dependencies: "@babel/standalone" "^7.0.0" "@gitlab/vue-toasted" "^1.3.0" |