From baab836b9bb5cab5fada8e0ce155b80b805c07b7 Mon Sep 17 00:00:00 2001
From: rpereira2 <rpereira@gitlab.com>
Date: Tue, 2 Apr 2019 11:24:36 +0530
Subject: Add a proxy method to PrometheusClient

- Also refactor the get and json_api_get methods so that the get method
can be reused by the new proxy method.
- The new proxy method makes no changes to the request to the prometheus
server and response from the prometheus server. This allows it to be
used as a proxy to the Prometheus server, hence the name.
---
 lib/gitlab/prometheus_client.rb           | 46 +++++++++++-----
 spec/lib/gitlab/prometheus_client_spec.rb | 89 ++++++++++++++++++++++++++++---
 2 files changed, 115 insertions(+), 20 deletions(-)

diff --git a/lib/gitlab/prometheus_client.rb b/lib/gitlab/prometheus_client.rb
index b4de7cd2bce..09609969afc 100644
--- a/lib/gitlab/prometheus_client.rb
+++ b/lib/gitlab/prometheus_client.rb
@@ -24,6 +24,19 @@ module Gitlab
       json_api_get('query', query: '1')
     end
 
+    def proxy(type, args)
+      path = api_path(type)
+      get(path, args)
+    rescue RestClient::ExceptionWithResponse => ex
+      if ex.response
+        ex.response
+      else
+        raise PrometheusClient::Error, "Network connection error"
+      end
+    rescue RestClient::Exception
+      raise PrometheusClient::Error, "Network connection error"
+    end
+
     def query(query, time: Time.now)
       get_result('vector') do
         json_api_get('query', query: query, time: time.to_f)
@@ -64,22 +77,16 @@ module Gitlab
 
     private
 
-    def json_api_get(type, args = {})
-      path = ['api', 'v1', type].join('/')
-      get(path, args)
-    rescue JSON::ParserError
-      raise PrometheusClient::Error, 'Parsing response failed'
-    rescue Errno::ECONNREFUSED
-      raise PrometheusClient::Error, 'Connection refused'
+    def api_path(type)
+      ['api', 'v1', type].join('/')
     end
 
-    def get(path, args)
-      response = rest_client[path].get(params: args)
+    def json_api_get(type, args = {})
+      path = api_path(type)
+      response = get(path, args)
       handle_response(response)
-    rescue SocketError
-      raise PrometheusClient::Error, "Can't connect to #{rest_client.url}"
-    rescue OpenSSL::SSL::SSLError
-      raise PrometheusClient::Error, "#{rest_client.url} contains invalid SSL data"
+    rescue JSON::ParserError
+      raise PrometheusClient::Error, 'Parsing response failed'
     rescue RestClient::ExceptionWithResponse => ex
       if ex.response
         handle_exception_response(ex.response)
@@ -90,6 +97,17 @@ module Gitlab
       raise PrometheusClient::Error, "Network connection error"
     end
 
+    def get(path, args)
+      response = rest_client[path].get(params: args)
+      response
+    rescue SocketError
+      raise PrometheusClient::Error, "Can't connect to #{rest_client.url}"
+    rescue OpenSSL::SSL::SSLError
+      raise PrometheusClient::Error, "#{rest_client.url} contains invalid SSL data"
+    rescue Errno::ECONNREFUSED
+      raise PrometheusClient::Error, 'Connection refused'
+    end
+
     def handle_response(response)
       json_data = JSON.parse(response.body)
       if response.code == 200 && json_data['status'] == 'success'
@@ -108,6 +126,8 @@ module Gitlab
       else
         raise PrometheusClient::Error, "#{response.code} - #{response.body}"
       end
+    rescue JSON::ParserError
+      raise PrometheusClient::Error, 'Parsing response failed'
     end
 
     def get_result(expected_type)
diff --git a/spec/lib/gitlab/prometheus_client_spec.rb b/spec/lib/gitlab/prometheus_client_spec.rb
index 2517ee71f24..d3a28b53da9 100644
--- a/spec/lib/gitlab/prometheus_client_spec.rb
+++ b/spec/lib/gitlab/prometheus_client_spec.rb
@@ -60,15 +60,13 @@ describe Gitlab::PrometheusClient do
   end
 
   describe 'failure to reach a provided prometheus url' do
-    let(:prometheus_url) {"https://prometheus.invalid.example.com"}
+    let(:prometheus_url) {"https://prometheus.invalid.example.com/api/v1/query?query=1"}
 
-    subject { described_class.new(RestClient::Resource.new(prometheus_url)) }
-
-    context 'exceptions are raised' do
+    shared_examples 'exceptions are raised' do
       it 'raises a Gitlab::PrometheusClient::Error error when a SocketError is rescued' do
         req_stub = stub_prometheus_request_with_exception(prometheus_url, SocketError)
 
-        expect { subject.send(:get, '/', {}) }
+        expect { subject }
           .to raise_error(Gitlab::PrometheusClient::Error, "Can't connect to #{prometheus_url}")
         expect(req_stub).to have_been_requested
       end
@@ -76,7 +74,7 @@ describe Gitlab::PrometheusClient do
       it 'raises a Gitlab::PrometheusClient::Error error when a SSLError is rescued' do
         req_stub = stub_prometheus_request_with_exception(prometheus_url, OpenSSL::SSL::SSLError)
 
-        expect { subject.send(:get, '/', {}) }
+        expect { subject }
           .to raise_error(Gitlab::PrometheusClient::Error, "#{prometheus_url} contains invalid SSL data")
         expect(req_stub).to have_been_requested
       end
@@ -84,11 +82,23 @@ describe Gitlab::PrometheusClient do
       it 'raises a Gitlab::PrometheusClient::Error error when a RestClient::Exception is rescued' do
         req_stub = stub_prometheus_request_with_exception(prometheus_url, RestClient::Exception)
 
-        expect { subject.send(:get, '/', {}) }
+        expect { subject }
           .to raise_error(Gitlab::PrometheusClient::Error, "Network connection error")
         expect(req_stub).to have_been_requested
       end
     end
+
+    context 'ping' do
+      subject { described_class.new(RestClient::Resource.new(prometheus_url)).ping }
+
+      it_behaves_like 'exceptions are raised'
+    end
+
+    context 'proxy' do
+      subject { described_class.new(RestClient::Resource.new(prometheus_url)).proxy('query', { query: '1' }) }
+
+      it_behaves_like 'exceptions are raised'
+    end
   end
 
   describe '#query' do
@@ -258,4 +268,69 @@ describe Gitlab::PrometheusClient do
       it { is_expected.to eq(step) }
     end
   end
+
+  describe 'proxy' do
+    context 'query' do
+      let(:prometheus_query) { prometheus_cpu_query('env-slug') }
+      let(:query_url) { prometheus_query_url(prometheus_query) }
+
+      around do |example|
+        Timecop.freeze { example.run }
+      end
+
+      it 'returns full response from the API call' do
+        req_stub = stub_prometheus_request(query_url, body: prometheus_value_body('vector'))
+
+        response = subject.proxy('query', { query: prometheus_query })
+        json_response = JSON.parse(response.body)
+
+        expect(response.code).to eq(200)
+        expect(json_response).to eq({
+          'status' => 'success',
+          'data' => {
+            'resultType' => 'vector',
+            'result' => [{ "metric" => {}, "value" => [1488772511.004, "0.000041021495238095323"] }]
+          }
+        })
+        expect(req_stub).to have_been_requested
+      end
+    end
+
+    context 'query_range' do
+      let(:prometheus_query) { prometheus_memory_query('env-slug') }
+      let(:query_url) { prometheus_query_range_url(prometheus_query, start: 2.hours.ago) }
+
+      around do |example|
+        Timecop.freeze { example.run }
+      end
+
+      it 'returns full response' do
+        req_stub = stub_prometheus_request(query_url, body: prometheus_values_body('vector'))
+
+        response = subject.proxy('query_range', {
+          query: prometheus_query,
+          start: 2.hours.ago.to_f,
+          end: Time.now.to_f,
+          step: 60
+        })
+        json_response = JSON.parse(response.body)
+
+        expect(response.code).to eq(200)
+        expect(json_response).to eq({
+          "status" => "success",
+          "data" => {
+            "resultType" => "vector",
+            "result" => [{
+              "metric" => {},
+              "values" => [
+                [1488758662.506, "0.00002996364761904785"],
+                [1488758722.506, "0.00003090239047619091"]
+              ]
+            }]
+          }
+        })
+        expect(req_stub).to have_been_requested
+      end
+    end
+  end
 end
-- 
cgit v1.2.1


From 399173d6fad09b8a6334d38c3b8412617fe1dd9a Mon Sep 17 00:00:00 2001
From: rpereira2 <rpereira@gitlab.com>
Date: Tue, 2 Apr 2019 11:35:29 +0530
Subject: Add a Prometheus::ProxyService

- The service uses the PrometheusClient.proxy method to call the
Prometheus API with the given parameters, and returns the body and
http status code of the API response to the caller of the service.
- The service uses reactive caching in order to prevent Puma/Unicorn
threads from being blocked until the Prometheus API responds.
---
 app/services/prometheus/proxy_service.rb       | 105 +++++++++++++
 spec/services/prometheus/proxy_service_spec.rb | 204 +++++++++++++++++++++++++
 2 files changed, 309 insertions(+)
 create mode 100644 app/services/prometheus/proxy_service.rb
 create mode 100644 spec/services/prometheus/proxy_service_spec.rb

