New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dnsdist: Allow editing the ACL via the API #4658

Merged
merged 1 commit into from Nov 18, 2016

Conversation

Projects
None yet
3 participants
@rgacogne
Member

rgacogne commented Nov 3, 2016

Short description

  • Add /api/v1/servers/localhost/config/acl to be able to retrieve and update the current ACL.
  • Add setAPIWritable(bool, [dir]) to allow modifications from the API, and to specify an optional directory where updated configuration files are written to on such update. The directory should be included in the configuration with includeDirectory() to be of any use.

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled and tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added regression tests
@@ -1224,6 +1224,7 @@ Here are all functions:
* quit or ^D: exit the console
* `webserver(address:port, password [, apiKey [, customHeaders ]])`: launch a webserver with stats on that address with that password
* `includeDirectory(dir)`: all files ending in `.conf` in the directory `dir` are loaded into the configuration
* `setAPIWritable(bool, [dir])`: allow modifications via the API. If `dir` iset set, it must be a valid directory where the configuration files will be written by the API. Otherwise the modifications done via the API will not be written to the configuration and will not persist after a reload

This comment has been minimized.

@zeha

zeha Nov 9, 2016

Collaborator

typo: iset set

@@ -321,6 +321,7 @@ const std::vector<ConsoleKeyword> g_consoleKeywords{
{ "QTypeRule", true, "qtype", "matches queries with the specified qtype" },
{ "RCodeRule", true, "rcode", "matches responses with the specified rcode" },
{ "setACL", true, "{netmask, netmask}", "replace the ACL set with these netmasks. Use `setACL({})` to reset the list, meaning no one can use us" },
{ "setAPIWritable", true, "bool, dir", "allow modifications via the API. if `dir`is set, it must be a valid directory where the configuration files will be written by the API" },

This comment has been minimized.

@zeha

zeha Nov 9, 2016

Collaborator

missed space before is set

g_apiConfigDirectory = *apiConfigDir;
}
else {
errlog("The API configuration directory cannot be empty!");

This comment has been minimized.

@zeha

zeha Nov 9, 2016

Collaborator

this has confusion potential: "is this about the string itself or the directory?"

}
if (resp.status == 200) {
vinfolog("Updating the ACL via the API");

This comment has been minimized.

@zeha

zeha Nov 9, 2016

Collaborator

maybe also log actual changes made? (for "audit")

@rgacogne rgacogne force-pushed the rgacogne:dnsdist-set-acl branch from dc73108 to aefbb0c Nov 14, 2016

@rgacogne

This comment has been minimized.

Member

rgacogne commented Nov 14, 2016

Updated to fix the issues reported by @zeha (thanks!).

@wojas

This comment has been minimized.

Member

wojas commented Nov 18, 2016

Can the config setting name be renamed from 'acl' to 'allow-from' to be consistent with the recursor and auth server REST API?

@rgacogne rgacogne force-pushed the rgacogne:dnsdist-set-acl branch from aefbb0c to 56d68fa Nov 18, 2016

@rgacogne rgacogne merged commit 7448bae into PowerDNS:master Nov 18, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@rgacogne rgacogne deleted the rgacogne:dnsdist-set-acl branch Nov 18, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment