From 39753bfb9cdd77ed7fc1458afc202b126ea6984d Mon Sep 17 00:00:00 2001
From: Ahmad Sherif <me@ahmadsherif.com>
Date: Thu, 30 Mar 2017 17:24:36 +0200
Subject: [PATCH] Add feature flags for enabling (Upload|Receive)Pack for
 Gitaly

Closes gitaly#168
---
 .../projects/git_http_controller.rb           |  2 +-
 lib/gitlab/workhorse.rb                       | 16 ++++-
 spec/lib/gitlab/workhorse_spec.rb             | 58 ++++++++++++++++---
 3 files changed, 66 insertions(+), 10 deletions(-)

diff --git a/app/controllers/projects/git_http_controller.rb b/app/controllers/projects/git_http_controller.rb
index 278098fcc58..37f6f637ff0 100644
--- a/app/controllers/projects/git_http_controller.rb
+++ b/app/controllers/projects/git_http_controller.rb
@@ -57,7 +57,7 @@ class Projects::GitHttpController < Projects::GitHttpClientController
 
   def render_ok
     set_workhorse_internal_api_content_type
-    render json: Gitlab::Workhorse.git_http_ok(repository, user)
+    render json: Gitlab::Workhorse.git_http_ok(repository, user, action_name)
   end
 
   def render_http_not_allowed
diff --git a/lib/gitlab/workhorse.rb b/lib/gitlab/workhorse.rb
index d1131ad65e0..d0637f8b394 100644
--- a/lib/gitlab/workhorse.rb
+++ b/lib/gitlab/workhorse.rb
@@ -16,7 +16,7 @@ module Gitlab
     SECRET_LENGTH = 32
 
     class << self
-      def git_http_ok(repository, user)
+      def git_http_ok(repository, user, action)
         repo_path = repository.path_to_repo
         params = {
           GL_ID: Gitlab::GlId.gl_id(user),
@@ -26,13 +26,25 @@ module Gitlab
         if Gitlab.config.gitaly.enabled
           storage = repository.project.repository_storage
           address = Gitlab::GitalyClient.get_address(storage)
-          params[:GitalySocketPath] = URI(address).path
           # TODO: use GitalyClient code to assemble the Repository message
           params[:Repository] = Gitaly::Repository.new(
             path: repo_path,
             storage_name: storage,
             relative_path: Gitlab::RepoPath.strip_storage_path(repo_path),
           ).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
+
+          params[:GitalySocketPath] = URI(address).path if feature_enabled
         end
 
         params
diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb
index cb7c810124e..3bd2a3238fe 100644
--- a/spec/lib/gitlab/workhorse_spec.rb
+++ b/spec/lib/gitlab/workhorse_spec.rb
@@ -180,24 +180,68 @@ describe Gitlab::Workhorse, lib: true do
   describe '.git_http_ok' do
     let(:user) { create(:user) }
     let(:repo_path) { repository.path_to_repo }
+    let(:action) { 'info_refs' }
 
-    subject { described_class.git_http_ok(repository, user) }
+    subject { described_class.git_http_ok(repository, user, action) }
 
-    it { expect(subject).to eq({ GL_ID: "user-#{user.id}", RepoPath: repo_path }) }
+    it { expect(subject).to include({ GL_ID: "user-#{user.id}", RepoPath: repo_path }) }
 
     context 'when Gitaly is enabled' do
+      let(:gitaly_params) do
+        {
+          GitalySocketPath: URI(Gitlab::GitalyClient.get_address('default')).path,
+        }
+      end
+
       before do
         allow(Gitlab.config.gitaly).to receive(:enabled).and_return(true)
       end
 
-      it 'includes Gitaly params in the returned value' do
-        gitaly_socket_path = URI(Gitlab::GitalyClient.get_address('default')).path
-        expect(subject).to include({ GitalySocketPath: gitaly_socket_path })
-        expect(subject[:Repository]).to include({
+      it 'includes a Repository param' do
+        repo_param = { Repository: {
           path: repo_path,
           storage_name: 'default',
           relative_path: project.full_path + '.git',
-        })
+        } }
+
+        expect(subject).to include(repo_param)
+      end
+
+      {
+        git_receive_pack: :post_receive_pack,
+        git_upload_pack: :post_upload_pack
+      }.each do |action_name, feature_flag|
+        context "when #{action_name} action is passed" do
+          let(:action) { action_name }
+
+          context 'when action is enabled by feature flag' do
+            it 'includes Gitaly params in the returned value' do
+              allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(feature_flag).and_return(true)
+
+              expect(subject).to include(gitaly_params)
+            end
+          end
+
+          context 'when action is not enabled by feature flag' do
+            it 'does not include Gitaly params in the returned value' do
+              allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(feature_flag).and_return(false)
+
+              expect(subject).not_to include(gitaly_params)
+            end
+          end
+        end
+      end
+
+      context "when info_refs action is passed" do
+        let(:action) { 'info_refs' }
+
+        it { expect(subject).to include(gitaly_params) }
+      end
+
+      context 'when action passed is not supported by Gitaly' do
+        let(:action) { 'download' }
+
+        it { expect { subject }.to raise_exception('Unsupported action: download') }
       end
     end
   end
-- 
GitLab