Merge branch '2670-always-set-rewrite-when-compacting-a-version-1-journal' into 'main'

Resolve "Always set 'rewrite' when compacting a version 1 journal."

Closes #2670

See merge request isc-projects/bind9!4985

CHANGES

@@ -1,3 +1,7 @@
+5635.	[bug]		Journal compaction could fail when a journal with
+			invalid transaction headers was not detected at
+			startup.  [GL #2670]
+
 5634.	[bug]		Don't roll keys when the private key file is offline.
 			[GL #2596]
 

bin/tests/system/journal/clean.sh

@@ -14,3 +14,4 @@ rm -f */named.run
 rm -f dig.out*
 rm -f journalprint.out.*
 rm -f ns1/managed-keys.bind
+rm -f tmp.jnl

bin/tests/system/journal/tests.sh

@@ -156,6 +156,32 @@ c2=$(cat -v ns1/*.jnl | grep -c "BIND LOG V9.2")
 [ $ret -eq 0 ] || echo_i "failed"
 status=`expr $status + $ret`
 
+n=`expr $n + 1`
+echo_i "Check that journal with mixed headers can be compacted (version 1,2,1,2) ($n)"
+ret=0
+journal=ns1/d1212.jnl.saved
+seriallist=$($JOURNALPRINT -x $journal | awk '$1 == "Transaction:" { print $11 }')
+for serial in $seriallist
+do
+        cp $journal tmp.jnl
+        $JOURNALPRINT -c $serial tmp.jnl || ret=1
+done
+[ $ret -eq 0 ] || echo_i "failed"
+status=`expr $status + $ret`
+
+n=`expr $n + 1`
+echo_i "Check that journal with mixed headers can be compacted (version 2,1,2,1) ($n)"
+ret=0
+journal=ns1/d2121.jnl.saved
+seriallist=$($JOURNALPRINT -x $journal | awk '$1 == "Transaction:" { print $11 }')
+for serial in $seriallist
+do
+        cp ns1/d1212.jnl.saved tmp.jnl
+        $JOURNALPRINT -c $serial tmp.jnl || ret=1
+done
+[ $ret -eq 0 ] || echo_i "failed"
+status=`expr $status + $ret`
+
 n=`expr $n + 1`
 echo_i "check upgrade of managed-keys.bind.jnl succeeded($n)"
 ret=0

bin/tools/named-journalprint.c

@@ -68,12 +68,24 @@ main(int argc, char **argv) {
 	isc_log_t *lctx = NULL;
 	uint32_t flags = 0U;
 	int ch;
+	bool compact = false;
 	bool downgrade = false;
 	bool upgrade = false;
+	unsigned int serial = 0;
+	char *endp = NULL;
 
 	progname = argv[0];
-	while ((ch = isc_commandline_parse(argc, argv, "dux")) != -1) {
+	while ((ch = isc_commandline_parse(argc, argv, "c:dux")) != -1) {
 		switch (ch) {
+		case 'c':
+			compact = true;
+			serial = strtoul(isc_commandline_argument, &endp, 0);
+			if (endp == isc_commandline_argument || *endp != 0) {
+				fprintf(stderr, "invalid serial: %s\n",
+					isc_commandline_argument);
+				exit(1);
+			}
+			break;
 		case 'd':
 			downgrade = true;
 			break;
@@ -105,6 +117,9 @@ main(int argc, char **argv) {
 	} else if (downgrade) {
 		flags = DNS_JOURNAL_COMPACTALL | DNS_JOURNAL_VERSION1;
 		result = dns_journal_compact(mctx, file, 0, flags, 0);
+	} else if (compact) {
+		flags = 0;
+		result = dns_journal_compact(mctx, file, serial, flags, 0);
 	} else {
 		result = dns_journal_print(mctx, flags, file, stdout);
 		if (result == DNS_R_NOJOURNAL) {

bin/tools/named-journalprint.rst

@@ -29,7 +29,7 @@ named-journalprint - print zone journal in human-readable form
 Synopsis
 ~~~~~~~~
 
-:program:`named-journalprint` [**-dux**] {journal}
+:program:`named-journalprint` [-c serial] [**-dux**] {journal}
 
 Description
 ~~~~~~~~~~~
@@ -51,6 +51,12 @@ into a human-readable text format. Each line begins with ``add`` or ``del``,
 to indicate whether the record was added or deleted, and continues with
 the resource record in master-file format.
 
+The ``-c`` (compact) option provides a mechanism to reduce the size of
+a journal by removing (most/all) transactions prior to the specified
+serial number. Note: this option *must not* be used while ``named`` is
+running, and can cause data loss if the zone file has not been updated
+to contain the data being removed from the journal. Use with extreme caution.
+
 The ``-x`` option causes additional data about the journal file to be
 printed at the beginning of the output and before each group of changes.
 

doc/man/named-journalprint.1in

@@ -32,7 +32,7 @@ level margin: \\n[rst2man-indent\\n[rst2man-indent-level]]
 ..
 .SH SYNOPSIS
 .sp
-\fBnamed\-journalprint\fP [\fB\-dux\fP] {journal}
+\fBnamed\-journalprint\fP [\-c serial] [\fB\-dux\fP] {journal}
 .SH DESCRIPTION
 .sp
 \fBnamed\-journalprint\fP scans the contents of a zone journal file,
@@ -52,6 +52,12 @@ into a human\-readable text format. Each line begins with \fBadd\fP or \fBdel\fP
 to indicate whether the record was added or deleted, and continues with
 the resource record in master\-file format.
 .sp
+The \fB\-c\fP (compact) option provides a mechanism to reduce the size of
+a journal by removing (most/all) transactions prior to the specified
+serial number. Note: this option \fImust not\fP be used while \fBnamed\fP is
+running, and can cause data loss if the zone file has not been updated
+to contain the data being removed from the journal. Use with extreme caution.
+.sp
 The \fB\-x\fP option causes additional data about the journal file to be
 printed at the beginning of the output and before each group of changes.
 .sp

doc/notes/notes-current.rst

@@ -85,3 +85,6 @@ Bug Fixes
 
 - With ``dnssec-policy```, don't roll keys if the private key file is offline.
   :gl:`#2596`
+
+- Journal compaction could fail when a journal with invalid transaction 
+  headers was not detected at startup. :gl:`#2670`

lib/dns/journal.c

@@ -2493,6 +2493,11 @@ dns_journal_compact(isc_mem_t *mctx, char *filename, uint32_t serial,
 		return (result);
 	}
 
+	/*
+	 * Always perform a re-write when processing a version 1 journal.
+	 */
+	rewrite = j1->header_ver1;
+
 	/*
 	 * Check whether we need to rewrite the whole journal
 	 * file (for example, to upversion it).
@@ -2581,6 +2586,7 @@ dns_journal_compact(isc_mem_t *mctx, char *filename, uint32_t serial,
 	INSIST(best_guess.serial != j1->header.end.serial);
 	if (best_guess.serial != serial) {
 		CHECK(journal_next(j1, &best_guess, false));
+		serial = best_guess.serial;
 	}
 
 	/*