diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 068bced378536b3a38438f93213178ca6357287e..5af665a17e0b1a936c75a2c79f9b55a91f1043d4 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.110.0 +0.111.0 diff --git a/GITLAB_PAGES_VERSION b/GITLAB_PAGES_VERSION index f374f6662e9a1983e9b8a534a3295df618772ffe..3eefcb9dd5b38e2c1dc061052455dd97bcd51e6c 100644 --- a/GITLAB_PAGES_VERSION +++ b/GITLAB_PAGES_VERSION @@ -1 +1 @@ -0.9.1 +1.0.0 diff --git a/Gemfile b/Gemfile index c7f5f71146afdeccb3e05dfd5b55a73b9a318ba9..71a3edae6d4660ca0fc7a0a5d1283a3b0283b07b 100644 --- a/Gemfile +++ b/Gemfile @@ -433,7 +433,7 @@ group :ed25519 do end # Gitaly GRPC client -gem 'gitaly-proto', '~> 0.103.0', require: 'gitaly' +gem 'gitaly-proto', '~> 0.105.0', require: 'gitaly' gem 'grpc', '~> 1.11.0' # Locked until https://github.com/google/protobuf/issues/4210 is closed diff --git a/Gemfile.lock b/Gemfile.lock index e0aa8c500f3eac6782af0d1d2dba5ba7857ff77a..f821474f271aa266613d55f8919ec8062f38427a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -306,7 +306,7 @@ GEM gettext_i18n_rails (>= 0.7.1) po_to_json (>= 1.0.0) rails (>= 3.2.0) - gitaly-proto (0.103.0) + gitaly-proto (0.105.0) google-protobuf (~> 3.1) grpc (~> 1.10) github-linguist (5.3.3) @@ -1071,7 +1071,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.3) - gitaly-proto (~> 0.103.0) + gitaly-proto (~> 0.105.0) github-linguist (~> 5.3.3) gitlab-flowdock-git-hook (~> 1.0.1) gitlab-gollum-lib (~> 4.2) diff --git a/Gemfile.rails5.lock b/Gemfile.rails5.lock index c32d8dcf99d2fb1239e86e3dc0b6ff787571666d..f1905fc21bf68d51b03e725dd22cbab80fc8e442 100644 --- a/Gemfile.rails5.lock +++ b/Gemfile.rails5.lock @@ -309,7 +309,7 @@ GEM gettext_i18n_rails (>= 0.7.1) po_to_json (>= 1.0.0) rails (>= 3.2.0) - gitaly-proto (0.103.0) + gitaly-proto (0.105.0) google-protobuf (~> 3.1) grpc (~> 1.10) github-linguist (5.3.3) @@ -1081,7 +1081,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.3) - gitaly-proto (~> 0.103.0) + gitaly-proto (~> 0.105.0) github-linguist (~> 5.3.3) gitlab-flowdock-git-hook (~> 1.0.1) gitlab-gollum-lib (~> 4.2) diff --git a/app/assets/javascripts/diffs/components/inline_diff_view.vue b/app/assets/javascripts/diffs/components/inline_diff_view.vue index b884230fb63fa60ee1887b85585a02a7b92164eb..83569346cf845881b2e0d33303d8df1039b525cc 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_view.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_view.vue @@ -20,16 +20,13 @@ export default { }, }, computed: { - ...mapGetters(['commit']), + ...mapGetters(['commitId']), normalizedDiffLines() { return this.diffLines.map(line => (line.richText ? trimFirstCharOfLineContent(line) : line)); }, diffLinesLength() { return this.normalizedDiffLines.length; }, - commitId() { - return this.commit && this.commit.id; - }, userColorScheme() { return window.gon.user_color_scheme; }, diff --git a/app/assets/javascripts/diffs/components/parallel_diff_view.vue b/app/assets/javascripts/diffs/components/parallel_diff_view.vue index 52561e197e6ded119f0d0707544b34148394a5d8..89148eb5e18f5d52a5f775b7652384d70feabac5 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_view.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_view.vue @@ -21,7 +21,7 @@ export default { }, }, computed: { - ...mapGetters(['commit']), + ...mapGetters(['commitId']), parallelDiffLines() { return this.diffLines.map(line => { const parallelLine = Object.assign({}, line); @@ -44,9 +44,6 @@ export default { diffLinesLength() { return this.parallelDiffLines.length; }, - commitId() { - return this.commit && this.commit.id; - }, userColorScheme() { return window.gon.user_color_scheme; }, diff --git a/app/assets/javascripts/diffs/store/getters.js b/app/assets/javascripts/diffs/store/getters.js index 66d0f47d1024a5e3021bc8938a685966e0be7f18..f3c2d7427e7c0fcfd09369a2f1c3f418ebf0a774 100644 --- a/app/assets/javascripts/diffs/store/getters.js +++ b/app/assets/javascripts/diffs/store/getters.js @@ -1,16 +1,12 @@ import { PARALLEL_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE } from '../constants'; -export default { - isParallelView(state) { - return state.diffViewType === PARALLEL_DIFF_VIEW_TYPE; - }, - isInlineView(state) { - return state.diffViewType === INLINE_DIFF_VIEW_TYPE; - }, - areAllFilesCollapsed(state) { - return state.diffFiles.every(file => file.collapsed); - }, - commit(state) { - return state.commit; - }, -}; +export const isParallelView = state => state.diffViewType === PARALLEL_DIFF_VIEW_TYPE; + +export const isInlineView = state => state.diffViewType === INLINE_DIFF_VIEW_TYPE; + +export const areAllFilesCollapsed = state => state.diffFiles.every(file => file.collapsed); + +export const commitId = state => (state.commit && state.commit.id ? state.commit.id : null); + +// prevent babel-plugin-rewire from generating an invalid default during karma tests +export default () => {}; diff --git a/app/assets/javascripts/diffs/store/modules/diff_state.js b/app/assets/javascripts/diffs/store/modules/diff_state.js new file mode 100644 index 0000000000000000000000000000000000000000..39d90a64aab0d7a8ac55dbcf65fea07cf1564c97 --- /dev/null +++ b/app/assets/javascripts/diffs/store/modules/diff_state.js @@ -0,0 +1,18 @@ +import Cookies from 'js-cookie'; +import { getParameterValues } from '~/lib/utils/url_utility'; +import { INLINE_DIFF_VIEW_TYPE, DIFF_VIEW_COOKIE_NAME } from '../../constants'; + +const viewTypeFromQueryString = getParameterValues('view')[0]; +const viewTypeFromCookie = Cookies.get(DIFF_VIEW_COOKIE_NAME); +const defaultViewType = INLINE_DIFF_VIEW_TYPE; + +export default () => ({ + isLoading: true, + endpoint: '', + basePath: '', + commit: null, + diffFiles: [], + mergeRequestDiffs: [], + diffLineCommentForms: {}, + diffViewType: viewTypeFromQueryString || viewTypeFromCookie || defaultViewType, +}); diff --git a/app/assets/javascripts/diffs/store/modules/index.js b/app/assets/javascripts/diffs/store/modules/index.js index 94caa1315067e885f48a76b86899da957585f110..c745320d532551fe6c27c068730c820f99743ba2 100644 --- a/app/assets/javascripts/diffs/store/modules/index.js +++ b/app/assets/javascripts/diffs/store/modules/index.js @@ -1,25 +1,10 @@ -import Cookies from 'js-cookie'; -import { getParameterValues } from '~/lib/utils/url_utility'; import actions from '../actions'; -import getters from '../getters'; +import * as getters from '../getters'; import mutations from '../mutations'; -import { INLINE_DIFF_VIEW_TYPE, DIFF_VIEW_COOKIE_NAME } from '../../constants'; - -const viewTypeFromQueryString = getParameterValues('view')[0]; -const viewTypeFromCookie = Cookies.get(DIFF_VIEW_COOKIE_NAME); -const defaultViewType = INLINE_DIFF_VIEW_TYPE; +import createState from './diff_state'; export default { - state: { - isLoading: true, - endpoint: '', - basePath: '', - commit: null, - diffFiles: [], - mergeRequestDiffs: [], - diffLineCommentForms: {}, - diffViewType: viewTypeFromQueryString || viewTypeFromCookie || defaultViewType, - }, + state: createState(), getters, actions, mutations, diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index 8aa8a114c6f219452dca33861121cddaab3202f9..a98b2be89a3c607a380f9bc42f835c3512f29fe0 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -66,15 +66,10 @@ export default { }, [types.EXPAND_ALL_FILES](state) { - const diffFiles = []; - - state.diffFiles.forEach(file => { - diffFiles.push({ - ...file, - collapsed: false, - }); - }); - - Object.assign(state, { diffFiles }); + // eslint-disable-next-line no-param-reassign + state.diffFiles = state.diffFiles.map(file => ({ + ...file, + collapsed: false, + })); }, }; diff --git a/app/assets/stylesheets/pages/boards.scss b/app/assets/stylesheets/pages/boards.scss index 1a5fc8d6ca5573999abed62d3af5cfd80cbf4612..7acd08ae1651947b82c4a2ffb4ae823b068481a0 100644 --- a/app/assets/stylesheets/pages/boards.scss +++ b/app/assets/stylesheets/pages/boards.scss @@ -117,7 +117,6 @@ overflow-x: scroll; white-space: nowrap; min-height: 200px; - display: flex; @include media-breakpoint-only(sm) { height: calc(100vh - #{$issue-board-list-difference-sm}); @@ -148,15 +147,17 @@ .board { display: inline-block; - flex: 1; - min-width: 300px; - max-width: 400px; + width: calc(85vw - 15px); height: 100%; padding-right: ($gl-padding / 2); padding-left: ($gl-padding / 2); white-space: normal; vertical-align: top; + @include media-breakpoint-up(sm) { + width: 400px; + } + &.is-expandable { .board-header { cursor: pointer; @@ -164,8 +165,6 @@ } &.is-collapsed { - flex: none; - min-width: 0; width: 50px; .board-header { diff --git a/app/controllers/concerns/group_tree.rb b/app/controllers/concerns/group_tree.rb index 56770a1740602cf9f764bf65bd524d5a9d5eea4a..6ec6897e707fc5789c50267f6e123cae23ce4a82 100644 --- a/app/controllers/concerns/group_tree.rb +++ b/app/controllers/concerns/group_tree.rb @@ -1,21 +1,16 @@ module GroupTree # rubocop:disable Gitlab/ModuleWithInstanceVariables def render_group_tree(groups) - @groups = if params[:filter].present? - # We find the ancestors by ID of the search results here. - # Otherwise the ancestors would also have filters applied, - # which would cause them not to be preloaded. - group_ids = groups.search(params[:filter]).select(:id) - Gitlab::GroupHierarchy.new(Group.where(id: group_ids)) - .base_and_ancestors - else - # Only show root groups if no parent-id is given - groups.where(parent_id: params[:parent_id]) - end + groups = groups.sort_by_attribute(@sort = params[:sort]) - @groups = @groups.with_selects_for_list(archived: params[:archived]) - .sort_by_attribute(@sort = params[:sort]) - .page(params[:page]) + groups = if params[:filter].present? + filtered_groups_with_ancestors(groups) + else + # If `params[:parent_id]` is `nil`, we will only show root-groups + groups.where(parent_id: params[:parent_id]).page(params[:page]) + end + + @groups = groups.with_selects_for_list(archived: params[:archived]) respond_to do |format| format.html @@ -28,4 +23,21 @@ def render_group_tree(groups) end # rubocop:enable Gitlab/ModuleWithInstanceVariables end + + def filtered_groups_with_ancestors(groups) + filtered_groups = groups.search(params[:filter]).page(params[:page]) + + if Group.supports_nested_groups? + # We find the ancestors by ID of the search results here. + # Otherwise the ancestors would also have filters applied, + # which would cause them not to be preloaded. + # + # Pagination needs to be applied before loading the ancestors to + # make sure ancestors are not cut off by pagination. + Gitlab::GroupHierarchy.new(Group.where(id: filtered_groups.select(:id))) + .base_and_ancestors + else + filtered_groups + end + end end diff --git a/app/models/concerns/group_descendant.rb b/app/models/concerns/group_descendant.rb index 261ace57a17745229cdec5635ced52dd2d33a5fa..5e9a95c32828d57089bbdbd9952bff7ea4b324d5 100644 --- a/app/models/concerns/group_descendant.rb +++ b/app/models/concerns/group_descendant.rb @@ -44,8 +44,8 @@ def expand_hierarchy_for_child(child, hierarchy, hierarchy_top, preloaded) This error is not user facing, but causes a +1 query. MSG extras = { - parent: parent, - child: child, + parent: parent.inspect, + child: child.inspect, preloaded: preloaded.map(&:full_path) } issue_url = 'https://gitlab.com/gitlab-org/gitlab-ce/issues/40785' diff --git a/app/models/repository.rb b/app/models/repository.rb index 094d2bbb2091d6fa231f0ce5f46529ce487f6af1..9569bb9d6e477df8b8a767da781116cc45826766 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -90,7 +90,7 @@ def cleanup @raw_repository&.cleanup end - # Return absolute path to repository + # Don't use this! It's going away. Use Gitaly to read or write from repos. def path_to_repo @path_to_repo ||= begin @@ -257,7 +257,7 @@ def keep_around(sha) # This will still fail if the file is corrupted (e.g. 0 bytes) raw_repository.write_ref(keep_around_ref_name(sha), sha, shell: false) rescue Gitlab::Git::CommandError => ex - Rails.logger.error "Unable to create keep-around reference for repository #{path}: #{ex}" + Rails.logger.error "Unable to create keep-around reference for repository #{disk_path}: #{ex}" end def kept_around?(sha) diff --git a/app/workers/repository_check/batch_worker.rb b/app/workers/repository_check/batch_worker.rb index d5d45a7821c6ef4d4bc932af17c81997ea51084b..849b70b4c5f7d963ea25ea62779aa60ba73d1740 100644 --- a/app/workers/repository_check/batch_worker.rb +++ b/app/workers/repository_check/batch_worker.rb @@ -6,9 +6,11 @@ class BatchWorker include ApplicationWorker include RepositoryCheckQueue + include ExclusiveLeaseGuard RUN_TIME = 3600 BATCH_SIZE = 10_000 + LEASE_TIMEOUT = 1.hour attr_reader :shard_name @@ -18,6 +20,20 @@ def perform(shard_name) return unless Gitlab::CurrentSettings.repository_checks_enabled return unless Gitlab::ShardHealthCache.healthy_shard?(shard_name) + try_obtain_lease do + perform_repository_checks + end + end + + def lease_timeout + LEASE_TIMEOUT + end + + def lease_key + "repository_check_batch_worker:#{shard_name}" + end + + def perform_repository_checks start = Time.now # This loop will break after a little more than one hour ('a little @@ -28,7 +44,7 @@ def perform(shard_name) project_ids.each do |project_id| break if Time.now - start >= RUN_TIME - next unless try_obtain_lease(project_id) + next unless try_obtain_lease_for_project(project_id) SingleRepositoryWorker.new.perform(project_id) end @@ -62,7 +78,7 @@ def projects_on_shard Project.where(repository_storage: shard_name) end - def try_obtain_lease(id) + def try_obtain_lease_for_project(id) # Use a 24-hour timeout because on servers/projects where 'git fsck' is # super slow we definitely do not want to run it twice in parallel. Gitlab::ExclusiveLease.new( diff --git a/app/workers/repository_check/dispatch_worker.rb b/app/workers/repository_check/dispatch_worker.rb index 891a273afd7417bd6b2ac79d720668c9755cc778..96634f09a15a15694286d2c0efdaa3aa9a2b34da 100644 --- a/app/workers/repository_check/dispatch_worker.rb +++ b/app/workers/repository_check/dispatch_worker.rb @@ -3,13 +3,22 @@ class DispatchWorker include ApplicationWorker include CronjobQueue include ::EachShardWorker + include ExclusiveLeaseGuard + + LEASE_TIMEOUT = 1.hour def perform return unless Gitlab::CurrentSettings.repository_checks_enabled - each_eligible_shard do |shard_name| - RepositoryCheck::BatchWorker.perform_async(shard_name) + try_obtain_lease do + each_eligible_shard do |shard_name| + RepositoryCheck::BatchWorker.perform_async(shard_name) + end end end + + def lease_timeout + LEASE_TIMEOUT + end end end diff --git a/changelogs/unreleased/bvl-preload-parents-after-pagination.yml b/changelogs/unreleased/bvl-preload-parents-after-pagination.yml new file mode 100644 index 0000000000000000000000000000000000000000..ff3d4716d349b531d5b2a9c885e76a2ed7c06a3f --- /dev/null +++ b/changelogs/unreleased/bvl-preload-parents-after-pagination.yml @@ -0,0 +1,5 @@ +--- +title: Reduce the number of queries when searching for groups +merge_request: 20398 +author: +type: performance diff --git a/changelogs/unreleased/fl-mr-refactor-performance-improvements.yml b/changelogs/unreleased/fl-mr-refactor-performance-improvements.yml new file mode 100644 index 0000000000000000000000000000000000000000..649d1b5da67927feec98ff505672976f625fa20c --- /dev/null +++ b/changelogs/unreleased/fl-mr-refactor-performance-improvements.yml @@ -0,0 +1,5 @@ +--- +title: Structure getters for diff Store properly and adds specs +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/rosulk-patch-12.yml b/changelogs/unreleased/rosulk-patch-12.yml deleted file mode 100644 index 9637c88d1a4e74e6becd1f2db84adcea10a22268..0000000000000000000000000000000000000000 --- a/changelogs/unreleased/rosulk-patch-12.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Flex issue board columns -merge_request: 19250 -author: Roman Rosluk -type: changed diff --git a/doc/integration/bitbucket.md b/doc/integration/bitbucket.md index 2a14c0397ca91d960ffc7b5bf22d6d77e2220cc1..9094d1f2419e21d431230aee03e109559a97d530 100644 --- a/doc/integration/bitbucket.md +++ b/doc/integration/bitbucket.md @@ -1,5 +1,8 @@ # Integrate your GitLab server with Bitbucket +NOTE: **Note:** +You need to [enable OmniAuth](omniauth.md) in order to use this. + Import projects from Bitbucket.org and login to your GitLab instance with your Bitbucket.org account. @@ -76,13 +79,13 @@ you to use. sudo -u git -H editor /home/git/gitlab/config/gitlab.yml ``` -1. Follow the [Initial OmniAuth Configuration](omniauth.md#initial-omniauth-configuration) - for initial settings. 1. Add the Bitbucket provider configuration: For Omnibus packages: ```ruby + gitlab_rails['omniauth_enabled'] = true + gitlab_rails['omniauth_providers'] = [ { "name" => "bitbucket", @@ -96,10 +99,13 @@ you to use. For installations from source: ```yaml - - { name: 'bitbucket', - app_id: 'BITBUCKET_APP_KEY', - app_secret: 'BITBUCKET_APP_SECRET', - url: 'https://bitbucket.org/' } + omniauth: + enabled: true + providers: + - { name: 'bitbucket', + app_id: 'BITBUCKET_APP_KEY', + app_secret: 'BITBUCKET_APP_SECRET', + url: 'https://bitbucket.org/' } ``` --- @@ -121,6 +127,9 @@ well, the user will be returned to GitLab and will be signed in. Once the above configuration is set up, you can use Bitbucket to sign into GitLab and [start importing your projects][bb-import]. +If you don't want to enable signing in with Bitbucket but just want to import +projects from Bitbucket, you could [disable it in the admin panel](omniauth.md#enable-or-disable-sign-in-with-an-omniauth-provider-without-disabling-import-sources). + [init-oauth]: omniauth.md#initial-omniauth-configuration [bb-import]: ../workflow/importing/import_projects_from_bitbucket.md [bb-old]: https://gitlab.com/gitlab-org/gitlab-ce/blob/8-14-stable/doc/integration/bitbucket.md diff --git a/doc/integration/saml.md b/doc/integration/saml.md index f1501c214d493f7a9da296ee60d5dad2253db2f6..341a0257de3f75efdcfd01a4481a26b30b51fa2d 100644 --- a/doc/integration/saml.md +++ b/doc/integration/saml.md @@ -1,5 +1,8 @@ # SAML OmniAuth Provider +NOTE: **Note:** +You need to [enable OmniAuth](omniauth.md) in order to use this. + GitLab can be configured to act as a SAML 2.0 Service Provider (SP). This allows GitLab to consume assertions from a SAML 2.0 Identity Provider (IdP) such as Microsoft ADFS to authenticate users. @@ -15,33 +18,33 @@ in your SAML IdP: For omnibus package: ```sh - sudo editor /etc/gitlab/gitlab.rb + sudo editor /etc/gitlab/gitlab.rb ``` For installations from source: ```sh - cd /home/git/gitlab + cd /home/git/gitlab - sudo -u git -H editor config/gitlab.yml + sudo -u git -H editor config/gitlab.yml ``` -1. See [Initial OmniAuth Configuration](omniauth.md#initial-omniauth-configuration) - for initial settings. - 1. To allow your users to use SAML to sign up without having to manually create an account first, don't forget to add the following values to your configuration: For omnibus package: ```ruby - gitlab_rails['omniauth_allow_single_sign_on'] = ['saml'] - gitlab_rails['omniauth_block_auto_created_users'] = false + gitlab_rails['omniauth_enabled'] = true + gitlab_rails['omniauth_allow_single_sign_on'] = ['saml'] + gitlab_rails['omniauth_block_auto_created_users'] = false ``` For installations from source: ```yaml + omniauth: + enabled: true allow_single_sign_on: ["saml"] block_auto_created_users: false ``` @@ -52,13 +55,13 @@ in your SAML IdP: For omnibus package: ```ruby - gitlab_rails['omniauth_auto_link_saml_user'] = true + gitlab_rails['omniauth_auto_link_saml_user'] = true ``` For installations from source: ```yaml - auto_link_saml_user: true + auto_link_saml_user: true ``` 1. Add the provider configuration: @@ -66,35 +69,37 @@ in your SAML IdP: For omnibus package: ```ruby - gitlab_rails['omniauth_providers'] = [ - { - name: 'saml', - args: { - assertion_consumer_service_url: 'https://gitlab.example.com/users/auth/saml/callback', - idp_cert_fingerprint: '43:51:43:a1:b5:fc:8b:b7:0a:3a:a9:b1:0f:66:73:a8', - idp_sso_target_url: 'https://login.example.com/idp', - issuer: 'https://gitlab.example.com', - name_identifier_format: 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent' - }, - label: 'Company Login' # optional label for SAML login button, defaults to "Saml" - } - ] - ``` - - For installations from source: - - ```yaml - - { - name: 'saml', - args: { + gitlab_rails['omniauth_providers'] = [ + { + name: 'saml', + args: { assertion_consumer_service_url: 'https://gitlab.example.com/users/auth/saml/callback', idp_cert_fingerprint: '43:51:43:a1:b5:fc:8b:b7:0a:3a:a9:b1:0f:66:73:a8', idp_sso_target_url: 'https://login.example.com/idp', issuer: 'https://gitlab.example.com', name_identifier_format: 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent' }, - label: 'Company Login' # optional label for SAML login button, defaults to "Saml" - } + label: 'Company Login' # optional label for SAML login button, defaults to "Saml" + } + ] + ``` + + For installations from source: + + ```yaml + omniauth: + providers: + - { + name: 'saml', + args: { + assertion_consumer_service_url: 'https://gitlab.example.com/users/auth/saml/callback', + idp_cert_fingerprint: '43:51:43:a1:b5:fc:8b:b7:0a:3a:a9:b1:0f:66:73:a8', + idp_sso_target_url: 'https://login.example.com/idp', + issuer: 'https://gitlab.example.com', + name_identifier_format: 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent' + }, + label: 'Company Login' # optional label for SAML login button, defaults to "Saml" + } ``` 1. Change the value for `assertion_consumer_service_url` to match the HTTPS endpoint @@ -140,8 +145,8 @@ This setting is only available on GitLab 8.7 and above. SAML login includes support for automatically identifying whether a user should be considered an [external](../user/permissions.md) user based on the user's group membership in the SAML identity provider. This feature **does not** allow you to -automatically add users to GitLab [Groups](../user/group/index.md), it simply -allows you to mark users as External if they are members of certain groups in the +automatically add users to GitLab [Groups](../user/group/index.md), it simply +allows you to mark users as External if they are members of certain groups in the Identity Provider. ### Requirements @@ -240,28 +245,28 @@ If you want some SAML authentication methods to count as 2FA on a per session ba 1. Edit `/etc/gitlab/gitlab.rb`: ```ruby - gitlab_rails['omniauth_providers'] = [ - { - name: 'saml', - args: { - assertion_consumer_service_url: 'https://gitlab.example.com/users/auth/saml/callback', - idp_cert_fingerprint: '43:51:43:a1:b5:fc:8b:b7:0a:3a:a9:b1:0f:66:73:a8', - idp_sso_target_url: 'https://login.example.com/idp', - issuer: 'https://gitlab.example.com', - name_identifier_format: 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent', - upstream_two_factor_authn_contexts: - %w( - urn:oasis:names:tc:SAML:2.0:ac:classes:CertificateProtectedTransport - urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorOTPSMS - urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorIGTOKEN - ) - - }, - label: 'Company Login' # optional label for SAML login button, defaults to "Saml" - } - ] + gitlab_rails['omniauth_providers'] = [ + { + name: 'saml', + args: { + assertion_consumer_service_url: 'https://gitlab.example.com/users/auth/saml/callback', + idp_cert_fingerprint: '43:51:43:a1:b5:fc:8b:b7:0a:3a:a9:b1:0f:66:73:a8', + idp_sso_target_url: 'https://login.example.com/idp', + issuer: 'https://gitlab.example.com', + name_identifier_format: 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent', + upstream_two_factor_authn_contexts: + %w( + urn:oasis:names:tc:SAML:2.0:ac:classes:CertificateProtectedTransport + urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorOTPSMS + urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorIGTOKEN + ) + + }, + label: 'Company Login' # optional label for SAML login button, defaults to "Saml" + } + ] ``` - + 1. Save the file and [reconfigure][] GitLab for the changes to take effect. --- @@ -269,40 +274,41 @@ If you want some SAML authentication methods to count as 2FA on a per session ba **For installations from source:** 1. Edit `config/gitlab.yml`: - - ```yaml - - { - name: 'saml', - args: { - assertion_consumer_service_url: 'https://gitlab.example.com/users/auth/saml/callback', - idp_cert_fingerprint: '43:51:43:a1:b5:fc:8b:b7:0a:3a:a9:b1:0f:66:73:a8', - idp_sso_target_url: 'https://login.example.com/idp', - issuer: 'https://gitlab.example.com', - name_identifier_format: 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent', - upstream_two_factor_authn_contexts: - [ - 'urn:oasis:names:tc:SAML:2.0:ac:classes:CertificateProtectedTransport', - 'urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorOTPSMS', - 'urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorIGTOKEN' - ] - - }, - label: 'Company Login' # optional label for SAML login button, defaults to "Saml" - } + + ```yaml + omniauth: + providers: + - { + name: 'saml', + args: { + assertion_consumer_service_url: 'https://gitlab.example.com/users/auth/saml/callback', + idp_cert_fingerprint: '43:51:43:a1:b5:fc:8b:b7:0a:3a:a9:b1:0f:66:73:a8', + idp_sso_target_url: 'https://login.example.com/idp', + issuer: 'https://gitlab.example.com', + name_identifier_format: 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent', + upstream_two_factor_authn_contexts: + [ + 'urn:oasis:names:tc:SAML:2.0:ac:classes:CertificateProtectedTransport', + 'urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorOTPSMS', + 'urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorIGTOKEN' + ] + }, + label: 'Company Login' # optional label for SAML login button, defaults to "Saml" + } ``` - + 1. Save the file and [restart GitLab][] for the changes ot take effect - + In addition to the changes in GitLab, make sure that your Idp is returning the `AuthnContext`. For example: ```xml - <saml:AuthnStatement> - <saml:AuthnContext> - <saml:AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:MediumStrongCertificateProtectedTransport</saml:AuthnContextClassRef> - </saml:AuthnContext> - </saml:AuthnStatement> +<saml:AuthnStatement> + <saml:AuthnContext> + <saml:AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:MediumStrongCertificateProtectedTransport</saml:AuthnContextClassRef> + </saml:AuthnContext> +</saml:AuthnStatement> ``` ## Customization diff --git a/lib/gitlab/database.rb b/lib/gitlab/database.rb index 0e9d22006a838df54d190c1f4cbd2b40069b160d..af8798cba8bc309b7416829f72ff9f54c0a1830b 100644 --- a/lib/gitlab/database.rb +++ b/lib/gitlab/database.rb @@ -42,6 +42,21 @@ def self.read_write? !self.read_only? end + # check whether the underlying database is in read-only mode + def self.db_read_only? + if postgresql? + ActiveRecord::Base.connection.execute('SELECT pg_is_in_recovery()') + .first + .fetch('pg_is_in_recovery') == 't' + else + false + end + end + + def self.db_read_write? + !self.db_read_only? + end + def self.version @version ||= database_version.match(/\A(?:PostgreSQL |)([^\s]+).*\z/)[1] end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index bbfe6ab1d9517310b4f41f858c41875b40da686a..29b3663a52a5c67af7fe94733761eda18f83c643 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -1115,8 +1115,18 @@ def write_config(full_path:) # This guard avoids Gitaly log/error spam raise NoRepository, 'repository does not exist' unless exists? + set_config('gitlab.fullpath' => full_path) + end + + def set_config(entries) + wrapped_gitaly_errors do + gitaly_repository_client.set_config(entries) + end + end + + def delete_config(*keys) wrapped_gitaly_errors do - gitaly_repository_client.write_config(full_path: full_path) + gitaly_repository_client.delete_config(keys) end end diff --git a/lib/gitlab/gitaly_client/repository_service.rb b/lib/gitlab/gitaly_client/repository_service.rb index 982f8d0963be499e66afb85644e28bbcf0746937..64b9af4d70c9355edd418e9f7958f8606cd4d2de 100644 --- a/lib/gitlab/gitaly_client/repository_service.rb +++ b/lib/gitlab/gitaly_client/repository_service.rb @@ -265,17 +265,39 @@ def write_ref(ref_path, ref, old_ref, shell) true end - def write_config(full_path:) - request = Gitaly::WriteConfigRequest.new(repository: @gitaly_repo, full_path: full_path) - response = GitalyClient.call( + def set_config(entries) + return if entries.empty? + + request = Gitaly::SetConfigRequest.new(repository: @gitaly_repo) + entries.each do |key, value| + request.entries << build_set_config_entry(key, value) + end + + GitalyClient.call( + @storage, + :repository_service, + :set_config, + request, + timeout: GitalyClient.fast_timeout + ) + + nil + end + + def delete_config(keys) + return if keys.empty? + + request = Gitaly::DeleteConfigRequest.new(repository: @gitaly_repo, keys: keys) + + GitalyClient.call( @storage, :repository_service, - :write_config, + :delete_config, request, timeout: GitalyClient.fast_timeout ) - raise Gitlab::Git::OSError.new(response.error) unless response.error.empty? + nil end def license_short_name @@ -352,6 +374,23 @@ def gitaly_repo_stream_request(file_path, rpc_name, request_class, timeout) timeout: timeout ) end + + def build_set_config_entry(key, value) + entry = Gitaly::SetConfigRequest::Entry.new(key: key) + + case value + when String + entry.value_str = value + when Integer + entry.value_int32 = value + when TrueClass, FalseClass + entry.value_bool = value + else + raise InvalidArgument, "invalid git config value: #{value.inspect}" + end + + entry + end end end end diff --git a/spec/controllers/concerns/group_tree_spec.rb b/spec/controllers/concerns/group_tree_spec.rb index ba84fbf8564cf4784cda8c265bea6f4dae5b0db7..503eb416962275ea8fa751af574792e2da78844e 100644 --- a/spec/controllers/concerns/group_tree_spec.rb +++ b/spec/controllers/concerns/group_tree_spec.rb @@ -63,6 +63,17 @@ def index expect(assigns(:groups)).to contain_exactly(parent, subgroup) end + + it 'preloads parents regardless of pagination' do + allow(Kaminari.config).to receive(:default_per_page).and_return(1) + group = create(:group, :public) + subgroup = create(:group, :public, parent: group) + search_result = create(:group, :public, name: 'result', parent: subgroup) + + get :index, filter: 'resu', format: :json + + expect(assigns(:groups)).to contain_exactly(group, subgroup, search_result) + end end context 'json content' do diff --git a/spec/javascripts/diffs/store/getters_spec.js b/spec/javascripts/diffs/store/getters_spec.js index 7945ddea91112ef3c604e1b7dbce29586895462c..7a94f18778b2d16a38cad732ad57f25848e0d327 100644 --- a/spec/javascripts/diffs/store/getters_spec.js +++ b/spec/javascripts/diffs/store/getters_spec.js @@ -1,24 +1,66 @@ -import getters from '~/diffs/store/getters'; +import * as getters from '~/diffs/store/getters'; +import state from '~/diffs/store/modules/diff_state'; import { PARALLEL_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE } from '~/diffs/constants'; describe('DiffsStoreGetters', () => { + let localState; + + beforeEach(() => { + localState = state(); + }); + describe('isParallelView', () => { it('should return true if view set to parallel view', () => { - expect(getters.isParallelView({ diffViewType: PARALLEL_DIFF_VIEW_TYPE })).toBeTruthy(); + localState.diffViewType = PARALLEL_DIFF_VIEW_TYPE; + + expect(getters.isParallelView(localState)).toEqual(true); }); it('should return false if view not to parallel view', () => { - expect(getters.isParallelView({ diffViewType: 'foo' })).toBeFalsy(); + localState.diffViewType = INLINE_DIFF_VIEW_TYPE; + + expect(getters.isParallelView(localState)).toEqual(false); }); }); describe('isInlineView', () => { it('should return true if view set to inline view', () => { - expect(getters.isInlineView({ diffViewType: INLINE_DIFF_VIEW_TYPE })).toBeTruthy(); + localState.diffViewType = INLINE_DIFF_VIEW_TYPE; + + expect(getters.isInlineView(localState)).toEqual(true); }); it('should return false if view not to inline view', () => { - expect(getters.isInlineView({ diffViewType: PARALLEL_DIFF_VIEW_TYPE })).toBeFalsy(); + localState.diffViewType = PARALLEL_DIFF_VIEW_TYPE; + + expect(getters.isInlineView(localState)).toEqual(false); + }); + }); + + describe('areAllFilesCollapsed', () => { + it('returns true when all files are collapsed', () => { + localState.diffFiles = [{ collapsed: true }, { collapsed: true }]; + expect(getters.areAllFilesCollapsed(localState)).toEqual(true); + }); + + it('returns false when at least one file is not collapsed', () => { + localState.diffFiles = [{ collapsed: false }, { collapsed: true }]; + expect(getters.areAllFilesCollapsed(localState)).toEqual(false); + }); + }); + + describe('commitId', () => { + it('returns commit id when is set', () => { + const commitID = '800f7a91'; + localState.commit = { + id: commitID, + }; + + expect(getters.commitId(localState)).toEqual(commitID); + }); + + it('returns null when no commit is set', () => { + expect(getters.commitId(localState)).toEqual(null); }); }); }); diff --git a/spec/lib/gitlab/database_spec.rb b/spec/lib/gitlab/database_spec.rb index 87cf9e2650a7aaebedac0688f25bafd2a451a38e..22bb9b5c72e007eaccc8a3f5cbd957e354dda04c 100644 --- a/spec/lib/gitlab/database_spec.rb +++ b/spec/lib/gitlab/database_spec.rb @@ -443,6 +443,7 @@ end end +<<<<<<< HEAD describe '#disable_prepared_statements' do it 'disables prepared statements' do config = {} @@ -488,6 +489,34 @@ it 'returns false' do expect(described_class.read_only?).to be_falsey end +======= + describe '.db_read_only?' do + context 'when using PostgreSQL' do + before do + allow(ActiveRecord::Base.connection).to receive(:execute).and_call_original + expect(described_class).to receive(:postgresql?).and_return(true) + end + + it 'detects a read only database' do + allow(ActiveRecord::Base.connection).to receive(:execute).with('SELECT pg_is_in_recovery()').and_return([{ "pg_is_in_recovery" => "t" }]) + + expect(described_class.db_read_only?).to be_truthy + end + + it 'detects a read write database' do + allow(ActiveRecord::Base.connection).to receive(:execute).with('SELECT pg_is_in_recovery()').and_return([{ "pg_is_in_recovery" => "f" }]) + + expect(described_class.db_read_only?).to be_falsey + end + end + + context 'when using MySQL' do + before do + expect(described_class).to receive(:postgresql?).and_return(false) + end + + it { expect(described_class.db_read_only?).to be_falsey } +>>>>>>> upstream/master end end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 5dd7af3a552fb498b9e51ee4c14416644e526d8d..e6268a05d441cca1e6045eecb60d34a27e0c6c75 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1856,6 +1856,54 @@ def commit_files(commit) end end + describe '#set_config' do + let(:repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } + let(:rugged) { repository_rugged } + let(:entries) do + { + 'test.foo1' => 'bla bla', + 'test.foo2' => 1234, + 'test.foo3' => true + } + end + + it 'can set config settings' do + expect(repository.set_config(entries)).to be_nil + + expect(rugged.config['test.foo1']).to eq('bla bla') + expect(rugged.config['test.foo2']).to eq('1234') + expect(rugged.config['test.foo3']).to eq('true') + end + + after do + entries.keys.each { |k| rugged.config.delete(k) } + end + end + + describe '#delete_config' do + let(:repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } + let(:rugged) { repository_rugged } + let(:entries) do + { + 'test.foo1' => 'bla bla', + 'test.foo2' => 1234, + 'test.foo3' => true + } + end + + it 'can delete config settings' do + entries.each do |key, value| + rugged.config[key] = value + end + + expect(repository.delete_config(*%w[does.not.exist test.foo1 test.foo2])).to be_nil + + config_keys = rugged.config.each_key.to_a + expect(config_keys).not_to include('test.foo1') + expect(config_keys).not_to include('test.foo2') + end + end + describe '#merge' do let(:repository) do Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') diff --git a/spec/workers/repository_check/batch_worker_spec.rb b/spec/workers/repository_check/batch_worker_spec.rb index 6bc551be9ad7086a9684da74ef1f060a655ae743..ede271b2cddbfc4aa1ff05c97a5c9ed287794f01 100644 --- a/spec/workers/repository_check/batch_worker_spec.rb +++ b/spec/workers/repository_check/batch_worker_spec.rb @@ -62,4 +62,12 @@ expect(subject.perform(shard_name)).to eq([]) end + + it 'does not run if the exclusive lease is taken' do + allow(subject).to receive(:try_obtain_lease).and_return(false) + + expect(subject).not_to receive(:perform_repository_checks) + + subject.perform(shard_name) + end end diff --git a/spec/workers/repository_check/dispatch_worker_spec.rb b/spec/workers/repository_check/dispatch_worker_spec.rb index 20a4f1f5344f5504636a68b76058f7e19299b07d..7877429aa8f0df18ff496b665e97772e758e0094 100644 --- a/spec/workers/repository_check/dispatch_worker_spec.rb +++ b/spec/workers/repository_check/dispatch_worker_spec.rb @@ -11,6 +11,14 @@ subject.perform end + it 'does nothing if the exclusive lease is taken' do + allow(subject).to receive(:try_obtain_lease).and_return(false) + + expect(RepositoryCheck::BatchWorker).not_to receive(:perform_async) + + subject.perform + end + it 'dispatches work to RepositoryCheck::BatchWorker' do expect(RepositoryCheck::BatchWorker).to receive(:perform_async).at_least(:once)