From da4eaec0b2c75418943b3e834f11e00aee8c5b0d Mon Sep 17 00:00:00 2001
From: Eloy Coto <eloy.coto@gmail.com>
Date: Tue, 10 Mar 2020 19:49:17 +0100
Subject: [PATCH] GRPC policy: Fix routing upstream issue.

This commit makes things super simpler in GRPC, and avoid the
miss-configuration that was made previously around upstream&balancers.

Before on GRPC we set the balancer, and all the balancer logic happened
on the policy itself, instead of using the apicast.upstream and
apicast.balancer.

With this change, the location is changed on the upstream.lua, so it
does not matter if was set in the apicast.policy, or in another policy,
in this case the routing policy.

This finally fix THREESCALE-4684

Signed-off-by: Eloy Coto <eloy.coto@gmail.com>
---
 CHANGELOG.md                             |  2 +-
 gateway/src/apicast/policy/grpc/grpc.lua | 51 ++----------------------
 gateway/src/apicast/upstream.lua         |  4 +-
 3 files changed, 7 insertions(+), 50 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1a8a672ed..a587d54d2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 - Fixed naming issues in policies [THREESCALE-4150](https://issues.jboss.org/browse/THREESCALE-4150) [PR #1167](https://github.com/3scale/APIcast/pull/1167)
 - Fixed issues on invalid config in logging policy [THREESCALE-4605](https://issues.jboss.org/browse/THREESCALE-4605) [PR #1168](https://github.com/3scale/APIcast/pull/1168)
-- Fixed issues with routing policy and GRPC one [THREESCALE-4684](https://issues.jboss.org/browse/THREESCALE-4684) [PR #1177](https://github.com/3scale/APIcast/pull/1177)
+- Fixed issues with routing policy and GRPC one [THREESCALE-4684](https://issues.jboss.org/browse/THREESCALE-4684) [PR #1177](https://github.com/3scale/APIcast/pull/1177) [PR #1179](https://github.com/3scale/APIcast/pull/1179)
 
 ## [3.7.0-alpha2]
 
diff --git a/gateway/src/apicast/policy/grpc/grpc.lua b/gateway/src/apicast/policy/grpc/grpc.lua
index 22df2938d..e40844cc8 100644
--- a/gateway/src/apicast/policy/grpc/grpc.lua
+++ b/gateway/src/apicast/policy/grpc/grpc.lua
@@ -2,66 +2,21 @@
 
 local policy = require('apicast.policy')
 local _M = policy.new('grpc', "builtin")
-local resty_url = require('resty.url')
-local round_robin = require 'resty.balancer.round_robin'
 
+local apicast_balancer = require('apicast.balancer')
 local new = _M.new
 
-local balancer = round_robin.new()
-
 function _M.new(config)
   local self = new(config)
   return self
 end
 
 function _M:rewrite(context)
-  if ngx.var.server_protocol == "HTTP/2.0" then
     -- upstream defined in gateway/conf.d/http2.conf
     context.upstream_location_name = "@grpc_upstream"
-  end
-end
-
-function _M:content(context)
-  -- This is needed within the combination of the routing policy, if not the
-  -- upstream got overwritten and balancer phase is called before.
-  if not context.upstream_location_name then
-    return
-  end
-
-  if ngx.var.server_protocol ~= "HTTP/2.0" then
-    ngx.var.host = context.upstream_location_name
-  end
-
+    ngx.var.proxy_host = "upstream"
 end
 
-function _M:balancer(context)
-  if not context.upstream_location_name then
-    return
-  end
-
-  -- balancer need to be used due to grpc_pass does not support variables and
-  -- upstream block need to be in place.
-  local upstream = context:get_upstream()
-  if not upstream then
-    ngx.log(ngx.WARN, "Upstream is not present in the balancer")
-    return
-  end
-
-  local peers = balancer:peers(upstream.servers)
-  local peer, err = balancer:select_peer(peers)
-  if err then
-    ngx.log(ngx.WARN, "Cannot get a peer for the given upstream: ", err)
-    return
-  end
-
-  local ip = peer[1]
-  local port = peer[2] or upstream.uri.port or resty_url.default_port(upstream.uri.scheme)
-  local _, err = balancer:set_current_peer(ip, port)
-
-  if err then
-    ngx.log(ngx.WARN, "Cannot set balancer IP and port '", ip, ":", port, "'")
-    return
-  end
-end
+_M.balancer = apicast_balancer.call
 
 return _M
diff --git a/gateway/src/apicast/upstream.lua b/gateway/src/apicast/upstream.lua
index 74221974e..b4c246dcb 100644
--- a/gateway/src/apicast/upstream.lua
+++ b/gateway/src/apicast/upstream.lua
@@ -221,7 +221,9 @@ function _M:call(context)
     end
 
     if not self.servers then self:resolve() end
-
+    if context.upstream_location_name then
+        self.location_name = context.upstream_location_name
+    end
     context[self.upstream_name] = self
 
     return exec(self)