Add latest changes from gitlab-org/security/gitlab@12-10-stable-ee

1 files changed, 87 insertions, 114 deletions

diff --git a/spec/services/prometheus/proxy_variable_substitution_service_spec.rb b/spec/services/prometheus/proxy_variable_substitution_service_spec.rb
index 9978c631366..7faac1b4fc6 100644
--- a/spec/services/prometheus/proxy_variable_substitution_service_spec.rb
+++ b/spec/services/prometheus/proxy_variable_substitution_service_spec.rb
@@ -6,19 +6,46 @@ describe Prometheus::ProxyVariableSubstitutionService do
   describe '#execute' do
     let_it_be(:environment) { create(:environment) }
 
-    let(:params_keys) { { query: 'up{environment="%{ci_environment_slug}"}' } }
+    let(:params_keys) { { query: "up{environment=\"#{w('ci_environment_slug')}\"}" } }
     let(:params) { ActionController::Parameters.new(params_keys).permit! }
     let(:result) { subject.execute }
 
     subject { described_class.new(environment, params) }
 
-    shared_examples 'success' do
+    # Default implementation of the w method. The `success` shared example overrides
+    # this implementation to test the Ruby and Liquid syntaxes.
+    # This method wraps the given variable name in the variable interpolation
+    # syntax. Using this method along with the `success` shared example allows
+    # a spec to test both syntaxes.
+    def w(variable_name)
+      "%{#{variable_name}}"
+    end
+
+    shared_examples 'replaces variables with values' do
       it 'replaces variables with values' do
         expect(result[:status]).to eq(:success)
         expect(result[:params][:query]).to eq(expected_query)
       end
     end
 
+    shared_examples 'success' do
+      context 'with Ruby syntax `${}`' do
+        it_behaves_like 'replaces variables with values'
+
+        def w(variable_name)
+          "%{#{variable_name}}"
+        end
+      end
+
+      context 'with Liquid syntax `{{}}`' do
+        it_behaves_like 'replaces variables with values'
+
+        def w(variable_name)
+          "{{#{variable_name}}}"
+        end
+      end
+    end
+
     shared_examples 'error' do |message|
       it 'returns error' do
         expect(result[:status]).to eq(:error)
@@ -39,11 +66,13 @@ describe Prometheus::ProxyVariableSubstitutionService do
     end
 
     context 'with predefined variables' do
-      let(:params_keys) { { query: 'up{%{environment_filter}}' } }
+      # Liquid replaces the opening brace of the query as well, if there is no space
+      # between `up{` and the rest of the string.
+      let(:params_keys) { { query: "up{ #{w('environment_filter')}}" } }
 
       it_behaves_like 'success' do
         let(:expected_query) do
-          %Q[up{container_name!="POD",environment="#{environment.slug}"}]
+          %Q[up{ container_name!="POD",environment="#{environment.slug}"}]
         end
       end
 
@@ -54,26 +83,16 @@ describe Prometheus::ProxyVariableSubstitutionService do
           let(:expected_query) { nil }
         end
       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
+    context 'with ruby and liquid formats' do
+      let(:params_keys) do
+        { query: 'up{%{environment_filter},env2="{{ci_environment_slug}}"}' }
       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
+      it_behaves_like 'success' do
+        let(:expected_query) do
+          %Q[up{container_name!="POD",environment="#{environment.slug}",env2="#{environment.slug}"}]
         end
       end
     end
@@ -83,7 +102,7 @@ describe Prometheus::ProxyVariableSubstitutionService do
 
       let(:params_keys) do
         {
-          query: 'up{pod_name="{{pod_name}}"}',
+          query: "up{pod_name=\"#{w('pod_name')}\"}",
           variables: ['pod_name', pod_name]
         }
       end
@@ -92,24 +111,10 @@ describe Prometheus::ProxyVariableSubstitutionService 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}}"}',
+            query: "up{pod_name=\"#{w('pod_name')}\",env=\"#{w('ci_environment_slug')}\"}",
             variables: ['pod_name', pod_name, 'ci_environment_slug', 'custom_value']
           }
         end
@@ -124,7 +129,7 @@ describe Prometheus::ProxyVariableSubstitutionService do
       context 'with invalid variables parameter' do
         let(:params_keys) do
           {
-            query: 'up{pod_name="{{pod_name}}"}',
+            query: "up{pod_name=\"#{w('pod_name')}\"}",
             variables: ['a']
           }
         end
@@ -136,68 +141,66 @@ describe Prometheus::ProxyVariableSubstitutionService do
       context 'with nil variables' do
         let(:params_keys) do
           {
-            query: 'up{pod_name="{{pod_name}}"}',
+            query: "up{pod_name=\"%{pod_name}\"}",
             variables: nil
           }
         end
 
-        it_behaves_like 'success' do
-          let(:expected_query) { 'up{pod_name=""}' }
+        it_behaves_like 'replaces variables with values' do
+          let(:expected_query) { "up{pod_name=\"%{pod_name}\"}" }
         end
       end
