Respond with 404 when metrics are not enabled or deployment_metrics is not implemented

2 files changed, 53 insertions, 22 deletions

diff --git a/app/controllers/projects/deployments_controller.rb b/app/controllers/projects/deployments_controller.rb
index 83e9c7a3f1d..6644deb49c9 100644
--- a/app/controllers/projects/deployments_controller.rb
+++ b/app/controllers/projects/deployments_controller.rb
@@ -11,13 +11,15 @@ class Projects::DeploymentsController < Projects::ApplicationController
   end
 
   def metrics
+    return render_404 unless deployment.has_metrics?
     @metrics = deployment.metrics
-
     if @metrics&.any?
       render json: @metrics, status: :ok
     else
       head :no_content
     end
+  rescue NotImplementedError
+    render_404
   end
 
   private
diff --git a/spec/controllers/projects/deployments_controller_spec.rb b/spec/controllers/projects/deployments_controller_spec.rb
index 3de38bb4dac..4c69443314d 100644
--- a/spec/controllers/projects/deployments_controller_spec.rb
+++ b/spec/controllers/projects/deployments_controller_spec.rb
@@ -42,39 +42,68 @@ describe Projects::DeploymentsController do
     before do
       allow(controller).to receive(:deployment).and_return(deployment)
     end
-
-    context 'when environment has no metrics' do
+    context 'when metrics are disabled' do
       before do
-        expect(deployment).to receive(:metrics).and_return(nil)
+        allow(deployment).to receive(:has_metrics?).and_return false
       end
 
-      it 'returns a empty response 204 resposne' do
+      it 'responds with not found' do
         get :metrics, deployment_params(id: deployment.id)
-        expect(response).to have_http_status(204)
-        expect(response.body).to eq('')
+
+        expect(response).to be_not_found
       end
     end
 
-    context 'when environment has some metrics' do
-      let(:empty_metrics) do
-        {
-          success: true,
-          metrics: {},
-          last_update: 42
-        }
+    context 'when metrics are enabled' do
+      before do
+        allow(deployment).to receive(:has_metrics?).and_return true
       end
 
-      before do
-        expect(deployment).to receive(:metrics).and_return(empty_metrics)
+      context 'when environment has no metrics' do
+        before do
+          expect(deployment).to receive(:metrics).and_return(nil)
+        end
+
+        it 'returns a empty response 204 resposne' do
+          get :metrics, deployment_params(id: deployment.id)
+          expect(response).to have_http_status(204)
+          expect(response.body).to eq('')
+        end
       end
 
-      it 'returns a metrics JSON document' do
-        get :metrics, deployment_params(id: deployment.id)
+      context 'when environment has some metrics' do
+        let(:empty_metrics) do
+          {
+            success: true,
+            metrics: {},
+            last_update: 42
+          }
+        end
+
+        before do
+          expect(deployment).to receive(:metrics).and_return(empty_metrics)
+        end
+
+        it 'returns a metrics JSON document' do
+          get :metrics, deployment_params(id: deployment.id)
+
+          expect(response).to be_ok
+          expect(json_response['success']).to be(true)
+          expect(json_response['metrics']).to eq({})
+          expect(json_response['last_update']).to eq(42)
+        end
+      end
+
+      context 'when metrics service does not implement deployment metrics' do
+        before do
+          allow(deployment).to receive(:metrics).and_raise(NotImplementedError)
+        end
+
+        it 'responds with not found' do
+          get :metrics, deployment_params(id: deployment.id)
 
-        expect(response).to be_ok
-        expect(json_response['success']).to be(true)
-        expect(json_response['metrics']).to eq({})
-        expect(json_response['last_update']).to eq(42)
+          expect(response).to be_not_found
+        end
       end
     end
   end