Use Gitlab::PushOptions for `ci.skip` push option

Previously the raw push option Array was sent to Pipeline::Chain::Skip.

This commit updates this class (and the chain of classes that pass the
push option parameters from the API internal `post_receive` endpoint to
that class) to treat push options as a Hash of options parsed by
GitLab::PushOptions.

The GitLab::PushOptions class takes options like this:

    -o ci.skip -o merge_request.create -o merge_request.target=branch

and turns them into a Hash like this:

    {
      ci: {
        skip: true
      },
      merge_request: {
        create: true,
        target: 'branch'
      }
    }

This now how Pipeline::Chain::Skip is determining if the `ci.skip` push
option was used.

11 files changed, 29 insertions, 16 deletions

diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb
index 8973c5ffc9e..41dee4e5641 100644
--- a/app/services/ci/create_pipeline_service.rb
+++ b/app/services/ci/create_pipeline_service.rb
@@ -37,7 +37,7 @@ module Ci
         variables_attributes: params[:variables_attributes],
         project: project,
         current_user: current_user,
-        push_options: params[:push_options],
+        push_options: params[:push_options] || {},
         chat_data: params[:chat_data],
         **extra_options(options))
 
diff --git a/app/services/git/base_hooks_service.rb b/app/services/git/base_hooks_service.rb
index e0dccb8716a..fce4040e390 100644
--- a/app/services/git/base_hooks_service.rb
+++ b/app/services/git/base_hooks_service.rb
@@ -79,7 +79,7 @@ module Git
         limited_commits,
         event_message,
         commits_count: commits_count,
-        push_options: params[:push_options] || []
+        push_options: params[:push_options] || {}
       )
 
       # Dependent code may modify the push data, so return a duplicate each time
diff --git a/app/workers/post_receive.rb b/app/workers/post_receive.rb
index 396f44396a3..a5554f07699 100644
--- a/app/workers/post_receive.rb
+++ b/app/workers/post_receive.rb
@@ -3,7 +3,7 @@
 class PostReceive
   include ApplicationWorker
 
-  def perform(gl_repository, identifier, changes, push_options = [])
+  def perform(gl_repository, identifier, changes, push_options = {})
     project, repo_type = Gitlab::GlRepository.parse(gl_repository)
 
     if project.nil?
diff --git a/lib/api/internal.rb b/lib/api/internal.rb
index 75202fa953c..ee1fb465608 100644
--- a/lib/api/internal.rb
+++ b/lib/api/internal.rb
@@ -262,7 +262,7 @@ module API
         push_options = Gitlab::PushOptions.new(params[:push_options])
 
         PostReceive.perform_async(params[:gl_repository], params[:identifier],
-          params[:changes], params[:push_options].to_a)
+          params[:changes], push_options.as_json)
 
         if (mr_options = push_options.get(:merge_request))
           begin
diff --git a/lib/gitlab/ci/pipeline/chain/skip.rb b/lib/gitlab/ci/pipeline/chain/skip.rb
index 79bbcc1ed1e..7d6e0704d4a 100644
--- a/lib/gitlab/ci/pipeline/chain/skip.rb
+++ b/lib/gitlab/ci/pipeline/chain/skip.rb
@@ -8,7 +8,6 @@ module Gitlab
           include ::Gitlab::Utils::StrongMemoize
 
           SKIP_PATTERN = /\[(ci[ _-]skip|skip[ _-]ci)\]/i
-          SKIP_PUSH_OPTION = 'ci.skip'
 
           def perform!
             if skipped?
@@ -35,7 +34,8 @@ module Gitlab
           end
 
           def push_option_skips_ci?
-            !!(@command.push_options&.include?(SKIP_PUSH_OPTION))
+            @command.push_options.present? &&
+              @command.push_options.deep_symbolize_keys.dig(:ci, :skip).present?
           end
         end
       end
