Merge branch...

Merge branch 'mwaw/208224-move-cluster-health-feature-into-gitlab-core-BE-prometheus-proxy' into 'master'

Extract Metrics Prometheus API proxy logic into shared module

See merge request gitlab-org/gitlab!35606

app/controllers/concerns/metrics/dashboard/prometheus_api_proxy.rb

+# frozen_string_literal: true
+
+module Metrics::Dashboard::PrometheusApiProxy
+  extend ActiveSupport::Concern
+  include RenderServiceResults
+
+  included do
+    before_action :authorize_read_prometheus!, only: [:prometheus_proxy]
+  end
+
+  def prometheus_proxy
+    variable_substitution_result =
+      proxy_variable_substitution_service.new(proxyable, permit_params).execute
+
+    if variable_substitution_result[:status] == :error
+      return error_response(variable_substitution_result)
+    end
+
+    prometheus_result = Prometheus::ProxyService.new(
+      proxyable,
+      proxy_method,
+      proxy_path,
+      variable_substitution_result[:params]
+    ).execute
+
+    return continue_polling_response if prometheus_result.nil?
+    return error_response(prometheus_result) if prometheus_result[:status] == :error
+
+    success_response(prometheus_result)
+  end
+
+  private
+
+  def proxyable
+    raise NotImplementedError, "#{self.class} must implement method: #{__callee__}"
+  end
+
+  def proxy_variable_substitution_service
+    raise NotImplementedError, "#{self.class} must implement method: #{__callee__}"
+  end
+
+  def permit_params
+    params.permit!
+  end
+
+  def proxy_method
+    request.method
+  end
+
+  def proxy_path
+    params[:proxy_path]
+  end
+end

app/controllers/projects/environments/prometheus_api_controller.rb

 # frozen_string_literal: true
 
 class Projects::Environments::PrometheusApiController < Projects::ApplicationController
-  include RenderServiceResults
+  include Metrics::Dashboard::PrometheusApiProxy
 
-  before_action :authorize_read_prometheus!
-  before_action :environment
-
-  def proxy
-    variable_substitution_result =
-      variable_substitution_service.new(environment, permit_params).execute
-
-    if variable_substitution_result[:status] == :error
-      return error_response(variable_substitution_result)
-    end
-
-    prometheus_result = Prometheus::ProxyService.new(
-      environment,
-      proxy_method,
-      proxy_path,
-      variable_substitution_result[:params]
-    ).execute
-
-    return continue_polling_response if prometheus_result.nil?
-    return error_response(prometheus_result) if prometheus_result[:status] == :error
-
-    success_response(prometheus_result)
-  end
+  before_action :proxyable
 
   private
 
-  def variable_substitution_service
-    Prometheus::ProxyVariableSubstitutionService
+  def proxyable
+    @proxyable ||= project.environments.find(params[:id])
   end
 
-  def permit_params
-    params.permit!
-  end
-
-  def environment
-    @environment ||= project.environments.find(params[:id])
-  end
-
-  def proxy_method
-    request.method
-  end
-
-  def proxy_path
-    params[:proxy_path]
+  def proxy_variable_substitution_service
+    Prometheus::ProxyVariableSubstitutionService
   end
 end

config/routes/project.rb

@@ -257,7 +257,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
             # This route is also defined in gitlab-workhorse. Make sure to update accordingly.
             get '/terminal.ws/authorize', to: 'environments#terminal_websocket_authorize', format: false
 
-            get '/prometheus/api/v1/*proxy_path', to: 'environments/prometheus_api#proxy', as: :prometheus_api
+            get '/prometheus/api/v1/*proxy_path', to: 'environments/prometheus_api#prometheus_proxy', as: :prometheus_api
 
             get '/sample_metrics', to: 'environments/sample_metrics#query'
           end

spec/controllers/projects/environments/prometheus_api_controller_spec.rb

@@ -3,83 +3,27 @@
 require 'spec_helper'
 
 RSpec.describe Projects::Environments::PrometheusApiController do
-  let_it_be(:project) { create(:project) }
-  let_it_be(:environment) { create(:environment, project: project) }
   let_it_be(:user) { create(:user) }
+  let_it_be(:project) { create(:project) }
+  let_it_be(:proxyable) { create(:environment, project: project) }
 
   before do
     project.add_reporter(user)
     sign_in(user)
   end
 
