New Consul storage adapter for lua-resty-auto-ssl

[fititnt]

• Initial commit based on MVP of Consul storage adapter to lua-resty-auto-ssl fititnt/ap-application-load-balancer/issues/25

---

What is Consul?

Consul is a service mesh solution providing a full featured control plane with service discovery, configuration, and segmentation functionality. (...) KV Store: Applications can make use of Consul's hierarchical key/value store for any number of purposes, including dynamic configuration, feature flagging, coordination, leader election, and more. The simple HTTP API makes it easy to use. (...) Multi Datacenter: Consul supports multiple datacenters out of the box. This means users of Consul do not have to worry about building additional layers of abstraction to grow to multiple regions. From https://www.consul.io/intro/index.html

---

The initial MVP is based on the on reverse engineering of existing adapters, resty/auto-ssl/storage_adapters/redis.lua and resty/auto-ssl/storage_adapters/file.lua. The end result of this PR is likely to:

1. Implement at least the same functionalities from Redis that Consul support
2. Improve the TODO at the readme marked as "Document and formalize the API for other storage adapters."
   1. Even if, be able to merge, just means documenting pretty well the new storage_adapters/consul.lua
   2. And make the API just a bit more flexible (both Redis and File use : in some parts, but for Consul this would be /)
3. Document the additional dependency if used (opm get hamishforbes/lua-resty-consul) and maybe other pertinent issues

I think I will try to keep this Pull Request simple and I'm specially open to discussion on how to implement the interfaces, but I will try not touch other core files for now.

---

Some information about me:

