Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Policy: HTTP proxy #1080

Merged
merged 6 commits into from
Jul 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

- Introduce possibility of specifying policy order restrictions in their schemas. APIcast now shows a warning when those restrictions are not respected [#1088](https://github.com/3scale/APIcast/pull/1088), [THREESCALE-2896](https://issues.jboss.org/browse/THREESCALE-2896)
- Added new parameters to logging policy to allow custom access log [PR #1081](https://github.com/3scale/APIcast/pull/1089) [THREESCALE-1234](https://issues.jboss.org/browse/THREESCALE-1234)[THREESCALE-2876](https://issues.jboss.org/browse/THREESCALE-2876)
- Added http_proxy policy to use an HTTP proxy in api_backed calls. [THREESCALE-2696](https://issues.jboss.org/browse/THREESCALE-2696) [PR #1080](https://github.com/3scale/APIcast/pull/1080)

## [3.6.0-rc1] - 2019-07-04

Expand Down
9 changes: 7 additions & 2 deletions gateway/src/apicast/http_proxy.lua
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ local function absolute_url(uri)
uri.scheme,
host,
port,
uri.path or ''
uri.path or '/'
)
end

Expand All @@ -97,6 +97,7 @@ local function forward_https_request(proxy_uri, uri)
-- In POST requests with HTTPS, the result of that call is nil, and it
-- results in a time-out.
body = ngx.req.get_body_data(),
proxy_uri = proxy_uri
}

local httpc, err = http_proxy.new(request)
Expand Down Expand Up @@ -138,8 +139,12 @@ function _M.request(upstream, proxy_uri)
local uri = upstream.uri

if uri.scheme == 'http' then -- rewrite the request to use http_proxy
local err
upstream:use_host_header(uri.host) -- to keep correct Host header in case we need to resolve it to IP
upstream.servers = resolve_servers(proxy_uri)
upstream.servers, err = resolve_servers(proxy_uri)
if err then
ngx.log(ngx.WARN, "HTTP proxy is set, but no servers have been resolved, err: ", err)
end
upstream.uri.path = absolute_url(uri)
upstream:rewrite_request()
return
Expand Down
81 changes: 81 additions & 0 deletions gateway/src/apicast/policy/http_proxy/Readme.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
# HTTP proxy policy

This policy allows users to define a HTTP proxy where the traffic will be send
over the defined proxy, the example traffic flow is the following:

```
,-.
`-'
/|\
| ,-------. ,---------. ,----------.
/ \ |Apicast| |HTTPPROXY| |APIBackend|
User `---+---' `----+----' `----------'
| GET /resource | | |
| --------------->| | |
| | | |
| | Get /resource | |
| |------------------>| |
| | | |
| | | Get /resource/ |
| | | - - - - - - - - - >|
| | | |
| | | response |
| | |<- - - - - - - - - -|
| | | |
| | response | |
| |<------------------| |
| | | |
| | | |
| <---------------| | |
User ,---+---. ,----+----. ,----------.
,-. |Apicast| |HTTPPROXY| |APIBackend|
`-' `-------' `---------' `----------'
/|\
|
/ \
```

All APIcast traffic to 3scale backend will not use the proxy, due to this only
applies for the service and the communication between Apicast and API backend.

If you want to use all traffic through a Proxy, an HTTP_PROXY env var need to be
used.

## Configuration

```
"policy_chain": [
{
"name": "apicast.policy.apicast"
},
{
"name": "apicast.policy.http_proxy",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's important to clarify that this only applies to the API backend and not the 3scale backend (apisonator).

"configuration": {
"all_proxy": "http://192.168.15.103:8888/",
"https_proxy": "https://192.168.15.103:8888/",
"http_proxy": "https://192.168.15.103:8888/"
}
}
]
```

- If http_proxy or https_proxy is not defined the all_proxy will be taken.

## Caveats

- This policy will disable all load-balancing policies and traffic will be
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Asking just in case. Does the order in the chain matter?

always send to the proxy.
- In case of HTTP_PROXY, HTTPS_PROXY or ALL_PROXY parameters are defined, this
policy will overwrite those values.
- Proxy connection does not support authentication.


## Example Use case

This policy was designed to be able to apply more fined grained policies and
transformation using Apache Camel.

An example project can be found
[here](https://github.com/zregvart/camel-netty-proxy). This project is an HTTP
Proxy that transforms to uppercase all the response body given by the API
backend.
27 changes: 27 additions & 0 deletions gateway/src/apicast/policy/http_proxy/apicast-policy.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
{
"$schema": "http://apicast.io/policy-v1/schema#manifest#",
"name": "Proxy service",
"summary": "Adds an HTTP proxy to the service.",
"description": [
"With this policy all the traffic for this service will be routed accross ",
"the defined proxy"
],
"version": "builtin",
"configuration": {
"type": "object",
"properties": {
"all_proxy": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understood correctly, all_proxy is only used when the others are not set. I wonder if we can prevent invalid configs (setting both all_proxy, and the other 2) by changing the schema.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took care in the code, also added a unittest.

it("all_proxy does not overwrite http/https proxies", function()
    local proxy = proxy_policy.new({
      all_proxy = all_proxy_val,
      http_proxy = http_proxy_val,
      https_proxy = https_proxy_val
    })
    local callback = proxy:export()

    assert.same(callback.has_http_proxy(_, http_uri), resty_url.parse(http_proxy_val))
    assert.same(callback.has_http_proxy(_, https_uri), resty_url.parse(https_proxy_val))
  end)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍
I think the description of all_proxy or the description above should mention this. Otherwise, the users will not be aware of this when looking at the form to configure the policy.

"description": "Defines a HTTP proxy to be used for connecting to services if a protocol-specific proxy is not specified. Authentication is not supported.",
"type": "string"
},
"https_proxy": {
"description": "Defines a HTTPS proxy to be used for connecting to HTTPS services. Authentication is not supported",
"type": "string"
},
"http_proxy": {
"description": "Defines a HTTP proxy to be used for connecting to HTTP services. Authentication is not supported",
"type": "string"
}
}
}
}
1 change: 1 addition & 0 deletions gateway/src/apicast/policy/http_proxy/init.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
return require("proxy")
48 changes: 48 additions & 0 deletions gateway/src/apicast/policy/http_proxy/proxy.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
local policy = require('apicast.policy')
local _M = policy.new('http_proxy', 'builtin')

local resty_url = require 'resty.url'
local ipairs = ipairs

local new = _M.new

local proxies = {"http", "https"}

function _M.new(config)
local self = new(config)
self.proxies = {}

if config.all_proxy then
local err
self.all_proxy, err = resty_url.parse(config.all_proxy)
if err then
ngx.log(ngx.WARN, "All proxy '", config.all_proxy, "' is not correctly defined, err:", err)
end
end

for _, proto in ipairs(proxies) do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

local ipairs = ipairs ?

local val, err = resty_url.parse(config[string.format("%s_proxy", proto)])
if err then
ngx.log(ngx.WARN, proto, " proxy is not correctly defined, err: ", err)
end
self.proxies[proto] = val or self.all_proxy
end
return self
end

local function find_proxy(self, scheme)
return self.proxies[scheme]
end

function _M:export()
return {
get_http_proxy = function(uri)
if not uri.scheme then
return nil
end
return find_proxy(self, uri.scheme)
end
}
end

return _M
12 changes: 10 additions & 2 deletions gateway/src/apicast/upstream.lua
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,19 @@ end
function _M:call(context)
if ngx.headers_sent then return nil, 'response sent already' end

local proxy_uri = http_proxy.find(self)
local proxy_uri

-- get_http_proxy is a property set by the http_proxy policy
if context.get_http_proxy then
proxy_uri = context.get_http_proxy(self.uri)
else
proxy_uri = http_proxy.find(self)
end

if proxy_uri then
ngx.log(ngx.DEBUG, 'using proxy: ', proxy_uri)
-- https requests will be terminated, http will be rewritten and sent to a proxy
-- https requests will be terminated, http will be rewritten and sent
-- to a proxy
http_proxy.request(self, proxy_uri)
else
self:rewrite_request()
Expand Down
4 changes: 3 additions & 1 deletion gateway/src/resty/http/proxy.lua
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,9 @@ local function find_proxy_url(request)
local uri = parse_request_uri(request)
if not uri then return end

return _M.find(uri)
-- request can have a local proxy defined and env variables have lower
-- priority, if the proxy is defined in the request that will be used.
return request.proxy_uri or _M.find(uri)
end

local function connect(request)
Expand Down
61 changes: 61 additions & 0 deletions spec/policy/http_proxy/http_proxy_spec.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
local proxy_policy = require('apicast.policy.http_proxy')
local resty_url = require 'resty.url'

describe('HTTP proxy policy', function()
local all_proxy_val = "http://all.com"
local http_proxy_val = "http://plain.com"
local https_proxy_val = "http://secure.com"

local http_uri = {scheme="http"}
local https_uri = {scheme="https"}

it("http[s] proxies are defined if all_proxy is in there", function()
local proxy = proxy_policy.new({
all_proxy = all_proxy_val
})
local callback = proxy:export()

assert.same(callback.get_http_proxy(http_uri), resty_url.parse(all_proxy_val))
assert.same(callback.get_http_proxy(https_uri), resty_url.parse(all_proxy_val))
end)

it("all_proxy does not overwrite http/https proxies", function()
local proxy = proxy_policy.new({
all_proxy = all_proxy_val,
http_proxy = http_proxy_val,
https_proxy = https_proxy_val
})
local callback = proxy:export()

assert.same(callback.get_http_proxy(http_uri), resty_url.parse(http_proxy_val))
assert.same(callback.get_http_proxy(https_uri), resty_url.parse(https_proxy_val))
end)

it("empty config return all nil", function()
local proxy = proxy_policy.new({})
local callback = proxy:export()

assert.is_nil(callback.get_http_proxy(https_uri))
assert.is_nil(callback.get_http_proxy(http_uri))
end)

describe("get_http_proxy callback", function()
local callback = proxy_policy.new({
all_proxy = all_proxy_val
}):export()

it("Valid protocol", function()

local result = callback.get_http_proxy(
resty_url.parse("http://google.com"))
assert.same(result, resty_url.parse(all_proxy_val))
end)

it("invalid protocol", function()
local result = callback:get_http_proxy(
{}, {scheme="invalid"})
assert.is_nil(result)
end)

end)
end)
Loading