diff --git a/app/services/prometheus/proxy_service.rb b/app/services/prometheus/proxy_service.rb
new file mode 100644
index 00000000000..6dc68e378fd
--- /dev/null
+++ b/app/services/prometheus/proxy_service.rb
@@ -0,0 +1,105 @@
+# frozen_string_literal: true
+
+module Prometheus
+  class ProxyService < BaseService
+    include ReactiveCaching
+
+    self.reactive_cache_key = ->(service) { service.cache_key }
+    self.reactive_cache_lease_timeout = 30.seconds
+    self.reactive_cache_refresh_interval = 30.seconds
+    self.reactive_cache_lifetime = 1.minute
+    self.reactive_cache_worker_finder = ->(_id, *args) { from_cache(*args) }
+
+    attr_accessor :prometheus_owner, :method, :path, :params
+
+    PROXY_SUPPORT = {
+      'query' => 'GET',
+      'query_range' => 'GET'
+    }.freeze
+
+    def self.from_cache(prometheus_owner_class_name, prometheus_owner_id, method, path, params)
+      prometheus_owner_class = begin
+        prometheus_owner_class_name.constantize
+      rescue NameError
+        nil
+      end
+      return unless prometheus_owner_class
+
+      prometheus_owner = prometheus_owner_class.find(prometheus_owner_id)
+
+      new(prometheus_owner, method, path, params)
+    end
+
+    # prometheus_owner can be any model which responds to .prometheus_adapter
+    # like Environment.
+    def initialize(prometheus_owner, method, path, params)
+      @prometheus_owner = prometheus_owner
+      @path = path
+      # Convert ActionController::Parameters to hash because reactive_cache_worker
+      # does not play nice with ActionController::Parameters.
+      @params = params.to_hash
+      @method = method
+    end
+
+    def id
+      nil
+    end
+
+    def execute
+      return cannot_proxy_response unless can_proxy?(@method, @path)
+      return no_prometheus_response unless can_query?
+
+      with_reactive_cache(*cache_key) do |result|
+        result
+      end
+    end
+
+    def calculate_reactive_cache(prometheus_owner_class_name, prometheus_owner_id, method, path, params)
+      @prometheus_owner = prometheus_owner_from_class(prometheus_owner_class_name, prometheus_owner_id)
+
+      return cannot_proxy_response unless can_proxy?(method, path)
+      return no_prometheus_response unless can_query?
+
+      response = prometheus_client_wrapper.proxy(path, params)
+
+      success({ http_status: response.code, body: response.body })
+
+    rescue Gitlab::PrometheusClient::Error => err
+      error(err.message, :service_unavailable)
+    end
+
+    def cache_key
+      [@prometheus_owner.class.name, @prometheus_owner.id, @method, @path, @params]
+    end
+
+    private
+
+    def no_prometheus_response
+      error('No prometheus server found', :service_unavailable)
+    end
+
+    def cannot_proxy_response
+      error('Proxy support for this API is not available currently')
+    end
+
+    def prometheus_owner_from_class(prometheus_owner_class_name, prometheus_owner_id)
+      Kernel.const_get(prometheus_owner_class_name).find(prometheus_owner_id)
+    end
+
+    def prometheus_adapter
+      @prometheus_adapter ||= @prometheus_owner.prometheus_adapter
+    end
+
+    def prometheus_client_wrapper
+      prometheus_adapter&.prometheus_client_wrapper
+    end
+
+    def can_query?
+      prometheus_adapter&.can_query?
+    end
+
+    def can_proxy?(method, path)
+      PROXY_SUPPORT[path] == method
+    end
+  end
+end
diff --git a/spec/services/prometheus/proxy_service_spec.rb b/spec/services/prometheus/proxy_service_spec.rb
new file mode 100644
index 00000000000..53e852aa79c
--- /dev/null
+++ b/spec/services/prometheus/proxy_service_spec.rb
@@ -0,0 +1,204 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Prometheus::ProxyService do
+  include ReactiveCachingHelpers
+
+  set(:project) { create(:project) }
+  set(:environment) { create(:environment, project: project) }
+
+  describe '#initialize' do
+    let(:params) { ActionController::Parameters.new({ query: '1' }).permit! }
+
+    it 'initializes attributes' do
+      result = described_class.new(environment, 'GET', 'query', { query: '1' })
+
+      expect(result.prometheus_owner).to eq(environment)
+      expect(result.method).to eq('GET')
+      expect(result.path).to eq('query')
+      expect(result.params).to eq({ query: '1' })
+    end
+
+    it 'converts ActionController::Parameters into hash' do
+      result = described_class.new(environment, 'GET', 'query', params)
+
+      expect(result.params).to be_an_instance_of(Hash)
+    end
+  end
+
+  describe '#execute' do
+    let(:prometheus_adapter) { instance_double(PrometheusService) }
+
+    subject { described_class.new(environment, 'GET', 'query', { query: '1' }) }
+
+    context 'When prometheus_adapter is nil' do
+      before do
+        allow(environment).to receive(:prometheus_adapter).and_return(nil)
+      end
+
+      it 'should return error' do
+        expect(subject.execute).to eq({
+          status: :error,
+          message: 'No prometheus server found',
+          http_status: :service_unavailable
+        })
+      end
+    end
+
+    context 'When prometheus_adapter cannot query' do
+      before do
+        allow(environment).to receive(:prometheus_adapter).and_return(prometheus_adapter)
+        allow(prometheus_adapter).to receive(:can_query?).and_return(false)
+      end
+
+      it 'should return error' do
+        expect(subject.execute).to eq({
+          status: :error,
+          message: 'No prometheus server found',
+          http_status: :service_unavailable
+        })
+      end
+    end
+
+    context 'Cannot proxy' do
+      subject { described_class.new(environment, 'POST', 'query', { query: '1' }) }
+
+      it 'returns error' do
+        expect(subject.execute).to eq({
+          message: 'Proxy support for this API is not available currently',
+          status: :error
+        })
+      end
+    end
+
+    context 'When cached', :use_clean_rails_memory_store_caching do
+      let(:return_value) { { 'http_status' => 200, 'body' => 'body' } }
+      let(:opts) { [environment.class.name, environment.id, 'GET', 'query', { query: '1' }] }
+
+      before do
+        stub_reactive_cache(subject, return_value, opts)
+
+        allow(environment).to receive(:prometheus_adapter)
+          .and_return(prometheus_adapter)
+        allow(prometheus_adapter).to receive(:can_query?).and_return(true)
+      end
+
+      it 'returns cached value' do
+        result = subject.execute
+
+        expect(result[:http_status]).to eq(return_value[:http_status])
+        expect(result[:body]).to eq(return_value[:body])
+      end
+    end
+
+    context 'When not cached' do
+      let(:return_value) { { 'http_status' => 200, 'body' => 'body' } }
+      let(:opts) { [environment.class.name, environment.id, 'GET', 'query', { query: '1' }] }
+
+      before do
+        allow(environment).to receive(:prometheus_adapter)
+          .and_return(prometheus_adapter)
+        allow(prometheus_adapter).to receive(:can_query?).and_return(true)
+      end
+
+      it 'returns nil' do
+        expect(ReactiveCachingWorker)
+          .to receive(:perform_async)
+          .with(subject.class, subject.id, *opts)
+
+        result = subject.execute
+
+        expect(result).to eq(nil)
+      end
+    end
+
+    context 'Call prometheus api' do
+      let(:prometheus_client) { instance_double(Gitlab::PrometheusClient) }
+
+      before do
+        synchronous_reactive_cache(subject)
+
+        allow(environment).to receive(:prometheus_adapter)
+          .and_return(prometheus_adapter)
+        allow(prometheus_adapter).to receive(:can_query?).and_return(true)
+        allow(prometheus_adapter).to receive(:prometheus_client_wrapper)
+          .and_return(prometheus_client)
+      end
+
+      context 'Connection to prometheus server succeeds' do
+        let(:rest_client_response) { instance_double(RestClient::Response) }
+
+        before do
+          allow(prometheus_client).to receive(:proxy).and_return(rest_client_response)
+
+          allow(rest_client_response).to receive(:code)
+            .and_return(prometheus_http_status_code)
+          allow(rest_client_response).to receive(:body).and_return(response_body)
+        end
+
+        shared_examples 'return prometheus http status code and body' do
+          it do
+            expect(subject.execute).to eq({
+              http_status: prometheus_http_status_code,
+              body: response_body,
+              status: :success
+            })
+          end
+        end
+
+        context 'prometheus returns success' do
+          let(:prometheus_http_status_code) { 200 }
+
+          let(:response_body) do
+            '{"status":"success","data":{"resultType":"scalar","result":[1553864609.117,"1"]}}'
+          end
+
+          before do
+          end
+
+          it_behaves_like 'return prometheus http status code and body'
+        end
+
+        context 'prometheus returns error' do
+          let(:prometheus_http_status_code) { 400 }
+
+          let(:response_body) do
+            '{"status":"error","errorType":"bad_data","error":"parse error at char 1: no expression found in input"}'
+          end
+
+          it_behaves_like 'return prometheus http status code and body'
+        end
+      end
+
+      context 'connection to prometheus server fails' do
+        context 'prometheus client raises Gitlab::PrometheusClient::Error' do
+          before do
+            allow(prometheus_client).to receive(:proxy)
+              .and_raise(Gitlab::PrometheusClient::Error, 'Network connection error')
+          end
+
+          it 'returns error' do
+            expect(subject.execute).to eq({
+              status: :error,
+              message: 'Network connection error',
+              http_status: :service_unavailable
+            })
+          end
+        end
+      end
+    end
+  end
+
+  describe '.from_cache' do
+    it 'initializes an instance of ProxyService class' do
+      result = described_class.from_cache(environment.class.name, environment.id, 'GET', 'query', { query: '1' })
+
+      expect(result).to be_an_instance_of(described_class)
+      expect(result.prometheus_owner).to eq(environment)
+      expect(result.method).to eq('GET')
+      expect(result.path).to eq('query')
+      expect(result.params).to eq({ query: '1' })
+    end
+  end
+end
-- 
cgit v1.2.1


From 5910f7600459cf1167492617c3e001c80c90c941 Mon Sep 17 00:00:00 2001
From: rpereira2 <rpereira@gitlab.com>
Date: Tue, 2 Apr 2019 19:18:26 +0530
Subject: Remove unnecessary line

- calculate_reactive_cache does not need to initialize @prometheus_owner
again. It's already being initialized in the initialize method.
---
 app/services/prometheus/proxy_service.rb | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/app/services/prometheus/proxy_service.rb b/app/services/prometheus/proxy_service.rb
index 6dc68e378fd..d6fe20d377e 100644
--- a/app/services/prometheus/proxy_service.rb
+++ b/app/services/prometheus/proxy_service.rb
@@ -55,8 +55,6 @@ module Prometheus
     end
 
     def calculate_reactive_cache(prometheus_owner_class_name, prometheus_owner_id, method, path, params)
-      @prometheus_owner = prometheus_owner_from_class(prometheus_owner_class_name, prometheus_owner_id)
-
       return cannot_proxy_response unless can_proxy?(method, path)
       return no_prometheus_response unless can_query?
 
@@ -82,10 +80,6 @@ module Prometheus
       error('Proxy support for this API is not available currently')
     end
 
-    def prometheus_owner_from_class(prometheus_owner_class_name, prometheus_owner_id)
-      Kernel.const_get(prometheus_owner_class_name).find(prometheus_owner_id)
-    end
-
     def prometheus_adapter
       @prometheus_adapter ||= @prometheus_owner.prometheus_adapter
     end
