Merge branch 'mwaw/208224-move-cluster-promethues-api-proxy-to-core-BE' into 'master'

[RUN AS-IF-FOSS] Move Promethues API proxy for cluster health into the core

See merge request gitlab-org/gitlab!35610

app/controllers/clusters/clusters_controller.rb

@@ -2,6 +2,7 @@
 
 class Clusters::ClustersController < Clusters::BaseController
   include RoutableActions
+  include Metrics::Dashboard::PrometheusApiProxy
 
   before_action :cluster, only: [:cluster_status, :show, :update, :destroy, :clear_cache]
   before_action :generate_gcp_authorize_url, only: [:new]
@@ -290,6 +291,29 @@ class Clusters::ClustersController < Clusters::BaseController
     @gcp_cluster = cluster.present(current_user: current_user)
   end
 
+  def proxyable
+    cluster.cluster
+  end
+
+  # During first iteration of dashboard variables implementation
+  # cluster health case was omitted. Existing service for now is tied to
+  # environment, which is not always present for cluster health dashboard.
+  # It is planned to break coupling to environment https://gitlab.com/gitlab-org/gitlab/-/issues/213833.
+  # It is also planned to move cluster health to metrics dashboard section https://gitlab.com/gitlab-org/gitlab/-/issues/220214
+  # but for now I've used dummy class to stub variable substitution service, as there are no variables
+  # in cluster health dashboard
+  def proxy_variable_substitution_service
+    @empty_service ||= Class.new(BaseService) do
+      def initialize(proxyable, params)
+        @proxyable, @params = proxyable, params
+      end
+
+      def execute
+        success(params: @params)
+      end
+    end
+  end
+
   def user_cluster
     cluster = Clusters::BuildService.new(clusterable.subject).execute
     cluster.build_platform_kubernetes

app/policies/group_policy.rb

@@ -103,6 +103,7 @@ class GroupPolicy < BasePolicy
     enable :admin_list
     enable :admin_issue
     enable :read_metrics_dashboard_annotation
+    enable :read_prometheus
   end
 
   rule { maintainer }.policy do

config/routes.rb

@@ -190,7 +190,6 @@ Rails.application.routes.draw do
         Gitlab.ee do
           get :metrics, format: :json
           get :metrics_dashboard
-          get :'/prometheus/api/v1/*proxy_path', to: 'clusters#prometheus_proxy', as: :prometheus_api
           get :environments, format: :json
         end
 
@@ -200,6 +199,7 @@ Rails.application.routes.draw do
           delete '/:application', to: 'clusters/applications#destroy', as: :uninstall_applications
         end
 
+        get :'/prometheus/api/v1/*proxy_path', to: 'clusters#prometheus_proxy', as: :prometheus_api
         get :cluster_status, format: :json
         delete :clear_cache
       end

ee/app/controllers/ee/clusters/clusters_controller.rb

@@ -8,7 +8,6 @@ module EE
 
       prepended do
         before_action :expire_etag_cache, only: [:show]
-        before_action :authorize_read_prometheus!, only: :prometheus_proxy
       end
 
       def metrics
@@ -27,31 +26,6 @@ module EE
         end
       end
 
-      def prometheus_proxy
-        result = ::Prometheus::ProxyService.new(
-          cluster.cluster,
-          proxy_method,
-          proxy_path,
-          proxy_params
-        ).execute
-
-        if result.nil?
-          return render status: :no_content, json: {
-            status: _('processing'),
-            message: _('Not ready yet. Try again later.')
-          }
-        end
-
-        if result[:status] == :success
-          render status: result[:http_status], json: result[:body]
-        else
-          render(
-            status: result[:http_status] || :bad_request,
-            json: { status: result[:status], message: result[:message] }
-          )
-        end
-      end
-
       def environments
         respond_to do |format|
           format.json do
@@ -91,18 +65,6 @@ module EE
 
         cluster.application_prometheus
       end
-
-      def proxy_method
-        request.method
-      end
-
-      def proxy_path
-        params[:proxy_path]
-      end
-
-      def proxy_params
-        params.permit!
-      end
     end
   end
 end

ee/app/policies/ee/group_policy.rb

@@ -97,7 +97,6 @@ module EE
       rule { reporter }.policy do
         enable :admin_list
         enable :admin_board
-        enable :read_prometheus
         enable :view_productivity_analytics
         enable :view_type_of_work_charts
         enable :read_group_timelogs

ee/spec/controllers/admin/clusters_controller_spec.rb

@@ -28,34 +28,6 @@ RSpec.describe Admin::ClustersController do
       allow(::Clusters::Instance).to receive(:new).and_return(cluster.instance)
     end
 
-    context 'with inappropriate requests' do
-      context 'with anoymous user' do
-        before do
-          sign_out(user)
-        end
-
-        it 'renders not found' do
-          get :prometheus_proxy, params: prometheus_proxy_params
-
-          expect(response).to have_gitlab_http_status(:not_found)
-        end
-      end
-
-      context 'with non-admin user' do
-        let(:user) { create(:user) }
-
-        before do
-          sign_in(user)
-        end
-
-        it 'renders not found' do
-          get :prometheus_proxy, params: prometheus_proxy_params
-
-          expect(response).to have_gitlab_http_status(:not_found)
-        end
-      end
-    end
-
     describe 'GET #metrics_dashboard' do
       it_behaves_like 'the default dashboard'
     end

ee/spec/controllers/projects/clusters_controller_spec.rb

@@ -25,34 +25,6 @@ RSpec.describe Projects::ClustersController do
       clusterable.add_maintainer(user)
     end
 
