Merge pull request #335 from Accenture/334-fix-endpoint-validation

334 fix endpoint validation

Author: mmacai

CHANGELOG.md

@@ -35,6 +35,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - Fixed a bug where distributed set processes would crash when one of their peers has died but hasn't been removed yet from the pg2 group.
+- Fixed wrong endpoint validation for reverse proxy. Now it should correctly check for `path` or `path_regex`. Before it would require `path` even with `path_regex` in place. [#334](https://github.com/Accenture/reactive-interaction-gateway/issues/334)
 
 <!-- ### Deprecated -->
 

lib/rig_api/v1/apis_test.exs

@@ -357,7 +357,7 @@ defmodule RigApi.V1.APIsTest do
              }
     end
 
-    test "should return 400 when 'endpoint' doesn't have required properties 'id', 'method' and 'path'" do
+    test "should return 400 when 'endpoint' doesn't have required properties 'id', 'method' and 'path' or 'path_regex'" do
       endpoints = [%{}]
       new_api = ProxyConfig.create_proxy_config(@invalid_config_id, endpoints)
       conn = build_conn() |> put("/v1/apis/#{@invalid_config_id}", new_api)
@@ -367,14 +367,74 @@ defmodule RigApi.V1.APIsTest do
                "invalid-config/" => [
                  %{"id" => "must be string"},
                  %{"id" => "must have a length of at least 1"},
-                 %{"path" => "must be string"},
-                 %{"path" => "must have a length of at least 1"},
+                 %{"path, path_regex" => "Either path or path_regex must be set"},
                  %{"method" => "must be string"},
                  %{"method" => "must have a length of at least 1"}
                ]
              }
     end
 
+    test "should return 400 when 'endpoint' has 'path' and 'path_regex' set at the same time" do
+      endpoints = [
+        %{
+          "id" => @invalid_config_id <> "1",
+          "method" => "GET",
+          "path" => "/",
+          "path_regex" => "/"
+        }
+      ]
+
+      new_api = ProxyConfig.create_proxy_config(@invalid_config_id, endpoints)
+      conn = build_conn() |> put("/v1/apis/#{@invalid_config_id}", new_api)
+      response = json_response(conn, 400)
+
+      assert response == %{
+               "invalid-config/invalid-config1" => [
+                 %{"path, path_regex" => "You can't set path and path_regex at the same time"}
+               ]
+             }
+    end
+
+    test "should return 400 when when 'path' is set, but incorrect" do
+      endpoints = [
+        %{
+          "id" => @invalid_config_id <> "1",
+          "method" => "GET",
+          "path" => ""
+        }
+      ]
+
+      new_api = ProxyConfig.create_proxy_config(@invalid_config_id, endpoints)
+      conn = build_conn() |> put("/v1/apis/#{@invalid_config_id}", new_api)
+      response = json_response(conn, 400)
+
+      assert response == %{
+               "invalid-config/invalid-config1" => [
+                 %{"path" => "must have a length of at least 1"}
+               ]
+             }
+    end
+
+    test "should return 400 when when 'path_regex' is set, but incorrect" do
+      endpoints = [
+        %{
+          "id" => @invalid_config_id <> "1",
+          "method" => "GET",
+          "path_regex" => ""
+        }
+      ]
+
+      new_api = ProxyConfig.create_proxy_config(@invalid_config_id, endpoints)
+      conn = build_conn() |> put("/v1/apis/#{@invalid_config_id}", new_api)
+      response = json_response(conn, 400)
+
+      assert response == %{
+               "invalid-config/invalid-config1" => [
+                 %{"path_regex" => "must have a length of at least 1"}
+               ]
+             }
+    end
+
     test "should return 400 when target is set to kafka or kinesis, but topic is not" do
       # kinesis topic not set
       endpoints = [

lib/rig_api/v2/apis_test.exs

@@ -357,7 +357,7 @@ defmodule RigApi.V2.APIsTest do
              }
     end
 
-    test "should return 400 when 'endpoint' doesn't have required properties 'id', 'method' and 'path'" do
+    test "should return 400 when 'endpoint' doesn't have required properties 'id', 'method' and 'path' or 'path_regex'" do
       endpoints = [%{}]
       new_api = ProxyConfig.create_proxy_config(@invalid_config_id, endpoints)
       conn = build_conn() |> put("/v2/apis/#{@invalid_config_id}", new_api)