-- 
cgit v1.2.1


From 6938977a0a2e2cf7c1916f92fb9d8e2119b73e3c Mon Sep 17 00:00:00 2001
From: rpereira2 <rpereira@gitlab.com>
Date: Tue, 2 Apr 2019 20:37:49 +0530
Subject: Improvements after review

- Add a method for returning a service_unavailable response.
- Keep only one spec to test that the prometheus response is returned
as-is.
---
 app/services/prometheus/proxy_service.rb       | 16 ++++++----
 spec/services/prometheus/proxy_service_spec.rb | 42 +++++++-------------------
 2 files changed, 21 insertions(+), 37 deletions(-)

diff --git a/app/services/prometheus/proxy_service.rb b/app/services/prometheus/proxy_service.rb
index d6fe20d377e..bd2f72df64d 100644
--- a/app/services/prometheus/proxy_service.rb
+++ b/app/services/prometheus/proxy_service.rb
@@ -46,7 +46,7 @@ module Prometheus
     end
 
     def execute
-      return cannot_proxy_response unless can_proxy?(@method, @path)
+      return cannot_proxy_response unless can_proxy?
       return no_prometheus_response unless can_query?
 
       with_reactive_cache(*cache_key) do |result|
@@ -55,15 +55,15 @@ module Prometheus
     end
 
     def calculate_reactive_cache(prometheus_owner_class_name, prometheus_owner_id, method, path, params)
-      return cannot_proxy_response unless can_proxy?(method, path)
+      return cannot_proxy_response unless can_proxy?
       return no_prometheus_response unless can_query?
 
       response = prometheus_client_wrapper.proxy(path, params)
 
-      success({ http_status: response.code, body: response.body })
+      success(http_status: response.code, body: response.body)
 
     rescue Gitlab::PrometheusClient::Error => err
-      error(err.message, :service_unavailable)
+      service_unavailable_response(err)
     end
 
     def cache_key
@@ -72,6 +72,10 @@ module Prometheus
 
     private
 
+    def service_unavailable_response(exception)
+      error(exception.message, :service_unavailable)
+    end
+
     def no_prometheus_response
       error('No prometheus server found', :service_unavailable)
     end
@@ -92,8 +96,8 @@ module Prometheus
       prometheus_adapter&.can_query?
     end
 
-    def can_proxy?(method, path)
-      PROXY_SUPPORT[path] == method
+    def can_proxy?
+      PROXY_SUPPORT[@path] == @method
     end
   end
 end
diff --git a/spec/services/prometheus/proxy_service_spec.rb b/spec/services/prometheus/proxy_service_spec.rb
index 53e852aa79c..391a108c862 100644
--- a/spec/services/prometheus/proxy_service_spec.rb
+++ b/spec/services/prometheus/proxy_service_spec.rb
@@ -128,6 +128,11 @@ describe Prometheus::ProxyService do
 
       context 'Connection to prometheus server succeeds' do
         let(:rest_client_response) { instance_double(RestClient::Response) }
+        let(:prometheus_http_status_code) { 400 }
+
+        let(:response_body) do
+          '{"status":"error","errorType":"bad_data","error":"parse error at char 1: no expression found in input"}'
+        end
 
         before do
           allow(prometheus_client).to receive(:proxy).and_return(rest_client_response)
@@ -137,37 +142,12 @@ describe Prometheus::ProxyService do
           allow(rest_client_response).to receive(:body).and_return(response_body)
         end
 
-        shared_examples 'return prometheus http status code and body' do
-          it do
-            expect(subject.execute).to eq({
-              http_status: prometheus_http_status_code,
-              body: response_body,
-              status: :success
-            })
-          end
-        end
-
-        context 'prometheus returns success' do
-          let(:prometheus_http_status_code) { 200 }
-
-          let(:response_body) do
-            '{"status":"success","data":{"resultType":"scalar","result":[1553864609.117,"1"]}}'
-          end
-
-          before do
-          end
-
-          it_behaves_like 'return prometheus http status code and body'
-        end
-
-        context 'prometheus returns error' do
-          let(:prometheus_http_status_code) { 400 }
-
-          let(:response_body) do
-            '{"status":"error","errorType":"bad_data","error":"parse error at char 1: no expression found in input"}'
-          end
-
-          it_behaves_like 'return prometheus http status code and body'
+        it 'returns the http status code and body from prometheus' do
+          expect(subject.execute).to eq(
+            http_status: prometheus_http_status_code,
+            body: response_body,
+            status: :success
+          )
         end
       end
 
-- 
cgit v1.2.1


From f71d243a6d7fc0675b0b3fc9e35f9523d8dcb145 Mon Sep 17 00:00:00 2001
From: rpereira2 <rpereira@gitlab.com>
Date: Tue, 2 Apr 2019 20:43:36 +0530
Subject: Remove all superfluous braces

---
 app/services/prometheus/proxy_service.rb       |  1 -
 spec/services/prometheus/proxy_service_spec.rb | 22 +++++++++++-----------
 2 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/app/services/prometheus/proxy_service.rb b/app/services/prometheus/proxy_service.rb
index bd2f72df64d..dccad287720 100644
--- a/app/services/prometheus/proxy_service.rb
+++ b/app/services/prometheus/proxy_service.rb
@@ -61,7 +61,6 @@ module Prometheus
       response = prometheus_client_wrapper.proxy(path, params)
 
       success(http_status: response.code, body: response.body)
-
     rescue Gitlab::PrometheusClient::Error => err
       service_unavailable_response(err)
     end
diff --git a/spec/services/prometheus/proxy_service_spec.rb b/spec/services/prometheus/proxy_service_spec.rb
index 391a108c862..1291c7a0217 100644
--- a/spec/services/prometheus/proxy_service_spec.rb
+++ b/spec/services/prometheus/proxy_service_spec.rb
@@ -9,7 +9,7 @@ describe Prometheus::ProxyService do
   set(:environment) { create(:environment, project: project) }
 
   describe '#initialize' do
-    let(:params) { ActionController::Parameters.new({ query: '1' }).permit! }
+    let(:params) { ActionController::Parameters.new(query: '1').permit! }
 
     it 'initializes attributes' do
       result = described_class.new(environment, 'GET', 'query', { query: '1' })
@@ -17,7 +17,7 @@ describe Prometheus::ProxyService do
       expect(result.prometheus_owner).to eq(environment)
       expect(result.method).to eq('GET')
       expect(result.path).to eq('query')
-      expect(result.params).to eq({ query: '1' })
+      expect(result.params).to eq(query: '1')
     end
 
     it 'converts ActionController::Parameters into hash' do
@@ -38,11 +38,11 @@ describe Prometheus::ProxyService do
       end
 
       it 'should return error' do
-        expect(subject.execute).to eq({
+        expect(subject.execute).to eq(
           status: :error,
           message: 'No prometheus server found',
           http_status: :service_unavailable
-        })
+        )
       end
     end
 
@@ -53,11 +53,11 @@ describe Prometheus::ProxyService do
       end
 
       it 'should return error' do
-        expect(subject.execute).to eq({
+        expect(subject.execute).to eq(
           status: :error,
           message: 'No prometheus server found',
           http_status: :service_unavailable
-        })
+        )
       end
     end
 
@@ -65,10 +65,10 @@ describe Prometheus::ProxyService do
       subject { described_class.new(environment, 'POST', 'query', { query: '1' }) }
 
       it 'returns error' do
-        expect(subject.execute).to eq({
+        expect(subject.execute).to eq(
           message: 'Proxy support for this API is not available currently',
           status: :error
-        })
+        )
       end
     end
 
@@ -159,11 +159,11 @@ describe Prometheus::ProxyService do
           end
 
           it 'returns error' do
-            expect(subject.execute).to eq({
+            expect(subject.execute).to eq(
               status: :error,
               message: 'Network connection error',
               http_status: :service_unavailable
-            })
+            )
           end
         end
       end
@@ -178,7 +178,7 @@ describe Prometheus::ProxyService do
       expect(result.prometheus_owner).to eq(environment)
       expect(result.method).to eq('GET')
       expect(result.path).to eq('query')
-      expect(result.params).to eq({ query: '1' })
+      expect(result.params).to eq(query: '1')
     end
   end
 end
-- 
cgit v1.2.1


From 9817124f14c378a018530a604fd63fc30584b1ff Mon Sep 17 00:00:00 2001
From: rpereira2 <rpereira@gitlab.com>
Date: Tue, 2 Apr 2019 20:44:06 +0530
Subject: Start context names with lowercase letter

---
 spec/services/prometheus/proxy_service_spec.rb | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/spec/services/prometheus/proxy_service_spec.rb b/spec/services/prometheus/proxy_service_spec.rb
index 1291c7a0217..1b98c68f99d 100644
--- a/spec/services/prometheus/proxy_service_spec.rb
+++ b/spec/services/prometheus/proxy_service_spec.rb
@@ -32,7 +32,7 @@ describe Prometheus::ProxyService do
 
     subject { described_class.new(environment, 'GET', 'query', { query: '1' }) }
 
-    context 'When prometheus_adapter is nil' do
+    context 'when prometheus_adapter is nil' do
       before do
         allow(environment).to receive(:prometheus_adapter).and_return(nil)
       end
@@ -46,7 +46,7 @@ describe Prometheus::ProxyService do
       end
     end
 
-    context 'When prometheus_adapter cannot query' do
+    context 'when prometheus_adapter cannot query' do
       before do
         allow(environment).to receive(:prometheus_adapter).and_return(prometheus_adapter)
         allow(prometheus_adapter).to receive(:can_query?).and_return(false)
@@ -61,7 +61,7 @@ describe Prometheus::ProxyService do
       end
     end
 
-    context 'Cannot proxy' do
+    context 'cannot proxy' do
       subject { described_class.new(environment, 'POST', 'query', { query: '1' }) }
 
       it 'returns error' do
@@ -72,7 +72,7 @@ describe Prometheus::ProxyService do
       end
     end
 
-    context 'When cached', :use_clean_rails_memory_store_caching do
+    context 'when cached', :use_clean_rails_memory_store_caching do
       let(:return_value) { { 'http_status' => 200, 'body' => 'body' } }
       let(:opts) { [environment.class.name, environment.id, 'GET', 'query', { query: '1' }] }
 
@@ -92,7 +92,7 @@ describe Prometheus::ProxyService do
       end
     end
 
