Skip to content

NGINX: Remove inline Lua from template. #11806

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

Merged
merged 8 commits into from
Sep 8, 2024
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
16 changes: 16 additions & 0 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,22 @@ jobs:
- 'images/nginx-1.25/**'
docs:
- '**/*.md'
lua:
- '**/*.lua'
lua-lint:
runs-on: ubuntu-latest
needs: changes
if: |
(needs.changes.outputs.lua == 'true') || ${{ github.event.workflow_dispatch.run_e2e == 'true' }}
Comment on lines +80 to +81
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if: |
(needs.changes.outputs.lua == 'true') || ${{ github.event.workflow_dispatch.run_e2e == 'true' }}
if: fromJSON(needs.changes.outputs.lua) || fromJSON(github.event.workflow_dispatch.run_e2e)

Have you tried this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

never tried. but then wouldn't it be better to change all of the ifs for this approach on a follow up PR?

steps:
- name: Checkout
uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7

- name: Lint Lua
uses: lunarmodules/luacheck@v1
with:
args: --codes --globals lua_ingress --globals configuration --globals balancer --globals monitor --globals certificate --globals tcp_udp_configuration --globals tcp_udp_balancer --no-max-comment-line-length -q rootfs/etc/nginx/lua/

test-go:
runs-on: ubuntu-latest
Expand Down
9 changes: 8 additions & 1 deletion hack/verify-lualint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ set -o errexit
set -o nounset
set -o pipefail

luacheck --codes -q rootfs/etc/nginx/lua/
luacheck --codes --globals lua_ingress \
--globals configuration \
--globals balancer \
--globals monitor \
--globals certificate \
--globals tcp_udp_configuration \
--globals tcp_udp_balancer \
--no-max-comment-line-length -q rootfs/etc/nginx/lua/

find rootfs/etc/nginx/lua/ -name "*.lua" -not -path "*/test/*" -exec lj-releng -L -s {} + && echo "lj-releng validation is success!"
30 changes: 30 additions & 0 deletions internal/ingress/controller/nginx.go
Original file line number Diff line number Diff line change
Expand Up @@ -690,6 +690,10 @@ func (n *NGINXController) OnUpdate(ingressCfg ingress.Configuration) error {
return err
}

