Skip to content

Commit

Permalink
Merge pull request #4592 from ElvinEfendi/force-ssl-redirect-refactoring
Browse files Browse the repository at this point in the history
refactor force ssl redirect logic
  • Loading branch information
k8s-ci-robot authored Sep 24, 2019
2 parents 73bc2cf + 8c64b12 commit 56edbb9
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 15 deletions.
20 changes: 9 additions & 11 deletions internal/ingress/controller/template/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -306,32 +306,30 @@ func configForLua(input interface{}) string {
}

// locationConfigForLua formats some location specific configuration into Lua table represented as string
func locationConfigForLua(l interface{}, s interface{}, a interface{}) string {
func locationConfigForLua(l interface{}, a interface{}) string {
location, ok := l.(*ingress.Location)
if !ok {
klog.Errorf("expected an '*ingress.Location' type but %T was given", l)
return "{}"
}

server, ok := s.(*ingress.Server)
if !ok {
klog.Errorf("expected an '*ingress.Server' type but %T was given", s)
return "{}"
}

all, ok := a.(config.TemplateConfig)
if !ok {
klog.Errorf("expected a 'config.TemplateConfig' type but %T was given", a)
return "{}"
}

forceSSLRedirect := location.Rewrite.ForceSSLRedirect || (server.SSLCert != nil && location.Rewrite.SSLRedirect)
forceSSLRedirect = forceSSLRedirect && !isLocationInLocationList(l, all.Cfg.NoTLSRedirectLocations)

return fmt.Sprintf(`{
force_ssl_redirect = %t,
ssl_redirect = %t,
force_no_ssl_redirect = %t,
use_port_in_redirects = %t,
}`, forceSSLRedirect, location.UsePortInRedirects)
}`,
location.Rewrite.ForceSSLRedirect,
location.Rewrite.SSLRedirect,
isLocationInLocationList(l, all.Cfg.NoTLSRedirectLocations),
location.UsePortInRedirects,
)
}

// buildResolvers returns the resolvers reading the /etc/resolv.conf file
Expand Down
8 changes: 8 additions & 0 deletions rootfs/etc/nginx/lua/certificate.lua
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,14 @@ local function get_pem_cert_key(raw_hostname)
return pem_cert_key
end

function _M.configured_for_server(hostname)
if not hostname then
return false
end

return get_pem_cert_key(hostname) ~= nil
end

function _M.call()
local hostname, hostname_err = ssl.server_name()
if hostname_err then
Expand Down
20 changes: 17 additions & 3 deletions rootfs/etc/nginx/lua/lua_ingress.lua
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
local ngx_re_split = require("ngx.re").split

local certificate_configured_for_server = require("certificate").configured_for_server

local original_randomseed = math.randomseed
local string_format = string.format
local ngx_redirect = ngx.redirect
Expand Down Expand Up @@ -54,8 +56,20 @@ local function randomseed()
math.randomseed(seed)
end

local function redirect_to_https()
return ngx.var.pass_access_scheme == "http" and (ngx.var.scheme == "http" or ngx.var.scheme == "https")
local function redirect_to_https(location_config)
if location_config.force_no_ssl_redirect then
return false
end

if ngx.var.pass_access_scheme ~= "http" then
return false
end

if location_config.force_ssl_redirect then
return true
end

return location_config.ssl_redirect and certificate_configured_for_server(ngx.var.host)
end

local function redirect_host()
Expand Down Expand Up @@ -119,7 +133,7 @@ function _M.rewrite(location_config)
ngx.var.pass_port = 443
end

if location_config.force_ssl_redirect and redirect_to_https() then
if redirect_to_https(location_config) then
local uri = string_format("https://%s%s", redirect_host(), ngx.var.request_uri)

if location_config.use_port_in_redirects then
Expand Down
18 changes: 18 additions & 0 deletions rootfs/etc/nginx/lua/test/certificate_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -129,4 +129,22 @@ describe("Certificate", function()
assert.spy(ngx.log).was_called_with(ngx.ERR, "failed to convert certificate chain from PEM to DER: PEM_read_bio_X509_AUX() failed")
end)
end)

describe("configured_for_server", function()
before_each(function()
set_certificate("hostname", EXAMPLE_CERT, UUID)
end)

it("returns true when certificate exists for given server", function()
assert.is_true(certificate.configured_for_server("hostname"))
end)

it("returns false when certificate does not exist for given server", function()
assert.is_false(certificate.configured_for_server("hostname.xyz"))
end)

it("returns false when no server given", function()
assert.is_false(certificate.configured_for_server())
end)
end)
end)
2 changes: 1 addition & 1 deletion rootfs/etc/nginx/template/nginx.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -967,7 +967,7 @@ stream {
{{ end }}

rewrite_by_lua_block {
lua_ingress.rewrite({{ locationConfigForLua $location $server $all }})
lua_ingress.rewrite({{ locationConfigForLua $location $all }})
balancer.rewrite()
plugins.run()
}
Expand Down

0 comments on commit 56edbb9

Please sign in to comment.