Add latest changes from gitlab-org/gitlab@master

.gitlab/ci/review.gitlab-ci.yml

@@ -150,7 +150,10 @@ schedule:review-deploy:
   needs: ["schedule:review-build-cng"]
 .base-review-stop:
-  extends: .review-workflow-base
+  extends:
+    - .review-workflow-base
+    - .only-review
+    - .only:changes-code-qa
   environment:
     action: stop
   variables:
@@ -164,7 +167,7 @@ schedule:review-deploy:
     - source utils.sh
     - source review-apps.sh
-review-cleanup-failed-deployment:
+review-stop-failed-deployment:
   extends: .base-review-stop
   stage: prepare
   script:
@@ -172,6 +175,7 @@ review-cleanup-failed-deployment:
 review-stop:
   extends: .base-review-stop
+  stage: review
   when: manual
   allow_failure: true
   script:

app/controllers/projects/settings/operations_controller.rb

@@ -70,7 +70,7 @@ module Projects
             project: [:slug, :name, :organization_slug, :organization_name]
           ],
-          grafana_integration_attributes: [:token, :grafana_url]
+          grafana_integration_attributes: [:token, :grafana_url, :enabled]
         }
       end
     end

app/helpers/projects_helper.rb

@@ -362,6 +362,10 @@ module ProjectsHelper
     @project.grafana_integration&.token
   end
+  def grafana_integration_enabled?
+    @project.grafana_integration&.enabled?
+  end
+
   private
   def get_project_nav_tabs(project, current_user)

app/models/grafana_integration.rb

@@ -14,7 +14,11 @@ class GrafanaIntegration < ApplicationRecord
   validates :token, :project, presence: true
+  validates :enabled, inclusion: { in: [true, false] }
+
   def client
+    return unless enabled?
+
     @client ||= ::Grafana::Client.new(api_url: grafana_url.chomp('/'), token: token)
   end
 end

changelogs/unreleased/33672-add-enable-checkbox-for-grafana-authentication-settings.yml

+---
+title: Migrate enabled flag on grafana_integrations table
+merge_request: 19234
+author:
+type: changed

config/locales/en.yml

@@ -19,6 +19,7 @@ en:
       project/grafana_integration:
         token: "Grafana HTTP API Token"
         grafana_url: "Grafana API URL"
+        grafana_enabled: "Grafana integration enabled"
   views:
     pagination:
       previous: "Prev"

db/migrate/20191029191901_add_enabled_to_grafana_integrations.rb

+# frozen_string_literal: true
+
+class AddEnabledToGrafanaIntegrations < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_column_with_default(
+      :grafana_integrations,
+      :enabled,
+      :boolean,
+      allow_null: false,
+      default: false
+    )
+  end
+
+  def down
+    remove_column(:grafana_integrations, :enabled)
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 2019_10_26_124116) do
+ActiveRecord::Schema.define(version: 2019_10_29_191901) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_trgm"
@@ -1809,6 +1809,7 @@ ActiveRecord::Schema.define(version: 2019_10_26_124116) do
     t.string "encrypted_token", limit: 255, null: false
     t.string "encrypted_token_iv", limit: 255, null: false
     t.string "grafana_url", limit: 1024, null: false
+    t.boolean "enabled", default: false, null: false
     t.index ["project_id"], name: "index_grafana_integrations_on_project_id"
   end

doc/administration/gitaly/praefect.md

@@ -26,33 +26,42 @@ For this document, the following network topology is assumed:
 graph TB
   GitLab --> Gitaly;
   GitLab --> Praefect;