-    context 'with inappropriate requests' do
-      context 'with annoymous user' do
-        before do
-          sign_out(user)
-        end
-
-        it 'redirects to signin page' do
-          get :prometheus_proxy, params: prometheus_proxy_params
-
-          expect(response).to redirect_to(new_user_session_path)
-        end
-      end
-
-      context 'with invalid clusterable id' do
-        before do
-          sign_in(user)
-        end
-
-        let(:other_clusterable) { create(:project) }
-
-        it 'returns 404' do
-          get :prometheus_proxy, params: prometheus_proxy_params(id: other_clusterable.id)
-
-          expect(response).to have_gitlab_http_status(:not_found)
-        end
-      end
-    end
-
     describe 'security' do
       let(:prometheus_adapter) { double(:prometheus_adapter, can_query?: true, query: nil) }
 

ee/spec/support/shared_examples/controllers/cluster_metrics_shared_examples.rb

@@ -74,118 +74,6 @@ RSpec.shared_examples 'cluster metrics' do
     end
   end
 
-  describe 'GET #prometheus_proxy' do
-    let(:prometheus_proxy_service) { instance_double(Prometheus::ProxyService) }
-    let(:expected_params) do
-      ActionController::Parameters.new(
-        prometheus_proxy_params(
-          proxy_path: 'query',
-          controller: subject.controller_path,
-          action: 'prometheus_proxy'
-        )
-      ).permit!
-    end
-
-    before do
-      sign_in(user)
-    end
-
-    context 'with valid requests' do
-      before do
-        allow(Prometheus::ProxyService).to receive(:new)
-          .with(cluster, '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"}' }
-
-        it 'returns prometheus response' do
-          prometheus_json_body = Gitlab::Json.parse(prometheus_body)
-
-          get :prometheus_proxy, params: prometheus_proxy_params
-
-          expect(Prometheus::ProxyService).to have_received(:new)
-            .with(cluster, 'GET', 'query', expected_params)
-          expect(response).to have_gitlab_http_status(:ok)
-          expect(json_response).to eq(prometheus_json_body)
-        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
-      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
-  end
-
   shared_examples 'the default dashboard' do
     it 'returns a json object with the correct keys' do
       get :metrics_dashboard, params: metrics_params, format: :json

spec/controllers/admin/clusters_controller_spec.rb

@@ -171,6 +171,29 @@ RSpec.describe Admin::ClustersController do
     end
   end
 
+  describe 'GET #prometheus_proxy' do
+    let(:user) { admin }
+    let(:proxyable) do
+      create(:cluster, :instance, :provided_by_gcp)
+    end
+
+    it_behaves_like 'metrics dashboard prometheus api proxy' do
+      context 'with anonymous user' do
+        let(:prometheus_body) { nil }
+
+        before do
+          sign_out(admin)
+        end
+
+        it 'returns 404' do
+          get :prometheus_proxy, params: prometheus_proxy_params
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
+    end
+  end
+
   describe 'POST #create_gcp' do
     let(:legacy_abac_param) { 'true' }
     let(:params) do

spec/controllers/groups/clusters_controller_spec.rb

@@ -192,6 +192,35 @@ RSpec.describe Groups::ClustersController do
     end
   end
 
+  describe 'GET #prometheus_proxy' do
+    let(:proxyable) do
+      create(:cluster, :provided_by_gcp, cluster_type: :group_type, groups: [group])
+    end
+
+    it_behaves_like 'metrics dashboard prometheus api proxy' do
+      let(:proxyable_params) do
+        {
+          id: proxyable.id.to_s,
+          group_id: group.name
+        }
+      end
+
+      context 'with anonymous user' do
+        let(:prometheus_body) { nil }
+
+        before do
+          sign_out(user)
+        end
+
+        it 'returns 404' do
+          get :prometheus_proxy, params: prometheus_proxy_params
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
+    end
+  end
+
   describe 'POST create for new cluster' do
     let(:legacy_abac_param) { 'true' }
     let(:params) do

spec/controllers/projects/clusters_controller_spec.rb

@@ -200,6 +200,36 @@ RSpec.describe Projects::ClustersController do
     end
   end
 
+  describe 'GET #prometheus_proxy' do
+    let(:proxyable) do
+      create(:cluster, :provided_by_gcp, projects: [project])
+    end
+
+    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 anonymous user' do
+        let(:prometheus_body) { nil }
+
+        before do
+          sign_out(user)
+        end
+
+        it 'redirects to signin page' do
+          get :prometheus_proxy, params: prometheus_proxy_params
+
+          expect(response).to redirect_to(new_user_session_path)
+        end
+      end
+    end
+  end
+
   describe 'POST create for new cluster' do
     let(:legacy_abac_param) { 'true' }
     let(:params) do

spec/support/shared_contexts/policies/group_policy_shared_context.rb

@@ -18,7 +18,7 @@ RSpec.shared_context 'GroupPolicy context' do
    ]
   end
   let(:read_group_permissions) { %i[read_label read_list read_milestone read_board] }
-  let(:reporter_permissions) { %i[admin_label read_container_image read_metrics_dashboard_annotation] }
+  let(:reporter_permissions) { %i[admin_label read_container_image read_metrics_dashboard_annotation read_prometheus] }
   let(:developer_permissions) { %i[admin_milestone create_metrics_dashboard_annotation delete_metrics_dashboard_annotation update_metrics_dashboard_annotation] }
   let(:maintainer_permissions) do
     %i[