Merge branch 'rndc-nta-preen' into 'master'

Fixes for `rndc nta` user interface

See merge request isc-projects/bind9!845

CHANGES

@@ -1,3 +1,13 @@
+5080.	[func]		Improvements to "rndc nta" user interface:
+			- catch and report invalid command line options
+			- when removing an NTA from all views, do not
+			  abort with an error if the NTA was not found
+			  in one of the views
+			- include the view name in "rndc nta -dump"
+			  output, for consistency with the add and remove
+			  actions
+			Thanks to Tony Finch. [GL !816]
+
 5079.	[func]		Disable IDN processing in dig and nslookup
 			when not on a tty. [GL #653]
 

bin/named/server.c

@@ -10877,7 +10877,7 @@ named_server_dumpsecroots(named_server_t *server, isc_lex_t *lex,
 				continue;
 			}
 			CHECK(putstr(text, "\n   Negative trust anchors:\n\n"));
-			CHECK(dns_ntatable_totext(ntatable, text));
+			CHECK(dns_ntatable_totext(ntatable, NULL, text));
 		}
 		if (ptr != NULL)
 			ptr = next_token(lex, text);
@@ -14314,6 +14314,7 @@ named_server_nta(named_server_t *server, isc_lex_t *lex,
 	isc_result_t result = ISC_R_SUCCESS;
 	char *ptr, *nametext = NULL, *viewname;
 	char namebuf[DNS_NAME_FORMATSIZE];
+	char viewbuf[DNS_NAME_FORMATSIZE];
 	isc_stdtime_t now, when;
 	isc_time_t t;
 	char tbuf[64];
@@ -14323,8 +14324,9 @@ named_server_nta(named_server_t *server, isc_lex_t *lex,
 	const dns_name_t *ntaname;
 	dns_name_t *fname;
 	dns_ttl_t ntattl;
-	bool ttlset = false, excl = false;
+	bool ttlset = false, excl = false, viewfound = false;
 	dns_rdataclass_t rdclass = dns_rdataclass_in;
+	bool first = true;
 
 	UNUSED(force);
 
@@ -14337,18 +14339,24 @@ named_server_nta(named_server_t *server, isc_lex_t *lex,
 	}
 
 	for (;;) {
+		bool opts = true;
+
 		/* Check for options */
 		ptr = next_token(lex, text);
 		if (ptr == NULL) {
 			return (ISC_R_UNEXPECTEDEND);
 		}
 
-		if (argcheck(ptr, "dump")) {
+		if (!opts) {
+			nametext = ptr;
+		} else if (strcmp(ptr, "--") == 0) {
+			opts = false;
+		} else if (argcheck(ptr, "dump")) {
 			dump = true;
 		} else if (argcheck(ptr, "remove")) {
 			ntattl = 0;
 			ttlset = true;
-		} else if (argcheck(ptr, "force")) {
+		} else if (opts && argcheck(ptr, "force")) {
 			force = true;
 			continue;
 		} else if (argcheck(ptr, "lifetime")) {
@@ -14388,6 +14396,9 @@ named_server_nta(named_server_t *server, isc_lex_t *lex,
 			tr.length = strlen(ptr);
 			CHECK(dns_rdataclass_fromtext(&rdclass, &tr));
 			continue;
+		} else if (ptr[0] == '-') {
+			msg = "Unknown option";
+			CHECK(DNS_R_SYNTAX);
 		} else {
 			nametext = ptr;
 		}
@@ -14410,7 +14421,8 @@ named_server_nta(named_server_t *server, isc_lex_t *lex,
 			if (result == ISC_R_NOTFOUND) {
 				continue;
 			}
-			CHECK(dns_ntatable_totext(ntatable, text));
+
+			CHECK(dns_ntatable_totext(ntatable, view->name, text));
 		}
 		CHECK(putnull(text));
 
@@ -14448,6 +14460,14 @@ named_server_nta(named_server_t *server, isc_lex_t *lex,
 
 	/* Look for the view name. */
 	viewname = next_token(lex, text);
+	if (viewname != NULL) {
+		strlcpy(viewbuf, viewname, DNS_NAME_FORMATSIZE);
+		viewname = viewbuf;
+	}
+
+	if (next_token(lex, text) != NULL) {
+		CHECK(DNS_R_SYNTAX);
+	}
 
 	isc_stdtime_get(&now);
 
@@ -14458,11 +14478,10 @@ named_server_nta(named_server_t *server, isc_lex_t *lex,
 	     view != NULL;
 	     view = ISC_LIST_NEXT(view, link))
 	{
-		static bool first = true;
-
 		if (viewname != NULL && strcmp(view->name, viewname) != 0) {
 			continue;
 		}
+		viewfound = true;
 
 		if (view->rdclass != rdclass && rdclass != dns_rdataclass_any) {
 			continue;
@@ -14518,22 +14537,37 @@ named_server_nta(named_server_t *server, isc_lex_t *lex,
 				      "added NTA '%s' (%d sec) in view '%s'",
 				      namebuf, ntattl, view->name);
 		} else {
-			CHECK(dns_ntatable_delete(ntatable, ntaname));
+			bool removed;
+
+			result = dns_ntatable_delete(ntatable, ntaname);
+			if (result == ISC_R_SUCCESS) {
+				removed = true;
+			} else if (result == ISC_R_NOTFOUND) {
+				removed = false;
+			} else {
+				goto cleanup;
+			}
 
 			if (!first) {
 				CHECK(putstr(text, "\n"));
 			}
 			first = false;
 
-			CHECK(putstr(text, "Negative trust anchor removed: "));
+			CHECK(putstr(text, "Negative trust anchor "));
+			CHECK(putstr(text, removed ? "removed: "
+						   : "not found: "));
 			CHECK(putstr(text, namebuf));
 			CHECK(putstr(text, "/"));
 			CHECK(putstr(text, view->name));
 
-			isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL,
+			if (removed) {
-				      NAMED_LOGMODULE_SERVER, ISC_LOG_INFO,
+				isc_log_write(named_g_lctx,
-				      "removed NTA '%s' in view %s",
+					      NAMED_LOGCATEGORY_GENERAL,
-				      namebuf, view->name);
+					      NAMED_LOGMODULE_SERVER,
+					      ISC_LOG_INFO,
+					      "removed NTA '%s' in view %s",
+					      namebuf, view->name);
+			}
 		}
 
 		result = dns_view_saventa(view);
@@ -14546,6 +14580,11 @@ named_server_nta(named_server_t *server, isc_lex_t *lex,
 		}
 	}
 
+	if (!viewfound) {
+		msg = "No such view";
+		CHECK(ISC_R_NOTFOUND);
+	}
+
 	CHECK(putnull(text));
 
  cleanup:
@@ -14553,6 +14592,7 @@ named_server_nta(named_server_t *server, isc_lex_t *lex,
 		(void) putstr(text, msg);
 		(void) putnull(text);
 	}
+
 	if (excl) {
 		isc_task_endexclusive(server->task);
 	}

bin/rndc/rndc.docbook

@@ -650,6 +650,12 @@
 	    <option>-l</option>, <option>-r</option>, <option>-d</option>,
 	    <option>-f</option>, and <option>-c</option>.
 	  </para>
+	  <para>
+	    Unrecognized options are treated as errors. To reference
+	    a domain or view name that begins with a hyphen,
+	    use a double-hyphen on the command line to indicate the
+	    end of options.
+	  </para>
 	</listitem>
       </varlistentry>
 

bin/tests/system/dnssec/tests.sh

@@ -1873,8 +1873,8 @@ $PERL -e 'my $delay = '$start' + 13 - time(); select(undef, undef, undef, $delay
 $RNDCCMD 10.53.0.4 nta -d > rndc.out.ns4.test$n._11
 lines=`grep " expiry " rndc.out.ns4.test$n._11 | wc -l`
 [ "$lines" -le 2 ] || ret=1
-grep "bogus.example: expiry" rndc.out.ns4.test$n._11 > /dev/null || ret=1
+grep "bogus.example/_default: expiry" rndc.out.ns4.test$n._11 > /dev/null || ret=1
-grep "badds.example: expiry" rndc.out.ns4.test$n._11 > /dev/null && ret=1
+grep "badds.example/_default: expiry" rndc.out.ns4.test$n._11 > /dev/null && ret=1
 $DIG $DIGOPTS b.bogus.example. a @10.53.0.4 > dig.out.ns4.test$n.11 || ret=1
 grep "status: SERVFAIL" dig.out.ns4.test$n.11 > /dev/null && ret=1
 $DIG $DIGOPTS a.badds.example. a @10.53.0.4 > dig.out.ns4.test$n.12 || ret=1
@@ -1910,14 +1910,14 @@ ret=0
 echo_i "testing NTA removals ($n)"
 $RNDCCMD 10.53.0.4 nta badds.example 2>&1 | sed 's/^/ns4 /' | cat_i
 $RNDCCMD 10.53.0.4 nta -d > rndc.out.ns4.test$n.1
-grep "badds.example: expiry" rndc.out.ns4.test$n.1 > /dev/null || ret=1
+grep "badds.example/_default: expiry" rndc.out.ns4.test$n.1 > /dev/null || ret=1
 $DIG $DIGOPTS a.badds.example. a @10.53.0.4 > dig.out.ns4.test$n.1 || ret=1
 grep "status: SERVFAIL" dig.out.ns4.test$n.1 > /dev/null && ret=1
 grep "^a.badds.example." dig.out.ns4.test$n.1 > /dev/null || ret=1
 $RNDCCMD 10.53.0.4 nta -remove badds.example > rndc.out.ns4.test$n.2
 grep "Negative trust anchor removed: badds.example/_default" rndc.out.ns4.test$n.2 > /dev/null || ret=1
 $RNDCCMD 10.53.0.4 nta -d > rndc.out.ns4.test$n.3
-grep "badds.example: expiry" rndc.out.ns4.test$n.3 > /dev/null && ret=1
+grep "badds.example/_default: expiry" rndc.out.ns4.test$n.3 > /dev/null && ret=1
 $DIG $DIGOPTS a.badds.example. a @10.53.0.4 > dig.out.ns4.test$n.2 || ret=1
 grep "status: SERVFAIL" dig.out.ns4.test$n.2 > /dev/null || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
@@ -1928,7 +1928,7 @@ echo_i "remove non-existent NTA three times"
 $RNDCCMD 10.53.0.4 nta -r foo > rndc.out.ns4.test$n.4 2>&1
 $RNDCCMD 10.53.0.4 nta -remove foo > rndc.out.ns4.test$n.5 2>&1
 $RNDCCMD 10.53.0.4 nta -r foo > rndc.out.ns4.test$n.6 2>&1
-grep "'nta' failed: not found" rndc.out.ns4.test$n.6 > /dev/null || ret=1
+grep "not found" rndc.out.ns4.test$n.6 > /dev/null || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 ret=0
@@ -2009,7 +2009,7 @@ sleep 4
 $RNDCCMD 10.53.0.4 nta -d > rndc.out.ns4.test$n.3
 lines=`wc -l < rndc.out.ns4.test$n.3`
 [ "$lines" -eq 1 ] || ret=1
-grep "bogus.example: expiry" rndc.out.ns4.test$n.3 > /dev/null || ret=1
+grep "bogus.example/_default: expiry" rndc.out.ns4.test$n.3 > /dev/null || ret=1
 $DIG $DIGOPTS b.bogus.example. a @10.53.0.4 > dig.out.ns4.test$n.4 || ret=1
 grep "status: SERVFAIL" dig.out.ns4.test$n.4 > /dev/null && ret=1
 grep "flags:[^;]* ad[^;]*;" dig.out.ns4.test$n.4 > /dev/null && ret=1

doc/arm/notes.xml

@@ -547,6 +547,13 @@
 	  added. This has been fixed. [GL #105]
 	</para>
       </listitem>
+      <listitem>
+	<para>
+	  The view name is now included in the output of
+	  <command>rndc nta -dump</command>, for consistency with
+	  other options. [GL !816]
+	</para>
+      </listitem>
       <listitem>
 	<para>
 	  <command>named</command> now rejects excessively large

lib/dns/include/dns/nta.h

@@ -182,9 +182,10 @@ dns_ntatable_covered(dns_ntatable_t *ntatable, isc_stdtime_t now,
  */
 
 isc_result_t
-dns_ntatable_totext(dns_ntatable_t *ntatable, isc_buffer_t **buf);
+dns_ntatable_totext(dns_ntatable_t *ntatable, const char *view,
+		    isc_buffer_t **buf);
 /*%<
- * Dump the NTA table to buffer at 'buf'
+ * Dump the NTA table to buffer at 'buf', with view names
  *
  * Requires:
  * \li   "ntatable" is a valid table.

lib/dns/nta.c

@@ -509,7 +509,9 @@ putstr(isc_buffer_t **b, const char *str) {
 }
 
 isc_result_t
-dns_ntatable_totext(dns_ntatable_t *ntatable, isc_buffer_t **buf) {
+dns_ntatable_totext(dns_ntatable_t *ntatable, const char *view,
+		    isc_buffer_t **buf)
+{
 	isc_result_t result;
 	dns_rbtnode_t *node;
 	dns_rbtnodechain_t chain;
@@ -552,8 +554,10 @@ dns_ntatable_totext(dns_ntatable_t *ntatable, isc_buffer_t **buf) {
 				isc_time_formattimestamp(&t, tbuf,
 							 sizeof(tbuf));
 
-				snprintf(obuf, sizeof(obuf), "%s%s: %s %s",
+				snprintf(obuf, sizeof(obuf), "%s%s%s%s: %s %s",
 					 first ? "" : "\n", nbuf,
+					 view != NULL ? "/" : "",
+					 view != NULL ? view : "",
 					 n->expiry <= now
 					  ? "expired"
 					  : "expiry",
@@ -588,7 +592,7 @@ dns_ntatable_dump(dns_ntatable_t *ntatable, FILE *fp) {
 	if (result != ISC_R_SUCCESS)
 		return (result);
 
-	result = dns_ntatable_totext(ntatable, &text);
+	result = dns_ntatable_totext(ntatable, NULL, &text);
 
 	if (isc_buffer_usedlength(text) != 0) {
 		(void) putstr(&text, "\n");