err = n.createLuaConfig(&cfg)
if err != nil {
return err
}
err = createOpentelemetryCfg(&cfg)
if err != nil {
return err
Expand Down Expand Up @@ -1079,6 +1083,32 @@ func createOpentelemetryCfg(cfg *ngx_config.Configuration) error {
return os.WriteFile(cfg.OpentelemetryConfig, tmplBuf.Bytes(), file.ReadWriteByUser)
}

func (n *NGINXController) createLuaConfig(cfg *ngx_config.Configuration) error {
luaconfigs := &ngx_template.LuaConfig{
EnableMetrics: n.cfg.EnableMetrics,
ListenPorts: ngx_template.LuaListenPorts{
HTTPSPort: strconv.Itoa(n.cfg.ListenPorts.HTTPS),
StatusPort: strconv.Itoa(nginx.StatusPort),
SSLProxyPort: strconv.Itoa(n.cfg.ListenPorts.SSLProxy),
},
UseProxyProtocol: cfg.UseProxyProtocol,
UseForwardedHeaders: cfg.UseForwardedHeaders,
IsSSLPassthroughEnabled: n.cfg.EnableSSLPassthrough,
HTTPRedirectCode: cfg.HTTPRedirectCode,
EnableOCSP: cfg.EnableOCSP,
MonitorBatchMaxSize: n.cfg.MonitorMaxBatchSize,
HSTS: cfg.HSTS,
HSTSMaxAge: cfg.HSTSMaxAge,
HSTSIncludeSubdomains: cfg.HSTSIncludeSubdomains,
HSTSPreload: cfg.HSTSPreload,
}
jsonCfg, err := json.Marshal(luaconfigs)
if err != nil {
return err
}
return os.WriteFile(luaCfgPath, jsonCfg, file.ReadWriteByUser)
}

func cleanTempNginxCfg() error {
var files []string

Expand Down
93 changes: 50 additions & 43 deletions internal/ingress/controller/template/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,41 @@ func cleanConf(in, out *bytes.Buffer) error {
}
}

/* LuaConfig defines the structure that will be written as a config for lua scripts
The json format should follow what's expected by lua:
use_forwarded_headers = %t,
use_proxy_protocol = %t,
is_ssl_passthrough_enabled = %t,
http_redirect_code = %v,
listen_ports = { ssl_proxy = "%v", https = "%v" },

hsts = %t,
hsts_max_age = %v,
hsts_include_subdomains = %t,
hsts_preload = %t,
*/

type LuaConfig struct {
EnableMetrics bool `json:"enable_metrics"`
ListenPorts LuaListenPorts `json:"listen_ports"`
UseForwardedHeaders bool `json:"use_forwarded_headers"`
UseProxyProtocol bool `json:"use_proxy_protocol"`
IsSSLPassthroughEnabled bool `json:"is_ssl_passthrough_enabled"`
HTTPRedirectCode int `json:"http_redirect_code"`
EnableOCSP bool `json:"enable_ocsp"`
MonitorBatchMaxSize int `json:"monitor_batch_max_size"`
HSTS bool `json:"hsts"`
HSTSMaxAge string `json:"hsts_max_age"`
HSTSIncludeSubdomains bool `json:"hsts_include_subdomains"`
HSTSPreload bool `json:"hsts_preload"`
}

type LuaListenPorts struct {
HTTPSPort string `json:"https"`
StatusPort string `json:"status_port"`
SSLProxyPort string `json:"ssl_proxy"`
}

// Write populates a buffer using a template with NGINX configuration
// and the servers and upstreams created by Ingress rules
func (t *Template) Write(conf *config.TemplateConfig) ([]byte, error) {
Expand Down Expand Up @@ -256,7 +291,6 @@ var funcMap = text_template.FuncMap{
"filterRateLimits": filterRateLimits,
"buildRateLimitZones": buildRateLimitZones,
"buildRateLimit": buildRateLimit,
"configForLua": configForLua,
"locationConfigForLua": locationConfigForLua,
"buildResolvers": buildResolvers,
"buildUpstreamName": buildUpstreamName,
Expand Down Expand Up @@ -383,41 +417,6 @@ func luaConfigurationRequestBodySize(c interface{}) string {
return dictKbToStr(size)
}

// configForLua returns some general configuration as Lua table represented as string
func configForLua(input interface{}) string {
all, ok := input.(config.TemplateConfig)
if !ok {
klog.Errorf("expected a 'config.TemplateConfig' type but %T was given", input)
return "{}"
}

return fmt.Sprintf(`{
use_forwarded_headers = %t,
use_proxy_protocol = %t,
is_ssl_passthrough_enabled = %t,
http_redirect_code = %v,
listen_ports = { ssl_proxy = "%v", https = "%v" },

hsts = %t,
hsts_max_age = %v,
hsts_include_subdomains = %t,
hsts_preload = %t,

}`,
all.Cfg.UseForwardedHeaders,
all.Cfg.UseProxyProtocol,
all.IsSSLPassthroughEnabled,
all.Cfg.HTTPRedirectCode,
all.ListenPorts.SSLProxy,
all.ListenPorts.HTTPS,

all.Cfg.HSTS,
all.Cfg.HSTSMaxAge,
all.Cfg.HSTSIncludeSubdomains,
all.Cfg.HSTSPreload,
)
}

// locationConfigForLua formats some location specific configuration into Lua table represented as string
func locationConfigForLua(l, a interface{}) string {
location, ok := l.(*ingress.Location)
Expand All @@ -432,13 +431,21 @@ func locationConfigForLua(l, a interface{}) string {
return "{}"
}

return fmt.Sprintf(`{
force_ssl_redirect = %t,
ssl_redirect = %t,
force_no_ssl_redirect = %t,
preserve_trailing_slash = %t,
use_port_in_redirects = %t,
}`,
/* Lua expects the following vars
force_ssl_redirect = string_to_bool(ngx.var.force_ssl_redirect),
ssl_redirect = string_to_bool(ngx.var.ssl_redirect),
force_no_ssl_redirect = string_to_bool(ngx.var.force_no_ssl_redirect),
preserve_trailing_slash = string_to_bool(ngx.var.preserve_trailing_slash),
use_port_in_redirects = string_to_bool(ngx.var.use_port_in_redirects),
*/

return fmt.Sprintf(`
set $force_ssl_redirect "%t";
set $ssl_redirect "%t";
set $force_no_ssl_redirect "%t";
set $preserve_trailing_slash "%t";
set $use_port_in_redirects "%t";
`,
location.Rewrite.ForceSSLRedirect,
location.Rewrite.SSLRedirect,
isLocationInLocationList(l, all.Cfg.NoTLSRedirectLocations),
Expand Down
5 changes: 3 additions & 2 deletions internal/ingress/controller/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,9 @@ func rlimitMaxNumFiles() int {
}

const (
defBinary = "/usr/bin/nginx"
cfgPath = "/etc/nginx/nginx.conf"
defBinary = "/usr/bin/nginx"
cfgPath = "/etc/nginx/nginx.conf"
luaCfgPath = "/etc/nginx/lua/cfg.json"
)

// NginxExecTester defines the interface to execute
Expand Down
12 changes: 11 additions & 1 deletion rootfs/etc/nginx/lua/lua_ingress.lua
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
local ngx_re_split = require("ngx.re").split
local string_to_bool = require("util").string_to_bool

local certificate_configured_for_current_request =
require("certificate").configured_for_current_request
Expand Down Expand Up @@ -108,7 +109,16 @@ end
-- rewrite gets called in every location context.
-- This is where we do variable assignments to be used in subsequent
-- phases or redirection
function _M.rewrite(location_config)
function _M.rewrite()

local location_config = {
force_ssl_redirect = string_to_bool(ngx.var.force_ssl_redirect),
ssl_redirect = string_to_bool(ngx.var.ssl_redirect),
force_no_ssl_redirect = string_to_bool(ngx.var.force_no_ssl_redirect),
preserve_trailing_slash = string_to_bool(ngx.var.preserve_trailing_slash),
use_port_in_redirects = string_to_bool(ngx.var.use_port_in_redirects),
}

ngx.var.pass_access_scheme = ngx.var.scheme

ngx.var.best_http_host = ngx.var.http_host or ngx.var.host
Expand Down
2 changes: 2 additions & 0 deletions rootfs/etc/nginx/lua/nginx/ngx_conf_balancer.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
local balancer = require("balancer")
balancer.balance()
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
balancer.balance()
balancer.balance()

Copy link
Member

Choose a reason for hiding this comment

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

Can we have the newline at EOF added in snippets?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry I am not following. What would be the goal of adding a new line here?

2 changes: 2 additions & 0 deletions rootfs/etc/nginx/lua/nginx/ngx_conf_balancer_tcp_udp.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
local tcp_udp_balancer = require("tcp_udp_balancer")
tcp_udp_balancer.balance()
2 changes: 2 additions & 0 deletions rootfs/etc/nginx/lua/nginx/ngx_conf_certificate.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
local certificate = require("certificate")
certificate.call()
2 changes: 2 additions & 0 deletions rootfs/etc/nginx/lua/nginx/ngx_conf_configuration.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
local configuration = require("configuration")
configuration.call()
2 changes: 2 additions & 0 deletions rootfs/etc/nginx/lua/nginx/ngx_conf_content_tcp_udp.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
local tcp_udp_configuration = require("tcp_udp_configuration")
tcp_udp_configuration.call()
2 changes: 2 additions & 0 deletions rootfs/etc/nginx/lua/nginx/ngx_conf_init_tcp_udp.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
local tcp_udp_balancer = require("tcp_udp_balancer")
tcp_udp_balancer.init_worker()
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
local configuration = require("configuration")
local backend_data = configuration.get_backends_data()
if not backend_data then
ngx.exit(ngx.HTTP_INTERNAL_SERVER_ERROR)
return
end

ngx.say("OK")
ngx.exit(ngx.HTTP_OK)
2 changes: 2 additions & 0 deletions rootfs/etc/nginx/lua/nginx/ngx_conf_log.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
local monitor = require("monitor")
monitor.call()
11 changes: 11 additions & 0 deletions rootfs/etc/nginx/lua/nginx/ngx_conf_log_block.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
local balancer = require("balancer")
local monitor = require("monitor")

local luaconfig = ngx.shared.luaconfig
local enablemetrics = luaconfig:get("enablemetrics")

balancer.log()

if enablemetrics then
monitor.call()
end
1 change: 1 addition & 0 deletions rootfs/etc/nginx/lua/nginx/ngx_conf_rewrite_auth.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ngx.var.cache_key = ngx.encode_base64(ngx.sha1_bin(ngx.var.tmp_cache_key))
2 changes: 2 additions & 0 deletions rootfs/etc/nginx/lua/nginx/ngx_conf_srv_hdr_filter.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
local lua_ingress = require("lua_ingress")
lua_ingress.header()
5 changes: 5 additions & 0 deletions rootfs/etc/nginx/lua/nginx/ngx_rewrite.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
local lua_ingress = require("lua_ingress")
local balancer = require("balancer")

lua_ingress.rewrite()
balancer.rewrite()
24 changes: 24 additions & 0 deletions rootfs/etc/nginx/lua/nginx/ngx_srv_redirect.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
local request_uri = ngx.var.request_uri
local redirect_to = ngx.arg[1]

local luaconfig = ngx.shared.luaconfig
local use_forwarded_headers = luaconfig:get("use_forwarded_headers")

if string.sub(request_uri, -1) == "/" then
request_uri = string.sub(request_uri, 1, -2)
end

local redirectScheme = ngx.var.scheme
local redirectPort = ngx.var.server_port

if use_forwarded_headers then
if ngx.var.http_x_forwarded_proto then
redirectScheme = ngx.var.http_x_forwarded_proto
end
if ngx.var.http_x_forwarded_port then
redirectPort = ngx.var.http_x_forwarded_port
end
end

return string.format("%s://%s:%s%s", redirectScheme,
redirect_to, redirectPort, request_uri)
53 changes: 53 additions & 0 deletions rootfs/etc/nginx/lua/ngx_conf_init.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
local cjson = require("cjson.safe")

collectgarbage("collect")
local f = io.open("/etc/nginx/lua/cfg.json", "r")
local content = f:read("*a")
f:close()
local configfile = cjson.decode(content)

local luaconfig = ngx.shared.luaconfig
luaconfig:set("enablemetrics", configfile.enable_metrics)
luaconfig:set("use_forwarded_headers", configfile.use_forwarded_headers)
-- init modules
local ok, res
ok, res = pcall(require, "lua_ingress")
if not ok then
error("require failed: " .. tostring(res))
else
lua_ingress = res
lua_ingress.set_config(configfile)
end
ok, res = pcall(require, "configuration")
if not ok then
error("require failed: " .. tostring(res))
else
configuration = res
if not configfile.listen_ports.status_port then
error("required status port not found")
end
configuration.prohibited_localhost_port = configfile.listen_ports.status_port
end
ok, res = pcall(require, "balancer")
if not ok then
error("require failed: " .. tostring(res))
else
balancer = res
end
if configfile.enable_metrics then
ok, res = pcall(require, "monitor")
if not ok then
error("require failed: " .. tostring(res))
else
monitor = res
end
end
ok, res = pcall(require, "certificate")
if not ok then
error("require failed: " .. tostring(res))
else
certificate = res
if configfile.enable_ocsp then
certificate.is_ocsp_stapling_enabled = configfile.enable_ocsp
end
end
Loading