-  Praefect --> Preafect-Git-1;
-  Praefect --> Preafect-Git-2;
-  Praefect --> Preafect-Git-3;
+  Praefect --> Praefect-Gitaly-1;
+  Praefect --> Praefect-Gitaly-2;
+  Praefect --> Praefect-Gitaly-3;
 ```
 Where `GitLab` is the collection of clients that can request Git operations.
-`Gitaly` is a Gitaly server before using Praefect. The Praefect node has two
+`Gitaly` is a Gitaly server before using Praefect. The Praefect node has three
 storage nodes attached. Praefect itself doesn't store data, but connects to
-three Gitaly nodes, `Praefect-Git-1`,  `Praefect-Git-2`, and `Praefect-Git-3`.
-There should be no knowledge other than with Praefect about the existence of
-the `Praefect-Git-X` nodes.
-### Setup
-In this setup guide, the Gitaly node will be added first, then Praefect, and
-lastly we update the GitLab configuration.
-#### Gitaly
-In their own machine, configure the Gitaly server as described in the
-[gitaly documentation](index.md#3-gitaly-server-configuration).
+three Gitaly nodes, `Praefect-Gitaly-1`,  `Praefect-Gitaly-2`, and `Praefect-Gitaly-3`.
+Praefect may be enabled on its own node or can be run on the GitLab server.
+In the example below we will use a separate server, but the optimal configuration
+for Praefect is still being determined.
+Praefect will handle all Gitaly RPC requests to its child nodes. However, the child nodes
+will still need to communicate with the GitLab server via its internal API for authentication
+purposes.
+### Setup
+In this setup guide we will start by configuring Praefect, then its child
+Gitaly nodes, and lastly the GitLab server configuration.
 #### Praefect
-Next, Praefect has to be enabled on its own node.
-##### Disable other services
+On the Praefect node we disable all other services, including Gitaly. We list each
+Gitaly node that will be connected to Praefect under `praefect['storage_nodes']`.
+In the example below, the Gitaly nodes are named `praefect-gitaly-N`. Note that one
+node is designated as primary by setting the primary to `true`.
+
+`praefect['auth_token']` is the token used to authenticate with the GitLab server,
+just like `gitaly['auth_token']` is used for a standard Gitaly server.
+
+The `token` field under each storage listed in `praefect['storage_nodes']` is used
+to authenticate each child Gitaly node with Praefect.
 ```ruby
 # /etc/gitlab/gitlab.rb
