Automatic merge of gitlab-org/gitlab-ce master

app/models/deployment.rb

@@ -84,12 +84,9 @@ class Deployment < ApplicationRecord
     Commit.truncate_sha(sha)
   end
 
-  def cluster
-    platform = project.deployment_platform(environment: environment.name)
-
-    if platform.present? && platform.respond_to?(:cluster)
-      platform.cluster
-    end
+  # Deprecated - will be replaced by a persisted cluster_id
+  def deployment_platform_cluster
+    environment.deployment_platform&.cluster
   end
 
   def execute_hooks

changelogs/unreleased/50834-change-http-status-code-when-repository-disabled.yml

+---
+title: "Changed HTTP Status Code for disabled repository on /branches and /commits to 404"
+merge_request: 29585
+author: Sam Battalio
+type: changed

lib/api/branches.rb

@@ -8,7 +8,10 @@ module API
 
     BRANCH_ENDPOINT_REQUIREMENTS = API::NAMESPACE_OR_PROJECT_REQUIREMENTS.merge(branch: API::NO_SLASH_URL_PART_REGEX)
 
-    before { authorize! :download_code, user_project }
+    before do
+      require_repository_enabled!
+      authorize! :download_code, user_project
+    end
 
     helpers do
       params :filter_params do

lib/api/commits.rb

@@ -6,7 +6,10 @@ module API
   class Commits < Grape::API
     include PaginationParams
 
-    before { authorize! :download_code, user_project }
+    before do
+      require_repository_enabled!
+      authorize! :download_code, user_project
+    end
 
     helpers do
       def user_access

lib/api/helpers.rb

@@ -250,6 +250,10 @@ module API
       authorize! :update_build, user_project
     end
 
+    def require_repository_enabled!(subject = :global)
+      not_found!("Repository") unless user_project.feature_available?(:repository, current_user)
+    end
+
     def require_gitlab_workhorse!
       unless env['HTTP_GITLAB_WORKHORSE'].present?
         forbidden!('Request should be executed via GitLab Workhorse')

lib/gitlab/ci/build/prerequisite/kubernetes_namespace.rb

@@ -20,7 +20,7 @@ module Gitlab
           private
 
           def deployment_cluster
-            build.deployment&.cluster
+            build.deployment&.deployment_platform_cluster
           end
 
           def kubernetes_namespace

spec/lib/gitlab/ci/build/prerequisite/kubernetes_namespace_spec.rb

@@ -23,7 +23,7 @@ describe Gitlab::Ci::Build::Prerequisite::KubernetesNamespace do
         let(:cluster) { create(:cluster, :group) }
 
         before do
-          allow(build.deployment).to receive(:cluster).and_return(cluster)
+          allow(build.deployment).to receive(:deployment_platform_cluster).and_return(cluster)
         end
 
         it { is_expected.to be_truthy }
@@ -49,7 +49,7 @@ describe Gitlab::Ci::Build::Prerequisite::KubernetesNamespace do
 
       context 'and no cluster to deploy to' do
         before do
-          expect(deployment.cluster).to be_nil
+          expect(deployment.deployment_platform_cluster).to be_nil
         end
 
         it { is_expected.to be_falsey }
@@ -67,7 +67,7 @@ describe Gitlab::Ci::Build::Prerequisite::KubernetesNamespace do
       let(:cluster) { create(:cluster, :group) }
 
       before do
-        allow(build.deployment).to receive(:cluster).and_return(cluster)
+        allow(build.deployment).to receive(:deployment_platform_cluster).and_return(cluster)
       end
 
       it 'creates a kubernetes namespace' do
@@ -84,7 +84,7 @@ describe Gitlab::Ci::Build::Prerequisite::KubernetesNamespace do
 
     context 'completion is not required' do
       before do
-        expect(deployment.cluster).to be_nil
+        expect(deployment.deployment_platform_cluster).to be_nil
       end
 
       it 'does not create a namespace' do

spec/models/deployment_spec.rb

