Merge pull request #4232 from ElvinEfendi/fix-dynamic-cert-bug

override least recently used entries when certificate_data dict is full
This commit is contained in:
Kubernetes Prow Robot 2019-07-01 08:03:22 -07:00 committed by GitHub
commit c01effb076
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 18 additions and 16 deletions

View file

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

View file

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

View file

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