Add latest changes from gitlab-org/gitlab@master

.gitlab/CODEOWNERS

@@ -6,9 +6,9 @@
 /doc/ @axil @marcia @eread @mikelewis
 # Frontend maintainers should see everything in `app/assets/`
-app/assets/ @ClemMakesApps @fatihacet @filipa @mikegreiling @timzallmann @kushalpandya @pslaughter @wortschi @ntepluhina @iamphill
-*.scss @annabeldunstone @ClemMakesApps @fatihacet @filipa @mikegreiling @timzallmann @kushalpandya @pslaughter @wortschi @ntepluhina @iamphill
-/scripts/frontend/ @ClemMakesApps @fatihacet @filipa @mikegreiling @timzallmann @kushalpandya @pslaughter @wortschi @ntepluhina @iamphill
+app/assets/ @gitlab-org/maintainers/frontend
+*.scss @annabeldunstone @gitlab-org/maintainers/frontend
+/scripts/frontend/ @gitlab-org/maintainers/frontend
 # Database maintainers should review changes in `db/`
 db/ @gitlab-org/maintainers/database

.rubocop.yml

@@ -297,3 +297,6 @@ Graphql/Descriptions:
   Include:
     - 'app/graphql/**/*'
     - 'ee/app/graphql/**/*'
+
+RSpec/AnyInstanceOf:
+  Enabled: false

app/assets/javascripts/error_tracking_settings/components/app.vue

@@ -43,16 +43,7 @@ export default {
       'isProjectInvalid',
       'projectSelectionLabel',
     ]),
