summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2021-03-22 12:09:02 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2021-03-22 12:09:02 +0000
commitcea25900b73fcb7029845eeca5e5a9170349f1dd (patch)
tree4d7c7c000de534569c65d71cc55e638a6a2dfbde
parentb014ff26b4416de3aa49d83483d8ce5d273c0708 (diff)
downloadgitlab-ce-cea25900b73fcb7029845eeca5e5a9170349f1dd.tar.gz
Add latest changes from gitlab-org/gitlab@master
-rw-r--r--GITALY_SERVER_VERSION2
-rw-r--r--app/models/concerns/safe_url.rb4
-rw-r--r--app/models/remote_mirror.rb2
-rw-r--r--app/views/users/terms/index.html.haml4
-rw-r--r--changelogs/unreleased/btn-confirm-users.yml5
-rw-r--r--changelogs/unreleased/qmnguyen0711-expand-a-histogram-bucket.yml5
-rw-r--r--doc/administration/pages/index.md5
-rw-r--r--doc/development/licensing.md7
-rw-r--r--lib/gitlab/metrics/subscribers/active_record.rb17
-rw-r--r--spec/lib/gitlab/metrics/subscribers/active_record_spec.rb170
-rw-r--r--spec/models/concerns/safe_url_spec.rb12
11 files changed, 163 insertions, 70 deletions
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION
index 0cbfc674111..b6707734a9a 100644
--- a/GITALY_SERVER_VERSION
+++ b/GITALY_SERVER_VERSION
@@ -1 +1 @@
-6314c40a2e0ae9d43eb4c01026d0aef57c10d91c
+cc879cfb5db4ed342c4f0ea744dbbfdc9649b35f
diff --git a/app/models/concerns/safe_url.rb b/app/models/concerns/safe_url.rb
index febca7d241f..7dce05bddba 100644
--- a/app/models/concerns/safe_url.rb
+++ b/app/models/concerns/safe_url.rb
@@ -3,12 +3,12 @@
module SafeUrl
extend ActiveSupport::Concern
- def safe_url(usernames_whitelist: [])
+ def safe_url(allowed_usernames: [])
return if url.nil?
uri = URI.parse(url)
uri.password = '*****' if uri.password
- uri.user = '*****' if uri.user && !usernames_whitelist.include?(uri.user)
+ uri.user = '*****' if uri.user && allowed_usernames.exclude?(uri.user)
uri.to_s
rescue URI::Error
end
diff --git a/app/models/remote_mirror.rb b/app/models/remote_mirror.rb
index 880970b72a8..1bde2727c44 100644
--- a/app/models/remote_mirror.rb
+++ b/app/models/remote_mirror.rb
@@ -207,7 +207,7 @@ class RemoteMirror < ApplicationRecord
end
def safe_url
- super(usernames_whitelist: %w[git])
+ super(allowed_usernames: %w[git])
end
def bare_url
diff --git a/app/views/users/terms/index.html.haml b/app/views/users/terms/index.html.haml
index da8b73fd4fd..73d0f51f9ac 100644
--- a/app/views/users/terms/index.html.haml
+++ b/app/views/users/terms/index.html.haml
@@ -6,11 +6,11 @@
.card-footer.footer-block.clearfix
- if can?(current_user, :accept_terms, @term)
.float-right
- = button_to accept_term_path(@term, redirect_params), class: 'gl-button btn btn-success gl-ml-3', data: { qa_selector: 'accept_terms_button' } do
+ = button_to accept_term_path(@term, redirect_params), class: 'gl-button btn btn-confirm gl-ml-3', data: { qa_selector: 'accept_terms_button' } do
= _('Accept terms')
- else
.float-right
- = link_to root_path, class: 'gl-button btn btn-success gl-ml-3' do
+ = link_to root_path, class: 'gl-button btn btn-confirm gl-ml-3' do
= _('Continue')
- if can?(current_user, :decline_terms, @term)
.float-right
diff --git a/changelogs/unreleased/btn-confirm-users.yml b/changelogs/unreleased/btn-confirm-users.yml
new file mode 100644
index 00000000000..2ebab7cea7d
--- /dev/null
+++ b/changelogs/unreleased/btn-confirm-users.yml
@@ -0,0 +1,5 @@
+---
+title: Move from btn-success to btn-confirm in users directory
+merge_request: 56945
+author: Yogi (@yo)
+type: changed
diff --git a/changelogs/unreleased/qmnguyen0711-expand-a-histogram-bucket.yml b/changelogs/unreleased/qmnguyen0711-expand-a-histogram-bucket.yml
new file mode 100644
index 00000000000..fe906d02551
--- /dev/null
+++ b/changelogs/unreleased/qmnguyen0711-expand-a-histogram-bucket.yml
@@ -0,0 +1,5 @@
+---
+title: Adjust gitlab_database_transaction_seconds histogram bucket
+merge_request: 56952
+author:
+type: changed
diff --git a/doc/administration/pages/index.md b/doc/administration/pages/index.md
index 3728948ef28..8e4e2f34fc5 100644
--- a/doc/administration/pages/index.md
+++ b/doc/administration/pages/index.md
@@ -600,8 +600,9 @@ the below steps to do a no downtime transfer to a new storage location.
## Configure listener for reverse proxy requests
-Follow the steps below to configure the proxy listener of GitLab Pages. [Introduced](https://gitlab.com/gitlab-org/omnibus-gitlab/-/merge_requests/2533) in
-Omnibus GitLab 11.1.
+> [Introduced](https://gitlab.com/gitlab-org/omnibus-gitlab/-/merge_requests/2533) in Omnibus GitLab 11.1.
+
+Follow the steps below to configure the proxy listener of GitLab Pages.
1. By default the listener is configured to listen for requests on `localhost:8090`.
diff --git a/doc/development/licensing.md b/doc/development/licensing.md
index c467fe81755..1ab56e60a0b 100644
--- a/doc/development/licensing.md
+++ b/doc/development/licensing.md
@@ -18,9 +18,6 @@ Some gems may not include their license information in their `gemspec` file, and
### License Finder commands
-NOTE:
-License Finder currently uses GitLab misused terms of `whitelist` and `blacklist`. As a result, the commands below reference those terms. We've created an [issue on their project](https://github.com/pivotal/LicenseFinder/issues/745) to propose that they rename their commands.
-
There are a few basic commands License Finder provides that you need in order to manage license detection.
To verify that the checks are passing, and/or to see what dependencies are causing the checks to fail:
@@ -32,13 +29,13 @@ bundle exec license_finder
To allowlist a new license:
```shell
-license_finder whitelist add MIT
+license_finder permitted_licenses add MIT
```
To denylist a new license:
```shell
-license_finder blacklist add Unlicense
+license_finder restricted_licenses add Unlicense
```
To tell License Finder about a dependency's license if it isn't auto-detected:
diff --git a/lib/gitlab/metrics/subscribers/active_record.rb b/lib/gitlab/metrics/subscribers/active_record.rb
index 5eefef02507..6de180337fc 100644
--- a/lib/gitlab/metrics/subscribers/active_record.rb
+++ b/lib/gitlab/metrics/subscribers/active_record.rb
@@ -11,13 +11,16 @@ module Gitlab
DB_COUNTERS = %i{db_count db_write_count db_cached_count}.freeze
SQL_COMMANDS_WITH_COMMENTS_REGEX = /\A(\/\*.*\*\/\s)?((?!(.*[^\w'"](DELETE|UPDATE|INSERT INTO)[^\w'"])))(WITH.*)?(SELECT)((?!(FOR UPDATE|FOR SHARE)).)*$/i.freeze
- DURATION_BUCKET = [0.05, 0.1, 0.25].freeze
+ SQL_DURATION_BUCKET = [0.05, 0.1, 0.25].freeze
+ TRANSACTION_DURATION_BUCKET = [0.1, 0.25, 1].freeze
# This event is published from ActiveRecordBaseTransactionMetrics and
# used to record a database transaction duration when calling
# ActiveRecord::Base.transaction {} block.
def transaction(event)
- observe(:gitlab_database_transaction_seconds, event)
+ observe(:gitlab_database_transaction_seconds, event) do
+ buckets TRANSACTION_DURATION_BUCKET
+ end
end
def sql(event)
@@ -33,7 +36,9 @@ module Gitlab
increment(:db_cached_count) if cached_query?(payload)
increment(:db_write_count) unless select_sql_command?(payload)
- observe(:gitlab_sql_duration_seconds, event)
+ observe(:gitlab_sql_duration_seconds, event) do
+ buckets SQL_DURATION_BUCKET
+ end
end
def self.db_counter_payload
@@ -66,10 +71,8 @@ module Gitlab
Gitlab::SafeRequestStore[counter] = Gitlab::SafeRequestStore[counter].to_i + 1
end
- def observe(histogram, event)
- current_transaction&.observe(histogram, event.duration / 1000.0) do
- buckets DURATION_BUCKET
- end
+ def observe(histogram, event, &block)
+ current_transaction&.observe(histogram, event.duration / 1000.0, &block)
end
def current_transaction
diff --git a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb
index dffd37eeb9d..9939e680fa9 100644
--- a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb
+++ b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb
@@ -8,65 +8,145 @@ RSpec.describe Gitlab::Metrics::Subscribers::ActiveRecord do
let(:env) { {} }
let(:subscriber) { described_class.new }
let(:connection) { double(:connection) }
- let(:payload) { { sql: 'SELECT * FROM users WHERE id = 10', connection: connection } }
-
- let(:event) do
- double(
- :event,
- name: 'sql.active_record',
- duration: 2,
- payload: payload
- )
- end
- # Emulate Marginalia pre-pending comments
- def sql(query, comments: true)
- if comments && !%w[BEGIN COMMIT].include?(query)
- "/*application:web,controller:badges,action:pipeline,correlation_id:01EYN39K9VMJC56Z7808N7RSRH*/ #{query}"
- else
- query
+ describe '#transaction' do
+ let(:web_transaction) { double('Gitlab::Metrics::WebTransaction') }
+ let(:background_transaction) { double('Gitlab::Metrics::WebTransaction') }
+
+ let(:event) do
+ double(
+ :event,
+ name: 'transaction.active_record',
+ duration: 230,
+ payload: { connection: connection }
+ )
end
- end
- shared_examples 'track generic sql events' do
- where(:name, :sql_query, :record_query, :record_write_query, :record_cached_query) do
- 'SQL' | 'SELECT * FROM users WHERE id = 10' | true | false | false
- 'SQL' | 'WITH active_milestones AS (SELECT COUNT(*), state FROM milestones GROUP BY state) SELECT * FROM active_milestones' | true | false | false
- 'SQL' | 'SELECT * FROM users WHERE id = 10 FOR UPDATE' | true | true | false
- 'SQL' | 'WITH archived_rows AS (SELECT * FROM users WHERE archived = true) INSERT INTO products_log SELECT * FROM archived_rows' | true | true | false
- 'SQL' | 'DELETE FROM users where id = 10' | true | true | false
- 'SQL' | 'INSERT INTO project_ci_cd_settings (project_id) SELECT id FROM projects' | true | true | false
- 'SQL' | 'UPDATE users SET admin = true WHERE id = 10' | true | true | false
- 'CACHE' | 'SELECT * FROM users WHERE id = 10' | true | false | true
- 'SCHEMA' | "SELECT attr.attname FROM pg_attribute attr INNER JOIN pg_constraint cons ON attr.attrelid = cons.conrelid AND attr.attnum = any(cons.conkey) WHERE cons.contype = 'p' AND cons.conrelid = '\"projects\"'::regclass" | false | false | false
- nil | 'BEGIN' | false | false | false
- nil | 'COMMIT' | false | false | false
+ before do
+ allow(background_transaction).to receive(:observe)
+ allow(web_transaction).to receive(:observe)
end
- with_them do
- let(:payload) { { name: name, sql: sql(sql_query, comments: comments), connection: connection } }
+ context 'when both web and background transaction are available' do
+ before do
+ allow(::Gitlab::Metrics::WebTransaction).to receive(:current)
+ .and_return(web_transaction)
+ allow(::Gitlab::Metrics::BackgroundTransaction).to receive(:current)
+ .and_return(background_transaction)
+ end
+
+ it 'captures the metrics for web only' do
+ expect(web_transaction).to receive(:observe).with(:gitlab_database_transaction_seconds, 0.23)
- it 'marks the current thread as using the database' do
- # since it would already have been toggled by other specs
- Thread.current[:uses_db_connection] = nil
+ expect(background_transaction).not_to receive(:observe)
+ expect(background_transaction).not_to receive(:increment)
- expect { subscriber.sql(event) }.to change { Thread.current[:uses_db_connection] }.from(nil).to(true)
+ subscriber.transaction(event)
end
+ end
+
+ context 'when web transaction is available' do
+ let(:web_transaction) { double('Gitlab::Metrics::WebTransaction') }
+
+ before do
+ allow(::Gitlab::Metrics::WebTransaction).to receive(:current)
+ .and_return(web_transaction)
+ allow(::Gitlab::Metrics::BackgroundTransaction).to receive(:current)
+ .and_return(nil)
+ end
+
+ it 'captures the metrics for web only' do
+ expect(web_transaction).to receive(:observe).with(:gitlab_database_transaction_seconds, 0.23)
- it_behaves_like 'record ActiveRecord metrics'
- it_behaves_like 'store ActiveRecord info in RequestStore'
+ expect(background_transaction).not_to receive(:observe)
+ expect(background_transaction).not_to receive(:increment)
+
+ subscriber.transaction(event)
+ end
end
- end
- context 'without Marginalia comments' do
- let(:comments) { false }
+ context 'when background transaction is available' do
+ let(:background_transaction) { double('Gitlab::Metrics::BackgroundTransaction') }
+
+ before do
+ allow(::Gitlab::Metrics::WebTransaction).to receive(:current)
+ .and_return(nil)
+ allow(::Gitlab::Metrics::BackgroundTransaction).to receive(:current)
+ .and_return(background_transaction)
+ end
- it_behaves_like 'track generic sql events'
+ it 'captures the metrics for web only' do
+ expect(background_transaction).to receive(:observe).with(:gitlab_database_transaction_seconds, 0.23)
+
+ expect(web_transaction).not_to receive(:observe)
+ expect(web_transaction).not_to receive(:increment)
+
+ subscriber.transaction(event)
+ end
+ end
end
- context 'with Marginalia comments' do
- let(:comments) { true }
+ describe '#sql' do
+ let(:payload) { { sql: 'SELECT * FROM users WHERE id = 10', connection: connection } }
- it_behaves_like 'track generic sql events'
+ let(:event) do
+ double(
+ :event,
+ name: 'sql.active_record',
+ duration: 2,
+ payload: payload
+ )
+ end
+
+ # Emulate Marginalia pre-pending comments
+ def sql(query, comments: true)
+ if comments && !%w[BEGIN COMMIT].include?(query)
+ "/*application:web,controller:badges,action:pipeline,correlation_id:01EYN39K9VMJC56Z7808N7RSRH*/ #{query}"
+ else
+ query
+ end
+ end
+
+ shared_examples 'track generic sql events' do
+ where(:name, :sql_query, :record_query, :record_write_query, :record_cached_query) do
+ 'SQL' | 'SELECT * FROM users WHERE id = 10' | true | false | false
+ 'SQL' | 'WITH active_milestones AS (SELECT COUNT(*), state FROM milestones GROUP BY state) SELECT * FROM active_milestones' | true | false | false
+ 'SQL' | 'SELECT * FROM users WHERE id = 10 FOR UPDATE' | true | true | false
+ 'SQL' | 'WITH archived_rows AS (SELECT * FROM users WHERE archived = true) INSERT INTO products_log SELECT * FROM archived_rows' | true | true | false
+ 'SQL' | 'DELETE FROM users where id = 10' | true | true | false
+ 'SQL' | 'INSERT INTO project_ci_cd_settings (project_id) SELECT id FROM projects' | true | true | false
+ 'SQL' | 'UPDATE users SET admin = true WHERE id = 10' | true | true | false
+ 'CACHE' | 'SELECT * FROM users WHERE id = 10' | true | false | true
+ 'SCHEMA' | "SELECT attr.attname FROM pg_attribute attr INNER JOIN pg_constraint cons ON attr.attrelid = cons.conrelid AND attr.attnum = any(cons.conkey) WHERE cons.contype = 'p' AND cons.conrelid = '\"projects\"'::regclass" | false | false | false
+ nil | 'BEGIN' | false | false | false
+ nil | 'COMMIT' | false | false | false
+ end
+
+ with_them do
+ let(:payload) { { name: name, sql: sql(sql_query, comments: comments), connection: connection } }
+
+ it 'marks the current thread as using the database' do
+ # since it would already have been toggled by other specs
+ Thread.current[:uses_db_connection] = nil
+
+ expect { subscriber.sql(event) }.to change { Thread.current[:uses_db_connection] }.from(nil).to(true)
+ end
+
+ it_behaves_like 'record ActiveRecord metrics'
+ it_behaves_like 'store ActiveRecord info in RequestStore'
+ end
+ end
+
+ context 'without Marginalia comments' do
+ let(:comments) { false }
+
+ it_behaves_like 'track generic sql events'
+ end
+
+ context 'with Marginalia comments' do
+ let(:comments) { true }
+
+ it_behaves_like 'track generic sql events'
+ end
end
end
diff --git a/spec/models/concerns/safe_url_spec.rb b/spec/models/concerns/safe_url_spec.rb
index 3d38c05bf11..c298e56b1b1 100644
--- a/spec/models/concerns/safe_url_spec.rb
+++ b/spec/models/concerns/safe_url_spec.rb
@@ -26,14 +26,16 @@ RSpec.describe SafeUrl do
context 'when URL contains credentials' do
let(:url) { 'http://foo:bar@example.com' }
- it { is_expected.to eq('http://*****:*****@example.com')}
+ it 'masks username and password' do
+ is_expected.to eq('http://*****:*****@example.com')
+ end
- context 'when username is whitelisted' do
- subject { test_class.safe_url(usernames_whitelist: usernames_whitelist) }
+ context 'when username is allowed' do
+ subject { test_class.safe_url(allowed_usernames: usernames) }
- let(:usernames_whitelist) { %w[foo] }
+ let(:usernames) { %w[foo] }
- it 'does expect the whitelisted username not to be masked' do
+ it 'masks the password, but not the username' do
is_expected.to eq('http://foo:*****@example.com')
end
end