+    end
 
-      context 'with ruby and liquid variables' do
+    context 'gsub variable substitution tolerance for weirdness' do
+      context 'with whitespace around variable' do
         let(:params_keys) do
           {
-            query: 'up{env1="%{ruby_variable}",env2="{{ liquid_variable }}"}',
-            variables: %w(ruby_variable value liquid_variable env_slug)
+            query: 'up{' \
+                "env1=#{w(' ci_environment_slug')}," \
+                "env2=#{w('ci_environment_slug ')}," \
+                "#{w('  environment_filter ')}" \
+              '}'
           }
         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"}] }
+          let(:expected_query) do
+            'up{' \
+              "env1=#{environment.slug}," \
+              "env2=#{environment.slug}," \
+              "container_name!=\"POD\",environment=\"#{environment.slug}\"" \
+            '}'
+          end
         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 '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
 
-    context 'ruby template rendering' do
-      let(:params_keys) do
-        { query: 'up{env=%{ci_environment_slug},%{environment_filter}}' }
+        it_behaves_like 'replaces variables with values' do
+          let(:expected_query) { "up{ env1=\"#{environment.slug}\",env2=\"#{environment.slug}\" }" }
+        end
       end
 
-      it_behaves_like 'success' do
-        let(:expected_query) do
-          "up{env=#{environment.slug},container_name!=\"POD\"," \
-          "environment=\"#{environment.slug}\"}"
+      context 'with empty variables' do
+        let(:params_keys) do
+          { query: "up{env1=%{},env2=%{  }}" }
+        end
+
+        it_behaves_like 'replaces variables with values' do
+          let(:expected_query) { "up{env1=%{},env2=%{  }}" }
         end
       end
 
       context 'with multiple occurrences of variable in string' do
         let(:params_keys) do
-          { query: 'up{env1=%{ci_environment_slug},env2=%{ci_environment_slug}}' }
+          { query: "up{env1=#{w('ci_environment_slug')},env2=#{w('ci_environment_slug')}}" }
         end
 
         it_behaves_like 'success' do
@@ -207,7 +210,7 @@ describe Prometheus::ProxyVariableSubstitutionService do
 
       context 'with multiple variables in string' do
         let(:params_keys) do
-          { query: 'up{env=%{ci_environment_slug},%{environment_filter}}' }
+          { query: "up{env=#{w('ci_environment_slug')},#{w('environment_filter')}}" }
         end
 
         it_behaves_like 'success' do
@@ -219,54 +222,24 @@ describe Prometheus::ProxyVariableSubstitutionService do
       end
 
       context 'with unknown variables in string' do
-        let(:params_keys) { { query: 'up{env=%{env_slug}}' } }
+        let(:params_keys) { { query: "up{env=#{w('env_slug')}}" } }
 
-        it_behaves_like 'success' do
-          let(:expected_query) { 'up{env=%{env_slug}}' }
-        end
-      end
-
-      # This spec is needed if there are multiple keys in the context provided
-      # by `Gitlab::Prometheus::QueryVariables.call(environment)` which is
-      # passed to the Ruby `%` operator.
-      # If the number of keys in the context is one, there is no need for
-      # this spec.
-      context 'with extra variables in context' do
-        let(:params_keys) { { query: 'up{env=%{ci_environment_slug}}' } }
-
-        it_behaves_like 'success' do
-          let(:expected_query) { "up{env=#{environment.slug}}" }
-        end
-
-        it 'has more than one variable in context' do
-          expect(Gitlab::Prometheus::QueryVariables.call(environment).size).to be > 1
+        it_behaves_like 'replaces variables with values' do
+          let(:expected_query) { "up{env=%{env_slug}}" }
         end
       end
 
       # The ruby % operator will not replace known variables if there are unknown
       # variables also in the string. It doesn't raise an error
       # (though the `sprintf` and `format` methods do).
+      # Fortunately, we do not use the % operator anymore.
       context 'with unknown and known variables in string' do
         let(:params_keys) do
-          { query: 'up{env=%{ci_environment_slug},other_env=%{env_slug}}' }
+          { query: "up{env=%{ci_environment_slug},other_env=%{env_slug}}" }
         end
 
-        it_behaves_like 'success' do
-          let(:expected_query) { 'up{env=%{ci_environment_slug},other_env=%{env_slug}}' }
-        end
-      end
-
-      context 'when rendering raises error' do
-        context 'when TypeError is raised' do
-          let(:params_keys) { { query: '{% a %}' } }
-
-          it_behaves_like 'error', 'Malformed string'
-        end
-
-        context 'when ArgumentError is raised' do
-          let(:params_keys) { { query: '%<' } }
-
-          it_behaves_like 'error', 'Malformed string'
+        it_behaves_like 'replaces variables with values' do
+          let(:expected_query) { "up{env=#{environment.slug},other_env=#{w('env_slug')}}" }
         end
       end
     end