From b66f9e329db3b0e69e732be94faca7cb9225a9e9 Mon Sep 17 00:00:00 2001 From: Elvin Efendi Date: Wed, 26 Jun 2019 18:40:55 -0400 Subject: [PATCH 1/2] override least recently used entries when certificate_data dictionary is full --- rootfs/etc/nginx/lua/configuration.lua | 13 +++++-------- .../etc/nginx/lua/test/configuration_test.lua | 19 ++++++++++++------- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/rootfs/etc/nginx/lua/configuration.lua b/rootfs/etc/nginx/lua/configuration.lua index 10f86eddc..3d9e1bdcf 100644 --- a/rootfs/etc/nginx/lua/configuration.lua +++ b/rootfs/etc/nginx/lua/configuration.lua @@ -59,18 +59,15 @@ local function handle_servers() local err_buf = {} for _, server in ipairs(servers) do if server.hostname and server.sslCert.pemCertKey then - local success - success, err = certificate_data:safe_set(server.hostname, server.sslCert.pemCertKey) + local success, err, forcible = certificate_data:set(server.hostname, server.sslCert.pemCertKey) if not success then - if err == "no memory" then - ngx.status = ngx.HTTP_INTERNAL_SERVER_ERROR - ngx.log(ngx.ERR, "no memory in certificate_data dictionary") - return - end - local err_msg = string.format("error setting certificate for %s: %s\n", server.hostname, tostring(err)) table.insert(err_buf, err_msg) end + if forcible then + local msg = string.format("certificate_data dictionary is full, LRU entry has been removed to store %s", server.hostname) + ngx.log(ngx.WARN, msg) + end else ngx.log(ngx.WARN, "hostname or pemCertKey are not present") end diff --git a/rootfs/etc/nginx/lua/test/configuration_test.lua b/rootfs/etc/nginx/lua/test/configuration_test.lua index 64c851b16..d5db0844a 100644 --- a/rootfs/etc/nginx/lua/test/configuration_test.lua +++ b/rootfs/etc/nginx/lua/test/configuration_test.lua @@ -208,7 +208,7 @@ describe("Configuration", function() it("should log an err and set status to Internal Server Error when a certificate cannot be set", function() ngx.var.request_method = "POST" - ngx.shared.certificate_data.safe_set = function(self, data) return false, "error" end + ngx.shared.certificate_data.set = function(self, data) return false, "error", nil end local mock_servers = cjson.encode({ { hostname = "hostname", @@ -232,9 +232,14 @@ describe("Configuration", function() assert.same(ngx.status, ngx.HTTP_INTERNAL_SERVER_ERROR) end) - it("should log an err, set status to Internal Server Error, and short circuit when shared dictionary is full", function() + it("logs a warning when entry is forcibly stored", function() + local stored_entries = {} + ngx.var.request_method = "POST" - ngx.shared.certificate_data.safe_set = function(self, data) return false, "no memory" end + ngx.shared.certificate_data.set = function(self, key, value) + stored_entries[key] = value + return true, nil, true + end local mock_servers = cjson.encode({ { hostname = "hostname", @@ -252,11 +257,11 @@ describe("Configuration", function() ngx.req.get_body_data = function() return mock_servers end local s1 = spy.on(ngx, "log") - local s2 = spy.on(ngx.shared.certificate_data, "safe_set") assert.has_no.errors(configuration.handle_servers) - assert.spy(s1).was_called_with(ngx.ERR, "no memory in certificate_data dictionary") - assert.spy(s2).was_not_called_with("hostname2", "pemCertKey2") - assert.same(ngx.status, ngx.HTTP_INTERNAL_SERVER_ERROR) + assert.spy(s1).was_called_with(ngx.WARN, "certificate_data dictionary is full, LRU entry has been removed to store hostname") + assert.equal("pemCertKey", stored_entries["hostname"]) + assert.equal("pemCertKey2", stored_entries["hostname2"]) + assert.same(ngx.HTTP_CREATED, ngx.status) end) end) end) From cd25a0c17a3f5a017f04582d7a9af8ce17c8a304 Mon Sep 17 00:00:00 2001 From: Elvin Efendi Date: Mon, 1 Jul 2019 10:24:09 -0400 Subject: [PATCH 2/2] adjust docs --- docs/user-guide/cli-arguments.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user-guide/cli-arguments.md b/docs/user-guide/cli-arguments.md index b8581a276..df08cfd29 100644 --- a/docs/user-guide/cli-arguments.md +++ b/docs/user-guide/cli-arguments.md @@ -15,7 +15,7 @@ They are set in the container spec of the `nginx-ingress-controller` Deployment | `--default-ssl-certificate string` | Secret containing a SSL certificate to be used by the default HTTPS server (catch-all). Takes the form "namespace/name". | | `--disable-catch-all` | Disable support for catch-all Ingresses. | | `--election-id string` | Election id to use for Ingress status updates. (default "ingress-controller-leader") | -| `--enable-dynamic-certificates` | Dynamically serves certificates instead of reloading NGINX when certificates are created, updated, or deleted. Currently does not support OCSP stapling, so --enable-ssl-chain-completion must be turned off (default behaviour). Assuming the certificate is generated with a 2048 bit RSA key/cert pair, this feature can store roughly 5000 certificates. (enabled by default) | +| `--enable-dynamic-certificates` | Dynamically serves certificates instead of reloading NGINX when certificates are created, updated, or deleted. Currently does not support OCSP stapling, so --enable-ssl-chain-completion must be turned off (default behaviour). Assuming the certificate is generated with a 2048 bit RSA key/cert pair, this feature can store roughly 5000 certificates. Once the backing Lua shared dictionary `certificate_data` is full, the least recently used certificate will be removed to store new ones. (enabled by default) | | `--enable-ssl-chain-completion` | Autocomplete SSL certificate chains with missing intermediate CA certificates. A valid certificate chain is required to enable OCSP stapling. Certificates uploaded to Kubernetes must have the "Authority Information Access" X.509 v3 extension for this to succeed. (default true) | | `--enable-ssl-passthrough` | Enable SSL Passthrough. | | `--health-check-path string` | URL path of the health check endpoint. Configured inside the NGINX status server. All requests received on the port defined by the healthz-port parameter are forwarded internally to this path. (default "/healthz") |