-    ...mapState([
-      'apiHost',
-      'connectError',
-      'connectSuccessful',
-      'enabled',
-      'projects',
-      'selectedProject',
-      'settingsLoading',
-      'token',
-    ]),
+    ...mapState(['enabled', 'projects', 'selectedProject', 'settingsLoading', 'token']),
   },
   created() {
     this.setInitialState({
@@ -65,15 +56,7 @@ export default {
     });
   },
   methods: {
-    ...mapActions([
-      'fetchProjects',
-      'setInitialState',
-      'updateApiHost',
-      'updateEnabled',
-      'updateSelectedProject',
-      'updateSettings',
-      'updateToken',
-    ]),
+    ...mapActions(['setInitialState', 'updateEnabled', 'updateSelectedProject', 'updateSettings']),
     handleSubmit() {
       this.updateSettings();
     },
@@ -95,15 +78,7 @@ export default {
         s__('ErrorTracking|Active')
       }}</label>
     </div>
-    <error-tracking-form
-      :api-host="apiHost"
-      :connect-error="connectError"
-      :connect-successful="connectSuccessful"
-      :token="token"
-      @handle-connect="fetchProjects"
-      @update-api-host="updateApiHost"
-      @update-token="updateToken"
-    />
+    <error-tracking-form />
     <div class="form-group">
       <project-dropdown
         :has-projects="hasProjects"

app/assets/javascripts/error_tracking_settings/components/error_tracking_form.vue

 <script>
+import { mapActions, mapState } from 'vuex';
 import { GlButton, GlFormInput } from '@gitlab/ui';
 import Icon from '~/vue_shared/components/icon.vue';
 export default {
   components: { GlButton, GlFormInput, Icon },
-  props: {
-    apiHost: {
-      type: String,
-      required: true,
-    },
-    connectError: {
-      type: Boolean,
-      required: true,
-    },
-    connectSuccessful: {
-      type: Boolean,
-      required: true,
-    },
-    token: {
-      type: String,
-      required: true,
-    },
-  },
   computed: {
+    ...mapState(['apiHost', 'connectError', 'connectSuccessful', 'token']),
     tokenInputState() {
       return this.connectError ? false : null;
     },
   },
+  methods: {
+    ...mapActions(['fetchProjects', 'updateApiHost', 'updateToken']),
+  },
 };
 </script>
@@ -41,7 +28,7 @@ export default {
             id="error-tracking-api-host"
             :value="apiHost"
             placeholder="https://mysentryserver.com"
-            @input="$emit('update-api-host', $event)"
+            @input="updateApiHost"
           />
           <!-- eslint-enable @gitlab/vue-i18n/no-bare-attribute-strings -->
         </div>
@@ -60,15 +47,13 @@ export default {
             id="error-tracking-token"
             :value="token"
             :state="tokenInputState"
-            @input="$emit('update-token', $event)"
+            @input="updateToken"
           />
         </div>
         <div class="col-4 col-md-3 gl-pl-0">
-          <gl-button
-            class="js-error-tracking-connect prepend-left-5"
-            @click="$emit('handle-connect')"
-            >{{ __('Connect') }}</gl-button
-          >
+          <gl-button class="js-error-tracking-connect prepend-left-5" @click="fetchProjects">{{
+            __('Connect')
+          }}</gl-button>
           <icon
             v-show="connectSuccessful"
             class="js-error-tracking-connect-success prepend-left-5 text-success align-middle"

app/assets/stylesheets/framework/files.scss

@@ -387,6 +387,10 @@ span.idiff {
     float: none;
   }
+  .file-actions .ide-edit-button {
+    z-index: 2;
+  }
+
   @include media-breakpoint-down(xs) {
     display: block;

app/controllers/health_controller.rb

@@ -5,6 +5,11 @@ class HealthController < ActionController::Base
   include RequiresWhitelistedMonitoringClient
   CHECKS = [
+    Gitlab::HealthChecks::MasterCheck
+  ].freeze
+
+  ALL_CHECKS = [
+    *CHECKS,
     Gitlab::HealthChecks::DbCheck,
     Gitlab::HealthChecks::Redis::RedisCheck,
     Gitlab::HealthChecks::Redis::CacheCheck,
@@ -14,8 +19,9 @@ class HealthController < ActionController::Base
   ].freeze
   def readiness
-    # readiness check is a collection with all above application-level checks
-    render_checks(*CHECKS)
+    # readiness check is a collection of application-level checks
+    # and optionally all service checks
+    render_checks(params[:all] ? ALL_CHECKS : CHECKS)
   end
   def liveness
@@ -25,7 +31,7 @@ class HealthController < ActionController::Base
   private
-  def render_checks(*checks)
+  def render_checks(checks = [])
     result = Gitlab::HealthChecks::Probes::Collection
       .new(*checks)
       .execute

app/controllers/projects/merge_requests_controller.rb

@@ -364,7 +364,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
     when :error
       render json: { status_reason: report_comparison[:status_reason] }, status: :bad_request
     else
-      render json: { status_reason: 'Unknown error' }, status: :internal_server_error
+      raise "Failed to build comparison response as comparison yielded unknown status '#{report_comparison[:status]}'"
     end
   end

app/helpers/blob_helper.rb

@@ -47,7 +47,7 @@ module BlobHelper
   def edit_blob_button(project = @project, ref = @ref, path = @path, options = {})
     return unless blob = readable_blob(options, path, project, ref)
-    common_classes = "btn js-edit-blob #{options[:extra_class]}"
+    common_classes = "btn btn-primary js-edit-blob #{options[:extra_class]}"
     edit_button_tag(blob,
                     common_classes,
@@ -62,7 +62,7 @@ module BlobHelper
     return unless blob = readable_blob(options, path, project, ref)
     edit_button_tag(blob,
-                    'btn btn-default',
+                    'btn btn-inverted btn-primary ide-edit-button',
                     _('Web IDE'),
                     ide_edit_path(project, ref, path, options),
                     project,
@@ -108,7 +108,7 @@ module BlobHelper
       path,
       label:      _("Delete"),
       action:     "delete",
-      btn_class:  "remove",
+      btn_class:  "default",
       modal_type: "remove"
     )
   end

app/policies/group_policy.rb

@@ -44,6 +44,7 @@ class GroupPolicy < BasePolicy
   rule { public_group }.policy do
     enable :read_group
+    enable :read_package
   end
   rule { logged_in_viewable }.enable :read_group
@@ -70,7 +71,10 @@ class GroupPolicy < BasePolicy
   rule { has_access }.enable :read_namespace
-  rule { developer }.enable :admin_milestone
+  rule { developer }.policy do
+    enable :admin_milestone
+    enable :read_package
+  end
   rule { reporter }.policy do
     enable :read_container_image

changelogs/unreleased/30810-blob-view-buttons.yml

+---
+title: Change blob edit view button styling
+merge_request: 19566
+author:
+type: other

config/gitlab.yml.example

@@ -1032,12 +1032,6 @@ production: &base
     #  enabled: true
     #  address: localhost
     #  port: 8083
-    #  # blackout_seconds:
-    #  #   defines an interval to block healthcheck,
-    #  #   but continue accepting application requests
-    #  #   this allows Load Balancer to notice service
-    #  #   being shutdown and not interrupt any of the clients
-    #  blackout_seconds: 10
   ## Prometheus settings
   # Do not modify these settings here. They should be modified in /etc/gitlab/gitlab.rb
@@ -1049,6 +1043,14 @@ production: &base
     # enable: true
     # listen_address: 'localhost:9090'
+  shutdown:
+    #  # blackout_seconds:
+    #  #   defines an interval to block healthcheck,
+    #  #   but continue accepting application requests
+    #  #   this allows Load Balancer to notice service
+    #  #   being shutdown and not interrupt any of the clients
+    #  blackout_seconds: 10
+
   #
   # 5. Extra customization
   # ==========================

config/initializers/1_settings.rb

@@ -676,7 +676,12 @@ Settings.monitoring['web_exporter'] ||= Settingslogic.new({})
 Settings.monitoring.web_exporter['enabled'] ||= false
 Settings.monitoring.web_exporter['address'] ||= 'localhost'
 Settings.monitoring.web_exporter['port'] ||= 8083
-Settings.monitoring.web_exporter['blackout_seconds'] ||= 10
+
+#
+# Shutdown settings
+#
+Settings['shutdown'] ||= Settingslogic.new({})
+Settings.shutdown['blackout_seconds'] ||= 10
 #
 # Testing settings

config/initializers/7_prometheus_metrics.rb

@@ -70,6 +70,13 @@ if defined?(::Unicorn) || defined?(::Puma)
     Gitlab::Metrics::Exporter::WebExporter.instance.start
   end
+  # DEPRECATED: TO BE REMOVED
+  # This is needed to implement blackout period of `web_exporter`
+  # https://gitlab.com/gitlab-org/gitlab/issues/35343#note_238479057
+  Gitlab::Cluster::LifecycleEvents.on_before_blackout_period do
+    Gitlab::Metrics::Exporter::WebExporter.instance.mark_as_not_running!
+  end
+
   Gitlab::Cluster::LifecycleEvents.on_before_graceful_shutdown do
     # We need to ensure that before we re-exec or shutdown server
     # we do stop the exporter

config/initializers/health_check.rb

@@ -8,3 +8,15 @@ HealthCheck.setup do |config|
     end
   end
 end
+
+Gitlab::Cluster::LifecycleEvents.on_before_fork do
+  Gitlab::HealthChecks::MasterCheck.register_master
+end
+
+Gitlab::Cluster::LifecycleEvents.on_before_blackout_period do
+  Gitlab::HealthChecks::MasterCheck.finish_master
+end
+
+Gitlab::Cluster::LifecycleEvents.on_worker_start do
+  Gitlab::HealthChecks::MasterCheck.register_worker
+end

config/initializers/rack_attack_new.rb

@@ -39,45 +39,62 @@ module Gitlab::Throttle
 end
 class Rack::Attack
+  # Order conditions by how expensive they are:
+  # 1. The most expensive is the `req.unauthenticated?` and
+  #    `req.authenticated_user_id` as it performs an expensive
+  #    DB/Redis query to validate the request
+  # 2. Slightly less expensive is the need to query DB/Redis
+  #    to unmarshal settings (`Gitlab::Throttle.settings`)
+  #
+  # We deliberately skip `/-/health|liveness|readiness`
+  # from Rack Attack as they need to always be accessible
+  # by Load Balancer and additional measure is implemented
+  # (token and whitelisting) to prevent abuse.
   throttle('throttle_unauthenticated', Gitlab::Throttle.unauthenticated_options) do |req|
-    Gitlab::Throttle.settings.throttle_unauthenticated_enabled &&
-      req.unauthenticated? &&
-      !req.should_be_skipped? &&
+    if !req.should_be_skipped? &&
+        Gitlab::Throttle.settings.throttle_unauthenticated_enabled &&
+        req.unauthenticated?
       req.ip
+    end
   end
   throttle('throttle_authenticated_api', Gitlab::Throttle.authenticated_api_options) do |req|
-    Gitlab::Throttle.settings.throttle_authenticated_api_enabled &&
-      req.api_request? &&
+    if req.api_request? &&
+        Gitlab::Throttle.settings.throttle_authenticated_api_enabled
       req.authenticated_user_id([:api])
+    end
   end
   throttle('throttle_authenticated_web', Gitlab::Throttle.authenticated_web_options) do |req|
-    Gitlab::Throttle.settings.throttle_authenticated_web_enabled &&
-      req.web_request? &&
+    if req.web_request? &&
+        Gitlab::Throttle.settings.throttle_authenticated_web_enabled
       req.authenticated_user_id([:api, :rss, :ics])
+    end
   end
   throttle('throttle_unauthenticated_protected_paths', Gitlab::Throttle.protected_paths_options) do |req|
-    Gitlab::Throttle.protected_paths_enabled? &&
-      req.unauthenticated? &&
-      !req.should_be_skipped? &&
-      req.protected_path? &&
+    if !req.should_be_skipped? &&
+        req.protected_path? &&
+        Gitlab::Throttle.protected_paths_enabled? &&
+        req.unauthenticated?
       req.ip
+    end
   end
   throttle('throttle_authenticated_protected_paths_api', Gitlab::Throttle.protected_paths_options) do |req|
-    Gitlab::Throttle.protected_paths_enabled? &&
-      req.api_request? &&
-      req.protected_path? &&
+    if req.api_request? &&
+        Gitlab::Throttle.protected_paths_enabled? &&
+        req.protected_path?
       req.authenticated_user_id([:api])
+    end
   end
   throttle('throttle_authenticated_protected_paths_web', Gitlab::Throttle.protected_paths_options) do |req|
-    Gitlab::Throttle.protected_paths_enabled? &&
-      req.web_request? &&
-      req.protected_path? &&
+    if req.web_request? &&
+        Gitlab::Throttle.protected_paths_enabled? &&
+        req.protected_path?
       req.authenticated_user_id([:api, :rss, :ics])
+    end
   end
   class Request
@@ -97,12 +114,16 @@ class Rack::Attack
       path =~ %r{^/api/v\d+/internal/}
     end
+    def health_check_request?
+      path =~ %r{^/-/(health|liveness|readiness)}
+    end
+
     def should_be_skipped?
-      api_internal_request?
+      api_internal_request? || health_check_request?
     end
     def web_request?
-      !api_request?
+      !api_request? && !health_check_request?
     end
     def protected_path?

doc/api/packages.md

@@ -2,7 +2,9 @@
 This is the API docs of [GitLab Packages](../administration/packages/index.md).
-## List project packages
+## List packages
+
+### Within a project
 > [Introduced](https://gitlab.com/gitlab-org/gitlab/merge_requests/9259) in GitLab 11.8.
@@ -42,6 +44,47 @@ Example response:
 By default, the `GET` request will return 20 results, since the API is [paginated](README.md#pagination).
+### Within a group
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/merge_requests/18871) in GitLab 12.5.
+
+Get a list of project packages at the group level.
+When accessed without authentication, only packages of public projects are returned.
+
+```
+GET /groups/:id/packages
+```
+
+| Attribute | Type | Required | Description |
+| --------- | ---- | -------- | ----------- |
+| `id`      | integer/string | yes | ID or [URL-encoded path of the group](README.md#namespaced-path-encoding). |
+| `exclude_subgroups` | boolean | false | If the param is included as true, packages from projects from subgroups are not listed. Default is `false`. |
+
+```bash
+curl --header "PRIVATE-TOKEN: <your_access_token>" https://gitlab.example.com/api/v4/group/:id/packages?exclude_subgroups=true
+```
+
+Example response:
+
+```json
+[
+  {
+    "id": 1,
+    "name": "com/mycompany/my-app",
+    "version": "1.0-SNAPSHOT",
+    "package_type": "maven"
+  },
+  {
+    "id": 2,
+    "name": "@foo/bar",
+    "version": "1.0.3",
+    "package_type": "npm"
+  }
+]
+```
+
+By default, the `GET` request will return 20 results, since the API is [paginated](README.md#pagination).
+
 ## Get a project package
 > [Introduced](https://gitlab.com/gitlab-org/gitlab/merge_requests/9667) in GitLab 11.9.

doc/development/fe_guide/style_guide_js.md

@@ -581,6 +581,18 @@ Please check this [rules][eslint-plugin-vue-rules] for more documentation.
    <component />
    ```
+#### Component usage within templates
+
+1. Prefer a component's kebab-cased name over other styles when using it in a template
+
+   ```javascript
+   // bad
+   <MyComponent />
+
+   // good
+   <my-component />
+   ```
+
 #### Ordering
 1. Tag order in `.vue` file

doc/development/testing_guide/end_to_end/feature_flags.md

@@ -2,6 +2,8 @@
 To run a specific test with a feature flag enabled you can use the `QA::Runtime::Feature` class to enabled and disable feature flags ([via the API](../../../api/features.md)).
+Note that administrator authorization is required to change feature flags. `QA::Runtime::Feature` will automatically authenticate as an administrator as long as you provide an appropriate access token via `GITLAB_QA_ADMIN_ACCESS_TOKEN` (recommended), or provide `GITLAB_ADMIN_USERNAME` and `GITLAB_ADMIN_PASSWORD`.
+
 ```ruby
 context "with feature flag enabled" do
   before do

doc/development/testing_guide/frontend_testing.md

@@ -501,6 +501,39 @@ it('waits for an event', () => {
 });
 ```
+#### Ensuring that tests are isolated
+
+Tests are normally architected in a pattern which requires a recurring setup and breakdown of the component under test. This is done by making use of the `beforeEach` and `afterEach` hooks.
+
+Example
+
+```javascript
+  let wrapper;
+
+  beforeEach(() => {
+    wrapper = mount(Component);
+  });
+
+  afterEach(() => {
+    wrapper.destroy();
+  });
+```
+
+When looking at this initially you'd suspect that the component is setup before each test and then broken down afterwards, providing isolation between tests.
+
+This is however not entirely true as the `destroy` method does not remove everything which has been mutated on the `wrapper` object. For functional components, destroy only removes the rendered DOM elements from the document.
+
+In order to ensure that a clean wrapper object and DOM are being used in each test, the breakdown of the component should rather be performed as follows:
+
+```javascript
+  afterEach(() => {
+    wrapper.destroy();
+    wrapper = null;
+  });
+```
+
+See also the [Vue Test Utils documention on `destroy`](https://vue-test-utils.vuejs.org/api/wrapper/#destroy).
+
 #### Migrating flaky Karma tests to Jest
 Some of our Karma tests are flaky because they access the properties of a shared scope.

doc/user/admin_area/monitoring/health_check.md

@@ -39,7 +39,11 @@ GET http://localhost/-/liveness
 ## Health
-Checks whether the application server is running. It does not verify the database or other services are running.
+Checks whether the application server is running.
+It does not verify the database or other services
+are running. This endpoint circumvents Rails Controllers
+and is implemented as additional middleware `BasicHealthCheck`
+very early into the request processing lifecycle.
 ```text
 GET /-/health
@@ -59,10 +63,17 @@ GitLab OK
 ## Readiness
-The readiness probe checks whether the GitLab instance is ready to use. It checks the dependent services (Database, Redis, Gitaly etc.) and gives a status for each.
+The readiness probe checks whether the GitLab instance is ready
+to accept traffic via Rails Controllers. The check by default
+does validate only instance-checks.
+
+If the `all=1` parameter is specified, the check will also validate
+the dependent services (Database, Redis, Gitaly etc.)
+and gives a status for each.
 ```text
 GET /-/readiness
+GET /-/readiness?all=1
 ```
 Example request:
@@ -75,37 +86,30 @@ Example response:
 ```json
 {
-   "db_check":{
+   "master_check":[{
       "status":"failed",
-      "message": "unexpected Db check result: 0"
-   },
-   "redis_check":{
-      "status":"ok"
-   },
-   "cache_check":{
-      "status":"ok"
-   },
-   "queues_check":{
-      "status":"ok"
-   },
-   "shared_state_check":{
-      "status":"ok"
-   },
-   "gitaly_check":{
-      "status":"ok",
-      "labels":{
-         "shard":"default"
-         }
-      }
-   }
+      "message": "unexpected Master check result: false"
+   }],
+   ...
+}
 ```
+On failure, the endpoint will return a `503` HTTP status code.
+
+This check does hit the database and Redis if authenticated via `token`.
+
+This check is being exempt from Rack Attack.
+
 ## Liveness
 DANGER: **Warning:**
-In Gitlab [12.4](https://about.gitlab.com/upcoming-releases/) the response body of the Liveness check will change to match the example below.
-The liveness probe checks whether the application server is alive. Unlike the [`health`](#health) check, this check hits the database.
+In Gitlab [12.4](https://about.gitlab.com/upcoming-releases/)
+the response body of the Liveness check was changed
+to match the example below.
+Checks whether the application server is running.
+This probe is used to know if Rails Controllers
+are not deadlocked due to a multi-threading.
 ```text
 GET /-/liveness
@@ -127,7 +131,9 @@ On success, the endpoint will return a `200` HTTP status code, and a response li
 }
 ```
-On failure, the endpoint will return a `500` HTTP status code.
+On failure, the endpoint will return a `503` HTTP status code.
+
+This check is being exempt from Rack Attack.
 ## Access token (Deprecated)