-    context 'When not cached' do
+    context 'when not cached' do
       let(:return_value) { { 'http_status' => 200, 'body' => 'body' } }
       let(:opts) { [environment.class.name, environment.id, 'GET', 'query', { query: '1' }] }
 
@@ -113,7 +113,7 @@ describe Prometheus::ProxyService do
       end
     end
 
-    context 'Call prometheus api' do
+    context 'call prometheus api' do
       let(:prometheus_client) { instance_double(Gitlab::PrometheusClient) }
 
       before do
@@ -126,7 +126,7 @@ describe Prometheus::ProxyService do
           .and_return(prometheus_client)
       end
 
-      context 'Connection to prometheus server succeeds' do
+      context 'connection to prometheus server succeeds' do
         let(:rest_client_response) { instance_double(RestClient::Response) }
         let(:prometheus_http_status_code) { 400 }
 
-- 
cgit v1.2.1


From a3a98039b96a7f47e015642fa2268ae7e4297063 Mon Sep 17 00:00:00 2001
From: Peter Leitzen <pleitzen@gitlab.com>
Date: Tue, 2 Apr 2019 17:21:22 +0200
Subject: Extract parsing JSON

---
 lib/gitlab/prometheus_client.rb | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/lib/gitlab/prometheus_client.rb b/lib/gitlab/prometheus_client.rb
index 09609969afc..7b9b951888d 100644
--- a/lib/gitlab/prometheus_client.rb
+++ b/lib/gitlab/prometheus_client.rb
@@ -85,8 +85,6 @@ module Gitlab
       path = api_path(type)
       response = get(path, args)
       handle_response(response)
-    rescue JSON::ParserError
-      raise PrometheusClient::Error, 'Parsing response failed'
     rescue RestClient::ExceptionWithResponse => ex
       if ex.response
         handle_exception_response(ex.response)
@@ -109,7 +107,7 @@ module Gitlab
     end
 
     def handle_response(response)
-      json_data = JSON.parse(response.body)
+      json_data = parse_json(response.body)
       if response.code == 200 && json_data['status'] == 'success'
         json_data['data'] || {}
       else
@@ -121,18 +119,22 @@ module Gitlab
       if response.code == 200 && response['status'] == 'success'
         response['data'] || {}
       elsif response.code == 400
-        json_data = JSON.parse(response.body)
+        json_data = parse_json(response.body)
         raise PrometheusClient::QueryError, json_data['error'] || 'Bad data received'
       else
         raise PrometheusClient::Error, "#{response.code} - #{response.body}"
       end
-    rescue JSON::ParserError
-      raise PrometheusClient::Error, 'Parsing response failed'
     end
 
     def get_result(expected_type)
       data = yield
       data['result'] if data['resultType'] == expected_type
     end
+
+    def parse_json(response_body)
+      JSON.parse(response_body)
+    rescue JSON::ParserError
+      raise PrometheusClient::Error, 'Parsing response failed'
+    end
   end
 end
-- 
cgit v1.2.1


From 8ffbd8e29aeeb33029831d66a2ae3cddd4fad46a Mon Sep 17 00:00:00 2001
From: Peter Leitzen <pleitzen@gitlab.com>
Date: Tue, 2 Apr 2019 17:35:17 +0200
Subject: Remove redundant variable declaration

---
 lib/gitlab/prometheus_client.rb | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/gitlab/prometheus_client.rb b/lib/gitlab/prometheus_client.rb
index 7b9b951888d..f13156f898e 100644
--- a/lib/gitlab/prometheus_client.rb
+++ b/lib/gitlab/prometheus_client.rb
@@ -96,8 +96,7 @@ module Gitlab
     end
 
     def get(path, args)
-      response = rest_client[path].get(params: args)
-      response
+      rest_client[path].get(params: args)
     rescue SocketError
       raise PrometheusClient::Error, "Can't connect to #{rest_client.url}"
     rescue OpenSSL::SSL::SSLError
-- 
cgit v1.2.1


From 90be98aac7bece5b619cf306e687943454c4ec6b Mon Sep 17 00:00:00 2001
From: rpereira2 <rpereira@gitlab.com>
Date: Tue, 2 Apr 2019 22:53:07 +0530
Subject: Remove no_proxy check in calculate_reactive_cache

- It is unlikely that we will stop supporting prometheus APIs that were
previously supported.
---
 app/services/prometheus/proxy_service.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/app/services/prometheus/proxy_service.rb b/app/services/prometheus/proxy_service.rb
index dccad287720..55888fedb04 100644
--- a/app/services/prometheus/proxy_service.rb
+++ b/app/services/prometheus/proxy_service.rb
@@ -55,7 +55,6 @@ module Prometheus
     end
 
     def calculate_reactive_cache(prometheus_owner_class_name, prometheus_owner_id, method, path, params)
-      return cannot_proxy_response unless can_proxy?
       return no_prometheus_response unless can_query?
 
       response = prometheus_client_wrapper.proxy(path, params)
-- 
cgit v1.2.1


From 359d00cd5ded5ef7d3cf33b34c1e4869f1e5c9c5 Mon Sep 17 00:00:00 2001
From: rpereira2 <rpereira@gitlab.com>
Date: Tue, 2 Apr 2019 23:44:50 +0530
Subject: Add spec for RestClient raising exception

---
 spec/lib/gitlab/prometheus_client_spec.rb | 84 ++++++++++++++-----------------
 1 file changed, 37 insertions(+), 47 deletions(-)

diff --git a/spec/lib/gitlab/prometheus_client_spec.rb b/spec/lib/gitlab/prometheus_client_spec.rb
index d3a28b53da9..f15ae83a02c 100644
--- a/spec/lib/gitlab/prometheus_client_spec.rb
+++ b/spec/lib/gitlab/prometheus_client_spec.rb
@@ -270,7 +270,7 @@ describe Gitlab::PrometheusClient do
   end
 
   describe 'proxy' do
-    context 'query' do
+    context 'get API' do
       let(:prometheus_query) { prometheus_cpu_query('env-slug') }
       let(:query_url) { prometheus_query_url(prometheus_query) }
 
@@ -278,58 +278,48 @@ describe Gitlab::PrometheusClient do
         Timecop.freeze { example.run }
       end
 
-      it 'returns full response from the API call' do
-        req_stub = stub_prometheus_request(query_url, body: prometheus_value_body('vector'))
-
-        response = subject.proxy('query', { query: prometheus_query })
-        json_response = JSON.parse(response.body)
-
-        expect(response.code).to eq(200)
-        expect(json_response).to eq({
-          'status' => 'success',
-          'data' => {
-            'resultType' => 'vector',
-            'result' => [{ "metric" => {}, "value" => [1488772511.004, "0.000041021495238095323"] }]
-          }
-        })
-        expect(req_stub).to have_been_requested
+      context 'when response status code is 200' do
+        it 'returns response object' do
+          req_stub = stub_prometheus_request(query_url, body: prometheus_value_body('vector'))
+
+          response = subject.proxy('query', { query: prometheus_query })
+          json_response = JSON.parse(response.body)
+
+          expect(response.code).to eq(200)
+          expect(json_response).to eq({
+            'status' => 'success',
+            'data' => {
+              'resultType' => 'vector',
+              'result' => [{ "metric" => {}, "value" => [1488772511.004, "0.000041021495238095323"] }]
+            }
+          })
+          expect(req_stub).to have_been_requested
+        end
       end
-    end
 
-    context 'query_range' do
-      let(:prometheus_query) { prometheus_memory_query('env-slug') }
-      let(:query_url) { prometheus_query_range_url(prometheus_query, start: 2.hours.ago) }
+      context 'when response status code is not 200' do
+        it 'returns response object' do
+          req_stub = stub_prometheus_request(query_url, status: 400, body: { error: 'error' })
 
-      around do |example|
-        Timecop.freeze { example.run }
+          response = subject.proxy('query', { query: prometheus_query })
+          json_response = JSON.parse(response.body)
+
+          expect(req_stub).to have_been_requested
+          expect(response.code).to eq(400)
+          expect(json_response).to eq('error' => 'error')
+        end
       end
 
-      it 'returns full response' do
-        req_stub = stub_prometheus_request(query_url, body: prometheus_values_body('vector'))
+      context 'when RestClient::Exception is raised' do
+        before do
+          stub_prometheus_request_with_exception(query_url, RestClient::Exception)
+        end
 
-        response = subject.proxy('query_range', {
-          query: prometheus_query,
-          start: 2.hours.ago.to_f,
-          end: Time.now.to_f,
-          step: 60
-        })
-        json_response = JSON.parse(response.body)
-
-        expect(response.code).to eq(200)
-        expect(json_response).to eq({
-          "status" => "success",
-          "data" => {
-            "resultType" => "vector",
-            "result" => [{
-              "metric" => {},
-              "values" => [
-                [1488758662.506, "0.00002996364761904785"],
-                [1488758722.506, "0.00003090239047619091"]
-              ]
-            }]
-          }
-        })
-        expect(req_stub).to have_been_requested
+        it 'raises PrometheusClient::Error' do
+          expect { subject.proxy('query', { query: prometheus_query }) }.to(
+            raise_error(Gitlab::PrometheusClient::Error, 'Network connection error')
+          )
+        end
       end
     end
   end
-- 
cgit v1.2.1


From aa27c9491f58ec30e5e3afd035e1337694102a80 Mon Sep 17 00:00:00 2001
From: rpereira2 <rpereira@gitlab.com>
Date: Wed, 3 Apr 2019 00:06:13 +0530
Subject: Rename prometheus_owner to proxyable

- proxyable is a better name for any model object that has a prometheus
server to which requests can be proxied.
---
 app/services/prometheus/proxy_service.rb       | 26 +++++++++++++-------------
 spec/services/prometheus/proxy_service_spec.rb |  4 ++--
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/app/services/prometheus/proxy_service.rb b/app/services/prometheus/proxy_service.rb
index 55888fedb04..4a0d1dcd83f 100644
--- a/app/services/prometheus/proxy_service.rb
+++ b/app/services/prometheus/proxy_service.rb
@@ -10,30 +10,30 @@ module Prometheus
     self.reactive_cache_lifetime = 1.minute
     self.reactive_cache_worker_finder = ->(_id, *args) { from_cache(*args) }
 
-    attr_accessor :prometheus_owner, :method, :path, :params
+    attr_accessor :proxyable, :method, :path, :params
 
     PROXY_SUPPORT = {
       'query' => 'GET',
       'query_range' => 'GET'
     }.freeze
 
-    def self.from_cache(prometheus_owner_class_name, prometheus_owner_id, method, path, params)
-      prometheus_owner_class = begin
-        prometheus_owner_class_name.constantize
+    def self.from_cache(proxyable_class_name, proxyable_id, method, path, params)
+      proxyable_class = begin
+        proxyable_class_name.constantize
       rescue NameError
         nil
       end
-      return unless prometheus_owner_class
+      return unless proxyable_class
 
-      prometheus_owner = prometheus_owner_class.find(prometheus_owner_id)
+      proxyable = proxyable_class.find(proxyable_id)
 
-      new(prometheus_owner, method, path, params)
+      new(proxyable, method, path, params)
     end
 
-    # prometheus_owner can be any model which responds to .prometheus_adapter
+    # proxyable can be any model which responds to .prometheus_adapter
     # like Environment.
-    def initialize(prometheus_owner, method, path, params)
-      @prometheus_owner = prometheus_owner
+    def initialize(proxyable, method, path, params)
+      @proxyable = proxyable
       @path = path
       # Convert ActionController::Parameters to hash because reactive_cache_worker
       # does not play nice with ActionController::Parameters.
@@ -54,7 +54,7 @@ module Prometheus
       end
     end
 
-    def calculate_reactive_cache(prometheus_owner_class_name, prometheus_owner_id, method, path, params)
+    def calculate_reactive_cache(proxyable_class_name, proxyable_id, method, path, params)
       return no_prometheus_response unless can_query?
 
       response = prometheus_client_wrapper.proxy(path, params)
@@ -65,7 +65,7 @@ module Prometheus
     end
 
     def cache_key
-      [@prometheus_owner.class.name, @prometheus_owner.id, @method, @path, @params]
+      [@proxyable.class.name, @proxyable.id, @method, @path, @params]
     end
 
     private
@@ -83,7 +83,7 @@ module Prometheus
     end
 
     def prometheus_adapter
-      @prometheus_adapter ||= @prometheus_owner.prometheus_adapter
+      @prometheus_adapter ||= @proxyable.prometheus_adapter
     end
 
     def prometheus_client_wrapper
diff --git a/spec/services/prometheus/proxy_service_spec.rb b/spec/services/prometheus/proxy_service_spec.rb
index 1b98c68f99d..c271fc586c1 100644
--- a/spec/services/prometheus/proxy_service_spec.rb
+++ b/spec/services/prometheus/proxy_service_spec.rb
@@ -14,7 +14,7 @@ describe Prometheus::ProxyService do
     it 'initializes attributes' do
       result = described_class.new(environment, 'GET', 'query', { query: '1' })
 
-      expect(result.prometheus_owner).to eq(environment)
+      expect(result.proxyable).to eq(environment)
       expect(result.method).to eq('GET')
       expect(result.path).to eq('query')
       expect(result.params).to eq(query: '1')
@@ -175,7 +175,7 @@ describe Prometheus::ProxyService do
       result = described_class.from_cache(environment.class.name, environment.id, 'GET', 'query', { query: '1' })
 
       expect(result).to be_an_instance_of(described_class)
-      expect(result.prometheus_owner).to eq(environment)
+      expect(result.proxyable).to eq(environment)
       expect(result.method).to eq('GET')
       expect(result.path).to eq('query')
       expect(result.params).to eq(query: '1')
-- 
cgit v1.2.1


From b3aba1203d7f6a0c96c836593f19165bbf066cd0 Mon Sep 17 00:00:00 2001
From: rpereira2 <rpereira@gitlab.com>
Date: Wed, 3 Apr 2019 14:50:48 +0530
Subject: Filter params based on the proxy_path

- Permit params in ProxyService class to avoid having to make changes
to both ProxyService and to PrometheusApiController when adding support
for more prometheus apis.
- Also refactor the cache specs.
---
 app/services/prometheus/proxy_service.rb       | 24 +++++++--
 spec/services/prometheus/proxy_service_spec.rb | 67 +++++++++++++++-----------
 2 files changed, 59 insertions(+), 32 deletions(-)

diff --git a/app/services/prometheus/proxy_service.rb b/app/services/prometheus/proxy_service.rb
index 4a0d1dcd83f..c08adfda851 100644
--- a/app/services/prometheus/proxy_service.rb
+++ b/app/services/prometheus/proxy_service.rb
@@ -13,8 +13,14 @@ module Prometheus
     attr_accessor :proxyable, :method, :path, :params
 
     PROXY_SUPPORT = {
-      'query' => 'GET',
-      'query_range' => 'GET'
+      'query' => {
+        method: ['GET'],
+        params: [:query, :time, :timeout]
+      },
+      'query_range' => {
+        method: ['GET'],
+        params: [:query, :start, :end, :step, :timeout]
+      }
     }.freeze
 
     def self.from_cache(proxyable_class_name, proxyable_id, method, path, params)
@@ -35,9 +41,11 @@ module Prometheus
     def initialize(proxyable, method, path, params)
       @proxyable = proxyable
       @path = path
+
       # Convert ActionController::Parameters to hash because reactive_cache_worker
       # does not play nice with ActionController::Parameters.
-      @params = params.to_hash
+      @params = permit_params(params, path).to_hash
+
       @method = method
     end
 
@@ -94,8 +102,16 @@ module Prometheus
       prometheus_adapter&.can_query?
     end
 
+    def permit_params(params, path)
+      if params.is_a?(ActionController::Parameters)
+        params.permit(PROXY_SUPPORT.dig(path, :params))
+      else
+        params
+      end
+    end
+
     def can_proxy?
-      PROXY_SUPPORT[@path] == @method
+      PROXY_SUPPORT.dig(@path, :method)&.include?(@method)
     end
   end
 end
diff --git a/spec/services/prometheus/proxy_service_spec.rb b/spec/services/prometheus/proxy_service_spec.rb
index c271fc586c1..8de5f491427 100644
--- a/spec/services/prometheus/proxy_service_spec.rb
+++ b/spec/services/prometheus/proxy_service_spec.rb
@@ -9,7 +9,7 @@ describe Prometheus::ProxyService do
   set(:environment) { create(:environment, project: project) }
 
   describe '#initialize' do
-    let(:params) { ActionController::Parameters.new(query: '1').permit! }
+    let(:params) { ActionController::Parameters.new(query: '1') }
 
     it 'initializes attributes' do
       result = described_class.new(environment, 'GET', 'query', { query: '1' })
@@ -25,12 +25,23 @@ describe Prometheus::ProxyService do
 
       expect(result.params).to be_an_instance_of(Hash)
     end
+
+    context 'with unknown params' do
+      let(:params) { ActionController::Parameters.new(query: '1', other_param: 'val') }
+
+      it 'filters unknown params' do
+        result = described_class.new(environment, 'GET', 'query', params)
+
+        expect(result.params).to eq('query' => '1')
+      end
+    end
   end
 
   describe '#execute' do
     let(:prometheus_adapter) { instance_double(PrometheusService) }
+    let(:params) { ActionController::Parameters.new(query: '1') }
 
-    subject { described_class.new(environment, 'GET', 'query', { query: '1' }) }
+    subject { described_class.new(environment, 'GET', 'query', params) }
 
     context 'when prometheus_adapter is nil' do
       before do
@@ -62,7 +73,7 @@ describe Prometheus::ProxyService do
     end
 
     context 'cannot proxy' do
