diff options
-rw-r--r-- | app/services/prometheus/proxy_service.rb | 24 | ||||
-rw-r--r-- | 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) |