zrhoffman commented on code in PR #7808:
URL: https://github.com/apache/trafficcontrol/pull/7808#discussion_r1336463546


##########
CHANGELOG.md:
##########
@@ -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.

Review Comment:
   Since `dns.negative.caching.ttl` is in the 8.0.0 release, this changelog 
entry should mention removing it.



##########
traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/dns/ZoneManager.java:
##########
@@ -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);
+                               soa = new SOARecord(soa.getName(), DClass.IN, 
soa.getTTL(), soa.getHost(), soa.getAdmin(),

Review Comment:
   > `DClass.IN`
   
   nit: Should this be `soa.getDClass()`?



##########
traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/dns/ZoneManager.java:
##########
@@ -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);

Review Comment:
   Move the
   
   ```java
   SOARecord soa = (SOARecord)records.get(i);
   ```
   out of the `if` block so you can use it in the `if` condition?



##########
traffic_router/core/src/main/java/org/apache/traffic_control/traffic_router/core/dns/ZoneManager.java:
##########
@@ -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);
+                               soa = new SOARecord(soa.getName(), DClass.IN, 
soa.getTTL(), soa.getHost(), soa.getAdmin(),
+                                               soa.getSerial(), 
soa.getRefresh(), soa.getRetry(), soa.getExpire(), getNegativeCachingTTL());
+                               records.remove(i);
+                               records.add(i, soa);

Review Comment:
   ```java
   records.set(i, soa);
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to