-      subject { described_class.new(environment, 'POST', 'query', { query: '1' }) }
+      subject { described_class.new(environment, 'POST', 'garbage', params) }
 
       it 'returns error' do
         expect(subject.execute).to eq(
@@ -72,44 +83,42 @@ describe Prometheus::ProxyService do
       end
     end
 
-    context 'when cached', :use_clean_rails_memory_store_caching do
+    context 'with caching', :use_clean_rails_memory_store_caching do
       let(:return_value) { { 'http_status' => 200, 'body' => 'body' } }
-      let(:opts) { [environment.class.name, environment.id, 'GET', 'query', { query: '1' }] }
 
-      before do
-        stub_reactive_cache(subject, return_value, opts)
+      let(:opts) do
+        [environment.class.name, environment.id, 'GET', 'query', { 'query' => '1' }]
+      end
 
+      before do
         allow(environment).to receive(:prometheus_adapter)
           .and_return(prometheus_adapter)
         allow(prometheus_adapter).to receive(:can_query?).and_return(true)
       end
 
-      it 'returns cached value' do
-        result = subject.execute
-
-        expect(result[:http_status]).to eq(return_value[:http_status])
-        expect(result[:body]).to eq(return_value[:body])
-      end
-    end
+      context 'when value present in cache' do
+        before do
+          stub_reactive_cache(subject, return_value, opts)
+        end
 
-    context 'when not cached' do
-      let(:return_value) { { 'http_status' => 200, 'body' => 'body' } }
-      let(:opts) { [environment.class.name, environment.id, 'GET', 'query', { query: '1' }] }
+        it 'returns cached value' do
+          result = subject.execute
 
-      before do
-        allow(environment).to receive(:prometheus_adapter)
-          .and_return(prometheus_adapter)
-        allow(prometheus_adapter).to receive(:can_query?).and_return(true)
+          expect(result[:http_status]).to eq(return_value[:http_status])
+          expect(result[:body]).to eq(return_value[:body])
+        end
       end
 
-      it 'returns nil' do
-        expect(ReactiveCachingWorker)
-          .to receive(:perform_async)
-          .with(subject.class, subject.id, *opts)
+      context 'when value not present in cache' do
+        it 'returns nil' do
+          expect(ReactiveCachingWorker)
+            .to receive(:perform_async)
+            .with(subject.class, subject.id, *opts)
 
-        result = subject.execute
+          result = subject.execute
 
-        expect(result).to eq(nil)
+          expect(result).to eq(nil)
+        end
       end
     end
 
@@ -172,7 +181,9 @@ describe Prometheus::ProxyService do
 
   describe '.from_cache' do
     it 'initializes an instance of ProxyService class' do
-      result = described_class.from_cache(environment.class.name, environment.id, 'GET', 'query', { query: '1' })
+      result = described_class.from_cache(
+        environment.class.name, environment.id, 'GET', 'query', { query: '1' }
+      )
 
       expect(result).to be_an_instance_of(described_class)
       expect(result.proxyable).to eq(environment)
-- 
cgit v1.2.1


From 833206abf833b0c43e35d97b3f2fea98d91d5572 Mon Sep 17 00:00:00 2001
From: rpereira2 <rpereira@gitlab.com>
Date: Wed, 3 Apr 2019 18:24:52 +0530
Subject: Use hash methods to filter params

- This is so that we don't have to check that params is of type
ActionController::Parameters in ProxyService.
---
 app/services/prometheus/proxy_service.rb       | 14 +++++---------
 spec/services/prometheus/proxy_service_spec.rb | 14 +++++++-------
 2 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/app/services/prometheus/proxy_service.rb b/app/services/prometheus/proxy_service.rb
index c08adfda851..cadfe59b900 100644
--- a/app/services/prometheus/proxy_service.rb
+++ b/app/services/prometheus/proxy_service.rb
@@ -15,11 +15,11 @@ module Prometheus
     PROXY_SUPPORT = {
       'query' => {
         method: ['GET'],
-        params: [:query, :time, :timeout]
+        params: %w(query time timeout)
       },
       'query_range' => {
         method: ['GET'],
-        params: [:query, :start, :end, :step, :timeout]
+        params: %w(query start end step timeout)
       }
     }.freeze
 
@@ -44,7 +44,7 @@ module Prometheus
 
       # Convert ActionController::Parameters to hash because reactive_cache_worker
       # does not play nice with ActionController::Parameters.
-      @params = permit_params(params, path).to_hash
+      @params = filter_params(params, path).to_hash
 
       @method = method
     end
@@ -102,12 +102,8 @@ module Prometheus
       prometheus_adapter&.can_query?
     end
 
-    def permit_params(params, path)
-      if params.is_a?(ActionController::Parameters)
-        params.permit(PROXY_SUPPORT.dig(path, :params))
-      else
-        params
-      end
+    def filter_params(params, path)
+      params.slice(*PROXY_SUPPORT.dig(path, :params))
     end
 
     def can_proxy?
diff --git a/spec/services/prometheus/proxy_service_spec.rb b/spec/services/prometheus/proxy_service_spec.rb
index 8de5f491427..3e2eea12d95 100644
--- a/spec/services/prometheus/proxy_service_spec.rb
+++ b/spec/services/prometheus/proxy_service_spec.rb
@@ -9,15 +9,15 @@ describe Prometheus::ProxyService do
   set(:environment) { create(:environment, project: project) }
 
   describe '#initialize' do
-    let(:params) { ActionController::Parameters.new(query: '1') }
+    let(:params) { ActionController::Parameters.new(query: '1').permit! }
 
     it 'initializes attributes' do
-      result = described_class.new(environment, 'GET', 'query', { query: '1' })
+      result = described_class.new(environment, 'GET', 'query', params)
 
       expect(result.proxyable).to eq(environment)
       expect(result.method).to eq('GET')
       expect(result.path).to eq('query')
-      expect(result.params).to eq(query: '1')
+      expect(result.params).to eq('query' => '1')
     end
 
     it 'converts ActionController::Parameters into hash' do
@@ -27,7 +27,7 @@ describe Prometheus::ProxyService do
     end
 
     context 'with unknown params' do
-      let(:params) { ActionController::Parameters.new(query: '1', other_param: 'val') }
+      let(:params) { ActionController::Parameters.new(query: '1', other_param: 'val').permit! }
 
       it 'filters unknown params' do
         result = described_class.new(environment, 'GET', 'query', params)
@@ -39,7 +39,7 @@ describe Prometheus::ProxyService do
 
   describe '#execute' do
     let(:prometheus_adapter) { instance_double(PrometheusService) }
-    let(:params) { ActionController::Parameters.new(query: '1') }
+    let(:params) { ActionController::Parameters.new(query: '1').permit! }
 
     subject { described_class.new(environment, 'GET', 'query', params) }
 
@@ -182,14 +182,14 @@ describe Prometheus::ProxyService do
   describe '.from_cache' do
     it 'initializes an instance of ProxyService class' do
       result = described_class.from_cache(
-        environment.class.name, environment.id, 'GET', 'query', { query: '1' }
+        environment.class.name, environment.id, 'GET', 'query', { 'query' => '1' }
       )
 
       expect(result).to be_an_instance_of(described_class)
       expect(result.proxyable).to eq(environment)
       expect(result.method).to eq('GET')
       expect(result.path).to eq('query')
-      expect(result.params).to eq(query: '1')
+      expect(result.params).to eq('query' => '1')
     end
   end
 end
-- 
cgit v1.2.1


From 5dd6e752c0c3d572c645ac56a79c6c5a3fbd5157 Mon Sep 17 00:00:00 2001
From: rpereira2 <rpereira@gitlab.com>
Date: Thu, 4 Apr 2019 16:14:08 +0530
Subject: Some code improvements

- Use strong_memoize for prometheus_adapter since it can be nil in some
cases.
- Do not phrase spec descriptions with 'should'.
---
 app/services/prometheus/proxy_service.rb       | 5 ++++-
 spec/services/prometheus/proxy_service_spec.rb | 4 ++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/app/services/prometheus/proxy_service.rb b/app/services/prometheus/proxy_service.rb
index cadfe59b900..c5d2b84878b 100644
--- a/app/services/prometheus/proxy_service.rb
+++ b/app/services/prometheus/proxy_service.rb
@@ -3,6 +3,7 @@
 module Prometheus
   class ProxyService < BaseService
     include ReactiveCaching
+    include Gitlab::Utils::StrongMemoize
 
     self.reactive_cache_key = ->(service) { service.cache_key }
     self.reactive_cache_lease_timeout = 30.seconds
@@ -91,7 +92,9 @@ module Prometheus
     end
 
     def prometheus_adapter
-      @prometheus_adapter ||= @proxyable.prometheus_adapter
+      strong_memoize(:prometheus_adapter) do
+        @proxyable.prometheus_adapter
+      end
     end
 
     def prometheus_client_wrapper
diff --git a/spec/services/prometheus/proxy_service_spec.rb b/spec/services/prometheus/proxy_service_spec.rb
index 3e2eea12d95..4bdb20de4c9 100644
--- a/spec/services/prometheus/proxy_service_spec.rb
+++ b/spec/services/prometheus/proxy_service_spec.rb
@@ -48,7 +48,7 @@ describe Prometheus::ProxyService do
         allow(environment).to receive(:prometheus_adapter).and_return(nil)
       end
 
-      it 'should return error' do
+      it 'returns error' do
         expect(subject.execute).to eq(
           status: :error,
           message: 'No prometheus server found',
@@ -63,7 +63,7 @@ describe Prometheus::ProxyService do
         allow(prometheus_adapter).to receive(:can_query?).and_return(false)
       end
 
-      it 'should return error' do
+      it 'returns error' do
         expect(subject.execute).to eq(
           status: :error,
           message: 'No prometheus server found',
-- 
cgit v1.2.1


From e1a167ee234fbb8886ac1a882002b76adeb67edf Mon Sep 17 00:00:00 2001
From: rpereira2 <rpereira@gitlab.com>
Date: Tue, 2 Apr 2019 11:56:09 +0530
Subject: Add a Prometheus API per environment

The api will proxy requests to the environment's prometheus server.
The Prometheus::ProxyService class can be reused when we add support for
group prometheus servers.
---
 .../environments/prometheus_api_controller.rb      |  47 +++++++
 app/policies/project_policy.rb                     |   1 +
 changelogs/unreleased/58375-api-controller.yml     |   5 +
 config/routes/project.rb                           |   2 +
 .../environments/prometheus_api_controller_spec.rb | 147 +++++++++++++++++++++
 .../policies/project_policy_shared_context.rb      |   1 +
 6 files changed, 203 insertions(+)
 create mode 100644 app/controllers/projects/environments/prometheus_api_controller.rb
 create mode 100644 changelogs/unreleased/58375-api-controller.yml
 create mode 100644 spec/controllers/projects/environments/prometheus_api_controller_spec.rb

diff --git a/app/controllers/projects/environments/prometheus_api_controller.rb b/app/controllers/projects/environments/prometheus_api_controller.rb
new file mode 100644
index 00000000000..337b7fdf65d
--- /dev/null
+++ b/app/controllers/projects/environments/prometheus_api_controller.rb
@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+class Projects::Environments::PrometheusApiController < Projects::ApplicationController
+  before_action :authorize_read_prometheus!
+  before_action :environment
+
+  def proxy
+    permitted = permit_params
+
+    result = Prometheus::ProxyService.new(
+      environment,
+      request.method,
+      permitted[:proxy_path],
+      permitted.except(:proxy_path) # rubocop: disable CodeReuse/ActiveRecord
+    ).execute
+
+    if result.nil?
+      render status: :accepted, json: {
+        status: 'processing',
+        message: 'Not ready yet. Try again later.'
+      }
+      return
+    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
+
+  private
+
+  def permit_params
+    params.permit([
+      :proxy_path, :query, :time, :timeout, :start, :end, :step, { match: [] },
+      :match_target, :metric, :limit
+    ])
+  end
+
+  def environment
+    @environment ||= project.environments.find(params[:id])
+  end
+end
diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index 75825c8fac0..26d7d6e84c4 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -204,6 +204,7 @@ class ProjectPolicy < BasePolicy
     enable :read_merge_request
     enable :read_sentry_issue
     enable :read_release
+    enable :read_prometheus
   end
 
   # We define `:public_user_access` separately because there are cases in gitlab-ee
diff --git a/changelogs/unreleased/58375-api-controller.yml b/changelogs/unreleased/58375-api-controller.yml
new file mode 100644
index 00000000000..60f21b37ae7
--- /dev/null
+++ b/changelogs/unreleased/58375-api-controller.yml
@@ -0,0 +1,5 @@
+---
+title: Add a Prometheus API per environment
+merge_request: 26841
+author:
+type: added
diff --git a/config/routes/project.rb b/config/routes/project.rb
index d60a5cc9ae8..1cb8f331f6f 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -219,6 +219,8 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
           get :metrics
           get :additional_metrics
           get '/terminal.ws/authorize', to: 'environments#terminal_websocket_authorize', constraints: { format: nil }
+
+          get '/prometheus/api/v1/*proxy_path', to: 'environments/prometheus_api#proxy'
         end
 
         collection do
diff --git a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb
new file mode 100644
index 00000000000..d943d006ae1
--- /dev/null
+++ b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb
@@ -0,0 +1,147 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Projects::Environments::PrometheusApiController do
+  set(:project) { create(:project) }
+  set(:environment) { create(:environment, project: project) }
+  set(:user) { create(:user) }
+
+  before do
+    project.add_reporter(user)
+    sign_in(user)
+  end
+
+  describe 'GET #proxy' do
+    let(:prometheus_proxy_service) { instance_double(Prometheus::ProxyService) }
+    let(:prometheus_response) { { status: :success, body: response_body } }
+    let(:json_response_body) { JSON.parse(response_body) }
+
+    let(:response_body) do
+      "{\"status\":\"success\",\"data\":{\"resultType\":\"scalar\",\"result\":[1553864609.117,\"1\"]}}"
+    end
+
+    before do
+      allow(Prometheus::ProxyService).to receive(:new)
+        .with(environment, 'GET', 'query', anything)
+        .and_return(prometheus_proxy_service)
+
+      allow(prometheus_proxy_service).to receive(:execute)
+        .and_return(prometheus_response)
+    end
+
+    it 'returns prometheus response' do
+      get :proxy, params: environment_params
+
+      expect(response).to have_gitlab_http_status(:ok)
+      expect(json_response).to eq(json_response_body)
+    end
+
+    it 'filters params' do
+      get :proxy, params: environment_params({ extra_param: 'dangerous value' })
+
+      expect(Prometheus::ProxyService).to have_received(:new)
+        .with(environment, 'GET', 'query', ActionController::Parameters.new({ 'query' => '1' }).permit!)
+    end
+
+    context 'Prometheus::ProxyService returns nil' do
+      before do
+        allow(prometheus_proxy_service).to receive(:execute)
+          .and_return(nil)
+      end
+
+      it 'returns 202 accepted' 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(:accepted)
+      end
+    end
+
+    context 'Prometheus::ProxyService returns status success' do
+      let(:service_response) { { http_status: 404, status: :success, body: '{"body": "value"}' } }
+
+      before do
+        allow(prometheus_proxy_service).to receive(:execute)
+          .and_return(service_response)
+      end
+
+      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 'Prometheus::ProxyService returns status error' do
+      before do
+        allow(prometheus_proxy_service).to receive(:execute)
+          .and_return(service_response)
+      end
+
+      context 'with http_status' do
+        let(:service_response) 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_response) { { status: :error, message: 'error message' } }
+
+        it 'returns message' 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
+
+    context 'with anonymous user' do
+      before do
+        sign_out(user)
+      end
+
+      it 'redirects to signin page' do
+        get :proxy, params: environment_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
+
+  private
+
+  def environment_params(params = {})
+    {
+      id: environment.id,
+      namespace_id: project.namespace,
+      project_id: project,
+      proxy_path: 'query',
+      query: '1'
+    }.merge(params)
+  end
+end
diff --git a/spec/support/shared_context/policies/project_policy_shared_context.rb b/spec/support/shared_context/policies/project_policy_shared_context.rb
index 3ad6e067674..ee5cfcd850d 100644
--- a/spec/support/shared_context/policies/project_policy_shared_context.rb
+++ b/spec/support/shared_context/policies/project_policy_shared_context.rb
@@ -25,6 +25,7 @@ RSpec.shared_context 'ProjectPolicy context' do
       admin_issue admin_label admin_list read_commit_status read_build
       read_container_image read_pipeline read_environment read_deployment
       read_merge_request download_wiki_code read_sentry_issue read_release
+      read_prometheus
     ]
   end
 
-- 
cgit v1.2.1


From 1427ad4f5ce11d8327d9258eb3ffc62803c1a304 Mon Sep 17 00:00:00 2001
From: Peter Leitzen <pleitzen@gitlab.com>
Date: Tue, 2 Apr 2019 18:38:49 +0200
Subject: Streamline controller specs

---
 .../environments/prometheus_api_controller.rb      |   6 +-
 .../environments/prometheus_api_controller_spec.rb | 157 ++++++++++-----------
 2 files changed, 76 insertions(+), 87 deletions(-)

diff --git a/app/controllers/projects/environments/prometheus_api_controller.rb b/app/controllers/projects/environments/prometheus_api_controller.rb
index 337b7fdf65d..aace846d68c 100644
--- a/app/controllers/projects/environments/prometheus_api_controller.rb
+++ b/app/controllers/projects/environments/prometheus_api_controller.rb
@@ -25,10 +25,8 @@ class Projects::Environments::PrometheusApiController < Projects::ApplicationCon
     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]
