>
> > I also opted to just use the nameserver's address as its name in the
> > resolvers section, as I thought that would have the highest probability
> of
> > avoiding name conflicts with other configured nameservers. Again I'm open
> > to feedback though.
>
> I think you're right at this point. Please just check what happens when the
> same IP address happens multiple times in the same file, as I've seen this
> quite a few times in field. As long as it doesn't prevent the conf from
> starting, it's OK.
>
I made sure that if the same IP occurs multiple times in the file that a
warning
is issued, and the repeated IPs are excluded.
>
> I'm just having a few comments on the patch itself :
>
> > @@ -2288,6 +2288,100 @@ int cfg_parse_resolvers(const char *file, int
> linenum, char **args, int kwm)
> >
> > newnameserver->addr = *sk;
> > }
> > + else if (strcmp(args[0], "parse-resolv-conf") == 0) {
>
> I think you should register a config keyword and parse this in its own
> function if at all possible, but I don't know if resolvers can use
> registered config keywords, so if it's not possible, please ignore this
> comment.
>
The resolvers section isn't registering any config keywords at the moment,
so I'm
going to leave it the way it is to be consistent.
Here you can accidently parse something else. For example if it's written
> "nameservers", the address will be "s". I think you should add a test
> before
> this one :
> if (address == resolv_line + 10)
> continue;
>
Ah, good catch. Fixed.
> > + memset(sk, 0, sizeof(*sk));
> > + sk = str2ip2(address, sk, 1);
> > + if (!sk) {
> > + ha_alert("parsing [/etc/resolv.conf:%d] :
> failed to populate sockaddr_storage for address '%s'\n",
> > + resolv_linenum, address);
> > + err_code |= ERR_ALERT | ERR_FATAL;
> > + goto out;
> > + }
>
> Just out of curiosity, what could cause this error to happen ? I'm asking
> because the alert doesn't make it easy for the end user to figure how to
> fix the problem, thus we should adjust the message in this direction.
>
I adjusted the message to indicate that the address couldn't be recognized.
>
> > + proto = protocol_by_family(sk->ss_family);
> > + if (!proto || !proto->connect) {
> > + ha_alert("parsing [/etc/resolv.conf:%d] :
> '%s' : connect() not supported for this address family.\n",
> > + resolv_linenum, address);
> > + err_code |= ERR_ALERT | ERR_FATAL;
> > + goto out;
> > + }
>
> At this point, I'm realizing that we should probably just ignore such
> entries
> and emit a warning : usually the admin will not have permissions to edit
> the
> resolv.conf file, or sometimes will not want to take risks. And just due to
> an issue in resolv.conf, we could prevent haproxy from starting at all.
>
Done.
>
> > + set_host_port(sk, 53);
> > + newnameserver->addr = *sk;
> > + }
> > +
> > + free(sk);
> > + free(resolv_line);
> > + if (fclose(f) != 0) {
> > + ha_warning("parsing [%s:%d] : failed to close
> handle to /etc/resolv.conf.\n",
> > + file, linenum);
> > + err_code |= ERR_WARN;
> > + }
>
> In practice you don't need to run this check on a read-only file, as it
> cannot fail, and if it really did, the user couldn't do anything about
> it anyway.
>
Great, removed.
I also fixed the memory leaks that you pointed out. (I think) But I did
notice that
valgrind reports that the 'newnameserver' allocation is being leaked
anyway, both
when using parse-resolv-conf as well as the regular nameserver
directive...Let
me know if I should do something about that. To me it seems the resolvers
code
should be freeing that.
Thanks,
Ben
From d859f1afc5082662345bd08265b2ac53396cf062 Mon Sep 17 00:00:00 2001
From: Ben Draut <[email protected]>
Date: Tue, 24 Apr 2018 21:08:37 -0600
Subject: [PATCH] MINOR: config: Implement `parse-resolv-conf` directive
This introduces a new directive for the `resolvers` section:
`parse-resolv-conf`. When present, it will attempt to add any
nameservers in `/etc/resolv.conf` to the list of nameservers
for the current `resolvers` section.
[Mailing list thread][1].
[1]: https://www.mail-archive.com/[email protected]/msg29600.html
---
doc/configuration.txt | 6 +++
src/cfgparse.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 109 insertions(+), 1 deletion(-)
diff --git a/doc/configuration.txt b/doc/configuration.txt
index dcc87fdc..69ea8730 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -12020,6 +12020,11 @@ nameserver <id> <ip>:<port>
<ip> : IP address of the server
<port> : port where the DNS service actually runs
+parse-resolv-conf
+ Adds all nameservers found in /etc/resolv.conf to this resolvers nameservers
+ list. Ordered as if each nameserver in /etc/resolv.conf was individually
+ placed in the resolvers section in place of this directive.
+
hold <status> <period>
Defines <period> during which the last name resolution should be kept based
on last resolution <status>
@@ -12063,6 +12068,7 @@ timeout <event> <time>
resolvers mydns
nameserver dns1 10.0.0.1:53
nameserver dns2 10.0.0.2:53
+ parse-resolv-conf
resolve_retries 3
timeout resolve 1s
timeout retry 1s
diff --git a/src/cfgparse.c b/src/cfgparse.c
index 621af6c8..ac0c399e 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -2239,7 +2239,7 @@ int cfg_parse_resolvers(const char *file, int linenum, char **args, int kwm)
/* Error if two resolvers owns the same name */
if (strcmp(newnameserver->id, args[1]) == 0) {
ha_alert("Parsing [%s:%d]: nameserver '%s' has same name as another nameserver (declared at %s:%d).\n",
- file, linenum, args[1], curr_resolvers->conf.file, curr_resolvers->conf.line);
+ file, linenum, args[1],newnameserver->conf.file, newnameserver->conf.line);
err_code |= ERR_ALERT | ERR_FATAL;
}
}
@@ -2288,6 +2288,108 @@ int cfg_parse_resolvers(const char *file, int linenum, char **args, int kwm)
newnameserver->addr = *sk;
}
+ else if (strcmp(args[0], "parse-resolv-conf") == 0) {
+ const char *whitespace = "\r\n\t ";
+ char *resolv_line = NULL;
+ int resolv_linenum = 0;
+ FILE *f = NULL;
+ char *address = NULL;
+ struct sockaddr_storage *sk = NULL;
+ struct protocol *proto;
+ int duplicate_name = 0;
+
+ if ((resolv_line = malloc(sizeof(*resolv_line) * LINESIZE)) == NULL) {
+ ha_alert("parsing [%s:%d] : out of memory.\n",
+ file, linenum);
+ err_code |= ERR_ALERT | ERR_FATAL;
+ goto resolv_out;
+ }
+
+ if ((f = fopen("/etc/resolv.conf", "r")) == NULL) {
+ ha_alert("parsing [%s:%d] : failed to open /etc/resolv.conf.\n",
+ file, linenum);
+ err_code |= ERR_ALERT | ERR_FATAL;
+ goto resolv_out;
+ }
+
+ sk = calloc(1, sizeof(*sk));
+ if (sk == NULL) {
+ ha_alert("parsing [/etc/resolv.conf:%d] : out of memory.\n",
+ resolv_linenum);
+ err_code |= ERR_ALERT | ERR_FATAL;
+ goto resolv_out;
+ }
+
+ while (fgets(resolv_line, LINESIZE, f) != NULL) {
+ resolv_linenum++;
+ if (strncmp(resolv_line, "nameserver", 10) != 0)
+ continue;
+
+ address = strtok(resolv_line + 10, whitespace);
+ if (address == resolv_line + 10)
+ continue;
+
+ if (address == NULL) {
+ ha_warning("parsing [/etc/resolv.conf:%d] : nameserver line is missing address.\n",
+ resolv_linenum);
+ err_code |= ERR_WARN;
+ continue;
+ }
+
+ duplicate_name = 0;
+ list_for_each_entry(newnameserver, &curr_resolvers->nameservers, list) {
+ if (strcmp(newnameserver->id, address) == 0) {
+ ha_warning("Parsing [/etc/resolv.conf:%d] : generated name for /etc/resolv.conf nameserver '%s' conflicts with another nameserver (declared at %s:%d), it appears to be a duplicate and will be excluded.\n",
+ resolv_linenum, address, newnameserver->conf.file, newnameserver->conf.line);
+ err_code |= ERR_WARN;
+ duplicate_name = 1;
+ }
+ }
+
+ if (duplicate_name)
+ continue;
+
+ memset(sk, 0, sizeof(*sk));
+ sk = str2ip2(address, sk, 1);
+ if (!sk) {
+ ha_warning("parsing [/etc/resolv.conf:%d] : address '%s' could not be recognized, namerserver will be excluded.\n",
+ resolv_linenum, address);
+ err_code |= ERR_WARN;
+ continue;
+ }
+
+ set_host_port(sk, 53);
+
+ proto = protocol_by_family(sk->ss_family);
+ if (!proto || !proto->connect) {
+ ha_warning("parsing [/etc/resolv.conf:%d] : '%s' : connect() not supported for this address family.\n",
+ resolv_linenum, address);
+ err_code |= ERR_WARN;
+ continue;
+ }
+
+ if ((newnameserver = calloc(1, sizeof(*newnameserver))) == NULL) {
+ ha_alert("parsing [/etc/resolv.conf:%d] : out of memory, \n", resolv_linenum);
+ err_code |= ERR_ALERT | ERR_FATAL;
+ goto resolv_out;
+ }
+
+ LIST_ADDQ(&curr_resolvers->nameservers, &newnameserver->list);
+ newnameserver->resolvers = curr_resolvers;
+ newnameserver->conf.file = strdup("/etc/resolv.conf");
+ newnameserver->conf.line = resolv_linenum;
+ newnameserver->id = strdup(address);
+ newnameserver->addr = *sk;
+ }
+
+resolv_out:
+ if (sk != NULL)
+ free(sk);
+ if (resolv_line != NULL)
+ free(resolv_line);
+ if (f != NULL)
+ fclose(f);
+ }
else if (strcmp(args[0], "hold") == 0) { /* hold periods */
const char *res;
unsigned int time;
--
2.14.1