@@ -380,12 +380,12 @@ describe Deployment do
     end
   end
 
-  describe '#cluster' do
+  describe '#deployment_platform_cluster' do
     let(:deployment) { create(:deployment) }
     let(:project) { deployment.project }
     let(:environment) { deployment.environment }
 
-    subject { deployment.cluster }
+    subject { deployment.deployment_platform_cluster }
 
     before do
       expect(project).to receive(:deployment_platform)

spec/requests/api/branches_spec.rb

@@ -65,7 +65,7 @@ describe API::Branches do
       context 'when repository is disabled' do
         include_context 'disabled repository'
 
-        it_behaves_like '403 response' do
+        it_behaves_like '404 response' do
           let(:request) { get api(route, current_user) }
         end
       end
@@ -175,7 +175,7 @@ describe API::Branches do
       context 'when repository is disabled' do
         include_context 'disabled repository'
 
-        it_behaves_like '403 response' do
+        it_behaves_like '404 response' do
           let(:request) { get api(route, current_user) }
         end
       end
@@ -337,7 +337,7 @@ describe API::Branches do
       context 'when repository is disabled' do
         include_context 'disabled repository'
 
-        it_behaves_like '403 response' do
+        it_behaves_like '404 response' do
           let(:request) { put api(route, current_user) }
         end
       end
@@ -471,7 +471,7 @@ describe API::Branches do
       context 'when repository is disabled' do
         include_context 'disabled repository'
 
-        it_behaves_like '403 response' do
+        it_behaves_like '404 response' do
           let(:request) { put api(route, current_user) }
         end
       end
@@ -547,7 +547,7 @@ describe API::Branches do
       context 'when repository is disabled' do
         include_context 'disabled repository'
 
-        it_behaves_like '403 response' do
+        it_behaves_like '404 response' do
           let(:request) { post api(route, current_user) }
         end
       end

spec/requests/api/commits_spec.rb

@@ -736,7 +736,7 @@ describe API::Commits do
     context 'when repository is disabled' do
       include_context 'disabled repository'
 
-      it_behaves_like '403 response' do
+      it_behaves_like '404 response' do
         let(:request) { get api(route, current_user) }
       end
     end
@@ -825,7 +825,7 @@ describe API::Commits do
       context 'when repository is disabled' do
         include_context 'disabled repository'
 
-        it_behaves_like '403 response' do
+        it_behaves_like '404 response' do
           let(:request) { get api(route, current_user) }
         end
       end
@@ -968,7 +968,7 @@ describe API::Commits do
       context 'when repository is disabled' do
         include_context 'disabled repository'
 
-        it_behaves_like '403 response' do
+        it_behaves_like '404 response' do
           let(:request) { get api(route, current_user) }
         end
       end
@@ -1067,7 +1067,7 @@ describe API::Commits do
       context 'when repository is disabled' do
         include_context 'disabled repository'
 
-        it_behaves_like '403 response' do
+        it_behaves_like '404 response' do
           let(:request) { get api(route, current_user) }
         end
       end
@@ -1169,7 +1169,7 @@ describe API::Commits do
       context 'when repository is disabled' do
         include_context 'disabled repository'
 
-        it_behaves_like '403 response' do
+        it_behaves_like '404 response' do
           let(:request) { post api(route, current_user), params: { branch: 'master' } }
         end
       end
@@ -1324,7 +1324,7 @@ describe API::Commits do
       context 'when repository is disabled' do
         include_context 'disabled repository'
 
-        it_behaves_like '403 response' do
+        it_behaves_like '404 response' do
           let(:request) { post api(route, current_user), params: { branch: branch } }
         end
       end
@@ -1435,7 +1435,7 @@ describe API::Commits do
       context 'when repository is disabled' do
         include_context 'disabled repository'
 
-        it_behaves_like '403 response' do
+        it_behaves_like '404 response' do
           let(:request) { post api(route, current_user), params: { note: 'My comment' } }
         end
       end