From cbcbe4b7ea295218732f03a48189d54faf66f5cd Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Thu, 24 Aug 2017 15:02:48 +0200 Subject: Use google-protobuf 3.4.0.2 --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 7d3c53ee010..8d0fec735f8 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -329,7 +329,7 @@ GEM multi_json (~> 1.10) retriable (~> 1.4) signet (~> 0.6) - google-protobuf (3.3.0) + google-protobuf (3.4.0.2) googleauth (0.5.1) faraday (~> 0.9) jwt (~> 1.4) -- cgit v1.2.1 From 884dbf0c6707b590fd4259b0644a1171e0f81cb4 Mon Sep 17 00:00:00 2001 From: Joshua Lambert Date: Fri, 25 Aug 2017 15:12:53 -0400 Subject: Update k8s docs --- doc/install/kubernetes/gitlab_chart.md | 6 ++---- doc/install/kubernetes/gitlab_omnibus.md | 8 +++----- doc/install/kubernetes/gitlab_runner_chart.md | 8 +++----- doc/install/kubernetes/index.md | 22 +--------------------- 4 files changed, 9 insertions(+), 35 deletions(-) diff --git a/doc/install/kubernetes/gitlab_chart.md b/doc/install/kubernetes/gitlab_chart.md index 81057736e3a..4f5a3028692 100644 --- a/doc/install/kubernetes/gitlab_chart.md +++ b/doc/install/kubernetes/gitlab_chart.md @@ -1,5 +1,5 @@ # GitLab Helm Chart -> These Helm charts are in beta. GitLab is working on a [cloud-native](http://docs.gitlab.com/omnibus/package-information/cloud_native.html) set of [Charts](https://gitlab.com/charts/helm.gitlab.io) which will replace these. +> These Helm charts are in beta. GitLab is working on a [cloud-native set of Charts](https://gitlab.com/charts/helm.gitlab.io/blob/master/README.md) which will replace these. > Officially supported cloud providers are Google Container Service and Azure Container Service. @@ -22,9 +22,7 @@ This chart includes the following: - [Persistent Volume](https://kubernetes.io/docs/concepts/storage/persistent-volumes/) provisioner support in the underlying infrastructure - The ability to point a DNS entry or URL at your GitLab install - The `kubectl` CLI installed locally and authenticated for the cluster -- The Helm Client installed locally -- The Helm Server (Tiller) already installed and running in the cluster, by running `helm init` -- The GitLab Helm Repo [added to your Helm Client](index.md#add-the-gitlab-helm-repository) +- The [Helm client](https://github.com/kubernetes/helm/blob/master/docs/quickstart.md) installed ## Configuring GitLab diff --git a/doc/install/kubernetes/gitlab_omnibus.md b/doc/install/kubernetes/gitlab_omnibus.md index 05e0a59ffeb..4a71ff599a8 100644 --- a/doc/install/kubernetes/gitlab_omnibus.md +++ b/doc/install/kubernetes/gitlab_omnibus.md @@ -1,5 +1,5 @@ # GitLab-Omnibus Helm Chart -> These Helm charts are in beta. GitLab is working on a [cloud-native](http://docs.gitlab.com/omnibus/package-information/cloud_native.html) set of [Charts](https://gitlab.com/charts/helm.gitlab.io) which will replace these. +> These Helm charts are in beta. GitLab is working on a [cloud-native set of Charts](https://gitlab.com/charts/helm.gitlab.io/blob/master/README.md) which will replace these. > Officially supported cloud providers are Google Container Service and Azure Container Service. @@ -35,9 +35,7 @@ Terms: - An [external IP address](#networking-prerequisites) - A [wildcard DNS entry](#networking-prerequisites), which resolves to the external IP address - The `kubectl` CLI installed locally and authenticated for the cluster -- The Helm Client installed locally -- The Helm Server (Tiller) already installed and running in the cluster, by running `helm init` -- The GitLab Helm Repo [added to your Helm Client](index.md#add-the-gitlab-helm-repository) +- The [Helm client](https://github.com/kubernetes/helm/blob/master/docs/quickstart.md) installed ### Networking Prerequisites @@ -126,7 +124,7 @@ Let's Encrypt limits a single TLD to five certificate requests within a single w ## Installing GitLab using the Helm Chart > You may see a temporary error message `SchedulerPredicates failed due to PersistentVolumeClaim is not bound` while storage provisions. Once the storage provisions, the pods will automatically restart. This may take a couple minutes depending on your cloud provider. If the error persists, please review the [prerequisites](#prerequisites) to ensure you have enough RAM, CPU, and storage. -Ensure the GitLab repo has been added and re-initialize Helm: +Add the GitLab Helm repository and initialize Helm: ```bash helm repo add gitlab https://charts.gitlab.io diff --git a/doc/install/kubernetes/gitlab_runner_chart.md b/doc/install/kubernetes/gitlab_runner_chart.md index 51f94a33109..5d080800279 100644 --- a/doc/install/kubernetes/gitlab_runner_chart.md +++ b/doc/install/kubernetes/gitlab_runner_chart.md @@ -1,5 +1,5 @@ # GitLab Runner Helm Chart -> These Helm charts are in beta. GitLab is working on a [cloud-native](http://docs.gitlab.com/omnibus/package-information/cloud_native.html) set of [Charts](https://gitlab.com/charts/helm.gitlab.io) which will replace these. +> These Helm charts are in beta. GitLab is working on a [cloud-native set of Charts](https://gitlab.com/charts/helm.gitlab.io/blob/master/README.md) which will replace these. > Officially supported cloud providers are Google Container Service and Azure Container Service. @@ -17,9 +17,7 @@ This chart configures the Runner to: - Your GitLab Server's API is reachable from the cluster - Kubernetes 1.4+ with Beta APIs enabled - The `kubectl` CLI installed locally and authenticated for the cluster -- The Helm Client installed locally -- The Helm Server (Tiller) already installed and running in the cluster, by running `helm init` -- The GitLab Helm Repo added to your Helm Client. See [Adding GitLab Helm Repo](index.md#add-the-gitlab-helm-repository) +- The [Helm client](https://github.com/kubernetes/helm/blob/master/docs/quickstart.md) installed ## Configuring GitLab Runner using the Helm Chart @@ -190,7 +188,7 @@ certsSecretName: ## Installing GitLab Runner using the Helm Chart -Ensure the GitLab repo has been added and re-initialize Helm: +Add the GitLab Helm repository and initialize Helm: ```bash helm repo add gitlab https://charts.gitlab.io diff --git a/doc/install/kubernetes/index.md b/doc/install/kubernetes/index.md index eb98dc06a18..d5ea81b13df 100644 --- a/doc/install/kubernetes/index.md +++ b/doc/install/kubernetes/index.md @@ -1,5 +1,5 @@ # Installing GitLab on Kubernetes -> These Helm charts are in beta. GitLab is working on a [cloud-native](http://docs.gitlab.com/omnibus/package-information/cloud_native.html) set of [Charts](https://gitlab.com/charts/helm.gitlab.io) which will replace these. +> These Helm charts are in beta. GitLab is working on a [cloud-native set of Charts](https://gitlab.com/charts/helm.gitlab.io/blob/master/README.md) which will replace these. > Officially supported cloud providers are Google Container Service and Azure Container Service. @@ -14,25 +14,6 @@ You can report any issues related to GitLab's Helm Charts at https://gitlab.com/charts/charts.gitlab.io/issues. Contributions and improvements are also very welcome. -## Prerequisites - -To use the charts, the Helm tool must be installed and initialized. The best -place to start is by reviewing the [Helm Quick Start Guide][helm-quick]. - -## Add the GitLab Helm repository - -Once Helm has been installed, the GitLab chart repository must be added: - -```bash -helm repo add gitlab https://charts.gitlab.io -``` - -After adding the repository, Helm must be re-initialized: - -```bash -helm init -``` - ## Using the GitLab Helm Charts GitLab makes available three Helm Charts. @@ -44,5 +25,4 @@ GitLab makes available three Helm Charts. We are also working on a new set of [cloud native Charts](https://gitlab.com/charts/helm.gitlab.io) which will eventually replace these. [chart]: https://github.com/kubernetes/charts -[helm-quick]: https://github.com/kubernetes/helm/blob/master/docs/quickstart.md [helm]: https://github.com/kubernetes/helm/blob/master/README.md -- cgit v1.2.1 From be925c0c75471120ebcf390d3339bd64575d5e69 Mon Sep 17 00:00:00 2001 From: Joshua Lambert Date: Fri, 25 Aug 2017 15:17:46 -0400 Subject: Minor tweaks --- doc/install/kubernetes/gitlab_chart.md | 4 ++-- doc/install/kubernetes/gitlab_omnibus.md | 2 +- doc/install/kubernetes/gitlab_runner_chart.md | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/install/kubernetes/gitlab_chart.md b/doc/install/kubernetes/gitlab_chart.md index 4f5a3028692..b64e86d57af 100644 --- a/doc/install/kubernetes/gitlab_chart.md +++ b/doc/install/kubernetes/gitlab_chart.md @@ -22,7 +22,7 @@ This chart includes the following: - [Persistent Volume](https://kubernetes.io/docs/concepts/storage/persistent-volumes/) provisioner support in the underlying infrastructure - The ability to point a DNS entry or URL at your GitLab install - The `kubectl` CLI installed locally and authenticated for the cluster -- The [Helm client](https://github.com/kubernetes/helm/blob/master/docs/quickstart.md) installed +- The [Helm client](https://github.com/kubernetes/helm/blob/master/docs/quickstart.md) installed locally on your machine ## Configuring GitLab @@ -426,7 +426,7 @@ ingress: ## Installing GitLab using the Helm Chart > You may see a temporary error message `SchedulerPredicates failed due to PersistentVolumeClaim is not bound` while storage provisions. Once the storage provisions, the pods will automatically restart. This may take a couple minutes depending on your cloud provider. If the error persists, please review the [prerequisites](#prerequisites) to ensure you have enough RAM, CPU, and storage. -Ensure the GitLab repo has been added and re-initialize Helm: +Add the GitLab Helm repository and initialize Helm: ```bash helm repo add gitlab https://charts.gitlab.io diff --git a/doc/install/kubernetes/gitlab_omnibus.md b/doc/install/kubernetes/gitlab_omnibus.md index 4a71ff599a8..374e693ba5d 100644 --- a/doc/install/kubernetes/gitlab_omnibus.md +++ b/doc/install/kubernetes/gitlab_omnibus.md @@ -35,7 +35,7 @@ Terms: - An [external IP address](#networking-prerequisites) - A [wildcard DNS entry](#networking-prerequisites), which resolves to the external IP address - The `kubectl` CLI installed locally and authenticated for the cluster -- The [Helm client](https://github.com/kubernetes/helm/blob/master/docs/quickstart.md) installed +- The [Helm client](https://github.com/kubernetes/helm/blob/master/docs/quickstart.md) installed locally on your machine ### Networking Prerequisites diff --git a/doc/install/kubernetes/gitlab_runner_chart.md b/doc/install/kubernetes/gitlab_runner_chart.md index 5d080800279..04f74be4454 100644 --- a/doc/install/kubernetes/gitlab_runner_chart.md +++ b/doc/install/kubernetes/gitlab_runner_chart.md @@ -17,7 +17,7 @@ This chart configures the Runner to: - Your GitLab Server's API is reachable from the cluster - Kubernetes 1.4+ with Beta APIs enabled - The `kubectl` CLI installed locally and authenticated for the cluster -- The [Helm client](https://github.com/kubernetes/helm/blob/master/docs/quickstart.md) installed +- The [Helm client](https://github.com/kubernetes/helm/blob/master/docs/quickstart.md) installed locally on your machine ## Configuring GitLab Runner using the Helm Chart -- cgit v1.2.1 From bc68bccab507b244f5246cb3449e8dfd0662d573 Mon Sep 17 00:00:00 2001 From: Manu Date: Fri, 25 Aug 2017 21:03:37 +0000 Subject: Added missing `S` to CI documentation for using SSh keys --- doc/ci/ssh_keys/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/ci/ssh_keys/README.md b/doc/ci/ssh_keys/README.md index cf25a8b618f..cdb9858e179 100644 --- a/doc/ci/ssh_keys/README.md +++ b/doc/ci/ssh_keys/README.md @@ -42,7 +42,7 @@ It is also good practice to check the server's own public key to make sure you are not being targeted by a man-in-the-middle attack. To do this, add another variable named `SSH_SERVER_HOSTKEYS`. To find out the hostkeys of your server, run the `ssh-keyscan YOUR_SERVER` command from a trusted network (ideally, from the -server itself), and paste its output into the `SSH_SERVER_HOSTKEY` variable. If +server itself), and paste its output into the `SSH_SERVER_HOSTKEYS` variable. If you need to connect to multiple servers, concatenate all the server public keys that you collected into the **Value** of the variable. There must be one key per line. -- cgit v1.2.1 From a30257c0321e872646fe945739482fdeadbba4c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Tue, 15 Aug 2017 12:03:54 -0500 Subject: Add some data attributes for options in namespace selector --- app/helpers/namespaces_helper.rb | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/app/helpers/namespaces_helper.rb b/app/helpers/namespaces_helper.rb index 7f656b8caae..df8b247aff5 100644 --- a/app/helpers/namespaces_helper.rb +++ b/app/helpers/namespaces_helper.rb @@ -4,7 +4,9 @@ module NamespacesHelper end def namespaces_options(selected = :current_user, display_path: false, extra_group: nil) - groups = current_user.owned_groups + current_user.masters_groups + groups = current_user.owned_groups + current_user.masters_groups + users = [current_user.namespace] + options = [] unless extra_group.nil? || extra_group.is_a?(Group) extra_group = Group.find(extra_group) if Namespace.find(extra_group).kind == 'group' @@ -14,22 +16,8 @@ module NamespacesHelper groups |= [extra_group] end - users = [current_user.namespace] - - data_attr_group = { 'data-options-parent' => 'groups' } - data_attr_users = { 'data-options-parent' => 'users' } - - group_opts = [ - "Groups", groups.sort_by(&:human_name).map { |g| [display_path ? g.full_path : g.human_name, g.id, data_attr_group] } - ] - - users_opts = [ - "Users", users.sort_by(&:human_name).map { |u| [display_path ? u.path : u.human_name, u.id, data_attr_users] } - ] - - options = [] - options << group_opts - options << users_opts + options << options_for_group(groups, display_path) + options << options_for_group(users, display_path) if selected == :current_user && current_user.namespace selected = current_user.namespace.id @@ -45,4 +33,17 @@ module NamespacesHelper avatar_icon(namespace.owner.email, size) end end + + private + + def options_for_group(namespaces, display_path) + type = namespaces.first.is_a?(Group) ? 'group' : 'users' + + elements = namespaces.sort_by(&:human_name).map! do |n| + [display_path ? n.full_path : n.human_name, n.id, + data: { options_parent: type, visibility_level: n.visibility_level_value, name: n.human_name }] + end + + [type.camelize, elements] + end end -- cgit v1.2.1 From d413f8e4e426e2cb2dc61d5a72d84a7dc67a28c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Tue, 15 Aug 2017 20:25:47 -0500 Subject: Add validation for visibility level of sub groups Sub groups should not have a visibility level higher than its parent. --- app/models/group.rb | 9 +++++++++ spec/models/group_spec.rb | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/app/models/group.rb b/app/models/group.rb index 2816a68257c..15355418d05 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -26,6 +26,7 @@ class Group < Namespace validate :avatar_type, if: ->(user) { user.avatar.present? && user.avatar_changed? } validate :visibility_level_allowed_by_projects + validate :visibility_level_allowed_by_parent validates :avatar, file_size: { maximum: 200.kilobytes.to_i } @@ -102,6 +103,14 @@ class Group < Namespace full_name end + def visibility_level_allowed_by_parent + return if parent_id.blank? + + if parent && (visibility_level > parent.visibility_level) + errors.add(:visibility_level, "#{visibility} is not allowed since the parent group has a #{parent.visibility} visibility.") + end + end + def visibility_level_allowed_by_projects allowed_by_projects = self.projects.where('visibility_level > ?', self.visibility_level).none? diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index c5bfae47606..a3310cf1dce 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -84,6 +84,39 @@ describe Group do expect(group).not_to be_valid end end + + describe '#visibility_level_allowed_by_parent' do + let(:parent) { create(:group, :internal) } + let(:sub_group) { build(:group, parent_id: parent.id) } + + context 'without a parent' do + it 'is valid' do + sub_group.parent_id = nil + + expect(sub_group).to be_valid + end + end + + context 'with a parent' do + context 'when visibility of sub group is greater than the parent' do + it 'is invalid' do + sub_group.visibility_level = Gitlab::VisibilityLevel::PUBLIC + + expect(sub_group).to be_invalid + end + end + + context 'when visibility of sub group is lower or equal to the parent' do + [Gitlab::VisibilityLevel::INTERNAL, Gitlab::VisibilityLevel::PRIVATE].each do |level| + it 'is valid' do + sub_group.visibility_level = level + + expect(sub_group).to be_valid + end + end + end + end + end end describe '.visible_to_user' do -- cgit v1.2.1 From b2b9d63f9b7301cbef9d1e1b8d4ad3cefeacf35d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Wed, 23 Aug 2017 11:51:11 -0500 Subject: Add validation to check visibility level of sub groups. --- app/models/group.rb | 20 ++++++++++++++++---- spec/models/group_spec.rb | 44 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 4 deletions(-) diff --git a/app/models/group.rb b/app/models/group.rb index 15355418d05..fdd175341b3 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -26,6 +26,7 @@ class Group < Namespace validate :avatar_type, if: ->(user) { user.avatar.present? && user.avatar_changed? } validate :visibility_level_allowed_by_projects + validate :visibility_level_allowed_by_sub_groups, if: :visibility_level_changed? validate :visibility_level_allowed_by_parent validates :avatar, file_size: { maximum: 200.kilobytes.to_i } @@ -112,14 +113,25 @@ class Group < Namespace end def visibility_level_allowed_by_projects - allowed_by_projects = self.projects.where('visibility_level > ?', self.visibility_level).none? + check_visibility_level_for(:projects) + end + + def visibility_level_allowed_by_sub_groups + check_visibility_level_for(:children) + end - unless allowed_by_projects + def check_visibility_level_for(children_type) + base_query = public_send(children_type) + children_have_higher_visibility = base_query.where('visibility_level > ?', visibility_level).exists? + + if children_have_higher_visibility + children_label = children_type == :projects ? 'projects' : 'sub groups' level_name = Gitlab::VisibilityLevel.level_name(visibility_level).downcase - self.errors.add(:visibility_level, "#{level_name} is not allowed since there are projects with higher visibility.") + + self.errors.add(:visibility_level, "#{level_name} is not allowed since there are #{children_label} with higher visibility.") end - allowed_by_projects + children_have_higher_visibility end def avatar_url(**args) diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index a3310cf1dce..c3342afb7fd 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -117,6 +117,50 @@ describe Group do end end end + + describe '#visibility_level_allowed_by_projects' do + let!(:internal_group) { create(:group, :internal) } + let!(:internal_project) { create(:project, :internal, group: internal_group) } + + context 'when group has a lower visibility' do + it 'is invalid' do + internal_group.visibility_level = Gitlab::VisibilityLevel::PRIVATE + + expect(internal_group).to be_invalid + expect(internal_group.errors[:visibility_level]).to include('private is not allowed since there are projects with higher visibility.') + end + end + + context 'when group has a higher visibility' do + it 'is valid' do + internal_group.visibility_level = Gitlab::VisibilityLevel::PUBLIC + + expect(internal_group).to be_valid + end + end + end + + describe '#visibility_level_allowed_by_sub_groups' do + let!(:internal_group) { create(:group, :internal) } + let!(:internal_sub_group) { create(:group, :internal, parent: internal_group) } + + context 'when parent group has a lower visibility' do + it 'is invalid' do + internal_group.visibility_level = Gitlab::VisibilityLevel::PRIVATE + + expect(internal_group).to be_invalid + expect(internal_group.errors[:visibility_level]).to include('private is not allowed since there are sub groups with higher visibility.') + end + end + + context 'when parent group has a higher visibility' do + it 'is valid' do + internal_group.visibility_level = Gitlab::VisibilityLevel::PUBLIC + + expect(internal_group).to be_valid + end + end + end end describe '.visible_to_user' do -- cgit v1.2.1 From af6968a15859a309cbb93a0327fc1d4be36041bc Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Thu, 24 Aug 2017 16:21:10 -0500 Subject: separate visibility_level_allowed logic from model validators --- app/models/group.rb | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/app/models/group.rb b/app/models/group.rb index fdd175341b3..b093e0b200c 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -105,33 +105,35 @@ class Group < Namespace end def visibility_level_allowed_by_parent - return if parent_id.blank? + return if visibility_level_allowed_by_parent? - if parent && (visibility_level > parent.visibility_level) - errors.add(:visibility_level, "#{visibility} is not allowed since the parent group has a #{parent.visibility} visibility.") - end + errors.add(:visibility_level, "#{visibility} is not allowed since the parent group has a #{parent.visibility} visibility.") end def visibility_level_allowed_by_projects - check_visibility_level_for(:projects) + return if visibility_level_allowed_by_projects? + + errors.add(:visibility_level, "#{visibility} is not allowed since this group contains projects with higher visibility.") end def visibility_level_allowed_by_sub_groups - check_visibility_level_for(:children) + return if visibility_level_allowed_by_sub_groups? + + errors.add(:visibility_level, "#{visibility} is not allowed since there are sub-groups with higher visibility.") end - def check_visibility_level_for(children_type) - base_query = public_send(children_type) - children_have_higher_visibility = base_query.where('visibility_level > ?', visibility_level).exists? + def visibility_level_allowed_by_parent?(level = self.visibility_level) + return true unless parent_id.present? || parent - if children_have_higher_visibility - children_label = children_type == :projects ? 'projects' : 'sub groups' - level_name = Gitlab::VisibilityLevel.level_name(visibility_level).downcase + level <= parent.visibility_level + end - self.errors.add(:visibility_level, "#{level_name} is not allowed since there are #{children_label} with higher visibility.") - end + def visibility_level_allowed_by_projects?(level = self.visibility_level) + projects.where('visibility_level > ?', level).none? + end - children_have_higher_visibility + def visibility_level_allowed_by_sub_groups?(level = self.visibility_level) + children.where('visibility_level > ?', level).none? end def avatar_url(**args) -- cgit v1.2.1 From 37edce19dd2f006ef2117ca8a9f05398e704a11c Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Thu, 24 Aug 2017 17:19:49 -0500 Subject: recognize instances where group visibility levels are unavailable --- app/helpers/visibility_level_helper.rb | 3 ++- app/models/group.rb | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/app/helpers/visibility_level_helper.rb b/app/helpers/visibility_level_helper.rb index 35755bc149b..ecc554f85d0 100644 --- a/app/helpers/visibility_level_helper.rb +++ b/app/helpers/visibility_level_helper.rb @@ -96,6 +96,7 @@ module VisibilityLevelHelper to: :current_application_settings def skip_level?(form_model, level) - form_model.is_a?(Project) && !form_model.visibility_level_allowed?(level) + return false unless form_model.respond_to?(:visibility_level_allowed?) + !form_model.visibility_level_allowed?(level) end end diff --git a/app/models/group.rb b/app/models/group.rb index b093e0b200c..257a5075d2c 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -136,6 +136,12 @@ class Group < Namespace children.where('visibility_level > ?', level).none? end + def visibility_level_allowed?(level = self.visibility_level) + visibility_level_allowed_by_parent?(level) && + visibility_level_allowed_by_projects?(level) && + visibility_level_allowed_by_sub_groups?(level) + end + def avatar_url(**args) # We use avatar_path instead of overriding avatar_url because of carrierwave. # See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11001/diffs#note_28659864 -- cgit v1.2.1 From 852f50977111fc11511682217e81cdce908c318f Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Thu, 24 Aug 2017 17:24:29 -0500 Subject: rename skip_level? to disallowed_visibitility_level? --- app/helpers/visibility_level_helper.rb | 2 +- app/views/shared/_visibility_radios.html.haml | 2 +- spec/helpers/visibility_level_helper_spec.rb | 26 +++++++++++++------------- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/app/helpers/visibility_level_helper.rb b/app/helpers/visibility_level_helper.rb index ecc554f85d0..c8401b784f6 100644 --- a/app/helpers/visibility_level_helper.rb +++ b/app/helpers/visibility_level_helper.rb @@ -95,7 +95,7 @@ module VisibilityLevelHelper :default_group_visibility, to: :current_application_settings - def skip_level?(form_model, level) + def disallowed_visibility_level?(form_model, level) return false unless form_model.respond_to?(:visibility_level_allowed?) !form_model.visibility_level_allowed?(level) end diff --git a/app/views/shared/_visibility_radios.html.haml b/app/views/shared/_visibility_radios.html.haml index 182c4eebd50..7b9453943d1 100644 --- a/app/views/shared/_visibility_radios.html.haml +++ b/app/views/shared/_visibility_radios.html.haml @@ -1,5 +1,5 @@ - Gitlab::VisibilityLevel.values.each do |level| - - next if skip_level?(form_model, level) + - next if disallowed_visibility_level?(form_model, level) .radio - restricted = restricted_visibility_levels.include?(level) = form.label "#{model_method}_#{level}" do diff --git a/spec/helpers/visibility_level_helper_spec.rb b/spec/helpers/visibility_level_helper_spec.rb index c3cccbb0d95..428eb664138 100644 --- a/spec/helpers/visibility_level_helper_spec.rb +++ b/spec/helpers/visibility_level_helper_spec.rb @@ -58,35 +58,35 @@ describe VisibilityLevelHelper do end end - describe "skip_level?" do + describe "disallowed_visibility_level?" do describe "forks" do let(:project) { create(:project, :internal) } let(:fork_project) { create(:project, forked_from_project: project) } - it "skips levels" do - expect(skip_level?(fork_project, Gitlab::VisibilityLevel::PUBLIC)).to be_truthy - expect(skip_level?(fork_project, Gitlab::VisibilityLevel::INTERNAL)).to be_falsey - expect(skip_level?(fork_project, Gitlab::VisibilityLevel::PRIVATE)).to be_falsey + it "disallows levels" do + expect(disallowed_visibility_level?(fork_project, Gitlab::VisibilityLevel::PUBLIC)).to be_truthy + expect(disallowed_visibility_level?(fork_project, Gitlab::VisibilityLevel::INTERNAL)).to be_falsey + expect(disallowed_visibility_level?(fork_project, Gitlab::VisibilityLevel::PRIVATE)).to be_falsey end end describe "non-forked project" do let(:project) { create(:project, :internal) } - it "skips levels" do - expect(skip_level?(project, Gitlab::VisibilityLevel::PUBLIC)).to be_falsey - expect(skip_level?(project, Gitlab::VisibilityLevel::INTERNAL)).to be_falsey - expect(skip_level?(project, Gitlab::VisibilityLevel::PRIVATE)).to be_falsey + it "disallows levels" do + expect(disallowed_visibility_level?(project, Gitlab::VisibilityLevel::PUBLIC)).to be_falsey + expect(disallowed_visibility_level?(project, Gitlab::VisibilityLevel::INTERNAL)).to be_falsey + expect(disallowed_visibility_level?(project, Gitlab::VisibilityLevel::PRIVATE)).to be_falsey end end describe "Snippet" do let(:snippet) { create(:snippet, :internal) } - it "skips levels" do - expect(skip_level?(snippet, Gitlab::VisibilityLevel::PUBLIC)).to be_falsey - expect(skip_level?(snippet, Gitlab::VisibilityLevel::INTERNAL)).to be_falsey - expect(skip_level?(snippet, Gitlab::VisibilityLevel::PRIVATE)).to be_falsey + it "disallows levels" do + expect(disallowed_visibility_level?(snippet, Gitlab::VisibilityLevel::PUBLIC)).to be_falsey + expect(disallowed_visibility_level?(snippet, Gitlab::VisibilityLevel::INTERNAL)).to be_falsey + expect(disallowed_visibility_level?(snippet, Gitlab::VisibilityLevel::PRIVATE)).to be_falsey end end end -- cgit v1.2.1 From 96c2672da8edff081702702a4a499bae14d27ff6 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Thu, 24 Aug 2017 17:55:43 -0500 Subject: add tests for groups within visibility level helper --- spec/helpers/visibility_level_helper_spec.rb | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/spec/helpers/visibility_level_helper_spec.rb b/spec/helpers/visibility_level_helper_spec.rb index 428eb664138..9a85fbe5258 100644 --- a/spec/helpers/visibility_level_helper_spec.rb +++ b/spec/helpers/visibility_level_helper_spec.rb @@ -80,7 +80,28 @@ describe VisibilityLevelHelper do end end - describe "Snippet" do + describe "group" do + let(:group) { create(:group, :internal) } + + it "disallows levels" do + expect(disallowed_visibility_level?(group, Gitlab::VisibilityLevel::PUBLIC)).to be_falsey + expect(disallowed_visibility_level?(group, Gitlab::VisibilityLevel::INTERNAL)).to be_falsey + expect(disallowed_visibility_level?(group, Gitlab::VisibilityLevel::PRIVATE)).to be_falsey + end + end + + describe "sub-group" do + let(:group) { create(:group, :private) } + let(:subgroup) { create(:group, :private, parent: group) } + + it "disallows levels" do + expect(disallowed_visibility_level?(subgroup, Gitlab::VisibilityLevel::PUBLIC)).to be_truthy + expect(disallowed_visibility_level?(subgroup, Gitlab::VisibilityLevel::INTERNAL)).to be_truthy + expect(disallowed_visibility_level?(subgroup, Gitlab::VisibilityLevel::PRIVATE)).to be_falsey + end + end + + describe "snippet" do let(:snippet) { create(:snippet, :internal) } it "disallows levels" do -- cgit v1.2.1 From d6d713f6bac9cd1840e45ed43911b99adba9c07b Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Fri, 25 Aug 2017 11:57:16 -0500 Subject: fix failing tests due to new group visibility restrictions --- spec/features/projects/new_project_spec.rb | 2 +- spec/models/group_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/features/projects/new_project_spec.rb b/spec/features/projects/new_project_spec.rb index 22fb1223739..9f8b1996249 100644 --- a/spec/features/projects/new_project_spec.rb +++ b/spec/features/projects/new_project_spec.rb @@ -87,7 +87,7 @@ feature 'New project' do end context 'with subgroup namespace' do - let(:group) { create(:group, :private, owner: user) } + let(:group) { create(:group, owner: user) } let(:subgroup) { create(:group, parent: group) } before do diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index c3342afb7fd..f9cd12c0ff3 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -127,7 +127,7 @@ describe Group do internal_group.visibility_level = Gitlab::VisibilityLevel::PRIVATE expect(internal_group).to be_invalid - expect(internal_group.errors[:visibility_level]).to include('private is not allowed since there are projects with higher visibility.') + expect(internal_group.errors[:visibility_level]).to include('private is not allowed since this group contains projects with higher visibility.') end end @@ -149,7 +149,7 @@ describe Group do internal_group.visibility_level = Gitlab::VisibilityLevel::PRIVATE expect(internal_group).to be_invalid - expect(internal_group.errors[:visibility_level]).to include('private is not allowed since there are sub groups with higher visibility.') + expect(internal_group.errors[:visibility_level]).to include('private is not allowed since there are sub-groups with higher visibility.') end end -- cgit v1.2.1 From 69f679ed3476a887e67a591114c50ebcd1efa1a6 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Fri, 25 Aug 2017 13:00:01 -0500 Subject: rename .project-visibility-level-holder to .visibility-level-setting and move from projects.scss to settings.scss --- app/assets/stylesheets/pages/projects.scss | 22 ---------------------- app/assets/stylesheets/pages/settings.scss | 22 ++++++++++++++++++++++ .../admin/application_settings/_form.html.haml | 6 +++--- app/views/projects/new.html.haml | 2 +- app/views/shared/_visibility_level.html.haml | 2 +- 5 files changed, 27 insertions(+), 27 deletions(-) diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index 39c4264e496..19caefa1961 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -299,28 +299,6 @@ } } -.project-visibility-level-holder { - .radio { - margin-bottom: 10px; - - i { - margin: 2px 0; - font-size: 20px; - } - - .option-title { - font-weight: $gl-font-weight-normal; - display: inline-block; - color: $gl-text-color; - } - - .option-descr { - margin-left: 29px; - color: $project-option-descr-color; - } - } -} - .save-project-loader { margin-top: 50px; margin-bottom: 50px; diff --git a/app/assets/stylesheets/pages/settings.scss b/app/assets/stylesheets/pages/settings.scss index 15df51e9c69..9a142f338e8 100644 --- a/app/assets/stylesheets/pages/settings.scss +++ b/app/assets/stylesheets/pages/settings.scss @@ -143,6 +143,28 @@ } } +.visibility-level-setting { + .radio { + margin-bottom: 10px; + + i { + margin: 2px 0; + font-size: 20px; + } + + .option-title { + font-weight: $gl-font-weight-normal; + display: inline-block; + color: $gl-text-color; + } + + .option-descr { + margin-left: 29px; + color: $project-option-descr-color; + } + } +} + .prometheus-metrics-monitoring { .panel { .panel-toggle { diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index 959af5c0d13..734a08c61fa 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -7,15 +7,15 @@ = f.label :default_branch_protection, class: 'control-label col-sm-2' .col-sm-10 = f.select :default_branch_protection, options_for_select(Gitlab::Access.protection_options, @application_setting.default_branch_protection), {}, class: 'form-control' - .form-group.project-visibility-level-holder + .form-group.visibility-level-setting = f.label :default_project_visibility, class: 'control-label col-sm-2' .col-sm-10 = render('shared/visibility_radios', model_method: :default_project_visibility, form: f, selected_level: @application_setting.default_project_visibility, form_model: Project.new) - .form-group.project-visibility-level-holder + .form-group.visibility-level-setting = f.label :default_snippet_visibility, class: 'control-label col-sm-2' .col-sm-10 = render('shared/visibility_radios', model_method: :default_snippet_visibility, form: f, selected_level: @application_setting.default_snippet_visibility, form_model: ProjectSnippet.new) - .form-group.project-visibility-level-holder + .form-group.visibility-level-setting = f.label :default_group_visibility, class: 'control-label col-sm-2' .col-sm-10 = render('shared/visibility_radios', model_method: :default_group_visibility, form: f, selected_level: @application_setting.default_group_visibility, form_model: Group.new) diff --git a/app/views/projects/new.html.haml b/app/views/projects/new.html.haml index 647e0a772b1..e2ea96b855d 100644 --- a/app/views/projects/new.html.haml +++ b/app/views/projects/new.html.haml @@ -111,7 +111,7 @@ %span.light (optional) = f.text_area :description, placeholder: 'Description format', class: "form-control", rows: 3, maxlength: 250 - .form-group.project-visibility-level-holder + .form-group.visibility-level-setting = f.label :visibility_level, class: 'label-light' do Visibility Level = link_to icon('question-circle'), help_page_path("public_access/public_access"), aria: { label: 'Documentation for Visibility Level' } diff --git a/app/views/shared/_visibility_level.html.haml b/app/views/shared/_visibility_level.html.haml index 73efec88bb1..192d2502aaf 100644 --- a/app/views/shared/_visibility_level.html.haml +++ b/app/views/shared/_visibility_level.html.haml @@ -1,6 +1,6 @@ - with_label = local_assigns.fetch(:with_label, true) -.form-group.project-visibility-level-holder +.form-group.visibility-level-setting - if with_label = f.label :visibility_level, class: 'control-label' do Visibility Level -- cgit v1.2.1 From 10a7478ed0b9cbc4b3d4a316f4e124796dbac495 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Fri, 25 Aug 2017 13:46:49 -0500 Subject: fix disabled state style for visibility level options --- app/assets/stylesheets/pages/settings.scss | 8 +++++++- app/views/shared/_visibility_radios.html.haml | 9 +++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/app/assets/stylesheets/pages/settings.scss b/app/assets/stylesheets/pages/settings.scss index 9a142f338e8..f7f8119994b 100644 --- a/app/assets/stylesheets/pages/settings.scss +++ b/app/assets/stylesheets/pages/settings.scss @@ -147,7 +147,7 @@ .radio { margin-bottom: 10px; - i { + i.fa { margin: 2px 0; font-size: 20px; } @@ -162,6 +162,12 @@ margin-left: 29px; color: $project-option-descr-color; } + + &.disabled { + i.fa { + opacity: 0.5; + } + } } } diff --git a/app/views/shared/_visibility_radios.html.haml b/app/views/shared/_visibility_radios.html.haml index 7b9453943d1..d8d1093d4e3 100644 --- a/app/views/shared/_visibility_radios.html.haml +++ b/app/views/shared/_visibility_radios.html.haml @@ -1,9 +1,10 @@ - Gitlab::VisibilityLevel.values.each do |level| - - next if disallowed_visibility_level?(form_model, level) - .radio - - restricted = restricted_visibility_levels.include?(level) + - disallowed = disallowed_visibility_level?(form_model, level) + - restricted = restricted_visibility_levels.include?(level) + - disabled = disallowed || restricted + .radio{ class: [('disabled' if disabled), ('restricted' if restricted)] } = form.label "#{model_method}_#{level}" do - = form.radio_button model_method, level, checked: (selected_level == level), disabled: restricted + = form.radio_button model_method, level, checked: (selected_level == level), disabled: disabled = visibility_level_icon(level) .option-title = visibility_level_label(level) -- cgit v1.2.1 From 04e3e609daa54a62b0caf5a5ed5ade1cdb8c5eae Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Fri, 25 Aug 2017 13:48:15 -0500 Subject: show alternate description text for disabled visibility settings --- app/assets/stylesheets/pages/settings.scss | 15 ++++++++++++++- app/views/shared/_visibility_radios.html.haml | 11 ++++++----- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/app/assets/stylesheets/pages/settings.scss b/app/assets/stylesheets/pages/settings.scss index f7f8119994b..41a6ba2023a 100644 --- a/app/assets/stylesheets/pages/settings.scss +++ b/app/assets/stylesheets/pages/settings.scss @@ -158,15 +158,28 @@ color: $gl-text-color; } - .option-descr { + .option-description, + .option-disabled-reason { margin-left: 29px; color: $project-option-descr-color; } + .option-disabled-reason { + display: none; + } + &.disabled { i.fa { opacity: 0.5; } + + .option-description { + display: none; + } + + .option-disabled-reason { + display: block; + } } } } diff --git a/app/views/shared/_visibility_radios.html.haml b/app/views/shared/_visibility_radios.html.haml index d8d1093d4e3..8a80ccd4030 100644 --- a/app/views/shared/_visibility_radios.html.haml +++ b/app/views/shared/_visibility_radios.html.haml @@ -8,9 +8,10 @@ = visibility_level_icon(level) .option-title = visibility_level_label(level) - .option-descr + .option-description = visibility_level_description(level, form_model) -- unless restricted_visibility_levels.empty? - %div - %span.info - Some visibility level settings have been restricted by the administrator. + .option-disabled-reason + - if restricted + This visibility level has been restricted by the administrator. + - elsif disallowed + This option is not available the visibility of parent or child items prevents it. -- cgit v1.2.1 From 8cf504a71c326033a5a0885fa950a7d2c37ca93c Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Fri, 25 Aug 2017 14:07:44 -0500 Subject: don't check disabled radio inputs --- app/views/shared/_visibility_radios.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/shared/_visibility_radios.html.haml b/app/views/shared/_visibility_radios.html.haml index 8a80ccd4030..13eeaeefa9f 100644 --- a/app/views/shared/_visibility_radios.html.haml +++ b/app/views/shared/_visibility_radios.html.haml @@ -4,7 +4,7 @@ - disabled = disallowed || restricted .radio{ class: [('disabled' if disabled), ('restricted' if restricted)] } = form.label "#{model_method}_#{level}" do - = form.radio_button model_method, level, checked: (selected_level == level), disabled: disabled + = form.radio_button model_method, level, checked: (selected_level == level && !disabled), disabled: disabled = visibility_level_icon(level) .option-title = visibility_level_label(level) -- cgit v1.2.1 From fc95395c5dc8b297e831a51bcec04c0644eb06d2 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Fri, 25 Aug 2017 15:01:56 -0500 Subject: display specific reasons when visibility options are disabled --- app/helpers/visibility_level_helper.rb | 50 +++++++++++++++++++++++++++ app/views/shared/_visibility_radios.html.haml | 4 +-- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/app/helpers/visibility_level_helper.rb b/app/helpers/visibility_level_helper.rb index c8401b784f6..347f796fdc1 100644 --- a/app/helpers/visibility_level_helper.rb +++ b/app/helpers/visibility_level_helper.rb @@ -63,6 +63,56 @@ module VisibilityLevelHelper end end + def restricted_visibility_level_description(level) + level_name = Gitlab::VisibilityLevel.level_name(level) + "#{level_name.capitalize} visibilitiy has been restricted by the administrator." + end + + def disallowed_visibility_level_description(level, form_model) + case form_model + when Project + disallowed_project_visibility_level_description(level, form_model) + when Group + disallowed_group_visibility_level_description(level, form_model) + end + end + + def disallowed_project_visibility_level_description(level, project) + level_name = Gitlab::VisibilityLevel.level_name(level).downcase + reasons = [] + + unless project.visibility_level_allowed_as_fork?(level) + reasons << "the fork source project has lower visibility" + end + + unless project.visibility_level_allowed_by_group?(level) + reasons << "the visibility of #{project.group.name} is #{project.group.visibility}" + end + + reasons = reasons.any? ? ' because ' + reasons.to_sentence : '' + "This project cannot be #{level_name}#{reasons}." + end + + def disallowed_group_visibility_level_description(level, group) + level_name = Gitlab::VisibilityLevel.level_name(level).downcase + reasons = [] + + unless group.visibility_level_allowed_by_projects?(level) + reasons << "it contains projects with higher visibility" + end + + unless group.visibility_level_allowed_by_sub_groups?(level) + reasons << "it contains sub-groups with higher visibility" + end + + unless group.visibility_level_allowed_by_parent?(level) + reasons << "the visibility of its parent group is #{group.parent.visibility}" + end + + reasons = reasons.any? ? ' because ' + reasons.to_sentence : '' + "This group cannot be #{level_name}#{reasons}." + end + def visibility_icon_description(form_model) case form_model when Project diff --git a/app/views/shared/_visibility_radios.html.haml b/app/views/shared/_visibility_radios.html.haml index 13eeaeefa9f..4a656ccfeac 100644 --- a/app/views/shared/_visibility_radios.html.haml +++ b/app/views/shared/_visibility_radios.html.haml @@ -12,6 +12,6 @@ = visibility_level_description(level, form_model) .option-disabled-reason - if restricted - This visibility level has been restricted by the administrator. + = restricted_visibility_level_description(level) - elsif disallowed - This option is not available the visibility of parent or child items prevents it. + = disallowed_visibility_level_description(level, form_model) -- cgit v1.2.1 From aff72ece729ab33afa8c3a4a83e97c5383ce5ed8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Fri, 25 Aug 2017 15:03:16 -0500 Subject: Fix broken spec. parent_id is being set to 0 by RSpec. --- app/models/group.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/group.rb b/app/models/group.rb index 257a5075d2c..78084f76869 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -123,7 +123,7 @@ class Group < Namespace end def visibility_level_allowed_by_parent?(level = self.visibility_level) - return true unless parent_id.present? || parent + return true unless parent_id && parent_id.nonzero? level <= parent.visibility_level end -- cgit v1.2.1 From 1c3b3facb7a2296ca10dcb57b4f3b8ba416ffd3b Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Fri, 25 Aug 2017 15:40:21 -0500 Subject: add tests for new visibility_level_helper methods --- spec/helpers/visibility_level_helper_spec.rb | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/spec/helpers/visibility_level_helper_spec.rb b/spec/helpers/visibility_level_helper_spec.rb index 9a85fbe5258..bd15abf5469 100644 --- a/spec/helpers/visibility_level_helper_spec.rb +++ b/spec/helpers/visibility_level_helper_spec.rb @@ -111,4 +111,30 @@ describe VisibilityLevelHelper do end end end + + describe "disallowed_visibility_level_description" do + let(:group) { create(:group, :internal) } + let!(:subgroup) { create(:group, :internal, parent: group) } + let!(:project) { create(:project, :internal, group: group) } + + describe "project" do + it "provides correct description for disabled levels" do + expect(disallowed_visibility_level?(project, Gitlab::VisibilityLevel::PUBLIC)).to be_truthy + expect(disallowed_visibility_level_description(Gitlab::VisibilityLevel::PUBLIC, project)) + .to include "the visibility of #{project.group.name} is internal" + end + end + + describe "group" do + it "provides correct description for disabled levels" do + expect(disallowed_visibility_level?(group, Gitlab::VisibilityLevel::PRIVATE)).to be_truthy + expect(disallowed_visibility_level_description(Gitlab::VisibilityLevel::PRIVATE, group)) + .to include "it contains projects with higher visibility", "it contains sub-groups with higher visibility" + + expect(disallowed_visibility_level?(subgroup, Gitlab::VisibilityLevel::PUBLIC)).to be_truthy + expect(disallowed_visibility_level_description(Gitlab::VisibilityLevel::PUBLIC, subgroup)) + .to include "the visibility of its parent group is internal" + end + end + end end -- cgit v1.2.1 From b7b49c4a15e592904a41df72aa5ab9b791abc0ef Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Fri, 25 Aug 2017 15:54:05 -0500 Subject: prefer !x.exists? instead of x.none? to prevent unnecessary instantiation of records --- app/models/group.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/group.rb b/app/models/group.rb index 78084f76869..d4d201f05b7 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -129,11 +129,11 @@ class Group < Namespace end def visibility_level_allowed_by_projects?(level = self.visibility_level) - projects.where('visibility_level > ?', level).none? + !projects.where('visibility_level > ?', level).exists? end def visibility_level_allowed_by_sub_groups?(level = self.visibility_level) - children.where('visibility_level > ?', level).none? + !children.where('visibility_level > ?', level).exists? end def visibility_level_allowed?(level = self.visibility_level) -- cgit v1.2.1 From b63c08b2638b01bdbb09ed26c9cce6f330f4a97e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Fri, 25 Aug 2017 17:09:15 -0500 Subject: Build Project in context of Namespace if available --- app/controllers/projects_controller.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 1d24563a6a6..8ad99c01e15 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -3,6 +3,7 @@ class ProjectsController < Projects::ApplicationController include ExtractsPath before_action :authenticate_user!, except: [:index, :show, :activity, :refs] + before_action :namespace, only: [:new] before_action :project, except: [:index, :new, :create] before_action :repository, except: [:index, :new, :create] before_action :assign_ref_vars, only: [:show], if: :repo_exists? @@ -20,7 +21,7 @@ class ProjectsController < Projects::ApplicationController end def new - @project = Project.new + build_project end def edit @@ -395,4 +396,12 @@ class ProjectsController < Projects::ApplicationController def project_export_enabled render_404 unless current_application_settings.project_export_enabled? end + + def namespace + @namespace ||= Namespace.find(params[:namespace_id]) if params[:namespace_id].present? + end + + def build_project + @project ||= namespace ? namespace.projects.new : Project.new + end end -- cgit v1.2.1 From 77a5d9db83ac54980eccfa57735af1ed01ba702c Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Fri, 25 Aug 2017 19:42:33 -0500 Subject: add polyfill for NodeList.prototype.forEach (unsupported in Internet Explorer) --- app/assets/javascripts/commons/polyfills.js | 1 + app/assets/javascripts/commons/polyfills/nodelist.js | 7 +++++++ 2 files changed, 8 insertions(+) create mode 100644 app/assets/javascripts/commons/polyfills/nodelist.js diff --git a/app/assets/javascripts/commons/polyfills.js b/app/assets/javascripts/commons/polyfills.js index bc3e741f524..b78089525cc 100644 --- a/app/assets/javascripts/commons/polyfills.js +++ b/app/assets/javascripts/commons/polyfills.js @@ -12,3 +12,4 @@ import 'core-js/fn/symbol'; // Browser polyfills import './polyfills/custom_event'; import './polyfills/element'; +import './polyfills/nodelist'; diff --git a/app/assets/javascripts/commons/polyfills/nodelist.js b/app/assets/javascripts/commons/polyfills/nodelist.js new file mode 100644 index 00000000000..3772c94b900 --- /dev/null +++ b/app/assets/javascripts/commons/polyfills/nodelist.js @@ -0,0 +1,7 @@ +if (window.NodeList && !NodeList.prototype.forEach) { + NodeList.prototype.forEach = function forEach(callback, thisArg = window) { + for (let i = 0; i < this.length; i += 1) { + callback.call(thisArg, this[i], i, this); + } + }; +} -- cgit v1.2.1 From 62be748ef81a9cd1d4e075940da2df20251605e2 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Fri, 25 Aug 2017 19:43:33 -0500 Subject: dynamically disable/enable visibility options when changing namespaces in new project form --- app/assets/javascripts/dispatcher.js | 2 ++ app/assets/javascripts/project_visibility.js | 37 ++++++++++++++++++++++++++++ app/helpers/namespaces_helper.rb | 2 +- 3 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 app/assets/javascripts/project_visibility.js diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index 2bba7f55de1..feb8735752b 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -74,6 +74,7 @@ import PerformanceBar from './performance_bar'; import initNotes from './init_notes'; import initLegacyFilters from './init_legacy_filters'; import initIssuableSidebar from './init_issuable_sidebar'; +import initProjectVisibilitySelector from './project_visibility'; import GpgBadges from './gpg_badges'; import UserFeatureHelper from './helpers/user_feature_helper'; import initChangesDropdown from './init_changes_dropdown'; @@ -575,6 +576,7 @@ import initChangesDropdown from './init_changes_dropdown'; break; case 'new': new ProjectNew(); + initProjectVisibilitySelector(); break; case 'show': new Star(); diff --git a/app/assets/javascripts/project_visibility.js b/app/assets/javascripts/project_visibility.js new file mode 100644 index 00000000000..b43c8ba56e2 --- /dev/null +++ b/app/assets/javascripts/project_visibility.js @@ -0,0 +1,37 @@ +function setVisibilityOptions(namespaceSelector) { + if (!namespaceSelector || !('selectedIndex' in namespaceSelector)) { + return; + } + const selectedNamespace = namespaceSelector.options[namespaceSelector.selectedIndex]; + const { name, visibility, visibilityLevel } = selectedNamespace.dataset; + + document.querySelectorAll('.visibility-level-setting .radio').forEach((option) => { + const optionInput = option.querySelector('input[type=radio]'); + const optionValue = optionInput ? optionInput.value : 0; + const optionTitle = option.querySelector('.option-title'); + const optionName = optionTitle ? optionTitle.innerText.toLowerCase() : ''; + + // don't change anything if the option is restricted by admin + if (!option.classList.contains('restricted')) { + if (visibilityLevel < optionValue) { + option.classList.add('disabled'); + optionInput.disabled = true; + const reason = option.querySelector('.option-disabled-reason'); + if (reason) { + reason.innerText = `This project cannot be ${optionName} because the visibility of ${name} is ${visibility}.`; + } + } else { + option.classList.remove('disabled'); + optionInput.disabled = false; + } + } + }); +} + +export default function initProjectVisibilitySelector() { + const namespaceSelector = document.querySelector('select.js-select-namespace'); + if (namespaceSelector) { + $('.select2.js-select-namespace').on('change', () => setVisibilityOptions(namespaceSelector)); + setVisibilityOptions(namespaceSelector); + } +} diff --git a/app/helpers/namespaces_helper.rb b/app/helpers/namespaces_helper.rb index df8b247aff5..ca149ac2c20 100644 --- a/app/helpers/namespaces_helper.rb +++ b/app/helpers/namespaces_helper.rb @@ -41,7 +41,7 @@ module NamespacesHelper elements = namespaces.sort_by(&:human_name).map! do |n| [display_path ? n.full_path : n.human_name, n.id, - data: { options_parent: type, visibility_level: n.visibility_level_value, name: n.human_name }] + data: { options_parent: type, visibility_level: n.visibility_level_value, visibility: n.visibility, name: n.name }] end [type.camelize, elements] -- cgit v1.2.1 From f82ec4a7709c73b843b72f69e70201917366db75 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Fri, 25 Aug 2017 19:56:11 -0500 Subject: add CHANGELOG.md entry for !13442 --- ...in-an-internal-sub-group-gives-the-option-to-set-it-a-public.yml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelogs/unreleased/31273-creating-an-project-within-an-internal-sub-group-gives-the-option-to-set-it-a-public.yml diff --git a/changelogs/unreleased/31273-creating-an-project-within-an-internal-sub-group-gives-the-option-to-set-it-a-public.yml b/changelogs/unreleased/31273-creating-an-project-within-an-internal-sub-group-gives-the-option-to-set-it-a-public.yml new file mode 100644 index 00000000000..4d21717e161 --- /dev/null +++ b/changelogs/unreleased/31273-creating-an-project-within-an-internal-sub-group-gives-the-option-to-set-it-a-public.yml @@ -0,0 +1,6 @@ +--- +title: Ensure correct visibility level options shown on all Project, Group, and Snippets + forms +merge_request: 13442 +author: +type: fixed -- cgit v1.2.1 From 3488e8f011e493a89df60cd0db0fff4086f42bd5 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Sat, 26 Aug 2017 01:50:12 -0500 Subject: add tests for dynamically changing namespace selector within new project form --- spec/features/projects/new_project_spec.rb | 53 +++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 16 deletions(-) diff --git a/spec/features/projects/new_project_spec.rb b/spec/features/projects/new_project_spec.rb index 9f8b1996249..cd3dc72d3c6 100644 --- a/spec/features/projects/new_project_spec.rb +++ b/spec/features/projects/new_project_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' feature 'New project' do + include Select2Helper + let(:user) { create(:admin) } before do @@ -68,22 +70,6 @@ feature 'New project' do expect(namespace.text).to eq group.name end - - context 'on validation error' do - before do - fill_in('project_path', with: 'private-group-project') - choose('Internal') - click_button('Create project') - - expect(page).to have_css '.project-edit-errors .alert.alert-danger' - end - - it 'selects the group namespace' do - namespace = find('#project_namespace_id option[selected]') - - expect(namespace.text).to eq group.name - end - end end context 'with subgroup namespace' do @@ -101,6 +87,41 @@ feature 'New project' do expect(namespace.text).to eq subgroup.full_path end end + + context 'when changing namespaces dynamically', :js do + let(:public_group) { create(:group, :public) } + let(:internal_group) { create(:group, :internal) } + let(:private_group) { create(:group, :private) } + + before do + public_group.add_owner(user) + internal_group.add_owner(user) + private_group.add_owner(user) + visit new_project_path(namespace_id: public_group.id) + end + + it 'enables the correct visibility options' do + select2(user.namespace_id, from: '#project_namespace_id') + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::PRIVATE}")).not_to be_disabled + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::INTERNAL}")).not_to be_disabled + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::PUBLIC}")).not_to be_disabled + + select2(public_group.id, from: '#project_namespace_id') + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::PRIVATE}")).not_to be_disabled + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::INTERNAL}")).not_to be_disabled + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::PUBLIC}")).not_to be_disabled + + select2(internal_group.id, from: '#project_namespace_id') + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::PRIVATE}")).not_to be_disabled + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::INTERNAL}")).not_to be_disabled + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::PUBLIC}")).to be_disabled + + select2(private_group.id, from: '#project_namespace_id') + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::PRIVATE}")).not_to be_disabled + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::INTERNAL}")).to be_disabled + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::PUBLIC}")).to be_disabled + end + end end context 'Import project options' do -- cgit v1.2.1 From 0a8d0924fe9a1525b92423411dc1bfcdc9760833 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Sat, 26 Aug 2017 02:43:45 -0500 Subject: expand the help text with links and additional instructions --- app/assets/javascripts/project_visibility.js | 8 ++++++-- app/helpers/namespaces_helper.rb | 9 ++++++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/project_visibility.js b/app/assets/javascripts/project_visibility.js index b43c8ba56e2..c3f5e8cb907 100644 --- a/app/assets/javascripts/project_visibility.js +++ b/app/assets/javascripts/project_visibility.js @@ -3,7 +3,7 @@ function setVisibilityOptions(namespaceSelector) { return; } const selectedNamespace = namespaceSelector.options[namespaceSelector.selectedIndex]; - const { name, visibility, visibilityLevel } = selectedNamespace.dataset; + const { name, visibility, visibilityLevel, showPath, editPath } = selectedNamespace.dataset; document.querySelectorAll('.visibility-level-setting .radio').forEach((option) => { const optionInput = option.querySelector('input[type=radio]'); @@ -18,7 +18,11 @@ function setVisibilityOptions(namespaceSelector) { optionInput.disabled = true; const reason = option.querySelector('.option-disabled-reason'); if (reason) { - reason.innerText = `This project cannot be ${optionName} because the visibility of ${name} is ${visibility}.`; + reason.innerHTML = + `This project cannot be ${optionName} because the visibility of + ${name} is ${visibility}. To make this project + ${optionName}, you must first change the visibility + of the parent group.`; } } else { option.classList.remove('disabled'); diff --git a/app/helpers/namespaces_helper.rb b/app/helpers/namespaces_helper.rb index ca149ac2c20..33d6875a704 100644 --- a/app/helpers/namespaces_helper.rb +++ b/app/helpers/namespaces_helper.rb @@ -41,7 +41,14 @@ module NamespacesHelper elements = namespaces.sort_by(&:human_name).map! do |n| [display_path ? n.full_path : n.human_name, n.id, - data: { options_parent: type, visibility_level: n.visibility_level_value, visibility: n.visibility, name: n.name }] + data: { + options_parent: type, + visibility_level: n.visibility_level_value, + visibility: n.visibility, + name: n.name, + show_path: n.is_a?(Group) ? group_path(n) : user_path(n), + edit_path: n.is_a?(Group) ? edit_group_path(n) : nil + }] end [type.camelize, elements] -- cgit v1.2.1 From fe5b30140c6d0415c6b3a1bee84265c3c7ef63bc Mon Sep 17 00:00:00 2001 From: Joshua Lambert Date: Mon, 28 Aug 2017 14:18:25 -0400 Subject: WIP update to index page --- doc/install/kubernetes/index.md | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/doc/install/kubernetes/index.md b/doc/install/kubernetes/index.md index d5ea81b13df..5177f4adc44 100644 --- a/doc/install/kubernetes/index.md +++ b/doc/install/kubernetes/index.md @@ -1,28 +1,35 @@ # Installing GitLab on Kubernetes -> These Helm charts are in beta. GitLab is working on a [cloud-native set of Charts](https://gitlab.com/charts/helm.gitlab.io/blob/master/README.md) which will replace these. - > Officially supported cloud providers are Google Container Service and Azure Container Service. The easiest method to deploy GitLab in [Kubernetes](https://kubernetes.io/) is -to take advantage of the official GitLab Helm charts. [Helm] is a package +to take advantage of GitLab's Helm charts. [Helm] is a package management tool for Kubernetes, allowing apps to be easily managed via their Charts. A [Chart] is a detailed description of the application including how it should be deployed, upgraded, and configured. -The GitLab Helm repository is located at https://charts.gitlab.io. -You can report any issues related to GitLab's Helm Charts at -https://gitlab.com/charts/charts.gitlab.io/issues. -Contributions and improvements are also very welcome. +GitLab provides [official Helm Charts](#official-gitlab-helm-charts-recommended) which is the recommended way to run GitLab with Kubernetes. + +There are also two other sets of GitLab Helm Charts: +* Our [upcoming cloud native Charts](#upcoming-cloud-native-helm-charts), which are in development but will eventually replace the current official charts. +* [Community contributed charts](#community-contributed-charts). + +## Official GitLab Helm Charts (Recommended) +> Note: These charts will eventually be replaced by the [cloud-native charts](https://gitlab.com/charts/helm.gitlab.io/), which are presently in development. -## Using the GitLab Helm Charts +The best way to deploy GitLab on Kubernetes is to use the [gitlab-omnibus](gitlab_omnibus.md) chart. It includes everything needed to run GitLab, including: a [Runner](https://docs.gitlab.com/runner/), [Container Registry](https://docs.gitlab.com/ee/user/project/container_registry.html#gitlab-container-registry), [automatic SSL](https://github.com/kubernetes/charts/tree/master/stable/kube-lego), and an [Ingress](https://github.com/kubernetes/ingress/tree/master/controllers/nginx). + +To deploy just the GitLab Runner, utilize the [gitlab-runner](gitlab_runner_chart.md) chart. It offers a quick way to configure and deploy the Runner on Kubernetes, regardless of where your GitLab server may be running. + +If advanced configuration of GitLab is required, the [gitlab](gitlab_chart.md) chart can be used which deploys the GitLab service along with optional Posgres and Redis. It offers extensive configuration options some of which may not yet be available in the `gitlab-omnibus` chart, but requires deep knowledge of Kubernetes and Helm to use. + +These charts utilize our [GitLab Omnibus Docker images](https://docs.gitlab.com/omnibus/docker/README.html). You can report any issues and feedback related these charts at +https://gitlab.com/charts/charts.gitlab.io/issues. -GitLab makes available three Helm Charts. +## Upcoming Cloud Native Helm Charts -- [gitlab-omnibus](gitlab_omnibus.md): **Recommended** and the easiest way to get started. Includes everything needed to run GitLab, including: a [Runner](https://docs.gitlab.com/runner/), [Container Registry](https://docs.gitlab.com/ee/user/project/container_registry.html#gitlab-container-registry), [automatic SSL](https://github.com/kubernetes/charts/tree/master/stable/kube-lego), and an [Ingress](https://github.com/kubernetes/ingress/tree/master/controllers/nginx). -- [gitlab](gitlab_chart.md): Just the GitLab service, with optional Postgres and Redis. -- [gitlab-runner](gitlab_runner_chart.md): GitLab Runner, to process CI jobs. +GitLab is working towards a building a [cloud native deployment model](https://gitlab.com/charts/helm.gitlab.io/blob/master/README.md). A key part of this is to isolate each service into it's [own Docker container and Helm chart](https://gitlab.com/gitlab-org/omnibus-gitlab/issues/2420), rather than utilizing the all-in-one container image of the [current charts](#official-gitlab-helm-charts-recommended). -We are also working on a new set of [cloud native Charts](https://gitlab.com/charts/helm.gitlab.io) which will eventually replace these. +## Community Contributed Helm Charts [chart]: https://github.com/kubernetes/charts [helm]: https://github.com/kubernetes/helm/blob/master/README.md -- cgit v1.2.1 From 83c5adceceb8f60a7d807c92a1dc8e4dfc15a29b Mon Sep 17 00:00:00 2001 From: Joshua Lambert Date: Mon, 28 Aug 2017 16:03:42 -0400 Subject: Updated index page --- doc/install/kubernetes/index.md | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/doc/install/kubernetes/index.md b/doc/install/kubernetes/index.md index 5177f4adc44..c763aa08770 100644 --- a/doc/install/kubernetes/index.md +++ b/doc/install/kubernetes/index.md @@ -9,27 +9,37 @@ should be deployed, upgraded, and configured. GitLab provides [official Helm Charts](#official-gitlab-helm-charts-recommended) which is the recommended way to run GitLab with Kubernetes. -There are also two other sets of GitLab Helm Charts: +There are also two other sets of charts: * Our [upcoming cloud native Charts](#upcoming-cloud-native-helm-charts), which are in development but will eventually replace the current official charts. * [Community contributed charts](#community-contributed-charts). ## Official GitLab Helm Charts (Recommended) -> Note: These charts will eventually be replaced by the [cloud-native charts](https://gitlab.com/charts/helm.gitlab.io/), which are presently in development. +> Note: These charts will eventually be replaced by the [cloud native charts](https://gitlab.com/charts/helm.gitlab.io/), which are presently in development. The best way to deploy GitLab on Kubernetes is to use the [gitlab-omnibus](gitlab_omnibus.md) chart. It includes everything needed to run GitLab, including: a [Runner](https://docs.gitlab.com/runner/), [Container Registry](https://docs.gitlab.com/ee/user/project/container_registry.html#gitlab-container-registry), [automatic SSL](https://github.com/kubernetes/charts/tree/master/stable/kube-lego), and an [Ingress](https://github.com/kubernetes/ingress/tree/master/controllers/nginx). To deploy just the GitLab Runner, utilize the [gitlab-runner](gitlab_runner_chart.md) chart. It offers a quick way to configure and deploy the Runner on Kubernetes, regardless of where your GitLab server may be running. -If advanced configuration of GitLab is required, the [gitlab](gitlab_chart.md) chart can be used which deploys the GitLab service along with optional Posgres and Redis. It offers extensive configuration options some of which may not yet be available in the `gitlab-omnibus` chart, but requires deep knowledge of Kubernetes and Helm to use. +If advanced configuration of GitLab is required, the [gitlab](gitlab_chart.md) chart can be used which deploys the GitLab service along with optional Posgres and Redis. It offers extensive configuration, but requires deep knowledge of Kubernetes and Helm to use. These charts utilize our [GitLab Omnibus Docker images](https://docs.gitlab.com/omnibus/docker/README.html). You can report any issues and feedback related these charts at https://gitlab.com/charts/charts.gitlab.io/issues. ## Upcoming Cloud Native Helm Charts -GitLab is working towards a building a [cloud native deployment model](https://gitlab.com/charts/helm.gitlab.io/blob/master/README.md). A key part of this is to isolate each service into it's [own Docker container and Helm chart](https://gitlab.com/gitlab-org/omnibus-gitlab/issues/2420), rather than utilizing the all-in-one container image of the [current charts](#official-gitlab-helm-charts-recommended). +GitLab is working towards a building a [cloud native deployment method](https://gitlab.com/charts/helm.gitlab.io/blob/master/README.md). A key part of this effort is to isolate each service into it's [own Docker container and Helm chart](https://gitlab.com/gitlab-org/omnibus-gitlab/issues/2420), rather than utilizing the all-in-one container image of the [current charts](#official-gitlab-helm-charts-recommended). + +By offering individual containers and charts, we will be able to provide a number of benefits: +* Easier horizontal scaling of each service +* Smaller more efficient images +* Potential for rolling updates and canaries within a service +* and plenty more. + +This is a large project and will be worked on over the span of multiple releases. For the most up to date status and release information, please see our [tracking issue](https://gitlab.com/gitlab-org/omnibus-gitlab/issues/2420). ## Community Contributed Helm Charts +The community has also [contributed GitLab charts](https://github.com/kubernetes/charts/tree/master/stable/gitlab-ce) to the [Helm Stable Repository](https://github.com/kubernetes/charts#repository-structure). + [chart]: https://github.com/kubernetes/charts [helm]: https://github.com/kubernetes/helm/blob/master/README.md -- cgit v1.2.1 From d95b14cdf9a9f3eb841cf13096217010d84f870a Mon Sep 17 00:00:00 2001 From: Joshua Lambert Date: Mon, 28 Aug 2017 22:44:12 -0400 Subject: Updates --- doc/install/kubernetes/gitlab_chart.md | 4 +++- doc/install/kubernetes/gitlab_omnibus.md | 8 +++++--- doc/install/kubernetes/gitlab_runner_chart.md | 13 +++++++++++-- doc/install/kubernetes/index.md | 8 ++++---- 4 files changed, 23 insertions(+), 10 deletions(-) diff --git a/doc/install/kubernetes/gitlab_chart.md b/doc/install/kubernetes/gitlab_chart.md index b64e86d57af..93a6bc40493 100644 --- a/doc/install/kubernetes/gitlab_chart.md +++ b/doc/install/kubernetes/gitlab_chart.md @@ -1,5 +1,7 @@ # GitLab Helm Chart -> These Helm charts are in beta. GitLab is working on a [cloud-native set of Charts](https://gitlab.com/charts/helm.gitlab.io/blob/master/README.md) which will replace these. +> This Helm chart is in beta, while [additional features](https://gitlab.com/charts/charts.gitlab.io/issues/68) are being worked on. + +> GitLab is working on a [cloud native set of Charts](https://gitlab.com/charts/helm.gitlab.io/blob/master/README.md) which will replace these. > Officially supported cloud providers are Google Container Service and Azure Container Service. diff --git a/doc/install/kubernetes/gitlab_omnibus.md b/doc/install/kubernetes/gitlab_omnibus.md index 374e693ba5d..33d89adb831 100644 --- a/doc/install/kubernetes/gitlab_omnibus.md +++ b/doc/install/kubernetes/gitlab_omnibus.md @@ -1,5 +1,7 @@ # GitLab-Omnibus Helm Chart -> These Helm charts are in beta. GitLab is working on a [cloud-native set of Charts](https://gitlab.com/charts/helm.gitlab.io/blob/master/README.md) which will replace these. +> This Helm chart is in beta, while [additional features](https://gitlab.com/charts/charts.gitlab.io/issues/68) are being worked on. + +> GitLab is working on a [cloud native set of Charts](https://gitlab.com/charts/helm.gitlab.io/blob/master/README.md) which will eventually replace these. > Officially supported cloud providers are Google Container Service and Azure Container Service. @@ -29,7 +31,7 @@ Terms: ## Prerequisites -- _At least_ 4 GB of RAM available on your cluster, in chunks of 1 GB. 41GB of storage and 2 CPU are also required. +- _At least_ 4 GB of RAM available on your cluster. 41GB of storage and 2 CPU are also required. - Kubernetes 1.4+ with Beta APIs enabled - [Persistent Volume](https://kubernetes.io/docs/concepts/storage/persistent-volumes/) provisioner support in the underlying infrastructure - An [external IP address](#networking-prerequisites) @@ -52,12 +54,12 @@ Now that an external IP address has been allocated, ensure that the wildcard DNS For most installations, only two parameters are required: - `baseIP`: the desired [external IP address](#networking-prerequisites) - `baseDomain`: the [base domain](#networking-prerequisites) with the wildcard host entry resolving to the `baseIP`. For example, `mycompany.io`. +- `legoEmail`: Email address to use when requesting new SSL certificates from Let's Encrypt Other common configuration options: - `gitlab`: Choose the [desired edition](https://about.gitlab.com/products), either `ee` or `ce`. `ce` is the default. - `gitlabEELicense`: For Enterprise Edition, the [license](https://docs.gitlab.com/ee/user/admin_area/license.html) can be installed directly via the Chart - `provider`: Optimizes the deployment for a cloud provider. The default is `gke` for GCP, with `acs` also supported for Azure. -- `legoEmail`: Email address to use when requesting new SSL certificates from Let's Encrypt For additional configuration options, consult the [values.yaml](https://gitlab.com/charts/charts.gitlab.io/blob/master/charts/gitlab-omnibus/values.yaml). diff --git a/doc/install/kubernetes/gitlab_runner_chart.md b/doc/install/kubernetes/gitlab_runner_chart.md index 04f74be4454..26fc55e30fa 100644 --- a/doc/install/kubernetes/gitlab_runner_chart.md +++ b/doc/install/kubernetes/gitlab_runner_chart.md @@ -1,6 +1,4 @@ # GitLab Runner Helm Chart -> These Helm charts are in beta. GitLab is working on a [cloud-native set of Charts](https://gitlab.com/charts/helm.gitlab.io/blob/master/README.md) which will replace these. - > Officially supported cloud providers are Google Container Service and Azure Container Service. The `gitlab-runner` Helm chart deploys a GitLab Runner instance into your @@ -113,6 +111,17 @@ runners: ``` +### Controlling maximum Runner concurrency + +A single GitLab Runner deployed on Kubernetes is able to execute multiple jobs in parallel by automatically starting additional Runner pods. The [`concurrent` setting](https://docs.gitlab.com/runner/configuration/advanced-configuration.html#the-global-section) controls the maximum number of pods allowed at a single time, and defaults to `10`. + +```yaml +## Configure the maximum number of concurrent jobs +## ref: https://docs.gitlab.com/runner/configuration/advanced-configuration.html#the-global-section +## +concurrent: 10 +``` + ### Running Docker-in-Docker containers with GitLab Runners See [Running Privileged Containers for the Runners](#running-privileged-containers-for-the-runners) for how to enable it, diff --git a/doc/install/kubernetes/index.md b/doc/install/kubernetes/index.md index c763aa08770..92c0ede93ad 100644 --- a/doc/install/kubernetes/index.md +++ b/doc/install/kubernetes/index.md @@ -11,16 +11,16 @@ GitLab provides [official Helm Charts](#official-gitlab-helm-charts-recommended) There are also two other sets of charts: * Our [upcoming cloud native Charts](#upcoming-cloud-native-helm-charts), which are in development but will eventually replace the current official charts. -* [Community contributed charts](#community-contributed-charts). +* [Community contributed charts](#community-contributed-charts). These charts should be considered deprecated, in favor of the official charts. ## Official GitLab Helm Charts (Recommended) > Note: These charts will eventually be replaced by the [cloud native charts](https://gitlab.com/charts/helm.gitlab.io/), which are presently in development. -The best way to deploy GitLab on Kubernetes is to use the [gitlab-omnibus](gitlab_omnibus.md) chart. It includes everything needed to run GitLab, including: a [Runner](https://docs.gitlab.com/runner/), [Container Registry](https://docs.gitlab.com/ee/user/project/container_registry.html#gitlab-container-registry), [automatic SSL](https://github.com/kubernetes/charts/tree/master/stable/kube-lego), and an [Ingress](https://github.com/kubernetes/ingress/tree/master/controllers/nginx). +The best way to deploy GitLab on Kubernetes is to use the [gitlab-omnibus](gitlab_omnibus.md) chart. It includes everything needed to run GitLab, including: a [Runner](https://docs.gitlab.com/runner/), [Container Registry](https://docs.gitlab.com/ee/user/project/container_registry.html#gitlab-container-registry), [automatic SSL](https://github.com/kubernetes/charts/tree/master/stable/kube-lego), and an [Ingress](https://github.com/kubernetes/ingress/tree/master/controllers/nginx). This chart is in beta while [additional features](https://gitlab.com/charts/charts.gitlab.io/issues/68) are being completed. To deploy just the GitLab Runner, utilize the [gitlab-runner](gitlab_runner_chart.md) chart. It offers a quick way to configure and deploy the Runner on Kubernetes, regardless of where your GitLab server may be running. -If advanced configuration of GitLab is required, the [gitlab](gitlab_chart.md) chart can be used which deploys the GitLab service along with optional Posgres and Redis. It offers extensive configuration, but requires deep knowledge of Kubernetes and Helm to use. +If advanced configuration of GitLab is required, the beta [gitlab](gitlab_chart.md) chart can be used which deploys the GitLab service along with optional Posgres and Redis. It offers extensive configuration, but requires deep knowledge of Kubernetes and Helm to use. These charts utilize our [GitLab Omnibus Docker images](https://docs.gitlab.com/omnibus/docker/README.html). You can report any issues and feedback related these charts at https://gitlab.com/charts/charts.gitlab.io/issues. @@ -39,7 +39,7 @@ This is a large project and will be worked on over the span of multiple releases ## Community Contributed Helm Charts -The community has also [contributed GitLab charts](https://github.com/kubernetes/charts/tree/master/stable/gitlab-ce) to the [Helm Stable Repository](https://github.com/kubernetes/charts#repository-structure). +The community has also [contributed GitLab charts](https://github.com/kubernetes/charts/tree/master/stable/gitlab-ce) to the [Helm Stable Repository](https://github.com/kubernetes/charts#repository-structure). These charts should be considered [deprecated](https://github.com/kubernetes/charts/issues/1138) in favor of the [official Charts](#official-gitlab-helm-charts-recommended). [chart]: https://github.com/kubernetes/charts [helm]: https://github.com/kubernetes/helm/blob/master/README.md -- cgit v1.2.1 From 6f03ddcdc3af1fbb840498a0e4765458079f0b0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Tue, 29 Aug 2017 00:49:01 -0500 Subject: Address some suggestions from first code review --- app/controllers/projects_controller.rb | 11 +--------- app/helpers/namespaces_helper.rb | 15 ++++++------- app/helpers/visibility_level_helper.rb | 12 +++++----- app/models/group.rb | 40 +++++++++++++++++----------------- app/models/project.rb | 6 +++-- lib/gitlab/visibility_level.rb | 18 +++++++++++++++ 6 files changed, 56 insertions(+), 46 deletions(-) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 8ad99c01e15..51cf37b9438 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -3,7 +3,6 @@ class ProjectsController < Projects::ApplicationController include ExtractsPath before_action :authenticate_user!, except: [:index, :show, :activity, :refs] - before_action :namespace, only: [:new] before_action :project, except: [:index, :new, :create] before_action :repository, except: [:index, :new, :create] before_action :assign_ref_vars, only: [:show], if: :repo_exists? @@ -21,7 +20,7 @@ class ProjectsController < Projects::ApplicationController end def new - build_project + @project ||= Project.new(params.permit(:namespace_id)) end def edit @@ -396,12 +395,4 @@ class ProjectsController < Projects::ApplicationController def project_export_enabled render_404 unless current_application_settings.project_export_enabled? end - - def namespace - @namespace ||= Namespace.find(params[:namespace_id]) if params[:namespace_id].present? - end - - def build_project - @project ||= namespace ? namespace.projects.new : Project.new - end end diff --git a/app/helpers/namespaces_helper.rb b/app/helpers/namespaces_helper.rb index 33d6875a704..3c784272df2 100644 --- a/app/helpers/namespaces_helper.rb +++ b/app/helpers/namespaces_helper.rb @@ -6,7 +6,6 @@ module NamespacesHelper def namespaces_options(selected = :current_user, display_path: false, extra_group: nil) groups = current_user.owned_groups + current_user.masters_groups users = [current_user.namespace] - options = [] unless extra_group.nil? || extra_group.is_a?(Group) extra_group = Group.find(extra_group) if Namespace.find(extra_group).kind == 'group' @@ -16,8 +15,9 @@ module NamespacesHelper groups |= [extra_group] end - options << options_for_group(groups, display_path) - options << options_for_group(users, display_path) + options = [] + options << options_for_group(groups, display_path: display_path, type: 'group') + options << options_for_group(users, display_path: display_path, type: 'user') if selected == :current_user && current_user.namespace selected = current_user.namespace.id @@ -36,13 +36,12 @@ module NamespacesHelper private - def options_for_group(namespaces, display_path) - type = namespaces.first.is_a?(Group) ? 'group' : 'users' - + def options_for_group(namespaces, display_path:, type:) + group_label = type.pluralize elements = namespaces.sort_by(&:human_name).map! do |n| [display_path ? n.full_path : n.human_name, n.id, data: { - options_parent: type, + options_parent: group_label, visibility_level: n.visibility_level_value, visibility: n.visibility, name: n.name, @@ -51,6 +50,6 @@ module NamespacesHelper }] end - [type.camelize, elements] + [group_label.camelize, elements] end end diff --git a/app/helpers/visibility_level_helper.rb b/app/helpers/visibility_level_helper.rb index 347f796fdc1..caadc12019c 100644 --- a/app/helpers/visibility_level_helper.rb +++ b/app/helpers/visibility_level_helper.rb @@ -65,7 +65,7 @@ module VisibilityLevelHelper def restricted_visibility_level_description(level) level_name = Gitlab::VisibilityLevel.level_name(level) - "#{level_name.capitalize} visibilitiy has been restricted by the administrator." + "#{level_name.capitalize} visibility has been restricted by the administrator." end def disallowed_visibility_level_description(level, form_model) @@ -82,11 +82,11 @@ module VisibilityLevelHelper reasons = [] unless project.visibility_level_allowed_as_fork?(level) - reasons << "the fork source project has lower visibility" + reasons << project.visibility_error_for(:fork, level: level_name) end unless project.visibility_level_allowed_by_group?(level) - reasons << "the visibility of #{project.group.name} is #{project.group.visibility}" + reasons << project.visibility_error_for(:group, level: level_name, group_level: project.group.visibility) end reasons = reasons.any? ? ' because ' + reasons.to_sentence : '' @@ -98,15 +98,15 @@ module VisibilityLevelHelper reasons = [] unless group.visibility_level_allowed_by_projects?(level) - reasons << "it contains projects with higher visibility" + reasons << group.visibility_error_for(:projects, level: level_name) end unless group.visibility_level_allowed_by_sub_groups?(level) - reasons << "it contains sub-groups with higher visibility" + reasons << group.visibility_error_for(:sub_groups, level: level_name) end unless group.visibility_level_allowed_by_parent?(level) - reasons << "the visibility of its parent group is #{group.parent.visibility}" + reasons << group.visibility_error_for(:parent, level: level_name, parent_level: group.parent.visibility) end reasons = reasons.any? ? ' because ' + reasons.to_sentence : '' diff --git a/app/models/group.rb b/app/models/group.rb index d4d201f05b7..0e6d88f6a91 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -26,7 +26,7 @@ class Group < Namespace validate :avatar_type, if: ->(user) { user.avatar.present? && user.avatar_changed? } validate :visibility_level_allowed_by_projects - validate :visibility_level_allowed_by_sub_groups, if: :visibility_level_changed? + validate :visibility_level_allowed_by_sub_groups validate :visibility_level_allowed_by_parent validates :avatar, file_size: { maximum: 200.kilobytes.to_i } @@ -104,24 +104,6 @@ class Group < Namespace full_name end - def visibility_level_allowed_by_parent - return if visibility_level_allowed_by_parent? - - errors.add(:visibility_level, "#{visibility} is not allowed since the parent group has a #{parent.visibility} visibility.") - end - - def visibility_level_allowed_by_projects - return if visibility_level_allowed_by_projects? - - errors.add(:visibility_level, "#{visibility} is not allowed since this group contains projects with higher visibility.") - end - - def visibility_level_allowed_by_sub_groups - return if visibility_level_allowed_by_sub_groups? - - errors.add(:visibility_level, "#{visibility} is not allowed since there are sub-groups with higher visibility.") - end - def visibility_level_allowed_by_parent?(level = self.visibility_level) return true unless parent_id && parent_id.nonzero? @@ -304,11 +286,29 @@ class Group < Namespace list_of_ids.reverse.map { |group| variables[group.id] }.compact.flatten end - protected + private def update_two_factor_requirement return unless require_two_factor_authentication_changed? || two_factor_grace_period_changed? users.find_each(&:update_two_factor_requirement) end + + def visibility_level_allowed_by_parent + return if visibility_level_allowed_by_parent? + + errors.add(:visibility_level, visibility_error_for(:parent, level: visibility, parent_level: parent.visibility)) + end + + def visibility_level_allowed_by_projects + return if visibility_level_allowed_by_projects? + + errors.add(:visibility_level, visibility_error_for(:projects, level: visibility)) + end + + def visibility_level_allowed_by_sub_groups + return if visibility_level_allowed_by_sub_groups? + + errors.add(:visibility_level, visibility_error_for(:sub_groups, level: visibility)) + end end diff --git a/app/models/project.rb b/app/models/project.rb index d5324ceac31..088fe5e1fed 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -671,14 +671,16 @@ class Project < ActiveRecord::Base level_name = Gitlab::VisibilityLevel.level_name(self.visibility_level).downcase group_level_name = Gitlab::VisibilityLevel.level_name(self.group.visibility_level).downcase - self.errors.add(:visibility_level, "#{level_name} is not allowed in a #{group_level_name} group.") + + self.errors.add(:visibility_level, visibility_error_for(:group, level: level_name, group_level: group_level_name)) end def visibility_level_allowed_as_fork return if visibility_level_allowed_as_fork? level_name = Gitlab::VisibilityLevel.level_name(self.visibility_level).downcase - self.errors.add(:visibility_level, "#{level_name} is not allowed since the fork source project has lower visibility.") + + self.errors.add(:visibility_level, visibility_error_for(:fork, level: level_name)) end def check_wiki_path_conflict diff --git a/lib/gitlab/visibility_level.rb b/lib/gitlab/visibility_level.rb index c60bd91ea6e..ef5c4d9a3b2 100644 --- a/lib/gitlab/visibility_level.rb +++ b/lib/gitlab/visibility_level.rb @@ -130,5 +130,23 @@ module Gitlab def visibility=(level) self[visibility_level_field] = Gitlab::VisibilityLevel.level_value(level) end + + def visibility_errors_template + @visibility_errors ||= { + 'Project' => { + group: "%{level} is not allowed in a %{group_level} group", + fork: "%{level} is not allowed since the fork source project has lower visibility" + }, + 'Group' => { + parent: "%{level} is not allowed since the parent group has a %{parent_level} visibility", + projects: "%{level} is not allowed since this group contains projects with higher visibility", + sub_groups: "%{level} is not allowed since there are sub-groups with higher visibility" + } + } + end + + def visibility_error_for(section, variables) + visibility_errors_template.dig(model_name.to_s, section) % variables + end end end -- cgit v1.2.1 From 3546da73705fa443de99ecd18d8e3f7728f0b620 Mon Sep 17 00:00:00 2001 From: Joshua Lambert Date: Tue, 29 Aug 2017 09:33:38 -0400 Subject: Review fixes --- doc/install/kubernetes/index.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/install/kubernetes/index.md b/doc/install/kubernetes/index.md index 92c0ede93ad..d85012e0e9f 100644 --- a/doc/install/kubernetes/index.md +++ b/doc/install/kubernetes/index.md @@ -11,16 +11,16 @@ GitLab provides [official Helm Charts](#official-gitlab-helm-charts-recommended) There are also two other sets of charts: * Our [upcoming cloud native Charts](#upcoming-cloud-native-helm-charts), which are in development but will eventually replace the current official charts. -* [Community contributed charts](#community-contributed-charts). These charts should be considered deprecated, in favor of the official charts. +* [Community contributed charts](#community-contributed-helm-charts). These charts should be considered deprecated, in favor of the official charts. ## Official GitLab Helm Charts (Recommended) -> Note: These charts will eventually be replaced by the [cloud native charts](https://gitlab.com/charts/helm.gitlab.io/), which are presently in development. +> *Note*: These charts will eventually be replaced by the [cloud native charts](#upcoming-cloud-native-helm-charts), which are presently in development. The best way to deploy GitLab on Kubernetes is to use the [gitlab-omnibus](gitlab_omnibus.md) chart. It includes everything needed to run GitLab, including: a [Runner](https://docs.gitlab.com/runner/), [Container Registry](https://docs.gitlab.com/ee/user/project/container_registry.html#gitlab-container-registry), [automatic SSL](https://github.com/kubernetes/charts/tree/master/stable/kube-lego), and an [Ingress](https://github.com/kubernetes/ingress/tree/master/controllers/nginx). This chart is in beta while [additional features](https://gitlab.com/charts/charts.gitlab.io/issues/68) are being completed. To deploy just the GitLab Runner, utilize the [gitlab-runner](gitlab_runner_chart.md) chart. It offers a quick way to configure and deploy the Runner on Kubernetes, regardless of where your GitLab server may be running. -If advanced configuration of GitLab is required, the beta [gitlab](gitlab_chart.md) chart can be used which deploys the GitLab service along with optional Posgres and Redis. It offers extensive configuration, but requires deep knowledge of Kubernetes and Helm to use. +If advanced configuration of GitLab is required, the beta [gitlab](gitlab_chart.md) chart can be used which deploys the GitLab service along with optional Posgres and Redis. It offers extensive configuration, but requires deep knowledge of Kubernetes and Helm to use. These charts utilize our [GitLab Omnibus Docker images](https://docs.gitlab.com/omnibus/docker/README.html). You can report any issues and feedback related these charts at https://gitlab.com/charts/charts.gitlab.io/issues. -- cgit v1.2.1 From 6036ecf74361f71fec837cb1e77f16d44b232d02 Mon Sep 17 00:00:00 2001 From: Joshua Lambert Date: Tue, 29 Aug 2017 14:39:06 -0400 Subject: Feedback updates --- doc/install/kubernetes/gitlab_omnibus.md | 15 +++++++++++++-- doc/install/kubernetes/index.md | 15 +++++++++++---- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/doc/install/kubernetes/gitlab_omnibus.md b/doc/install/kubernetes/gitlab_omnibus.md index 33d89adb831..7656f7353d5 100644 --- a/doc/install/kubernetes/gitlab_omnibus.md +++ b/doc/install/kubernetes/gitlab_omnibus.md @@ -34,7 +34,6 @@ Terms: - _At least_ 4 GB of RAM available on your cluster. 41GB of storage and 2 CPU are also required. - Kubernetes 1.4+ with Beta APIs enabled - [Persistent Volume](https://kubernetes.io/docs/concepts/storage/persistent-volumes/) provisioner support in the underlying infrastructure -- An [external IP address](#networking-prerequisites) - A [wildcard DNS entry](#networking-prerequisites), which resolves to the external IP address - The `kubectl` CLI installed locally and authenticated for the cluster - The [Helm client](https://github.com/kubernetes/helm/blob/master/docs/quickstart.md) installed locally on your machine @@ -43,12 +42,24 @@ Terms: This chart configures a GitLab server and Kubernetes cluster which can support dynamic [Review Apps](https://docs.gitlab.com/ee/ci/review_apps/index.html), as well as services like the integrated [Container Registry](https://docs.gitlab.com/ee/user/project/container_registry.html) and [Mattermost](https://docs.gitlab.com/omnibus/gitlab-mattermost/). -To support the GitLab services and dynamic environments, a wildcard DNS entry is required which resolves to the external Load Balancer IP. +To support the GitLab services and dynamic environments, a wildcard DNS entry is required which resolves to the [Load Balancer](load-balancer) or [External IP](#external-ip). Configuration of the DNS entry will depend upon the DNS service being used. + +#### Load Balancer IP + +If you do not specify a `baseIP`, an ephemeral IP will be assigned to the Load Balancer or Ingress. You can retrieve this IP by running the following command *after* deploying GitLab: + +`kubectl get svc -w --namespace nginx-ingress nginx` + +The IP address will be displayed in the `EXTERNAL-IP` field, and should be used to configure the Wildcard DNS entry. For more information on creating a wildcard DNS entry, consult the documentation for the DNS server you are using. + +#### External IP To provision an external IP on GCP and Azure, simply request a new address from the Networking section. Ensure that the region matches the region your container cluster is created in. Note, it is important that the IP is not assigned at this point in time. It will be automatically assigned once the Helm chart is installed, and assigned to the Load Balancer. Now that an external IP address has been allocated, ensure that the wildcard DNS entry you would like to use resolves to this IP. Please consult the documentation for your DNS service for more information on creating DNS records. +Finally, set the `baseIP` setting to this IP address when [deploying GitLab](#configuring-and-installing-gitlab). + ## Configuring and Installing GitLab For most installations, only two parameters are required: diff --git a/doc/install/kubernetes/index.md b/doc/install/kubernetes/index.md index d85012e0e9f..8418b04936b 100644 --- a/doc/install/kubernetes/index.md +++ b/doc/install/kubernetes/index.md @@ -14,16 +14,23 @@ There are also two other sets of charts: * [Community contributed charts](#community-contributed-helm-charts). These charts should be considered deprecated, in favor of the official charts. ## Official GitLab Helm Charts (Recommended) -> *Note*: These charts will eventually be replaced by the [cloud native charts](#upcoming-cloud-native-helm-charts), which are presently in development. + +These charts utilize our [GitLab Omnibus Docker images](https://docs.gitlab.com/omnibus/docker/README.html). You can report any issues and feedback related to these charts at +https://gitlab.com/charts/charts.gitlab.io/issues. + +### Deploying GitLab on Kubernetes (Recommended) +> *Note*: This chart will eventually be replaced by the [cloud native charts](#upcoming-cloud-native-helm-charts), which are presently in development. The best way to deploy GitLab on Kubernetes is to use the [gitlab-omnibus](gitlab_omnibus.md) chart. It includes everything needed to run GitLab, including: a [Runner](https://docs.gitlab.com/runner/), [Container Registry](https://docs.gitlab.com/ee/user/project/container_registry.html#gitlab-container-registry), [automatic SSL](https://github.com/kubernetes/charts/tree/master/stable/kube-lego), and an [Ingress](https://github.com/kubernetes/ingress/tree/master/controllers/nginx). This chart is in beta while [additional features](https://gitlab.com/charts/charts.gitlab.io/issues/68) are being completed. +### Deploying just the GitLab Runner + To deploy just the GitLab Runner, utilize the [gitlab-runner](gitlab_runner_chart.md) chart. It offers a quick way to configure and deploy the Runner on Kubernetes, regardless of where your GitLab server may be running. -If advanced configuration of GitLab is required, the beta [gitlab](gitlab_chart.md) chart can be used which deploys the GitLab service along with optional Posgres and Redis. It offers extensive configuration, but requires deep knowledge of Kubernetes and Helm to use. +### Advanced deployment of GitLab (Not recommended) +> *Note*: This chart will eventually be replaced by the [cloud native charts](#upcoming-cloud-native-helm-charts), which are presently in development. -These charts utilize our [GitLab Omnibus Docker images](https://docs.gitlab.com/omnibus/docker/README.html). You can report any issues and feedback related these charts at -https://gitlab.com/charts/charts.gitlab.io/issues. +If advanced configuration of GitLab is required, the beta [gitlab](gitlab_chart.md) chart can be used which deploys the GitLab service along with optional Postgres and Redis. It offers extensive configuration, but requires deep knowledge of Kubernetes and Helm to use. ## Upcoming Cloud Native Helm Charts -- cgit v1.2.1 From a4b18dc6fe58b117335ad4410d568db8056b0775 Mon Sep 17 00:00:00 2001 From: Joshua Lambert Date: Tue, 29 Aug 2017 14:41:42 -0400 Subject: Add recommendation for external IP --- doc/install/kubernetes/gitlab_omnibus.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/install/kubernetes/gitlab_omnibus.md b/doc/install/kubernetes/gitlab_omnibus.md index 7656f7353d5..c0aacacc3d3 100644 --- a/doc/install/kubernetes/gitlab_omnibus.md +++ b/doc/install/kubernetes/gitlab_omnibus.md @@ -52,6 +52,8 @@ If you do not specify a `baseIP`, an ephemeral IP will be assigned to the Load B The IP address will be displayed in the `EXTERNAL-IP` field, and should be used to configure the Wildcard DNS entry. For more information on creating a wildcard DNS entry, consult the documentation for the DNS server you are using. +For production deployments of GitLab, we strongly recommend using an [External IP](#external-ip). + #### External IP To provision an external IP on GCP and Azure, simply request a new address from the Networking section. Ensure that the region matches the region your container cluster is created in. Note, it is important that the IP is not assigned at this point in time. It will be automatically assigned once the Helm chart is installed, and assigned to the Load Balancer. -- cgit v1.2.1 From 4af6b025c56047c14d7e935bad67eedac7334abe Mon Sep 17 00:00:00 2001 From: Joshua Lambert Date: Tue, 29 Aug 2017 14:42:21 -0400 Subject: Add recommendation for external IP --- doc/install/kubernetes/gitlab_omnibus.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/doc/install/kubernetes/gitlab_omnibus.md b/doc/install/kubernetes/gitlab_omnibus.md index c0aacacc3d3..c7a402b5ffe 100644 --- a/doc/install/kubernetes/gitlab_omnibus.md +++ b/doc/install/kubernetes/gitlab_omnibus.md @@ -44,6 +44,14 @@ This chart configures a GitLab server and Kubernetes cluster which can support d To support the GitLab services and dynamic environments, a wildcard DNS entry is required which resolves to the [Load Balancer](load-balancer) or [External IP](#external-ip). Configuration of the DNS entry will depend upon the DNS service being used. +#### External IP (Recommended) + +To provision an external IP on GCP and Azure, simply request a new address from the Networking section. Ensure that the region matches the region your container cluster is created in. Note, it is important that the IP is not assigned at this point in time. It will be automatically assigned once the Helm chart is installed, and assigned to the Load Balancer. + +Now that an external IP address has been allocated, ensure that the wildcard DNS entry you would like to use resolves to this IP. Please consult the documentation for your DNS service for more information on creating DNS records. + +Finally, set the `baseIP` setting to this IP address when [deploying GitLab](#configuring-and-installing-gitlab). + #### Load Balancer IP If you do not specify a `baseIP`, an ephemeral IP will be assigned to the Load Balancer or Ingress. You can retrieve this IP by running the following command *after* deploying GitLab: @@ -54,14 +62,6 @@ The IP address will be displayed in the `EXTERNAL-IP` field, and should be used For production deployments of GitLab, we strongly recommend using an [External IP](#external-ip). -#### External IP - -To provision an external IP on GCP and Azure, simply request a new address from the Networking section. Ensure that the region matches the region your container cluster is created in. Note, it is important that the IP is not assigned at this point in time. It will be automatically assigned once the Helm chart is installed, and assigned to the Load Balancer. - -Now that an external IP address has been allocated, ensure that the wildcard DNS entry you would like to use resolves to this IP. Please consult the documentation for your DNS service for more information on creating DNS records. - -Finally, set the `baseIP` setting to this IP address when [deploying GitLab](#configuring-and-installing-gitlab). - ## Configuring and Installing GitLab For most installations, only two parameters are required: -- cgit v1.2.1 From 7026c027ac75573ba9f686e63a04616aae0a8d14 Mon Sep 17 00:00:00 2001 From: Joshua Lambert Date: Tue, 29 Aug 2017 14:46:54 -0400 Subject: Fix installation and configuration order --- doc/install/kubernetes/gitlab_omnibus.md | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/doc/install/kubernetes/gitlab_omnibus.md b/doc/install/kubernetes/gitlab_omnibus.md index c7a402b5ffe..e1433cac405 100644 --- a/doc/install/kubernetes/gitlab_omnibus.md +++ b/doc/install/kubernetes/gitlab_omnibus.md @@ -65,30 +65,17 @@ For production deployments of GitLab, we strongly recommend using an [External I ## Configuring and Installing GitLab For most installations, only two parameters are required: -- `baseIP`: the desired [external IP address](#networking-prerequisites) - `baseDomain`: the [base domain](#networking-prerequisites) with the wildcard host entry resolving to the `baseIP`. For example, `mycompany.io`. - `legoEmail`: Email address to use when requesting new SSL certificates from Let's Encrypt Other common configuration options: +- `baseIP`: the desired [external IP address](#networking-prerequisites) - `gitlab`: Choose the [desired edition](https://about.gitlab.com/products), either `ee` or `ce`. `ce` is the default. - `gitlabEELicense`: For Enterprise Edition, the [license](https://docs.gitlab.com/ee/user/admin_area/license.html) can be installed directly via the Chart - `provider`: Optimizes the deployment for a cloud provider. The default is `gke` for GCP, with `acs` also supported for Azure. For additional configuration options, consult the [values.yaml](https://gitlab.com/charts/charts.gitlab.io/blob/master/charts/gitlab-omnibus/values.yaml). -These settings can either be passed directly on the command line: -```bash -helm install --name gitlab --set baseDomain=gitlab.io,baseIP=1.1.1.1,gitlab=ee,gitlabEELicense=$LICENSE,legoEmail=email@gitlab.com gitlab/gitlab-omnibus -``` - -or within a YAML file: -```bash -helm install --name gitlab -f values.yaml gitlab/gitlab-omnibus -``` - -> **Note:** -If you are using a machine type with support for less than 4 attached disks, like an Azure trial, you should disable dedicated storage for [Postgres and Redis](#persistent-storage). - ### Choosing a different GitLab release version The version of GitLab installed is based on the `gitlab` setting (see [section](#choosing-gitlab-edition) above), and @@ -108,6 +95,8 @@ There is no guarantee that other release versions of GitLab, other than what are used by default in the chart, will be supported by a chart install. ### Persistent storage +> **Note:** +If you are using a machine type with support for less than 4 attached disks, like an Azure trial, you should disable dedicated storage for [Postgres and Redis](#persistent-storage). By default, persistent storage is enabled for GitLab and the charts it depends on (Redis and PostgreSQL). -- cgit v1.2.1 From 88c42fa3450cd9935e7d18dcfc77d487cdffaaed Mon Sep 17 00:00:00 2001 From: Joshua Lambert Date: Tue, 29 Aug 2017 14:51:02 -0400 Subject: More updates --- doc/install/kubernetes/gitlab_chart.md | 8 +++----- doc/install/kubernetes/gitlab_omnibus.md | 12 ++++++------ doc/install/kubernetes/gitlab_runner_chart.md | 3 ++- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/doc/install/kubernetes/gitlab_chart.md b/doc/install/kubernetes/gitlab_chart.md index 93a6bc40493..6bcc58bb805 100644 --- a/doc/install/kubernetes/gitlab_chart.md +++ b/doc/install/kubernetes/gitlab_chart.md @@ -1,9 +1,7 @@ # GitLab Helm Chart -> This Helm chart is in beta, while [additional features](https://gitlab.com/charts/charts.gitlab.io/issues/68) are being worked on. - -> GitLab is working on a [cloud native set of Charts](https://gitlab.com/charts/helm.gitlab.io/blob/master/README.md) which will replace these. - -> Officially supported cloud providers are Google Container Service and Azure Container Service. +> **Note:** +* GitLab is working on a [cloud native set of Charts](https://gitlab.com/charts/helm.gitlab.io/blob/master/README.md) which will replace these. +* Officially supported cloud providers are Google Container Service and Azure Container Service. The `gitlab` Helm chart deploys GitLab into your Kubernetes cluster. diff --git a/doc/install/kubernetes/gitlab_omnibus.md b/doc/install/kubernetes/gitlab_omnibus.md index e1433cac405..a91c058d415 100644 --- a/doc/install/kubernetes/gitlab_omnibus.md +++ b/doc/install/kubernetes/gitlab_omnibus.md @@ -1,9 +1,8 @@ # GitLab-Omnibus Helm Chart -> This Helm chart is in beta, while [additional features](https://gitlab.com/charts/charts.gitlab.io/issues/68) are being worked on. - -> GitLab is working on a [cloud native set of Charts](https://gitlab.com/charts/helm.gitlab.io/blob/master/README.md) which will eventually replace these. - -> Officially supported cloud providers are Google Container Service and Azure Container Service. +> **Note:** +* This Helm chart is in beta, while [additional features](https://gitlab.com/charts/charts.gitlab.io/issues/68) are being worked on. +* GitLab is working on a [cloud native set of Charts](https://gitlab.com/charts/helm.gitlab.io/blob/master/README.md) which will eventually replace these. +* Officially supported cloud providers are Google Container Service and Azure Container Service. This work is based partially on: https://github.com/lwolf/kubernetes-gitlab/. GitLab would like to thank Sergey Nuzhdin for his work. @@ -126,7 +125,8 @@ Ingress routing and SSL are automatically configured within this Chart. An NGINX Let's Encrypt limits a single TLD to five certificate requests within a single week. This means that common DNS wildcard services like [xip.io](http://xip.io) and [nip.io](http://nip.io) are unlikely to work. ## Installing GitLab using the Helm Chart -> You may see a temporary error message `SchedulerPredicates failed due to PersistentVolumeClaim is not bound` while storage provisions. Once the storage provisions, the pods will automatically restart. This may take a couple minutes depending on your cloud provider. If the error persists, please review the [prerequisites](#prerequisites) to ensure you have enough RAM, CPU, and storage. +> **Note:** +You may see a temporary error message `SchedulerPredicates failed due to PersistentVolumeClaim is not bound` while storage provisions. Once the storage provisions, the pods will automatically start. This may take a couple minutes depending on your cloud provider. If the error persists, please review the [prerequisites](#prerequisites) to ensure you have enough RAM, CPU, and storage. Add the GitLab Helm repository and initialize Helm: diff --git a/doc/install/kubernetes/gitlab_runner_chart.md b/doc/install/kubernetes/gitlab_runner_chart.md index 26fc55e30fa..66f42973708 100644 --- a/doc/install/kubernetes/gitlab_runner_chart.md +++ b/doc/install/kubernetes/gitlab_runner_chart.md @@ -1,5 +1,6 @@ # GitLab Runner Helm Chart -> Officially supported cloud providers are Google Container Service and Azure Container Service. +> **Note:** +Officially supported cloud providers are Google Container Service and Azure Container Service. The `gitlab-runner` Helm chart deploys a GitLab Runner instance into your Kubernetes cluster. -- cgit v1.2.1 From ea5f2da46412a4be3ce025d3d6e8d31c5b43e900 Mon Sep 17 00:00:00 2001 From: Joshua Lambert Date: Tue, 29 Aug 2017 14:54:17 -0400 Subject: fix link --- doc/install/kubernetes/gitlab_omnibus.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/install/kubernetes/gitlab_omnibus.md b/doc/install/kubernetes/gitlab_omnibus.md index a91c058d415..8636ce2507c 100644 --- a/doc/install/kubernetes/gitlab_omnibus.md +++ b/doc/install/kubernetes/gitlab_omnibus.md @@ -41,7 +41,7 @@ Terms: This chart configures a GitLab server and Kubernetes cluster which can support dynamic [Review Apps](https://docs.gitlab.com/ee/ci/review_apps/index.html), as well as services like the integrated [Container Registry](https://docs.gitlab.com/ee/user/project/container_registry.html) and [Mattermost](https://docs.gitlab.com/omnibus/gitlab-mattermost/). -To support the GitLab services and dynamic environments, a wildcard DNS entry is required which resolves to the [Load Balancer](load-balancer) or [External IP](#external-ip). Configuration of the DNS entry will depend upon the DNS service being used. +To support the GitLab services and dynamic environments, a wildcard DNS entry is required which resolves to the [Load Balancer](#load-balancer-ip) or [External IP](#external-ip). Configuration of the DNS entry will depend upon the DNS service being used. #### External IP (Recommended) -- cgit v1.2.1 From ae458ad38d80696d7e8e360172457ca66cdd44d6 Mon Sep 17 00:00:00 2001 From: Joshua Lambert Date: Tue, 29 Aug 2017 15:14:42 -0400 Subject: Add install link for Runner from config section --- doc/install/kubernetes/gitlab_runner_chart.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/install/kubernetes/gitlab_runner_chart.md b/doc/install/kubernetes/gitlab_runner_chart.md index 66f42973708..d31c763ed64 100644 --- a/doc/install/kubernetes/gitlab_runner_chart.md +++ b/doc/install/kubernetes/gitlab_runner_chart.md @@ -33,6 +33,8 @@ In order for GitLab Runner to function, your config file **must** specify the fo - `runnerRegistrationToken` - The Registration Token for adding new Runners to the GitLab Server. This must be retrieved from your GitLab Instance. See the [GitLab Runner Documentation](../../ci/runners/README.md#creating-and-registering-a-runner) for more information. +Unless you need to specify additional configuration, you are [ready to install](#installing-gitlab-runner-using-the-helm-chart). + ### Other configuration The rest of the configuration is [documented in the `values.yaml`](https://gitlab.com/charts/charts.gitlab.io/blob/master/charts/gitlab-runner/values.yaml) in the chart repository. -- cgit v1.2.1 From 91cca6e4943dd64d3bfe029e3da275ed1113bf47 Mon Sep 17 00:00:00 2001 From: Mark Fletcher Date: Fri, 25 Aug 2017 11:01:01 +0700 Subject: Add a Build Variable to represent the triggering GitLab user's name * Predefined variable represents the name of the GitLab user that started a build --- app/models/ci/build.rb | 3 ++- doc/ci/variables/README.md | 6 ++++++ spec/models/ci/build_spec.rb | 7 +++++-- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 4692fb5644a..ace86e5e517 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -384,7 +384,8 @@ module Ci [ { key: 'GITLAB_USER_ID', value: user.id.to_s, public: true }, - { key: 'GITLAB_USER_EMAIL', value: user.email, public: true } + { key: 'GITLAB_USER_EMAIL', value: user.email, public: true }, + { key: 'GITLAB_USER_NAME', value: user.name, public: true } ] end diff --git a/doc/ci/variables/README.md b/doc/ci/variables/README.md index 22e7f6879ed..4b48a1f1c51 100644 --- a/doc/ci/variables/README.md +++ b/doc/ci/variables/README.md @@ -77,6 +77,12 @@ future GitLab releases.** | **GITLAB_CI** | all | all | Mark that job is executed in GitLab CI environment | | **GITLAB_USER_ID** | 8.12 | all | The id of the user who started the job | | **GITLAB_USER_EMAIL** | 8.12 | all | The email of the user who started the job | +<<<<<<< HEAD +| **GITLAB_USER_NAME** | 10.0 | all | The name of the user who started the job | +======= +| **GITLAB_USER_USERNAME** | 10.0 | all | The username of the user who started the job | +| **GITLAB_USER_NAME** | 10.0 | all | The real name of the user who started the job | +>>>>>>> 940c584882... fixup name | **RESTORE_CACHE_ATTEMPTS** | 8.15 | 1.9 | Number of attempts to restore the cache running a job | ## 9.0 Renaming diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 767f0ad9e65..97fce7ded74 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1247,8 +1247,11 @@ describe Ci::Build do context 'when build has user' do let(:user_variables) do - [{ key: 'GITLAB_USER_ID', value: user.id.to_s, public: true }, - { key: 'GITLAB_USER_EMAIL', value: user.email, public: true }] + [ + { key: 'GITLAB_USER_ID', value: user.id.to_s, public: true }, + { key: 'GITLAB_USER_EMAIL', value: user.email, public: true }, + { key: 'GITLAB_USER_NAME', value: user.name, public: true } + ] end before do -- cgit v1.2.1 From 22d5cca8a7afe69000faac6e9011f6662f610123 Mon Sep 17 00:00:00 2001 From: Mark Fletcher Date: Fri, 25 Aug 2017 11:01:55 +0700 Subject: Add a Build Variable to represent the triggering GitLab user's login username * Predefined variable represents the username of the GitLab user that started a build --- app/models/ci/build.rb | 1 + .../unreleased/26692-predefined-variable-gitlab-user-name.yml | 5 +++++ doc/ci/variables/README.md | 6 +----- spec/models/ci/build_spec.rb | 1 + 4 files changed, 8 insertions(+), 5 deletions(-) create mode 100644 changelogs/unreleased/26692-predefined-variable-gitlab-user-name.yml diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index ace86e5e517..b7dcae0f9b3 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -385,6 +385,7 @@ module Ci [ { key: 'GITLAB_USER_ID', value: user.id.to_s, public: true }, { key: 'GITLAB_USER_EMAIL', value: user.email, public: true }, + { key: 'GITLAB_USER_LOGIN', value: user.username, public: true }, { key: 'GITLAB_USER_NAME', value: user.name, public: true } ] end diff --git a/changelogs/unreleased/26692-predefined-variable-gitlab-user-name.yml b/changelogs/unreleased/26692-predefined-variable-gitlab-user-name.yml new file mode 100644 index 00000000000..fa1ca3d25b2 --- /dev/null +++ b/changelogs/unreleased/26692-predefined-variable-gitlab-user-name.yml @@ -0,0 +1,5 @@ +--- +title: Add CI/CD job predefined variables with user name and login +merge_request: 13824 +author: +type: added diff --git a/doc/ci/variables/README.md b/doc/ci/variables/README.md index 4b48a1f1c51..2b189c80066 100644 --- a/doc/ci/variables/README.md +++ b/doc/ci/variables/README.md @@ -77,12 +77,8 @@ future GitLab releases.** | **GITLAB_CI** | all | all | Mark that job is executed in GitLab CI environment | | **GITLAB_USER_ID** | 8.12 | all | The id of the user who started the job | | **GITLAB_USER_EMAIL** | 8.12 | all | The email of the user who started the job | -<<<<<<< HEAD -| **GITLAB_USER_NAME** | 10.0 | all | The name of the user who started the job | -======= -| **GITLAB_USER_USERNAME** | 10.0 | all | The username of the user who started the job | +| **GITLAB_USER_LOGIN** | 10.0 | all | The login username of the user who started the job | | **GITLAB_USER_NAME** | 10.0 | all | The real name of the user who started the job | ->>>>>>> 940c584882... fixup name | **RESTORE_CACHE_ATTEMPTS** | 8.15 | 1.9 | Number of attempts to restore the cache running a job | ## 9.0 Renaming diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 97fce7ded74..1a6c373ab45 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1250,6 +1250,7 @@ describe Ci::Build do [ { key: 'GITLAB_USER_ID', value: user.id.to_s, public: true }, { key: 'GITLAB_USER_EMAIL', value: user.email, public: true }, + { key: 'GITLAB_USER_LOGIN', value: user.username, public: true }, { key: 'GITLAB_USER_NAME', value: user.name, public: true } ] end -- cgit v1.2.1 From c158a22fd3cc5c3d60b5e0e1bf44f8aa75617d70 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Fri, 25 Aug 2017 15:26:22 +0200 Subject: Only update the sidebar count caches when needed This ensures the issues/MR cache of the sidebar is only updated when the state or confidential flags changes, instead of changing this for every update. --- app/models/issue.rb | 6 ++++++ app/models/merge_request.rb | 6 ++++++ changelogs/unreleased/sidebar-cache-updates.yml | 5 +++++ spec/models/issue_spec.rb | 18 ++++++++++++++++++ spec/models/merge_request_spec.rb | 12 ++++++++++++ 5 files changed, 47 insertions(+) create mode 100644 changelogs/unreleased/sidebar-cache-updates.yml diff --git a/app/models/issue.rb b/app/models/issue.rb index b9aa937d2f9..dfcd4030ec3 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -269,7 +269,13 @@ class Issue < ActiveRecord::Base end end + def update_project_counter_caches? + state_changed? || confidential_changed? + end + def update_project_counter_caches + return unless update_project_counter_caches? + Projects::OpenIssuesCountService.new(project).refresh_cache end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 7f73de67625..764bc08923e 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -942,7 +942,13 @@ class MergeRequest < ActiveRecord::Base true end + def update_project_counter_caches? + state_changed? + end + def update_project_counter_caches + return unless update_project_counter_caches? + Projects::OpenMergeRequestsCountService.new(target_project).refresh_cache end diff --git a/changelogs/unreleased/sidebar-cache-updates.yml b/changelogs/unreleased/sidebar-cache-updates.yml new file mode 100644 index 00000000000..aebe53ba5b2 --- /dev/null +++ b/changelogs/unreleased/sidebar-cache-updates.yml @@ -0,0 +1,5 @@ +--- +title: Only update the sidebar count caches when needed +merge_request: +author: +type: other diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index de86788d142..e547da0cfbe 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -769,4 +769,22 @@ describe Issue do expect(described_class.public_only).to eq([public_issue]) end end + + describe '#update_project_counter_caches?' do + it 'returns true when the state changes' do + subject.state = 'closed' + + expect(subject.update_project_counter_caches?).to eq(true) + end + + it 'returns true when the confidential flag changes' do + subject.confidential = true + + expect(subject.update_project_counter_caches?).to eq(true) + end + + it 'returns false when the state or confidential flag did not change' do + expect(subject.update_project_counter_caches?).to eq(false) + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 92cf15a5a51..09f3b97ec58 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1700,4 +1700,16 @@ describe MergeRequest do .to change { project.open_merge_requests_count }.from(1).to(0) end end + + describe '#update_project_counter_caches?' do + it 'returns true when the state changes' do + subject.state = 'closed' + + expect(subject.update_project_counter_caches?).to eq(true) + end + + it 'returns false when the state did not change' do + expect(subject.update_project_counter_caches?).to eq(false) + end + end end -- cgit v1.2.1 From 4342f81af28cf4abdeefe9e0b7f5281ff644c43d Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Wed, 30 Aug 2017 15:16:37 +0100 Subject: Removes disabled state from projects dropdown in dashboard page --- .../javascripts/project_select_combo_button.js | 14 ++++-- .../37179-dashboard-project-dropdown.yml | 5 ++ .../project_select_combo_button_spec.js | 55 ++++++++-------------- 3 files changed, 35 insertions(+), 39 deletions(-) create mode 100644 changelogs/unreleased/37179-dashboard-project-dropdown.yml diff --git a/app/assets/javascripts/project_select_combo_button.js b/app/assets/javascripts/project_select_combo_button.js index 46a26fb91f4..55e7b830716 100644 --- a/app/assets/javascripts/project_select_combo_button.js +++ b/app/assets/javascripts/project_select_combo_button.js @@ -13,8 +13,16 @@ export default class ProjectSelectComboButton { } bindEvents() { - this.projectSelectInput.siblings('.new-project-item-select-button') - .on('click', this.openDropdown); + const dropdownButton = this.projectSelectInput.siblings('.new-project-item-select-button'); + + dropdownButton.on('click', this.openDropdown); + + this.newItemBtn.on('click', (e) => { + if (!this.getProjectFromLocalStorage()) { + e.preventDefault(); + dropdownButton.trigger('click'); + } + }); this.projectSelectInput.on('change', () => this.selectProject()); } @@ -56,10 +64,8 @@ export default class ProjectSelectComboButton { if (project) { this.newItemBtn.attr('href', project.url); this.newItemBtn.text(`${this.formattedText.defaultTextPrefix} in ${project.name}`); - this.newItemBtn.enable(); } else { this.newItemBtn.text(`Select project to create ${this.formattedText.presetTextSuffix}`); - this.newItemBtn.disable(); } } diff --git a/changelogs/unreleased/37179-dashboard-project-dropdown.yml b/changelogs/unreleased/37179-dashboard-project-dropdown.yml new file mode 100644 index 00000000000..3ef080b8eae --- /dev/null +++ b/changelogs/unreleased/37179-dashboard-project-dropdown.yml @@ -0,0 +1,5 @@ +--- +title: Removes disabled state from dashboard project button +merge_request: +author: +type: fixed diff --git a/spec/javascripts/project_select_combo_button_spec.js b/spec/javascripts/project_select_combo_button_spec.js index 021804e0769..eb724e1b93e 100644 --- a/spec/javascripts/project_select_combo_button_spec.js +++ b/spec/javascripts/project_select_combo_button_spec.js @@ -2,10 +2,10 @@ import ProjectSelectComboButton from '~/project_select_combo_button'; const fixturePath = 'static/project_select_combo_button.html.raw'; -describe('Project Select Combo Button', function () { +describe('Project Select Combo Button', () => { preloadFixtures(fixturePath); - beforeEach(function () { + beforeEach(() => { this.defaults = { label: 'Select project to create issue', groupId: 12345, @@ -27,53 +27,43 @@ describe('Project Select Combo Button', function () { this.projectSelectInput = document.querySelector('.project-item-select'); }); - describe('on page load when localStorage is empty', function () { - beforeEach(function () { + describe('on page load when localStorage is empty', () => { + beforeEach(() => { this.comboButton = new ProjectSelectComboButton(this.projectSelectInput); }); - it('newItemBtn is disabled', function () { - expect(this.newItemBtn.hasAttribute('disabled')).toBe(true); - expect(this.newItemBtn.classList.contains('disabled')).toBe(true); - }); - - it('newItemBtn href is null', function () { + it('newItemBtn href is null', () => { expect(this.newItemBtn.getAttribute('href')).toBe(''); }); - it('newItemBtn text is the plain default label', function () { + it('newItemBtn text is the plain default label', () => { expect(this.newItemBtn.textContent).toBe(this.defaults.label); }); }); - describe('on page load when localStorage is filled', function () { - beforeEach(function () { + describe('on page load when localStorage is filled', () => { + beforeEach(() => { window.localStorage .setItem(this.defaults.localStorageKey, JSON.stringify(this.defaults.projectMeta)); this.comboButton = new ProjectSelectComboButton(this.projectSelectInput); }); - it('newItemBtn is not disabled', function () { - expect(this.newItemBtn.hasAttribute('disabled')).toBe(false); - expect(this.newItemBtn.classList.contains('disabled')).toBe(false); - }); - - it('newItemBtn href is correctly set', function () { + it('newItemBtn href is correctly set', () => { expect(this.newItemBtn.getAttribute('href')).toBe(this.defaults.projectMeta.url); }); - it('newItemBtn text is the cached label', function () { + it('newItemBtn text is the cached label', () => { expect(this.newItemBtn.textContent) .toBe(`New issue in ${this.defaults.projectMeta.name}`); }); - afterEach(function () { + afterEach(() => { window.localStorage.clear(); }); }); - describe('after selecting a new project', function () { - beforeEach(function () { + describe('after selecting a new project', () => { + beforeEach(() => { this.comboButton = new ProjectSelectComboButton(this.projectSelectInput); // mock the effect of selecting an item from the projects dropdown (select2) @@ -82,28 +72,23 @@ describe('Project Select Combo Button', function () { .trigger('change'); }); - it('newItemBtn is not disabled', function () { - expect(this.newItemBtn.hasAttribute('disabled')).toBe(false); - expect(this.newItemBtn.classList.contains('disabled')).toBe(false); - }); - - it('newItemBtn href is correctly set', function () { + it('newItemBtn href is correctly set', () => { expect(this.newItemBtn.getAttribute('href')) .toBe('http://myothercoolproject.com/issues/new'); }); - it('newItemBtn text is the selected project label', function () { + it('newItemBtn text is the selected project label', () => { expect(this.newItemBtn.textContent) .toBe(`New issue in ${this.defaults.newProjectMeta.name}`); }); - afterEach(function () { + afterEach(() => { window.localStorage.clear(); }); }); - describe('deriveTextVariants', function () { - beforeEach(function () { + describe('deriveTextVariants', () => { + beforeEach(() => { this.mockExecutionContext = { resourceType: '', resourceLabel: '', @@ -114,7 +99,7 @@ describe('Project Select Combo Button', function () { this.method = this.comboButton.deriveTextVariants.bind(this.mockExecutionContext); }); - it('correctly derives test variants for merge requests', function () { + it('correctly derives test variants for merge requests', () => { this.mockExecutionContext.resourceType = 'merge_requests'; this.mockExecutionContext.resourceLabel = 'New merge request'; @@ -125,7 +110,7 @@ describe('Project Select Combo Button', function () { expect(returnedVariants.presetTextSuffix).toBe('merge request'); }); - it('correctly derives text variants for issues', function () { + it('correctly derives text variants for issues', () => { this.mockExecutionContext.resourceType = 'issues'; this.mockExecutionContext.resourceLabel = 'New issue'; -- cgit v1.2.1 From 0232cabca33cf07ea6ea6cebaaa63f59cbf18784 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Wed, 30 Aug 2017 15:44:44 +0100 Subject: puts back `function` statements --- .../project_select_combo_button_spec.js | 40 +++++++++++----------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/spec/javascripts/project_select_combo_button_spec.js b/spec/javascripts/project_select_combo_button_spec.js index eb724e1b93e..dda83645c92 100644 --- a/spec/javascripts/project_select_combo_button_spec.js +++ b/spec/javascripts/project_select_combo_button_spec.js @@ -2,10 +2,10 @@ import ProjectSelectComboButton from '~/project_select_combo_button'; const fixturePath = 'static/project_select_combo_button.html.raw'; -describe('Project Select Combo Button', () => { +describe('Project Select Combo Button', function () { preloadFixtures(fixturePath); - beforeEach(() => { + beforeEach(function () { this.defaults = { label: 'Select project to create issue', groupId: 12345, @@ -27,43 +27,43 @@ describe('Project Select Combo Button', () => { this.projectSelectInput = document.querySelector('.project-item-select'); }); - describe('on page load when localStorage is empty', () => { - beforeEach(() => { + describe('on page load when localStorage is empty', function () { + beforeEach(function () { this.comboButton = new ProjectSelectComboButton(this.projectSelectInput); }); - it('newItemBtn href is null', () => { + it('newItemBtn href is null', function () { expect(this.newItemBtn.getAttribute('href')).toBe(''); }); - it('newItemBtn text is the plain default label', () => { + it('newItemBtn text is the plain default label', function () { expect(this.newItemBtn.textContent).toBe(this.defaults.label); }); }); - describe('on page load when localStorage is filled', () => { - beforeEach(() => { + describe('on page load when localStorage is filled', function () { + beforeEach(function () { window.localStorage .setItem(this.defaults.localStorageKey, JSON.stringify(this.defaults.projectMeta)); this.comboButton = new ProjectSelectComboButton(this.projectSelectInput); }); - it('newItemBtn href is correctly set', () => { + it('newItemBtn href is correctly set', function () { expect(this.newItemBtn.getAttribute('href')).toBe(this.defaults.projectMeta.url); }); - it('newItemBtn text is the cached label', () => { + it('newItemBtn text is the cached label', function () { expect(this.newItemBtn.textContent) .toBe(`New issue in ${this.defaults.projectMeta.name}`); }); - afterEach(() => { + afterEach(function () { window.localStorage.clear(); }); }); - describe('after selecting a new project', () => { - beforeEach(() => { + describe('after selecting a new project', function () { + beforeEach(function () { this.comboButton = new ProjectSelectComboButton(this.projectSelectInput); // mock the effect of selecting an item from the projects dropdown (select2) @@ -72,23 +72,23 @@ describe('Project Select Combo Button', () => { .trigger('change'); }); - it('newItemBtn href is correctly set', () => { + it('newItemBtn href is correctly set', function () { expect(this.newItemBtn.getAttribute('href')) .toBe('http://myothercoolproject.com/issues/new'); }); - it('newItemBtn text is the selected project label', () => { + it('newItemBtn text is the selected project label', function () { expect(this.newItemBtn.textContent) .toBe(`New issue in ${this.defaults.newProjectMeta.name}`); }); - afterEach(() => { + afterEach(function () { window.localStorage.clear(); }); }); - describe('deriveTextVariants', () => { - beforeEach(() => { + describe('deriveTextVariants', function () { + beforeEach(function () { this.mockExecutionContext = { resourceType: '', resourceLabel: '', @@ -99,7 +99,7 @@ describe('Project Select Combo Button', () => { this.method = this.comboButton.deriveTextVariants.bind(this.mockExecutionContext); }); - it('correctly derives test variants for merge requests', () => { + it('correctly derives test variants for merge requests', function () { this.mockExecutionContext.resourceType = 'merge_requests'; this.mockExecutionContext.resourceLabel = 'New merge request'; @@ -110,7 +110,7 @@ describe('Project Select Combo Button', () => { expect(returnedVariants.presetTextSuffix).toBe('merge request'); }); - it('correctly derives text variants for issues', () => { + it('correctly derives text variants for issues', function () { this.mockExecutionContext.resourceType = 'issues'; this.mockExecutionContext.resourceLabel = 'New issue'; -- cgit v1.2.1 From b9b0b37b3695d5925c3ba6cd90cdefcc3c67ba6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Wed, 30 Aug 2017 12:24:49 -0500 Subject: Add check for access to Namespace --- app/controllers/projects_controller.rb | 5 ++++- app/helpers/namespaces_helper.rb | 4 ++-- spec/controllers/projects_controller_spec.rb | 32 ++++++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 3 deletions(-) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 51cf37b9438..ed17b3b4689 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -20,7 +20,10 @@ class ProjectsController < Projects::ApplicationController end def new - @project ||= Project.new(params.permit(:namespace_id)) + namespace = Namespace.find_by(id: params[:namespace_id]) if params[:namespace_id] + return access_denied! if namespace && !can?(current_user, :create_projects, namespace) + + @project = Project.new(namespace_id: namespace&.id) end def edit diff --git a/app/helpers/namespaces_helper.rb b/app/helpers/namespaces_helper.rb index 3c784272df2..d7df9bb06d2 100644 --- a/app/helpers/namespaces_helper.rb +++ b/app/helpers/namespaces_helper.rb @@ -45,8 +45,8 @@ module NamespacesHelper visibility_level: n.visibility_level_value, visibility: n.visibility, name: n.name, - show_path: n.is_a?(Group) ? group_path(n) : user_path(n), - edit_path: n.is_a?(Group) ? edit_group_path(n) : nil + show_path: (type == 'group') ? group_path(n) : user_path(n), + edit_path: (type == 'group') ? edit_group_path(n) : nil }] end diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index c0e48046937..4459e227fb3 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -7,6 +7,38 @@ describe ProjectsController do let(:jpg) { fixture_file_upload(Rails.root + 'spec/fixtures/rails_sample.jpg', 'image/jpg') } let(:txt) { fixture_file_upload(Rails.root + 'spec/fixtures/doc_sample.txt', 'text/plain') } + describe 'GET new' do + context 'with an authenticated user' do + let(:group) { create(:group) } + + before do + sign_in(user) + end + + context 'when namespace_id param is present' do + context 'when user has access to the namespace' do + it 'renders the template' do + group.add_owner(user) + + get :new, namespace_id: group.id + + expect(response).to have_http_status(200) + expect(response).to render_template('new') + end + end + + context 'when user does not have access to the namespace' do + it 'responds with status 404' do + get :new, namespace_id: group.id + + expect(response).to have_http_status(404) + expect(response).not_to render_template('new') + end + end + end + end + end + describe 'GET index' do context 'as a user' do it 'redirects to root page' do -- cgit v1.2.1 From f3b6c552f6763e20bbccaa80e306d939d6cf602c Mon Sep 17 00:00:00 2001 From: Marc Siegfriedt Date: Mon, 21 Aug 2017 21:09:45 +0000 Subject: fix :file_path - add requirements: --- changelogs/unreleased/31470-fix-api-files-raw.yml | 5 +++++ lib/api/files.rb | 14 ++++++++------ spec/requests/api/files_spec.rb | 9 +++++++++ 3 files changed, 22 insertions(+), 6 deletions(-) create mode 100644 changelogs/unreleased/31470-fix-api-files-raw.yml diff --git a/changelogs/unreleased/31470-fix-api-files-raw.yml b/changelogs/unreleased/31470-fix-api-files-raw.yml new file mode 100644 index 00000000000..271a945a998 --- /dev/null +++ b/changelogs/unreleased/31470-fix-api-files-raw.yml @@ -0,0 +1,5 @@ +--- +title: Fix the /projects/:id/repository/files/:file_path/raw endpoint to handle dots in the file_path +merge_request: 13512 +author: mahcsig +type: fixed diff --git a/lib/api/files.rb b/lib/api/files.rb index e2ac7142bc4..1598d3c00b8 100644 --- a/lib/api/files.rb +++ b/lib/api/files.rb @@ -1,5 +1,7 @@ module API class Files < Grape::API + FILE_ENDPOINT_REQUIREMENTS = API::PROJECT_ENDPOINT_REQUIREMENTS.merge(file_path: API::NO_SLASH_URL_PART_REGEX) + # Prevents returning plain/text responses for files with .txt extension after_validation { content_type "application/json" } @@ -58,13 +60,13 @@ module API params do requires :id, type: String, desc: 'The project ID' end - resource :projects, requirements: { id: %r{[^/]+} } do + resource :projects, requirements: FILE_ENDPOINT_REQUIREMENTS do desc 'Get raw file contents from the repository' params do requires :file_path, type: String, desc: 'The url encoded path to the file. Ex. lib%2Fclass%2Erb' requires :ref, type: String, desc: 'The name of branch, tag commit' end - get ":id/repository/files/:file_path/raw" do + get ":id/repository/files/:file_path/raw", requirements: FILE_ENDPOINT_REQUIREMENTS do assign_file_vars! send_git_blob @repo, @blob @@ -75,7 +77,7 @@ module API requires :file_path, type: String, desc: 'The url encoded path to the file. Ex. lib%2Fclass%2Erb' requires :ref, type: String, desc: 'The name of branch, tag or commit' end - get ":id/repository/files/:file_path", requirements: { file_path: /.+/ } do + get ":id/repository/files/:file_path", requirements: FILE_ENDPOINT_REQUIREMENTS do assign_file_vars! { @@ -95,7 +97,7 @@ module API params do use :extended_file_params end - post ":id/repository/files/:file_path", requirements: { file_path: /.+/ } do + post ":id/repository/files/:file_path", requirements: FILE_ENDPOINT_REQUIREMENTS do authorize! :push_code, user_project file_params = declared_params(include_missing: false) @@ -113,7 +115,7 @@ module API params do use :extended_file_params end - put ":id/repository/files/:file_path", requirements: { file_path: /.+/ } do + put ":id/repository/files/:file_path", requirements: FILE_ENDPOINT_REQUIREMENTS do authorize! :push_code, user_project file_params = declared_params(include_missing: false) @@ -137,7 +139,7 @@ module API params do use :simple_file_params end - delete ":id/repository/files/:file_path", requirements: { file_path: /.+/ } do + delete ":id/repository/files/:file_path", requirements: FILE_ENDPOINT_REQUIREMENTS do authorize! :push_code, user_project file_params = declared_params(include_missing: false) diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index ea97c556430..971eaf837cb 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -125,6 +125,15 @@ describe API::Files do expect(response).to have_http_status(200) end + it 'returns raw file info for files with dots' do + url = route('.gitignore') + "/raw" + expect(Gitlab::Workhorse).to receive(:send_git_blob) + + get api(url, current_user), params + + expect(response).to have_http_status(200) + end + it 'returns file by commit sha' do # This file is deleted on HEAD file_path = "files%2Fjs%2Fcommit%2Ejs%2Ecoffee" -- cgit v1.2.1 From 7a0a9c23028aa487078bac6499bd86f3356c84a0 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Wed, 30 Aug 2017 19:47:44 +0100 Subject: Use the correct scope to avoid triggering all dropdowns --- app/assets/javascripts/project_select_combo_button.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/project_select_combo_button.js b/app/assets/javascripts/project_select_combo_button.js index 55e7b830716..99cea683d9a 100644 --- a/app/assets/javascripts/project_select_combo_button.js +++ b/app/assets/javascripts/project_select_combo_button.js @@ -13,14 +13,13 @@ export default class ProjectSelectComboButton { } bindEvents() { - const dropdownButton = this.projectSelectInput.siblings('.new-project-item-select-button'); - - dropdownButton.on('click', this.openDropdown); + this.projectSelectInput.siblings('.new-project-item-select-button') + .on('click', e => this.openDropdown(e)); this.newItemBtn.on('click', (e) => { if (!this.getProjectFromLocalStorage()) { e.preventDefault(); - dropdownButton.trigger('click'); + this.openDropdown(e); } }); @@ -36,8 +35,9 @@ export default class ProjectSelectComboButton { } } - openDropdown() { - $(this).siblings('.project-item-select').select2('open'); + // eslint-disable-next-line class-methods-use-this + openDropdown(event) { + $(event.currentTarget).siblings('.project-item-select').select2('open'); } selectProject() { -- cgit v1.2.1 From b77176d11a7a31acd38b05aa39afc9ebed5a3915 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20=22BKC=22=20Carlb=C3=A4cker?= Date: Thu, 10 Aug 2017 16:08:48 +0200 Subject: Migrate Repository.FetchRemote to Gitaly - `Gitlab::Shell.fetch_remote` now takes a `Gitlab::Git::Repository` instead --- GITALY_SERVER_VERSION | 2 +- app/models/repository.rb | 2 +- lib/gitlab/git/repository.rb | 3 ++ lib/gitlab/gitaly_client/repository_service.rb | 16 +++++++ lib/gitlab/shell.rb | 55 ++++++++++++++-------- spec/lib/gitlab/shell_spec.rb | 64 ++++++++++++++++++++------ 6 files changed, 107 insertions(+), 35 deletions(-) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index be386c9ede3..85e60ed180c 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.33.0 +0.34.0 diff --git a/app/models/repository.rb b/app/models/repository.rb index 9fb2e2aa306..9368207b0fb 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -1027,7 +1027,7 @@ class Repository end def fetch_remote(remote, forced: false, no_tags: false) - gitlab_shell.fetch_remote(repository_storage_path, disk_path, remote, forced: forced, no_tags: no_tags) + gitlab_shell.fetch_remote(raw_repository, remote, forced: forced, no_tags: no_tags) end def fetch_ref(source_path, source_ref, target_ref) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index b835dec24eb..ec923c990ae 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -46,6 +46,9 @@ module Gitlab # Directory name of repo attr_reader :name + # Relative path of repo + attr_reader :relative_path + # Rugged repo object attr_reader :rugged diff --git a/lib/gitlab/gitaly_client/repository_service.rb b/lib/gitlab/gitaly_client/repository_service.rb index a74a6dc6e78..177a1284f38 100644 --- a/lib/gitlab/gitaly_client/repository_service.rb +++ b/lib/gitlab/gitaly_client/repository_service.rb @@ -37,6 +37,22 @@ module Gitlab request = Gitaly::ApplyGitattributesRequest.new(repository: @gitaly_repo, revision: revision) GitalyClient.call(@storage, :repository_service, :apply_gitattributes, request) end + + def fetch_remote(remote, ssh_auth: nil, forced: false, no_tags: false) + request = Gitaly::FetchRemoteRequest.new(repository: @gitaly_repo, remote: remote, force: forced, no_tags: no_tags) + + if ssh_auth&.ssh_import? + if ssh_auth.ssh_key_auth? && ssh_auth.ssh_private_key.present? + request.ssh_key = ssh_auth.ssh_private_key + end + + if ssh_auth.ssh_known_hosts.present? + request.known_hosts = ssh_auth.ssh_known_hosts + end + end + + GitalyClient.call(@storage, :repository_service, :fetch_remote, request) + end end end end diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb index 280a9abf03e..81ecdf43ef9 100644 --- a/lib/gitlab/shell.rb +++ b/lib/gitlab/shell.rb @@ -98,33 +98,24 @@ module Gitlab # Fetch remote for repository # - # name - project path with namespace + # repository - an instance of Git::Repository # remote - remote name # forced - should we use --force flag? # no_tags - should we use --no-tags flag? # # Ex. - # fetch_remote("gitlab/gitlab-ci", "upstream") + # fetch_remote(my_repo, "upstream") # # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/387 - def fetch_remote(storage, name, remote, ssh_auth: nil, forced: false, no_tags: false) - args = [gitlab_shell_projects_path, 'fetch-remote', storage, "#{name}.git", remote, "#{Gitlab.config.gitlab_shell.git_timeout}"] - args << '--force' if forced - args << '--no-tags' if no_tags - - vars = {} - - if ssh_auth&.ssh_import? - if ssh_auth.ssh_key_auth? && ssh_auth.ssh_private_key.present? - vars['GITLAB_SHELL_SSH_KEY'] = ssh_auth.ssh_private_key - end - - if ssh_auth.ssh_known_hosts.present? - vars['GITLAB_SHELL_KNOWN_HOSTS'] = ssh_auth.ssh_known_hosts + def fetch_remote(repository, remote, ssh_auth: nil, forced: false, no_tags: false) + gitaly_migrate(:fetch_remote) do |is_enabled| + if is_enabled + repository.gitaly_repository_client.fetch_remote(remote, ssh_auth: ssh_auth, forced: forced, no_tags: no_tags) + else + storage_path = Gitlab.config.repositories.storages[repository.storage]["path"] + local_fetch_remote(storage_path, repository.relative_path, remote, ssh_auth: ssh_auth, forced: forced, no_tags: no_tags) end end - - gitlab_shell_fast_execute_raise_error(args, vars) end # Move repository @@ -302,6 +293,26 @@ module Gitlab private + def local_fetch_remote(storage, name, remote, ssh_auth: nil, forced: false, no_tags: false) + args = [gitlab_shell_projects_path, 'fetch-remote', storage, name, remote, "#{Gitlab.config.gitlab_shell.git_timeout}"] + args << '--force' if forced + args << '--no-tags' if no_tags + + vars = {} + + if ssh_auth&.ssh_import? + if ssh_auth.ssh_key_auth? && ssh_auth.ssh_private_key.present? + vars['GITLAB_SHELL_SSH_KEY'] = ssh_auth.ssh_private_key + end + + if ssh_auth.ssh_known_hosts.present? + vars['GITLAB_SHELL_KNOWN_HOSTS'] = ssh_auth.ssh_known_hosts + end + end + + gitlab_shell_fast_execute_raise_error(args, vars) + end + def gitlab_shell_fast_execute(cmd) output, status = gitlab_shell_fast_execute_helper(cmd) @@ -325,5 +336,13 @@ module Gitlab # from wasting I/O by searching through GEM_PATH Bundler.with_original_env { Popen.popen(cmd, nil, vars) } end + + def gitaly_migrate(method, &block) + Gitlab::GitalyClient.migrate(method, &block) + rescue GRPC::NotFound, GRPC::BadStatus => e + # Old Popen code returns [Error, output] to the caller, so we + # need to do the same here... + raise Error, e + end end end diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index cfadee0bcf5..c7930378240 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -186,22 +186,48 @@ describe Gitlab::Shell do end end - describe '#fetch_remote' do + shared_examples 'fetch_remote' do |gitaly_on| + let(:project2) { create(:project, :repository) } + let(:repository) { project2.repository } + def fetch_remote(ssh_auth = nil) - gitlab_shell.fetch_remote('current/storage', 'project/path', 'new/storage', ssh_auth: ssh_auth) + gitlab_shell.fetch_remote(repository.raw_repository, 'new/storage', ssh_auth: ssh_auth) end - def expect_popen(vars = {}) + def expect_popen(fail = false, vars = {}) popen_args = [ projects_path, 'fetch-remote', - 'current/storage', - 'project/path.git', + TestEnv.repos_path, + repository.relative_path, 'new/storage', Gitlab.config.gitlab_shell.git_timeout.to_s ] - expect(Gitlab::Popen).to receive(:popen).with(popen_args, nil, popen_vars.merge(vars)) + return_value = fail ? ["error", 1] : [nil, 0] + + expect(Gitlab::Popen).to receive(:popen).with(popen_args, nil, popen_vars.merge(vars)).and_return(return_value) + end + + def expect_gitaly_call(fail, vars = {}) + receive_fetch_remote = + if fail + receive(:fetch_remote).and_raise(GRPC::NotFound) + else + receive(:fetch_remote).and_return(true) + end + + expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive_fetch_remote + end + + if gitaly_on + def expect_call(fail, vars = {}) + expect_gitaly_call(fail, vars) + end + else + def expect_call(fail, vars = {}) + expect_popen(fail, vars) + end end def build_ssh_auth(opts = {}) @@ -216,20 +242,20 @@ describe Gitlab::Shell do end it 'returns true when the command succeeds' do - expect_popen.and_return([nil, 0]) + expect_call(false) expect(fetch_remote).to be_truthy end it 'raises an exception when the command fails' do - expect_popen.and_return(["error", 1]) + expect_call(true) - expect { fetch_remote }.to raise_error(Gitlab::Shell::Error, "error") + expect { fetch_remote }.to raise_error(Gitlab::Shell::Error) end context 'SSH auth' do it 'passes the SSH key if specified' do - expect_popen('GITLAB_SHELL_SSH_KEY' => 'foo').and_return([nil, 0]) + expect_call(false, 'GITLAB_SHELL_SSH_KEY' => 'foo') ssh_auth = build_ssh_auth(ssh_key_auth?: true, ssh_private_key: 'foo') @@ -237,7 +263,7 @@ describe Gitlab::Shell do end it 'does not pass an empty SSH key' do - expect_popen.and_return([nil, 0]) + expect_call(false) ssh_auth = build_ssh_auth(ssh_key_auth: true, ssh_private_key: '') @@ -245,7 +271,7 @@ describe Gitlab::Shell do end it 'does not pass the key unless SSH key auth is to be used' do - expect_popen.and_return([nil, 0]) + expect_call(false) ssh_auth = build_ssh_auth(ssh_key_auth: false, ssh_private_key: 'foo') @@ -253,7 +279,7 @@ describe Gitlab::Shell do end it 'passes the known_hosts data if specified' do - expect_popen('GITLAB_SHELL_KNOWN_HOSTS' => 'foo').and_return([nil, 0]) + expect_call(false, 'GITLAB_SHELL_KNOWN_HOSTS' => 'foo') ssh_auth = build_ssh_auth(ssh_known_hosts: 'foo') @@ -261,7 +287,7 @@ describe Gitlab::Shell do end it 'does not pass empty known_hosts data' do - expect_popen.and_return([nil, 0]) + expect_call(false) ssh_auth = build_ssh_auth(ssh_known_hosts: '') @@ -269,7 +295,7 @@ describe Gitlab::Shell do end it 'does not pass known_hosts data unless SSH is to be used' do - expect_popen(popen_vars).and_return([nil, 0]) + expect_call(false, popen_vars) ssh_auth = build_ssh_auth(ssh_import?: false, ssh_known_hosts: 'foo') @@ -278,6 +304,14 @@ describe Gitlab::Shell do end end + describe '#fetch_remote local', skip_gitaly_mock: true do + it_should_behave_like 'fetch_remote', false + end + + describe '#fetch_remote gitaly' do + it_should_behave_like 'fetch_remote', true + end + describe '#import_repository' do it 'returns true when the command succeeds' do expect(Gitlab::Popen).to receive(:popen) -- cgit v1.2.1 From 6cad21efbe25ffe1c0a3a153a25ed9601b50c427 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Wed, 30 Aug 2017 14:37:08 -0500 Subject: revert changes to visibility level helpers from 6f03ddc --- app/helpers/visibility_level_helper.rb | 10 +++++----- app/models/group.rb | 6 +++--- app/models/project.rb | 6 ++---- lib/gitlab/visibility_level.rb | 18 ------------------ 4 files changed, 10 insertions(+), 30 deletions(-) diff --git a/app/helpers/visibility_level_helper.rb b/app/helpers/visibility_level_helper.rb index caadc12019c..a13127a6365 100644 --- a/app/helpers/visibility_level_helper.rb +++ b/app/helpers/visibility_level_helper.rb @@ -82,11 +82,11 @@ module VisibilityLevelHelper reasons = [] unless project.visibility_level_allowed_as_fork?(level) - reasons << project.visibility_error_for(:fork, level: level_name) + reasons << "the fork source project has lower visibility" end unless project.visibility_level_allowed_by_group?(level) - reasons << project.visibility_error_for(:group, level: level_name, group_level: project.group.visibility) + reasons << "the visibility of #{project.group.name} is #{project.group.visibility}" end reasons = reasons.any? ? ' because ' + reasons.to_sentence : '' @@ -98,15 +98,15 @@ module VisibilityLevelHelper reasons = [] unless group.visibility_level_allowed_by_projects?(level) - reasons << group.visibility_error_for(:projects, level: level_name) + reasons << "it contains projects with higher visibility" end unless group.visibility_level_allowed_by_sub_groups?(level) - reasons << group.visibility_error_for(:sub_groups, level: level_name) + reasons << "it contains sub-groups with higher visibility" end unless group.visibility_level_allowed_by_parent?(level) - reasons << group.visibility_error_for(:parent, level: level_name, parent_level: group.parent.visibility) + reasons << "the visibility of its parent group is #{group.parent.visibility}" end reasons = reasons.any? ? ' because ' + reasons.to_sentence : '' diff --git a/app/models/group.rb b/app/models/group.rb index 0e6d88f6a91..3778b832a47 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -297,18 +297,18 @@ class Group < Namespace def visibility_level_allowed_by_parent return if visibility_level_allowed_by_parent? - errors.add(:visibility_level, visibility_error_for(:parent, level: visibility, parent_level: parent.visibility)) + errors.add(:visibility_level, "#{visibility} is not allowed since the parent group has a #{parent.visibility} visibility.") end def visibility_level_allowed_by_projects return if visibility_level_allowed_by_projects? - errors.add(:visibility_level, visibility_error_for(:projects, level: visibility)) + errors.add(:visibility_level, "#{visibility} is not allowed since this group contains projects with higher visibility.") end def visibility_level_allowed_by_sub_groups return if visibility_level_allowed_by_sub_groups? - errors.add(:visibility_level, visibility_error_for(:sub_groups, level: visibility)) + errors.add(:visibility_level, "#{visibility} is not allowed since there are sub-groups with higher visibility.") end end diff --git a/app/models/project.rb b/app/models/project.rb index 088fe5e1fed..d5324ceac31 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -671,16 +671,14 @@ class Project < ActiveRecord::Base level_name = Gitlab::VisibilityLevel.level_name(self.visibility_level).downcase group_level_name = Gitlab::VisibilityLevel.level_name(self.group.visibility_level).downcase - - self.errors.add(:visibility_level, visibility_error_for(:group, level: level_name, group_level: group_level_name)) + self.errors.add(:visibility_level, "#{level_name} is not allowed in a #{group_level_name} group.") end def visibility_level_allowed_as_fork return if visibility_level_allowed_as_fork? level_name = Gitlab::VisibilityLevel.level_name(self.visibility_level).downcase - - self.errors.add(:visibility_level, visibility_error_for(:fork, level: level_name)) + self.errors.add(:visibility_level, "#{level_name} is not allowed since the fork source project has lower visibility.") end def check_wiki_path_conflict diff --git a/lib/gitlab/visibility_level.rb b/lib/gitlab/visibility_level.rb index ef5c4d9a3b2..c60bd91ea6e 100644 --- a/lib/gitlab/visibility_level.rb +++ b/lib/gitlab/visibility_level.rb @@ -130,23 +130,5 @@ module Gitlab def visibility=(level) self[visibility_level_field] = Gitlab::VisibilityLevel.level_value(level) end - - def visibility_errors_template - @visibility_errors ||= { - 'Project' => { - group: "%{level} is not allowed in a %{group_level} group", - fork: "%{level} is not allowed since the fork source project has lower visibility" - }, - 'Group' => { - parent: "%{level} is not allowed since the parent group has a %{parent_level} visibility", - projects: "%{level} is not allowed since this group contains projects with higher visibility", - sub_groups: "%{level} is not allowed since there are sub-groups with higher visibility" - } - } - end - - def visibility_error_for(section, variables) - visibility_errors_template.dig(model_name.to_s, section) % variables - end end end -- cgit v1.2.1 From b1a14f5fc94aa900e06aaddb3d13a5775ccffc54 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Wed, 30 Aug 2017 14:40:09 -0500 Subject: add notes to the disabled visibility setting string helper to ensure changes are reflected in the model as well --- app/helpers/visibility_level_helper.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/helpers/visibility_level_helper.rb b/app/helpers/visibility_level_helper.rb index a13127a6365..19550d37b2f 100644 --- a/app/helpers/visibility_level_helper.rb +++ b/app/helpers/visibility_level_helper.rb @@ -77,6 +77,8 @@ module VisibilityLevelHelper end end + # Note: these messages closely mirror the form validation strings found in the project + # model and any changes or additons to these may also need to be made there. def disallowed_project_visibility_level_description(level, project) level_name = Gitlab::VisibilityLevel.level_name(level).downcase reasons = [] @@ -93,6 +95,8 @@ module VisibilityLevelHelper "This project cannot be #{level_name}#{reasons}." end + # Note: these messages closely mirror the form validation strings found in the group + # model and any changes or additons to these may also need to be made there. def disallowed_group_visibility_level_description(level, group) level_name = Gitlab::VisibilityLevel.level_name(level).downcase reasons = [] -- cgit v1.2.1 From 6dd69950edf6b98600d0bead148969ef347bc1ec Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Wed, 30 Aug 2017 15:08:30 -0500 Subject: add links and instructions to disabled visibility option help text --- app/helpers/visibility_level_helper.rb | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/app/helpers/visibility_level_helper.rb b/app/helpers/visibility_level_helper.rb index 19550d37b2f..569872e528b 100644 --- a/app/helpers/visibility_level_helper.rb +++ b/app/helpers/visibility_level_helper.rb @@ -82,17 +82,22 @@ module VisibilityLevelHelper def disallowed_project_visibility_level_description(level, project) level_name = Gitlab::VisibilityLevel.level_name(level).downcase reasons = [] + instructions = '' unless project.visibility_level_allowed_as_fork?(level) reasons << "the fork source project has lower visibility" end unless project.visibility_level_allowed_by_group?(level) - reasons << "the visibility of #{project.group.name} is #{project.group.visibility}" + group = link_to project.group.name, group_path(project.group) + change_visiblity = link_to 'change the visibility', edit_group_path(project.group) + + reasons << "the visibility of #{group} is #{project.group.visibility}" + instructions << " To make this project #{level_name}, you must first #{change_visiblity} of the parent group." end reasons = reasons.any? ? ' because ' + reasons.to_sentence : '' - "This project cannot be #{level_name}#{reasons}." + "This project cannot be #{level_name}#{reasons}.#{instructions}".html_safe end # Note: these messages closely mirror the form validation strings found in the group -- cgit v1.2.1 From 8a72203a589bc3b7a74f5dd60a3c607d21436e6a Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Wed, 30 Aug 2017 15:51:03 -0500 Subject: update test to match new disabled option description --- spec/helpers/visibility_level_helper_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/helpers/visibility_level_helper_spec.rb b/spec/helpers/visibility_level_helper_spec.rb index bd15abf5469..0375fa600ed 100644 --- a/spec/helpers/visibility_level_helper_spec.rb +++ b/spec/helpers/visibility_level_helper_spec.rb @@ -120,7 +120,7 @@ describe VisibilityLevelHelper do describe "project" do it "provides correct description for disabled levels" do expect(disallowed_visibility_level?(project, Gitlab::VisibilityLevel::PUBLIC)).to be_truthy - expect(disallowed_visibility_level_description(Gitlab::VisibilityLevel::PUBLIC, project)) + expect(strip_tags disallowed_visibility_level_description(Gitlab::VisibilityLevel::PUBLIC, project)) .to include "the visibility of #{project.group.name} is internal" end end -- cgit v1.2.1 From 129823664bb2287545b0402823366261151273ca Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Wed, 30 Aug 2017 16:01:57 -0500 Subject: enhance disabled group visibility options with links and instructions --- app/helpers/visibility_level_helper.rb | 9 +++++++-- spec/helpers/visibility_level_helper_spec.rb | 4 ++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/app/helpers/visibility_level_helper.rb b/app/helpers/visibility_level_helper.rb index 569872e528b..e21a7b04bb4 100644 --- a/app/helpers/visibility_level_helper.rb +++ b/app/helpers/visibility_level_helper.rb @@ -105,6 +105,7 @@ module VisibilityLevelHelper def disallowed_group_visibility_level_description(level, group) level_name = Gitlab::VisibilityLevel.level_name(level).downcase reasons = [] + instructions = '' unless group.visibility_level_allowed_by_projects?(level) reasons << "it contains projects with higher visibility" @@ -115,11 +116,15 @@ module VisibilityLevelHelper end unless group.visibility_level_allowed_by_parent?(level) - reasons << "the visibility of its parent group is #{group.parent.visibility}" + parent_group = link_to group.parent.name, group_path(group.parent) + change_visiblity = link_to 'change the visibility', edit_group_path(group.parent) + + reasons << "the visibility of #{parent_group} is #{group.parent.visibility}" + instructions << " To make this group #{level_name}, you must first #{change_visiblity} of the parent group." end reasons = reasons.any? ? ' because ' + reasons.to_sentence : '' - "This group cannot be #{level_name}#{reasons}." + "This group cannot be #{level_name}#{reasons}.#{instructions}".html_safe end def visibility_icon_description(form_model) diff --git a/spec/helpers/visibility_level_helper_spec.rb b/spec/helpers/visibility_level_helper_spec.rb index 0375fa600ed..5077c89d7b4 100644 --- a/spec/helpers/visibility_level_helper_spec.rb +++ b/spec/helpers/visibility_level_helper_spec.rb @@ -132,8 +132,8 @@ describe VisibilityLevelHelper do .to include "it contains projects with higher visibility", "it contains sub-groups with higher visibility" expect(disallowed_visibility_level?(subgroup, Gitlab::VisibilityLevel::PUBLIC)).to be_truthy - expect(disallowed_visibility_level_description(Gitlab::VisibilityLevel::PUBLIC, subgroup)) - .to include "the visibility of its parent group is internal" + expect(strip_tags disallowed_visibility_level_description(Gitlab::VisibilityLevel::PUBLIC, subgroup)) + .to include "the visibility of #{group.name} is internal" end end end -- cgit v1.2.1 From 8ffbab216b90c7743ee15a652bb72eaf460bc3ba Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Wed, 30 Aug 2017 16:27:20 -0500 Subject: ensure disabled radio options still show up as checked when necessary --- app/views/shared/_visibility_radios.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/shared/_visibility_radios.html.haml b/app/views/shared/_visibility_radios.html.haml index 4a656ccfeac..0ec7677a566 100644 --- a/app/views/shared/_visibility_radios.html.haml +++ b/app/views/shared/_visibility_radios.html.haml @@ -4,7 +4,7 @@ - disabled = disallowed || restricted .radio{ class: [('disabled' if disabled), ('restricted' if restricted)] } = form.label "#{model_method}_#{level}" do - = form.radio_button model_method, level, checked: (selected_level == level && !disabled), disabled: disabled + = form.radio_button model_method, level, checked: (selected_level == level), disabled: disabled = visibility_level_icon(level) .option-title = visibility_level_label(level) -- cgit v1.2.1 From 68de5dcba28d83089fd563434ba9d1ba1d882b76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Wed, 30 Aug 2017 18:38:06 -0500 Subject: Fix error reported by Flay --- app/helpers/visibility_level_helper.rb | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/app/helpers/visibility_level_helper.rb b/app/helpers/visibility_level_helper.rb index e21a7b04bb4..4b4f7c6a57a 100644 --- a/app/helpers/visibility_level_helper.rb +++ b/app/helpers/visibility_level_helper.rb @@ -89,11 +89,10 @@ module VisibilityLevelHelper end unless project.visibility_level_allowed_by_group?(level) - group = link_to project.group.name, group_path(project.group) - change_visiblity = link_to 'change the visibility', edit_group_path(project.group) + errors = visibility_level_errors_for_group(project.group, level_name) - reasons << "the visibility of #{group} is #{project.group.visibility}" - instructions << " To make this project #{level_name}, you must first #{change_visiblity} of the parent group." + reasons << errors[:reason] + instructions << errors[:instruction] end reasons = reasons.any? ? ' because ' + reasons.to_sentence : '' @@ -116,11 +115,10 @@ module VisibilityLevelHelper end unless group.visibility_level_allowed_by_parent?(level) - parent_group = link_to group.parent.name, group_path(group.parent) - change_visiblity = link_to 'change the visibility', edit_group_path(group.parent) + errors = visibility_level_errors_for_group(group.parent, level_name) - reasons << "the visibility of #{parent_group} is #{group.parent.visibility}" - instructions << " To make this group #{level_name}, you must first #{change_visiblity} of the parent group." + reasons << errors[:reason] + instructions << errors[:instruction] end reasons = reasons.any? ? ' because ' + reasons.to_sentence : '' @@ -163,4 +161,14 @@ module VisibilityLevelHelper return false unless form_model.respond_to?(:visibility_level_allowed?) !form_model.visibility_level_allowed?(level) end + + private + + def visibility_level_errors_for_group(group, level_name) + group = link_to group.name, group_path(group) + change_visiblity = link_to 'change the visibility', edit_group_path(group) + + { reason: "the visibility of #{group} is #{group.visibility}", + instruction: " To make this group #{level_name}, you must first #{change_visiblity} of the parent group." } + end end -- cgit v1.2.1 From cf37f0b173abacaef36660f1c9875f8fee8b78d8 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Wed, 30 Aug 2017 19:33:24 -0500 Subject: fix variable naming conflict --- app/helpers/visibility_level_helper.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/helpers/visibility_level_helper.rb b/app/helpers/visibility_level_helper.rb index 4b4f7c6a57a..46867d2d974 100644 --- a/app/helpers/visibility_level_helper.rb +++ b/app/helpers/visibility_level_helper.rb @@ -165,10 +165,10 @@ module VisibilityLevelHelper private def visibility_level_errors_for_group(group, level_name) - group = link_to group.name, group_path(group) + group_name = link_to group.name, group_path(group) change_visiblity = link_to 'change the visibility', edit_group_path(group) - { reason: "the visibility of #{group} is #{group.visibility}", + { reason: "the visibility of #{group_name} is #{group.visibility}", instruction: " To make this group #{level_name}, you must first #{change_visiblity} of the parent group." } end end -- cgit v1.2.1 From d74fecac031df1c3b4e817f49f7bafe2b175be11 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Wed, 30 Aug 2017 21:14:29 -0700 Subject: Filter additional secrets from Rails logs Upon inspection of logs, there were a number of fields not filtered. For example: * authenticity_token: CSRF token * rss_token: Used for RSS feeds * secret: Used with Projects::UploadController Rails provides a way to match regexps, so we now filter: * Any parameter ending with `_token` * Any parameter containing `password` * Any parameter containing `secret` --- changelogs/unreleased/sh-filter-csrf-params.yml | 5 +++++ config/application.rb | 15 ++++----------- 2 files changed, 9 insertions(+), 11 deletions(-) create mode 100644 changelogs/unreleased/sh-filter-csrf-params.yml diff --git a/changelogs/unreleased/sh-filter-csrf-params.yml b/changelogs/unreleased/sh-filter-csrf-params.yml new file mode 100644 index 00000000000..70eb3321e77 --- /dev/null +++ b/changelogs/unreleased/sh-filter-csrf-params.yml @@ -0,0 +1,5 @@ +--- +title: Filter additional secrets from Rails logs +merge_request: +author: +type: security diff --git a/config/application.rb b/config/application.rb index f69dab4de39..32a290f2002 100644 --- a/config/application.rb +++ b/config/application.rb @@ -51,31 +51,24 @@ module Gitlab # Configure sensitive parameters which will be filtered from the log file. # # Parameters filtered: - # - Password (:password, :password_confirmation) - # - Private tokens + # - Any parameter ending with `_token` + # - Any parameter containing `password` + # - Any parameter containing `secret` # - Two-factor tokens (:otp_attempt) # - Repo/Project Import URLs (:import_url) # - Build variables (:variables) # - GitLab Pages SSL cert/key info (:certificate, :encrypted_key) # - Webhook URLs (:hook) - # - GitLab-shell secret token (:secret_token) # - Sentry DSN (:sentry_dsn) # - Deploy keys (:key) + config.filter_parameters += [/_token$/, /password/, /secret/] config.filter_parameters += %i( - authentication_token certificate encrypted_key hook import_url - incoming_email_token - rss_token key otp_attempt - password - password_confirmation - private_token - runners_token - secret_token sentry_dsn variables ) -- cgit v1.2.1 From 04cd47dd5a08ca5cc84c44346b2893111da9594c Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Tue, 29 Aug 2017 12:15:19 +0200 Subject: Don't show references to Pages when not available In this instance its subgroups, and given we can't deploy it, we shouldn't allow it to be shown. Fixes gitlab-org/gitlab-ce#34864 --- app/controllers/projects/application_controller.rb | 2 +- app/models/namespace.rb | 4 ++++ app/models/project.rb | 4 ++++ .../layouts/nav/_new_project_sidebar.html.haml | 2 +- app/views/projects/settings/_head.html.haml | 2 +- .../unreleased/zj-disable-pages-in-subgroups.yml | 5 +++++ spec/controllers/projects/pages_controller_spec.rb | 13 ++++++++++++- spec/models/project_spec.rb | 22 ++++++++++++++++++++++ 8 files changed, 50 insertions(+), 4 deletions(-) create mode 100644 changelogs/unreleased/zj-disable-pages-in-subgroups.yml diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb index 221e01b415a..d7dd8ddcb7d 100644 --- a/app/controllers/projects/application_controller.rb +++ b/app/controllers/projects/application_controller.rb @@ -94,6 +94,6 @@ class Projects::ApplicationController < ApplicationController end def require_pages_enabled! - not_found unless Gitlab.config.pages.enabled + not_found unless @project.pages_available? end end diff --git a/app/models/namespace.rb b/app/models/namespace.rb index e7bc1d1b080..e7cbc5170e8 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -195,6 +195,10 @@ class Namespace < ActiveRecord::Base parent.present? end + def subgroup? + has_parent? + end + def soft_delete_without_removing_associations # We can't use paranoia's `#destroy` since this will hard-delete projects. # Project uses `pending_delete` instead of the acts_as_paranoia gem. diff --git a/app/models/project.rb b/app/models/project.rb index d5324ceac31..db7183e4c9a 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1235,6 +1235,10 @@ class Project < ActiveRecord::Base File.join(pages_path, 'public') end + def pages_available? + Gitlab.config.pages.enabled && !namespace.subgroup? + end + def remove_private_deploy_keys exclude_keys_linked_to_other_projects = <<-SQL NOT EXISTS ( diff --git a/app/views/layouts/nav/_new_project_sidebar.html.haml b/app/views/layouts/nav/_new_project_sidebar.html.haml index 341943cf833..6ae816d0bbb 100644 --- a/app/views/layouts/nav/_new_project_sidebar.html.haml +++ b/app/views/layouts/nav/_new_project_sidebar.html.haml @@ -208,7 +208,7 @@ = link_to project_settings_ci_cd_path(@project), title: 'CI / CD' do %span CI / CD - - if Gitlab.config.pages.enabled + - if @project.pages_available? = nav_link(controller: :pages) do = link_to project_pages_path(@project), title: 'Pages' do %span diff --git a/app/views/projects/settings/_head.html.haml b/app/views/projects/settings/_head.html.haml index 15ba09b10ba..7d24c6a9122 100644 --- a/app/views/projects/settings/_head.html.haml +++ b/app/views/projects/settings/_head.html.haml @@ -23,7 +23,7 @@ = link_to project_settings_ci_cd_path(@project), title: 'Pipelines' do %span Pipelines - - if Gitlab.config.pages.enabled + - if @project.pages_available? = nav_link(controller: :pages) do = link_to project_pages_path(@project), title: 'Pages' do %span diff --git a/changelogs/unreleased/zj-disable-pages-in-subgroups.yml b/changelogs/unreleased/zj-disable-pages-in-subgroups.yml new file mode 100644 index 00000000000..22c36214e1f --- /dev/null +++ b/changelogs/unreleased/zj-disable-pages-in-subgroups.yml @@ -0,0 +1,5 @@ +--- +title: Remove pages settings when not available +merge_request: +author: +type: changed diff --git a/spec/controllers/projects/pages_controller_spec.rb b/spec/controllers/projects/pages_controller_spec.rb index 4d0111302f3..83c7744a231 100644 --- a/spec/controllers/projects/pages_controller_spec.rb +++ b/spec/controllers/projects/pages_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Projects::PagesController do let(:user) { create(:user) } - let(:project) { create(:project, :public, :access_requestable) } + let(:project) { create(:project, :public) } let(:request_params) do { @@ -23,6 +23,17 @@ describe Projects::PagesController do expect(response).to have_http_status(200) end + + context 'when the project is in a subgroup' do + let(:group) { create(:group, :nested) } + let(:project) { create(:project, namespace: group) } + + it 'returns a 404 status code' do + get :show, request_params + + expect(response).to have_http_status(404) + end + end end describe 'DELETE destroy' do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 2e613c44357..867f629264c 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2225,6 +2225,28 @@ describe Project do end end + describe '#pages_available?' do + let(:project) { create(:project, group: group) } + + subject { project.pages_available? } + + before do + allow(Gitlab.config.pages).to receive(:enabled).and_return(true) + end + + context 'when the project is in a top level namespace' do + let(:group) { create(:group) } + + it { is_expected.to be(true) } + end + + context 'when the project is in a subgroup' do + let(:group) { create(:group, :nested) } + + it { is_expected.to be(false) } + end + end + describe '#remove_private_deploy_keys' do let!(:project) { create(:project) } -- cgit v1.2.1 From a4f3b03571e2866c92f01d3f026b6d85c409a950 Mon Sep 17 00:00:00 2001 From: Maxim Rydkin Date: Thu, 31 Aug 2017 10:51:28 +0300 Subject: Decrease Metrics/PerceivedComplexity threshold to 17 --- .rubocop.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.rubocop.yml b/.rubocop.yml index 3edd1643d48..0c7928f2ef5 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -665,7 +665,7 @@ Metrics/ParameterLists: # A complexity metric geared towards measuring complexity for a human reader. Metrics/PerceivedComplexity: Enabled: true - Max: 18 + Max: 17 # Lint ######################################################################## -- cgit v1.2.1 From f212210dd97ca3ab0953531dcce892f2939676f8 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 30 Aug 2017 12:54:23 +0200 Subject: Optimize system note service specs --- spec/services/system_note_service_spec.rb | 30 ++++++++---------------------- 1 file changed, 8 insertions(+), 22 deletions(-) diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index e6a18654651..c2d6d7781b9 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -3,9 +3,9 @@ require 'spec_helper' describe SystemNoteService do include Gitlab::Routing - let(:group) { create(:group) } - let(:project) { create(:project, group: group) } - let(:author) { create(:user) } + set(:group) { create(:group) } + set(:project) { create(:project, :repository, group: group) } + set(:author) { create(:user) } let(:noteable) { create(:issue, project: project) } let(:issue) { noteable } @@ -29,8 +29,7 @@ describe SystemNoteService do describe '.add_commits' do subject { described_class.add_commits(noteable, project, author, new_commits, old_commits, oldrev) } - let(:project) { create(:project, :repository) } - let(:noteable) { create(:merge_request, source_project: project) } + let(:noteable) { create(:merge_request, source_project: project, target_project: project) } let(:new_commits) { noteable.commits } let(:old_commits) { [] } let(:oldrev) { nil } @@ -185,7 +184,7 @@ describe SystemNoteService do describe '.change_label' do subject { described_class.change_label(noteable, project, author, added, removed) } - let(:labels) { create_list(:label, 2) } + let(:labels) { create_list(:label, 2, project: project) } let(:added) { [] } let(:removed) { [] } @@ -294,7 +293,6 @@ describe SystemNoteService do end describe '.merge_when_pipeline_succeeds' do - let(:project) { create(:project, :repository) } let(:pipeline) { build(:ci_pipeline_without_jobs )} let(:noteable) do create(:merge_request, source_project: project, target_project: project) @@ -312,7 +310,6 @@ describe SystemNoteService do end describe '.cancel_merge_when_pipeline_succeeds' do - let(:project) { create(:project, :repository) } let(:noteable) do create(:merge_request, source_project: project, target_project: project) end @@ -390,7 +387,6 @@ describe SystemNoteService do describe '.change_branch' do subject { described_class.change_branch(noteable, project, author, 'target', old_branch, new_branch) } - let(:project) { create(:project, :repository) } let(:old_branch) { 'old_branch'} let(:new_branch) { 'new_branch'} @@ -408,8 +404,6 @@ describe SystemNoteService do describe '.change_branch_presence' do subject { described_class.change_branch_presence(noteable, project, author, :source, 'feature', :delete) } - let(:project) { create(:project, :repository) } - it_behaves_like 'a system note' do let(:action) { 'branch' } end @@ -424,8 +418,6 @@ describe SystemNoteService do describe '.new_issue_branch' do subject { described_class.new_issue_branch(noteable, project, author, "1-mepmep") } - let(:project) { create(:project, :repository) } - it_behaves_like 'a system note' do let(:action) { 'branch' } end @@ -471,7 +463,7 @@ describe SystemNoteService do describe 'note_body' do context 'cross-project' do - let(:project2) { create(:project, :repository) } + let(:project2) { create(:project, :repository) } let(:mentioner) { create(:issue, project: project2) } context 'from Commit' do @@ -491,7 +483,6 @@ describe SystemNoteService do context 'within the same project' do context 'from Commit' do - let(:project) { create(:project, :repository) } let(:mentioner) { project.repository.commit } it 'references the mentioning commit' do @@ -533,7 +524,6 @@ describe SystemNoteService do end context 'when mentioner is a MergeRequest' do - let(:project) { create(:project, :repository) } let(:mentioner) { create(:merge_request, :simple, source_project: project) } let(:noteable) { project.commit } @@ -561,7 +551,6 @@ describe SystemNoteService do end describe '.cross_reference_exists?' do - let(:project) { create(:project, :repository) } let(:commit0) { project.commit } let(:commit1) { project.commit('HEAD~2') } @@ -899,9 +888,8 @@ describe SystemNoteService do end describe '.discussion_continued_in_issue' do - let(:discussion) { create(:diff_note_on_merge_request).to_discussion } + let(:discussion) { create(:diff_note_on_merge_request, project: project).to_discussion } let(:merge_request) { discussion.noteable } - let(:project) { merge_request.source_project } let(:issue) { create(:issue, project: project) } def reloaded_merge_request @@ -1023,7 +1011,6 @@ describe SystemNoteService do end describe '.add_merge_request_wip_from_commit' do - let(:project) { create(:project, :repository) } let(:noteable) do create(:merge_request, source_project: project, target_project: project) end @@ -1078,9 +1065,8 @@ describe SystemNoteService do end describe '.diff_discussion_outdated' do - let(:discussion) { create(:diff_note_on_merge_request).to_discussion } + let(:discussion) { create(:diff_note_on_merge_request, project: project).to_discussion } let(:merge_request) { discussion.noteable } - let(:project) { merge_request.source_project } let(:change_position) { discussion.position } def reloaded_merge_request -- cgit v1.2.1 From 35cd92a69cf74f137031f3e35d25785dcca67d68 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Thu, 31 Aug 2017 10:38:23 +0000 Subject: Adds a tooltip to the branch name --- .../javascripts/vue_shared/components/commit.vue | 20 +++++++++++++++----- changelogs/unreleased/36917-branch-tooltip.yml | 5 +++++ 2 files changed, 20 insertions(+), 5 deletions(-) create mode 100644 changelogs/unreleased/36917-branch-tooltip.yml diff --git a/app/assets/javascripts/vue_shared/components/commit.vue b/app/assets/javascripts/vue_shared/components/commit.vue index 262584769e0..50d14282cad 100644 --- a/app/assets/javascripts/vue_shared/components/commit.vue +++ b/app/assets/javascripts/vue_shared/components/commit.vue @@ -1,6 +1,7 @@