diff options
| -rw-r--r-- | Gemfile | 2 | ||||
| -rw-r--r-- | Gemfile.lock | 2 | ||||
| -rw-r--r-- | app/controllers/graphql_controller.rb | 1 | ||||
| -rw-r--r-- | app/services/prometheus/proxy_variable_substitution_service.rb | 55 | ||||
| -rw-r--r-- | app/services/template_engines/liquid_service.rb | 48 | ||||
| -rw-r--r-- | changelogs/unreleased/35242-add-liquid-template.yml | 5 | ||||
| -rw-r--r-- | doc/administration/monitoring/performance/gitlab_configuration.md | 4 | ||||
| -rw-r--r-- | doc/administration/monitoring/performance/index.md | 4 | ||||
| -rw-r--r-- | doc/administration/monitoring/performance/influxdb_configuration.md | 4 | ||||
| -rw-r--r-- | doc/administration/monitoring/performance/influxdb_schema.md | 4 | ||||
| -rw-r--r-- | doc/user/project/integrations/prometheus.md | 5 | ||||
| -rw-r--r-- | locale/gitlab.pot | 9 | ||||
| -rw-r--r-- | spec/controllers/projects/environments/prometheus_api_controller_spec.rb | 34 | ||||
| -rw-r--r-- | spec/services/prometheus/proxy_variable_substitution_service_spec.rb | 146 | ||||
| -rw-r--r-- | spec/services/template_engines/liquid_service_spec.rb | 126 |
15 files changed, 440 insertions, 9 deletions
@@ -477,3 +477,5 @@ gem 'gitlab-net-dns', '~> 0.9.1' gem 'countries', '~> 3.0' gem 'retriable', '~> 3.1.2' + +gem 'liquid', '~> 4.0' diff --git a/Gemfile.lock b/Gemfile.lock index 0e322705862..fb660e53765 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -577,6 +577,7 @@ GEM xml-simple licensee (8.9.2) rugged (~> 0.24) + liquid (4.0.3) listen (3.1.5) rb-fsevent (~> 0.9, >= 0.9.4) rb-inotify (~> 0.9, >= 0.9.7) @@ -1245,6 +1246,7 @@ DEPENDENCIES letter_opener_web (~> 1.3.4) license_finder (~> 5.4) licensee (~> 8.9) + liquid (~> 4.0) lograge (~> 0.5) loofah (~> 2.2) mail_room (~> 0.10.0) diff --git a/app/controllers/graphql_controller.rb b/app/controllers/graphql_controller.rb index 72d40f709e6..d7ff2ded5ae 100644 --- a/app/controllers/graphql_controller.rb +++ b/app/controllers/graphql_controller.rb @@ -3,6 +3,7 @@ class GraphqlController < ApplicationController # Unauthenticated users have access to the API for public data skip_before_action :authenticate_user! + skip_around_action :set_session_storage # Allow missing CSRF tokens, this would mean that if a CSRF is invalid or missing, # the user won't be authenticated but can proceed as an anonymous user. diff --git a/app/services/prometheus/proxy_variable_substitution_service.rb b/app/services/prometheus/proxy_variable_substitution_service.rb index ca56292e9d6..b34afaf80b8 100644 --- a/app/services/prometheus/proxy_variable_substitution_service.rb +++ b/app/services/prometheus/proxy_variable_substitution_service.rb @@ -4,7 +4,10 @@ module Prometheus class ProxyVariableSubstitutionService < BaseService include Stepable - steps :add_params_to_result, :substitute_ruby_variables + steps :validate_variables, + :add_params_to_result, + :substitute_ruby_variables, + :substitute_liquid_variables def initialize(environment, params = {}) @environment, @params = environment, params.deep_dup @@ -16,24 +19,45 @@ module Prometheus private + def validate_variables(_result) + return success unless variables + + unless variables.is_a?(Array) && variables.size.even? + return error(_('Optional parameter "variables" must be an array of keys and values. Ex: [key1, value1, key2, value2]')) + end + + success + end + def add_params_to_result(result) result[:params] = params success(result) end + def substitute_liquid_variables(result) + return success(result) unless query(result) + + result[:params][:query] = + TemplateEngines::LiquidService.new(query(result)).render(full_context) + + success(result) + rescue TemplateEngines::LiquidService::RenderError => e + error(e.message) + end + def substitute_ruby_variables(result) - return success(result) unless query + return success(result) unless query(result) # The % operator doesn't replace variables if the hash contains string # keys. - result[:params][:query] = query % predefined_context.symbolize_keys + result[:params][:query] = query(result) % predefined_context.symbolize_keys success(result) rescue TypeError, ArgumentError => exception log_error(exception.message) - Gitlab::ErrorTracking.track_exception(exception, extra: { - template_string: query, + Gitlab::ErrorTracking.track_exception(exception, { + template_string: query(result), variables: predefined_context }) @@ -44,8 +68,25 @@ module Prometheus @predefined_context ||= Gitlab::Prometheus::QueryVariables.call(@environment) end - def query - params[:query] + def full_context + @full_context ||= predefined_context.reverse_merge(variables_hash) + end + + def variables + params[:variables] + end + + def variables_hash + # .each_slice(2) converts ['key1', 'value1', 'key2', 'value2'] into + # [['key1', 'value1'], ['key2', 'value2']] which is then converted into + # a hash by to_h: {'key1' => 'value1', 'key2' => 'value2'} + # to_h will raise an ArgumentError if the number of elements in the original + # array is not even. + variables&.each_slice(2).to_h + end + + def query(result) + result[:params][:query] end end end diff --git a/app/services/template_engines/liquid_service.rb b/app/services/template_engines/liquid_service.rb new file mode 100644 index 00000000000..809ebd0316b --- /dev/null +++ b/app/services/template_engines/liquid_service.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +module TemplateEngines + class LiquidService < BaseService + RenderError = Class.new(StandardError) + + DEFAULT_RENDER_SCORE_LIMIT = 1_000 + + def initialize(string) + @template = Liquid::Template.parse(string) + end + + def render(context, render_score_limit: DEFAULT_RENDER_SCORE_LIMIT) + set_limits(render_score_limit) + + @template.render!(context.stringify_keys) + rescue Liquid::MemoryError => e + handle_exception(e, string: @string, context: context) + + raise RenderError, _('Memory limit exceeded while rendering template') + rescue Liquid::Error => e + handle_exception(e, string: @string, context: context) + + raise RenderError, _('Error rendering query') + end + + private + + def set_limits(render_score_limit) + @template.resource_limits.render_score_limit = render_score_limit + + # We can also set assign_score_limit and render_length_limit if required. + + # render_score_limit limits the number of nodes (string, variable, block, tags) + # that are allowed in the template. + # render_length_limit seems to limit the sum of the bytesize of all node blocks. + # assign_score_limit seems to limit the sum of the bytesize of all capture blocks. + end + + def handle_exception(exception, extra = {}) + log_error(exception.message) + Gitlab::ErrorTracking.track_exception(exception, { + template_string: extra[:string], + variables: extra[:context] + }) + end + end +end diff --git a/changelogs/unreleased/35242-add-liquid-template.yml b/changelogs/unreleased/35242-add-liquid-template.yml new file mode 100644 index 00000000000..34f3ad3e758 --- /dev/null +++ b/changelogs/unreleased/35242-add-liquid-template.yml @@ -0,0 +1,5 @@ +--- +title: Add support for Liquid format in Prometheus queries +merge_request: 20793 +author: +type: added diff --git a/doc/administration/monitoring/performance/gitlab_configuration.md b/doc/administration/monitoring/performance/gitlab_configuration.md index 9a25cc04ee7..8245c25ad25 100644 --- a/doc/administration/monitoring/performance/gitlab_configuration.md +++ b/doc/administration/monitoring/performance/gitlab_configuration.md @@ -1,5 +1,9 @@ # GitLab Configuration +CAUTION: **InfluxDB is deprecated in favor of Prometheus:** +InfluxDB support is scheduled to be removed in GitLab 13.0. +You are advised to use [Prometheus](../prometheus/index.md) instead. + GitLab Performance Monitoring is disabled by default. To enable it and change any of its settings, navigate to the Admin area in **Settings > Metrics** (`/admin/application_settings`). diff --git a/doc/administration/monitoring/performance/index.md b/doc/administration/monitoring/performance/index.md index 5204ab40dc9..6569f6a8c6d 100644 --- a/doc/administration/monitoring/performance/index.md +++ b/doc/administration/monitoring/performance/index.md @@ -1,5 +1,9 @@ # GitLab Performance Monitoring +CAUTION: **InfluxDB is deprecated in favor of Prometheus:** +InfluxDB support is scheduled to be removed in GitLab 13.0. +You are advised to use [Prometheus](../prometheus/index.md) instead. + GitLab comes with its own application performance measuring system as of GitLab 8.4, simply called "GitLab Performance Monitoring". GitLab Performance Monitoring is available in both the Community and Enterprise editions. diff --git a/doc/administration/monitoring/performance/influxdb_configuration.md b/doc/administration/monitoring/performance/influxdb_configuration.md index f1f588a924d..aaea0a5968d 100644 --- a/doc/administration/monitoring/performance/influxdb_configuration.md +++ b/doc/administration/monitoring/performance/influxdb_configuration.md @@ -1,5 +1,9 @@ # InfluxDB Configuration +CAUTION: **InfluxDB is being deprecated in favor of Prometheus:** +InfluxDB support is scheduled to be dropped in GitLab 13.0. +You are advised to use [Prometheus](../prometheus/index.md) instead. + The default settings provided by [InfluxDB] are not sufficient for a high traffic GitLab environment. The settings discussed in this document are based on the settings GitLab uses for GitLab.com, depending on your own needs you may need to diff --git a/doc/administration/monitoring/performance/influxdb_schema.md b/doc/administration/monitoring/performance/influxdb_schema.md index eff0e29f58d..71814deb2bc 100644 --- a/doc/administration/monitoring/performance/influxdb_schema.md +++ b/doc/administration/monitoring/performance/influxdb_schema.md @@ -1,5 +1,9 @@ # InfluxDB Schema +CAUTION: **InfluxDB is deprecated in favor of Prometheus:** +InfluxDB support is scheduled to be removed in GitLab 13.0. +You are advised to use [Prometheus](../prometheus/index.md) instead. + The following measurements are currently stored in InfluxDB: - `PROCESS_file_descriptors` diff --git a/doc/user/project/integrations/prometheus.md b/doc/user/project/integrations/prometheus.md index 3b7309ea7e4..a5c666f08a6 100644 --- a/doc/user/project/integrations/prometheus.md +++ b/doc/user/project/integrations/prometheus.md @@ -139,7 +139,10 @@ GitLab supports a limited set of [CI variables](../../../ci/variables/README.htm - CI_ENVIRONMENT_SLUG - KUBE_NAMESPACE -To specify a variable in a query, enclose it in quotation marks with curly braces with a leading percent. For example: `"%{ci_environment_slug}"`. +There are 2 methods to specify a variable in a query or dashboard: + +1. Variables can be specified using the [Liquid template format](https://help.shopify.com/en/themes/liquid/basics), for example `{{ci_environment_slug}}` ([added](https://gitlab.com/gitlab-org/gitlab/merge_requests/20793) in GitLab 12.6). +1. You can also enclose it in quotation marks with curly braces with a leading percent, for example `"%{ci_environment_slug}"`. This method is deprecated though and support will be [removed in the next major release](https://gitlab.com/gitlab-org/gitlab/issues/37990). ### Defining custom dashboards per project diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 9044dcc2ff8..dee48529f60 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -7094,6 +7094,9 @@ msgstr "" msgid "Error rendering markdown preview" msgstr "" +msgid "Error rendering query" +msgstr "" + msgid "Error saving label update." msgstr "" @@ -11075,6 +11078,9 @@ msgstr "" msgid "Memory Usage" msgstr "" +msgid "Memory limit exceeded while rendering template" +msgstr "" + msgid "Merge" msgstr "" @@ -12410,6 +12416,9 @@ msgstr "" msgid "Optional" msgstr "" +msgid "Optional parameter \"variables\" must be an array of keys and values. Ex: [key1, value1, key2, value2]" +msgstr "" + msgid "Optionally, you can %{link_to_customize} how FogBugz email addresses and usernames are imported into GitLab." msgstr "" diff --git a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb index 0940fccb431..793c10f0b21 100644 --- a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb +++ b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb @@ -78,6 +78,40 @@ describe Projects::Environments::PrometheusApiController do end end end + + context 'with variables' do + let(:pod_name) { "pod1" } + + before do + expected_params[:query] = %{up{pod_name="#{pod_name}"}} + expected_params[:variables] = ['pod_name', pod_name] + end + + it 'replaces variables with values' do + get :proxy, params: environment_params.merge( + query: 'up{pod_name="{{pod_name}}"}', variables: ['pod_name', pod_name] + ) + + expect(response).to have_gitlab_http_status(:success) + expect(Prometheus::ProxyService).to have_received(:new) + .with(environment, 'GET', 'query', expected_params) + end + + context 'with invalid variables' do + let(:params_with_invalid_variables) do + environment_params.merge( + query: 'up{pod_name="{{pod_name}}"}', variables: ['a'] + ) + end + + it 'returns 400' do + get :proxy, params: params_with_invalid_variables + + expect(response).to have_gitlab_http_status(:bad_request) + expect(Prometheus::ProxyService).not_to receive(:new) + end + end + end end context 'with nil result' do diff --git a/spec/services/prometheus/proxy_variable_substitution_service_spec.rb b/spec/services/prometheus/proxy_variable_substitution_service_spec.rb index b1cdb8fd3ae..9978c631366 100644 --- a/spec/services/prometheus/proxy_variable_substitution_service_spec.rb +++ b/spec/services/prometheus/proxy_variable_substitution_service_spec.rb @@ -39,8 +39,12 @@ describe Prometheus::ProxyVariableSubstitutionService do end context 'with predefined variables' do + let(:params_keys) { { query: 'up{%{environment_filter}}' } } + it_behaves_like 'success' do - let(:expected_query) { %Q[up{environment="#{environment.slug}"}] } + let(:expected_query) do + %Q[up{container_name!="POD",environment="#{environment.slug}"}] + end end context 'with nil query' do @@ -50,6 +54,133 @@ describe Prometheus::ProxyVariableSubstitutionService do let(:expected_query) { nil } end end + + context 'with liquid format' do + let(:params_keys) do + { query: 'up{environment="{{ci_environment_slug}}"}' } + end + + it_behaves_like 'success' do + let(:expected_query) { %Q[up{environment="#{environment.slug}"}] } + end + end + + context 'with ruby and liquid formats' do + let(:params_keys) do + { query: 'up{%{environment_filter},env2="{{ci_environment_slug}}"}' } + end + + it_behaves_like 'success' do + let(:expected_query) do + %Q[up{container_name!="POD",environment="#{environment.slug}",env2="#{environment.slug}"}] + end + end + end + end + + context 'with custom variables' do + let(:pod_name) { "pod1" } + + let(:params_keys) do + { + query: 'up{pod_name="{{pod_name}}"}', + variables: ['pod_name', pod_name] + } + end + + it_behaves_like 'success' do + let(:expected_query) { %q[up{pod_name="pod1"}] } + end + + context 'with ruby variable interpolation format' do + let(:params_keys) do + { + query: 'up{pod_name="%{pod_name}"}', + variables: ['pod_name', pod_name] + } + end + + it_behaves_like 'success' do + # Custom variables cannot be used with the Ruby interpolation format. + let(:expected_query) { "up{pod_name=\"%{pod_name}\"}" } + end + end + + context 'with predefined variables in variables parameter' do + let(:params_keys) do + { + query: 'up{pod_name="{{pod_name}}",env="{{ci_environment_slug}}"}', + variables: ['pod_name', pod_name, 'ci_environment_slug', 'custom_value'] + } + end + + it_behaves_like 'success' do + # Predefined variable values should not be overwritten by custom variable + # values. + let(:expected_query) { "up{pod_name=\"#{pod_name}\",env=\"#{environment.slug}\"}" } + end + end + + context 'with invalid variables parameter' do + let(:params_keys) do + { + query: 'up{pod_name="{{pod_name}}"}', + variables: ['a'] + } + end + + it_behaves_like 'error', 'Optional parameter "variables" must be an ' \ + 'array of keys and values. Ex: [key1, value1, key2, value2]' + end + + context 'with nil variables' do + let(:params_keys) do + { + query: 'up{pod_name="{{pod_name}}"}', + variables: nil + } + end + + it_behaves_like 'success' do + let(:expected_query) { 'up{pod_name=""}' } + end + end + + context 'with ruby and liquid variables' do + let(:params_keys) do + { + query: 'up{env1="%{ruby_variable}",env2="{{ liquid_variable }}"}', + variables: %w(ruby_variable value liquid_variable env_slug) + } + end + + it_behaves_like 'success' do + # It should replace only liquid variables with their values + let(:expected_query) { %q[up{env1="%{ruby_variable}",env2="env_slug"}] } + end + end + end + + context 'with liquid tags and ruby format variables' do + let(:params_keys) do + { + query: 'up{ {% if true %}env1="%{ci_environment_slug}",' \ + 'env2="{{ci_environment_slug}}"{% endif %} }' + } + end + + # The following spec will fail and should be changed to a 'success' spec + # once we remove support for the Ruby interpolation format. + # https://gitlab.com/gitlab-org/gitlab/issues/37990 + # + # Liquid tags `{% %}` cannot be used currently because the Ruby `%` + # operator raises an error when it encounters a Liquid `{% %}` tag in the + # string. + # + # Once we remove support for the Ruby format, users can start using + # Liquid tags. + + it_behaves_like 'error', 'Malformed string' end context 'ruby template rendering' do @@ -139,5 +270,18 @@ describe Prometheus::ProxyVariableSubstitutionService do end end end + + context 'when liquid template rendering raises error' do + before do + liquid_service = instance_double(TemplateEngines::LiquidService) + + allow(TemplateEngines::LiquidService).to receive(:new).and_return(liquid_service) + allow(liquid_service).to receive(:render).and_raise( + TemplateEngines::LiquidService::RenderError, 'error message' + ) + end + + it_behaves_like 'error', 'error message' + end end end diff --git a/spec/services/template_engines/liquid_service_spec.rb b/spec/services/template_engines/liquid_service_spec.rb new file mode 100644 index 00000000000..7c5262bc264 --- /dev/null +++ b/spec/services/template_engines/liquid_service_spec.rb @@ -0,0 +1,126 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe TemplateEngines::LiquidService do + describe '#render' do + let(:template) { 'up{env={{ci_environment_slug}}}' } + let(:result) { subject } + + let_it_be(:slug) { 'env_slug' } + + let_it_be(:context) do + { + ci_environment_slug: slug, + environment_filter: "container_name!=\"POD\",environment=\"#{slug}\"" + } + end + + subject { described_class.new(template).render(context) } + + it 'with symbol keys in context it substitutes variables' do + expect(result).to include("up{env=#{slug}") + end + + context 'with multiple occurrences of variable in template' do + let(:template) do + 'up{env1={{ci_environment_slug}},env2={{ci_environment_slug}}}' + end + + it 'substitutes variables' do + expect(result).to eq("up{env1=#{slug},env2=#{slug}}") + end + end + + context 'with multiple variables in template' do + let(:template) do + 'up{env={{ci_environment_slug}},' \ + '{{environment_filter}}}' + end + + it 'substitutes all variables' do + expect(result).to eq( + "up{env=#{slug}," \ + "container_name!=\"POD\",environment=\"#{slug}\"}" + ) + end + end + + context 'with unknown variables in template' do + let(:template) { 'up{env={{env_slug}}}' } + + it 'does not substitute unknown variables' do + expect(result).to eq("up{env=}") + end + end + + context 'with extra variables in context' do + let(:template) { 'up{env={{ci_environment_slug}}}' } + + it 'substitutes variables' do + # If context has only 1 key, there is no need for this spec. + expect(context.count).to be > 1 + expect(result).to eq("up{env=#{slug}}") + end + end + + context 'with unknown and known variables in template' do + let(:template) { 'up{env={{ci_environment_slug}},other_env={{env_slug}}}' } + + it 'substitutes known variables' do + expect(result).to eq("up{env=#{slug},other_env=}") + end + end + + context 'Liquid errors' do + shared_examples 'raises RenderError' do |message| + it do + expect { result }.to raise_error(described_class::RenderError, message) + end + end + + context 'when liquid raises error' do + let(:template) { 'up{env={{ci_environment_slug}}' } + let(:liquid_template) { Liquid::Template.new } + + before do + allow(Liquid::Template).to receive(:parse).with(template).and_return(liquid_template) + allow(liquid_template).to receive(:render!).and_raise(exception, message) + end + + context 'raises Liquid::MemoryError' do + let(:exception) { Liquid::MemoryError } + let(:message) { 'Liquid error: Memory limits exceeded' } + + it_behaves_like 'raises RenderError', 'Memory limit exceeded while rendering template' + end + + context 'raises Liquid::Error' do + let(:exception) { Liquid::Error } + let(:message) { 'Liquid error: Generic error message' } + + it_behaves_like 'raises RenderError', 'Error rendering query' + end + end + + context 'with template that is expensive to render' do + let(:template) do + '{% assign loop_count = 1000 %}'\ + '{% assign padStr = "0" %}'\ + '{% assign number_to_pad = "1" %}'\ + '{% assign strLength = number_to_pad | size %}'\ + '{% assign padLength = loop_count | minus: strLength %}'\ + '{% if padLength > 0 %}'\ + ' {% assign padded = number_to_pad %}'\ + ' {% for position in (1..padLength) %}'\ + ' {% assign padded = padded | prepend: padStr %}'\ + ' {% endfor %}'\ + ' {{ padded }}'\ + '{% endif %}' + end + + it_behaves_like 'raises RenderError', 'Memory limit exceeded while rendering template' + end + end + end +end |