diff --git a/lib/gitlab/data_builder/push.rb b/lib/gitlab/data_builder/push.rb
index ea08b5f7eae..af385d7d4ca 100644
--- a/lib/gitlab/data_builder/push.rb
+++ b/lib/gitlab/data_builder/push.rb
@@ -32,10 +32,7 @@ module Gitlab
             }
           ],
           total_commits_count: 1,
-          push_options: [
-            "ci.skip",
-            "custom option"
-          ]
+          push_options: { ci: { skip: true } }
         }.freeze
 
       # Produce a hash of post-receive data
@@ -57,11 +54,11 @@ module Gitlab
       #   },
       #   commits: Array,
       #   total_commits_count: Fixnum,
-      #   push_options: Array
+      #   push_options: Hash
       # }
       #
       # rubocop:disable Metrics/ParameterLists
-      def build(project, user, oldrev, newrev, ref, commits = [], message = nil, commits_count: nil, push_options: [])
+      def build(project, user, oldrev, newrev, ref, commits = [], message = nil, commits_count: nil, push_options: {})
         commits = Array(commits)
 
         # Total commits count
diff --git a/lib/gitlab/git_post_receive.rb b/lib/gitlab/git_post_receive.rb
index 426436c2164..d98b85fecc4 100644
--- a/lib/gitlab/git_post_receive.rb
+++ b/lib/gitlab/git_post_receive.rb
@@ -5,7 +5,7 @@ module Gitlab
     include Gitlab::Identifier
     attr_reader :project, :identifier, :changes, :push_options
 
-    def initialize(project, identifier, changes, push_options)
+    def initialize(project, identifier, changes, push_options = {})
       @project = project
       @identifier = identifier
       @changes = deserialize_changes(changes)
diff --git a/lib/gitlab/push_options.rb b/lib/gitlab/push_options.rb
index 923aa09527d..7c655c12995 100644
--- a/lib/gitlab/push_options.rb
+++ b/lib/gitlab/push_options.rb
@@ -27,6 +27,11 @@ module Gitlab
       options.dig(*args)
     end
 
+    # Allow #to_json serialization
+    def as_json(*_args)
+      options
+    end
+
     private
 
     def parse_options(raw_options)
diff --git a/spec/lib/gitlab/push_options_spec.rb b/spec/lib/gitlab/push_options_spec.rb
index 68b64863c21..fc9e421bea6 100644
--- a/spec/lib/gitlab/push_options_spec.rb
+++ b/spec/lib/gitlab/push_options_spec.rb
@@ -39,6 +39,18 @@ describe Gitlab::PushOptions do
     end
   end
 
+  describe '#as_json' do
+    it 'returns all options' do
+      options = described_class.new(['merge_request.target=value'])
+
+      expect(options.as_json).to include(
+        merge_request: {
+          target: 'value'
+        }
+      )
+    end
+  end
+
   it 'can parse multiple push options' do
     options = described_class.new([
       'merge_request.create',
diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb
index 3cc0cceeab1..a5a6e08e73b 100644
--- a/spec/requests/api/internal_spec.rb
+++ b/spec/requests/api/internal_spec.rb
@@ -905,7 +905,7 @@ describe API::Internal do
 
     it 'enqueues a PostReceive worker job' do
       expect(PostReceive).to receive(:perform_async)
-        .with(gl_repository, identifier, changes, push_options)
+        .with(gl_repository, identifier, changes, { ci: { skip: true } })
 
       post api('/internal/post_receive'), params: valid_params
     end
diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb
index 866d709d446..101b91e9cd8 100644
--- a/spec/services/ci/create_pipeline_service_spec.rb
+++ b/spec/services/ci/create_pipeline_service_spec.rb
@@ -418,8 +418,7 @@ describe Ci::CreatePipelineService do
 
     context 'when push options contain ci.skip' do
       let(:push_options) do
-        ['ci.skip',
-         'another push option']
+        { 'ci' => { 'skip' => true } }
       end
 
       it 'creates a pipline in the skipped state' do