Fix empty "regex" field in API sub settings causing problems.

Similar to issue for rewrites in
#360

Add missing validations for the regex field, and update proxy to deal
more gracefully with any existing backends where this is missing.

src/api-umbrella/admin-ui/app/components/apis/sub-settings-form.js

@@ -1,5 +1,6 @@
 import Ember from 'ember';
 import BufferedProxy from 'ember-buffered-proxy/proxy';
+import SubSettings from 'api-umbrella-admin-ui/models/api/sub-settings';
 
 export default Ember.Component.extend({
   openModal: false,
@@ -25,7 +26,8 @@ export default Ember.Component.extend({
   }),
 
   bufferedModel: Ember.computed('model', function() {
-    return BufferedProxy.create({ content: this.get('model') });
+    let owner = Ember.getOwner(this).ownerInjection();
+    return BufferedProxy.extend(SubSettings.validationClass).create(owner, { content: this.get('model') });
   }),
 
   actions: {

src/api-umbrella/admin-ui/app/models/api/sub-settings.js

@@ -1,6 +1,16 @@
 import DS from 'ember-data';
+import { validator, buildValidations } from 'ember-cp-validations';
 
-export default DS.Model.extend({
+const Validations = buildValidations({
+  httpMethod: [
+    validator('presence', true),
+  ],
+  regex: [
+    validator('presence', true),
+  ],
+});
+
+export default DS.Model.extend(Validations, {
   sortOrder: DS.attr('number'),
   httpMethod: DS.attr(),
   regex: DS.attr(),
@@ -17,4 +27,6 @@ export default DS.Model.extend({
       this.set('settings', this.get('store').createRecord('api/settings'));
     }
   },
+}).reopenClass({
+  validationClass: Validations,
 });

src/api-umbrella/admin-ui/app/templates/components/apis/sub-settings-table.hbs

@@ -5,6 +5,7 @@
     <tr>
       <th style="width: 110px;">HTTP Method</th>
       <th>URL Matcher</th>
+      <th></th>
       <th class="reorder-handle"></th>
     </tr>
   </thead>

src/api-umbrella/proxy/middleware/api_settings.lua

@@ -21,7 +21,7 @@ return function(api)
     local request_method = ngx.ctx.request_method
     local request_uri = ngx.ctx.request_uri
     for _, sub_settings in ipairs(api["sub_settings"]) do
-      if sub_settings["http_method"] == "any" or sub_settings["http_method"] == request_method then
+      if (sub_settings["http_method"] == "any" or sub_settings["http_method"] == request_method) and sub_settings["regex"] then
         local matches, match_err = ngx.re.match(request_uri, sub_settings["regex"], "ijo")
         if matches then
           local original_required_roles

src/api-umbrella/web-app/app/models/api/sub_settings.rb

@@ -13,6 +13,8 @@ class Api::SubSettings
   # Validations
   validates :http_method,
     :inclusion => { :in => %w(any GET POST PUT DELETE HEAD TRACE OPTIONS CONNECT PATCH) }
+  validates :regex,
+    :presence => true
 
   # Nested attributes
   accepts_nested_attributes_for :settings

test/apis/v1/apis/test_save_rewrite_validations.rb

@@ -2,6 +2,7 @@
 
 class Test::Apis::V1::Apis::TestSaveRewriteValidations < Minitest::Test
   include ApiUmbrellaTestHelpers::AdminAuth
+  include ApiUmbrellaTestHelpers::ApiSaveValidations
   include ApiUmbrellaTestHelpers::Setup
   parallelize_me!
 
@@ -97,79 +98,4 @@ def test_rejects_blank_backend_replacement
       ],
     }, ["rewrites[0].backend_replacement"])
   end
-
-  private
-
-  def assert_valid(overrides)
-    assert_valid_create(overrides)
-    assert_valid_update(overrides)
-  end
-
-  def assert_valid_create(overrides)
-    assert_valid_action(:create, overrides)
-  end
-
-  def assert_valid_update(overrides)
-    assert_valid_action(:update, overrides)
-  end
-
-  def assert_valid_action(action, overrides)
-    attributes = attributes_for(action).deep_merge(overrides.deep_stringify_keys)
-
-    response = create_or_update(action, attributes)
-    if(action == :create)
-      assert_response_code(201, response)
-    elsif(action == :update)
-      assert_response_code(204, response)
-    end
-  end
-
-  def assert_invalid(overrides, expected_error_fields)
-    assert_invalid_create(overrides, expected_error_fields)
-    assert_invalid_update(overrides, expected_error_fields)
-  end
-
-  def assert_invalid_create(overrides, expected_error_fields)
-    assert_invalid_action(:create, overrides, expected_error_fields)
-  end
-
-  def assert_invalid_update(overrides, expected_error_fields)
-    assert_invalid_action(:update, overrides, expected_error_fields)
-  end
-
-  def assert_invalid_action(action, overrides, expected_error_fields)
-    attributes = attributes_for(action).deep_merge(overrides.deep_stringify_keys)
-
-    response = create_or_update(action, attributes)
-    assert_response_code(422, response)
-    data = MultiJson.load(response.body)
-    assert_equal(["errors"], data.keys)
-    assert_equal(expected_error_fields.sort, data["errors"].keys.sort)
-  end
-
-  def attributes_for(action)
-    if(action == :create)
-      FactoryGirl.attributes_for(:api).deep_stringify_keys
-    elsif(action == :update)
-      FactoryGirl.create(:api).serializable_hash
-    else
-      flunk("Unknown action: #{action.inspect}")
-    end
-  end
-
-  def create_or_update(action, attributes)
-    if(action == :create)
-      Typhoeus.post("https://127.0.0.1:9081/api-umbrella/v1/apis.json", http_options.deep_merge(admin_token).deep_merge({
-        :headers => { "Content-Type" => "application/json" },
-        :body => MultiJson.dump(:api => attributes),
-      }))
-    elsif(action == :update)
-      Typhoeus.put("https://127.0.0.1:9081/api-umbrella/v1/apis/#{attributes["id"]}.json", http_options.deep_merge(admin_token).deep_merge({
-        :headers => { "Content-Type" => "application/json" },
-        :body => MultiJson.dump(:api => attributes),
-      }))
-    else
-      flunk("Unknown action: #{action.inspect}")
-    end
-  end
 end

test/apis/v1/apis/test_save_sub_settings_validations.rb

@@ -0,0 +1,61 @@
+require_relative "../../../test_helper"
+
+class Test::Apis::V1::Apis::TestSaveSubSettingsValidations < Minitest::Test
+  include ApiUmbrellaTestHelpers::AdminAuth
+  include ApiUmbrellaTestHelpers::ApiSaveValidations
+  include ApiUmbrellaTestHelpers::Setup
+  parallelize_me!
+
+  def setup
+    super
+    setup_server
+  end
+
+  def test_accepts_valid_rewrite
+    assert_valid({
+      :sub_settings => [
+        FactoryGirl.attributes_for(:api_sub_setting),
+      ],
+    })
+  end
+
+  def test_rejects_null_http_method
+    assert_invalid({
+      :sub_settings => [
+        FactoryGirl.attributes_for(:api_sub_setting, :http_method => nil),
+      ],
+    }, ["sub_settings[0].http_method"])
+  end
+
+  def test_rejects_blank_http_method
+    assert_invalid({
+      :sub_settings => [
+        FactoryGirl.attributes_for(:api_sub_setting, :http_method => ""),
+      ],
+    }, ["sub_settings[0].http_method"])
+  end
+
+  def test_rejects_invalid_http_method
+    assert_invalid({
+      :sub_settings => [
+        FactoryGirl.attributes_for(:api_sub_setting, :http_method => "zzz"),
+      ],
+    }, ["sub_settings[0].http_method"])
+  end
+
+  def test_rejects_null_regex
+    assert_invalid({
+      :sub_settings => [
+        FactoryGirl.attributes_for(:api_sub_setting, :regex => nil),
+      ],
+    }, ["sub_settings[0].regex"])
+  end
+
+  def test_rejects_blank_regex
+    assert_invalid({
+      :sub_settings => [
+        FactoryGirl.attributes_for(:api_sub_setting, :regex => ""),
+      ],
+    }, ["sub_settings[0].regex"])
+  end
+end

test/proxy/test_sub_settings.rb

@@ -0,0 +1,74 @@
+require_relative "../test_helper"
+
+class Test::Proxy::TestSubSettings < Minitest::Test
+  include ApiUmbrellaTestHelpers::Setup
+  parallelize_me!
+
+  def setup
+    super
+    setup_server
+  end
+
+  def test_sub_settings
+    prepend_api_backends([
+      {
+        :frontend_host => "127.0.0.1",
+        :backend_host => "127.0.0.1",
+        :servers => [{ :host => "127.0.0.1", :port => 9444 }],
+        :url_matches => [{ :frontend_prefix => "/#{unique_test_id}/", :backend_prefix => "/" }],
+        :sub_settings => [
+          {
+            :http_method => "any",
+            :regex => "^/info/sub/",
+            :settings => {
+              :headers => [
+                { :key => "X-Sub1", :value => "sub-value1" },
+              ],
+            },
+          },
+        ],
+      },
+    ]) do
+      response = Typhoeus.get("http://127.0.0.1:9080/#{unique_test_id}/info/sub/", http_options)
+      assert_response_code(200, response)
+      data = MultiJson.load(response.body)
+      assert_equal("sub-value1", data["headers"]["x-sub1"])
+    end
+  end
+
+  def test_ignores_invalid_sub_settings_without_regex
+    prepend_api_backends([
+      {
+        :frontend_host => "127.0.0.1",
+        :backend_host => "127.0.0.1",
+        :servers => [{ :host => "127.0.0.1", :port => 9444 }],
+        :url_matches => [{ :frontend_prefix => "/#{unique_test_id}/", :backend_prefix => "/" }],
+        :sub_settings => [
+          {
+            :http_method => "any",
+            :settings => {
+              :headers => [
+                { :key => "X-Sub1", :value => "sub-value1" },
+              ],
+            },
+          },
+          {
+            :http_method => "any",
+            :regex => "^/info/sub/",
+            :settings => {
+              :headers => [
+                { :key => "X-Sub2", :value => "sub-value2" },
+              ],
+            },
+          },
+        ],
+      },
+    ]) do
+      response = Typhoeus.get("http://127.0.0.1:9080/#{unique_test_id}/info/sub/", http_options)
+      assert_response_code(200, response)
+      data = MultiJson.load(response.body)
+      assert_nil(data["headers"]["x-sub1"])
+      assert_equal("sub-value2", data["headers"]["x-sub2"])
+    end
+  end
+end

test/support/api_umbrella_shared_tests/api_save_validations.rb

@@ -0,0 +1,78 @@
+module ApiUmbrellaTestHelpers
+  module ApiSaveValidations
+    private
+
+    def assert_valid(overrides)
+      assert_valid_create(overrides)
+      assert_valid_update(overrides)
+    end
+
+    def assert_valid_create(overrides)
+      assert_valid_action(:create, overrides)
+    end
+
+    def assert_valid_update(overrides)
+      assert_valid_action(:update, overrides)
+    end
+
+    def assert_valid_action(action, overrides)
+      attributes = attributes_for(action).deep_merge(overrides.deep_stringify_keys)
+
+      response = create_or_update(action, attributes)
+      if(action == :create)
+        assert_response_code(201, response)
+      elsif(action == :update)
+        assert_response_code(204, response)
+      end
+    end
+
+    def assert_invalid(overrides, expected_error_fields)
+      assert_invalid_create(overrides, expected_error_fields)
+      assert_invalid_update(overrides, expected_error_fields)
+    end
+
+    def assert_invalid_create(overrides, expected_error_fields)
+      assert_invalid_action(:create, overrides, expected_error_fields)
+    end
+
+    def assert_invalid_update(overrides, expected_error_fields)
+      assert_invalid_action(:update, overrides, expected_error_fields)
+    end
+
+    def assert_invalid_action(action, overrides, expected_error_fields)
+      attributes = attributes_for(action).deep_merge(overrides.deep_stringify_keys)
+
+      response = create_or_update(action, attributes)
+      assert_response_code(422, response)
+      data = MultiJson.load(response.body)
+      assert_equal(["errors"], data.keys)
+      assert_equal(expected_error_fields.sort, data["errors"].keys.sort)
+    end
+
+    def attributes_for(action)
+      if(action == :create)
+        FactoryGirl.attributes_for(:api).deep_stringify_keys
+      elsif(action == :update)
+        FactoryGirl.create(:api).serializable_hash
+      else
+        flunk("Unknown action: #{action.inspect}")
+      end
+    end
+
+    def create_or_update(action, attributes)
+      if(action == :create)
+        Typhoeus.post("https://127.0.0.1:9081/api-umbrella/v1/apis.json", http_options.deep_merge(admin_token).deep_merge({
+          :headers => { "Content-Type" => "application/json" },
+          :body => MultiJson.dump(:api => attributes),
+        }))
+      elsif(action == :update)
+        Typhoeus.put("https://127.0.0.1:9081/api-umbrella/v1/apis/#{attributes["id"]}.json", http_options.deep_merge(admin_token).deep_merge({
+          :headers => { "Content-Type" => "application/json" },
+          :body => MultiJson.dump(:api => attributes),
+        }))
+      else
+        flunk("Unknown action: #{action.inspect}")
+      end
+    end
+  end
+end