dnsdist: extend lua capabilities by allowing additional HTTP methods & URL path

dch commented 3 years ago

Program: dnsdist
Issue type: Feature request

Short description

I want to extend the API with simple lua functions, including ones that modify the state of dnsdist. This is already possible so long as you don't respect HTTP verbs and query parameters, but it would be nice to do it properly, and to keep custom URLs out of the way of the dnsdist own API.

Usecase

I wanted to add a simple cache flushing API like HTTP DELETE /cache/do.ma.in/ function via lua. As we know from https://github.com/PowerDNS/pdns/issues/10468 this is actually pretty easy (in fact its double-plus-good awesome) but additional verbs, custom paths & pattern matching would make this a significantly more useful and normal API instead of a hideous chimaera.

Description

allow registerWebHandler() to accept custom methods such as DELETE/PUT/POST etc, as these are currently blocked
enable wildcard url path elements (wildcard fields) in registerWebHandler() like /v1/api/stuff/*
ideally, allow URL paths that are not nested under v1/api/... so that they're clearly custom and not part of dnsdist official API

Example

-- your dnsdist.conf
setWebserverConfig({ ...    })
-- listen out for custom URL handlers
function flushCacheHandler(req, resp)
-- .... do awesome stuff
end

-- day 2 of lua but I think we need to pass a table for options instead of 2 parameters now
registerWebHandler({
  -- register a custom path to keep this separate from dnsdist own namespace /api/v1/...
  path='/cache/*', -- note the * which would match a single path element here
  allowedMethods={ "DELETE"; "GET" }, -- defaults to just get
  handler=flushCacheHandler
})
-- woot

rgacogne commented 3 years ago

allow registerWebHandler() to accept custom methods such as DELETE/PUT/POST etc, as these are currently blocked

That makes sense to me, we could totally allow the custom handler to specify the methods that it wants to handle!

enable wildcard url path elements (wildcard fields) in registerWebHandler() like /v1/api/stuff/*

I guess we could do that, perhaps by accepting an optional regex that would match the path?

ideally, allow URL paths that are not nested under v1/api/... so that they're clearly custom and not part of dnsdist official API

I think we already allow that, but perhaps you mean that we should actually enforce that the new endpoints are not under /v1/api?

dch commented 3 years ago

I don't know how the custom handler threading/blocking is configured, but personally I would not use a regex engine (opens denial-of-service risk) but a simple "split on /" thing - you are allowed one and only one per route, so use it wisely...

I believe google's re2 library has less unbounded time risks than the more common pcre library, but for this use case both are heavy imports.

wrt URL paths, I tried to use a shorter path and always get "unauthorized".

rgacogne commented 3 years ago

I don't know how the custom handler threading/blocking is configured, but personally I would not use a regex engine (opens denial-of-service risk) but a simple "split on /" thing - you are allowed one and only one per route, so use it wisely...

We create one new thread to deal with every new connection to the web server. This is not optimal but at least it is simple, and since access to the server requires some authorization (API key or password) we are not too worried about denial of service attacks. But if you think optionally allowing sub-path matching only is good enough, that's even better, of course :)

wrt URL paths, I tried to use a shorter path and always get "unauthorized".

That's weird because our regression tests use /foo: https://github.com/PowerDNS/pdns/blob/master/regression-tests.dnsdist/test_API.py#L667

PowerDNS / pdns