-        }
+      render status: result[:http_status] || :bad_request,
+        json: { status: result[:status], message: result[:message] }
     end
   end
 
diff --git a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb
index d943d006ae1..b1b68bb42d8 100644
--- a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb
+++ b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb
@@ -14,121 +14,112 @@ describe Projects::Environments::PrometheusApiController do
 
   describe 'GET #proxy' do
     let(:prometheus_proxy_service) { instance_double(Prometheus::ProxyService) }
-    let(:prometheus_response) { { status: :success, body: response_body } }
-    let(:json_response_body) { JSON.parse(response_body) }
 
-    let(:response_body) do
-      "{\"status\":\"success\",\"data\":{\"resultType\":\"scalar\",\"result\":[1553864609.117,\"1\"]}}"
-    end
-
-    before do
-      allow(Prometheus::ProxyService).to receive(:new)
-        .with(environment, 'GET', 'query', anything)
-        .and_return(prometheus_proxy_service)
+    context 'with valid requests' do
+      before do
+        allow(Prometheus::ProxyService).to receive(:new)
+          .with(environment, 'GET', 'query', anything)
+          .and_return(prometheus_proxy_service)
 
-      allow(prometheus_proxy_service).to receive(:execute)
-        .and_return(prometheus_response)
-    end
+        allow(prometheus_proxy_service).to receive(:execute)
+          .and_return(service_result)
+      end
 
-    it 'returns prometheus response' do
-      get :proxy, params: environment_params
+      context 'with success result' do
+        let(:service_result) { { status: :success, body: prometheus_body } }
+        let(:prometheus_body) { '{"status":"success"}' }
+        let(:prometheus_json_body) { JSON.parse(prometheus_body) }
 
-      expect(response).to have_gitlab_http_status(:ok)
-      expect(json_response).to eq(json_response_body)
-    end
+        it 'returns prometheus response' do
+          get :proxy, params: environment_params
 
-    it 'filters params' do
-      get :proxy, params: environment_params({ extra_param: 'dangerous value' })
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response).to eq(prometheus_json_body)
+        end
 
-      expect(Prometheus::ProxyService).to have_received(:new)
-        .with(environment, 'GET', 'query', ActionController::Parameters.new({ 'query' => '1' }).permit!)
-    end
+        it 'filters params' do
+          get :proxy, params: environment_params({ extra_param: 'dangerous value' })
 
-    context 'Prometheus::ProxyService returns nil' do
-      before do
-        allow(prometheus_proxy_service).to receive(:execute)
-          .and_return(nil)
+          expect(Prometheus::ProxyService).to have_received(:new)
+            .with(environment, 'GET', 'query', ActionController::Parameters.new({ 'query' => '1' }).permit!)
+        end
       end
 
-      it 'returns 202 accepted' 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(:accepted)
-      end
-    end
+      context 'with nil result' do
+        let(:service_result) { nil }
 
-    context 'Prometheus::ProxyService returns status success' do
-      let(:service_response) { { http_status: 404, status: :success, body: '{"body": "value"}' } }
+        it 'returns 202 accepted' do
+          get :proxy, params: environment_params
 
-      before do
-        allow(prometheus_proxy_service).to receive(:execute)
-          .and_return(service_response)
+          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(:accepted)
+        end
       end
 
-      it 'returns body' do
-        get :proxy, params: environment_params
+      context 'with 404 result' do
+        let(:service_result) { { http_status: 404, status: :success, body: '{"body": "value"}' } }
 
-        expect(response).to have_gitlab_http_status(:not_found)
-        expect(json_response['body']).to eq('value')
-      end
-    end
+        it 'returns body' do
+          get :proxy, params: environment_params
 
-    context 'Prometheus::ProxyService returns status error' do
-      before do
-        allow(prometheus_proxy_service).to receive(:execute)
-          .and_return(service_response)
+          expect(response).to have_gitlab_http_status(:not_found)
+          expect(json_response['body']).to eq('value')
+        end
       end
 
-      context 'with http_status' do
-        let(:service_response) do
-          { http_status: :service_unavailable, status: :error, message: 'error message' }
-        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
+          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')
+            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
-      end
 
-      context 'without http_status' do
-        let(:service_response) { { status: :error, message: 'error message' } }
+        context 'without http_status' do
+          let(:service_result) { { status: :error, message: 'error message' } }
 
-        it 'returns message' do
-          get :proxy, params: environment_params
+          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')
+            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 anonymous user' do
-      before do
-        sign_out(user)
-      end
+    context 'with inappropriate requests' do
+      context 'with anonymous user' do
+        before do
+          sign_out(user)
+        end
 
-      it 'redirects to signin page' do
-        get :proxy, params: environment_params
+        it 'redirects to signin page' do
+          get :proxy, params: environment_params
 
-        expect(response).to redirect_to(new_user_session_path)
+          expect(response).to redirect_to(new_user_session_path)
+        end
       end
-    end
 
-    context 'without correct permissions' do
-      before do
-        project.team.truncate
-      end
+      context 'without correct permissions' do
+        before do
+          project.team.truncate
+        end
 
-      it 'returns 404' do
-        get :proxy, params: environment_params
+        it 'returns 404' do
+          get :proxy, params: environment_params
 
-        expect(response).to have_gitlab_http_status(:not_found)
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
       end
     end
   end
@@ -142,6 +133,6 @@ describe Projects::Environments::PrometheusApiController do
       project_id: project,
       proxy_path: 'query',
       query: '1'
-    }.merge(params)
+    }.reverse_merge(params)
   end
 end
-- 
cgit v1.2.1


From 4a0c8b4a9c25257d9f0addfbc603f2bac80b63db Mon Sep 17 00:00:00 2001
From: Peter Leitzen <pleitzen@gitlab.com>
Date: Wed, 3 Apr 2019 08:42:12 +0200
Subject: Make filter params specs more readable

---
 .../projects/environments/prometheus_api_controller_spec.rb  | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb
index b1b68bb42d8..d9ff420b682 100644
--- a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb
+++ b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb
@@ -37,11 +37,13 @@ describe Projects::Environments::PrometheusApiController do
           expect(json_response).to eq(prometheus_json_body)
         end
 
-        it 'filters params' do
-          get :proxy, params: environment_params({ extra_param: 'dangerous value' })
+        it 'filters unknown params' do
+          get :proxy, params: environment_params(unknown_param: 'value')
 