@@ -367,14 +367,74 @@ defmodule RigApi.V2.APIsTest do
                "invalid-config/" => [
                  %{"id" => "must be string"},
                  %{"id" => "must have a length of at least 1"},
-                 %{"path" => "must be string"},
-                 %{"path" => "must have a length of at least 1"},
+                 %{"path, path_regex" => "Either path or path_regex must be set"},
                  %{"method" => "must be string"},
                  %{"method" => "must have a length of at least 1"}
                ]
              }
     end
 
+    test "should return 400 when 'endpoint' has 'path' and 'path_regex' set at the same time" do
+      endpoints = [
+        %{
+          "id" => @invalid_config_id <> "1",
+          "method" => "GET",
+          "path" => "/",
+          "path_regex" => "/"
+        }
+      ]
+
+      new_api = ProxyConfig.create_proxy_config(@invalid_config_id, endpoints)
+      conn = build_conn() |> put("/v2/apis/#{@invalid_config_id}", new_api)
+      response = json_response(conn, 400)
+
+      assert response == %{
+               "invalid-config/invalid-config1" => [
+                 %{"path, path_regex" => "You can't set path and path_regex at the same time"}
+               ]
+             }
+    end
+
+    test "should return 400 when when 'path' is set, but incorrect" do
+      endpoints = [
+        %{
+          "id" => @invalid_config_id <> "1",
+          "method" => "GET",
+          "path" => ""
+        }
+      ]
+
+      new_api = ProxyConfig.create_proxy_config(@invalid_config_id, endpoints)
+      conn = build_conn() |> put("/v2/apis/#{@invalid_config_id}", new_api)
+      response = json_response(conn, 400)
+
+      assert response == %{
+               "invalid-config/invalid-config1" => [
+                 %{"path" => "must have a length of at least 1"}
+               ]
+             }
+    end
+
+    test "should return 400 when when 'path_regex' is set, but incorrect" do
+      endpoints = [
+        %{
+          "id" => @invalid_config_id <> "1",
+          "method" => "GET",
+          "path_regex" => ""
+        }
+      ]
+
+      new_api = ProxyConfig.create_proxy_config(@invalid_config_id, endpoints)
+      conn = build_conn() |> put("/v2/apis/#{@invalid_config_id}", new_api)
+      response = json_response(conn, 400)
+
+      assert response == %{
+               "invalid-config/invalid-config1" => [
+                 %{"path_regex" => "must have a length of at least 1"}
+               ]
+             }
+    end
+
     test "should return 400 when target is set to kafka or kinesis, but topic is not" do
       # kinesis topic not set
       endpoints = [

lib/rig_api/v3/apis_test.exs

@@ -357,7 +357,7 @@ defmodule RigApi.V3.APIsTest do
              }
     end
 
-    test "should return 400 when 'endpoint' doesn't have required properties 'id', 'method' and 'path'" do
+    test "should return 400 when 'endpoint' doesn't have required properties 'id', 'method' and 'path' or 'path_regex'" do
       endpoints = [%{}]
       new_api = ProxyConfig.create_proxy_config(@invalid_config_id, endpoints)
       conn = build_conn() |> put("/v3/apis/#{@invalid_config_id}", new_api)
@@ -367,14 +367,74 @@ defmodule RigApi.V3.APIsTest do
                "invalid-config/" => [
                  %{"id" => "must be string"},
                  %{"id" => "must have a length of at least 1"},
-                 %{"path" => "must be string"},
-                 %{"path" => "must have a length of at least 1"},
+                 %{"path, path_regex" => "Either path or path_regex must be set"},
                  %{"method" => "must be string"},
                  %{"method" => "must have a length of at least 1"}
                ]
              }
     end
 
