From 26a7fff7deb2cbbbd88875a0da5a74f0a85bc382 Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Tue, 4 Jul 2017 12:40:48 +0100 Subject: Upgrade GitLab Pages to v0.5.0 --- GITLAB_PAGES_VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/GITLAB_PAGES_VERSION b/GITLAB_PAGES_VERSION index 17b2ccd9bf9..8f0916f768f 100644 --- a/GITLAB_PAGES_VERSION +++ b/GITLAB_PAGES_VERSION @@ -1 +1 @@ -0.4.3 +0.5.0 -- cgit v1.2.1 From f63ca524c9d80107d2328e3d8175d04aad9b4b8a Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Thu, 6 Jul 2017 14:34:26 +0200 Subject: Configure token on client side too --- doc/administration/gitaly/index.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/doc/administration/gitaly/index.md b/doc/administration/gitaly/index.md index 332457cb384..005ef9388cb 100644 --- a/doc/administration/gitaly/index.md +++ b/doc/administration/gitaly/index.md @@ -149,6 +149,8 @@ git_data_dirs({ { 'default' => { 'path' => '/mnt/gitlab/default', 'gitaly_address' => 'tcp://gitlab.internal:9999' } }, { 'storage1' => { 'path' => '/mnt/gitlab/storage1', 'gitaly_address' => 'tcp://gitlab.internal:9999' } }, }) + +gitlab_rails['gitaly_token'] = 'abc123secret' ``` Source installations: @@ -164,6 +166,9 @@ gitlab: storage1: path: /mnt/gitlab/storage1/repositories gitaly_address: tcp://gitlab.internal:9999 + + gitaly: + token: 'abc123secret' ``` Now reconfigure (Omnibus) or restart (source). When you tail the -- cgit v1.2.1 From d195db17e9ff62c3dbfb8ba03dacadf965b1fb8b Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 6 Jul 2017 14:21:08 -0500 Subject: Don't show auxiliary blob viewer for README when there is no wiki --- app/models/blob_viewer/readme.rb | 6 +++ app/models/project_wiki.rb | 4 ++ app/views/projects/blob/viewers/_readme.html.haml | 2 +- ...m-readme-auxiliary-blob-viewer-without-wiki.yml | 4 ++ spec/models/blob_viewer/readme_spec.rb | 49 ++++++++++++++++++++++ 5 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/dm-readme-auxiliary-blob-viewer-without-wiki.yml create mode 100644 spec/models/blob_viewer/readme_spec.rb diff --git a/app/models/blob_viewer/readme.rb b/app/models/blob_viewer/readme.rb index 75c373a03bb..4604a9934a0 100644 --- a/app/models/blob_viewer/readme.rb +++ b/app/models/blob_viewer/readme.rb @@ -10,5 +10,11 @@ module BlobViewer def visible_to?(current_user) can?(current_user, :read_wiki, project) end + + def render_error + return if project.has_external_wiki? || (project.wiki_enabled? && project.wiki.has_home_page?) + + :no_wiki + end end end diff --git a/app/models/project_wiki.rb b/app/models/project_wiki.rb index beaadbbd1ab..dfca0031af8 100644 --- a/app/models/project_wiki.rb +++ b/app/models/project_wiki.rb @@ -63,6 +63,10 @@ class ProjectWiki !!repository.exists? end + def has_home_page? + !!find_page('home') + end + # Returns an Array of Gitlab WikiPage instances or an # empty Array if this Wiki has no pages. def pages diff --git a/app/views/projects/blob/viewers/_readme.html.haml b/app/views/projects/blob/viewers/_readme.html.haml index 507f44d4745..d8492abc638 100644 --- a/app/views/projects/blob/viewers/_readme.html.haml +++ b/app/views/projects/blob/viewers/_readme.html.haml @@ -1,4 +1,4 @@ = icon('info-circle fw') = succeed '.' do To learn more about this project, read - = link_to "the wiki", project_wikis_path(viewer.project) + = link_to "the wiki", get_project_wiki_path(viewer.project) diff --git a/changelogs/unreleased/dm-readme-auxiliary-blob-viewer-without-wiki.yml b/changelogs/unreleased/dm-readme-auxiliary-blob-viewer-without-wiki.yml new file mode 100644 index 00000000000..8b026a4c289 --- /dev/null +++ b/changelogs/unreleased/dm-readme-auxiliary-blob-viewer-without-wiki.yml @@ -0,0 +1,4 @@ +--- +title: Don't show auxiliary blob viewer for README when there is no wiki +merge_request: +author: diff --git a/spec/models/blob_viewer/readme_spec.rb b/spec/models/blob_viewer/readme_spec.rb new file mode 100644 index 00000000000..02679dbb544 --- /dev/null +++ b/spec/models/blob_viewer/readme_spec.rb @@ -0,0 +1,49 @@ +require 'spec_helper' + +describe BlobViewer::Readme, model: true do + include FakeBlobHelpers + + let(:project) { create(:project, :repository) } + let(:blob) { fake_blob(path: 'README.md') } + subject { described_class.new(blob) } + + describe '#render_error' do + context 'when there is no wiki' do + it 'returns :no_wiki' do + expect(subject.render_error).to eq(:no_wiki) + end + end + + context 'when there is an external wiki' do + before do + project.has_external_wiki = true + end + + it 'returns nil' do + expect(subject.render_error).to be_nil + end + end + + context 'when there is a local wiki' do + before do + project.wiki_enabled = true + end + + context 'when the wiki is empty' do + it 'returns :no_wiki' do + expect(subject.render_error).to eq(:no_wiki) + end + end + + context 'when the wiki is not empty' do + before do + WikiPages::CreateService.new(project, project.owner, title: 'home', content: 'Home page').execute + end + + it 'returns nil' do + expect(subject.render_error).to be_nil + end + end + end + end +end -- cgit v1.2.1 From 5b0954759cc24bdba97be89bb117c5440174f859 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 4 May 2017 03:51:55 +0900 Subject: Basic BE change Fix static-snalysis Move the precedence of group secure variable before project secure variable. Allow project_id to be null. Separate Ci::VariableProject and Ci::VariableGroup Add the forgotton files Add migration file to update type of ci_variables Fix form_for fpr VariableProject Fix test Change the table structure according to the yorik advice Add necessary migration files. Remove unnecessary migration spec. Revert safe_model_attributes.yml Fix models Fix spec Avoid self.variable. Use becomes for correct routing. Use unique index on group_id and key Add null: false for t.timestamps Fix schema version Rename VariableProject and VariableGroup to ProjectVariable and GroupVariable Rename the rest of them Add the rest of files Basic BE change Fix static-snalysis Move the precedence of group secure variable before project secure variable. Allow project_id to be null. Separate Ci::VariableProject and Ci::VariableGroup Add the forgotton files Add migration file to update type of ci_variables Fix form_for fpr VariableProject Fix test Change the table structure according to the yorik advice Add necessary migration files. Remove unnecessary migration spec. Revert safe_model_attributes.yml Fix models Fix spec Avoid self.variable. Use becomes for correct routing. Use unique index on group_id and key Add null: false for t.timestamps Fix schema version Rename VariableProject and VariableGroup to ProjectVariable and GroupVariable Rename the rest of them Add the rest of files Implement CURD Rename codes related to VariableGroup and VariableProject FE part Remove unneccesary changes Make Fe code up-to-date Add protected flag to migration file Protected group variables essential package Update schema Improve doc Fix logic and spec for models Fix logic and spec for controllers Fix logic and spec for views(pre feature) Add feature spec Fixed bugs. placeholder. reveal button. doc. Add changelog Remove unnecessary comment godfat nice catches Improve secret_variables_for arctecture Fix spec Fix StaticAnlysys & path_regex spec Revert "Improve secret_variables_for arctecture" This reverts commit c3216ca212322ecf6ca534cb12ce75811a4e77f1. Use ayufan suggestion for secret_variables_for Use find instead of find_by Fix spec message for variable is invalid Fix spec remove variable.group_id = group.id godffat spec nitpicks Use include Gitlab::Routing.url_helpers for presenter spec --- app/assets/javascripts/dispatcher.js | 1 + .../groups/settings/ci_cd_controller.rb | 24 +++++++ app/controllers/groups/variables_controller.rb | 57 ++++++++++++++++ .../projects/settings/ci_cd_controller.rb | 5 +- app/controllers/projects/variables_controller.rb | 33 +++++---- app/models/ci/build.rb | 1 + app/models/ci/group_variable.rb | 13 ++++ app/models/ci/variable.rb | 1 + app/models/group.rb | 8 +++ app/policies/group_policy.rb | 2 + app/presenters/ci/group_variable_presenter.rb | 25 +++++++ app/presenters/ci/variable_presenter.rb | 25 +++++++ app/views/ci/variables/_content.html.haml | 9 +++ app/views/ci/variables/_form.html.haml | 19 ++++++ app/views/ci/variables/_index.html.haml | 16 +++++ app/views/ci/variables/_show.html.haml | 9 +++ app/views/ci/variables/_table.html.haml | 28 ++++++++ app/views/groups/_settings_head.html.haml | 5 ++ app/views/groups/settings/ci_cd/show.html.haml | 4 ++ app/views/groups/variables/show.html.haml | 1 + app/views/projects/settings/ci_cd/show.html.haml | 2 +- app/views/projects/variables/_content.html.haml | 9 --- app/views/projects/variables/_form.html.haml | 19 ------ app/views/projects/variables/_index.html.haml | 16 ----- app/views/projects/variables/_table.html.haml | 28 -------- app/views/projects/variables/show.html.haml | 10 +-- ...e-intermediate-12729-group-secret-variables.yml | 4 ++ config/routes/group.rb | 6 ++ .../20170525130346_create_group_variables_table.rb | 23 +++++++ ...525130758_add_foreign_key_to_group_variables.rb | 15 +++++ db/schema.rb | 15 +++++ doc/ci/variables/README.md | 21 +++++- lib/gitlab/path_regex.rb | 2 + .../groups/settings/ci_cd_controller_spec.rb | 20 ++++++ .../groups/variables_controller_spec.rb | 56 ++++++++++++++++ .../projects/variables_controller_spec.rb | 3 +- spec/factories/ci/group_variables.rb | 12 ++++ spec/features/group_variables_spec.rb | 78 ++++++++++++++++++++++ spec/features/variables_spec.rb | 10 +-- spec/models/ci/build_spec.rb | 53 +++++++++++++++ spec/models/ci/group_variable_spec.rb | 31 +++++++++ spec/models/ci/variable_spec.rb | 3 +- spec/models/group_spec.rb | 59 ++++++++++++++++ .../presenters/ci/group_variable_presenter_spec.rb | 63 +++++++++++++++++ spec/presenters/ci/variable_presenter_spec.rb | 63 +++++++++++++++++ 45 files changed, 794 insertions(+), 113 deletions(-) create mode 100644 app/controllers/groups/settings/ci_cd_controller.rb create mode 100644 app/controllers/groups/variables_controller.rb create mode 100644 app/models/ci/group_variable.rb create mode 100644 app/presenters/ci/group_variable_presenter.rb create mode 100644 app/presenters/ci/variable_presenter.rb create mode 100644 app/views/ci/variables/_content.html.haml create mode 100644 app/views/ci/variables/_form.html.haml create mode 100644 app/views/ci/variables/_index.html.haml create mode 100644 app/views/ci/variables/_show.html.haml create mode 100644 app/views/ci/variables/_table.html.haml create mode 100644 app/views/groups/settings/ci_cd/show.html.haml create mode 100644 app/views/groups/variables/show.html.haml delete mode 100644 app/views/projects/variables/_content.html.haml delete mode 100644 app/views/projects/variables/_form.html.haml delete mode 100644 app/views/projects/variables/_index.html.haml delete mode 100644 app/views/projects/variables/_table.html.haml create mode 100644 changelogs/unreleased/feature-intermediate-12729-group-secret-variables.yml create mode 100644 db/migrate/20170525130346_create_group_variables_table.rb create mode 100644 db/migrate/20170525130758_add_foreign_key_to_group_variables.rb create mode 100644 spec/controllers/groups/settings/ci_cd_controller_spec.rb create mode 100644 spec/controllers/groups/variables_controller_spec.rb create mode 100644 spec/factories/ci/group_variables.rb create mode 100644 spec/features/group_variables_spec.rb create mode 100644 spec/models/ci/group_variable_spec.rb create mode 100644 spec/presenters/ci/group_variable_presenter_spec.rb create mode 100644 spec/presenters/ci/variable_presenter_spec.rb diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index ffd8d494a40..81267c68cfc 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -396,6 +396,7 @@ import PerformanceBar from './performance_bar'; initSettingsPanels(); break; case 'projects:settings:ci_cd:show': + case 'groups:settings:ci_cd:show': new gl.ProjectVariables(); break; case 'ci:lints:create': diff --git a/app/controllers/groups/settings/ci_cd_controller.rb b/app/controllers/groups/settings/ci_cd_controller.rb new file mode 100644 index 00000000000..0142ad8278c --- /dev/null +++ b/app/controllers/groups/settings/ci_cd_controller.rb @@ -0,0 +1,24 @@ +module Groups + module Settings + class CiCdController < Groups::ApplicationController + before_action :authorize_admin_pipeline! + + def show + define_secret_variables + end + + private + + def define_secret_variables + @variable = Ci::GroupVariable.new(group: group) + .present(current_user: current_user) + @variables = group.variables.order_key_asc + .map { |variable| variable.present(current_user: current_user) } + end + + def authorize_admin_pipeline! + return render_404 unless can?(current_user, :admin_pipeline, group) + end + end + end +end diff --git a/app/controllers/groups/variables_controller.rb b/app/controllers/groups/variables_controller.rb new file mode 100644 index 00000000000..6fba9dab2a7 --- /dev/null +++ b/app/controllers/groups/variables_controller.rb @@ -0,0 +1,57 @@ +module Groups + class VariablesController < Groups::ApplicationController + before_action :variable, only: [:show, :update, :destroy] + before_action :authorize_admin_build! + + def index + redirect_to group_settings_ci_cd_path(group) + end + + def show + end + + def update + if variable.update(group_params) + redirect_to group_variables_path(group), + notice: 'Variable was successfully updated.' + else + render "show" + end + end + + def create + new_variable = Ci::GroupVariable.new(group_params) + + if new_variable.valid? && group.variables << new_variable + redirect_to group_settings_ci_cd_path(group), + notice: 'Variables were successfully updated.' + else + @variable = new_variable.present(current_user: current_user) + render "show" + end + end + + def destroy + variable.destroy + + redirect_to group_settings_ci_cd_path(group), + status: 302, + notice: 'Variable was successfully removed.' + end + + private + + def authorize_admin_build! + return render_404 unless can?(current_user, :admin_build, group) + end + + def group_params + params.require(:variable) + .permit([:key, :value, :protected]) + end + + def variable + @variable ||= group.variables.find(params[:id]).present(current_user: current_user) + end + end +end diff --git a/app/controllers/projects/settings/ci_cd_controller.rb b/app/controllers/projects/settings/ci_cd_controller.rb index 24fe78bc1bd..ea7ceb3eaa5 100644 --- a/app/controllers/projects/settings/ci_cd_controller.rb +++ b/app/controllers/projects/settings/ci_cd_controller.rb @@ -21,7 +21,10 @@ module Projects end def define_secret_variables - @variable = Ci::Variable.new + @variable = Ci::Variable.new(project: project) + .present(current_user: current_user) + @variables = project.variables.order_key_asc + .map { |variable| variable.present(current_user: current_user) } end def define_triggers_variables diff --git a/app/controllers/projects/variables_controller.rb b/app/controllers/projects/variables_controller.rb index 326d31ecec2..716e1347604 100644 --- a/app/controllers/projects/variables_controller.rb +++ b/app/controllers/projects/variables_controller.rb @@ -1,53 +1,50 @@ class Projects::VariablesController < Projects::ApplicationController + before_action :variable, only: [:show, :update, :destroy] before_action :authorize_admin_build! layout 'project_settings' def index - redirect_to project_settings_ci_cd_path(@project) + redirect_to namespace_project_settings_ci_cd_path(@project.namespace, @project) end def show - @variable = @project.variables.find(params[:id]) end def update - @variable = @project.variables.find(params[:id]) - - if @variable.update_attributes(variable_params) - redirect_to project_variables_path(project), notice: 'Variable was successfully updated.' + if @variable.update(project_params) + redirect_to namespace_project_variables_path(project.namespace, project), notice: 'Variable was successfully updated.' else - render action: "show" + render "show" end end def create - @variable = @project.variables.new(variable_params) + @variable = Ci::Variable.new(project_params) - if @variable.save - flash[:notice] = 'Variables were successfully updated.' - redirect_to project_settings_ci_cd_path(project) + if @variable.valid? && @project.variables << @variable + redirect_to namespace_project_settings_ci_cd_path(project.namespace, project), notice: 'Variables were successfully updated.' else render "show" end end def destroy - @key = @project.variables.find(params[:id]) - @key.destroy + variable.destroy - redirect_to project_settings_ci_cd_path(project), + redirect_to namespace_project_settings_ci_cd_path(project.namespace, project), status: 302, notice: 'Variable was successfully removed.' end private - def variable_params - params.require(:variable).permit(*variable_params_attributes) + def project_params + params.require(:variable) + .permit([:id, :key, :value, :protected, :_destroy]) end - def variable_params_attributes - %i[id key value protected _destroy] + def variable + @variable ||= project.variables.find(params[:id]).present(current_user: current_user) end end diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index ba2ecbf82a2..25e75a19f37 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -200,6 +200,7 @@ module Ci variables += project.deployment_variables if has_environment? variables += yaml_variables variables += user_variables + variables += project.group.secret_variables_for(ref, project).map(&:to_runner_variable) if project.group variables += secret_variables(environment: environment) variables += trigger_request.user_variables if trigger_request variables += persisted_environment_variables if environment diff --git a/app/models/ci/group_variable.rb b/app/models/ci/group_variable.rb new file mode 100644 index 00000000000..f64bc245a67 --- /dev/null +++ b/app/models/ci/group_variable.rb @@ -0,0 +1,13 @@ +module Ci + class GroupVariable < ActiveRecord::Base + extend Ci::Model + include HasVariable + include Presentable + + belongs_to :group + + validates :key, uniqueness: { scope: :group_id } + + scope :unprotected, -> { where(protected: false) } + end +end diff --git a/app/models/ci/variable.rb b/app/models/ci/variable.rb index 0b8d0ff881a..cf0fe04ddaf 100644 --- a/app/models/ci/variable.rb +++ b/app/models/ci/variable.rb @@ -2,6 +2,7 @@ module Ci class Variable < ActiveRecord::Base extend Ci::Model include HasVariable + include Presentable belongs_to :project diff --git a/app/models/group.rb b/app/models/group.rb index b93fce6100d..f625b8c250c 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -22,6 +22,7 @@ class Group < Namespace has_many :shared_projects, through: :project_group_links, source: :project has_many :notification_settings, dependent: :destroy, as: :source # rubocop:disable Cop/ActiveRecordDependent has_many :labels, class_name: 'GroupLabel' + has_many :variables, class_name: 'Ci::GroupVariable' validate :avatar_type, if: ->(user) { user.avatar.present? && user.avatar_changed? } validate :visibility_level_allowed_by_projects @@ -248,6 +249,13 @@ class Group < Namespace } end + def secret_variables_for(ref, project) + variables = [] + variables += parent.secret_variables_for(ref, project) if has_parent? + variables += project.protected_for?(ref) ? self.variables : self.variables.unprotected + variables + end + protected def update_two_factor_requirement diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index dcb37416ca3..6defab75fce 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -31,6 +31,8 @@ class GroupPolicy < BasePolicy rule { master }.policy do enable :create_projects enable :admin_milestones + enable :admin_pipeline + enable :admin_build end rule { owner }.policy do diff --git a/app/presenters/ci/group_variable_presenter.rb b/app/presenters/ci/group_variable_presenter.rb new file mode 100644 index 00000000000..81fea106a5c --- /dev/null +++ b/app/presenters/ci/group_variable_presenter.rb @@ -0,0 +1,25 @@ +module Ci + class GroupVariablePresenter < Gitlab::View::Presenter::Delegated + presents :variable + + def placeholder + 'GROUP_VARIABLE' + end + + def form_path + if variable.persisted? + group_variable_path(group, variable) + else + group_variables_path(group) + end + end + + def edit_path + group_variable_path(group, variable) + end + + def delete_path + group_variable_path(group, variable) + end + end +end diff --git a/app/presenters/ci/variable_presenter.rb b/app/presenters/ci/variable_presenter.rb new file mode 100644 index 00000000000..3f14d972c50 --- /dev/null +++ b/app/presenters/ci/variable_presenter.rb @@ -0,0 +1,25 @@ +module Ci + class VariablePresenter < Gitlab::View::Presenter::Delegated + presents :variable + + def placeholder + 'PROJECT_VARIABLE' + end + + def form_path + if variable.persisted? + namespace_project_variable_path(project.namespace, project, variable) + else + namespace_project_variables_path(project.namespace, project) + end + end + + def edit_path + namespace_project_variable_path(project.namespace, project, variable) + end + + def delete_path + namespace_project_variable_path(project.namespace, project, variable) + end + end +end diff --git a/app/views/ci/variables/_content.html.haml b/app/views/ci/variables/_content.html.haml new file mode 100644 index 00000000000..98f618ca3b8 --- /dev/null +++ b/app/views/ci/variables/_content.html.haml @@ -0,0 +1,9 @@ +%h4.prepend-top-0 + Secret variables + = link_to icon('question-circle'), help_page_path('ci/variables/README', anchor: 'secret-variables'), target: '_blank' +%p + These variables will be set to environment by the runner, and could be protected by exposing only to protected branches or tags. +%p + So you can use them for passwords, secret keys or whatever you want. +%p + The value of the variable can be visible in job log if explicitly asked to do so. diff --git a/app/views/ci/variables/_form.html.haml b/app/views/ci/variables/_form.html.haml new file mode 100644 index 00000000000..eebd0955c80 --- /dev/null +++ b/app/views/ci/variables/_form.html.haml @@ -0,0 +1,19 @@ += form_for @variable, as: :variable, url: @variable.form_path do |f| + = form_errors(@variable) + + .form-group + = f.label :key, "Key", class: "label-light" + = f.text_field :key, class: "form-control", placeholder: @variable.placeholder, required: true + .form-group + = f.label :value, "Value", class: "label-light" + = f.text_area :value, class: "form-control", placeholder: @variable.placeholder + .form-group + .checkbox + = f.label :protected do + = f.check_box :protected + %strong Protected + .help-block + This variable will be passed only to pipelines running on protected branches and tags + = link_to icon('question-circle'), help_page_path('ci/variables/README', anchor: 'protected-secret-variables'), target: '_blank' + + = f.submit btn_text, class: "btn btn-save" diff --git a/app/views/ci/variables/_index.html.haml b/app/views/ci/variables/_index.html.haml new file mode 100644 index 00000000000..007c2344b5a --- /dev/null +++ b/app/views/ci/variables/_index.html.haml @@ -0,0 +1,16 @@ +.row.prepend-top-default.append-bottom-default + .col-lg-4 + = render "ci/variables/content" + .col-lg-8 + %h5.prepend-top-0 + Add a variable + = render "ci/variables/form", btn_text: "Add new variable" + %hr + %h5.prepend-top-0 + Your variables (#{@variables.size}) + - if @variables.empty? + %p.settings-message.text-center.append-bottom-0 + No variables found, add one with the form above. + - else + = render "ci/variables/table" + %button.btn.btn-info.js-btn-toggle-reveal-values{ "data-status" => 'hidden' } Reveal Values diff --git a/app/views/ci/variables/_show.html.haml b/app/views/ci/variables/_show.html.haml new file mode 100644 index 00000000000..2bfb290629d --- /dev/null +++ b/app/views/ci/variables/_show.html.haml @@ -0,0 +1,9 @@ +- page_title "Variables" + +.row.prepend-top-default.append-bottom-default + .col-lg-3 + = render "ci/variables/content" + .col-lg-9 + %h5.prepend-top-0 + Update variable + = render "ci/variables/form", btn_text: "Save variable" diff --git a/app/views/ci/variables/_table.html.haml b/app/views/ci/variables/_table.html.haml new file mode 100644 index 00000000000..71a0b56c4f4 --- /dev/null +++ b/app/views/ci/variables/_table.html.haml @@ -0,0 +1,28 @@ +.table-responsive.variables-table + %table.table + %colgroup + %col + %col + %col + %col{ width: 100 } + %thead + %th Key + %th Value + %th Protected + %th + %tbody + - @variables.each do |variable| + - if variable.id? + %tr + %td.variable-key= variable.key + %td.variable-value{ "data-value" => variable.value }****** + %td.variable-protected= Gitlab::Utils.boolean_to_yes_no(variable.protected) + %td.variable-menu + = link_to variable.edit_path, class: "btn btn-transparent btn-variable-edit" do + %span.sr-only + Update + = icon("pencil") + = link_to variable.delete_path, class: "btn btn-transparent btn-variable-delete", method: :delete, data: { confirm: "Are you sure?" } do + %span.sr-only + Remove + = icon("trash") diff --git a/app/views/groups/_settings_head.html.haml b/app/views/groups/_settings_head.html.haml index 2454e7355a7..623d233a46a 100644 --- a/app/views/groups/_settings_head.html.haml +++ b/app/views/groups/_settings_head.html.haml @@ -12,3 +12,8 @@ = link_to projects_group_path(@group), title: 'Projects' do %span Projects + + = nav_link(controller: :ci_cd) do + = link_to group_settings_ci_cd_path(@group), title: 'Pipelines' do + %span + Pipelines diff --git a/app/views/groups/settings/ci_cd/show.html.haml b/app/views/groups/settings/ci_cd/show.html.haml new file mode 100644 index 00000000000..bf36baf48ab --- /dev/null +++ b/app/views/groups/settings/ci_cd/show.html.haml @@ -0,0 +1,4 @@ +- page_title "Pipelines" += render "groups/settings_head" + += render 'ci/variables/index' diff --git a/app/views/groups/variables/show.html.haml b/app/views/groups/variables/show.html.haml new file mode 100644 index 00000000000..df533952b76 --- /dev/null +++ b/app/views/groups/variables/show.html.haml @@ -0,0 +1 @@ += render 'ci/variables/show' diff --git a/app/views/projects/settings/ci_cd/show.html.haml b/app/views/projects/settings/ci_cd/show.html.haml index 00ccc3ec41e..6afb38c5709 100644 --- a/app/views/projects/settings/ci_cd/show.html.haml +++ b/app/views/projects/settings/ci_cd/show.html.haml @@ -3,6 +3,6 @@ = render "projects/settings/head" = render 'projects/runners/index' -= render 'projects/variables/index' += render 'ci/variables/index' = render 'projects/triggers/index' = render 'projects/pipelines_settings/show' diff --git a/app/views/projects/variables/_content.html.haml b/app/views/projects/variables/_content.html.haml deleted file mode 100644 index 98f618ca3b8..00000000000 --- a/app/views/projects/variables/_content.html.haml +++ /dev/null @@ -1,9 +0,0 @@ -%h4.prepend-top-0 - Secret variables - = link_to icon('question-circle'), help_page_path('ci/variables/README', anchor: 'secret-variables'), target: '_blank' -%p - These variables will be set to environment by the runner, and could be protected by exposing only to protected branches or tags. -%p - So you can use them for passwords, secret keys or whatever you want. -%p - The value of the variable can be visible in job log if explicitly asked to do so. diff --git a/app/views/projects/variables/_form.html.haml b/app/views/projects/variables/_form.html.haml deleted file mode 100644 index 0a70a301cb4..00000000000 --- a/app/views/projects/variables/_form.html.haml +++ /dev/null @@ -1,19 +0,0 @@ -= form_for [@project.namespace.becomes(Namespace), @project, @variable] do |f| - = form_errors(@variable) - - .form-group - = f.label :key, "Key", class: "label-light" - = f.text_field :key, class: "form-control", placeholder: "PROJECT_VARIABLE", required: true - .form-group - = f.label :value, "Value", class: "label-light" - = f.text_area :value, class: "form-control", placeholder: "PROJECT_VARIABLE" - .form-group - .checkbox - = f.label :protected do - = f.check_box :protected - %strong Protected - .help-block - This variable will be passed only to pipelines running on protected branches and tags - = link_to icon('question-circle'), help_page_path('ci/variables/README', anchor: 'protected-secret-variables'), target: '_blank' - - = f.submit btn_text, class: "btn btn-save" diff --git a/app/views/projects/variables/_index.html.haml b/app/views/projects/variables/_index.html.haml deleted file mode 100644 index 5e6786f6698..00000000000 --- a/app/views/projects/variables/_index.html.haml +++ /dev/null @@ -1,16 +0,0 @@ -.row.prepend-top-default.append-bottom-default - .col-lg-4 - = render "projects/variables/content" - .col-lg-8 - %h5.prepend-top-0 - Add a variable - = render "projects/variables/form", btn_text: "Add new variable" - %hr - %h5.prepend-top-0 - Your variables (#{@project.variables.size}) - - if @project.variables.empty? - %p.settings-message.text-center.append-bottom-0 - No variables found, add one with the form above. - - else - = render "projects/variables/table" - %button.btn.btn-info.js-btn-toggle-reveal-values{ "data-status" => 'hidden' } Reveal Values diff --git a/app/views/projects/variables/_table.html.haml b/app/views/projects/variables/_table.html.haml deleted file mode 100644 index 4ce6a828812..00000000000 --- a/app/views/projects/variables/_table.html.haml +++ /dev/null @@ -1,28 +0,0 @@ -.table-responsive.variables-table - %table.table - %colgroup - %col - %col - %col - %col{ width: 100 } - %thead - %th Key - %th Value - %th Protected - %th - %tbody - - @project.variables.order_key_asc.each do |variable| - - if variable.id? - %tr - %td.variable-key= variable.key - %td.variable-value{ "data-value" => variable.value }****** - %td.variable-protected= Gitlab::Utils.boolean_to_yes_no(variable.protected) - %td.variable-menu - = link_to project_variable_path(@project, variable), class: "btn btn-transparent btn-variable-edit" do - %span.sr-only - Update - = icon("pencil") - = link_to project_variable_path(@project, variable), class: "btn btn-transparent btn-variable-delete", method: :delete, data: { confirm: "Are you sure?" } do - %span.sr-only - Remove - = icon("trash") diff --git a/app/views/projects/variables/show.html.haml b/app/views/projects/variables/show.html.haml index 297a53ca98c..df533952b76 100644 --- a/app/views/projects/variables/show.html.haml +++ b/app/views/projects/variables/show.html.haml @@ -1,9 +1 @@ -- page_title "Variables" - -.row.prepend-top-default.append-bottom-default - .col-lg-3 - = render "content" - .col-lg-9 - %h5.prepend-top-0 - Update variable - = render "form", btn_text: "Save variable" += render 'ci/variables/show' diff --git a/changelogs/unreleased/feature-intermediate-12729-group-secret-variables.yml b/changelogs/unreleased/feature-intermediate-12729-group-secret-variables.yml new file mode 100644 index 00000000000..333895ffba9 --- /dev/null +++ b/changelogs/unreleased/feature-intermediate-12729-group-secret-variables.yml @@ -0,0 +1,4 @@ +--- +title: Add Group secret variables +merge_request: 12582 +author: diff --git a/config/routes/group.rb b/config/routes/group.rb index 11cdff55ed8..bd04ad51076 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -23,6 +23,12 @@ scope(path: 'groups/*group_id', resources :labels, except: [:show] do post :toggle_subscription, on: :member end + + namespace :settings do + resource :ci_cd, only: [:show], controller: 'ci_cd' + end + + resources :variables, only: [:index, :show, :update, :create, :destroy] end scope(path: 'groups/*id', diff --git a/db/migrate/20170525130346_create_group_variables_table.rb b/db/migrate/20170525130346_create_group_variables_table.rb new file mode 100644 index 00000000000..eaa38dbc40d --- /dev/null +++ b/db/migrate/20170525130346_create_group_variables_table.rb @@ -0,0 +1,23 @@ +class CreateGroupVariablesTable < ActiveRecord::Migration + DOWNTIME = false + + def up + create_table :ci_group_variables do |t| + t.string :key, null: false + t.text :value + t.text :encrypted_value + t.string :encrypted_value_salt + t.string :encrypted_value_iv + t.integer :group_id, null: false + t.boolean :protected, default: false, null: false + + t.timestamps_with_timezone null: false + end + + add_index :ci_group_variables, [:group_id, :key], unique: true + end + + def down + drop_table :ci_group_variables + end +end diff --git a/db/migrate/20170525130758_add_foreign_key_to_group_variables.rb b/db/migrate/20170525130758_add_foreign_key_to_group_variables.rb new file mode 100644 index 00000000000..0146235c5ba --- /dev/null +++ b/db/migrate/20170525130758_add_foreign_key_to_group_variables.rb @@ -0,0 +1,15 @@ +class AddForeignKeyToGroupVariables < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :ci_group_variables, :namespaces, column: :group_id + end + + def down + remove_foreign_key :ci_group_variables, column: :group_id + end +end diff --git a/db/schema.rb b/db/schema.rb index a47a6ae9a98..f4d83a4dd9c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -253,6 +253,20 @@ ActiveRecord::Schema.define(version: 20170703102400) do add_index "ci_builds", ["updated_at"], name: "index_ci_builds_on_updated_at", using: :btree add_index "ci_builds", ["user_id"], name: "index_ci_builds_on_user_id", using: :btree + create_table "ci_group_variables", force: :cascade do |t| + t.string "key", null: false + t.text "value" + t.text "encrypted_value" + t.string "encrypted_value_salt" + t.string "encrypted_value_iv" + t.integer "group_id", null: false + t.boolean "protected", default: false, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + + add_index "ci_group_variables", ["group_id", "key"], name: "index_ci_group_variables_on_group_id_and_key", unique: true, using: :btree + create_table "ci_pipeline_schedules", force: :cascade do |t| t.string "description" t.string "ref" @@ -1550,6 +1564,7 @@ ActiveRecord::Schema.define(version: 20170703102400) do add_foreign_key "ci_builds", "ci_pipelines", column: "auto_canceled_by_id", name: "fk_a2141b1522", on_delete: :nullify add_foreign_key "ci_builds", "ci_stages", column: "stage_id", name: "fk_3a9eaa254d", on_delete: :cascade add_foreign_key "ci_builds", "projects", name: "fk_befce0568a", on_delete: :cascade + add_foreign_key "ci_group_variables", "namespaces", column: "group_id", name: "fk_33ae4d58d8", on_delete: :cascade add_foreign_key "ci_pipeline_schedules", "projects", name: "fk_8ead60fcc4", on_delete: :cascade add_foreign_key "ci_pipeline_schedules", "users", column: "owner_id", name: "fk_9ea99f58d2", on_delete: :nullify add_foreign_key "ci_pipelines", "ci_pipeline_schedules", column: "pipeline_schedule_id", name: "fk_3d34ab2e06", on_delete: :nullify diff --git a/doc/ci/variables/README.md b/doc/ci/variables/README.md index 3501aae75ec..ee3349d9526 100644 --- a/doc/ci/variables/README.md +++ b/doc/ci/variables/README.md @@ -10,7 +10,8 @@ The variables can be overwritten and they take precedence over each other in this order: 1. [Trigger variables][triggers] (take precedence over all) -1. [Secret variables](#secret-variables) or [protected secret variables](#protected-secret-variables) +1. [Project-level secret variables](#project-level-secret-variables) +1. [Group-level secret variables](#group-level-secret-variables) 1. YAML-defined [job-level variables](../yaml/README.md#job-variables) 1. YAML-defined [global variables](../yaml/README.md#variables) 1. [Deployment variables](#deployment-variables) @@ -138,7 +139,7 @@ script: - 'eval $LS_CMD' # will execute 'ls -al $TMP_DIR' ``` -## Secret variables +## Project-level secret variables >**Notes:** - This feature requires GitLab Runner 0.4.0 or higher. @@ -158,7 +159,8 @@ Secret variables can be added by going to your project's **Settings ➔ Pipelines**, then finding the section called **Secret variables**. -Once you set them, they will be available for all subsequent pipelines. +Once you set them, they will be available for all subsequent pipelines. You can also +[protect your variables](#protected-secret-variables). ### Protected secret variables @@ -176,6 +178,19 @@ Protected variables can be added by going to your project's Once you set them, they will be available for all subsequent pipelines. +## Group-level secret variables + +>**Notes:** +This feature requires GitLab 9.4 or higher. + +You can also define variables per Group. The essential functionality is exactly the +same with [project-level secret variables](#project-level-secret-variables). You +can also [protect your variables](#protected-secret-variables). + +Secret variables can be added by going to your group's +**Settings ➔ Pipelines**, then finding the section called +**Secret variables**. + ## Deployment variables >**Note:** diff --git a/lib/gitlab/path_regex.rb b/lib/gitlab/path_regex.rb index 10eb99fb461..f0d5e3f7bc0 100644 --- a/lib/gitlab/path_regex.rb +++ b/lib/gitlab/path_regex.rb @@ -129,6 +129,8 @@ module Gitlab pipeline_quota projects subgroups + settings + variables ].freeze ILLEGAL_PROJECT_PATH_WORDS = PROJECT_WILDCARD_ROUTES diff --git a/spec/controllers/groups/settings/ci_cd_controller_spec.rb b/spec/controllers/groups/settings/ci_cd_controller_spec.rb new file mode 100644 index 00000000000..2e0efb57c74 --- /dev/null +++ b/spec/controllers/groups/settings/ci_cd_controller_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper' + +describe Groups::Settings::CiCdController do + let(:group) { create(:group) } + let(:user) { create(:user) } + + before do + group.add_master(user) + sign_in(user) + end + + describe 'GET #show' do + it 'renders show with 200 status code' do + get :show, group_id: group + + expect(response).to have_http_status(200) + expect(response).to render_template(:show) + end + end +end diff --git a/spec/controllers/groups/variables_controller_spec.rb b/spec/controllers/groups/variables_controller_spec.rb new file mode 100644 index 00000000000..c11fe93ffca --- /dev/null +++ b/spec/controllers/groups/variables_controller_spec.rb @@ -0,0 +1,56 @@ +require 'spec_helper' + +describe Groups::VariablesController do + let(:group) { create(:group) } + let(:user) { create(:user) } + + before do + sign_in(user) + group.add_master(user) + end + + describe 'POST #create' do + context 'variable is valid' do + it 'shows a success flash message' do + post :create, group_id: group, variable: { key: "one", value: "two" } + + expect(flash[:notice]).to include 'Variables were successfully updated.' + expect(response).to redirect_to(group_settings_ci_cd_path(group)) + end + end + + context 'variable is invalid' do + it 'renders show' do + post :create, group_id: group, variable: { key: "..one", value: "two" } + + expect(response).to render_template("groups/variables/show") + end + end + end + + describe 'POST #update' do + let(:variable) { create(:ci_group_variable) } + + context 'updating a variable with valid characters' do + before do + group.variables << variable + end + + it 'shows a success flash message' do + post :update, group_id: group, + id: variable.id, variable: { key: variable.key, value: 'two' } + + expect(flash[:notice]).to include 'Variable was successfully updated.' + expect(response).to redirect_to(group_variables_path(group)) + end + + it 'renders the action #show if the variable key is invalid' do + post :update, group_id: group, + id: variable.id, variable: { key: '?', value: variable.value } + + expect(response).to have_http_status(200) + expect(response).to render_template :show + end + end + end +end diff --git a/spec/controllers/projects/variables_controller_spec.rb b/spec/controllers/projects/variables_controller_spec.rb index a0ecc756653..0304ea6f93c 100644 --- a/spec/controllers/projects/variables_controller_spec.rb +++ b/spec/controllers/projects/variables_controller_spec.rb @@ -21,7 +21,7 @@ describe Projects::VariablesController do end context 'variable is invalid' do - it 'shows an alert flash message' do + it 'renders show' do post :create, namespace_id: project.namespace.to_param, project_id: project, variable: { key: "..one", value: "two" } @@ -35,7 +35,6 @@ describe Projects::VariablesController do context 'updating a variable with valid characters' do before do - variable.project_id = project.id project.variables << variable end diff --git a/spec/factories/ci/group_variables.rb b/spec/factories/ci/group_variables.rb new file mode 100644 index 00000000000..565ced9eb1a --- /dev/null +++ b/spec/factories/ci/group_variables.rb @@ -0,0 +1,12 @@ +FactoryGirl.define do + factory :ci_group_variable, class: Ci::GroupVariable do + sequence(:key) { |n| "VARIABLE_#{n}" } + value 'VARIABLE_VALUE' + + trait(:protected) do + protected true + end + + group factory: :group + end +end diff --git a/spec/features/group_variables_spec.rb b/spec/features/group_variables_spec.rb new file mode 100644 index 00000000000..37814ba6238 --- /dev/null +++ b/spec/features/group_variables_spec.rb @@ -0,0 +1,78 @@ +require 'spec_helper' + +feature 'Group variables', js: true do + let(:user) { create(:user) } + let(:group) { create(:group) } + + background do + group.add_master(user) + gitlab_sign_in(user) + end + + context 'when user creates a new variable' do + background do + visit group_settings_ci_cd_path(group) + fill_in 'variable_key', with: 'AAA' + fill_in 'variable_value', with: 'AAA123' + find(:css, "#variable_protected").set(true) + click_on 'Add new variable' + end + + scenario 'user sees the created variable' do + page.within('.variables-table') do + expect(find(".variable-key")).to have_content('AAA') + expect(find(".variable-value")).to have_content('******') + expect(find(".variable-protected")).to have_content('Yes') + end + click_on 'Reveal Values' + page.within('.variables-table') do + expect(find(".variable-value")).to have_content('AAA123') + end + end + end + + context 'when user edits a variable' do + background do + create(:ci_group_variable, key: 'AAA', value: 'AAA123', protected: true, + group: group) + + visit group_settings_ci_cd_path(group) + + page.within('.variable-menu') do + click_on 'Update' + end + + fill_in 'variable_key', with: 'BBB' + fill_in 'variable_value', with: 'BBB123' + find(:css, "#variable_protected").set(false) + click_on 'Save variable' + end + + scenario 'user sees the updated variable' do + page.within('.variables-table') do + expect(find(".variable-key")).to have_content('BBB') + expect(find(".variable-value")).to have_content('******') + expect(find(".variable-protected")).to have_content('No') + end + end + end + + context 'when user deletes a variable' do + background do + create(:ci_group_variable, key: 'BBB', value: 'BBB123', protected: false, + group: group) + + visit group_settings_ci_cd_path(group) + + page.within('.variable-menu') do + page.accept_alert 'Are you sure?' do + click_on 'Remove' + end + end + end + + scenario 'user does not see the deleted variable' do + expect(page).to have_no_css('.variables-table') + end + end +end diff --git a/spec/features/variables_spec.rb b/spec/features/variables_spec.rb index 1a2dedf27eb..a7c24c7d718 100644 --- a/spec/features/variables_spec.rb +++ b/spec/features/variables_spec.rb @@ -82,7 +82,7 @@ describe 'Project variables', js: true do it 'deletes variable' do page.within('.variables-table') do - find('.btn-variable-delete').click + click_on 'Remove' end expect(page).not_to have_selector('variables-table') @@ -90,7 +90,7 @@ describe 'Project variables', js: true do it 'edits variable' do page.within('.variables-table') do - find('.btn-variable-edit').click + click_on 'Update' end expect(page).to have_content('Update variable') @@ -104,7 +104,7 @@ describe 'Project variables', js: true do it 'edits variable with empty value' do page.within('.variables-table') do - find('.btn-variable-edit').click + click_on 'Update' end expect(page).to have_content('Update variable') @@ -117,7 +117,7 @@ describe 'Project variables', js: true do it 'edits variable to be protected' do page.within('.variables-table') do - find('.btn-variable-edit').click + click_on 'Update' end expect(page).to have_content('Update variable') @@ -132,7 +132,7 @@ describe 'Project variables', js: true do project.variables.first.update(protected: true) page.within('.variables-table') do - find('.btn-variable-edit').click + click_on 'Update' end expect(page).to have_content('Update variable') diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 431fcda165d..cf6d356c524 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1356,6 +1356,59 @@ describe Ci::Build, :models do end end + context 'when group secret variable is defined' do + let(:secret_variable) do + { key: 'SECRET_KEY', value: 'secret_value', public: false } + end + + let(:group) { create(:group, :access_requestable) } + + before do + build.project.update(group: group) + + create(:ci_group_variable, + secret_variable.slice(:key, :value).merge(group: group)) + end + + it { is_expected.to include(secret_variable) } + end + + context 'when group protected variable is defined' do + let(:protected_variable) do + { key: 'PROTECTED_KEY', value: 'protected_value', public: false } + end + + let(:group) { create(:group, :access_requestable) } + + before do + build.project.update(group: group) + + create(:ci_group_variable, + :protected, + protected_variable.slice(:key, :value).merge(group: group)) + end + + context 'when the branch is protected' do + before do + create(:protected_branch, project: build.project, name: build.ref) + end + + it { is_expected.to include(protected_variable) } + end + + context 'when the tag is protected' do + before do + create(:protected_tag, project: build.project, name: build.ref) + end + + it { is_expected.to include(protected_variable) } + end + + context 'when the ref is not protected' do + it { is_expected.not_to include(protected_variable) } + end + end + context 'when build is for triggers' do let(:trigger) { create(:ci_trigger, project: project) } let(:trigger_request) { create(:ci_trigger_request_with_variables, pipeline: pipeline, trigger: trigger) } diff --git a/spec/models/ci/group_variable_spec.rb b/spec/models/ci/group_variable_spec.rb new file mode 100644 index 00000000000..24b914face9 --- /dev/null +++ b/spec/models/ci/group_variable_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe Ci::GroupVariable, models: true do + subject { build(:ci_group_variable) } + + it { is_expected.to include_module(HasVariable) } + it { is_expected.to include_module(Presentable) } + it { is_expected.to validate_uniqueness_of(:key).scoped_to(:group_id) } + + describe '.unprotected' do + subject { described_class.unprotected } + + context 'when variable is protected' do + before do + create(:ci_group_variable, :protected) + end + + it 'returns nothing' do + is_expected.to be_empty + end + end + + context 'when variable is not protected' do + let(:variable) { create(:ci_group_variable, protected: false) } + + it 'returns the variable' do + is_expected.to contain_exactly(variable) + end + end + end +end diff --git a/spec/models/ci/variable_spec.rb b/spec/models/ci/variable_spec.rb index 4ffbfa6c130..890ffaae494 100644 --- a/spec/models/ci/variable_spec.rb +++ b/spec/models/ci/variable_spec.rb @@ -3,10 +3,9 @@ require 'spec_helper' describe Ci::Variable, models: true do subject { build(:ci_variable) } - let(:secret_value) { 'secret' } - describe 'validations' do it { is_expected.to include_module(HasVariable) } + it { is_expected.to include_module(Presentable) } it { is_expected.to validate_uniqueness_of(:key).scoped_to(:project_id, :environment_scope) } end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 4de1683b21c..dab33aa4418 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -13,6 +13,7 @@ describe Group, models: true do it { is_expected.to have_many(:shared_projects).through(:project_group_links) } it { is_expected.to have_many(:notification_settings).dependent(:destroy) } it { is_expected.to have_many(:labels).class_name('GroupLabel') } + it { is_expected.to have_many(:variables).class_name('Ci::GroupVariable') } it { is_expected.to have_many(:uploads).dependent(:destroy) } it { is_expected.to have_one(:chat_team) } @@ -418,4 +419,62 @@ describe Group, models: true do expect(calls).to eq 2 end end + + describe '#secret_variables_for' do + let(:project) { create(:empty_project, group: group) } + + let!(:secret_variable) do + create(:ci_group_variable, value: 'secret', group: group) + end + + let!(:protected_variable) do + create(:ci_group_variable, :protected, value: 'protected', group: group) + end + + subject { group.secret_variables_for('ref', project) } + + shared_examples 'ref is protected' do + it 'contains all the variables' do + is_expected.to contain_exactly(secret_variable, protected_variable) + end + end + + context 'when the ref is not protected' do + before do + stub_application_setting( + default_branch_protection: Gitlab::Access::PROTECTION_NONE) + end + + it 'contains only the secret variables' do + is_expected.to contain_exactly(secret_variable) + end + end + + context 'when the ref is a protected branch' do + before do + create(:protected_branch, name: 'ref', project: project) + end + + it_behaves_like 'ref is protected' + end + + context 'when the ref is a protected tag' do + before do + create(:protected_tag, name: 'ref', project: project) + end + + it_behaves_like 'ref is protected' + end + + context 'when group has a child' do + let!(:group_child) { create(:group, :access_requestable, parent: group) } + let!(:variable_child) { create(:ci_group_variable, group: group_child) } + + subject { group_child.secret_variables_for('ref', project) } + + it 'returns all variables belong to the group and parent groups' do + is_expected.to contain_exactly(secret_variable, protected_variable, variable_child) + end + end + end end diff --git a/spec/presenters/ci/group_variable_presenter_spec.rb b/spec/presenters/ci/group_variable_presenter_spec.rb new file mode 100644 index 00000000000..d404028405b --- /dev/null +++ b/spec/presenters/ci/group_variable_presenter_spec.rb @@ -0,0 +1,63 @@ +require 'spec_helper' + +describe Ci::GroupVariablePresenter do + include Gitlab::Routing.url_helpers + + let(:group) { create(:group) } + let(:variable) { create(:ci_group_variable, group: group) } + + subject(:presenter) do + described_class.new(variable) + end + + it 'inherits from Gitlab::View::Presenter::Delegated' do + expect(described_class.superclass).to eq(Gitlab::View::Presenter::Delegated) + end + + describe '#initialize' do + it 'takes a variable and optional params' do + expect { presenter }.not_to raise_error + end + + it 'exposes variable' do + expect(presenter.variable).to eq(variable) + end + + it 'forwards missing methods to variable' do + expect(presenter.key).to eq(variable.key) + end + end + + describe '#placeholder' do + subject { described_class.new(variable).placeholder } + + it { is_expected.to eq('GROUP_VARIABLE') } + end + + describe '#form_path' do + context 'when variable is persisted' do + subject { described_class.new(variable).form_path } + + it { is_expected.to eq(group_variable_path(group, variable)) } + end + + context 'when variable is not persisted' do + let(:variable) { build(:ci_group_variable, group: group) } + subject { described_class.new(variable).form_path } + + it { is_expected.to eq(group_variables_path(group)) } + end + end + + describe '#edit_path' do + subject { described_class.new(variable).edit_path } + + it { is_expected.to eq(group_variable_path(group, variable)) } + end + + describe '#delete_path' do + subject { described_class.new(variable).delete_path } + + it { is_expected.to eq(group_variable_path(group, variable)) } + end +end diff --git a/spec/presenters/ci/variable_presenter_spec.rb b/spec/presenters/ci/variable_presenter_spec.rb new file mode 100644 index 00000000000..3b615c4bf36 --- /dev/null +++ b/spec/presenters/ci/variable_presenter_spec.rb @@ -0,0 +1,63 @@ +require 'spec_helper' + +describe Ci::VariablePresenter do + include Gitlab::Routing.url_helpers + + let(:project) { create(:empty_project) } + let(:variable) { create(:ci_variable, project: project) } + + subject(:presenter) do + described_class.new(variable) + end + + it 'inherits from Gitlab::View::Presenter::Delegated' do + expect(described_class.superclass).to eq(Gitlab::View::Presenter::Delegated) + end + + describe '#initialize' do + it 'takes a variable and optional params' do + expect { presenter }.not_to raise_error + end + + it 'exposes variable' do + expect(presenter.variable).to eq(variable) + end + + it 'forwards missing methods to variable' do + expect(presenter.key).to eq(variable.key) + end + end + + describe '#placeholder' do + subject { described_class.new(variable).placeholder } + + it { is_expected.to eq('PROJECT_VARIABLE') } + end + + describe '#form_path' do + context 'when variable is persisted' do + subject { described_class.new(variable).form_path } + + it { is_expected.to eq(namespace_project_variable_path(project.namespace, project, variable)) } + end + + context 'when variable is not persisted' do + let(:variable) { build(:ci_variable, project: project) } + subject { described_class.new(variable).form_path } + + it { is_expected.to eq(namespace_project_variables_path(project.namespace, project)) } + end + end + + describe '#edit_path' do + subject { described_class.new(variable).edit_path } + + it { is_expected.to eq(namespace_project_variable_path(project.namespace, project, variable)) } + end + + describe '#delete_path' do + subject { described_class.new(variable).delete_path } + + it { is_expected.to eq(namespace_project_variable_path(project.namespace, project, variable)) } + end +end -- cgit v1.2.1 From d633bf0afbcd18e8762eef58d12ec3a2bbb1a7bd Mon Sep 17 00:00:00 2001 From: Achilleas Pipinellis Date: Mon, 3 Jul 2017 16:43:06 +0200 Subject: Copyedit docs for group-level secret variables [ci skip] --- doc/ci/variables/README.md | 40 ++++++++++++++++------------------------ 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/doc/ci/variables/README.md b/doc/ci/variables/README.md index ee3349d9526..548716448e6 100644 --- a/doc/ci/variables/README.md +++ b/doc/ci/variables/README.md @@ -10,8 +10,8 @@ The variables can be overwritten and they take precedence over each other in this order: 1. [Trigger variables][triggers] (take precedence over all) -1. [Project-level secret variables](#project-level-secret-variables) -1. [Group-level secret variables](#group-level-secret-variables) +1. Project-level [secret variables](#secret-variables) or [protected secret variables](#protected-secret-variables) +1. Group-level [secret variables](#secret-variables) or [protected secret variables](#protected-secret-variables) 1. YAML-defined [job-level variables](../yaml/README.md#job-variables) 1. YAML-defined [global variables](../yaml/README.md#variables) 1. [Deployment variables](#deployment-variables) @@ -139,25 +139,29 @@ script: - 'eval $LS_CMD' # will execute 'ls -al $TMP_DIR' ``` -## Project-level secret variables +## Secret variables >**Notes:** - This feature requires GitLab Runner 0.4.0 or higher. +- Group-level secret variables added in GitLab 9.4. - Be aware that secret variables are not masked, and their values can be shown in the job logs if explicitly asked to do so. If your project is public or internal, you can set the pipelines private from your project's Pipelines settings. Follow the discussion in issue [#13784][ce-13784] for masking the secret variables. -GitLab CI allows you to define per-project **secret variables** that are set in -the build environment. The secret variables are stored out of the repository -(`.gitlab-ci.yml`) and are securely passed to GitLab Runner making them -available in the build environment. It's the recommended method to use for -storing things like passwords, secret keys and credentials. +GitLab CI allows you to define per-project or per-group **secret variables** +that are set in the build environment. The secret variables are stored out of +the repository (`.gitlab-ci.yml`) and are securely passed to GitLab Runner +making them available in the build environment. It's the recommended method to +use for storing things like passwords, secret keys and credentials. -Secret variables can be added by going to your project's -**Settings ➔ Pipelines**, then finding the section called -**Secret variables**. +Project-level secret variables can be added by going to your project's +**Settings ➔ Pipelines**, then finding the section called **Secret variables**. + +Likewise, group-level secret variables can be added by going to your group's +**Settings ➔ Pipelines**, then finding the section called **Secret variables**. +Any variables of [subgroups] will be inherited recursively. Once you set them, they will be available for all subsequent pipelines. You can also [protect your variables](#protected-secret-variables). @@ -178,19 +182,6 @@ Protected variables can be added by going to your project's Once you set them, they will be available for all subsequent pipelines. -## Group-level secret variables - ->**Notes:** -This feature requires GitLab 9.4 or higher. - -You can also define variables per Group. The essential functionality is exactly the -same with [project-level secret variables](#project-level-secret-variables). You -can also [protect your variables](#protected-secret-variables). - -Secret variables can be added by going to your group's -**Settings ➔ Pipelines**, then finding the section called -**Secret variables**. - ## Deployment variables >**Note:** @@ -449,3 +440,4 @@ export CI_REGISTRY_PASSWORD="longalfanumstring" [shellexecutors]: https://docs.gitlab.com/runner/executors/ [triggered]: ../triggers/README.md [triggers]: ../triggers/README.md#pass-job-variables-to-a-trigger +[subgroups]: ../../user/group/subgroups/index.md -- cgit v1.2.1 From f8a2f6f11598b6565ba2a0bb143f7d1e0f07f610 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 3 Jul 2017 23:46:08 +0900 Subject: Wrap additional routes by dash(-). And remove those routes from path_regex.rb. --- config/routes/group.rb | 10 ++++++---- lib/gitlab/path_regex.rb | 2 -- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/config/routes/group.rb b/config/routes/group.rb index bd04ad51076..e578dd8b082 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -24,11 +24,13 @@ scope(path: 'groups/*group_id', post :toggle_subscription, on: :member end - namespace :settings do - resource :ci_cd, only: [:show], controller: 'ci_cd' - end + scope path: '-' do + namespace :settings do + resource :ci_cd, only: [:show], controller: 'ci_cd' + end - resources :variables, only: [:index, :show, :update, :create, :destroy] + resources :variables, only: [:index, :show, :update, :create, :destroy] + end end scope(path: 'groups/*id', diff --git a/lib/gitlab/path_regex.rb b/lib/gitlab/path_regex.rb index f0d5e3f7bc0..10eb99fb461 100644 --- a/lib/gitlab/path_regex.rb +++ b/lib/gitlab/path_regex.rb @@ -129,8 +129,6 @@ module Gitlab pipeline_quota projects subgroups - settings - variables ].freeze ILLEGAL_PROJECT_PATH_WORDS = PROJECT_WILDCARD_ROUTES -- cgit v1.2.1 From d228662fb7cfdfaea749e4b733197e3d3a9ac23b Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Tue, 4 Jul 2017 01:08:05 +0900 Subject: Add dash for GROUP_ROUTES --- lib/gitlab/path_regex.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/gitlab/path_regex.rb b/lib/gitlab/path_regex.rb index 10eb99fb461..d81f825ef96 100644 --- a/lib/gitlab/path_regex.rb +++ b/lib/gitlab/path_regex.rb @@ -112,6 +112,7 @@ module Gitlab # this group would not be accessible through `/groups/parent/activity` since # this would map to the activity-page of its parent. GROUP_ROUTES = %w[ + - activity analytics audit_events -- cgit v1.2.1 From 2dd9a9af2f0033be8fb627e2113710505874008b Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Tue, 4 Jul 2017 01:28:55 +0900 Subject: Fix variables_controller.rb and format --- app/controllers/groups/variables_controller.rb | 3 +-- app/controllers/projects/variables_controller.rb | 14 ++++++++------ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/app/controllers/groups/variables_controller.rb b/app/controllers/groups/variables_controller.rb index 6fba9dab2a7..beafea7b451 100644 --- a/app/controllers/groups/variables_controller.rb +++ b/app/controllers/groups/variables_controller.rb @@ -46,8 +46,7 @@ module Groups end def group_params - params.require(:variable) - .permit([:key, :value, :protected]) + params.require(:variable).permit([:key, :value, :protected]) end def variable diff --git a/app/controllers/projects/variables_controller.rb b/app/controllers/projects/variables_controller.rb index 716e1347604..dbd1e506002 100644 --- a/app/controllers/projects/variables_controller.rb +++ b/app/controllers/projects/variables_controller.rb @@ -13,18 +13,21 @@ class Projects::VariablesController < Projects::ApplicationController def update if @variable.update(project_params) - redirect_to namespace_project_variables_path(project.namespace, project), notice: 'Variable was successfully updated.' + redirect_to namespace_project_variables_path(project.namespace, project), + notice: 'Variable was successfully updated.' else render "show" end end def create - @variable = Ci::Variable.new(project_params) + new_variable = Ci::Variable.new(project_params) - if @variable.valid? && @project.variables << @variable - redirect_to namespace_project_settings_ci_cd_path(project.namespace, project), notice: 'Variables were successfully updated.' + if new_variable.valid? && @project.variables << new_variable + redirect_to namespace_project_settings_ci_cd_path(project.namespace, project), + notice: 'Variables were successfully updated.' else + @variable = new_variable.present(current_user: current_user) render "show" end end @@ -40,8 +43,7 @@ class Projects::VariablesController < Projects::ApplicationController private def project_params - params.require(:variable) - .permit([:id, :key, :value, :protected, :_destroy]) + params.require(:variable).permit([:id, :key, :value, :protected, :_destroy]) end def variable -- cgit v1.2.1 From e255e4de6943f419e4d1137104f990a120e3f72a Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Tue, 4 Jul 2017 22:27:42 +0900 Subject: ayufan nice catches --- app/controllers/groups/variables_controller.rb | 18 +++++++++++------- app/controllers/projects/variables_controller.rb | 18 +++++++++++------- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/app/controllers/groups/variables_controller.rb b/app/controllers/groups/variables_controller.rb index beafea7b451..ece2ba62e0a 100644 --- a/app/controllers/groups/variables_controller.rb +++ b/app/controllers/groups/variables_controller.rb @@ -20,9 +20,9 @@ module Groups end def create - new_variable = Ci::GroupVariable.new(group_params) + new_variable = group.variables.create(group_params) - if new_variable.valid? && group.variables << new_variable + if new_variable.persisted? redirect_to group_settings_ci_cd_path(group), notice: 'Variables were successfully updated.' else @@ -32,11 +32,15 @@ module Groups end def destroy - variable.destroy - - redirect_to group_settings_ci_cd_path(group), - status: 302, - notice: 'Variable was successfully removed.' + if variable.destroy + redirect_to group_settings_ci_cd_path(group), + status: 302, + notice: 'Variable was successfully removed.' + else + redirect_to group_settings_ci_cd_path(group), + status: 302, + notice: 'Failed to remove the variable' + end end private diff --git a/app/controllers/projects/variables_controller.rb b/app/controllers/projects/variables_controller.rb index dbd1e506002..176c0294ead 100644 --- a/app/controllers/projects/variables_controller.rb +++ b/app/controllers/projects/variables_controller.rb @@ -21,9 +21,9 @@ class Projects::VariablesController < Projects::ApplicationController end def create - new_variable = Ci::Variable.new(project_params) + new_variable = project.variables.create(project_params) - if new_variable.valid? && @project.variables << new_variable + if new_variable.persisted? redirect_to namespace_project_settings_ci_cd_path(project.namespace, project), notice: 'Variables were successfully updated.' else @@ -33,11 +33,15 @@ class Projects::VariablesController < Projects::ApplicationController end def destroy - variable.destroy - - redirect_to namespace_project_settings_ci_cd_path(project.namespace, project), - status: 302, - notice: 'Variable was successfully removed.' + if variable.destroy + redirect_to namespace_project_settings_ci_cd_path(project.namespace, project), + status: 302, + notice: 'Variable was successfully removed.' + else + redirect_to namespace_project_settings_ci_cd_path(project.namespace, project), + status: 302, + notice: 'Failed to remove the variable' + end end private -- cgit v1.2.1 From bd846f7d93d1c7fd1d7ffdf097be88cf4ddf6581 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 5 Jul 2017 02:27:06 +0900 Subject: Use ancestors for avoiding N queries --- app/models/group.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/app/models/group.rb b/app/models/group.rb index f625b8c250c..480b90b279e 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -250,10 +250,9 @@ class Group < Namespace end def secret_variables_for(ref, project) - variables = [] - variables += parent.secret_variables_for(ref, project) if has_parent? - variables += project.protected_for?(ref) ? self.variables : self.variables.unprotected - variables + list_of_ids = ([self] + ancestors).map { |l| l.id } + variables = Ci::GroupVariable.where("group_id IN (#{list_of_ids.join(", ")})") + project.protected_for?(ref) ? variables : variables.unprotected end protected -- cgit v1.2.1 From 5cb45b6a44a2cefff4f9cd7d5fd0b98b51416e94 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Wed, 5 Jul 2017 17:55:48 +0900 Subject: Add CASE When Clause for saving order when using where IN --- app/models/group.rb | 10 +++++++++- spec/models/group_spec.rb | 17 ++++++++++++----- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/app/models/group.rb b/app/models/group.rb index 480b90b279e..ece68cd3753 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -251,7 +251,15 @@ class Group < Namespace def secret_variables_for(ref, project) list_of_ids = ([self] + ancestors).map { |l| l.id } - variables = Ci::GroupVariable.where("group_id IN (#{list_of_ids.join(", ")})") + + order = list_of_ids.map.with_index do |id, index| + "WHEN #{id} THEN #{index}" + end.join("\n") + + variables = Ci::GroupVariable + .where("group_id IN (#{list_of_ids.join(", ")})") + .order("CASE group_id #{order} END DESC") + project.protected_for?(ref) ? variables : variables.unprotected end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index dab33aa4418..399020953e8 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -466,14 +466,21 @@ describe Group, models: true do it_behaves_like 'ref is protected' end - context 'when group has a child' do - let!(:group_child) { create(:group, :access_requestable, parent: group) } + context 'when group has children' do + let!(:group_child) { create(:group, parent: group) } let!(:variable_child) { create(:ci_group_variable, group: group_child) } - - subject { group_child.secret_variables_for('ref', project) } + let!(:group_child_3) { create(:group, parent: group_child_2) } + let!(:variable_child_3) { create(:ci_group_variable, group: group_child_3) } + let!(:group_child_2) { create(:group, parent: group_child) } + let!(:variable_child_2) { create(:ci_group_variable, group: group_child_2) } it 'returns all variables belong to the group and parent groups' do - is_expected.to contain_exactly(secret_variable, protected_variable, variable_child) + expected_array1 = [protected_variable, secret_variable] + expected_array2 = [variable_child, variable_child_2, variable_child_3] + got_array = group_child_3.secret_variables_for('ref', project).to_a + + expect(got_array.shift(2)).to contain_exactly(*expected_array1) + expect(got_array).to eq(expected_array2) end end end -- cgit v1.2.1 From 5c68fa66ccb17cb9a2c17574c3136c89b1a5d19f Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 6 Jul 2017 00:13:01 +0900 Subject: secret_variables_for: rails readability versino --- app/models/group.rb | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/app/models/group.rb b/app/models/group.rb index ece68cd3753..f29e642ac91 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -250,17 +250,11 @@ class Group < Namespace end def secret_variables_for(ref, project) - list_of_ids = ([self] + ancestors).map { |l| l.id } - - order = list_of_ids.map.with_index do |id, index| - "WHEN #{id} THEN #{index}" - end.join("\n") - - variables = Ci::GroupVariable - .where("group_id IN (#{list_of_ids.join(", ")})") - .order("CASE group_id #{order} END DESC") - - project.protected_for?(ref) ? variables : variables.unprotected + list_of_ids = [self] + ancestors + variables = Ci::GroupVariable.where(group: list_of_ids) + variables = variables.unprotected unless project.protected_for?(ref) + variables = variables.group_by(&:group_id) + list_of_ids.reverse.map { |group| variables[group.id] }.compact.flatten end protected -- cgit v1.2.1 From 61d5b13888e9eb83cd86ca9849034d320b94d2b7 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 6 Jul 2017 16:39:45 +0900 Subject: Adopt project_variable_path instead of namespace_project_variable_path. (Resolve confilct from master) --- app/controllers/projects/variables_controller.rb | 4 ++-- app/presenters/ci/variable_presenter.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/controllers/projects/variables_controller.rb b/app/controllers/projects/variables_controller.rb index 176c0294ead..04d7429d939 100644 --- a/app/controllers/projects/variables_controller.rb +++ b/app/controllers/projects/variables_controller.rb @@ -13,7 +13,7 @@ class Projects::VariablesController < Projects::ApplicationController def update if @variable.update(project_params) - redirect_to namespace_project_variables_path(project.namespace, project), + redirect_to project_variables_path(project), notice: 'Variable was successfully updated.' else render "show" @@ -24,7 +24,7 @@ class Projects::VariablesController < Projects::ApplicationController new_variable = project.variables.create(project_params) if new_variable.persisted? - redirect_to namespace_project_settings_ci_cd_path(project.namespace, project), + redirect_to project_settings_ci_cd_path(project), notice: 'Variables were successfully updated.' else @variable = new_variable.present(current_user: current_user) diff --git a/app/presenters/ci/variable_presenter.rb b/app/presenters/ci/variable_presenter.rb index 3f14d972c50..710604c653c 100644 --- a/app/presenters/ci/variable_presenter.rb +++ b/app/presenters/ci/variable_presenter.rb @@ -15,11 +15,11 @@ module Ci end def edit_path - namespace_project_variable_path(project.namespace, project, variable) + project_variable_path(project, variable) end def delete_path - namespace_project_variable_path(project.namespace, project, variable) + project_variable_path(project, variable) end end end -- cgit v1.2.1 From 8c434a52fcfc92ffbd8bf9aa5ee2893724d3c666 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 6 Jul 2017 20:10:07 +0900 Subject: gb nice catches --- app/controllers/groups/variables_controller.rb | 10 +++++----- app/controllers/projects/variables_controller.rb | 10 +++++----- spec/controllers/groups/variables_controller_spec.rb | 2 +- spec/controllers/projects/variables_controller_spec.rb | 2 +- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/app/controllers/groups/variables_controller.rb b/app/controllers/groups/variables_controller.rb index ece2ba62e0a..423f11e2234 100644 --- a/app/controllers/groups/variables_controller.rb +++ b/app/controllers/groups/variables_controller.rb @@ -20,13 +20,13 @@ module Groups end def create - new_variable = group.variables.create(group_params) + @variable = group.variables.create(group_params) + .present(current_user: current_user) - if new_variable.persisted? + if @variable.persisted? redirect_to group_settings_ci_cd_path(group), - notice: 'Variables were successfully updated.' + notice: 'Variable was successfully created.' else - @variable = new_variable.present(current_user: current_user) render "show" end end @@ -39,7 +39,7 @@ module Groups else redirect_to group_settings_ci_cd_path(group), status: 302, - notice: 'Failed to remove the variable' + notice: 'Failed to remove the variable.' end end diff --git a/app/controllers/projects/variables_controller.rb b/app/controllers/projects/variables_controller.rb index 04d7429d939..e8dcfa92348 100644 --- a/app/controllers/projects/variables_controller.rb +++ b/app/controllers/projects/variables_controller.rb @@ -21,13 +21,13 @@ class Projects::VariablesController < Projects::ApplicationController end def create - new_variable = project.variables.create(project_params) + @variable = project.variables.create(project_params) + .present(current_user: current_user) - if new_variable.persisted? + if @variable.persisted? redirect_to project_settings_ci_cd_path(project), - notice: 'Variables were successfully updated.' + notice: 'Variable was successfully created.' else - @variable = new_variable.present(current_user: current_user) render "show" end end @@ -40,7 +40,7 @@ class Projects::VariablesController < Projects::ApplicationController else redirect_to namespace_project_settings_ci_cd_path(project.namespace, project), status: 302, - notice: 'Failed to remove the variable' + notice: 'Failed to remove the variable.' end end diff --git a/spec/controllers/groups/variables_controller_spec.rb b/spec/controllers/groups/variables_controller_spec.rb index c11fe93ffca..02f2fa46047 100644 --- a/spec/controllers/groups/variables_controller_spec.rb +++ b/spec/controllers/groups/variables_controller_spec.rb @@ -14,7 +14,7 @@ describe Groups::VariablesController do it 'shows a success flash message' do post :create, group_id: group, variable: { key: "one", value: "two" } - expect(flash[:notice]).to include 'Variables were successfully updated.' + expect(flash[:notice]).to include 'Variable was successfully created.' expect(response).to redirect_to(group_settings_ci_cd_path(group)) end end diff --git a/spec/controllers/projects/variables_controller_spec.rb b/spec/controllers/projects/variables_controller_spec.rb index 0304ea6f93c..da06fcb7cfb 100644 --- a/spec/controllers/projects/variables_controller_spec.rb +++ b/spec/controllers/projects/variables_controller_spec.rb @@ -15,7 +15,7 @@ describe Projects::VariablesController do post :create, namespace_id: project.namespace.to_param, project_id: project, variable: { key: "one", value: "two" } - expect(flash[:notice]).to include 'Variables were successfully updated.' + expect(flash[:notice]).to include 'Variable was successfully created.' expect(response).to redirect_to(project_settings_ci_cd_path(project)) end end -- cgit v1.2.1 From b7d17aab66343d94e5aa9c1680d6bbf5fdfc173f Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 6 Jul 2017 20:18:11 +0900 Subject: Use new project_variables_path in this MR --- app/controllers/projects/variables_controller.rb | 6 +++--- app/presenters/ci/variable_presenter.rb | 4 ++-- spec/presenters/ci/variable_presenter_spec.rb | 8 ++++---- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/app/controllers/projects/variables_controller.rb b/app/controllers/projects/variables_controller.rb index e8dcfa92348..a7fd4c2657c 100644 --- a/app/controllers/projects/variables_controller.rb +++ b/app/controllers/projects/variables_controller.rb @@ -5,7 +5,7 @@ class Projects::VariablesController < Projects::ApplicationController layout 'project_settings' def index - redirect_to namespace_project_settings_ci_cd_path(@project.namespace, @project) + redirect_to project_settings_ci_cd_path(@project) end def show @@ -34,11 +34,11 @@ class Projects::VariablesController < Projects::ApplicationController def destroy if variable.destroy - redirect_to namespace_project_settings_ci_cd_path(project.namespace, project), + redirect_to project_settings_ci_cd_path(project), status: 302, notice: 'Variable was successfully removed.' else - redirect_to namespace_project_settings_ci_cd_path(project.namespace, project), + redirect_to project_settings_ci_cd_path(project), status: 302, notice: 'Failed to remove the variable.' end diff --git a/app/presenters/ci/variable_presenter.rb b/app/presenters/ci/variable_presenter.rb index 710604c653c..5d7998393a6 100644 --- a/app/presenters/ci/variable_presenter.rb +++ b/app/presenters/ci/variable_presenter.rb @@ -8,9 +8,9 @@ module Ci def form_path if variable.persisted? - namespace_project_variable_path(project.namespace, project, variable) + project_variable_path(project, variable) else - namespace_project_variables_path(project.namespace, project) + project_variables_path(project) end end diff --git a/spec/presenters/ci/variable_presenter_spec.rb b/spec/presenters/ci/variable_presenter_spec.rb index 3b615c4bf36..9e6aae7bcad 100644 --- a/spec/presenters/ci/variable_presenter_spec.rb +++ b/spec/presenters/ci/variable_presenter_spec.rb @@ -38,26 +38,26 @@ describe Ci::VariablePresenter do context 'when variable is persisted' do subject { described_class.new(variable).form_path } - it { is_expected.to eq(namespace_project_variable_path(project.namespace, project, variable)) } + it { is_expected.to eq(project_variable_path(project, variable)) } end context 'when variable is not persisted' do let(:variable) { build(:ci_variable, project: project) } subject { described_class.new(variable).form_path } - it { is_expected.to eq(namespace_project_variables_path(project.namespace, project)) } + it { is_expected.to eq(project_variables_path(project)) } end end describe '#edit_path' do subject { described_class.new(variable).edit_path } - it { is_expected.to eq(namespace_project_variable_path(project.namespace, project, variable)) } + it { is_expected.to eq(project_variable_path(project, variable)) } end describe '#delete_path' do subject { described_class.new(variable).delete_path } - it { is_expected.to eq(namespace_project_variable_path(project.namespace, project, variable)) } + it { is_expected.to eq(project_variable_path(project, variable)) } end end -- cgit v1.2.1 From 4fbfe475d88553eb44c4d51bb535794d19348c40 Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Thu, 6 Jul 2017 21:40:27 +0900 Subject: Fix to Variable was successfully created --- spec/features/variables_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/features/variables_spec.rb b/spec/features/variables_spec.rb index a7c24c7d718..7acf7a089af 100644 --- a/spec/features/variables_spec.rb +++ b/spec/features/variables_spec.rb @@ -24,7 +24,7 @@ describe 'Project variables', js: true do fill_in('variable_value', with: 'key value') click_button('Add new variable') - expect(page).to have_content('Variables were successfully updated.') + expect(page).to have_content('Variable was successfully created.') page.within('.variables-table') do expect(page).to have_content('key') expect(page).to have_content('No') @@ -36,7 +36,7 @@ describe 'Project variables', js: true do fill_in('variable_value', with: '') click_button('Add new variable') - expect(page).to have_content('Variables were successfully updated.') + expect(page).to have_content('Variable was successfully created.') page.within('.variables-table') do expect(page).to have_content('new_key') end @@ -48,7 +48,7 @@ describe 'Project variables', js: true do check('Protected') click_button('Add new variable') - expect(page).to have_content('Variables were successfully updated.') + expect(page).to have_content('Variable was successfully created.') page.within('.variables-table') do expect(page).to have_content('key') expect(page).to have_content('Yes') -- cgit v1.2.1 From 474d25e2e18b38f578ebce6f68009e5a154baadf Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 7 Jul 2017 15:46:23 +0900 Subject: Use variable_params && variable_params_attributes in project variables_controller.rb --- app/controllers/projects/variables_controller.rb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/app/controllers/projects/variables_controller.rb b/app/controllers/projects/variables_controller.rb index a7fd4c2657c..6a825137564 100644 --- a/app/controllers/projects/variables_controller.rb +++ b/app/controllers/projects/variables_controller.rb @@ -12,7 +12,7 @@ class Projects::VariablesController < Projects::ApplicationController end def update - if @variable.update(project_params) + if variable.update(variable_params) redirect_to project_variables_path(project), notice: 'Variable was successfully updated.' else @@ -21,7 +21,7 @@ class Projects::VariablesController < Projects::ApplicationController end def create - @variable = project.variables.create(project_params) + @variable = project.variables.create(variable_params) .present(current_user: current_user) if @variable.persisted? @@ -46,8 +46,12 @@ class Projects::VariablesController < Projects::ApplicationController private - def project_params - params.require(:variable).permit([:id, :key, :value, :protected, :_destroy]) + def variable_params + params.require(:variable).permit(*variable_params_attributes) + end + + def variable_params_attributes + %i[id key value protected _destroy] end def variable -- cgit v1.2.1 From 3b2f09289f64850586af2b2db54466fe230c907c Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Fri, 7 Jul 2017 17:14:29 +0900 Subject: Name as variable_params like project variable controller --- app/controllers/groups/variables_controller.rb | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/app/controllers/groups/variables_controller.rb b/app/controllers/groups/variables_controller.rb index 423f11e2234..10038ff3ad9 100644 --- a/app/controllers/groups/variables_controller.rb +++ b/app/controllers/groups/variables_controller.rb @@ -11,7 +11,7 @@ module Groups end def update - if variable.update(group_params) + if variable.update(variable_params) redirect_to group_variables_path(group), notice: 'Variable was successfully updated.' else @@ -20,7 +20,7 @@ module Groups end def create - @variable = group.variables.create(group_params) + @variable = group.variables.create(variable_params) .present(current_user: current_user) if @variable.persisted? @@ -45,16 +45,20 @@ module Groups private - def authorize_admin_build! - return render_404 unless can?(current_user, :admin_build, group) + def variable_params + params.require(:variable).permit(*variable_params_attributes) end - def group_params - params.require(:variable).permit([:key, :value, :protected]) + def variable_params_attributes + %i[key value protected] end def variable @variable ||= group.variables.find(params[:id]).present(current_user: current_user) end + + def authorize_admin_build! + return render_404 unless can?(current_user, :admin_build, group) + end end end -- cgit v1.2.1 From 58359f249f4a6e9d68736fb98b536a4703d0b919 Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Fri, 7 Jul 2017 10:45:33 +0200 Subject: Update VERSION to 9.4.0-pre. --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index d821c124047..be3d36737cc 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -9.3.0-pre +9.4.0-pre -- cgit v1.2.1 From 1f40dfaea0758f595cbd7e4a2a8030af0397738e Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Gray Date: Fri, 7 Jul 2017 09:18:05 +0000 Subject: Extend MR tabs a bit to cover up the avatar holder and collapse icon on scroll --- app/assets/stylesheets/pages/merge_requests.scss | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 59e0624d94e..7adf17dddb8 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -731,11 +731,11 @@ .merge-request-tabs-holder { top: $header-height; - z-index: 100; + z-index: 200; background-color: $white-light; border-bottom: 1px solid $border-color; - @media(min-width: $screen-sm-min) { + @media (min-width: $screen-sm-min) { position: sticky; position: -webkit-sticky; } @@ -770,6 +770,12 @@ max-width: $limited-layout-width; margin-left: auto; margin-right: auto; + + .inner-page-scroll-tabs { + background-color: $white-light; + margin-left: -$gl-padding; + padding-left: $gl-padding; + } } } -- cgit v1.2.1 From 78089d1153ce9a2e3d32cbcaafb8f9757c56a9d4 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Thu, 6 Jul 2017 14:45:29 +0200 Subject: Remove option to disable Gitaly completely --- changelogs/unreleased/gitaly-mandatory.yml | 4 +++ config/gitlab.yml.example | 4 --- config/initializers/1_settings.rb | 1 - config/initializers/8_gitaly.rb | 8 +++--- doc/README.md | 1 + doc/administration/gitaly/index.md | 36 ++++----------------------- doc/update/9.3-to-9.4.md | 2 ++ lib/gitlab/gitaly_client.rb | 6 +---- lib/gitlab/workhorse.rb | 40 ++++++++++++++---------------- spec/support/gitaly.rb | 10 +++----- spec/support/test_env.rb | 2 +- 11 files changed, 40 insertions(+), 74 deletions(-) create mode 100644 changelogs/unreleased/gitaly-mandatory.yml diff --git a/changelogs/unreleased/gitaly-mandatory.yml b/changelogs/unreleased/gitaly-mandatory.yml new file mode 100644 index 00000000000..c060e0add29 --- /dev/null +++ b/changelogs/unreleased/gitaly-mandatory.yml @@ -0,0 +1,4 @@ +--- +title: Remove option to disable Gitaly +merge_request: 12677 +author: diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 4b81fd90f59..856131a4b0d 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -450,10 +450,6 @@ production: &base # Gitaly settings gitaly: - # This setting controls whether GitLab uses Gitaly (new component - # introduced in 9.0). Eventually Gitaly use will become mandatory and - # this option will disappear. - enabled: true # Default Gitaly authentication token. Can be overriden per storage. Can # be left blank when Gitaly is running locally on a Unix socket, which # is the normal way to deploy Gitaly. diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index cb11d2c34f4..fa33e602e93 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -483,7 +483,6 @@ Settings.rack_attack.git_basic_auth['bantime'] ||= 1.hour # Gitaly # Settings['gitaly'] ||= Settingslogic.new({}) -Settings.gitaly['enabled'] = true if Settings.gitaly['enabled'].nil? # # Webpack settings diff --git a/config/initializers/8_gitaly.rb b/config/initializers/8_gitaly.rb index 31c7c91d78f..f4f116e67f7 100644 --- a/config/initializers/8_gitaly.rb +++ b/config/initializers/8_gitaly.rb @@ -1,8 +1,6 @@ require 'uri' -if Gitlab.config.gitaly.enabled || Rails.env.test? - Gitlab.config.repositories.storages.keys.each do |storage| - # Force validation of each address - Gitlab::GitalyClient.address(storage) - end +Gitlab.config.repositories.storages.keys.each do |storage| + # Force validation of each address + Gitlab::GitalyClient.address(storage) end diff --git a/doc/README.md b/doc/README.md index fa755852304..ebf1a0415d2 100644 --- a/doc/README.md +++ b/doc/README.md @@ -179,6 +179,7 @@ have access to GitLab administration tools and settings. ### Admin tools +- [Gitaly](administration/gitaly/index.md): Configuring Gitaly, GitLab's Git repository storage service - [Raketasks](raketasks/README.md): Backups, maintenance, automatic webhook setup and the importing of projects. - [Backup and restore](raketasks/backup_restore.md): Backup and restore your GitLab instance. - [Reply by email](administration/reply_by_email.md): Allow users to comment on issues and merge requests by replying to notification emails. diff --git a/doc/administration/gitaly/index.md b/doc/administration/gitaly/index.md index 005ef9388cb..5732b6a1ca4 100644 --- a/doc/administration/gitaly/index.md +++ b/doc/administration/gitaly/index.md @@ -2,8 +2,7 @@ [Gitaly](https://gitlab.com/gitlab-org/gitaly) (introduced in GitLab 9.0) is a service that provides high-level RPC access to Git -repositories. As of GitLab 9.3 it is still an optional component with -limited scope. +repositories. Gitaly is a mandatory component in GitLab 9.4 and newer. GitLab components that access Git repositories (gitlab-rails, gitlab-shell, gitlab-workhorse) act as clients to Gitaly. End users do @@ -177,36 +176,11 @@ Gitaly logs on your Gitaly server (`sudo gitlab-ctl tail gitaly` or coming in. One sure way to trigger a Gitaly request is to clone a repository from your GitLab server over HTTP. -## Configuring GitLab to not use Gitaly - -Gitaly is still an optional component in GitLab 9.3. This means you -can choose to not use it. - -In Omnibus you can make the following change in -`/etc/gitlab/gitlab.rb` and reconfigure. This will both disable the -Gitaly service and configure the rest of GitLab not to use it. - -```ruby -gitaly['enable'] = false -``` - -In source installations, edit `/home/git/gitlab/config/gitlab.yml` and -make sure `enabled` in the `gitaly` section is set to 'false'. This -does not disable the Gitaly service in your init script; it only -prevents it from being used. - -Apply the change with `service gitlab restart`. - -```yaml - gitaly: - enabled: false -``` - ## Disabling or enabling the Gitaly service -Be careful: if you disable Gitaly without instructing the rest of your -GitLab installation not to use Gitaly, you may end up with errors -because GitLab tries to access a service that is not running. +If you are running Gitaly [as a remote +service](#running-gitaly-on-its-own-server) you may want to disable +the local Gitaly service that runs on your Gitlab server by default. To disable the Gitaly service in your Omnibus installation, add the following line to `/etc/gitlab/gitlab.rb`: @@ -225,4 +199,4 @@ following to `/etc/default/gitlab`: gitaly_enabled=false ``` -When you run `service gitlab restart` Gitaly will be disabled. \ No newline at end of file +When you run `service gitlab restart` Gitaly will be disabled. diff --git a/doc/update/9.3-to-9.4.md b/doc/update/9.3-to-9.4.md index a712ce5a8b1..bbb7f4a8d48 100644 --- a/doc/update/9.3-to-9.4.md +++ b/doc/update/9.3-to-9.4.md @@ -148,6 +148,8 @@ sudo -u git -H make If you have not yet set up Gitaly then follow [Gitaly section of the installation guide](../install/installation.md#install-gitaly). +As of GitLab 9.4, Gitaly is a mandatory component of GitLab. + #### Check Gitaly configuration Due to a bug in the `rake gitlab:gitaly:install` script your Gitaly diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index f605c06dfc3..197a94487eb 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -70,12 +70,8 @@ module Gitlab params['gitaly_token'].presence || Gitlab.config.gitaly['token'] end - def self.enabled? - Gitlab.config.gitaly.enabled - end - def self.feature_enabled?(feature, status: MigrationStatus::OPT_IN) - return false if !enabled? || status == MigrationStatus::DISABLED + return false if status == MigrationStatus::DISABLED feature = Feature.get("gitaly_#{feature}") diff --git a/lib/gitlab/workhorse.rb b/lib/gitlab/workhorse.rb index f96ee69096d..4aef23b6aee 100644 --- a/lib/gitlab/workhorse.rb +++ b/lib/gitlab/workhorse.rb @@ -25,27 +25,25 @@ module Gitlab RepoPath: repo_path } - if Gitlab.config.gitaly.enabled - server = { - address: Gitlab::GitalyClient.address(project.repository_storage), - token: Gitlab::GitalyClient.token(project.repository_storage) - } - params[:Repository] = repository.gitaly_repository.to_h - - feature_enabled = case action.to_s - when 'git_receive_pack' - Gitlab::GitalyClient.feature_enabled?(:post_receive_pack) - when 'git_upload_pack' - Gitlab::GitalyClient.feature_enabled?(:post_upload_pack) - when 'info_refs' - true - else - raise "Unsupported action: #{action}" - end - if feature_enabled - params[:GitalyAddress] = server[:address] # This field will be deprecated - params[:GitalyServer] = server - end + server = { + address: Gitlab::GitalyClient.address(project.repository_storage), + token: Gitlab::GitalyClient.token(project.repository_storage) + } + params[:Repository] = repository.gitaly_repository.to_h + + feature_enabled = case action.to_s + when 'git_receive_pack' + Gitlab::GitalyClient.feature_enabled?(:post_receive_pack) + when 'git_upload_pack' + Gitlab::GitalyClient.feature_enabled?(:post_upload_pack) + when 'info_refs' + true + else + raise "Unsupported action: #{action}" + end + if feature_enabled + params[:GitalyAddress] = server[:address] # This field will be deprecated + params[:GitalyServer] = server end params diff --git a/spec/support/gitaly.rb b/spec/support/gitaly.rb index 2bf159002a0..89fb362cf14 100644 --- a/spec/support/gitaly.rb +++ b/spec/support/gitaly.rb @@ -1,8 +1,6 @@ -if Gitlab::GitalyClient.enabled? - RSpec.configure do |config| - config.before(:each) do |example| - next if example.metadata[:skip_gitaly_mock] - allow(Gitlab::GitalyClient).to receive(:feature_enabled?).and_return(true) - end +RSpec.configure do |config| + config.before(:each) do |example| + next if example.metadata[:skip_gitaly_mock] + allow(Gitlab::GitalyClient).to receive(:feature_enabled?).and_return(true) end end diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index 32546abcad4..0cae5620920 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -69,7 +69,7 @@ module TestEnv # Setup GitLab shell for test instance setup_gitlab_shell - setup_gitaly if Gitlab::GitalyClient.enabled? + setup_gitaly # Create repository for FactoryGirl.create(:project) setup_factory_repo -- cgit v1.2.1 From 38fd773bd3bb7ff479ca3d607da6966139e262e3 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Fri, 7 Jul 2017 10:57:34 +0100 Subject: Fix ShaAttribute concern when there is no table When this is added to a new model, it would fail before the migrations were run - including when trying to run migrations in production mode! --- app/models/concerns/sha_attribute.rb | 2 ++ spec/models/concerns/sha_attribute_spec.rb | 31 ++++++++++++++++++++++++------ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/app/models/concerns/sha_attribute.rb b/app/models/concerns/sha_attribute.rb index c28974a3cdf..67ecf470f7e 100644 --- a/app/models/concerns/sha_attribute.rb +++ b/app/models/concerns/sha_attribute.rb @@ -3,6 +3,8 @@ module ShaAttribute module ClassMethods def sha_attribute(name) + return unless table_exists? + column = columns.find { |c| c.name == name.to_s } # In case the table doesn't exist we won't be able to find the column, diff --git a/spec/models/concerns/sha_attribute_spec.rb b/spec/models/concerns/sha_attribute_spec.rb index 9e37c2b20c4..610793ee557 100644 --- a/spec/models/concerns/sha_attribute_spec.rb +++ b/spec/models/concerns/sha_attribute_spec.rb @@ -13,15 +13,34 @@ describe ShaAttribute do end describe '#sha_attribute' do - it 'defines a SHA attribute for a binary column' do - expect(model).to receive(:attribute) - .with(:sha1, an_instance_of(Gitlab::Database::ShaAttribute)) + context' when the table exists' do + before do + allow(model).to receive(:table_exists?).and_return(true) + end - model.sha_attribute(:sha1) + it 'defines a SHA attribute for a binary column' do + expect(model).to receive(:attribute) + .with(:sha1, an_instance_of(Gitlab::Database::ShaAttribute)) + + model.sha_attribute(:sha1) + end + + it 'raises ArgumentError when the column type is not :binary' do + expect { model.sha_attribute(:name) }.to raise_error(ArgumentError) + end end - it 'raises ArgumentError when the column type is not :binary' do - expect { model.sha_attribute(:name) }.to raise_error(ArgumentError) + context' when the table does not exist' do + before do + allow(model).to receive(:table_exists?).and_return(false) + end + + it 'does nothing' do + expect(model).not_to receive(:columns) + expect(model).not_to receive(:attribute) + + model.sha_attribute(:name) + end end end end -- cgit v1.2.1