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

22 files changed, 208 insertions, 40 deletions

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/projects/variables/_content.html.haml b/app/views/ci/variables/_content.html.haml
index 98f618ca3b8..98f618ca3b8 100644
--- a/app/views/projects/variables/_content.html.haml
+++ b/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
index 0a70a301cb4..eebd0955c80 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
index 5e6786f6698..007c2344b5a 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 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/projects/variables/_table.html.haml b/app/views/ci/variables/_table.html.haml
index 4ce6a828812..71a0b56c4f4 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 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/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'