Traffic Router: Set "minimum" for SOA records to a custom value defin…

…ed by a parameter (#7808)

* initial commit

* set param to dns neg ttl

* adding changelog

* change param name

* fix tests

* modify changelog

* fix pmd

* code review

CHANGELOG.md

@@ -187,6 +187,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 - [#7817](https://github.com/apache/trafficcontrol/pull/7817) *Traffic Control Cache Config (t3c)* fixed issue that would cause null ptr panic on client fallback.
 
 ### Removed
+- [#7808](https://github.com/apache/trafficcontrol/pull/7808) *Traffic Router*: Set SOA `minimum` field to a custom value defined in the `tld.soa.minimum` param, and remove the previously added `dns.negative.caching.ttl` property.
 - [#7804](https://github.com/apache/trafficcontrol/pull/7804) Removed unneeded V5 client methods for `deliveryServiceRequiredcapabilities`.
 - [#7271](https://github.com/apache/trafficcontrol/pull/7271) Removed components in `infrastructre/docker/`, not in use as cdn-in-a-box performs the same functionality.
 - [#7271](https://github.com/apache/trafficcontrol/pull/7271) Removed `misc/jira_github_issue_import.py`, the project does not use JIRA.

docs/source/admin/traffic_router.rst

@@ -128,8 +128,6 @@ For the most part, the configuration files and :term:`Parameters` used by Traffi
 	|                            |                                           | To disable the queue, set to 0, or to allow an unlimited sized queue, set to -1. |                                                    |
 	|                            +-------------------------------------------+----------------------------------------------------------------------------------+----------------------------------------------------+
 	|                            | dns.zones.dir                             | Path to automatically generated zone files for reference                         | ``/opt/traffic_router/var/auto-zones``             |
-	|                            +-------------------------------------------+----------------------------------------------------------------------------------+----------------------------------------------------+
-	|                            | dns.negative.caching.ttl                  | Value (in seconds) to set as the ``minimum`` for NXDOMAIN and NXRRSET responses  | ``900``                                            |
 	+----------------------------+-------------------------------------------+----------------------------------------------------------------------------------+----------------------------------------------------+
 	| traffic_ops.properties     | traffic_ops.username                      | Username with which to access the :ref:`to-api`                                  | ``admin``                                          |
 	|                            |                                           | (must have the ``admin`` :term:`Role`)                                           |                                                    |

traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/dns/NameServer.java

@@ -42,7 +42,6 @@
 
 @SuppressWarnings("PMD.CyclomaticComplexity")
 public class NameServer {
-	private static long negativeCachingTTL = 0L;
 	private static final int MAX_SUPPORTED_EDNS_VERS = 0;
 	private static final int MAX_ITERATIONS = 6;
 	private static final int NUM_SECTIONS = 4;
@@ -329,20 +328,12 @@ private static RRset setNegativeTTL(final RRset original, final int flags) {
 			if (record instanceof SOARecord) {
 				final SOARecord soa = (SOARecord) record;
 
-				// Set the "minimum" attribute to be the maximum of the current minimum value and 900 (seconds)
-				// This is done to increase the negative caching TTL, so as to maximize the time interval between
-				// successive NXDOMAIN or NXRRSET responses.
-				final long minimum = Math.max(soa.getMinimum(), negativeCachingTTL);
-				final long ttl;
 				// the value of the minimum field is less than the actual TTL; adjust
-				if (minimum != 0 || soa.getTTL() > minimum) {
-					ttl = minimum;
-				} else {
-					ttl = soa.getTTL();
-				}
-				record = new SOARecord(soa.getName(), DClass.IN, ttl, soa.getHost(), soa.getAdmin(),
-						soa.getSerial(), soa.getRefresh(), soa.getRetry(), soa.getExpire(),
-						minimum);
+				if (soa.getMinimum() != 0 || soa.getTTL() > soa.getMinimum()) {
+					record = new SOARecord(soa.getName(), DClass.IN, soa.getMinimum(), soa.getHost(), soa.getAdmin(),
+							soa.getSerial(), soa.getRefresh(), soa.getRetry(), soa.getExpire(),
+							soa.getMinimum());
+				} // else use the unmodified record
 			}
 
 			rrset.addRR(record);
@@ -442,14 +433,6 @@ public void setTrafficRouterManager(final TrafficRouterManager trafficRouterMana
 		this.trafficRouterManager = trafficRouterManager;
 	}
 
-	public long getNegativeCachingTTL() {
-		return negativeCachingTTL;
-	}
-
-	public void setNegativeCachingTTL(final long negativeCachingTTL) {
-		this.negativeCachingTTL = negativeCachingTTL;
-	}
-
 	public void destroy() {
 		/*
 		 * Yes, this is odd. We need to call destroy on ZoneManager, but it's static, so

traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/dns/ZoneManager.java

@@ -101,6 +101,7 @@
 public class ZoneManager extends Resolver {
 	private static final Logger LOGGER = LogManager.getLogger(ZoneManager.class);
 
+	private static long negativeCachingTTL = 0L;
 	private final TrafficRouter trafficRouter;
 	private static LoadingCache<ZoneKey, Zone> dynamicZoneCache = null;
 	private static LoadingCache<ZoneKey, Zone> zoneCache = null;
@@ -155,6 +156,20 @@ private void initSignatureManager(final CacheRegister cacheRegister, final Traff
 		ZoneManager.signatureManager = sm;
 	}
 
+	public static void setNegativeCachingTTL(final JsonNode config) {
+		JsonNode node = null;
+		try {
+			node = JsonUtils.getJsonNode(JsonUtils.getJsonNode(config, "config"), "soa");
+		} catch (JsonUtilsException e) {
+			LOGGER.warn("Couldn't find a JSON node for config or soa; continuing by setting the minimum value to 900", e);
+		} finally {
+			negativeCachingTTL = JsonUtils.optLong(node, "minimum", 900L);
+		}
+	}
+	public static long getNegativeCachingTTL() {
+		return negativeCachingTTL;
+	}
+
 	@SuppressWarnings({"PMD.CyclomaticComplexity", "PMD.NPathComplexity"})
 	protected static void initZoneCache(final TrafficRouter tr) {
 		synchronized(ZoneManager.class) {
@@ -172,6 +187,7 @@ protected static void initZoneCache(final TrafficRouter tr) {
 			final int maintenanceInterval = JsonUtils.optInt(config, "zonemanager.cache.maintenance.interval", 300); // default 5 minutes
 			final int initTimeout = JsonUtils.optInt(config, "zonemanager.init.timeout", 10);
 
+			setNegativeCachingTTL(config);
 			final LoadingCache<ZoneKey, Zone> dzc = createZoneCache(ZoneCacheType.DYNAMIC, getDynamicZoneCacheSpec(config, poolSize));
 			final LoadingCache<ZoneKey, Zone> zc = createZoneCache(ZoneCacheType.STATIC);
 
@@ -377,6 +393,17 @@ public static Zone loadZone(final ZoneKey zoneKey, final boolean writeZone) thro
 		LOGGER.debug("Attempting to load " + zoneKey.getName());
 		final Name name = zoneKey.getName();
 		List<Record> records = zoneKey.getRecords();
+		// For SOA records, set the "minimum" to the value set in the tld.soa.minimum parameter in
+		// CRConfig.json.
+		for (int i=0; i < records.size(); i++) {
+			if (records.get(i).getType() == Type.SOA) {
+				SOARecord soa = (SOARecord)records.get(i);
+				soa = new SOARecord(soa.getName(), soa.getDClass(), soa.getTTL(), soa.getHost(), soa.getAdmin(),
+						soa.getSerial(), soa.getRefresh(), soa.getRetry(), soa.getExpire(), getNegativeCachingTTL());
+				records.set(i, soa);
+				break;
+			}
+		}
 		zoneKey.updateTimestamp();
 
 		if (zoneKey instanceof SignedZoneKey) {

traffic_router/core/src/main/webapp/WEB-INF/applicationContext.xml

@@ -267,7 +267,6 @@
 
 	<bean id="NameServer" class="org.apache.traffic_control.traffic_router.core.dns.NameServer">
 		<property name="trafficRouterManager" ref="trafficRouterManager" />
-		<property name="negativeCachingTTL" value="$[dns.negative.caching.ttl:900]" />
 	</bean>
 
 	<bean id="UDPBlockingQueue" class="java.util.concurrent.LinkedBlockingQueue" />

traffic_router/core/src/test/java/org/apache/traffic_control/traffic_router/core/dns/NameServerTest.java

@@ -82,12 +82,6 @@ public void before() throws Exception {
 	    ns = new NSRecord(m_an, DClass.IN, 12345L, m_an);
     }
 
-    @Test
-    public void TestNegativeCachingTTLGetterAndSetter() throws Exception {
-        nameServer.setNegativeCachingTTL(900L);
-        assertThat(nameServer.getNegativeCachingTTL(), equalTo(900L));
-    }
-
     @Test
     public void TestARecordQueryWithClientSubnetOption() throws Exception {
 

traffic_router/core/src/test/java/org/apache/traffic_control/traffic_router/core/dns/ZoneManagerUnitTest.java

@@ -54,6 +54,7 @@
 import org.xbill.DNS.NSRecord;
 import org.xbill.DNS.CNAMERecord;
 
+import java.io.File;
 import java.net.InetAddress;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -93,6 +94,15 @@ public void before() throws Exception {
 
     }
 
+    @Test
+    public void testNegativeCachingTTLGetterAndSetter() throws Exception {
+        final File file = new File("src/test/resources/publish/CrConfig5.json");
+        final ObjectMapper mapper = new ObjectMapper();
+        final JsonNode jo = mapper.readTree(file);
+        zoneManager.setNegativeCachingTTL(jo);
+        assertThat(zoneManager.getNegativeCachingTTL(), equalTo(1200L));
+    }
+
     @Test
     public void testGetLocalTRHostnameUsesSingleTRHostname() throws Exception {
         JsonNode trs = new ObjectMapper().readTree("{\"tr-01\": {}}");