From f5ae9d0960aa422a65a2a22e230100257dddb9ed Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Tue, 10 Mar 2020 03:08:03 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- app/models/epic.rb | 2 + .../119107-respect-dnt-for-experiments.yml | 5 + .../geo/replication/high_availability.md | 99 +++++++++++++++++-- .../testing_guide/frontend_testing.md | 4 +- .../testing_guide/testing_levels.md | 2 +- .../admin_area/settings/usage_statistics.md | 3 +- .../project/merge_requests/code_quality.md | 7 +- lib/gitlab/experimentation.rb | 13 ++- spec/lib/gitlab/experimentation_spec.rb | 43 +++++++- 9 files changed, 159 insertions(+), 19 deletions(-) create mode 100644 changelogs/unreleased/119107-respect-dnt-for-experiments.yml diff --git a/app/models/epic.rb b/app/models/epic.rb index ea4a231931d..04e19c17e18 100644 --- a/app/models/epic.rb +++ b/app/models/epic.rb @@ -5,6 +5,8 @@ class Epic < ApplicationRecord include IgnorableColumns + ignore_column :health_status, remove_with: '13.0', remove_after: '2019-05-22' + def self.link_reference_pattern nil end diff --git a/changelogs/unreleased/119107-respect-dnt-for-experiments.yml b/changelogs/unreleased/119107-respect-dnt-for-experiments.yml new file mode 100644 index 00000000000..0132599d40b --- /dev/null +++ b/changelogs/unreleased/119107-respect-dnt-for-experiments.yml @@ -0,0 +1,5 @@ +--- +title: 'Use DNT: 1 as an experiment opt-out mechanism' +merge_request: 22100 +author: +type: other diff --git a/doc/administration/geo/replication/high_availability.md b/doc/administration/geo/replication/high_availability.md index 8dd04cdace1..3e7102b96da 100644 --- a/doc/administration/geo/replication/high_availability.md +++ b/doc/administration/geo/replication/high_availability.md @@ -359,13 +359,6 @@ On the secondary the following GitLab frontend services will be enabled: Verify these services by running `sudo gitlab-ctl status` on the frontend application servers. -You may wish to run backend application services on backend-specific servers. -For example, you can disable the `geo-logcursor` service with -`geo_logcursor['enable'] = false` and run it on application servers not -attached to the load balancer. On those backend application servers, you would -disable Unicorn with `unicorn['enable'] = false`. You might also choose to do -the same thing with the `sidekiq` service. - ### Step 5: Set up the LoadBalancer for the **secondary** node In this topology, a load balancer is required at each geographic location to @@ -374,5 +367,97 @@ route traffic to the application servers. See [Load Balancer for GitLab HA](../../high_availability/load_balancer.md) for more information. +### Step 6: Configure the backend application servers on the **secondary** node + +The minimal reference architecture diagram above shows all application services +running together on the same machines. However, for high availability we +[strongly recommend running all services separately](../../high_availability/README.md). + +For example, a Sidekiq server could be configured similarly to the frontend +application servers above, with some changes to run only the `sidekiq` service: + +1. Edit `/etc/gitlab/gitlab.rb` on each Sidekiq server in the **secondary** + cluster, and add the following: + + ```ruby + ## + ## Enable the Geo secondary role + ## + roles ['geo_secondary_role'] + + ## + ## Enable the Sidekiq service + ## + sidekiq['enable'] = true + + ## + ## Ensure unnecessary services are disabled + ## + alertmanager['enable'] = false + consul['enable'] = false + geo_logcursor['enable'] = false + gitaly['enable'] = false + gitlab_exporter['enable'] = false + gitlab_workhorse['enable'] = false + nginx['enable'] = false + node_exporter['enable'] = false + pgbouncer_exporter['enable'] = false + postgresql['enable'] = false + prometheus['enable'] = false + redis['enable'] = false + redis_exporter['enable'] = false + repmgr['enable'] = false + unicorn['enable'] = false + + ## + ## The unique identifier for the Geo node. + ## + gitlab_rails['geo_node_name'] = '' + + ## + ## Disable automatic migrations + ## + gitlab_rails['auto_migrate'] = false + + ## + ## Configure the connection to the tracking DB. And disable application + ## servers from running tracking databases. + ## + geo_secondary['db_host'] = '' + geo_secondary['db_password'] = '' + geo_postgresql['enable'] = false + + ## + ## Configure connection to the streaming replica database, if you haven't + ## already + ## + gitlab_rails['db_host'] = '' + gitlab_rails['db_password'] = '' + + ## + ## Configure connection to Redis, if you haven't already + ## + gitlab_rails['redis_host'] = '' + gitlab_rails['redis_password'] = '' + + ## + ## If you are using custom users not managed by Omnibus, you need to specify + ## UIDs and GIDs like below, and ensure they match between servers in a + ## cluster to avoid permissions issues + ## + user['uid'] = 9000 + user['gid'] = 9000 + web_server['uid'] = 9001 + web_server['gid'] = 9001 + registry['uid'] = 9002 + registry['gid'] = 9002 + ``` + + You can similarly configure a server to run only the `geo-logcursor` service + with `geo_logcursor['enable'] = true` and disabling Sidekiq with + `sidekiq['enable'] = false`. + + These servers do not need to be attached to the load balancer. + [diagram-source]: https://docs.google.com/drawings/d/1z0VlizKiLNXVVVaERFwgsIOuEgjcUqDTWPdQYsE7Z4c/edit [gitlab-reconfigure]: ../../restart_gitlab.md#omnibus-gitlab-reconfigure diff --git a/doc/development/testing_guide/frontend_testing.md b/doc/development/testing_guide/frontend_testing.md index 26357d4fdfd..619c6fdb7a1 100644 --- a/doc/development/testing_guide/frontend_testing.md +++ b/doc/development/testing_guide/frontend_testing.md @@ -58,7 +58,7 @@ which could arise (especially with testing against browser specific features). ### Differences to Karma -- Jest runs in a Node.js environment, not in a browser. Support for running Jest tests in a browser [is planned](https://gitlab.com/gitlab-org/gitlab-foss/issues/58205). +- Jest runs in a Node.js environment, not in a browser. Support for running Jest tests in a browser [is planned](https://gitlab.com/gitlab-org/gitlab/-/issues/26982). - Because Jest runs in a Node.js environment, it uses [jsdom](https://github.com/jsdom/jsdom) by default. See also its [limitations](#limitations-of-jsdom) below. - Jest does not have access to Webpack loaders or aliases. The aliases used by Jest are defined in its [own config](https://gitlab.com/gitlab-org/gitlab/blob/master/jest.config.js). @@ -81,7 +81,7 @@ This comes with a number of limitations, namely: - [No element sizes or positions](https://github.com/jsdom/jsdom/blob/15.1.1/lib/jsdom/living/nodes/Element-impl.js#L334-L371) - [No layout engine](https://github.com/jsdom/jsdom/issues/1322) in general -See also the issue for [support running Jest tests in browsers](https://gitlab.com/gitlab-org/gitlab-foss/issues/58205). +See also the issue for [support running Jest tests in browsers](https://gitlab.com/gitlab-org/gitlab/-/issues/26982). ### Debugging Jest tests diff --git a/doc/development/testing_guide/testing_levels.md b/doc/development/testing_guide/testing_levels.md index 58f00829b80..295aa6609a8 100644 --- a/doc/development/testing_guide/testing_levels.md +++ b/doc/development/testing_guide/testing_levels.md @@ -314,7 +314,7 @@ controller.instance_variable_set(:@user, user) and use methods which are deprecated in Rails 5 ([#23768]). -[#23768]: https://gitlab.com/gitlab-org/gitlab-foss/issues/23768 +[#23768]: https://gitlab.com/gitlab-org/gitlab/-/issues/16260 ### About Karma diff --git a/doc/user/admin_area/settings/usage_statistics.md b/doc/user/admin_area/settings/usage_statistics.md index feb99ca488d..8998a2430f3 100644 --- a/doc/user/admin_area/settings/usage_statistics.md +++ b/doc/user/admin_area/settings/usage_statistics.md @@ -11,7 +11,8 @@ All statistics are opt-out. You can enable/disable them in the **Admin Area > Settings > Metrics and profiling** section **Usage statistics**. NOTE: **Note:** -Allow network traffic from your GitLab instance to IP address 104.196.17.203 to send usage statistics to GitLab Inc. +Allow network traffic from your GitLab instance to IP address `104.196.17.203:443`, to send +usage statistics to GitLab Inc. ## Version Check **(CORE ONLY)** diff --git a/doc/user/project/merge_requests/code_quality.md b/doc/user/project/merge_requests/code_quality.md index b2b28c54350..21153588c31 100644 --- a/doc/user/project/merge_requests/code_quality.md +++ b/doc/user/project/merge_requests/code_quality.md @@ -6,7 +6,7 @@ type: reference, howto > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/1984) in [GitLab Starter](https://about.gitlab.com/pricing/) 9.3. -With the help of [GitLab CI/CD](../../../ci/README.md), you can analyze your +Ensuring your project's code stays simple, readable and easy to contribute to can be problematic. With the help of [GitLab CI/CD](../../../ci/README.md), you can analyze your source code quality using GitLab Code Quality. Code Quality: @@ -14,12 +14,13 @@ Code Quality: - Uses [Code Climate Engines](https://codeclimate.com), which are free and open source. Code Quality doesn't require a Code Climate subscription. -- Runs in [pipelines](../../../ci/pipelines.md) using an Docker image built in +- Runs in [pipelines](../../../ci/pipelines.md) using a Docker image built in the [GitLab Code - Quality](https://gitlab.com/gitlab-org/security-products/codequality) project. + Quality](https://gitlab.com/gitlab-org/security-products/codequality) project using [default Code Climate configurations](https://gitlab.com/gitlab-org/security-products/codequality/-/tree/master/codeclimate_defaults). - Can make use of a [template](#example-configuration). - Is available with [Auto DevOps](../../../topics/autodevops/index.md#auto-code-quality-starter). +- Can be extended through [Analysis Plugins](https://docs.codeclimate.com/docs/list-of-engines) or a [custom tool](#implementing-a-custom-tool). Going a step further, GitLab can show the Code Quality report right in the merge request widget area: diff --git a/lib/gitlab/experimentation.rb b/lib/gitlab/experimentation.rb index 7c59267c0b6..30c8eaf605a 100644 --- a/lib/gitlab/experimentation.rb +++ b/lib/gitlab/experimentation.rb @@ -40,7 +40,7 @@ module Gitlab extend ActiveSupport::Concern included do - before_action :set_experimentation_subject_id_cookie + before_action :set_experimentation_subject_id_cookie, unless: :dnt_enabled? helper_method :experiment_enabled? end @@ -56,7 +56,12 @@ module Gitlab end def experiment_enabled?(experiment_key) - Experimentation.enabled_for_user?(experiment_key, experimentation_subject_index) || forced_enabled?(experiment_key) + return false if dnt_enabled? + + return true if Experimentation.enabled_for_user?(experiment_key, experimentation_subject_index) + return true if forced_enabled?(experiment_key) + + false end def track_experiment_event(experiment_key, action, value = nil) @@ -73,6 +78,10 @@ module Gitlab private + def dnt_enabled? + Gitlab::Utils.to_boolean(request.headers['DNT']) + end + def experimentation_subject_id cookies.signed[:experimentation_subject_id] end diff --git a/spec/lib/gitlab/experimentation_spec.rb b/spec/lib/gitlab/experimentation_spec.rb index 1506794cbb5..a39c50ab038 100644 --- a/spec/lib/gitlab/experimentation_spec.rb +++ b/spec/lib/gitlab/experimentation_spec.rb @@ -30,7 +30,12 @@ describe Gitlab::Experimentation do end describe '#set_experimentation_subject_id_cookie' do + let(:do_not_track) { nil } + let(:cookie) { cookies.permanent.signed[:experimentation_subject_id] } + before do + request.headers['DNT'] = do_not_track if do_not_track.present? + get :index end @@ -46,12 +51,30 @@ describe Gitlab::Experimentation do context 'cookie is not present' do it 'sets a permanent signed cookie' do - expect(cookies.permanent.signed[:experimentation_subject_id]).to be_present + expect(cookie).to be_present + end + + context 'DNT: 0' do + let(:do_not_Track) { '0' } + + it 'sets a permanent signed cookie' do + expect(cookie).to be_present + end + end + + context 'DNT: 1' do + let(:do_not_track) { '1' } + + it 'does nothing' do + expect(cookie).not_to be_present + end end end end describe '#experiment_enabled?' do + subject { controller.experiment_enabled?(:test_experiment) } + context 'cookie is not present' do it 'calls Gitlab::Experimentation.enabled_for_user? with the name of the experiment and an experimentation_subject_index of nil' do expect(Gitlab::Experimentation).to receive(:enabled_for_user?).with(:test_experiment, nil) @@ -72,11 +95,25 @@ describe Gitlab::Experimentation do end end + it 'returns true when DNT: 0 is set in the request' do + allow(Gitlab::Experimentation).to receive(:enabled_for_user?) { true } + controller.request.headers['DNT'] = '0' + + is_expected.to be_truthy + end + + it 'returns false when DNT: 1 is set in the request' do + allow(Gitlab::Experimentation).to receive(:enabled_for_user?) { true } + controller.request.headers['DNT'] = '1' + + is_expected.to be_falsy + end + describe 'URL parameter to force enable experiment' do - it 'returns true' do + it 'returns true unconditionally' do get :index, params: { force_experiment: :test_experiment } - expect(controller.experiment_enabled?(:test_experiment)).to be_truthy + is_expected.to be_truthy end end end