Merge pull request #4592 from ElvinEfendi/force-ssl-redirect-refactoring

refactor force ssl redirect logic

internal/ingress/controller/template/template.go

@@ -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

rootfs/etc/nginx/lua/certificate.lua

@@ -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

rootfs/etc/nginx/lua/lua_ingress.lua

@@ -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
@@ -54,8 +56,20 @@ local function randomseed()
   math.randomseed(seed)
 end
 
-local function redirect_to_https()
+local function redirect_to_https(location_config)
-  return ngx.var.pass_access_scheme == "http" and (ngx.var.scheme == "http" or ngx.var.scheme == "https")
+  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()
@@ -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

rootfs/etc/nginx/lua/test/certificate_test.lua

@@ -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)

rootfs/etc/nginx/template/nginx.tmpl

@@ -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()
             }