-  describe 'GET #proxy' do
-    let(:prometheus_proxy_service) { instance_double(Prometheus::ProxyService) }
-
-    let(:expected_params) do
-      ActionController::Parameters.new(
-        environment_params(
-          proxy_path: 'query',
-          controller: 'projects/environments/prometheus_api',
-          action: 'proxy'
-        )
-      ).permit!
-    end
-
-    context 'with valid requests' do
-      before do
-        allow(Prometheus::ProxyService).to receive(:new)
-          .with(environment, 'GET', 'query', expected_params)
-          .and_return(prometheus_proxy_service)
-
-        allow(prometheus_proxy_service).to receive(:execute)
-          .and_return(service_result)
-      end
-
-      context 'with success result' do
-        let(:service_result) { { status: :success, body: prometheus_body } }
-        let(:prometheus_body) { '{"status":"success"}' }
-        let(:prometheus_json_body) { Gitlab::Json.parse(prometheus_body) }
-
-        it 'returns prometheus response' do
-          get :proxy, params: environment_params
-
-          expect(Prometheus::ProxyService).to have_received(:new)
-            .with(environment, 'GET', 'query', expected_params)
-          expect(response).to have_gitlab_http_status(:ok)
-          expect(json_response).to eq(prometheus_json_body)
-        end
-
-        context 'with format string' do
-          before do
-            expected_params[:query] = %{up{environment="#{environment.slug}"}}
-          end
-
-          it 'replaces variables with values' do
-            get :proxy, params: environment_params.merge(query: 'up{environment="{{ci_environment_slug}}"}')
-
-            expect(Prometheus::ProxyService).to have_received(:new)
-              .with(environment, 'GET', 'query', expected_params)
-          end
-
-          context 'with nil query' do
-            let(:params_without_query) do
-              environment_params.except(:query)
-            end
-
-            before do
-              expected_params.delete(:query)
-            end
-
-            it 'does not raise error' do
-              get :proxy, params: params_without_query
-
-              expect(Prometheus::ProxyService).to have_received(:new)
-                .with(environment, 'GET', 'query', expected_params)
-            end
-          end
+  describe 'GET #prometheus_proxy' do
+    it_behaves_like 'metrics dashboard prometheus api proxy' do
+      let(:proxyable_params) do
+        {
+          id: proxyable.id.to_s,
+          namespace_id: project.namespace.full_path,
+          project_id: project.name
+        }
       end
 
       context 'with variables' do
+        let(:prometheus_body) { '{"status":"success"}' }
         let(:pod_name) { "pod1" }
 
         before do
@@ -88,130 +32,44 @@ RSpec.describe Projects::Environments::PrometheusApiController do
         end
 
         it 'replaces variables with values' do
