From 3bec99ecfce78a535b17d1cd87c5cae105328190 Mon Sep 17 00:00:00 2001 From: Ricardo Katz Date: Wed, 21 Aug 2024 10:54:29 -0300 Subject: [PATCH] Remove 3rd party lua plugin support (#11821) --- .../nginx-configuration/configmap.md | 5 -- internal/ingress/controller/config/config.go | 5 -- .../ingress/controller/template/configmap.go | 6 -- rootfs/etc/nginx/lua/plugins.lua | 61 ------------------- rootfs/etc/nginx/lua/plugins/README.md | 36 ----------- .../nginx/lua/plugins/hello_world/main.lua | 13 ---- .../plugins/hello_world/test/main_test.lua | 24 -------- rootfs/etc/nginx/lua/test/plugins_test.lua | 23 ------- rootfs/etc/nginx/template/nginx.tmpl | 19 ------ test/data/cleanConf.expected.conf | 4 -- test/data/cleanConf.src.conf | 7 +-- test/e2e/settings/plugins.go | 55 ----------------- test/test-lua.sh | 2 +- 13 files changed, 2 insertions(+), 258 deletions(-) delete mode 100644 rootfs/etc/nginx/lua/plugins.lua delete mode 100644 rootfs/etc/nginx/lua/plugins/README.md delete mode 100644 rootfs/etc/nginx/lua/plugins/hello_world/main.lua delete mode 100644 rootfs/etc/nginx/lua/plugins/hello_world/test/main_test.lua delete mode 100644 rootfs/etc/nginx/lua/test/plugins_test.lua delete mode 100644 test/e2e/settings/plugins.go diff --git a/docs/user-guide/nginx-configuration/configmap.md b/docs/user-guide/nginx-configuration/configmap.md index 0b8e03af1..51e4edfa3 100644 --- a/docs/user-guide/nginx-configuration/configmap.md +++ b/docs/user-guide/nginx-configuration/configmap.md @@ -82,7 +82,6 @@ The following table shows a configuration option's name, type, and the default v | [server-name-hash-bucket-size](#server-name-hash-bucket-size) | int | `` | | [proxy-headers-hash-max-size](#proxy-headers-hash-max-size) | int | 512 | | | [proxy-headers-hash-bucket-size](#proxy-headers-hash-bucket-size) | int | 64 | | -| [plugins](#plugins) | []string | | | | [reuse-port](#reuse-port) | bool | "true" | | | [server-tokens](#server-tokens) | bool | "false" | | | [ssl-ciphers](#ssl-ciphers) | string | "ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384" | | @@ -612,10 +611,6 @@ _References:_ - [https://nginx.org/en/docs/hash.html](https://nginx.org/en/docs/hash.html) - [https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_headers_hash_bucket_size](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_headers_hash_bucket_size) -## plugins - -Activates plugins installed in `/etc/nginx/lua/plugins`. Refer to [ingress-nginx plugins README](https://github.com/kubernetes/ingress-nginx/blob/main/rootfs/etc/nginx/lua/plugins/README.md) for more information on how to write and install a plugin. - ## server-tokens Send NGINX Server header in responses and display NGINX version in error pages. _**default:**_ is disabled diff --git a/internal/ingress/controller/config/config.go b/internal/ingress/controller/config/config.go index 5e0ca2296..337cb9e86 100644 --- a/internal/ingress/controller/config/config.go +++ b/internal/ingress/controller/config/config.go @@ -318,11 +318,6 @@ type Configuration struct { NginxStatusIpv4Whitelist []string `json:"nginx-status-ipv4-whitelist,omitempty"` NginxStatusIpv6Whitelist []string `json:"nginx-status-ipv6-whitelist,omitempty"` - // Plugins configures plugins to use placed in the directory /etc/nginx/lua/plugins. - // Every plugin has to have main.lua in the root. Every plugin has to bundle all of its dependencies. - // The execution order follows the definition. - Plugins []string `json:"plugins,omitempty"` - // If UseProxyProtocol is enabled ProxyRealIPCIDR defines the default the IP/network address // of your external load balancer ProxyRealIPCIDR []string `json:"proxy-real-ip-cidr,omitempty"` diff --git a/internal/ingress/controller/template/configmap.go b/internal/ingress/controller/template/configmap.go index 1a7f15f1c..2f7b0a09c 100644 --- a/internal/ingress/controller/template/configmap.go +++ b/internal/ingress/controller/template/configmap.go @@ -67,7 +67,6 @@ const ( globalAuthCacheDuration = "global-auth-cache-duration" globalAuthAlwaysSetCookie = "global-auth-always-set-cookie" luaSharedDictsKey = "lua-shared-dicts" - plugins = "plugins" debugConnections = "debug-connections" workerSerialReloads = "enable-serial-reloads" ) @@ -416,11 +415,6 @@ func ReadConfig(src map[string]string) config.Configuration { delete(conf, workerSerialReloads) } - if val, ok := conf[plugins]; ok { - to.Plugins = splitAndTrimSpace(val, ",") - delete(conf, plugins) - } - if val, ok := conf[debugConnections]; ok { delete(conf, debugConnections) for _, i := range splitAndTrimSpace(val, ",") { diff --git a/rootfs/etc/nginx/lua/plugins.lua b/rootfs/etc/nginx/lua/plugins.lua deleted file mode 100644 index 55e208a32..000000000 --- a/rootfs/etc/nginx/lua/plugins.lua +++ /dev/null @@ -1,61 +0,0 @@ -local require = require -local ngx = ngx -local ipairs = ipairs -local string_format = string.format -local ngx_log = ngx.log -local INFO = ngx.INFO -local ERR = ngx.ERR -local pcall = pcall - -local _M = {} -local MAX_NUMBER_OF_PLUGINS = 20 -local plugins = {} - -local function load_plugin(name) - local path = string_format("plugins.%s.main", name) - - local ok, plugin = pcall(require, path) - if not ok then - ngx_log(ERR, string_format("error loading plugin \"%s\": %s", path, plugin)) - return - end - local index = #plugins - if (plugin.name == nil or plugin.name == '') then - plugin.name = name - end - plugins[index + 1] = plugin -end - -function _M.init(names) - local count = 0 - for _, name in ipairs(names) do - if count >= MAX_NUMBER_OF_PLUGINS then - ngx_log(ERR, "the total number of plugins exceed the maximum number: ", MAX_NUMBER_OF_PLUGINS) - break - end - load_plugin(name) - count = count + 1 -- ignore loading failure, just count the total - end -end - -function _M.run() - local phase = ngx.get_phase() - - for _, plugin in ipairs(plugins) do - if plugin[phase] then - ngx_log(INFO, string_format("running plugin \"%s\" in phase \"%s\"", plugin.name, phase)) - - -- TODO: consider sandboxing this, should we? - -- probably yes, at least prohibit plugin from accessing env vars etc - -- but since the plugins are going to be installed by ingress-nginx - -- operator they can be assumed to be safe also - local ok, err = pcall(plugin[phase]) - if not ok then - ngx_log(ERR, string_format("error while running plugin \"%s\" in phase \"%s\": %s", - plugin.name, phase, err)) - end - end - end -end - -return _M diff --git a/rootfs/etc/nginx/lua/plugins/README.md b/rootfs/etc/nginx/lua/plugins/README.md deleted file mode 100644 index 64f4912f0..000000000 --- a/rootfs/etc/nginx/lua/plugins/README.md +++ /dev/null @@ -1,36 +0,0 @@ -# Custom Lua plugins - -ingress-nginx uses [https://github.com/openresty/lua-nginx-module](https://github.com/openresty/lua-nginx-module) to run custom Lua code -within Nginx workers. It is recommended to familiarize yourself with that ecosystem before deploying your custom Lua based ingress-nginx plugin. - -### Writing a plugin - -Every ingress-nginx Lua plugin is expected to have `main.lua` file and all of its dependencies. -`main.lua` is the entry point of the plugin. The plugin manager uses convention over configuration -strategy and automatically runs functions defined in `main.lua` in the corresponding Nginx phase based on their name. - -Nginx has different [request processing phases](https://nginx.org/en/docs/dev/development_guide.html#http_phases). -By defining functions with the following names, you can run your custom Lua code in the corresponding Nginx phase: - - - `init_worker`: useful for initializing some data per Nginx worker process - - `rewrite`: useful for modifying request, changing headers, redirection, dropping request, doing authentication etc - - `header_filter`: this is called when backend response header is received, it is useful for modifying response headers - - `body_filter`: this is called when response body is received, it is useful for logging response body - - `log`: this is called when request processing is completed and a response is delivered to the client - -Check this [`hello_world`](https://github.com/kubernetes/ingress-nginx/tree/main/rootfs/etc/nginx/lua/plugins/hello_world) plugin as a simple example or refer to [OpenID Connect integration](https://github.com/ElvinEfendi/ingress-nginx-openidc/tree/master/rootfs/etc/nginx/lua/plugins/openidc) for more advanced usage. - -Do not forget to write tests for your plugin. - -### Installing a plugin - -There are two options: - - - mount your plugin into `/etc/nginx/lua/plugins/` in the ingress-nginx pod - - build your own ingress-nginx image like it is done in the [example](https://github.com/ElvinEfendi/ingress-nginx-openidc/tree/master/rootfs/etc/nginx/lua/plugins/openidc) and install your plugin during image build - -Mounting is the quickest option. - -### Enabling plugins - -Once your plugin is ready you need to use [`plugins` configuration setting](https://kubernetes.github.io/ingress-nginx/user-guide/nginx-configuration/configmap/#plugins) to activate it. Let's say you want to activate `hello_world` and `open_idc` plugins, then you set `plugins` setting to `"hello_world, open_idc"`. _Note_ that the plugins will be executed in the given order. diff --git a/rootfs/etc/nginx/lua/plugins/hello_world/main.lua b/rootfs/etc/nginx/lua/plugins/hello_world/main.lua deleted file mode 100644 index 03316c3ee..000000000 --- a/rootfs/etc/nginx/lua/plugins/hello_world/main.lua +++ /dev/null @@ -1,13 +0,0 @@ -local ngx = ngx - -local _M = {} - -function _M.rewrite() - local ua = ngx.var.http_user_agent - - if ua == "hello" then - ngx.req.set_header("x-hello-world", "1") - end -end - -return _M diff --git a/rootfs/etc/nginx/lua/plugins/hello_world/test/main_test.lua b/rootfs/etc/nginx/lua/plugins/hello_world/test/main_test.lua deleted file mode 100644 index 5eda52259..000000000 --- a/rootfs/etc/nginx/lua/plugins/hello_world/test/main_test.lua +++ /dev/null @@ -1,24 +0,0 @@ - -local main = require("plugins.hello_world.main") - --- The unit tests are run within a timer phase in a headless Nginx process. --- Since `set_header` and `ngx.var.http_` API are disabled in this phase we have to stub it --- to avoid `API disabled in the current context` error. - -describe("main", function() - describe("rewrite", function() - it("sets x-hello-world header to 1 when user agent is hello", function() - ngx.var = { http_user_agent = "hello" } - stub(ngx.req, "set_header") - main.rewrite() - assert.stub(ngx.req.set_header).was_called_with("x-hello-world", "1") - end) - - it("does not set x-hello-world header to 1 when user agent is not hello", function() - ngx.var = { http_user_agent = "not-hello" } - stub(ngx.req, "set_header") - main.rewrite() - assert.stub(ngx.req.set_header).was_not_called_with("x-hello-world", "1") - end) - end) -end) diff --git a/rootfs/etc/nginx/lua/test/plugins_test.lua b/rootfs/etc/nginx/lua/test/plugins_test.lua deleted file mode 100644 index d7f789d0f..000000000 --- a/rootfs/etc/nginx/lua/test/plugins_test.lua +++ /dev/null @@ -1,23 +0,0 @@ -describe("plugins", function() - describe("#run", function() - it("runs the plugins in the given order", function() - ngx.get_phase = function() return "rewrite" end - local plugins = require("plugins") - local called_plugins = {} - local plugins_to_mock = {"plugins.pluginfirst.main", "plugins.pluginsecond.main", "plugins.pluginthird.main"} - for i=1, 3, 1 - do - package.loaded[plugins_to_mock[i]] = { - rewrite = function() - called_plugins[#called_plugins + 1] = plugins_to_mock[i] - end - } - end - assert.has_no.errors(function() - plugins.init({"pluginfirst", "pluginsecond", "pluginthird"}) - end) - assert.has_no.errors(plugins.run) - assert.are.same(plugins_to_mock, called_plugins) - end) - end) -end) \ No newline at end of file diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index 93a04e3e6..1c630bb4d 100644 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -113,15 +113,6 @@ http { certificate = res certificate.is_ocsp_stapling_enabled = {{ $cfg.EnableOCSP }} end - - ok, res = pcall(require, "plugins") - if not ok then - error("require failed: " .. tostring(res)) - else - plugins = res - end - -- load all plugins that'll be used here - plugins.init({ {{ range $idx, $plugin := $cfg.Plugins }}{{ if $idx }},{{ end }}{{ $plugin | quote }}{{ end }} }) } init_worker_by_lua_block { @@ -130,8 +121,6 @@ http { {{ if $all.EnableMetrics }} monitor.init_worker({{ $all.MonitorMaxBatchSize }}) {{ end }} - - plugins.run() } {{/* Enable the real_ip module only if we use either X-Forwarded headers or Proxy Protocol. */}} @@ -1265,7 +1254,6 @@ stream { rewrite_by_lua_block { lua_ingress.rewrite({{ locationConfigForLua $location $all }}) balancer.rewrite() - plugins.run() } # be careful with `access_by_lua_block` and `satisfy any` directives as satisfy any @@ -1276,11 +1264,6 @@ stream { header_filter_by_lua_block { lua_ingress.header() - plugins.run() - } - - body_filter_by_lua_block { - plugins.run() } log_by_lua_block { @@ -1288,8 +1271,6 @@ stream { {{ if $all.EnableMetrics }} monitor.call() {{ end }} - - plugins.run() } {{ if not $location.Logs.Access }} diff --git a/test/data/cleanConf.expected.conf b/test/data/cleanConf.expected.conf index 7c4a16824..9c0513b37 100644 --- a/test/data/cleanConf.expected.conf +++ b/test/data/cleanConf.expected.conf @@ -67,8 +67,6 @@ http { balancer.init_worker() monitor.init_worker(10000) - - plugins.run() } map $request_uri $loggable { @@ -120,7 +118,6 @@ http { use_port_in_redirects = false, }) balancer.rewrite() - plugins.run() } # be careful with `access_by_lua_block` and `satisfy any` directives as satisfy any @@ -130,7 +127,6 @@ http { header_filter_by_lua_block { lua_ingress.header() - plugins.run() } } diff --git a/test/data/cleanConf.src.conf b/test/data/cleanConf.src.conf index 89954cf0d..6da578106 100644 --- a/test/data/cleanConf.src.conf +++ b/test/data/cleanConf.src.conf @@ -86,11 +86,8 @@ lua_shared_dict ocsp_response_cache 5M; init_worker_by_lua_block { lua_ingress.init_worker() balancer.init_worker() - - monitor.init_worker(10000) - - plugins.run() + monitor.init_worker(10000) } @@ -164,7 +161,6 @@ lua_shared_dict ocsp_response_cache 5M; use_port_in_redirects = false, }) balancer.rewrite() - plugins.run() } # be careful with `access_by_lua_block` and `satisfy any` directives as satisfy any @@ -174,7 +170,6 @@ lua_shared_dict ocsp_response_cache 5M; header_filter_by_lua_block { lua_ingress.header() - plugins.run() } diff --git a/test/e2e/settings/plugins.go b/test/e2e/settings/plugins.go deleted file mode 100644 index 659acd42c..000000000 --- a/test/e2e/settings/plugins.go +++ /dev/null @@ -1,55 +0,0 @@ -/* -Copyright 2020 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package settings - -import ( - "fmt" - "net/http" - "strings" - - "github.com/onsi/ginkgo/v2" - "k8s.io/ingress-nginx/test/e2e/framework" -) - -var _ = framework.IngressNginxDescribe("plugins", func() { - f := framework.NewDefaultFramework("plugins") - - ginkgo.BeforeEach(func() { - f.NewEchoDeployment() - }) - - ginkgo.It("should exist a x-hello-world header", func() { - f.UpdateNginxConfigMapData("plugins", "hello_world, invalid") - - host := "example.com" - f.EnsureIngress(framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, nil)) - - f.WaitForNginxConfiguration( - func(server string) bool { - return strings.Contains(server, fmt.Sprintf("server_name %v", host)) && - strings.Contains(server, `plugins.init({ "hello_world","invalid" })`) - }) - - f.HTTPTestClient(). - GET("/"). - WithHeader("Host", host). - WithHeader("User-Agent", "hello"). - Expect(). - Status(http.StatusOK). - Body().Contains("x-hello-world=1") - }) -}) diff --git a/test/test-lua.sh b/test/test-lua.sh index fc60023f8..1aff5f30c 100755 --- a/test/test-lua.sh +++ b/test/test-lua.sh @@ -41,7 +41,7 @@ SHDICT_ARGS=( ) if [ $# -eq 0 ]; then - resty "${SHDICT_ARGS[@]}" ./rootfs/etc/nginx/lua/test/ ./rootfs/etc/nginx/lua/plugins/**/test ${BUSTED_ARGS} + resty "${SHDICT_ARGS[@]}" ./rootfs/etc/nginx/lua/test/ ${BUSTED_ARGS} else resty "${SHDICT_ARGS[@]}" $@ ${BUSTED_ARGS} fi