@@ -76,58 +85,101 @@ primary, by setting the primary to `true`:
 ```ruby
 # /etc/gitlab/gitlab.rb
+# Prevent database connections during 'gitlab-ctl reconfigure'
+gitlab_rails['rake_cache_clear'] = false
+gitlab_rails['auto_migrate'] = false
+
+praefect['enable'] = true
+
+# Make Praefect accept connections on all network interfaces. You must use
+# firewalls to restrict access to this address/port.
+praefect['listen_addr'] = '0.0.0.0:2305'
+
 # virtual_storage_name must match the same storage name given to praefect in git_data_dirs
 praefect['virtual_storage_name'] = 'praefect'
-praefect['auth_token'] = 'abc123secret'
-praefect['enable'] = true
-praefect['storage_nodes'] = [
-  {
-    'storage' => 'praefect-git-1',
-    'address' => 'tcp://praefect-git-1.internal',
-    'token'   => 'xyz123secret',
+
+# Authentication token to ensure only authorized servers can communicate with
+# Praefect server
+praefect['auth_token'] = 'praefect-token'
+praefect['storage_nodes'] = {
+  'praefect-gitaly-1' => {
+    'address' => 'tcp://praefect-git-1.internal:8075',
+    'token'   => 'praefect-gitaly-token',
     'primary' => true
   },
-  {
-    'storage' => 'praefect-git-2',
-    'address' => 'tcp://praefect-git-2.internal',
-    'token'   => 'xyz456secret',
+  'praefect-gitaly-2' => {
+    'address' => 'tcp://praefect-git-2.internal:8075',
+    'token'   => 'praefect-gitaly-token'
   },
-  {
-    'storage' => 'praefect-git-3',
-    'address' => 'tcp://praefect-git-3.internal',
-    'token'   => 'xyz789secret',
+  'praefect-gitaly-3' => {
+    'address' => 'tcp://praefect-git-3.internal:8075',
+    'token'   => 'praefect-gitaly-token'
   }
-]
+}
 ```
 Save the file and [reconfigure Praefect](../restart_gitlab.md#omnibus-gitlab-reconfigure).
-#### GitLab
-When Praefect is running, it should be exposed as a storage to GitLab. This
-is done through setting the `git_data_dirs`.
-##### On a fresh GitLab installation
-On a fresh Gitlab installation, set up the `default` storage to point to praefect:
+#### Gitaly
+Next we will configure each Gitaly server assigned to Praefect.  Configuration for these
+is the same as a normal standalone Gitaly server, except that we use storage names and
+auth tokens from Praefect instead of GitLab.
+Below is an example configuration for `praefect-gitaly-1`, the only difference for the
+other Gitaly nodes is the storage name under `git_data_dirs`.
+Note that `gitaly['auth_token']` matches the `token` value listed under `praefect['storage_nodes']`
+on the Praefect node.
 ```ruby
+# /etc/gitlab/gitlab.rb
+
+# Avoid running unnecessary services on the Gitaly server
+postgresql['enable'] = false
+redis['enable'] = false
+nginx['enable'] = false
+prometheus['enable'] = false
+unicorn['enable'] = false
+sidekiq['enable'] = false
+gitlab_workhorse['enable'] = false
+
+# Prevent database connections during 'gitlab-ctl reconfigure'
+gitlab_rails['rake_cache_clear'] = false
+gitlab_rails['auto_migrate'] = false
+
+# Configure the gitlab-shell API callback URL. Without this, `git push` will
+# fail. This can be your 'front door' GitLab URL or an internal load
+# balancer.
+# Don't forget to copy `/etc/gitlab/gitlab-secrets.json` from web server to Gitaly server.
+gitlab_rails['internal_api_url'] = 'https://gitlab.example.com'
+
+# Authentication token to ensure only authorized servers can communicate with
+# Gitaly server
+gitaly['auth_token'] = 'praefect-gitaly-token'
+
+# Make Gitaly accept connections on all network interfaces. You must use
+# firewalls to restrict access to this address/port.
+# Comment out following line if you only want to support TLS connections
+gitaly['listen_addr'] = "0.0.0.0:8075"
+
 git_data_dirs({
-  "default" => {
-    "gitaly_address" => "tcp://praefect.internal:2305"
-  },
+  "praefect-gitaly-1" => {
+    "path" => "/var/opt/gitlab/git-data"
+  }
 })
 ```
-##### On an existing GitLab instance
-On an existing GitLab instance, the `default` storage is already being served by a
-Gitaly node, so an additional storage can be added. `praefect` is chosen in the example
-below, but it can be any name as long as it matches the `virtual_storage_name` in the
-praefect attributes above.
-Assuming the default storage
-configuration is used, there would be two storages available to GitLab:
+Note that just as with a standard Gitaly server, `/etc/gitlab/gitlab-secrets.json` must
+be copied from the GitLab server to the Gitaly node for authentication purposes.
+For more information on Gitaly server configuration, see our [gitaly documentation](index.md#3-gitaly-server-configuration).
+#### GitLab
+
+When Praefect is running, it should be exposed as a storage to GitLab. This
+is done through setting the `git_data_dirs`. Assuming the default storage
+is present, there should be two storages available to GitLab:
 ```ruby
 git_data_dirs({
@@ -138,6 +190,28 @@ git_data_dirs({
     "gitaly_address" => "tcp://praefect.internal:2305"
   }
 })
+
+gitlab_rails['gitaly_token'] = 'praefect-token'
 ```
+Note that the storage name used is the same as the `praefect['virtual_storage_name']` set
+on the Praefect node.
+
+Also, the `gitlab_rails['gitaly_token']` matches the value of `praefect['auth_token']`
+on Praefect.
+
 Restart GitLab using `gitlab-ctl restart` on the GitLab node.
+
+### Testing Praefect
+
+To test Praefect, first set it as the default storage node for new projects
+using **Admin Area > Settings > Repository > Repository storage**. Next,
+create a new project and check the "Initialize repository with a README" box.
+
+If you receive a 503 error, check `/var/log/gitlab/gitlab-rails/production.log`.
+A `GRPC::Unavailable (14:failed to connect to all addresses)` error indicates
+that GitLab was unable to connect to Praefect.
+
+If the project is created but the README is not, then ensure that the
+`/etc/gitlab/gitlab-secrets.json` file from the GitLab server has been copied
+to the Gitaly servers.

doc/administration/monitoring/gitlab_instance_administration_project/index.md

 # GitLab instance administration project
-> [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/issues/56883) in GitLab 12.2.
+NOTE: **Note:**
+This feature is not yet available and is [planned for 12.6](https://gitlab.com/gitlab-org/gitlab/issues/32351).
 GitLab has been adding the ability for administrators to see insights into the health of
 their GitLab instance. In order to surface this experience in a native way, similar to how

doc/development/migration_style_guide.md

@@ -211,7 +211,7 @@ class MyMigration < ActiveRecord::Migration[4.2]
   end
   def down
-    remove_index :table, :column if index_exists?(:table, :column)
+    remove_concurrent_index :table, :column
   end
 end
 ```

doc/user/project/pages/getting_started/new_or_existing_website.md

@@ -2,7 +2,7 @@
 type: reference, howto
 ---
-# Start a new Pages website from scratch or deploy an exising website
+# Start a new Pages website from scratch or deploy an existing website
 If you already have a website and want to deploy it with GitLab Pages,
 or, if you want to start a new site from scratch, you'll need to:

spec/controllers/projects/settings/operations_controller_spec.rb

@@ -186,7 +186,8 @@ describe Projects::Settings::OperationsController do
         {
           grafana_integration_attributes: {
             grafana_url: 'https://grafana.gitlab.com',
-            token: 'eyJrIjoicDRlRTREdjhhOEZ5WjZPWXUzazJOSW0zZHJUejVOd3IiLCJuIjoiVGVzdCBLZXkiLCJpZCI6MX0='
+            token: 'eyJrIjoicDRlRTREdjhhOEZ5WjZPWXUzazJOSW0zZHJUejVOd3IiLCJuIjoiVGVzdCBLZXkiLCJpZCI6MX0=',
+            enabled: 'true'
           }
         }
       end

spec/factories/grafana_integrations.rb

@@ -5,5 +5,6 @@ FactoryBot.define do
     project
     grafana_url { 'https://grafana.example.com' }
     token { SecureRandom.hex(10) }
+    enabled { true }
   end
 end

spec/helpers/projects_helper_spec.rb

@@ -938,4 +938,22 @@ describe ProjectsHelper do
       it { is_expected.to eq(grafana_integration.token) }
     end
   end
+
+  describe '#grafana_integration_enabled?' do
+    let(:project) { create(:project) }
+
+    before do
+      helper.instance_variable_set(:@project, project)
+    end
+
+    subject { helper.grafana_integration_enabled? }
+
+    it { is_expected.to eq(nil) }
+
+    context 'grafana integration exists' do
+      let!(:grafana_integration) { create(:grafana_integration, project: project) }
+
+      it { is_expected.to eq(grafana_integration.enabled) }
+    end
+  end
 end

spec/models/grafana_integration_spec.rb

@@ -34,5 +34,36 @@ describe GrafanaIntegration do
         internal_url
       ).for(:grafana_url)
     end
+
+    it 'disallows non-booleans in enabled column' do
+      is_expected.not_to allow_value(
+        nil
+      ).for(:enabled)
+    end
+
+    it 'allows booleans in enabled column' do
+      is_expected.to allow_value(
+        true,
+        false
+      ).for(:enabled)
+    end
+  end
+
+  describe '.client' do
+    subject(:grafana_integration) { create(:grafana_integration) }
+
+    context 'with grafana integration disabled' do
+      it 'returns a grafana client' do
+        expect(grafana_integration.client).to be_an_instance_of(::Grafana::Client)
+      end
+    end
+
+    context 'with grafana integration enabled' do
+      it 'returns nil' do
+        grafana_integration.update(enabled: false)
+
+        expect(grafana_integration.client).to be(nil)
+      end
+    end
   end
 end