Pull request 2113: 6409-cache-ttl-override

Updates #6409.

Squashed commit of the following:

commit 7dea2a383ecdcef0de651f2cf720c1eb7a4486a3
Merge: 959b61862 ad147ac7b
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Tue Dec 26 14:32:15 2023 +0300

    Merge branch 'master' into 6409-cache-ttl-override

commit 959b61862fc007118e287e5ed7aefa8605b0164a
Merge: ca02da2d2 4bc5c346a
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Fri Dec 22 15:13:12 2023 +0300

    Merge branch 'master' into 6409-cache-ttl-override

commit ca02da2d292a6ecf020c2957c22b8b0fa5d86104
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Fri Dec 22 15:12:12 2023 +0300

    all: imp docs

commit 183eab145fbc09dd9a7a7beea357e3d50dea3f3f
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Thu Dec 21 17:29:39 2023 +0300

    all: upd chlog

commit 51c7a526019a676411b06d4ee80ad1c95bde5a2e
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Mon Dec 18 20:23:13 2023 +0300

    all: cache ttl override

CHANGELOG.md

@@ -34,7 +34,7 @@ NOTE: Add new changes BELOW THIS COMMENT.
 ### Changed
 
 - The field `"upstream_mode"` in `POST /control/dns_config` and
-  `GET /control/dns_info` HTTP APIs now accepts `load_balance` value. Check
+  `GET /control/dns_info` HTTP APIs now accepts `load_balance` value.  Check
   `openapi/CHANGELOG.md` for more details.
 
 #### Configuration changes
@@ -58,6 +58,8 @@ NOTE: Add new changes BELOW THIS COMMENT.
 
 ### Fixed
 
+- Maximum cache TTL requirement when editing minimum cache TTL in the Web UI
+  ([#6409]).
 - Load balancing algorithm stuck on a single server ([#6480]).
 - Statistics for 7 days displayed as 168 hours on the dashboard.
 - Pre-filling the Edit static lease window with data ([#6534]).
@@ -67,6 +69,7 @@ NOTE: Add new changes BELOW THIS COMMENT.
   work on iOS ([#6352]).
 
 [#6352]: https://github.com/AdguardTeam/AdGuardHome/issues/6352
+[#6409]: https://github.com/AdguardTeam/AdGuardHome/issues/6409
 [#6480]: https://github.com/AdguardTeam/AdGuardHome/issues/6480
 [#6534]: https://github.com/AdguardTeam/AdGuardHome/issues/6534
 [#6541]: https://github.com/AdguardTeam/AdGuardHome/issues/6541

client/src/components/Settings/Dns/Cache/Form.js

@@ -41,7 +41,7 @@ const Form = ({
         cache_ttl_max, cache_ttl_min,
     } = useSelector((state) => state.form[FORM_NAME.CACHE].values, shallowEqual);
 
-    const minExceedsMax = cache_ttl_min > cache_ttl_max;
+    const minExceedsMax = cache_ttl_min > 0 && cache_ttl_max > 0 && cache_ttl_min > cache_ttl_max;
 
     const handleClearCache = () => {
         if (window.confirm(t('confirm_dns_cache_clear'))) {

internal/dnsforward/config.go

@@ -329,11 +329,6 @@ func (s *Server) newProxyConfig() (conf *proxy.Config, err error) {
 		conf.EDNSAddr = net.IP(srvConf.EDNSClientSubnet.CustomIP.AsSlice())
 	}
 
-	if srvConf.CacheSize != 0 {
-		conf.CacheEnabled = true
-		conf.CacheSizeBytes = int(srvConf.CacheSize)
-	}
-
 	err = setProxyUpstreamMode(conf, srvConf.UpstreamMode, srvConf.FastestTimeout.Duration)
 	if err != nil {
 		return nil, fmt.Errorf("upstream mode: %w", err)
@@ -365,6 +360,37 @@ func (s *Server) newProxyConfig() (conf *proxy.Config, err error) {
 		return nil, errors.Error("no default upstream servers configured")
 	}
 
+	conf, err = prepareCacheConfig(conf,
+		srvConf.CacheSize,
+		srvConf.CacheMinTTL,
+		srvConf.CacheMaxTTL,
+	)
+	if err != nil {
+		// Don't wrap the error since it's informative enough as is.
+		return nil, err
+	}
+
+	return conf, nil
+}
+
+// prepareCacheConfig prepares the cache configuration and returns an error if
+// there is one.
+func prepareCacheConfig(
+	conf *proxy.Config,
+	size uint32,
+	minTTL uint32,
+	maxTTL uint32,
+) (prepared *proxy.Config, err error) {
+	if size != 0 {
+		conf.CacheEnabled = true
+		conf.CacheSizeBytes = int(size)
+	}
+
+	err = validateCacheTTL(minTTL, maxTTL)
+	if err != nil {
+		return nil, fmt.Errorf("validating cache ttl: %w", err)
+	}
+
 	return conf, nil
 }
 
@@ -743,3 +769,19 @@ func (s *Server) enableProtectionAfterPause() {
 
 	log.Info("dns: protection is restarted after pause")
 }
+
+// validateCacheTTL returns an error if the configuration of the cache TTL
+// invalid.
+//
+// TODO(s.chzhen):  Move to dnsproxy.
+func validateCacheTTL(minTTL, maxTTL uint32) (err error) {
+	if minTTL == 0 && maxTTL == 0 {
+		return nil
+	}
+
+	if maxTTL > 0 && minTTL > maxTTL {
+		return errors.Error("cache_ttl_min must be less than or equal to cache_ttl_max")
+	}
+
+	return nil
+}

internal/dnsforward/http.go

@@ -378,15 +378,12 @@ func (req *jsonDNSConfig) checkCacheTTL() (err error) {
 	if req.CacheMinTTL != nil {
 		minTTL = *req.CacheMinTTL
 	}
+
 	if req.CacheMaxTTL != nil {
 		maxTTL = *req.CacheMaxTTL
 	}
 
-	if minTTL <= maxTTL {
+	return validateCacheTTL(minTTL, maxTTL)
-		return nil
-	}
-
-	return errors.Error("cache_ttl_min must be less or equal than cache_ttl_max")
 }
 
 // checkRatelimitSubnetMaskLen returns an error if the length of the subnet mask

internal/dnsforward/http_test.go

@@ -231,7 +231,7 @@ func TestDNSForwardHTTP_handleSetConfig(t *testing.T) {
 			`ParseAddr("a"): unable to parse IP`,
 	}, {
 		name:    "cache_bad_ttl",
-		wantSet: `validating dns config: cache_ttl_min must be less or equal than cache_ttl_max`,
+		wantSet: `validating dns config: cache_ttl_min must be less than or equal to cache_ttl_max`,
 	}, {
 		name:    "upstream_mode_bad",
 		wantSet: `validating dns config: upstream_mode: incorrect value "somethingelse"`,