Re: [PATCH][MINOR]: config: Warn if resolvers section has no namerservers configured

[Willy Tarreau]

Hi Ben,

On Fri, Apr 13, 2018 at 03:51:17PM -0600, Ben Draut wrote:
> This implements a simple warning for 'resolvers' sections that have no
> nameservers.

Thank you, now merged. However :

> (Also trimmed lines with trailing whitespace in this file.)

Please don't do this, it needlessly inflates the patch, complicates
the review process and possibly makes backports more painful. While
it can sometimes be fine to fix these where you are editing, it's not
much welcome in other places, especially mixed with a feature. As a
rule of thumb, if a patch contains some hunks irrelevant to the patch's
initial purpose, these changes should be dropped.

Thanks,
Willy

[PATCH][MINOR]: config: Warn if resolvers section has no namerservers configured

[Ben Draut]

This implements a simple warning for 'resolvers' sections that have no
nameservers.

Previously discussed here:
https://www.mail-archive.com/haproxy@formilux.org/msg29600.html

Thanks,

Ben
From fc6a36dabec89eef0eba13146cecbf157f0675b9 Mon Sep 17 00:00:00 2001
From: Ben Draut 
Date: Fri, 13 Apr 2018 15:43:04 -0600
Subject: [PATCH] MINOR: config: Warn if resolvers has no nameservers

Today, a `resolvers` section may be configured without any `nameserver`
directives, which is useless. This implements a warning when such
sections are detected.

[List thread][1].

(Also trimmed lines with trailing whitespace in this file.)

[1]: https://www.mail-archive.com/haproxy@formilux.org/msg29600.html
---
 src/cfgparse.c | 20 +++-
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/src/cfgparse.c b/src/cfgparse.c
index 37bbf453..e529fc6b 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -1798,7 +1798,7 @@ int cfg_parse_global(const char *file, int linenum, char **args, int kwm)
 }
 			}
 		}
-		
+
 		ha_alert("parsing [%s:%d] : unknown keyword '%s' in '%s' section\n", file, linenum, args[0], "global");
 		err_code |= ERR_ALERT | ERR_FATAL;
 	}
@@ -4785,7 +4785,7 @@ stats_error_parsing:
 	reqlen += strlen(args[4]);
 else
 	reqlen += strlen("HTTP/1.0");
-		
+
 curproxy->check_req = malloc(reqlen);
 curproxy->check_len = snprintf(curproxy->check_req, reqlen,
 			   "%s %s %s\r\n", args[2], args[3], *args[4]?args[4]:"HTTP/1.0");
@@ -5094,7 +5094,7 @@ stats_error_parsing:
 			int cur_arg;
 
 			/* insert x-forwarded-for field, but not for the IP address listed as an except.
-			 * set default options (ie: bitfield, header name, etc) 
+			 * set default options (ie: bitfield, header name, etc)
 			 */
 
 			curproxy->options |= PR_O_FWDFOR | PR_O_FF_ALWAYS;
@@ -6071,7 +6071,7 @@ stats_error_parsing:
 			goto out;
 		}
 
-		/* we must first clear any optional default setting */	
+		/* we must first clear any optional default setting */
 		curproxy->conn_src.opts &= ~CO_SRC_TPROXY_MASK;
 		free(curproxy->conn_src.iface_name);
 		curproxy->conn_src.iface_name = NULL;
@@ -6442,7 +6442,7 @@ stats_error_parsing:
 			err_code |= ERR_ALERT | ERR_FATAL;
 			goto out;
 		}
-	
+
 		if ((strcmp(args[2], "if") == 0 || strcmp(args[2], "unless") == 0)) {
 			if ((cond = build_acl_cond(file, linenum, >acl, curproxy, (const char **)args+2, )) == NULL) {
 ha_alert("parsing [%s:%d] : error detected while parsing a '%s' condition : %s.\n",
@@ -7336,6 +7336,7 @@ int check_config_validity()
 	struct bind_conf *bind_conf;
 	char *err;
 	struct cfg_postparser *postparser;
+	struct dns_resolvers *curr_resolvers = NULL;
 
 	bind_conf = NULL;
 	/*
@@ -8976,6 +8977,15 @@ out_uri_auth_compat:
 global.tune.max_http_hdr * sizeof(struct hdr_idx_elem),
 MEM_F_SHARED);
 
+	list_for_each_entry(curr_resolvers, _resolvers, list) {
+		if (LIST_ISEMPTY(_resolvers->nameservers)) {
+			ha_warning("config : resolvers '%s' [%s:%d] has no nameservers configured!\n",
+   curr_resolvers->id, curr_resolvers->conf.file, 
+   curr_resolvers->conf.line);
+			err_code |= ERR_WARN;
+		}
+	}
+
 	list_for_each_entry(postparser, , list) {
 		if (postparser->func)
 			cfgerr += postparser->func();
-- 
2.14.1