From 77a73903aa803869d0ab7fe544cc2d8b1a6aa1e0 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Fri, 1 Nov 2019 18:06:00 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .gitlab/ci/review.gitlab-ci.yml | 8 +- .../settings/operations_controller.rb | 2 +- app/helpers/projects_helper.rb | 4 + app/models/grafana_integration.rb | 4 + ...ox-for-grafana-authentication-settings.yml | 5 + config/locales/en.yml | 1 + ...901_add_enabled_to_grafana_integrations.rb | 23 +++ db/schema.rb | 3 +- doc/administration/gitaly/praefect.md | 168 +++++++++++++----- .../index.md | 3 +- doc/development/migration_style_guide.md | 2 +- .../new_or_existing_website.md | 2 +- .../settings/operations_controller_spec.rb | 3 +- spec/factories/grafana_integrations.rb | 1 + spec/helpers/projects_helper_spec.rb | 18 ++ spec/models/grafana_integration_spec.rb | 31 ++++ 16 files changed, 223 insertions(+), 55 deletions(-) create mode 100644 changelogs/unreleased/33672-add-enable-checkbox-for-grafana-authentication-settings.yml create mode 100644 db/migrate/20191029191901_add_enabled_to_grafana_integrations.rb diff --git a/.gitlab/ci/review.gitlab-ci.yml b/.gitlab/ci/review.gitlab-ci.yml index 05e974ccfb5..a790dd3e02f 100644 --- a/.gitlab/ci/review.gitlab-ci.yml +++ b/.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: diff --git a/app/controllers/projects/settings/operations_controller.rb b/app/controllers/projects/settings/operations_controller.rb index 5bf3618b389..1571cb8cd34 100644 --- a/app/controllers/projects/settings/operations_controller.rb +++ b/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 diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 16360c7139a..47214ac4ee2 100644 --- a/app/helpers/projects_helper.rb +++ b/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) diff --git a/app/models/grafana_integration.rb b/app/models/grafana_integration.rb index 51cc398394d..620bb4231a0 100644 --- a/app/models/grafana_integration.rb +++ b/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 diff --git a/changelogs/unreleased/33672-add-enable-checkbox-for-grafana-authentication-settings.yml b/changelogs/unreleased/33672-add-enable-checkbox-for-grafana-authentication-settings.yml new file mode 100644 index 00000000000..bb45c5ec4d2 --- /dev/null +++ b/changelogs/unreleased/33672-add-enable-checkbox-for-grafana-authentication-settings.yml @@ -0,0 +1,5 @@ +--- +title: Migrate enabled flag on grafana_integrations table +merge_request: 19234 +author: +type: changed diff --git a/config/locales/en.yml b/config/locales/en.yml index eff015459e3..950529f0355 100644 --- a/config/locales/en.yml +++ b/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" diff --git a/db/migrate/20191029191901_add_enabled_to_grafana_integrations.rb b/db/migrate/20191029191901_add_enabled_to_grafana_integrations.rb new file mode 100644 index 00000000000..8db11724874 --- /dev/null +++ b/db/migrate/20191029191901_add_enabled_to_grafana_integrations.rb @@ -0,0 +1,23 @@ +# 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 diff --git a/db/schema.rb b/db/schema.rb index b07ba57ba77..f6a445f3da6 100644 --- a/db/schema.rb +++ b/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 diff --git a/doc/administration/gitaly/praefect.md b/doc/administration/gitaly/praefect.md index 1973ed39b89..83c9aa3f013 100644 --- a/doc/administration/gitaly/praefect.md +++ b/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. +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, 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). +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. +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']`. -##### Disable other services +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 +#### Gitaly -When Praefect is running, it should be exposed as a storage to GitLab. This -is done through setting the `git_data_dirs`. +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. -##### On a fresh GitLab installation +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`. -On a fresh Gitlab installation, set up the `default` storage to point to praefect: +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 +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. -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. +For more information on Gitaly server configuration, see our [gitaly documentation](index.md#3-gitaly-server-configuration). -Assuming the default storage -configuration is used, there would be two storages available to GitLab: +#### 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. diff --git a/doc/administration/monitoring/gitlab_instance_administration_project/index.md b/doc/administration/monitoring/gitlab_instance_administration_project/index.md index bb76ad59e3b..b07bbafaf7d 100644 --- a/doc/administration/monitoring/gitlab_instance_administration_project/index.md +++ b/doc/administration/monitoring/gitlab_instance_administration_project/index.md @@ -1,6 +1,7 @@ # 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 diff --git a/doc/development/migration_style_guide.md b/doc/development/migration_style_guide.md index 20d705136b2..32c4313a1ed 100644 --- a/doc/development/migration_style_guide.md +++ b/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 ``` diff --git a/doc/user/project/pages/getting_started/new_or_existing_website.md b/doc/user/project/pages/getting_started/new_or_existing_website.md index 188b76e4224..efdd4e6a158 100644 --- a/doc/user/project/pages/getting_started/new_or_existing_website.md +++ b/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: diff --git a/spec/controllers/projects/settings/operations_controller_spec.rb b/spec/controllers/projects/settings/operations_controller_spec.rb index 0b34656e9e2..667a6336952 100644 --- a/spec/controllers/projects/settings/operations_controller_spec.rb +++ b/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 diff --git a/spec/factories/grafana_integrations.rb b/spec/factories/grafana_integrations.rb index 4eb3bee8b28..ae819ca828c 100644 --- a/spec/factories/grafana_integrations.rb +++ b/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 diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index 1fa3c639603..cd1b1f91e9f 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/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 diff --git a/spec/models/grafana_integration_spec.rb b/spec/models/grafana_integration_spec.rb index f8973097a40..615865e17b9 100644 --- a/spec/models/grafana_integration_spec.rb +++ b/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