Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Traffic Router: Set "minimum" for SOA records to a custom value defined by a parameter #7808

Merged
merged 8 commits into from
Sep 26, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
- [#7784](https://github.com/apache/trafficcontrol/pull/7784) *Traffic Portal*: Added revert certificate functionality to the ssl-keys page.

### Changed
- [#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.
srijeet0406 marked this conversation as resolved.
Show resolved Hide resolved
- [#7776](https://github.com/apache/trafficcontrol/pull/7776) *tc-health-client*: Added error message while issues interacting with Traffic Ops.
- [#7766](https://github.com/apache/trafficcontrol/pull/7766) *Traffic Portal*: now uses Traffic Ops APIv5.
- [#7765](https://github.com/apache/trafficcontrol/pull/7765) *Traffic Stats*: now uses Traffic Ops APIv5.
Expand Down
2 changes: 0 additions & 2 deletions docs/source/admin/traffic_router.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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`) | |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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);

Expand Down Expand Up @@ -377,6 +393,18 @@ 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);
zrhoffman marked this conversation as resolved.
Show resolved Hide resolved
soa = new SOARecord(soa.getName(), DClass.IN, soa.getTTL(), soa.getHost(), soa.getAdmin(),
srijeet0406 marked this conversation as resolved.
Show resolved Hide resolved
soa.getSerial(), soa.getRefresh(), soa.getRetry(), soa.getExpire(), getNegativeCachingTTL());
records.remove(i);
records.add(i, soa);
srijeet0406 marked this conversation as resolved.
Show resolved Hide resolved
break;
}
}
zoneKey.updateTimestamp();

if (zoneKey instanceof SignedZoneKey) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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\": {}}");
Expand Down
Loading
Loading