+    test "should return 400 when 'endpoint' has 'path' and 'path_regex' set at the same time" do
+      endpoints = [
+        %{
+          "id" => @invalid_config_id <> "1",
+          "method" => "GET",
+          "path" => "/",
+          "path_regex" => "/"
+        }
+      ]
+
+      new_api = ProxyConfig.create_proxy_config(@invalid_config_id, endpoints)
+      conn = build_conn() |> put("/v3/apis/#{@invalid_config_id}", new_api)
+      response = json_response(conn, 400)
+
+      assert response == %{
+               "invalid-config/invalid-config1" => [
+                 %{"path, path_regex" => "You can't set path and path_regex at the same time"}
+               ]
+             }
+    end
+
+    test "should return 400 when when 'path' is set, but incorrect" do
+      endpoints = [
+        %{
+          "id" => @invalid_config_id <> "1",
+          "method" => "GET",
+          "path" => ""
+        }
+      ]
+
+      new_api = ProxyConfig.create_proxy_config(@invalid_config_id, endpoints)
+      conn = build_conn() |> put("/v3/apis/#{@invalid_config_id}", new_api)
+      response = json_response(conn, 400)
+
+      assert response == %{
+               "invalid-config/invalid-config1" => [
+                 %{"path" => "must have a length of at least 1"}
+               ]
+             }
+    end
+
+    test "should return 400 when when 'path_regex' is set, but incorrect" do
+      endpoints = [
+        %{
+          "id" => @invalid_config_id <> "1",
+          "method" => "GET",
+          "path_regex" => ""
+        }
+      ]
+
+      new_api = ProxyConfig.create_proxy_config(@invalid_config_id, endpoints)
+      conn = build_conn() |> put("/v3/apis/#{@invalid_config_id}", new_api)
+      response = json_response(conn, 400)
+
+      assert response == %{
+               "invalid-config/invalid-config1" => [
+                 %{"path_regex" => "must have a length of at least 1"}
+               ]
+             }
+    end
+
     test "should return 400 when target is set to kafka or kinesis, but topic is not" do
       # kinesis topic not set
       endpoints = [

lib/rig_inbound_gateway/api_proxy/validations.ex

@@ -12,6 +12,9 @@ defmodule RigInboundGateway.ApiProxy.Validations do
 
   require Logger
 
+  @endpoint_paths ["path", "path_regex"]
+  @endpoint_paths_error_key "path, path_regex"
+
   @type error_t :: [{:error, String.t() | atom, atom, String.t()}]
   @type error_list_t :: [{String.t(), error_t()}]
   @type error_map_t :: %{String.t() => [%{(String.t() | atom) => String.t()}]}
@@ -88,6 +91,29 @@ defmodule RigInboundGateway.ApiProxy.Validations do
 
   # ---
 
+  @spec validate_endpoint_path(Api.endpoint()) :: error_list_t()
+  def validate_endpoint_path(endpoint) do
+    present_paths =
+      @endpoint_paths
+      |> Enum.filter(fn key -> Map.has_key?(endpoint, key) end)
+
+    case present_paths do
+      [] ->
+        [{:error, @endpoint_paths_error_key, :by, "Either path or path_regex must be set"}]
+
+      [path] ->
+        validate_string(endpoint, path)
+
+      _ ->
+        [
+          {:error, @endpoint_paths_error_key, :by,
+           "You can't set path and path_regex at the same time"}
+        ]
+    end
+  end
+
+  # ---
+
   @spec with_any_error(error_list_t(), integer) :: error_list_t()
   def with_any_error(errors, min_errors \\ 1)
   def with_any_error(errors, min_errors) when length(errors) > min_errors, do: errors
@@ -127,13 +153,15 @@ defmodule RigInboundGateway.ApiProxy.Validations do
         |> Vex.valid?(%{"schema" => [presence: true]})
         |> with_nested_presence("target", endpoint)
 
+      endpoint_path_errors = validate_endpoint_path(endpoint)
+
       all_errors =
         validate_secured_endpoint(api, endpoint) ++
           with_any_error(topic_presence) ++
           with_any_error(stream_presence) ++
           schema_presence_config ++
           validate_string(endpoint, "id") ++
-          validate_string(endpoint, "path") ++ validate_string(endpoint, "method")
+          endpoint_path_errors ++ validate_string(endpoint, "method")
 
       merge_errors(acc, [{"#{id}/#{endpoint["id"]}", all_errors}])
     end)