Allow setting Gitaly feature flags for individual projects

When we call Gitaly and set feature flags, we can't easily guarantee
that we can get the current project (if there even is such a thing).
However, there are some cases where we do have easy access to a project,
if one exists: when we're performing Git HTTP or SSH operations.

This allows - but doesn't require - Gitaly feature flags to be set for a
project, as long as the call site supports it. In this commit, that
means the two cases above (Git HTTP and SSH).

lib/api/helpers/internal_helpers.rb

@@ -124,7 +124,7 @@ module API
           repository: repository.gitaly_repository.to_h,
           address: Gitlab::GitalyClient.address(repository.shard),
           token: Gitlab::GitalyClient.token(repository.shard),
-          features: Feature::Gitaly.server_feature_flags
+          features: Feature::Gitaly.server_feature_flags(repository.project)
         }
       end
     end

lib/feature/gitaly.rb

@@ -5,25 +5,25 @@ class Feature
     PREFIX = "gitaly_"
 
     class << self
-      def enabled?(feature_flag)
+      def enabled?(feature_flag, project = nil)
         return false unless Feature::FlipperFeature.table_exists?
 
-        Feature.enabled?("#{PREFIX}#{feature_flag}")
+        Feature.enabled?("#{PREFIX}#{feature_flag}", project)
       rescue ActiveRecord::NoDatabaseError, PG::ConnectionBad
         false
       end
 
-      def server_feature_flags
+      def server_feature_flags(project = nil)
         # We need to check that both the DB connection and table exists
         return {} unless ::Gitlab::Database.cached_table_exists?(FlipperFeature.table_name)
 
         Feature.persisted_names
           .select { |f| f.start_with?(PREFIX) }
-          .map do |f|
+          .to_h do |f|
           flag = f.delete_prefix(PREFIX)
 
-          ["gitaly-feature-#{flag.tr('_', '-')}", enabled?(flag).to_s]
-        end.to_h
+          ["gitaly-feature-#{flag.tr('_', '-')}", enabled?(flag, project).to_s]
+        end
       end
     end
   end

lib/gitlab/workhorse.rb

@@ -32,7 +32,7 @@ module Gitlab
           GitalyServer: {
             address: Gitlab::GitalyClient.address(repository.storage),
             token: Gitlab::GitalyClient.token(repository.storage),
-            features: Feature::Gitaly.server_feature_flags
+            features: Feature::Gitaly.server_feature_flags(repository.project)
           }
         }
 
@@ -231,7 +231,7 @@ module Gitlab
         {
           address: Gitlab::GitalyClient.address(repository.shard),
           token: Gitlab::GitalyClient.token(repository.shard),
-          features: Feature::Gitaly.server_feature_flags
+          features: Feature::Gitaly.server_feature_flags(repository.project)
         }
       end
 

spec/lib/feature/gitaly_spec.rb

@@ -3,35 +3,78 @@
 require 'spec_helper'
 
 RSpec.describe Feature::Gitaly do
-  let(:feature_flag) { "mep_mep" }
+  let_it_be(:project) { create(:project) }
+  let_it_be(:project_2) { create(:project) }
+
+  before do
+    skip_feature_flags_yaml_validation
+  end
 
   describe ".enabled?" do
-    context 'when the gate is closed' do
-      before do
-        stub_feature_flags(gitaly_mep_mep: false)
+    context 'when the flag is set globally' do
+      let(:feature_flag) { 'global_flag' }
+
+      context 'when the gate is closed' do
+        before do
+          stub_feature_flags(gitaly_global_flag: false)
+        end
+
+        it 'returns false' do
+          expect(described_class.enabled?(feature_flag)).to be(false)
+        end
       end
 
-      it 'returns false' do
-        expect(described_class.enabled?(feature_flag)).to be(false)
+      context 'when the flag defaults to on' do
+        it 'returns true' do
+          expect(described_class.enabled?(feature_flag)).to be(true)
+        end
       end
     end
 
-    context 'when the flag defaults to on' do
-      it 'returns true' do
-        expect(described_class.enabled?(feature_flag)).to be(true)
+    context 'when the flag is enabled for a particular project' do
+      let(:feature_flag) { 'project_flag' }
+
+      before do
+        stub_feature_flags(gitaly_project_flag: project)
+      end
+
+      it 'returns true for that project' do
+        expect(described_class.enabled?(feature_flag, project)).to be(true)
+      end
+
+      it 'returns false for any other project' do
+        expect(described_class.enabled?(feature_flag, project_2)).to be(false)
+      end
+
+      it 'returns false when no project is passed' do
+        expect(described_class.enabled?(feature_flag)).to be(false)
       end
     end
   end
 
   describe ".server_feature_flags" do
     before do
