diff options
39 files changed, 677 insertions, 941 deletions
diff --git a/README.md b/README.md index b3b66959884..1372e47d52f 100644 --- a/README.md +++ b/README.md @@ -82,9 +82,9 @@ GitLab is a Ruby on Rails application that runs on the following software: - Ruby (MRI) 2.6.5 - Git 2.8.4+ - Redis 2.8+ -- PostgreSQL (preferred) or MySQL +- PostgreSQL 9.6+ -For more information please see the [architecture documentation](https://docs.gitlab.com/ce/development/architecture.html). +For more information please see the [architecture](https://docs.gitlab.com/ee/development/architecture.html) and [requirements](https://docs.gitlab.com/ee/install/requirements.html) documentation. ## UX design diff --git a/app/assets/javascripts/environments/components/enable_review_app_button.vue b/app/assets/javascripts/environments/components/enable_review_app_button.vue new file mode 100644 index 00000000000..2f9e9cb628f --- /dev/null +++ b/app/assets/javascripts/environments/components/enable_review_app_button.vue @@ -0,0 +1,107 @@ +<script> +import { GlButton, GlModal, GlModalDirective, GlLink, GlSprintf } from '@gitlab/ui'; +import ModalCopyButton from '~/vue_shared/components/modal_copy_button.vue'; +import { s__ } from '~/locale'; + +export default { + components: { + GlButton, + GlLink, + GlModal, + GlSprintf, + ModalCopyButton, + }, + directives: { + 'gl-modal': GlModalDirective, + }, + instructionText: { + step1: s__( + 'EnableReviewApp|%{stepStart}Step 1%{stepEnd}. Ensure you have Kubernetes set up and have a base domain for your %{linkStart}cluster%{linkEnd}.', + ), + step2: s__('EnableReviewApp|%{stepStart}Step 2%{stepEnd}. Copy the following snippet:'), + step3: s__( + `EnableReviewApp|%{stepStart}Step 3%{stepEnd}. Add it to the project %{linkStart}gitlab-ci.yml%{linkEnd} file.`, + ), + }, + modalInfo: { + closeText: s__('EnableReviewApp|Close'), + copyToClipboardText: s__('EnableReviewApp|Copy snippet text'), + copyString: `deploy_review + stage: deploy + script: + - echo "Deploy a review app" + environment: + name: review/$CI_COMMIT_REF_NAME + url: https://$CI_ENVIRONMENT_SLUG.example.com + only: branches + except: master`, + id: 'enable-review-app-info', + title: s__('ReviewApp|Enable Review App'), + }, +}; +</script> +<template> + <div> + <gl-button + v-gl-modal="$options.modalInfo.id" + variant="info" + category="secondary" + type="button" + class="js-enable-review-app-button" + > + {{ s__('Environments|Enable review app') }} + </gl-button> + <gl-modal + :modal-id="$options.modalInfo.id" + :title="$options.modalInfo.title" + size="lg" + class="text-2 ws-normal" + ok-only + ok-variant="light" + :ok-title="$options.modalInfo.closeText" + > + <p> + <gl-sprintf :message="$options.instructionText.step1"> + <template #step="{ content }"> + <strong>{{ content }}</strong> + </template> + <template #link="{ content }"> + <gl-link + href="https://docs.gitlab.com/ee/user/project/clusters/add_remove_clusters.html" + target="_blank" + >{{ content }}</gl-link + > + </template> + </gl-sprintf> + </p> + <div> + <p> + <gl-sprintf :message="$options.instructionText.step2"> + <template #step="{ content }"> + <strong>{{ content }}</strong> + </template> + </gl-sprintf> + </p> + <div class="flex align-items-start"> + <pre class="w-100"> {{ $options.modalInfo.copyString }} </pre> + <modal-copy-button + :title="$options.modalInfo.copyToClipboardText" + :text="$options.modalInfo.copyString" + :modal-id="$options.modalInfo.id" + css-classes="border-0" + /> + </div> + </div> + <p> + <gl-sprintf :message="$options.instructionText.step3"> + <template #step="{ content }"> + <strong>{{ content }}</strong> + </template> + <template #link="{ content }"> + <gl-link href="blob/master/.gitlab-ci.yml" target="_blank">{{ content }}</gl-link> + </template> + </gl-sprintf> + </p> + </gl-modal> + </div> +</template> diff --git a/app/assets/javascripts/environments/components/environments_app.vue b/app/assets/javascripts/environments/components/environments_app.vue index 50c667e6966..07b8d20fde0 100644 --- a/app/assets/javascripts/environments/components/environments_app.vue +++ b/app/assets/javascripts/environments/components/environments_app.vue @@ -1,19 +1,23 @@ <script> +import { GlButton } from '@gitlab/ui'; import envrionmentsAppMixin from 'ee_else_ce/environments/mixins/environments_app_mixin'; -import Flash from '../../flash'; -import { s__ } from '../../locale'; +import Flash from '~/flash'; +import { s__ } from '~/locale'; import emptyState from './empty_state.vue'; import eventHub from '../event_hub'; import environmentsMixin from '../mixins/environments_mixin'; -import CIPaginationMixin from '../../vue_shared/mixins/ci_pagination_api_mixin'; +import CIPaginationMixin from '~/vue_shared/mixins/ci_pagination_api_mixin'; +import EnableReviewAppButton from './enable_review_app_button.vue'; import StopEnvironmentModal from './stop_environment_modal.vue'; import ConfirmRollbackModal from './confirm_rollback_modal.vue'; export default { components: { + ConfirmRollbackModal, emptyState, + EnableReviewAppButton, + GlButton, StopEnvironmentModal, - ConfirmRollbackModal, }, mixins: [CIPaginationMixin, environmentsMixin, envrionmentsAppMixin], @@ -96,10 +100,16 @@ export default { <div class="top-area"> <tabs :tabs="tabs" scope="environments" @onChangeTab="onChangeTab" /> - <div v-if="canCreateEnvironment && !isLoading" class="nav-controls"> - <a :href="newEnvironmentPath" class="btn btn-success"> + <div class="nav-controls"> + <enable-review-app-button v-if="state.reviewAppDetails.can_setup_review_app" class="mr-2" /> + <gl-button + v-if="canCreateEnvironment && !isLoading" + :href="newEnvironmentPath" + category="primary" + variant="success" + > {{ s__('Environments|New environment') }} - </a> + </gl-button> </div> </div> diff --git a/app/assets/javascripts/environments/mixins/environments_mixin.js b/app/assets/javascripts/environments/mixins/environments_mixin.js index 34374e306a4..1c5884b541c 100644 --- a/app/assets/javascripts/environments/mixins/environments_mixin.js +++ b/app/assets/javascripts/environments/mixins/environments_mixin.js @@ -52,6 +52,7 @@ export default { this.store.storeAvailableCount(resp.data.available_count); this.store.storeStoppedCount(resp.data.stopped_count); this.store.storeEnvironments(resp.data.environments); + this.store.setReviewAppDetails(resp.data.review_app); this.store.setPagination(resp.headers); } }, diff --git a/app/assets/javascripts/environments/stores/environments_store.js b/app/assets/javascripts/environments/stores/environments_store.js index 81c257acd53..6b7c1ff627d 100644 --- a/app/assets/javascripts/environments/stores/environments_store.js +++ b/app/assets/javascripts/environments/stores/environments_store.js @@ -14,6 +14,7 @@ export default class EnvironmentsStore { this.state.stoppedCounter = 0; this.state.availableCounter = 0; this.state.paginationInformation = {}; + this.state.reviewAppDetails = {}; return this; } @@ -104,6 +105,11 @@ export default class EnvironmentsStore { return paginationInformation; } + setReviewAppDetails(details = {}) { + this.state.reviewAppDetails = details; + return details; + } + /** * Stores the number of available environments. * diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 7cb629dee21..fa88ca91170 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -37,7 +37,6 @@ class ApplicationController < ActionController::Base around_action :set_current_context around_action :set_locale around_action :set_session_storage - around_action :set_current_admin after_action :set_page_title_header, if: :json_request? after_action :limit_session_time, if: -> { !current_user } @@ -474,13 +473,6 @@ class ApplicationController < ActionController::Base response.headers['Page-Title'] = URI.escape(page_title('GitLab')) end - def set_current_admin(&block) - return yield unless Feature.enabled?(:user_mode_in_session) - return yield unless current_user - - Gitlab::Auth::CurrentUserMode.with_current_admin(current_user, &block) - end - def html_request? request.format.html? end diff --git a/changelogs/unreleased/118844-one-button-review-app-fe.yml b/changelogs/unreleased/118844-one-button-review-app-fe.yml new file mode 100644 index 00000000000..f7a3cf6dd72 --- /dev/null +++ b/changelogs/unreleased/118844-one-button-review-app-fe.yml @@ -0,0 +1,5 @@ +--- +title: Create conditional Enable Review App button +merge_request: 23703 +author: +type: added diff --git a/changelogs/unreleased/36675-add-median-line-to-cycle-analytics-duration-chart.yml b/changelogs/unreleased/36675-add-median-line-to-cycle-analytics-duration-chart.yml new file mode 100644 index 00000000000..f9ca6b1a2d2 --- /dev/null +++ b/changelogs/unreleased/36675-add-median-line-to-cycle-analytics-duration-chart.yml @@ -0,0 +1,5 @@ +--- +title: Add cycle analytics duration chart with median line +merge_request: 23971 +author: +type: added diff --git a/changelogs/unreleased/refactor-admin-mode-in-sidekiq-jobs.yml b/changelogs/unreleased/refactor-admin-mode-in-sidekiq-jobs.yml deleted file mode 100644 index 37e34b3174c..00000000000 --- a/changelogs/unreleased/refactor-admin-mode-in-sidekiq-jobs.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Admin mode support in sidekiq jobs -merge_request: 21792 -author: Diego Louzán -type: changed diff --git a/doc/development/code_review.md b/doc/development/code_review.md index 9b82cf5ff99..3d41ff64380 100644 --- a/doc/development/code_review.md +++ b/doc/development/code_review.md @@ -347,6 +347,12 @@ reviewee. of the contributed code. It's usually a good idea to ask another maintainer or reviewer before doing it, but have the courage to do it when you believe it is important. +- In the interest of [Iteration](https://about.gitlab.com/handbook/values/#iteration), + if, as a reviewer, your suggestions are non-blocking changes or personal preference + (not a documented or agreed requirement), consider approving the merge request + before passing it back to the author. This allows them to implement your suggestions + if they agree, or allows them to pass it onto the + maintainer for review straight away. This can help reduce our overall time-to-merge. - There is a difference in doing things right and doing things right now. Ideally, we should do the former, but in the real world we need the latter as well. A good example is a security fix which should be released as soon as diff --git a/doc/development/migration_style_guide.md b/doc/development/migration_style_guide.md index cccea4ee9f4..209fa5d610e 100644 --- a/doc/development/migration_style_guide.md +++ b/doc/development/migration_style_guide.md @@ -379,10 +379,6 @@ Rails migration example: ```ruby add_column_with_default(:projects, :foo, :integer, default: 10, limit: 8) - -# or - -add_column(:projects, :foo, :integer, default: 10, limit: 8) ``` ## Timestamp column type diff --git a/doc/development/reactive_caching.md b/doc/development/reactive_caching.md new file mode 100644 index 00000000000..de93a5aa1d0 --- /dev/null +++ b/doc/development/reactive_caching.md @@ -0,0 +1,276 @@ +# `ReactiveCaching` + +> This doc refers to <https://gitlab.com/gitlab-org/gitlab/blob/master/app/models/concerns/reactive_caching.rb>. + +The `ReactiveCaching` concern is used for fetching some data in the background and store it +in the Rails cache, keeping it up-to-date for as long as it is being requested. If the +data hasn't been requested for `reactive_cache_lifetime`, it will stop being refreshed, +and then be removed. + +## Examples + +```ruby +class Foo < ApplicationRecord + include ReactiveCaching + + after_save :clear_reactive_cache! + + def calculate_reactive_cache(param1, param2) + # Expensive operation here. The return value of this method is cached + end + + def result + # Any arguments can be passed to `with_reactive_cache`. `calculate_reactive_cache` + # will be called with the same arguments. + with_reactive_cache(param1, param2) do |data| + # ... + end + end +end +``` + +In this example, the first time `#result` is called, it will return `nil`. However, +it will enqueue a background worker to call `#calculate_reactive_cache` and set an +initial cache lifetime of 10 min. + +## How it works + +The first time `#with_reactive_cache` is called, a background job is enqueued and +`with_reactive_cache` returns `nil`. The background job calls `#calculate_reactive_cache` +and stores its return value. It also re-enqueues the background job to run again after +`reactive_cache_refresh_interval`. Therefore, it will keep the stored value up to date. +Calculations never run concurrently. + +Calling `#with_reactive_cache` while a value is cached will call the block given to +`#with_reactive_cache`, yielding the cached value. It will also extend the lifetime +of the cache by the `reactive_cache_lifetime` value. + +Once the lifetime has expired, no more background jobs will be enqueued and calling +`#with_reactive_cache` will again return `nil` - starting the process all over again. + +## When to use + +- If we need to make a request to an external API (for example, requests to the k8s API). +It is not advisable to keep the application server worker blocked for the duration of +the external request. +- If a model needs to perform a lot of database calls or other time consuming +calculations. + +## How to use + +### In models and services + +The ReactiveCaching concern can be used in models as well as `project_services` +(`app/models/project_services`). + +1. Include the concern in your model or service. + + When including in a model: + + ```ruby + include ReactiveCaching + ``` + + or when including in a `project_service`: + + ```ruby + include ReactiveService + ``` + +1. Implement the `calculate_reactive_cache` method in your model/service. +1. Call `with_reactive_cache` in your model/service where the cached value is needed. + +### In controllers + +Controller endpoints that call a model or service method that uses `ReactiveCaching` should +not wait until the background worker completes. + +- An API that calls a model or service method that uses `ReactiveCaching` should return +`202 accepted` when the cache is being calculated (when `#with_reactive_cache` returns `nil`). +- It should also +[set the polling interval header](fe_guide/performance.md#realtime-components) with +`Gitlab::PollingInterval.set_header`. +- The consumer of the API is expected to poll the API. +- You can also consider implementing [ETag caching](polling.md) to reduce the server +load caused by polling. + +### Methods to implement in a model or service + +These are methods that should be implemented in the model/service that includes `ReactiveCaching`. + +#### `#calculate_reactive_cache` (required) + +- This method must be implemented. Its return value will be cached. +- It will be called by `ReactiveCaching` when it needs to populate the cache. +- Any arguments passed to `with_reactive_cache` will also be passed to `calculate_reactive_cache`. + +#### `#reactive_cache_updated` (optional) + +- This method can be implemented if needed. +- It is called by the `ReactiveCaching` concern whenever the cache is updated. +If the cache is being refreshed and the new cache value is the same as the old cache +value, this method will not be called. It is only called if a new value is stored in +the cache. +- It can be used to perform an action whenever the cache is updated. + +### Methods called by a model or service + +These are methods provided by `ReactiveCaching` and should be called in +the model/service. + +#### `#with_reactive_cache` (required) + +- `with_reactive_cache` must be called where the result of `calculate_reactive_cache` +is required. +- A block can be given to `with_reactive_cache`. `with_reactive_cache` can also take +any number of arguments. Any arguments passed to `with_reactive_cache` will be +passed to `calculate_reactive_cache`. The arguments passed to `with_reactive_cache` +will be appended to the cache key name. +- If `with_reactive_cache` is called when the result has already been cached, the +block will be called, yielding the cached value and the return value of the block +will be returned by `with_reactive_cache`. It will also reset the timeout of the +cache to the `reactive_cache_lifetime` value. +- If the result has not been cached as yet, `with_reactive_cache` will return nil. +It will also enqueue a background job, which will call `calculate_reactive_cache` +and cache the result. +- Once the background job has completed and the result is cached, the next call +to `with_reactive_cache` will pick up the cached value. +- In the example below, `data` is the cached value which is yielded to the block +given to `with_reactive_cache`. + + ```ruby + class Foo < ApplicationRecord + include ReactiveCaching + + def calculate_reactive_cache(param1, param2) + # Expensive operation here. The return value of this method is cached + end + + def result + with_reactive_cache(param1, param2) do |data| + # ... + end + end + end + ``` + +#### `#clear_reactive_cache!` (optional) + +- This method can be called when the cache needs to be expired/cleared. For example, +it can be called in an `after_save` callback in a model so that the cache is +cleared after the model is modified. +- This method should be called with the same parameters that are passed to +`with_reactive_cache` because the parameters are part of the cache key. + +#### `#without_reactive_cache` (optional) + +- This is a convenience method that can be used for debugging purposes. +- This method calls `calculate_reactive_cache` in the current process instead of +in a background worker. + +### Configurable options + +There are some `class_attribute` options which can be tweaked. + +#### `self.reactive_cache_key` + +- The value of this attribute is the prefix to the `data` and `alive` cache key names. +The parameters passed to `with_reactive_cache` form the rest of the cache key names. +- By default, this key uses the model's name and the ID of the record. + + ```ruby + self.reactive_cache_key = -> (record) { [model_name.singular, record.id] } + ``` + +- The `data` and `alive` cache keys in this case will be `"ExampleModel:1:arg1:arg2"` +and `"ExampleModel:1:arg1:arg2:alive"` respectively, where `ExampleModel` is the +name of the model, `1` is the ID of the record, `arg1` and `arg2` are parameters +passed to `with_reactive_cache`. +- If you're including this concern in a service instead, you will need to override +the default by adding the following to your service: + + ```ruby + self.reactive_cache_key = ->(service) { [service.class.model_name.singular, service.project_id] } + ``` + + If your reactive_cache_key is exactly like the above, you can use the existing + `ReactiveService` concern instead. + +#### `self.reactive_cache_lease_timeout` + +- `ReactiveCaching` uses `Gitlab::ExclusiveLease` to ensure that the cache calculation +is never run concurrently by multiple workers. +- This attribute is the timeout for the `Gitlab::ExclusiveLease`. +- It defaults to 2 minutes, but can be overriden if a different timeout is required. + +```ruby +self.reactive_cache_lease_timeout = 2.minutes +``` + +#### `self.reactive_cache_refresh_interval` + +- This is the interval at which the cache is refreshed. +- It defaults to 1 minute. + +```ruby +self.reactive_cache_lease_timeout = 1.minute +``` + +#### `self.reactive_cache_lifetime` + +- This is the duration after which the cache will be cleared if there are no requests. +- The default is 10 minutes. If there are no requests for this cache value for 10 minutes, +the cache will expire. +- If the cache value is requested before it expires, the timeout of the cache will +be reset to `reactive_cache_lifetime`. + +```ruby +self.reactive_cache_lifetime = 10.minutes +``` + +#### `self.reactive_cache_worker_finder` + +- This is the method used by the background worker to find or generate the object on +which `calculate_reactive_cache` can be called. +- By default it uses the model primary key to find the object: + + ```ruby + self.reactive_cache_worker_finder = ->(id, *_args) do + find_by(primary_key => id) + end + ``` + +- The default behaviour can be overridden by defining a custom `reactive_cache_worker_finder`. + + ```ruby + class Foo < ApplicationRecord + include ReactiveCaching + + self.reactive_cache_worker_finder = ->(_id, *args) { from_cache(*args) } + + def self.from_cache(var1, var2) + # This method will be called by the background worker with "bar1" and + # "bar2" as arguments. + new(var1, var2) + end + + def initialize(var1, var2) + # ... + end + + def calculate_reactive_cache(var1, var2) + # Expensive operation here. The return value of this method is cached + end + + def result + with_reactive_cache("bar1", "bar2") do |data| + # ... + end + end + end + ``` + + - In this example, the primary key ID will be passed to `reactive_cache_worker_finder` + along with the parameters passed to `with_reactive_cache`. + - The custom `reactive_cache_worker_finder` calls `.from_cache` with the parameters + passed to `with_reactive_cache`. diff --git a/doc/development/utilities.md b/doc/development/utilities.md index 561d5efc696..dfdc5c66114 100644 --- a/doc/development/utilities.md +++ b/doc/development/utilities.md @@ -196,277 +196,4 @@ end ## `ReactiveCaching` -> This doc refers to <https://gitlab.com/gitlab-org/gitlab/blob/master/app/models/concerns/reactive_caching.rb>. - -The `ReactiveCaching` concern is used for fetching some data in the background and store it -in the Rails cache, keeping it up-to-date for as long as it is being requested. If the -data hasn't been requested for `reactive_cache_lifetime`, it will stop being refreshed, -and then be removed. - -### Examples - -```ruby -class Foo < ApplicationRecord - include ReactiveCaching - - after_save :clear_reactive_cache! - - def calculate_reactive_cache(param1, param2) - # Expensive operation here. The return value of this method is cached - end - - def result - # Any arguments can be passed to `with_reactive_cache`. `calculate_reactive_cache` - # will be called with the same arguments. - with_reactive_cache(param1, param2) do |data| - # ... - end - end -end -``` - -In this example, the first time `#result` is called, it will return `nil`. However, -it will enqueue a background worker to call `#calculate_reactive_cache` and set an -initial cache lifetime of 10 min. - -### How it works - -The first time `#with_reactive_cache` is called, a background job is enqueued and -`with_reactive_cache` returns `nil`. The background job calls `#calculate_reactive_cache` -and stores its return value. It also re-enqueues the background job to run again after -`reactive_cache_refresh_interval`. Therefore, it will keep the stored value up to date. -Calculations never run concurrently. - -Calling `#with_reactive_cache` while a value is cached will call the block given to -`#with_reactive_cache`, yielding the cached value. It will also extend the lifetime -of the cache by the `reactive_cache_lifetime` value. - -Once the lifetime has expired, no more background jobs will be enqueued and calling -`#with_reactive_cache` will again return `nil` - starting the process all over again. - -### When to use - -- If we need to make a request to an external API (for example, requests to the k8s API). -It is not advisable to keep the application server worker blocked for the duration of -the external request. -- If a model needs to perform a lot of database calls or other time consuming -calculations. - -### How to use - -#### In models and services - -The ReactiveCaching concern can be used in models as well as `project_services` -(`app/models/project_services`). - -1. Include the concern in your model or service. - - When including in a model: - - ```ruby - include ReactiveCaching - ``` - - or when including in a `project_service`: - - ```ruby - include ReactiveService - ``` - -1. Implement the `calculate_reactive_cache` method in your model/service. -1. Call `with_reactive_cache` in your model/service where the cached value is needed. - -#### In controllers - -Controller endpoints that call a model or service method that uses `ReactiveCaching` should -not wait until the background worker completes. - -- An API that calls a model or service method that uses `ReactiveCaching` should return -`202 accepted` when the cache is being calculated (when `#with_reactive_cache` returns `nil`). -- It should also -[set the polling interval header](fe_guide/performance.md#realtime-components) with -`Gitlab::PollingInterval.set_header`. -- The consumer of the API is expected to poll the API. -- You can also consider implementing [ETag caching](polling.md) to reduce the server -load caused by polling. - -#### Methods to implement in a model or service - -These are methods that should be implemented in the model/service that includes `ReactiveCaching`. - -##### `#calculate_reactive_cache` (required) - -- This method must be implemented. Its return value will be cached. -- It will be called by `ReactiveCaching` when it needs to populate the cache. -- Any arguments passed to `with_reactive_cache` will also be passed to `calculate_reactive_cache`. - -##### `#reactive_cache_updated` (optional) - -- This method can be implemented if needed. -- It is called by the `ReactiveCaching` concern whenever the cache is updated. -If the cache is being refreshed and the new cache value is the same as the old cache -value, this method will not be called. It is only called if a new value is stored in -the cache. -- It can be used to perform an action whenever the cache is updated. - -#### Methods called by a model or service - -These are methods provided by `ReactiveCaching` and should be called in -the model/service. - -##### `#with_reactive_cache` (required) - -- `with_reactive_cache` must be called where the result of `calculate_reactive_cache` -is required. -- A block can be given to `with_reactive_cache`. `with_reactive_cache` can also take -any number of arguments. Any arguments passed to `with_reactive_cache` will be -passed to `calculate_reactive_cache`. The arguments passed to `with_reactive_cache` -will be appended to the cache key name. -- If `with_reactive_cache` is called when the result has already been cached, the -block will be called, yielding the cached value and the return value of the block -will be returned by `with_reactive_cache`. It will also reset the timeout of the -cache to the `reactive_cache_lifetime` value. -- If the result has not been cached as yet, `with_reactive_cache` will return nil. -It will also enqueue a background job, which will call `calculate_reactive_cache` -and cache the result. -- Once the background job has completed and the result is cached, the next call -to `with_reactive_cache` will pick up the cached value. -- In the example below, `data` is the cached value which is yielded to the block -given to `with_reactive_cache`. - - ```ruby - class Foo < ApplicationRecord - include ReactiveCaching - - def calculate_reactive_cache(param1, param2) - # Expensive operation here. The return value of this method is cached - end - - def result - with_reactive_cache(param1, param2) do |data| - # ... - end - end - end - ``` - -##### `#clear_reactive_cache!` (optional) - -- This method can be called when the cache needs to be expired/cleared. For example, -it can be called in an `after_save` callback in a model so that the cache is -cleared after the model is modified. -- This method should be called with the same parameters that are passed to -`with_reactive_cache` because the parameters are part of the cache key. - -##### `#without_reactive_cache` (optional) - -- This is a convenience method that can be used for debugging purposes. -- This method calls `calculate_reactive_cache` in the current process instead of -in a background worker. - -#### Configurable options - -There are some `class_attribute` options which can be tweaked. - -##### `self.reactive_cache_key` - -- The value of this attribute is the prefix to the `data` and `alive` cache key names. -The parameters passed to `with_reactive_cache` form the rest of the cache key names. -- By default, this key uses the model's name and the ID of the record. - - ```ruby - self.reactive_cache_key = -> (record) { [model_name.singular, record.id] } - ``` - -- The `data` and `alive` cache keys in this case will be `"ExampleModel:1:arg1:arg2"` -and `"ExampleModel:1:arg1:arg2:alive"` respectively, where `ExampleModel` is the -name of the model, `1` is the ID of the record, `arg1` and `arg2` are parameters -passed to `with_reactive_cache`. -- If you're including this concern in a service instead, you will need to override -the default by adding the following to your service: - - ```ruby - self.reactive_cache_key = ->(service) { [service.class.model_name.singular, service.project_id] } - ``` - - If your reactive_cache_key is exactly like the above, you can use the existing - `ReactiveService` concern instead. - -##### `self.reactive_cache_lease_timeout` - -- `ReactiveCaching` uses `Gitlab::ExclusiveLease` to ensure that the cache calculation -is never run concurrently by multiple workers. -- This attribute is the timeout for the `Gitlab::ExclusiveLease`. -- It defaults to 2 minutes, but can be overriden if a different timeout is required. - -```ruby -self.reactive_cache_lease_timeout = 2.minutes -``` - -##### `self.reactive_cache_refresh_interval` - -- This is the interval at which the cache is refreshed. -- It defaults to 1 minute. - -```ruby -self.reactive_cache_lease_timeout = 1.minute -``` - -##### `self.reactive_cache_lifetime` - -- This is the duration after which the cache will be cleared if there are no requests. -- The default is 10 minutes. If there are no requests for this cache value for 10 minutes, -the cache will expire. -- If the cache value is requested before it expires, the timeout of the cache will -be reset to `reactive_cache_lifetime`. - -```ruby -self.reactive_cache_lifetime = 10.minutes -``` - -##### `self.reactive_cache_worker_finder` - -- This is the method used by the background worker to find or generate the object on -which `calculate_reactive_cache` can be called. -- By default it uses the model primary key to find the object: - - ```ruby - self.reactive_cache_worker_finder = ->(id, *_args) do - find_by(primary_key => id) - end - ``` - -- The default behaviour can be overridden by defining a custom `reactive_cache_worker_finder`. - - ```ruby - class Foo < ApplicationRecord - include ReactiveCaching - - self.reactive_cache_worker_finder = ->(_id, *args) { from_cache(*args) } - - def self.from_cache(var1, var2) - # This method will be called by the background worker with "bar1" and - # "bar2" as arguments. - new(var1, var2) - end - - def initialize(var1, var2) - # ... - end - - def calculate_reactive_cache(var1, var2) - # Expensive operation here. The return value of this method is cached - end - - def result - with_reactive_cache("bar1", "bar2") do |data| - # ... - end - end - end - ``` - - - In this example, the primary key ID will be passed to `reactive_cache_worker_finder` - along with the parameters passed to `with_reactive_cache`. - - The custom `reactive_cache_worker_finder` calls `.from_cache` with the parameters - passed to `with_reactive_cache`. +Read the documentation on [`ReactiveCaching`](reactive_caching.md). diff --git a/doc/user/analytics/cycle_analytics.md b/doc/user/analytics/cycle_analytics.md index e0bb4e03c41..8d3eaade759 100644 --- a/doc/user/analytics/cycle_analytics.md +++ b/doc/user/analytics/cycle_analytics.md @@ -172,16 +172,23 @@ For example, if 30 days worth of data has been selected (for example, 2019-12-16 median line will represent the previous 30 days worth of data (2019-11-16 to 2019-12-16) as a metric to compare against. -### Enabling chart +### Disabling chart -By default, this chart is disabled for self-managed instances. To enable it, ask an -administrator with Rails console access to run the following: +This chart is enabled by default. If you have a self-managed instance, an +administrator can open a Rails console and disable it with the following command: ```ruby -Feature.enable(:cycle_analytics_scatterplot_enabled) +Feature.disable(:cycle_analytics_scatterplot_enabled) ``` -This chart is enabled by default on GitLab.com. +### Disabling chart median line + +This chart median line is enabled by default. If you have a self-managed instance, an +administrator can open a Rails console and disable it with the following command: + +```ruby +Feature.disable(:cycle_analytics_scatterplot_median_enabled) +``` ## Permissions diff --git a/doc/user/project/merge_requests/img/approvals_premium_mr_widget.png b/doc/user/project/merge_requests/img/approvals_premium_mr_widget.png Binary files differdeleted file mode 100644 index 2598cc71c33..00000000000 --- a/doc/user/project/merge_requests/img/approvals_premium_mr_widget.png +++ /dev/null diff --git a/doc/user/project/merge_requests/img/approvals_premium_mr_widget_v12_7.png b/doc/user/project/merge_requests/img/approvals_premium_mr_widget_v12_7.png Binary files differnew file mode 100644 index 00000000000..f9348b0eefc --- /dev/null +++ b/doc/user/project/merge_requests/img/approvals_premium_mr_widget_v12_7.png diff --git a/doc/user/project/merge_requests/img/mr_approvals_by_code_owners_v12_4.png b/doc/user/project/merge_requests/img/mr_approvals_by_code_owners_v12_4.png Binary files differdeleted file mode 100644 index c704129685f..00000000000 --- a/doc/user/project/merge_requests/img/mr_approvals_by_code_owners_v12_4.png +++ /dev/null diff --git a/doc/user/project/merge_requests/img/mr_approvals_by_code_owners_v12_7.png b/doc/user/project/merge_requests/img/mr_approvals_by_code_owners_v12_7.png Binary files differnew file mode 100644 index 00000000000..c2e5714e78d --- /dev/null +++ b/doc/user/project/merge_requests/img/mr_approvals_by_code_owners_v12_7.png diff --git a/doc/user/project/merge_requests/merge_request_approvals.md b/doc/user/project/merge_requests/merge_request_approvals.md index fa30f4e2eb4..d7bb2dd16f9 100644 --- a/doc/user/project/merge_requests/merge_request_approvals.md +++ b/doc/user/project/merge_requests/merge_request_approvals.md @@ -74,9 +74,9 @@ To enable this merge request approval rule: 1. Navigate to your project's **Settings > General** and expand **Merge request approvals**. -1. Locate **All members with Developer role or higher and code owners (if any)** and click **Edit** to choose the number of approvals required. +1. Locate **Any eligible user** and choose the number of approvals required. - + Once set, merge requests can only be merged once approved by the number of approvals you've set. GitLab will accept approvals from @@ -145,7 +145,7 @@ a rule is already defined. When an [eligible approver](#eligible-approvers) approves a merge request, it will reduce the number of approvals left for all rules that the approver belongs to. - + ## Adding or removing an approval diff --git a/lib/gitlab/auth/current_user_mode.rb b/lib/gitlab/auth/current_user_mode.rb index 1ef95c03cfc..cb39baaa6cc 100644 --- a/lib/gitlab/auth/current_user_mode.rb +++ b/lib/gitlab/auth/current_user_mode.rb @@ -10,54 +10,12 @@ module Gitlab class CurrentUserMode NotRequestedError = Class.new(StandardError) - # RequestStore entries - CURRENT_REQUEST_BYPASS_SESSION_ADMIN_ID_RS_KEY = { res: :current_user_mode, data: :bypass_session_admin_id }.freeze - CURRENT_REQUEST_ADMIN_MODE_USER_RS_KEY = { res: :current_user_mode, data: :current_admin }.freeze - - # SessionStore entries SESSION_STORE_KEY = :current_user_mode - ADMIN_MODE_START_TIME_KEY = :admin_mode - ADMIN_MODE_REQUESTED_TIME_KEY = :admin_mode_requested + ADMIN_MODE_START_TIME_KEY = 'admin_mode' + ADMIN_MODE_REQUESTED_TIME_KEY = 'admin_mode_requested' MAX_ADMIN_MODE_TIME = 6.hours ADMIN_MODE_REQUESTED_GRACE_PERIOD = 5.minutes - class << self - # Admin mode activation requires storing a flag in the user session. Using this - # method when scheduling jobs in Sidekiq will bypass the session check for a - # user that was already in admin mode - def bypass_session!(admin_id) - Gitlab::SafeRequestStore[CURRENT_REQUEST_BYPASS_SESSION_ADMIN_ID_RS_KEY] = admin_id - - Gitlab::AppLogger.debug("Bypassing session in admin mode for: #{admin_id}") - - yield - ensure - Gitlab::SafeRequestStore.delete(CURRENT_REQUEST_BYPASS_SESSION_ADMIN_ID_RS_KEY) - end - - def bypass_session_admin_id - Gitlab::SafeRequestStore[CURRENT_REQUEST_BYPASS_SESSION_ADMIN_ID_RS_KEY] - end - - # Store in the current request the provided user model (only if in admin mode) - # and yield - def with_current_admin(admin) - return yield unless self.new(admin).admin_mode? - - Gitlab::SafeRequestStore[CURRENT_REQUEST_ADMIN_MODE_USER_RS_KEY] = admin - - Gitlab::AppLogger.debug("Admin mode active for: #{admin.username}") - - yield - ensure - Gitlab::SafeRequestStore.delete(CURRENT_REQUEST_ADMIN_MODE_USER_RS_KEY) - end - - def current_admin - Gitlab::SafeRequestStore[CURRENT_REQUEST_ADMIN_MODE_USER_RS_KEY] - end - end - def initialize(user) @user = user end @@ -84,7 +42,7 @@ module Gitlab raise NotRequestedError unless admin_mode_requested? - reset_request_store_cache_entries + reset_request_store current_session_data[ADMIN_MODE_REQUESTED_TIME_KEY] = nil current_session_data[ADMIN_MODE_START_TIME_KEY] = Time.now @@ -97,7 +55,7 @@ module Gitlab def disable_admin_mode! return unless user&.admin? - reset_request_store_cache_entries + reset_request_store current_session_data[ADMIN_MODE_REQUESTED_TIME_KEY] = nil current_session_data[ADMIN_MODE_START_TIME_KEY] = nil @@ -106,7 +64,7 @@ module Gitlab def request_admin_mode! return unless user&.admin? - reset_request_store_cache_entries + reset_request_store current_session_data[ADMIN_MODE_REQUESTED_TIME_KEY] = Time.now end @@ -115,12 +73,10 @@ module Gitlab attr_reader :user - # RequestStore entry to cache #admin_mode? result def admin_mode_rs_key @admin_mode_rs_key ||= { res: :current_user_mode, user: user.id, method: :admin_mode? } end - # RequestStore entry to cache #admin_mode_requested? result def admin_mode_requested_rs_key @admin_mode_requested_rs_key ||= { res: :current_user_mode, user: user.id, method: :admin_mode_requested? } end @@ -130,7 +86,6 @@ module Gitlab end def any_session_with_admin_mode? - return true if bypass_session? return true if current_session_data.initiated? && current_session_data[ADMIN_MODE_START_TIME_KEY].to_i > MAX_ADMIN_MODE_TIME.ago.to_i all_sessions.any? do |session| @@ -148,11 +103,7 @@ module Gitlab current_session_data[ADMIN_MODE_REQUESTED_TIME_KEY].to_i > ADMIN_MODE_REQUESTED_GRACE_PERIOD.ago.to_i end - def bypass_session? - user&.id && user.id == self.class.bypass_session_admin_id - end - - def reset_request_store_cache_entries + def reset_request_store Gitlab::SafeRequestStore.delete(admin_mode_rs_key) Gitlab::SafeRequestStore.delete(admin_mode_requested_rs_key) end diff --git a/lib/gitlab/diff/highlight_cache.rb b/lib/gitlab/diff/highlight_cache.rb index 0a8fbb9a673..0cb8020bacb 100644 --- a/lib/gitlab/diff/highlight_cache.rb +++ b/lib/gitlab/diff/highlight_cache.rb @@ -93,6 +93,8 @@ module Gitlab # redis.expire(key, EXPIRATION) end + + record_metrics(redis.memory("USAGE", key)) end # Subsequent read_file calls would need the latest cache. @@ -101,6 +103,10 @@ module Gitlab clear_memoization(:cacheable_files) end + def record_metrics(memory_usage) + self.class.gitlab_redis_diff_caching_memory_usage_bytes.observe({}, memory_usage) + end + def file_paths strong_memoize(:file_paths) do diff_files.collect(&:file_path) diff --git a/lib/gitlab/sidekiq_middleware.rb b/lib/gitlab/sidekiq_middleware.rb index 6c27213df49..439d45b7a14 100644 --- a/lib/gitlab/sidekiq_middleware.rb +++ b/lib/gitlab/sidekiq_middleware.rb @@ -17,7 +17,6 @@ module Gitlab chain.add Gitlab::SidekiqMiddleware::BatchLoader chain.add Labkit::Middleware::Sidekiq::Server chain.add Gitlab::SidekiqMiddleware::InstrumentationLogger - chain.add Gitlab::SidekiqMiddleware::AdminMode::Server chain.add Gitlab::SidekiqStatus::ServerMiddleware chain.add Gitlab::SidekiqMiddleware::WorkerContext::Server end @@ -32,7 +31,6 @@ module Gitlab chain.add Gitlab::SidekiqMiddleware::ClientMetrics chain.add Gitlab::SidekiqMiddleware::WorkerContext::Client # needs to be before the Labkit middleware chain.add Labkit::Middleware::Sidekiq::Client - chain.add Gitlab::SidekiqMiddleware::AdminMode::Client end end end diff --git a/lib/gitlab/sidekiq_middleware/admin_mode/client.rb b/lib/gitlab/sidekiq_middleware/admin_mode/client.rb deleted file mode 100644 index e227ee654ee..00000000000 --- a/lib/gitlab/sidekiq_middleware/admin_mode/client.rb +++ /dev/null @@ -1,34 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module SidekiqMiddleware - module AdminMode - # Checks if admin mode is enabled for the request creating the sidekiq job - # by examining if admin mode has been enabled for the user - # If enabled then it injects a job field that persists through the job execution - class Client - def call(_worker_class, job, _queue, _redis_pool) - return yield unless Feature.enabled?(:user_mode_in_session) - - # Admin mode enabled in the original request or in a nested sidekiq job - admin_mode_user_id = find_admin_user_id - - if admin_mode_user_id - job['admin_mode_user_id'] ||= admin_mode_user_id - - Gitlab::AppLogger.debug("AdminMode::Client injected admin mode for job: #{job.inspect}") - end - - yield - end - - private - - def find_admin_user_id - Gitlab::Auth::CurrentUserMode.current_admin&.id || - Gitlab::Auth::CurrentUserMode.bypass_session_admin_id - end - end - end - end -end diff --git a/lib/gitlab/sidekiq_middleware/admin_mode/server.rb b/lib/gitlab/sidekiq_middleware/admin_mode/server.rb deleted file mode 100644 index 6366867a0fa..00000000000 --- a/lib/gitlab/sidekiq_middleware/admin_mode/server.rb +++ /dev/null @@ -1,24 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module SidekiqMiddleware - module AdminMode - class Server - def call(_worker, job, _queue) - return yield unless Feature.enabled?(:user_mode_in_session) - - admin_mode_user_id = job['admin_mode_user_id'] - - # Do not bypass session if this job was not enabled with admin mode on - return yield unless admin_mode_user_id - - Gitlab::Auth::CurrentUserMode.bypass_session!(admin_mode_user_id) do - Gitlab::AppLogger.debug("AdminMode::Server bypasses session for admin mode in job: #{job.inspect}") - - yield - end - end - end - end - end -end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 67362b2efcc..7bf7cc9051f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -7137,6 +7137,21 @@ msgstr "" msgid "Enable/disable your service desk. %{link_start}Learn more about service desk%{link_end}." msgstr "" +msgid "EnableReviewApp|%{stepStart}Step 1%{stepEnd}. Ensure you have Kubernetes set up and have a base domain for your %{linkStart}cluster%{linkEnd}." +msgstr "" + +msgid "EnableReviewApp|%{stepStart}Step 2%{stepEnd}. Copy the following snippet:" +msgstr "" + +msgid "EnableReviewApp|%{stepStart}Step 3%{stepEnd}. Add it to the project %{linkStart}gitlab-ci.yml%{linkEnd} file." +msgstr "" + +msgid "EnableReviewApp|Close" +msgstr "" + +msgid "EnableReviewApp|Copy snippet text" +msgstr "" + msgid "Enabled" msgstr "" @@ -7311,6 +7326,9 @@ msgstr "" msgid "Environments|Deployment" msgstr "" +msgid "Environments|Enable review app" +msgstr "" + msgid "Environments|Environment" msgstr "" @@ -11529,6 +11547,9 @@ msgstr "" msgid "Manage two-factor authentication" msgstr "" +msgid "Manage your license" +msgstr "" + msgid "Manifest" msgstr "" @@ -16189,6 +16210,9 @@ msgstr "" msgid "Review time is defined as the time it takes from first comment until merged." msgstr "" +msgid "ReviewApp|Enable Review App" +msgstr "" + msgid "Reviewing" msgstr "" @@ -19205,6 +19229,9 @@ msgstr "" msgid "This GitLab instance does not provide any shared Runners yet. Instance administrators can register shared Runners in the admin area." msgstr "" +msgid "This GitLab instance is licensed at the %{insufficient_license} tier. Geo is only available for users who have at least a Premium license." +msgstr "" + msgid "This action can lead to data loss. To prevent accidental actions we ask you to confirm your intention." msgstr "" @@ -21868,9 +21895,6 @@ msgstr "" msgid "You need a different license to enable FileLocks feature" msgstr "" -msgid "You need a different license to use Geo replication." -msgstr "" - msgid "You need git-lfs version %{min_git_lfs_version} (or greater) to continue. Please visit https://git-lfs.github.com" msgstr "" diff --git a/qa/qa/page/project/pipeline/show.rb b/qa/qa/page/project/pipeline/show.rb index 45fffbf6000..1003b828a32 100644 --- a/qa/qa/page/project/pipeline/show.rb +++ b/qa/qa/page/project/pipeline/show.rb @@ -30,9 +30,9 @@ module QA::Page element :pipeline_badges end - def running? + def running?(wait: 0) within('.ci-header-container') do - page.has_content?('running') + page.has_content?('running', wait: wait) end end diff --git a/qa/qa/specs/features/browser_ui/4_verify/pipeline/create_and_process_pipeline_spec.rb b/qa/qa/specs/features/browser_ui/4_verify/pipeline/create_and_process_pipeline_spec.rb index d4853a7bcf3..3e000c6381e 100644 --- a/qa/qa/specs/features/browser_ui/4_verify/pipeline/create_and_process_pipeline_spec.rb +++ b/qa/qa/specs/features/browser_ui/4_verify/pipeline/create_and_process_pipeline_spec.rb @@ -1,78 +1,74 @@ # frozen_string_literal: true module QA - context 'Verify', :orchestrated, :docker do + context 'Verify', :docker do describe 'Pipeline creation and processing' do let(:executor) { "qa-runner-#{Time.now.to_i}" } - after do - Service::DockerRun::GitlabRunner.new(executor).remove! - end - - it 'users creates a pipeline which gets processed' do - Flow::Login.sign_in - - project = Resource::Project.fabricate! do |project| - project.name = 'project-with-pipelines' - project.description = 'Project with CI/CD Pipelines.' + let(:project) do + Resource::Project.fabricate_via_api! do |project| + project.name = 'project-with-pipeline' end + end + before do Resource::Runner.fabricate! do |runner| runner.project = project runner.name = executor - runner.tags = %w[qa test] + runner.tags = [executor] end + end - Resource::Repository::ProjectPush.fabricate! do |push| - push.project = project - push.file_name = '.gitlab-ci.yml' - push.commit_message = 'Add .gitlab-ci.yml' - push.file_content = <<~EOF - test-success: - tags: - - qa - - test - script: echo 'OK' - - test-failure: - tags: - - qa - - test - script: - - echo 'FAILURE' - - exit 1 - - test-tags: - tags: - - qa - - docker - script: echo 'NOOP' + after do + Service::DockerRun::GitlabRunner.new(executor).remove! + end - test-artifacts: - tags: - - qa - - test - script: mkdir my-artifacts; echo "CONTENTS" > my-artifacts/artifact.txt - artifacts: - paths: - - my-artifacts/ - EOF - end.project.visit! + it 'users creates a pipeline which gets processed', :smoke do + Flow::Login.sign_in - expect(page).to have_content('Add .gitlab-ci.yml') + Resource::Repository::Commit.fabricate_via_api! do |commit| + commit.project = project + commit.commit_message = 'Add .gitlab-ci.yml' + commit.add_files( + [ + { + file_path: '.gitlab-ci.yml', + content: <<~YAML + test-success: + tags: + - #{executor} + script: echo 'OK' - Page::Project::Menu.perform(&:click_ci_cd_pipelines) + test-failure: + tags: + - #{executor} + script: + - echo 'FAILURE' + - exit 1 - expect(page).to have_content('All 1') - expect(page).to have_content('Add .gitlab-ci.yml') + test-tags: + tags: + - invalid + script: echo 'NOOP' - puts 'Waiting for the runner to process the pipeline' - sleep 15 # Runner should process all jobs within 15 seconds. + test-artifacts: + tags: + - #{executor} + script: mkdir my-artifacts; echo "CONTENTS" > my-artifacts/artifact.txt + artifacts: + paths: + - my-artifacts/ + YAML + } + ] + ) + end.project.visit! + Page::Project::Menu.perform(&:click_ci_cd_pipelines) Page::Project::Pipeline::Index.perform(&:click_on_latest_pipeline) Page::Project::Pipeline::Show.perform do |pipeline| - expect(pipeline).to be_running + expect(pipeline).to be_running(wait: 30) expect(pipeline).to have_build('test-success', status: :success) expect(pipeline).to have_build('test-failure', status: :failed) expect(pipeline).to have_build('test-tags', status: :pending) diff --git a/spec/features/admin/admin_mode/workers_spec.rb b/spec/features/admin/admin_mode/workers_spec.rb deleted file mode 100644 index e33c9d7e64c..00000000000 --- a/spec/features/admin/admin_mode/workers_spec.rb +++ /dev/null @@ -1,77 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -# Test an operation that triggers background jobs requiring administrative rights -describe 'Admin mode for workers', :do_not_mock_admin_mode, :request_store, :clean_gitlab_redis_shared_state do - let(:user) { create(:user) } - let(:user_to_delete) { create(:user) } - - before do - add_sidekiq_middleware - - sign_in(user) - end - - context 'as a regular user' do - it 'cannot delete user' do - visit admin_user_path(user_to_delete) - - expect(page).to have_gitlab_http_status(:not_found) - end - end - - context 'as an admin user' do - let(:user) { create(:admin) } - - context 'when admin mode disabled' do - it 'cannot delete user', :js do - visit admin_user_path(user_to_delete) - - expect(page).to have_content('Re-authentication required') - end - end - - context 'when admin mode enabled', :delete do - before do - gitlab_enable_admin_mode_sign_in(user) - end - - it 'can delete user', :js do - visit admin_user_path(user_to_delete) - click_button 'Delete user' - - page.within '.modal-dialog' do - find("input[name='username']").send_keys(user_to_delete.name) - click_button 'Delete user' - - wait_for_requests - end - - expect(page).to have_content('The user is being deleted.') - - # Perform jobs while logged out so that admin mode is only enabled in job metadata - execute_jobs_signed_out(user) - - visit admin_user_path(user_to_delete) - - expect(page).to have_title('Not Found') - end - end - end - - def add_sidekiq_middleware - Sidekiq::Testing.server_middleware do |chain| - chain.add Gitlab::SidekiqMiddleware::AdminMode::Server - end - end - - def execute_jobs_signed_out(user) - gitlab_sign_out - - Sidekiq::Worker.drain_all - - sign_in(user) - gitlab_enable_admin_mode_sign_in(user) - end -end diff --git a/spec/features/admin/admin_uses_repository_checks_spec.rb b/spec/features/admin/admin_uses_repository_checks_spec.rb index 954773e766d..3e8197588ed 100644 --- a/spec/features/admin/admin_uses_repository_checks_spec.rb +++ b/spec/features/admin/admin_uses_repository_checks_spec.rb @@ -2,64 +2,46 @@ require 'spec_helper' -describe 'Admin uses repository checks', :request_store, :clean_gitlab_redis_shared_state, :do_not_mock_admin_mode do +describe 'Admin uses repository checks' do include StubENV - let(:admin) { create(:admin) } - before do stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') - sign_in(admin) + sign_in(create(:admin)) end - context 'when admin mode is disabled' do - it 'admin project page requires admin mode' do - project = create(:project) - visit_admin_project_page(project) + it 'to trigger a single check' do + project = create(:project) + visit_admin_project_page(project) - expect(page).not_to have_css('.repository-check') - expect(page).to have_content('Enter Admin Mode') + page.within('.repository-check') do + click_button 'Trigger repository check' end - end - - context 'when admin mode is enabled' do - before do - gitlab_enable_admin_mode_sign_in(admin) - end - - it 'to trigger a single check', :js do - project = create(:project) - visit_admin_project_page(project) - page.within('.repository-check') do - click_button 'Trigger repository check' - end - - expect(page).to have_content('Repository check was triggered') - end + expect(page).to have_content('Repository check was triggered') + end - it 'to see a single failed repository check', :js do - project = create(:project) - project.update_columns( - last_repository_check_failed: true, - last_repository_check_at: Time.now - ) - visit_admin_project_page(project) + it 'to see a single failed repository check', :js do + project = create(:project) + project.update_columns( + last_repository_check_failed: true, + last_repository_check_at: Time.now + ) + visit_admin_project_page(project) - page.within('.alert') do - expect(page.text).to match(/Last repository check \(just now\) failed/) - end + page.within('.alert') do + expect(page.text).to match(/Last repository check \(just now\) failed/) end + end - it 'to clear all repository checks', :js do - visit repository_admin_application_settings_path + it 'to clear all repository checks', :js do + visit repository_admin_application_settings_path - expect(RepositoryCheck::ClearWorker).to receive(:perform_async) + expect(RepositoryCheck::ClearWorker).to receive(:perform_async) - accept_confirm { find(:link, 'Clear all repository checks').send_keys(:return) } + accept_confirm { find(:link, 'Clear all repository checks').send_keys(:return) } - expect(page).to have_content('Started asynchronous removal of all repository check states.') - end + expect(page).to have_content('Started asynchronous removal of all repository check states.') end def visit_admin_project_page(project) diff --git a/spec/frontend/environments/enable_review_app_button_spec.js b/spec/frontend/environments/enable_review_app_button_spec.js new file mode 100644 index 00000000000..5549a1737fc --- /dev/null +++ b/spec/frontend/environments/enable_review_app_button_spec.js @@ -0,0 +1,31 @@ +import { shallowMount, mount } from '@vue/test-utils'; +import ModalCopyButton from '~/vue_shared/components/modal_copy_button.vue'; +import EnableReviewAppButton from '~/environments/components/enable_review_app_button.vue'; + +describe('Enable Review App Button', () => { + let wrapper; + + afterEach(() => { + wrapper.destroy(); + }); + + describe('renders button with text', () => { + beforeEach(() => { + wrapper = mount(EnableReviewAppButton); + }); + + it('renders Enable Review text', () => { + expect(wrapper.text()).toBe('Enable review app'); + }); + }); + + describe('renders the modal', () => { + beforeEach(() => { + wrapper = shallowMount(EnableReviewAppButton); + }); + + it('renders the copyToClipboard button', () => { + expect(wrapper.find(ModalCopyButton).exists()).toBe(true); + }); + }); +}); diff --git a/spec/javascripts/environments/environments_app_spec.js b/spec/javascripts/environments/environments_app_spec.js index 75526c2ba74..6c05b609923 100644 --- a/spec/javascripts/environments/environments_app_spec.js +++ b/spec/javascripts/environments/environments_app_spec.js @@ -55,6 +55,26 @@ describe('Environment', () => { "You don't have any environments right now", ); }); + + describe('when it is possible to enable a review app', () => { + beforeEach(done => { + mock + .onGet(mockData.endpoint) + .reply(200, { environments: [], review_app: { can_setup_review_app: true } }); + + component = mountComponent(EnvironmentsComponent, mockData); + + setTimeout(() => { + done(); + }, 0); + }); + + it('should render the enable review app button', () => { + expect(component.$el.querySelector('.js-enable-review-app-button').textContent).toContain( + 'Enable review app', + ); + }); + }); }); describe('with paginated environments', () => { diff --git a/spec/lib/gitlab/auth/current_user_mode_spec.rb b/spec/lib/gitlab/auth/current_user_mode_spec.rb index 7c2fdac6c25..3b3db0f7315 100644 --- a/spec/lib/gitlab/auth/current_user_mode_spec.rb +++ b/spec/lib/gitlab/auth/current_user_mode_spec.rb @@ -2,10 +2,10 @@ require 'spec_helper' -describe Gitlab::Auth::CurrentUserMode, :do_not_mock_admin_mode, :request_store do +describe Gitlab::Auth::CurrentUserMode, :do_not_mock_admin_mode do include_context 'custom session' - let(:user) { build_stubbed(:user) } + let(:user) { build(:user) } subject { described_class.new(user) } @@ -13,66 +13,54 @@ describe Gitlab::Auth::CurrentUserMode, :do_not_mock_admin_mode, :request_store allow(ActiveSession).to receive(:list_sessions).with(user).and_return([session]) end - shared_examples 'admin mode cannot be enabled' do - it 'is false by default' do - expect(subject.admin_mode?).to be(false) - end - - it 'cannot be enabled with a valid password' do - subject.enable_admin_mode!(password: user.password) - - expect(subject.admin_mode?).to be(false) - end - - it 'cannot be enabled with an invalid password' do - subject.enable_admin_mode!(password: nil) - - expect(subject.admin_mode?).to be(false) - end + describe '#admin_mode?', :request_store do + context 'when the user is a regular user' do + it 'is false by default' do + expect(subject.admin_mode?).to be(false) + end - it 'cannot be enabled with empty params' do - subject.enable_admin_mode! + it 'cannot be enabled with a valid password' do + subject.enable_admin_mode!(password: user.password) - expect(subject.admin_mode?).to be(false) - end + expect(subject.admin_mode?).to be(false) + end - it 'disable has no effect' do - subject.enable_admin_mode! - subject.disable_admin_mode! + it 'cannot be enabled with an invalid password' do + subject.enable_admin_mode!(password: nil) - expect(subject.admin_mode?).to be(false) - end + expect(subject.admin_mode?).to be(false) + end - context 'skipping password validation' do - it 'cannot be enabled with a valid password' do - subject.enable_admin_mode!(password: user.password, skip_password_validation: true) + it 'cannot be enabled with empty params' do + subject.enable_admin_mode! expect(subject.admin_mode?).to be(false) end - it 'cannot be enabled with an invalid password' do - subject.enable_admin_mode!(skip_password_validation: true) + it 'disable has no effect' do + subject.enable_admin_mode! + subject.disable_admin_mode! expect(subject.admin_mode?).to be(false) end - end - end - describe '#admin_mode?' do - context 'when the user is a regular user' do - it_behaves_like 'admin mode cannot be enabled' + context 'skipping password validation' do + it 'cannot be enabled with a valid password' do + subject.enable_admin_mode!(password: user.password, skip_password_validation: true) - context 'bypassing session' do - it_behaves_like 'admin mode cannot be enabled' do - around do |example| - described_class.bypass_session!(user.id) { example.run } - end + expect(subject.admin_mode?).to be(false) + end + + it 'cannot be enabled with an invalid password' do + subject.enable_admin_mode!(skip_password_validation: true) + + expect(subject.admin_mode?).to be(false) end end end context 'when the user is an admin' do - let(:user) { build_stubbed(:user, :admin) } + let(:user) { build(:user, :admin) } context 'when admin mode not requested' do it 'is false by default' do @@ -160,36 +148,11 @@ describe Gitlab::Auth::CurrentUserMode, :do_not_mock_admin_mode, :request_store end end end - - context 'bypassing session' do - it 'is active by default' do - described_class.bypass_session!(user.id) do - expect(subject.admin_mode?).to be(true) - end - end - - it 'enable has no effect' do - described_class.bypass_session!(user.id) do - subject.request_admin_mode! - subject.enable_admin_mode!(password: user.password) - - expect(subject.admin_mode?).to be(true) - end - end - - it 'disable has no effect' do - described_class.bypass_session!(user.id) do - subject.disable_admin_mode! - - expect(subject.admin_mode?).to be(true) - end - end - end end end describe '#enable_admin_mode!' do - let(:user) { build_stubbed(:user, :admin) } + let(:user) { build(:user, :admin) } it 'creates a timestamp in the session' do subject.request_admin_mode! @@ -200,7 +163,7 @@ describe Gitlab::Auth::CurrentUserMode, :do_not_mock_admin_mode, :request_store end describe '#enable_sessionless_admin_mode!' do - let(:user) { build_stubbed(:user, :admin) } + let(:user) { build(:user, :admin) } it 'enabled admin mode without password' do subject.enable_sessionless_admin_mode! @@ -210,7 +173,7 @@ describe Gitlab::Auth::CurrentUserMode, :do_not_mock_admin_mode, :request_store end describe '#disable_admin_mode!' do - let(:user) { build_stubbed(:user, :admin) } + let(:user) { build(:user, :admin) } it 'sets the session timestamp to nil' do subject.request_admin_mode! @@ -220,73 +183,6 @@ describe Gitlab::Auth::CurrentUserMode, :do_not_mock_admin_mode, :request_store end end - describe '.bypass_session!' do - context 'with a regular user' do - it 'admin mode is false' do - described_class.bypass_session!(user.id) do - expect(subject.admin_mode?).to be(false) - expect(described_class.bypass_session_admin_id).to be(user.id) - end - - expect(described_class.bypass_session_admin_id).to be_nil - end - end - - context 'with an admin user' do - let(:user) { build_stubbed(:user, :admin) } - - it 'admin mode is true' do - described_class.bypass_session!(user.id) do - expect(subject.admin_mode?).to be(true) - expect(described_class.bypass_session_admin_id).to be(user.id) - end - - expect(described_class.bypass_session_admin_id).to be_nil - end - end - end - - describe '.with_current_request_admin_mode' do - context 'with a regular user' do - it 'user is not available inside nor outside the yielded block' do - described_class.with_current_admin(user) do - expect(described_class.current_admin).to be_nil - end - - expect(described_class.bypass_session_admin_id).to be_nil - end - end - - context 'with an admin user' do - let(:user) { build_stubbed(:user, :admin) } - - context 'admin mode is disabled' do - it 'user is not available inside nor outside the yielded block' do - described_class.with_current_admin(user) do - expect(described_class.current_admin).to be_nil - end - - expect(described_class.bypass_session_admin_id).to be_nil - end - end - - context 'admin mode is enabled' do - before do - subject.request_admin_mode! - subject.enable_admin_mode!(password: user.password) - end - - it 'user is available only inside the yielded block' do - described_class.with_current_admin(user) do - expect(described_class.current_admin).to be(user) - end - - expect(described_class.current_admin).to be_nil - end - end - end - end - def expected_session_entry(value_matcher) { Gitlab::Auth::CurrentUserMode::SESSION_STORE_KEY => a_hash_including( diff --git a/spec/lib/gitlab/diff/highlight_cache_spec.rb b/spec/lib/gitlab/diff/highlight_cache_spec.rb index 218c393c409..c1ec97ae88c 100644 --- a/spec/lib/gitlab/diff/highlight_cache_spec.rb +++ b/spec/lib/gitlab/diff/highlight_cache_spec.rb @@ -101,6 +101,13 @@ describe Gitlab::Diff::HighlightCache, :clean_gitlab_redis_cache do let(:paths) { merge_request.diffs.raw_diff_files.select(&:text?).map(&:file_path) } end + it 'updates memory usage metrics' do + expect(described_class.gitlab_redis_diff_caching_memory_usage_bytes) + .to receive(:observe).and_call_original + + cache.send(:write_to_redis_hash, diff_hash) + end + context 'different diff_collections for the same diffable' do before do cache.write_if_empty diff --git a/spec/lib/gitlab/sidekiq_middleware/admin_mode/client_spec.rb b/spec/lib/gitlab/sidekiq_middleware/admin_mode/client_spec.rb deleted file mode 100644 index f6449bae8c3..00000000000 --- a/spec/lib/gitlab/sidekiq_middleware/admin_mode/client_spec.rb +++ /dev/null @@ -1,94 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe Gitlab::SidekiqMiddleware::AdminMode::Client, :do_not_mock_admin_mode, :request_store do - include AdminModeHelper - - let(:worker) do - Class.new do - def perform; end - end - end - - let(:job) { {} } - let(:queue) { :test } - - it 'yields block' do - expect do |b| - subject.call(worker, job, queue, nil, &b) - end.to yield_control.once - end - - context 'user is a regular user' do - it 'no admin mode field in payload' do - subject.call(worker, job, queue, nil) { nil } - - expect(job).not_to include('admin_mode_user_id') - end - end - - context 'user is an administrator' do - let(:admin) { create(:admin) } - - context 'admin mode disabled' do - it 'no admin mode field in payload' do - subject.call(worker, job, queue, nil) { nil } - - expect(job).not_to include('admin_mode_user_id') - end - end - - context 'admin mode enabled' do - before do - enable_admin_mode!(admin) - end - - context 'when sidekiq required context not set' do - it 'no admin mode field in payload' do - subject.call(worker, job, queue, nil) { nil } - - expect(job).not_to include('admin_mode_user_id') - end - end - - context 'when user stored in current request' do - it 'has admin mode field in payload' do - Gitlab::Auth::CurrentUserMode.with_current_admin(admin) do - subject.call(worker, job, queue, nil) { nil } - - expect(job).to include('admin_mode_user_id' => admin.id) - end - end - end - - context 'when bypassing session' do - it 'has admin mode field in payload' do - Gitlab::Auth::CurrentUserMode.bypass_session!(admin.id) do - subject.call(worker, job, queue, nil) { nil } - - expect(job).to include('admin_mode_user_id' => admin.id) - end - end - end - end - end - - context 'admin mode feature disabled' do - before do - stub_feature_flags(user_mode_in_session: false) - end - - it 'yields block' do - expect do |b| - subject.call(worker, job, queue, nil, &b) - end.to yield_control.once - end - - it 'no admin mode field in payload' do - subject.call(worker, job, queue, nil) { nil } - - expect(job).not_to include('admin_mode_user_id') - end - end -end diff --git a/spec/lib/gitlab/sidekiq_middleware/admin_mode/server_spec.rb b/spec/lib/gitlab/sidekiq_middleware/admin_mode/server_spec.rb deleted file mode 100644 index 60475f0e403..00000000000 --- a/spec/lib/gitlab/sidekiq_middleware/admin_mode/server_spec.rb +++ /dev/null @@ -1,72 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe Gitlab::SidekiqMiddleware::AdminMode::Server, :do_not_mock_admin_mode, :request_store do - include AdminModeHelper - - let(:worker) do - Class.new do - def perform; end - end - end - - let(:job) { {} } - let(:queue) { :test } - - it 'yields block' do - expect do |b| - subject.call(worker, job, queue, &b) - end.to yield_control.once - end - - context 'job has no admin mode field' do - it 'session is not bypassed' do - subject.call(worker, job, queue) do - expect(Gitlab::Auth::CurrentUserMode.bypass_session_admin_id).to be_nil - end - end - end - - context 'job has admin mode field' do - let(:admin) { create(:admin) } - - context 'nil admin mode id' do - let(:job) { { 'admin_mode_user_id' => nil } } - - it 'session is not bypassed' do - subject.call(worker, job, queue) do - expect(Gitlab::Auth::CurrentUserMode.bypass_session_admin_id).to be_nil - end - end - end - - context 'valid admin mode id' do - let(:job) { { 'admin_mode_user_id' => admin.id } } - - it 'session is bypassed' do - subject.call(worker, job, queue) do - expect(Gitlab::Auth::CurrentUserMode.bypass_session_admin_id).to be(admin.id) - end - end - end - end - - context 'admin mode feature disabled' do - before do - stub_feature_flags(user_mode_in_session: false) - end - - it 'yields block' do - expect do |b| - subject.call(worker, job, queue, &b) - end.to yield_control.once - end - - it 'session is not bypassed' do - subject.call(worker, job, queue) do - expect(Gitlab::Auth::CurrentUserMode.bypass_session_admin_id).to be_nil - end - end - end -end diff --git a/spec/lib/gitlab/sidekiq_middleware_spec.rb b/spec/lib/gitlab/sidekiq_middleware_spec.rb index 19242d25e27..e8dcbbd2ee1 100644 --- a/spec/lib/gitlab/sidekiq_middleware_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware_spec.rb @@ -45,8 +45,7 @@ describe Gitlab::SidekiqMiddleware do Gitlab::SidekiqMiddleware::ArgumentsLogger, Gitlab::SidekiqMiddleware::MemoryKiller, Gitlab::SidekiqMiddleware::RequestStoreMiddleware, - Gitlab::SidekiqMiddleware::WorkerContext::Server, - Gitlab::SidekiqMiddleware::AdminMode::Server + Gitlab::SidekiqMiddleware::WorkerContext::Server ] end let(:enabled_sidekiq_middlewares) { all_sidekiq_middlewares - disabled_sidekiq_middlewares } @@ -116,8 +115,7 @@ describe Gitlab::SidekiqMiddleware do Gitlab::SidekiqStatus::ClientMiddleware, Gitlab::SidekiqMiddleware::ClientMetrics, Gitlab::SidekiqMiddleware::WorkerContext::Client, - Labkit::Middleware::Sidekiq::Client, - Gitlab::SidekiqMiddleware::AdminMode::Client + Labkit::Middleware::Sidekiq::Client ] end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 610b3e47c11..d14de7e99de 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2985,9 +2985,9 @@ describe User, :do_not_mock_admin_mode do end end - describe '#can_read_all_resources?', :request_store do + describe '#can_read_all_resources?' do it 'returns false for regular user' do - user = build_stubbed(:user) + user = build(:user) expect(user.can_read_all_resources?).to be_falsy end @@ -2995,7 +2995,7 @@ describe User, :do_not_mock_admin_mode do context 'for admin user' do include_context 'custom session' - let(:user) { build_stubbed(:user, :admin) } + let(:user) { build(:user, :admin) } context 'when admin mode is disabled' do it 'returns false' do diff --git a/spec/policies/base_policy_spec.rb b/spec/policies/base_policy_spec.rb index ae5af9e0f29..81aee4cfcac 100644 --- a/spec/policies/base_policy_spec.rb +++ b/spec/policies/base_policy_spec.rb @@ -23,8 +23,8 @@ describe BasePolicy, :do_not_mock_admin_mode do end describe 'read cross project' do - let(:current_user) { build_stubbed(:user) } - let(:user) { build_stubbed(:user) } + let(:current_user) { create(:user) } + let(:user) { create(:user) } subject { described_class.new(current_user, [user]) } @@ -38,7 +38,7 @@ describe BasePolicy, :do_not_mock_admin_mode do it { is_expected.not_to be_allowed(:read_cross_project) } context 'for admins' do - let(:current_user) { build_stubbed(:admin) } + let(:current_user) { build(:admin) } subject { described_class.new(current_user, nil) } @@ -56,14 +56,14 @@ describe BasePolicy, :do_not_mock_admin_mode do end describe 'full private access' do - let(:current_user) { build_stubbed(:user) } + let(:current_user) { create(:user) } subject { described_class.new(current_user, nil) } it { is_expected.not_to be_allowed(:read_all_resources) } context 'for admins' do - let(:current_user) { build_stubbed(:admin) } + let(:current_user) { build(:admin) } it 'allowed when in admin mode' do enable_admin_mode!(current_user) diff --git a/spec/support/shared_examples/features/discussion_comments_shared_example.rb b/spec/support/shared_examples/features/discussion_comments_shared_example.rb index 257b53b2a7b..81433d124c9 100644 --- a/spec/support/shared_examples/features/discussion_comments_shared_example.rb +++ b/spec/support/shared_examples/features/discussion_comments_shared_example.rb @@ -8,44 +8,40 @@ RSpec.shared_examples 'thread comments' do |resource_name| let(:submit_selector) { "#{form_selector} .js-comment-submit-button" } let(:close_selector) { "#{form_selector} .btn-comment-and-close" } let(:comments_selector) { '.timeline > .note.timeline-entry' } + let(:comment) { 'My comment' } - it 'clicking "Comment" will post a comment', :quarantine do + it 'clicking "Comment" will post a comment' do expect(page).to have_selector toggle_selector - find("#{form_selector} .note-textarea").send_keys('a') + find("#{form_selector} .note-textarea").send_keys(comment) - find(submit_selector).click + click_button 'Comment' - wait_for_requests + expect(page).to have_content(comment) - find(comments_selector, match: :first) new_comment = all(comments_selector).last - expect(new_comment).to have_content 'a' expect(new_comment).not_to have_selector '.discussion' end if resource_name == 'issue' it "clicking 'Comment & close #{resource_name}' will post a comment and close the #{resource_name}" do - find("#{form_selector} .note-textarea").send_keys('a') + find("#{form_selector} .note-textarea").send_keys(comment) - find(close_selector).click - wait_for_requests + click_button 'Comment & close issue' - find(comments_selector, match: :first) - find("#{comments_selector}.system-note") - entries = all(comments_selector) - close_note = entries.last - new_comment = entries[-2] + expect(page).to have_content(comment) + expect(page).to have_content "@#{user.username} closed" + + new_comment = all(comments_selector).last - expect(close_note).to have_content 'closed' expect(new_comment).not_to have_selector '.discussion' end end describe 'when the toggle is clicked' do before do - find("#{form_selector} .note-textarea").send_keys('a') + find("#{form_selector} .note-textarea").send_keys(comment) find(toggle_selector).click end @@ -153,10 +149,11 @@ RSpec.shared_examples 'thread comments' do |resource_name| end it 'clicking "Start thread" will post a thread' do + expect(page).to have_content(comment) + new_comment = all(comments_selector).last - expect(new_comment).to have_content 'a' - expect(new_comment).to have_selector '.discussion' + expect(new_comment).to have_selector('.discussion') end if resource_name =~ /(issue|merge request)/ @@ -208,15 +205,13 @@ RSpec.shared_examples 'thread comments' do |resource_name| if resource_name == 'issue' it "clicking 'Start thread & close #{resource_name}' will post a thread and close the #{resource_name}" do - find(close_selector).click + click_button 'Start thread & close issue' - find(comments_selector, match: :first) - find("#{comments_selector}.system-note") - entries = all(comments_selector) - close_note = entries.last - new_discussion = entries[-2] + expect(page).to have_content(comment) + expect(page).to have_content "@#{user.username} closed" + + new_discussion = all(comments_selector)[-2] - expect(close_note).to have_content 'closed' expect(new_discussion).to have_selector '.discussion' end end |