Actually, I literally started to code in Lua 2 days ago. (started with Lean lua in an Hour https://www.youtube.com/watch?v=S4eNl1rA1Ns, but already had interest on this language before, just does not had a reason to use in production). And oh, god, this language is beautiful and very powerful (at least compared with the time debugging and implementing things direct with NGinx/Apache/At application level.)

This maybe explain why I would avoid touch other files and to have some reference, is implementing some documenting patterns (one that seems to be more used is ldoc, https://stevedonovan.github.io/ldoc/manual/doc.md.html). I'm also even looking for VSCode extensions that could allow same level of IDE I do have with other languages.

I have strong proficiency with NGinx/Docker and on last months with Ansible automation. So it means that, even if not for this Pull Request, I later could try to already change one or more Dockerfiles to be able to do the full testing with Consul. And also that if have some interest in discuss things that could be drafted to formalize other storage adapters we could do.

Ah, and other pertinent aspect, I'm specially interested in clustering, so maybe even some Ansible demo with Redis and/or Consul is not even far of what I would do anyway.

[fititnt]

I'm back. I will take another round today to improve this PR today.

The warnings from the CI I'm likely to solve today, but before the merge I remove the dump functions.

Some things I will ask later, but already looking upfront:

I'm already looking the file redis_spec.lua (https://github.com/GUI/lua-resty-auto-ssl/blob/master/spec/redis_spec.lua). I guess I could also do a draft of something like this, but at this exact moment did not know how to use the Lua part of these tests.

All Dockerfile's also do install redis or redis-server without download newer versions, so I guess for Consul the tests (if have option) should not require complex or updated versions. Alpine have this https://pkgs.alpinelinux.org/package/edge/testing/x86/consul, so Centos and Ubuntu I could spin a full VM.

At this moment I not sure if, at least for Consul (and maybe other drivers) the maintainers would prefer install the full tools on all actual distributions or create one or two Dockerfiles for each new driver.

[fititnt]

Fedback of the actual state of this Pull Request

• Maybe not because of this plugin, but because of my infrastructure (I'm initially testing behind HAProxy, and with IPv6 is not fine tunned) that needs to be fixed:
  • the "the new certificate is saved, cached, and returned to the client (without dropping the original request)" does not work smootly on first request. I in investigate further. The connection is dropped, sometimes need press F5 (but on Chrome, it does automatically)
  • I'm having some warnings related to IPv6
• Still need some code clean up (can do in an hour if need when we're ok to consider merge)

All this issues will be solved.

About the need or not to integrate full rests with each new adapter

With this PR, we will have Consul adapter. In theory this would means add tests to

• Dockerfile-test
• Dockerfile-test-alpine
• Dockerfile-test-lua51
• Dockerfile-test-openresty1.13
• Dockerfile-test-ubuntu

These tests in fact, use 3 linux distributions

• alpine
• centos
• bionic

Another point of discussion we're should have, not because of me, but to incentive other drivers, is the bare minimum ideal tests addition to have an PR Accepted.

Lets give an example

Rocha is adding Consul. Etcd (https://etcd.io/) is also pretty popular (to a point even me will submit a new PR eventually, or documenting so much that someone else will do it.

@arturmkrtchyan here https://github.com/GUI/lua-resty-auto-ssl/issues/187 even was thinking about an S3 storage.

So, How do we scale testing for new drivers?

About documenting the Storage Driver API

I think I will submit a different pull request only to draft example.lua storage, and try to make it very friendly for someone new draft a new driver. I will remove some of my comments from the consul.lua

[GUI]

@fititnt: Sorry for the long delay and lack of response! I'll try to review this soon, but many thanks for your contributions!

[fititnt]

@GUI actually no problem. The only very, very important thing that we should do before you release a version is, at least, do this:

• Delete the local function dumpvar(data)
• Delete dump calls, like this dump({'started', os.date("!%Y-%m-%dT%TZ")})
• Delete some ngx.log that are not called just on errors, like this one ngx.log(ngx.ERR, "\n\n\n\n\n\n\n\n\n\n---")

I just already doesn't' deleted these lines to help you or who review this PR to test or if I add new unitary tests with Consul.

About how to scale integrate automated tests

Like I said before, If you want discuss this before merge (even if you want to implement without asking help) you can do it.

But if want merge more faster, we can do this without adding the tests.

[maxramqvist]

Hi,

We are using this code in production and it works great - except for renewal of expired (or to be expired) certificates.

The renewal doesn't work with this code as the function _M.keys_with_suffix isn't complete. This code works though:

function _M.keys_with_suffix(self, suffix)
  local connection, connection_err = self:get_connection()
  if connection_err then
    ngx.log(ngx.EMERG, '_M.keys_with_suffix: ', connection_err)
    return false, connection_err
  end

  local result = {}
  local res, err = connection:list_keys(self.options["prefix"])
  if res and self.options["prefix"] then

    local keys = {}
    if res.status == 200 then
      keys = res.body
    end

    local unprefixed_keys = {}
    -- First character past the prefix and a colon
    local offset = string.len(self.options["prefix"]) + 2

    for _, key in ipairs(keys) do

      local unprefixed = string.sub(key, offset)

      table.insert(unprefixed_keys, unprefixed)

    end
    result = unprefixed_keys

  end

  return result, err
end

@fititnt if you could update your PR?

[fititnt]

@maxramqvist I also had problem with renewal of expired (or to be expired) certificates! And this type of thing is hard to test (can take like months until come the issue!).

On next days I will merge your suggestion here. And is good that at least someone else is testing this.

[maxramqvist]

@fititnt Realized I made a mistake in the code for the keys_with_suffix function. It didn't actually return only the keys with the suffix. Here is the proper code:

function _M.keys_with_suffix(self, suffix)
  local connection, connection_err = self:get_connection()
  if connection_err then
    ngx.log(ngx.EMERG, '_M.keys_with_suffix: ', connection_err)
    return false, connection_err
  end

  local result = {}

  -- Get all keys under prefix/*
  local res, err = connection:list_keys(self.options["prefix"])
  if res and self.options["prefix"] then

    local keys = {}
    if res.status == 200 then
      keys = res.body
    end

    -- Match keys with requested suffix.
    local keys_with_suffix = {}
    for _, key in ipairs(keys) do

      local match = string.match(key, '.*' ..suffix .. '$')
      if match == nil then
        goto continue
      end

      table.insert(keys_with_suffix, key)

      ::continue::
    end


    local unprefixed_keys = {}
    -- First character past the prefix and a colon
    local offset = string.len(self.options["prefix"]) + 2
    
    -- Remove prefix from key so that only the actual key remains.
    for _, key in ipairs(keys_with_suffix) do
      local unprefixed = string.sub(key, offset)
      table.insert(unprefixed_keys, unprefixed)
    end
    result = keys_with_suffix

  end

  return result, err
end