-          expect(Prometheus::ProxyService).to have_received(:new)
-            .with(environment, 'GET', 'query', ActionController::Parameters.new({ 'query' => '1' }).permit!)
+          params = ActionController::Parameters.new('query' => '1').permit!
+          expect(Prometheus::ProxyService)
+            .to have_received(:new)
+            .with(environment, 'GET', 'query', params)
         end
       end
 
@@ -133,6 +135,6 @@ describe Projects::Environments::PrometheusApiController do
       project_id: project,
       proxy_path: 'query',
       query: '1'
-    }.reverse_merge(params)
+    }.merge(params)
   end
 end
-- 
cgit v1.2.1


From eac3e2302c4155ed1890e39b7124672d79b795b6 Mon Sep 17 00:00:00 2001
From: rpereira2 <rpereira@gitlab.com>
Date: Wed, 3 Apr 2019 13:32:45 +0530
Subject: Add spec for invalid environment id

---
 .../projects/environments/prometheus_api_controller.rb         |  3 +--
 .../projects/environments/prometheus_api_controller_spec.rb    | 10 ++++++++++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/app/controllers/projects/environments/prometheus_api_controller.rb b/app/controllers/projects/environments/prometheus_api_controller.rb
index aace846d68c..bbd937981bb 100644
--- a/app/controllers/projects/environments/prometheus_api_controller.rb
+++ b/app/controllers/projects/environments/prometheus_api_controller.rb
@@ -15,11 +15,10 @@ class Projects::Environments::PrometheusApiController < Projects::ApplicationCon
     ).execute
 
     if result.nil?
-      render status: :accepted, json: {
+      return render status: :accepted, json: {
         status: 'processing',
         message: 'Not ready yet. Try again later.'
       }
-      return
     end
 
     if result[:status] == :success
diff --git a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb
index d9ff420b682..2cabb3f75d2 100644
--- a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb
+++ b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb
@@ -124,6 +124,16 @@ describe Projects::Environments::PrometheusApiController do
         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
-- 
cgit v1.2.1


From 16772b91f0b71b47c8e72bfa2d11693aed58e3ba Mon Sep 17 00:00:00 2001
From: rpereira2 <rpereira@gitlab.com>
Date: Wed, 3 Apr 2019 15:01:44 +0530
Subject: Remove permitting of params

- It is now being done in ProxyService class.
---
 .../projects/environments/prometheus_api_controller.rb  | 17 +++++------------
 .../environments/prometheus_api_controller_spec.rb      |  9 ---------
 2 files changed, 5 insertions(+), 21 deletions(-)

diff --git a/app/controllers/projects/environments/prometheus_api_controller.rb b/app/controllers/projects/environments/prometheus_api_controller.rb
index bbd937981bb..0ce97706792 100644
--- a/app/controllers/projects/environments/prometheus_api_controller.rb
+++ b/app/controllers/projects/environments/prometheus_api_controller.rb
@@ -5,13 +5,11 @@ class Projects::Environments::PrometheusApiController < Projects::ApplicationCon
   before_action :environment
 
   def proxy
-    permitted = permit_params
-
     result = Prometheus::ProxyService.new(
       environment,
       request.method,
-      permitted[:proxy_path],
-      permitted.except(:proxy_path) # rubocop: disable CodeReuse/ActiveRecord
+      params[:proxy_path],
+      params
     ).execute
 
     if result.nil?
@@ -24,20 +22,15 @@ class Projects::Environments::PrometheusApiController < Projects::ApplicationCon
     if result[:status] == :success
       render status: result[:http_status], json: result[:body]
     else
-      render status: result[:http_status] || :bad_request,
+      render(
+        status: result[:http_status] || :bad_request,
         json: { status: result[:status], message: result[:message] }
+      )
     end
   end
 
   private
 
-  def permit_params
-    params.permit([
-      :proxy_path, :query, :time, :timeout, :start, :end, :step, { match: [] },
-      :match_target, :metric, :limit
-    ])
-  end
-
   def environment
     @environment ||= project.environments.find(params[:id])
   end
diff --git a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb
index 2cabb3f75d2..f1ac127fc9e 100644
--- a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb
+++ b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb
@@ -36,15 +36,6 @@ describe Projects::Environments::PrometheusApiController do
           expect(response).to have_gitlab_http_status(:ok)
           expect(json_response).to eq(prometheus_json_body)
         end
-
-        it 'filters unknown params' do
-          get :proxy, params: environment_params(unknown_param: 'value')
-
-          params = ActionController::Parameters.new('query' => '1').permit!
-          expect(Prometheus::ProxyService)
-            .to have_received(:new)
-            .with(environment, 'GET', 'query', params)
-        end
       end
 
       context 'with nil result' do
-- 
cgit v1.2.1


From 5ee7c419ea97f9322dcaf6da02c85f2ac25f477d Mon Sep 17 00:00:00 2001
From: rpereira2 <rpereira@gitlab.com>
Date: Wed, 3 Apr 2019 19:30:22 +0530
Subject: Call permit! on params

---
 .../projects/environments/prometheus_api_controller.rb    |  2 +-
 .../environments/prometheus_api_controller_spec.rb        | 15 ++++++++++++++-
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/app/controllers/projects/environments/prometheus_api_controller.rb b/app/controllers/projects/environments/prometheus_api_controller.rb
index 0ce97706792..13b5a300af6 100644
--- a/app/controllers/projects/environments/prometheus_api_controller.rb
+++ b/app/controllers/projects/environments/prometheus_api_controller.rb
@@ -9,7 +9,7 @@ class Projects::Environments::PrometheusApiController < Projects::ApplicationCon
       environment,
       request.method,
       params[:proxy_path],
-      params
+      params.permit!
     ).execute
 
     if result.nil?
diff --git a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb
index f1ac127fc9e..ee5202ac798 100644
--- a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb
+++ b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb
@@ -14,11 +14,22 @@ describe Projects::Environments::PrometheusApiController do
 
   describe 'GET #proxy' do
     let(:prometheus_proxy_service) { instance_double(Prometheus::ProxyService) }
+    let(:expected_params) do
+      ActionController::Parameters.new(
+        "query" => "1",
+        "id" => "1",
+        "namespace_id" => "namespace1",
+        "project_id" => "project1",
+        "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', anything)
+          .with(environment, 'GET', 'query', expected_params)
           .and_return(prometheus_proxy_service)
 
         allow(prometheus_proxy_service).to receive(:execute)
@@ -33,6 +44,8 @@ describe Projects::Environments::PrometheusApiController do
         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
-- 
cgit v1.2.1


From e89b07327166b2af8557d0f177264da25e4df0d3 Mon Sep 17 00:00:00 2001
From: rpereira2 <rpereira@gitlab.com>
Date: Thu, 4 Apr 2019 13:15:15 +0530
Subject: Do not hardcode project and namespace name in url

The names can change in different runs of the spec.
---
 .../projects/environments/prometheus_api_controller_spec.rb         | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb
index ee5202ac798..f0de211c09c 100644
--- a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb
+++ b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb
@@ -17,9 +17,9 @@ describe Projects::Environments::PrometheusApiController do
     let(:expected_params) do
       ActionController::Parameters.new(
         "query" => "1",
-        "id" => "1",
-        "namespace_id" => "namespace1",
-        "project_id" => "project1",
+        "id" => environment.id.to_s,
+        "namespace_id" => project.namespace.name,
+        "project_id" => project.name,
         "proxy_path" => "query",
         "controller" => "projects/environments/prometheus_api",
         "action" => "proxy"
-- 
cgit v1.2.1


From 3888b4d35c8e6e24d1871bbc4ec2e1f63a19ed4d Mon Sep 17 00:00:00 2001
From: rpereira2 <rpereira@gitlab.com>
Date: Thu, 4 Apr 2019 17:10:06 +0530
Subject: Allow message strings to be translated

---
 app/controllers/projects/environments/prometheus_api_controller.rb | 4 ++--
 locale/gitlab.pot                                                  | 6 ++++++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/app/controllers/projects/environments/prometheus_api_controller.rb b/app/controllers/projects/environments/prometheus_api_controller.rb
index 13b5a300af6..fd3320637b0 100644
--- a/app/controllers/projects/environments/prometheus_api_controller.rb
+++ b/app/controllers/projects/environments/prometheus_api_controller.rb
@@ -14,8 +14,8 @@ class Projects::Environments::PrometheusApiController < Projects::ApplicationCon
 
     if result.nil?
       return render status: :accepted, json: {
-        status: 'processing',
-        message: 'Not ready yet. Try again later.'
+        status: _('processing'),
+        message: _('Not ready yet. Try again later.')
       }
     end
 
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 3a140aef0e7..8f4e118b69f 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -5483,6 +5483,9 @@ msgstr ""
 msgid "Not now"
 msgstr ""
 
+msgid "Not ready yet. Try again later."
+msgstr ""
+
 msgid "Not started"
 msgstr ""
 
@@ -10040,6 +10043,9 @@ msgstr ""
 msgid "private"
 msgstr ""
 
+msgid "processing"
+msgstr ""
+
 msgid "project"
 msgstr ""
 
-- 
cgit v1.2.1


From 20594de8cc3311ddc53b29ffdf60bcf4a580a5b3 Mon Sep 17 00:00:00 2001
From: rpereira2 <rpereira@gitlab.com>
Date: Thu, 4 Apr 2019 19:16:45 +0530
Subject: Use environment_params when defining expected_params

---
 .../environments/prometheus_api_controller_spec.rb     | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb
index f0de211c09c..5a0b92c2514 100644
--- a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb
+++ b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb
@@ -16,13 +16,11 @@ describe Projects::Environments::PrometheusApiController do
     let(:prometheus_proxy_service) { instance_double(Prometheus::ProxyService) }
     let(:expected_params) do
       ActionController::Parameters.new(
-        "query" => "1",
-        "id" => environment.id.to_s,
-        "namespace_id" => project.namespace.name,
-        "project_id" => project.name,
-        "proxy_path" => "query",
-        "controller" => "projects/environments/prometheus_api",
-        "action" => "proxy"
+        environment_params(
+          proxy_path: 'query',
+          controller: 'projects/environments/prometheus_api',
+          action: 'proxy'
+        )
       ).permit!
     end
 
@@ -144,9 +142,9 @@ describe Projects::Environments::PrometheusApiController do
 
   def environment_params(params = {})
     {
-      id: environment.id,
-      namespace_id: project.namespace,
-      project_id: project,
+      id: environment.id.to_s,
+      namespace_id: project.namespace.name,
+      project_id: project.name,
       proxy_path: 'query',
       query: '1'
     }.merge(params)
-- 
cgit v1.2.1