-      stub_feature_flags(gitaly_mep_mep: true, foo: true)
+      stub_feature_flags(gitaly_global_flag: true, gitaly_project_flag: project, non_gitaly_flag: false)
     end
 
     subject { described_class.server_feature_flags }
 
-    it { is_expected.to be_a(Hash) }
-    it { is_expected.to eq("gitaly-feature-mep-mep" => "true") }
+    it 'returns a hash of flags starting with the prefix, with dashes instead of underscores' do
+      expect(subject).to eq('gitaly-feature-global-flag' => 'true',
+                            'gitaly-feature-project-flag' => 'false')
+    end
+
+    context 'when a project is passed' do
+      it 'returns the value for the flag on the given project' do
+        expect(described_class.server_feature_flags(project))
+          .to eq('gitaly-feature-global-flag' => 'true',
+                 'gitaly-feature-project-flag' => 'true')
+
+        expect(described_class.server_feature_flags(project_2))
+          .to eq('gitaly-feature-global-flag' => 'true',
+                 'gitaly-feature-project-flag' => 'false')
+      end
+    end
 
     context 'when table does not exist' do
       before do

spec/lib/gitlab/workhorse_spec.rb

@@ -15,9 +15,7 @@ RSpec.describe Gitlab::Workhorse do
   end
 
   before do
-    allow(Feature::Gitaly).to receive(:server_feature_flags).and_return({
-      'gitaly-feature-foobar' => 'true'
-    })
+    stub_feature_flags(gitaly_enforce_requests_limits: true)
   end
 
   describe ".send_git_archive" do
