diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index ffd8d494a401a508a5cca36ac2d5d56fae07f96b..81267c68cfc240d42789069f4ac5add96db8ae30 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 0000000000000000000000000000000000000000..0142ad8278c38ac82157874e7431b7c159cda8c3 --- /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 0000000000000000000000000000000000000000..10038ff3ad9b60084add00c752b818c1290e8467 --- /dev/null +++ b/app/controllers/groups/variables_controller.rb @@ -0,0 +1,64 @@ +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(variable_params) + redirect_to group_variables_path(group), + notice: 'Variable was successfully updated.' + else + render "show" + end + end + + def create + @variable = group.variables.create(variable_params) + .present(current_user: current_user) + + if @variable.persisted? + redirect_to group_settings_ci_cd_path(group), + notice: 'Variable was successfully created.' + else + render "show" + end + end + + def destroy + 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 + + def variable_params + params.require(:variable).permit(*variable_params_attributes) + end + + 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 diff --git a/app/controllers/projects/settings/ci_cd_controller.rb b/app/controllers/projects/settings/ci_cd_controller.rb index 24fe78bc1bd0d62bd062c35fed90a3e8b8d32a82..ea7ceb3eaa5e7977e66d3b1bd6eba2a4e8405596 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 326d31ecec201d66024ae15b1d347cfbede8933d..6a8251375649fe54e15891b52cd1c63316902b8d 100644 --- a/app/controllers/projects/variables_controller.rb +++ b/app/controllers/projects/variables_controller.rb @@ -1,4 +1,5 @@ class Projects::VariablesController < Projects::ApplicationController + before_action :variable, only: [:show, :update, :destroy] before_action :authorize_admin_build! layout 'project_settings' @@ -8,37 +9,39 @@ class Projects::VariablesController < Projects::ApplicationController 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(variable_params) + redirect_to project_variables_path(project), + notice: 'Variable was successfully updated.' else - render action: "show" + render "show" end end def create - @variable = @project.variables.new(variable_params) + @variable = project.variables.create(variable_params) + .present(current_user: current_user) - if @variable.save - flash[:notice] = 'Variables were successfully updated.' - redirect_to project_settings_ci_cd_path(project) + if @variable.persisted? + redirect_to project_settings_ci_cd_path(project), + notice: 'Variable was successfully created.' else render "show" end end def destroy - @key = @project.variables.find(params[:id]) - @key.destroy - - redirect_to project_settings_ci_cd_path(project), - status: 302, - notice: 'Variable was successfully removed.' + if variable.destroy + redirect_to project_settings_ci_cd_path(project), + status: 302, + notice: 'Variable was successfully removed.' + else + redirect_to project_settings_ci_cd_path(project), + status: 302, + notice: 'Failed to remove the variable.' + end end private @@ -50,4 +53,8 @@ class Projects::VariablesController < Projects::ApplicationController def variable_params_attributes %i[id key value protected _destroy] end + + 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 ba2ecbf82a228eed428239de69dd16e465abbe42..25e75a19f373eb5f0e4394d13aad5ccf4b5387e8 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 0000000000000000000000000000000000000000..f64bc245a67e26597d9f5aec1aa9e5374b1b3db2 --- /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 0b8d0ff881a2b252f4bdc888153fde376431b74e..cf0fe04ddafe643bda2f410b1232cadf9acd7bba 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 b93fce6100d1fc7530adab14a850dcb8fb201465..f29e642ac91018e182adfe3594cf5fbf63075c7c 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,14 @@ class Group < Namespace } end + def secret_variables_for(ref, project) + 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 def update_two_factor_requirement diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index dcb37416ca3756e799c11d0a3c860d01840e7a68..6defab75fce4bae0b51ec0fcd34ddefebb9857ca 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 0000000000000000000000000000000000000000..81fea106a5c8194a7b0e8eada3d955c25dc36407 --- /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 0000000000000000000000000000000000000000..5d7998393a6c1abc6a125d574f6866e0384725a5 --- /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? + project_variable_path(project, variable) + else + project_variables_path(project) + end + end + + def edit_path + project_variable_path(project, variable) + end + + def delete_path + project_variable_path(project, variable) + end + end +end diff --git a/app/views/projects/variables/_content.html.haml b/app/views/ci/variables/_content.html.haml similarity index 100% rename from app/views/projects/variables/_content.html.haml rename to app/views/ci/variables/_content.html.haml diff --git a/app/views/projects/variables/_form.html.haml b/app/views/ci/variables/_form.html.haml similarity index 68% rename from app/views/projects/variables/_form.html.haml rename to app/views/ci/variables/_form.html.haml index 0a70a301cb4ec3986f73b0f220a210866e9a51f6..eebd0955c801061d974cc38d95bc3374c4a05656 100644 --- a/app/views/projects/variables/_form.html.haml +++ b/app/views/ci/variables/_form.html.haml @@ -1,12 +1,12 @@ -= form_for [@project.namespace.becomes(Namespace), @project, @variable] do |f| += 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: "PROJECT_VARIABLE", required: true + = 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: "PROJECT_VARIABLE" + = f.text_area :value, class: "form-control", placeholder: @variable.placeholder .form-group .checkbox = f.label :protected do diff --git a/app/views/projects/variables/_index.html.haml b/app/views/ci/variables/_index.html.haml similarity index 60% rename from app/views/projects/variables/_index.html.haml rename to app/views/ci/variables/_index.html.haml index 5e6786f66985502d58d779d156acff3bd472f47b..007c2344b5acf673d4373b534df80ebdac5b9436 100644 --- a/app/views/projects/variables/_index.html.haml +++ b/app/views/ci/variables/_index.html.haml @@ -1,16 +1,16 @@ .row.prepend-top-default.append-bottom-default .col-lg-4 - = render "projects/variables/content" + = render "ci/variables/content" .col-lg-8 %h5.prepend-top-0 Add a variable - = render "projects/variables/form", btn_text: "Add new variable" + = render "ci/variables/form", btn_text: "Add new variable" %hr %h5.prepend-top-0 - Your variables (#{@project.variables.size}) - - if @project.variables.empty? + 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 "projects/variables/table" + = 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 0000000000000000000000000000000000000000..2bfb290629d9d64ce71d4fcfe4805bac412c4f0b --- /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/projects/variables/_table.html.haml b/app/views/ci/variables/_table.html.haml similarity index 65% rename from app/views/projects/variables/_table.html.haml rename to app/views/ci/variables/_table.html.haml index 4ce6a82881218cd0929d75a2a676ea7ab46fcf83..71a0b56c4f4cf612811d880b46bc84630070bbae 100644 --- a/app/views/projects/variables/_table.html.haml +++ b/app/views/ci/variables/_table.html.haml @@ -11,18 +11,18 @@ %th Protected %th %tbody - - @project.variables.order_key_asc.each do |variable| + - @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 project_variable_path(@project, variable), class: "btn btn-transparent btn-variable-edit" do + = link_to variable.edit_path, 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 + = 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 2454e7355a71d7218827703b3ab3c5803658f3f3..623d233a46a0b03edbb41ade80f5f4bf6abcff50 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 0000000000000000000000000000000000000000..bf36baf48abddf8ebcf60439bb5bdb3bb3370b99 --- /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 0000000000000000000000000000000000000000..df533952b76ac2624929392f6bdab78347ee0679 --- /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 00ccc3ec41ef455bfccc93cd46f93a165ffc91ce..6afb38c5709b1ea9b2fd9fbc35a52026adc8cac1 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/show.html.haml b/app/views/projects/variables/show.html.haml index 297a53ca98c679861d38e5ccae109b46d46818e0..df533952b76ac2624929392f6bdab78347ee0679 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 0000000000000000000000000000000000000000..333895ffba9f64bf1e64f488c6a27c80658c4333 --- /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 11cdff55ed89d9ddafb9bed5a0f7e4839527b2d3..e578dd8b08274b4178a3bf0c5e3b877490b4a2cd 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -23,6 +23,14 @@ scope(path: 'groups/*group_id', resources :labels, except: [:show] do post :toggle_subscription, on: :member end + + scope path: '-' do + namespace :settings do + resource :ci_cd, only: [:show], controller: 'ci_cd' + end + + resources :variables, only: [:index, :show, :update, :create, :destroy] + end 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 0000000000000000000000000000000000000000..eaa38dbc40d466a3a136e9cd8f116d9238bb54b5 --- /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 0000000000000000000000000000000000000000..0146235c5baa920bac304b2163f89e4b587042a3 --- /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 a47a6ae9a986ea375c054234df2c4863b0fe6584..f4d83a4dd9cab84ee294be937fb0305cceb67986 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 3501aae75ec1f8952cc5f3a82bd7726d4cb9989e..548716448e61e05b760dc66a7158167a0c886c01 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](#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) @@ -142,23 +143,28 @@ script: >**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**. -Once you set them, they will be available for all subsequent pipelines. +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). ### Protected secret variables @@ -434,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 diff --git a/lib/gitlab/path_regex.rb b/lib/gitlab/path_regex.rb index 10eb99fb46134e9658e2c848469a3e0cf3a9da01..d81f825ef9609d65cef4c586394f51f00e0a18d4 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 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 0000000000000000000000000000000000000000..2e0efb57c74428717ad9d525739e33048c2d7207 --- /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 0000000000000000000000000000000000000000..02f2fa4604741134eecc0785add9b3761612b7b1 --- /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 'Variable was successfully created.' + 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 a0ecc7566537a59db820586eaab62b9f2ea09ecc..da06fcb7cfbb652465698674da38e1a06fe82823 100644 --- a/spec/controllers/projects/variables_controller_spec.rb +++ b/spec/controllers/projects/variables_controller_spec.rb @@ -15,13 +15,13 @@ 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 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 0000000000000000000000000000000000000000..565ced9eb1a81c2ec9b3b156f0eb77ad5939839b --- /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 0000000000000000000000000000000000000000..37814ba623863e446dc54afa7831ff86e49357a5 --- /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 1a2dedf27eb63d9070ce9e07a94f8e6801b5924e..7acf7a089af6e6fb18be714fb29c8208a84c2ec8 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') @@ -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 431fcda165d2e76f26707f60e7e321189a85e413..cf6d356c524cc8bab24de9c3db6ab1a2c430e5f8 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 0000000000000000000000000000000000000000..24b914face94bcd81b9063d9ff4f119a2875af21 --- /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 4ffbfa6c130c0c8b0ecaa10ba79ba9c235246bb3..890ffaae49466efc3954c91ba71147fdf75eacec 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 4de1683b21c4538a9362a773042ee46524b7f43f..399020953e8e26c77d70521b00096620fef8d017 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,69 @@ 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 children' do + let!(:group_child) { create(:group, parent: group) } + let!(:variable_child) { create(:ci_group_variable, group: group_child) } + 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 + 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 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 0000000000000000000000000000000000000000..d404028405b293ba446a25d608bca3a87f731fb5 --- /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 0000000000000000000000000000000000000000..9e6aae7bcad9ed3968d5c342ccc439daeddc3fda --- /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(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(project_variables_path(project)) } + end + end + + describe '#edit_path' do + subject { described_class.new(variable).edit_path } + + 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(project_variable_path(project, variable)) } + end +end