From bfb4d52aa196575108fafdfb239f75e897aad4e5 Mon Sep 17 00:00:00 2001 From: Cory O'Daniel Date: Wed, 27 Nov 2019 12:07:22 -0800 Subject: [PATCH] Base.run/5 abstracted to middleware Refactored Base to use a middleware-based approach to handling requests. This is still a WIP. * [ ] Cluster middleware registry * [ ] Middleware error handling --- Makefile | 5 +- lib/k8s/client/runner/base.ex | 78 +++++++++---------- lib/k8s/middleware.ex | 17 +++- lib/k8s/middleware/request.ex | 3 +- lib/k8s/middleware/request/encode_body.ex | 25 ++++++ lib/k8s/middleware/request/initialize.ex | 22 ++---- test/k8s/client/runner/base_test.exs | 11 ++- .../middleware/request/encode_body_test.exs | 28 +++++++ .../middleware/request/initialize_test.exs | 23 ++++++ test/k8s/middleware_test.exs | 13 ---- 10 files changed, 150 insertions(+), 75 deletions(-) create mode 100644 lib/k8s/middleware/request/encode_body.ex create mode 100644 test/k8s/middleware/request/encode_body_test.exs create mode 100644 test/k8s/middleware/request/initialize_test.exs delete mode 100644 test/k8s/middleware_test.exs diff --git a/Makefile b/Makefile index daa30558..cd998f7c 100644 --- a/Makefile +++ b/Makefile @@ -5,6 +5,9 @@ help: ## Show this help help: @grep -E '^[\/a-zA-Z0-9._%-]+:.*?## .*$$' Makefile | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}' +quality: ## Run code quality and test targets +quality: lint test analyze + clean: ## Remove build/doc dirs rm -rf _build rm -rf cover @@ -33,7 +36,7 @@ test/master: ## Run test suite against master K8S_SPEC=${MASTER_SWAGGER_PATH} mix test test/all: ## Run full test suite against 1.10+ -test/all: test/1.10 test/1.11 test/1.12 test/1.13 test/1.14 test/1.15 +test/all: test/1.10 test/1.11 test/1.12 test/1.13 test/1.14 test/1.15 test/%: ## Run full test suite against a specific k8s version K8S_SPEC=test/support/swagger/$*.json mix test diff --git a/lib/k8s/client/runner/base.ex b/lib/k8s/client/runner/base.ex index 1cf945e1..ab71d96a 100644 --- a/lib/k8s/client/runner/base.ex +++ b/lib/k8s/client/runner/base.ex @@ -6,8 +6,8 @@ defmodule K8s.Client.Runner.Base do @type result_t :: {:ok, map() | reference()} | {:error, atom} | {:error, binary()} alias K8s.Cluster - alias K8s.Conn.RequestOptions alias K8s.Operation + alias K8s.Middleware.Request @doc """ Runs a `K8s.Operation`. @@ -72,64 +72,62 @@ defmodule K8s.Client.Runner.Base do @doc """ Run an operation and pass `opts` to HTTPoison. + Destructures `Operation` data and passes as the HTTP body. See `run/2` """ - @spec run(Operation.t(), binary | atom, keyword()) :: result_t + @spec run(Operation.t(), atom, keyword()) :: result_t def run(%Operation{} = operation, cluster_name, opts) when is_list(opts) do run(operation, cluster_name, operation.data, opts) end - def apply_middlewares(_cluster_name, headers, body) do - middlewares = [ - fn headers, body -> - IO.puts("Inspecting some body: #{inspect(body)}") - [headers, body] - end - ] - - state_args = [headers, body] - - Enum.reduce(middlewares, state_args, fn middleware, args -> - apply(middleware, args) - end) - end - @doc """ - Run an operation with an alternative HTTP Body (map) and pass `opts` to HTTPoison. + Run an operation with an HTTP Body (map) and pass `opts` to HTTPoison. See `run/2` """ @spec run(Operation.t(), atom, map(), keyword()) :: result_t def run(%Operation{} = operation, cluster, body, opts \\ []) do - with {:ok, url} <- Cluster.url_for(operation, cluster), - req <- %K8s.Middleware.Request{cluster: cluster, method: operation.method}, - {:ok, req} <- K8s.Middleware.Request.Initialize.call(req) do - - # ^ replace above with apply_middleware(cluster) + with req <- new_request(cluster, operation, body, opts), + {:ok, url} <- Cluster.url_for(operation, cluster), + # TODO: handle error return here type + {:ok, req} <- apply_middleware(req) do + K8s.http_provider().request(req.method, url, req.body, req.headers, req.opts) + end + end - # Operation.data and body are deconstructed above @ L79... - [http_headers, raw_body] = apply_middlewares(cluster, req.headers, body) + # TODO: handle error return here type + @spec apply_middleware(Request.t()) :: {:ok, Request.t()} + defp apply_middleware(req) do + middlewares = K8s.Middleware.list(:request, req.cluster) - {:ok, http_body} = encode(raw_body, operation.method) + # TODO: handle error return here type + # case( K8s.Middleware.Request | K8s.Middleware.Error(mw, req, actual_error)) - http_opts_params = build_http_params(opts[:params], operation.label_selector) - opts_with_selector_params = Keyword.put(opts, :params, http_opts_params) + updated_request = + Enum.reduce_while(middlewares, req, fn middleware, req -> + case apply(middleware, :call, [req]) do + {:ok, updated_request} -> + {:cont, updated_request} - http_opts = Keyword.merge(req.opts, opts_with_selector_params) + # TODO: handle error return here type + _error -> + {:halt, :handler_error_return_type_here} + end + end) - K8s.http_provider().request( - operation.method, - url, - http_body, - http_headers, - http_opts - ) - end + {:ok, updated_request} end - @spec encode(any(), atom()) :: {:ok, binary} | {:error, any} - def encode(body, http_method) when http_method in [:put, :patch, :post], do: Jason.encode(body) - def encode(_, _), do: {:ok, ""} + @spec new_request(atom(), K8s.Operation.t(), list(map()) | map() | binary() | nil, Keyword.t()) :: + Request.t() + defp new_request(cluster, %Operation{} = operation, body, opts) do + req = %Request{cluster: cluster, method: operation.method, body: body} + http_opts_params = build_http_params(opts[:params], operation.label_selector) + opts_with_selector_params = Keyword.put(opts, :params, http_opts_params) + + http_opts = Keyword.merge(req.opts, opts_with_selector_params) + %Request{req | opts: http_opts} + end @spec build_http_params(nil | keyword | map, nil | K8s.Selector.t()) :: map() defp build_http_params(nil, nil), do: %{} diff --git a/lib/k8s/middleware.ex b/lib/k8s/middleware.ex index c52fc3b0..25e171dd 100644 --- a/lib/k8s/middleware.ex +++ b/lib/k8s/middleware.ex @@ -1,4 +1,17 @@ -# defmodule K8s.Middleware do +defmodule K8s.Middleware do + @moduledoc "Interface for interacting with cluster middleware" + + @doc "Retrieve a list of middleware registered to a cluster" + @spec list(:request | :response, atom()) :: list(module()) + def list(:request, _cluster) do + [ + K8s.Middleware.Request.Initialize, + K8s.Middleware.Request.EncodeBody + ] + end +end + +# Agent should be in Middleware.Registry # use Agent # @doc """ @@ -21,5 +34,5 @@ # def increment do # Agent.update(__MODULE__, &(&1 + 1)) -# end +# end # end diff --git a/lib/k8s/middleware/request.ex b/lib/k8s/middleware/request.ex index dca949a0..b30a085c 100644 --- a/lib/k8s/middleware/request.ex +++ b/lib/k8s/middleware/request.ex @@ -1,7 +1,7 @@ defmodule K8s.Middleware.Request do @moduledoc "HTTP Request middleware" - @typedoc "MIddleware Request type" + @typedoc "Middleware Request type" @type t :: %__MODULE__{ cluster: atom(), method: atom(), @@ -14,5 +14,6 @@ defmodule K8s.Middleware.Request do defstruct cluster: nil, method: nil, url: nil, body: nil, headers: [], opts: [] @doc "Request middleware callback" + # TODO: handle error return here type @callback call(t()) :: {:ok, t()} | :error end diff --git a/lib/k8s/middleware/request/encode_body.ex b/lib/k8s/middleware/request/encode_body.ex new file mode 100644 index 00000000..16b2173a --- /dev/null +++ b/lib/k8s/middleware/request/encode_body.ex @@ -0,0 +1,25 @@ +defmodule K8s.Middleware.Request.EncodeBody do + @moduledoc """ + Naive JSON body encoder. + + Encodes JSON payloads when given an modifiying HTTP verb, otherwise returns an empty string. + """ + @behaviour K8s.Middleware.Request + alias K8s.Middleware.Request + + @impl true + def call(%Request{method: method, body: body} = req) do + case encode(body, method) do + {:ok, encoded_body} -> + req = %Request{req | body: encoded_body} + {:ok, req} + + error -> + error + end + end + + @spec encode(any(), atom()) :: {:ok, binary} | {:error, any} + defp encode(body, http_method) when http_method in [:put, :patch, :post], do: Jason.encode(body) + defp encode(_, _), do: {:ok, ""} +end diff --git a/lib/k8s/middleware/request/initialize.ex b/lib/k8s/middleware/request/initialize.ex index 9fd7ccb0..d58095f4 100644 --- a/lib/k8s/middleware/request/initialize.ex +++ b/lib/k8s/middleware/request/initialize.ex @@ -1,24 +1,18 @@ defmodule K8s.Middleware.Request.Initialize do + @moduledoc """ + Initializes a request with connection details (header and HTTPoison opts) from `K8s.Conn.RequestOptions` + """ @behaviour K8s.Middleware.Request - @doc """ + alias K8s.Middleware.Request - ## Examples - iex> conn = K8s.Conn.from_file("./test/support/kube-config.yaml") - ...> K8s.Cluster.Registry.add(:test_cluster, conn) - ...> request = %K8s.Middleware.Request{cluster: :test_cluster} - ...> K8s.Middleware.Request.Initialize.call(request) - {:ok, %K8s.Middleware.Request{cluster: :test_cluster, headers: [{"Accept", "application/json"}, {"Content-Type", "application/json"}], opts: [ssl: [cert: ""]]}} - """ @impl true - def call(%K8s.Middleware.Request{cluster: cluster, method: method, headers: headers, opts: opts} = req) do + def call(%Request{cluster: cluster, method: method, headers: headers, opts: opts} = req) do with {:ok, conn} <- K8s.Cluster.conn(cluster), {:ok, request_options} <- K8s.Conn.RequestOptions.generate(conn) do - new_headers = K8s.http_provider().headers(method, request_options) - updated_headers = Keyword.merge(headers, new_headers) + request_option_headers = K8s.http_provider().headers(method, request_options) + updated_headers = Keyword.merge(headers, request_option_headers) updated_opts = Keyword.merge([ssl: request_options.ssl_options], opts) - - updated_request = %K8s.Middleware.Request{req| headers: updated_headers, opts: updated_opts} - + updated_request = %Request{req | headers: updated_headers, opts: updated_opts} {:ok, updated_request} end end diff --git a/test/k8s/client/runner/base_test.exs b/test/k8s/client/runner/base_test.exs index 608d58ae..f5d9f43b 100644 --- a/test/k8s/client/runner/base_test.exs +++ b/test/k8s/client/runner/base_test.exs @@ -15,7 +15,7 @@ defmodule K8s.Client.Runner.BaseTest do def request(:get, @namespaced_url, _, _, _), do: render(nil) - def request(:post, @namespaced_url, _, _, _), do: render(nil) + def request(:post, @namespaced_url, body, _, _), do: render(body) def request(:get, @namespaced_url <> "/test", _body, _headers, _opts) do render(nil) @@ -45,11 +45,11 @@ defmodule K8s.Client.Runner.BaseTest do def request( :post, @base_url <> "/api/v1/namespaces/default/pods/nginx/eviction", - _body, + body, _headers, _opts ) do - render(nil) + render(body) end end @@ -98,7 +98,10 @@ defmodule K8s.Client.Runner.BaseTest do labels = %{"env" => "test"} body = put_in(make_namespace("test"), ["metadata", "labels"], labels) - assert {:ok, _} = Base.run(operation, cluster, body) + assert {:ok, body} = Base.run(operation, cluster, body) + + assert body == + ~s({"apiVersion":"v1","kind":"Namespace","metadata":{"labels":{"env":"test"},"name":"test"}}) end test "running an operation with a custom HTTP body and options", %{ diff --git a/test/k8s/middleware/request/encode_body_test.exs b/test/k8s/middleware/request/encode_body_test.exs new file mode 100644 index 00000000..af7904d9 --- /dev/null +++ b/test/k8s/middleware/request/encode_body_test.exs @@ -0,0 +1,28 @@ +defmodule K8s.Middleware.Request.EncodeBodyTest do + use ExUnit.Case, async: true + + test "encode JSON payloads when given a modifying HTTP verb" do + data = %{"hello" => "world"} + request = %K8s.Middleware.Request{body: data, method: :put} + {:ok, %{body: body}} = K8s.Middleware.Request.EncodeBody.call(request) + + assert body == ~s({"hello":"world"}) + end + + test "returns an empty string if not a modifying verb" do + data = %{"hello" => "world"} + request = %K8s.Middleware.Request{body: data, method: :get} + {:ok, %{body: body}} = K8s.Middleware.Request.EncodeBody.call(request) + + assert body == "" + end + + # TODO: handle error return here type + # test "failure" do + # data = [should: :fail] + # request = %K8s.Middleware.Request{body: data, method: :post} + # {:ok, %{body: body}} = K8s.Middleware.Request.EncodeBody.call(request) + + # assert body == "" + # end +end diff --git a/test/k8s/middleware/request/initialize_test.exs b/test/k8s/middleware/request/initialize_test.exs new file mode 100644 index 00000000..b570c4a3 --- /dev/null +++ b/test/k8s/middleware/request/initialize_test.exs @@ -0,0 +1,23 @@ +defmodule K8s.Middleware.Request.InitializeTest do + use ExUnit.Case, async: true + + test "initializes a request headers from K8s.Conn.RequestOptions" do + conn = K8s.Conn.from_file("./test/support/kube-config.yaml") + K8s.Cluster.Registry.add(:test_cluster, conn) + + request = %K8s.Middleware.Request{cluster: :test_cluster} + {:ok, %{headers: headers}} = K8s.Middleware.Request.Initialize.call(request) + + assert headers == [{"Accept", "application/json"}, {"Content-Type", "application/json"}] + end + + test "initializes a HTTPoison options from K8s.Conn.RequestOptions" do + conn = K8s.Conn.from_file("./test/support/kube-config.yaml") + K8s.Cluster.Registry.add(:test_cluster, conn) + + request = %K8s.Middleware.Request{cluster: :test_cluster} + {:ok, %{opts: opts}} = K8s.Middleware.Request.Initialize.call(request) + + assert Keyword.has_key?(opts, :ssl) + end +end diff --git a/test/k8s/middleware_test.exs b/test/k8s/middleware_test.exs deleted file mode 100644 index 56ed6b12..00000000 --- a/test/k8s/middleware_test.exs +++ /dev/null @@ -1,13 +0,0 @@ -defmodule K8s.MiddlewareTest do - use ExUnit.Case, async: true - doctest K8s.Middleware.Request.Initialize - - # TODO: - # # K8s.Middleware.Request.EncodeBody - # # K8s.Middleware.Request.DefaultParams - # # K8s.Middleware.Request.DefaultHTTPOpts - # # K8s.Middleware.Request.BaseURL (DiscoveryCluster?) - # <- Actually this step is Cluster.url_for ... includes path... - # <- May need to visit after JIT Registry - # Decide on K8s.Middleware behavior callback error types... {:error, t()} | {:error, String.t(), t()} -end