@@ -43,7 +41,7 @@ RSpec.describe Gitlab::Workhorse do
       expect(command).to eq('git-archive')
       expect(params).to eq({
         'GitalyServer' => {
-          features: { 'gitaly-feature-foobar' => 'true' },
+          features: { 'gitaly-feature-enforce-requests-limits' => 'true' },
           address: Gitlab::GitalyClient.address(project.repository_storage),
           token: Gitlab::GitalyClient.token(project.repository_storage)
         },
@@ -73,7 +71,7 @@ RSpec.describe Gitlab::Workhorse do
         expect(command).to eq('git-archive')
         expect(params).to eq({
           'GitalyServer' => {
-            features: { 'gitaly-feature-foobar' => 'true' },
+            features: { 'gitaly-feature-enforce-requests-limits' => 'true' },
             address: Gitlab::GitalyClient.address(project.repository_storage),
             token: Gitlab::GitalyClient.token(project.repository_storage)
           },
@@ -124,7 +122,7 @@ RSpec.describe Gitlab::Workhorse do
       expect(command).to eq("git-format-patch")
       expect(params).to eq({
         'GitalyServer' => {
-          features: { 'gitaly-feature-foobar' => 'true' },
+          features: { 'gitaly-feature-enforce-requests-limits' => 'true' },
           address: Gitlab::GitalyClient.address(project.repository_storage),
           token: Gitlab::GitalyClient.token(project.repository_storage)
         },
@@ -187,7 +185,7 @@ RSpec.describe Gitlab::Workhorse do
       expect(command).to eq("git-diff")
       expect(params).to eq({
         'GitalyServer' => {
-          features: { 'gitaly-feature-foobar' => 'true' },
+          features: { 'gitaly-feature-enforce-requests-limits' => 'true' },
           address: Gitlab::GitalyClient.address(project.repository_storage),
           token: Gitlab::GitalyClient.token(project.repository_storage)
         },
@@ -274,7 +272,7 @@ RSpec.describe Gitlab::Workhorse do
       let(:gitaly_params) do
         {
           GitalyServer: {
-            features: { 'gitaly-feature-foobar' => 'true' },
+            features: { 'gitaly-feature-enforce-requests-limits' => 'true' },
             address: Gitlab::GitalyClient.address('default'),
             token: Gitlab::GitalyClient.token('default')
           }
@@ -310,6 +308,35 @@ RSpec.describe Gitlab::Workhorse do
 
           it { is_expected.to include(ShowAllRefs: true) }
         end
+
+        context 'when a feature flag is set for a single project' do
+          before do
+            stub_feature_flags(gitaly_mep_mep: project)
+          end
+
+          it 'sets the flag to true for that project' do
+            response = described_class.git_http_ok(repository, Gitlab::GlRepository::PROJECT, user, action)
+
+            expect(response.dig(:GitalyServer, :features)).to eq('gitaly-feature-enforce-requests-limits' => 'true',
+                                                                 'gitaly-feature-mep-mep' => 'true')
+          end
+
+          it 'sets the flag to false for other projects' do
+            other_project = create(:project, :public, :repository)
+            response = described_class.git_http_ok(other_project.repository, Gitlab::GlRepository::PROJECT, user, action)
+
+            expect(response.dig(:GitalyServer, :features)).to eq('gitaly-feature-enforce-requests-limits' => 'true',
+                                                                 'gitaly-feature-mep-mep' => 'false')
+          end
+
+          it 'sets the flag to false when there is no project' do
+            snippet = create(:personal_snippet, :repository)
+            response = described_class.git_http_ok(snippet.repository, Gitlab::GlRepository::SNIPPET, user, action)
+
+            expect(response.dig(:GitalyServer, :features)).to eq('gitaly-feature-enforce-requests-limits' => 'true',
+                                                                 'gitaly-feature-mep-mep' => 'false')
+          end
+        end
       end
 
       context "when git_receive_pack action is passed" do
@@ -423,7 +450,7 @@ RSpec.describe Gitlab::Workhorse do
       expect(command).to eq('git-blob')
       expect(params).to eq({
         'GitalyServer' => {
-          features: { 'gitaly-feature-foobar' => 'true' },
+          features: { 'gitaly-feature-enforce-requests-limits' => 'true' },
           address: Gitlab::GitalyClient.address(project.repository_storage),
           token: Gitlab::GitalyClient.token(project.repository_storage)
         },
@@ -485,7 +512,7 @@ RSpec.describe Gitlab::Workhorse do
       expect(command).to eq('git-snapshot')
       expect(params).to eq(
         'GitalyServer' => {
-          'features' => { 'gitaly-feature-foobar' => 'true' },
+          'features' => { 'gitaly-feature-enforce-requests-limits' => 'true' },
           'address' => Gitlab::GitalyClient.address(project.repository_storage),
           'token' => Gitlab::GitalyClient.token(project.repository_storage)
         },

spec/requests/api/internal/base_spec.rb

@@ -543,25 +543,51 @@ RSpec.describe API::Internal::Base do
       end
 
       context "git pull" do
-        before do
-          stub_feature_flags(gitaly_mep_mep: true)
+        context "with a feature flag enabled globally" do
+          before do
+            stub_feature_flags(gitaly_mep_mep: true)
+          end
+
+          it "has the correct payload" do
+            pull(key, project)
+
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(json_response["status"]).to be_truthy
+            expect(json_response["gl_repository"]).to eq("project-#{project.id}")
+            expect(json_response["gl_project_path"]).to eq(project.full_path)
+            expect(json_response["gitaly"]).not_to be_nil
+            expect(json_response["gitaly"]["repository"]).not_to be_nil
+            expect(json_response["gitaly"]["repository"]["storage_name"]).to eq(project.repository.gitaly_repository.storage_name)
+            expect(json_response["gitaly"]["repository"]["relative_path"]).to eq(project.repository.gitaly_repository.relative_path)
+            expect(json_response["gitaly"]["address"]).to eq(Gitlab::GitalyClient.address(project.repository_storage))
+            expect(json_response["gitaly"]["token"]).to eq(Gitlab::GitalyClient.token(project.repository_storage))
+            expect(json_response["gitaly"]["features"]).to eq('gitaly-feature-mep-mep' => 'true')
+            expect(user.reload.last_activity_on).to eql(Date.today)
+          end
         end
 
-        it "has the correct payload" do
-          pull(key, project)
+        context "with a feature flag enabled for a project" do
+          before do
+            stub_feature_flags(gitaly_mep_mep: project)
+          end
 
-          expect(response).to have_gitlab_http_status(:ok)
-          expect(json_response["status"]).to be_truthy
-          expect(json_response["gl_repository"]).to eq("project-#{project.id}")
-          expect(json_response["gl_project_path"]).to eq(project.full_path)
-          expect(json_response["gitaly"]).not_to be_nil
-          expect(json_response["gitaly"]["repository"]).not_to be_nil
-          expect(json_response["gitaly"]["repository"]["storage_name"]).to eq(project.repository.gitaly_repository.storage_name)
-          expect(json_response["gitaly"]["repository"]["relative_path"]).to eq(project.repository.gitaly_repository.relative_path)
-          expect(json_response["gitaly"]["address"]).to eq(Gitlab::GitalyClient.address(project.repository_storage))
-          expect(json_response["gitaly"]["token"]).to eq(Gitlab::GitalyClient.token(project.repository_storage))
-          expect(json_response["gitaly"]["features"]).to eq('gitaly-feature-mep-mep' => 'true')
-          expect(user.reload.last_activity_on).to eql(Date.today)
+          it "has the flag set to true for that project" do
+            pull(key, project)
+
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(json_response["gl_repository"]).to eq("project-#{project.id}")
+            expect(json_response["gitaly"]["features"]).to eq('gitaly-feature-mep-mep' => 'true')
+          end
+
+          it "has the flag set to false for other projects" do
+            other_project = create(:project, :public, :repository)
+
+            pull(key, other_project)
+
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(json_response["gl_repository"]).to eq("project-#{other_project.id}")
+            expect(json_response["gitaly"]["features"]).to eq('gitaly-feature-mep-mep' => 'false')
+          end
         end
       end