-            get :proxy, params: environment_params.merge(
+          get :prometheus_proxy, params: prometheus_proxy_params.merge(
             query: 'up{pod_name="{{pod_name}}"}', variables: { 'pod_name' => pod_name }
           )
 
           expect(response).to have_gitlab_http_status(:success)
           expect(Prometheus::ProxyService).to have_received(:new)
-              .with(environment, 'GET', 'query', expected_params)
+                                                .with(proxyable, 'GET', 'query', expected_params)
         end
 
         context 'with invalid variables' do
           let(:params_with_invalid_variables) do
-              environment_params.merge(
+            prometheus_proxy_params.merge(
               query: 'up{pod_name="{{pod_name}}"}', variables: ['a']
             )
           end
 
           it 'returns 400' do
-              get :proxy, params: params_with_invalid_variables
+            get :prometheus_proxy, params: params_with_invalid_variables
 
             expect(response).to have_gitlab_http_status(:bad_request)
             expect(Prometheus::ProxyService).not_to receive(:new)
           end
         end
       end
-      end
-
-      context 'with nil result' do
-        let(:service_result) { nil }
 
-        it 'returns 204 no_content' do
-          get :proxy, params: environment_params
-
-          expect(json_response['status']).to eq(_('processing'))
-          expect(json_response['message']).to eq(_('Not ready yet. Try again later.'))
-          expect(response).to have_gitlab_http_status(:no_content)
-        end
-      end
-
-      context 'with 404 result' do
-        let(:service_result) { { http_status: 404, status: :success, body: '{"body": "value"}' } }
-
-        it 'returns body' do
-          get :proxy, params: environment_params
-
-          expect(response).to have_gitlab_http_status(:not_found)
-          expect(json_response['body']).to eq('value')
-        end
-      end
-
-      context 'with error result' do
-        context 'with http_status' do
-          let(:service_result) do
-            { http_status: :service_unavailable, status: :error, message: 'error message' }
-          end
-
-          it 'sets the http response status code' do
-            get :proxy, params: environment_params
-
-            expect(response).to have_gitlab_http_status(:service_unavailable)
-            expect(json_response['status']).to eq('error')
-            expect(json_response['message']).to eq('error message')
-          end
-        end
-
-        context 'without http_status' do
-          let(:service_result) { { status: :error, message: 'error message' } }
-
-          it 'returns bad_request' do
-            get :proxy, params: environment_params
-
-            expect(response).to have_gitlab_http_status(:bad_request)
-            expect(json_response['status']).to eq('error')
-            expect(json_response['message']).to eq('error message')
-          end
-        end
-      end
-    end
-
-    context 'with inappropriate requests' do
       context 'with anonymous user' do
+        let(:prometheus_body) { nil }
+
         before do
           sign_out(user)
         end
 
         it 'redirects to signin page' do
-          get :proxy, params: environment_params
+          get :prometheus_proxy, params: prometheus_proxy_params
 
           expect(response).to redirect_to(new_user_session_path)
         end
       end
-
-      context 'without correct permissions' do
-        before do
-          project.team.truncate
-        end
-
-        it 'returns 404' do
-          get :proxy, params: environment_params
-
-          expect(response).to have_gitlab_http_status(:not_found)
-        end
     end
   end
-
-    context 'with invalid environment id' do
-      let(:other_environment) { create(:environment) }
-
-      it 'returns 404' do
-        get :proxy, params: environment_params(id: other_environment.id)
-
-        expect(response).to have_gitlab_http_status(:not_found)
-      end
-    end
-  end
-
-  private
-
-  def environment_params(params = {})
-    {
-      id: environment.id.to_s,
-      namespace_id: project.namespace.full_path,
-      project_id: project.name,
-      proxy_path: 'query',
-      query: '1'
-    }.merge(params)
-  end
 end

spec/support/shared_examples/controllers/metrics/dashboard/prometheus_api_proxy_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples_for 'metrics dashboard prometheus api proxy' do
+  let(:service_params) { [proxyable, 'GET', 'query', expected_params] }
+  let(:service_result) { { status: :success, body: prometheus_body } }
+  let(:prometheus_proxy_service) { instance_double(Prometheus::ProxyService) }
+  let(:proxyable_params) do
+    {
+      id: proxyable.id.to_s
+    }
+  end
+  let(:expected_params) do
+    ActionController::Parameters.new(
+      prometheus_proxy_params(
+        proxy_path: 'query',
+        controller: described_class.controller_path,
+        action: 'prometheus_proxy'
+      )
+    ).permit!
+  end
+
+  before do
+    allow_next_instance_of(Prometheus::ProxyService, *service_params) do |proxy_service|
+      allow(proxy_service).to receive(:execute).and_return(service_result)
+    end
+  end
+
+  context 'with valid requests' do
+    context 'with success result' do
+      let(:prometheus_body) { '{"status":"success"}' }
+      let(:prometheus_json_body) { Gitlab::Json.parse(prometheus_body) }
+
+      it 'returns prometheus response' do
+        get :prometheus_proxy, params: prometheus_proxy_params
+
+        expect(Prometheus::ProxyService).to have_received(:new).with(*service_params)
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response).to eq(prometheus_json_body)
+      end
+
+      context 'with nil query' do
+        let(:params_without_query) do
+          prometheus_proxy_params.except(:query)
+        end
+
+        before do
+          expected_params.delete(:query)
+        end
+
+        it 'does not raise error' do
+          get :prometheus_proxy, params: params_without_query
+
+          expect(Prometheus::ProxyService).to have_received(:new).with(*service_params)
+        end
+      end
+    end
+
+    context 'with nil result' do
+      let(:service_result) { nil }
+
+      it 'returns 204 no_content' do
+        get :prometheus_proxy, params: prometheus_proxy_params
+
+        expect(json_response['status']).to eq(_('processing'))
+        expect(json_response['message']).to eq(_('Not ready yet. Try again later.'))
+        expect(response).to have_gitlab_http_status(:no_content)
+      end
+    end
+
+    context 'with 404 result' do
+      let(:service_result) { { http_status: 404, status: :success, body: '{"body": "value"}' } }
+
+      it 'returns body' do
+        get :prometheus_proxy, params: prometheus_proxy_params
+
+        expect(response).to have_gitlab_http_status(:not_found)
+        expect(json_response['body']).to eq('value')
+      end
+    end
+
+    context 'with error result' do
+      context 'with http_status' do
+        let(:service_result) do
+          { http_status: :service_unavailable, status: :error, message: 'error message' }
+        end
+
+        it 'sets the http response status code' do
+          get :prometheus_proxy, params: prometheus_proxy_params
+
+          expect(response).to have_gitlab_http_status(:service_unavailable)
+          expect(json_response['status']).to eq('error')
+          expect(json_response['message']).to eq('error message')
+        end
+      end
+
+      context 'without http_status' do
+        let(:service_result) { { status: :error, message: 'error message' } }
+
+        it 'returns bad_request' do
+          get :prometheus_proxy, params: prometheus_proxy_params
+
+          expect(response).to have_gitlab_http_status(:bad_request)
+          expect(json_response['status']).to eq('error')
+          expect(json_response['message']).to eq('error message')
+        end
+      end
+    end
+  end
+
+  context 'with inappropriate requests' do
+    let(:prometheus_body) { nil }
+
+    context 'without correct permissions' do
+      let(:user2) { create(:user) }
+
+      before do
+        sign_out(user)
+        sign_in(user2)
+      end
+
+      it 'returns 404' do
+        get :prometheus_proxy, params: prometheus_proxy_params
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+  end
+
+  context 'with invalid proxyable id' do
+    let(:prometheus_body) { nil }
+
+    it 'returns 404' do
+      get :prometheus_proxy, params: prometheus_proxy_params(id: proxyable.id + 1)
+
+      expect(response).to have_gitlab_http_status(:not_found)
+    end
+  end
+
+  private
+
+  def prometheus_proxy_params(params = {})
+    {
+      proxy_path: 'query',
+      query: '1'
+    }.merge(proxyable_params).merge(params)
+  end
+end