Pull request: 3020 runtime clients sources control

Merge in DNS/adguard-home from 3020-client-sources to master Closes #3020. Squashed commit of the following: commit f8e6b6d63373f99b52f7b8c32f4242c453daf1a4 Merge: 41fb071d 0a1ff65b Author: Ainar Garipov <A.Garipov@AdGuard.COM> Date: Mon Apr 25 19:19:15 2022 +0300 Merge branch 'master' into 3020-client-sources commit 41fb071deb2a87e0a69d09c8f418a016b4dd7e93 Author: Eugene Burkov <E.Burkov@AdGuard.COM> Date: Mon Apr 25 13:38:28 2022 +0300 home: fix nil, imp docs commit aaa7765914a8a4645eba357cd088a9470611ffdc Author: Eugene Burkov <E.Burkov@AdGuard.COM> Date: Mon Apr 25 12:25:47 2022 +0300 home: imp code commit 3f71b999564c604583b46313d29f5b70cf51ee14 Author: Eugene Burkov <E.Burkov@AdGuard.COM> Date: Fri Apr 22 19:12:27 2022 +0300 home: runtime clients sources control
2024-08-16 22:20:30 +03:00 · 2022-04-26 13:04:16 +03:00 · 2022-04-26 13:04:16 +03:00 · 235316e050
commit 235316e050
parent 0a1ff65b4a
8 changed files with 283 additions and 65 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -23,6 +23,8 @@ and this project adheres to

 ### Added

+- The ability to control each source of runtime clients separately via
+  `clients.runtime_sources` configuration object ([#3020]).
 - The ability to customize the set of networks that are considered private
  through the new `dns.private_networks` property in the configuration file
  ([#3142]).
@ -63,8 +65,36 @@ and this project adheres to

 #### Configuration Changes

-In this release, the schema version has changed from 12 to 13.
+In this release, the schema version has changed from 12 to 14.

+- Object `clients`, which in schema versions 13 and earlier was an array of
+  actual persistent clients, is now consist of `persistent` and
+  `runtime_sources` properties:
+
+  ```yaml
+  # BEFORE:
+  'clients':
+  - name: client-name
+    # …
+
+  # AFTER:
+  'clients':
+    'persistent':
+      - name: client-name
+        # …
+    'runtime_sources':
+      whois: true
+      arp: true
+      rdns: true
+      dhcp: true
+      hosts: true
+  ```
+
+  The value for `clients.runtime_sources.rdns` field is taken from
+  `dns.resolve_clients` property.  To rollback this change, remove the
+  `runtime_sources` property, move the contents of `persistent` into the
+  `clients` itself, the value of `clients.runtime_sources.rdns` into the
+  `dns.resolve_clietns`, and change the `schema_version` back to `13`.
 - Property `local_domain_name`, which in schema versions 12 and earlier used to
  be a part of the `dns` object, is now a part of the `dhcp` object:

@ -85,6 +115,9 @@ In this release, the schema version has changed from 12 to 13.

 ### Deprecated

+- The `--no-etc-hosts` option.  Its' functionality is now controlled by
+  `clients.runtime_sources.hosts` configuration property.  v0.109.0 will remove
+  the flag completely.
 - Go 1.17 support.  v0.109.0 will require at least Go 1.18 to build.

 ### Fixed
@ -94,6 +127,7 @@ In this release, the schema version has changed from 12 to 13.

 [#1730]: https://github.com/AdguardTeam/AdGuardHome/issues/1730
 [#2993]: https://github.com/AdguardTeam/AdGuardHome/issues/2993
+[#3020]: https://github.com/AdguardTeam/AdGuardHome/issues/3020
 [#3057]: https://github.com/AdguardTeam/AdGuardHome/issues/3057
 [#3142]: https://github.com/AdguardTeam/AdGuardHome/issues/3142
 [#3157]: https://github.com/AdguardTeam/AdGuardHome/issues/3157
--- a/internal/home/clients.go
+++ b/internal/home/clients.go
@ -59,6 +59,16 @@ const (
 	ClientSourceHostsFile
 )

+// clientSourceConf is used to configure where the runtime clients will be
+// obtained from.
+type clientSourcesConf struct {
+	WHOIS     bool `yaml:"whois"`
+	ARP       bool `yaml:"arp"`
+	RDNS      bool `yaml:"rdns"`
+	DHCP      bool `yaml:"dhcp"`
+	HostsFile bool `yaml:"hosts"`
+}
+
 // RuntimeClient information
 type RuntimeClient struct {
 	WHOISInfo *RuntimeClientWHOISInfo
@ -134,14 +144,14 @@ func (clients *clientsContainer) Init(
 		clients.dhcpServer.SetOnLeaseChanged(clients.onDHCPLeaseChanged)
 	}

-	go clients.handleHostsUpdates()
+	if clients.etcHosts != nil {
+		go clients.handleHostsUpdates()
+	}
 }

 func (clients *clientsContainer) handleHostsUpdates() {
-	if clients.etcHosts != nil {
-		for upd := range clients.etcHosts.Upd() {
-			clients.addFromHostsFile(upd)
-		}
+	for upd := range clients.etcHosts.Upd() {
+		clients.addFromHostsFile(upd)
 	}
 }

@ -158,7 +168,9 @@ func (clients *clientsContainer) Start() {

 // Reload reloads runtime clients.
 func (clients *clientsContainer) Reload() {
-	clients.addFromSystemARP()
+	if clients.arpdb != nil {
+		clients.addFromSystemARP()
+	}
 }

 type clientObject struct {
@ -843,7 +855,7 @@ func (clients *clientsContainer) addFromSystemARP() {
 // updateFromDHCP adds the clients that have a non-empty hostname from the DHCP
 // server.
 func (clients *clientsContainer) updateFromDHCP(add bool) {
-	if clients.dhcpServer == nil {
+	if clients.dhcpServer == nil || !config.Clients.Sources.DHCP {
 		return
 	}

--- a/internal/home/config.go
+++ b/internal/home/config.go
@ -51,6 +51,13 @@ type osConfig struct {
 	RlimitNoFile uint64 `yaml:"rlimit_nofile"`
 }

+type clientsConfig struct {
+	// Sources defines the set of sources to fetch the runtime clients from.
+	Sources *clientSourcesConf `yaml:"runtime_sources"`
+	// Persistent are the configured clients.
+	Persistent []*clientObject `yaml:"persistent"`
+}
+
 // configuration is loaded from YAML
 // field ordering is important -- yaml fields will mirror ordering from here
 type configuration struct {
@ -88,7 +95,7 @@ type configuration struct {
 	// Clients contains the YAML representations of the persistent clients.
 	// This field is only used for reading and writing persistent client data.
 	// Keep this field sorted to ensure consistent ordering.
-	Clients []*clientObject `yaml:"clients"`
+	Clients *clientsConfig `yaml:"clients"`

 	logSettings `yaml:",inline"`

@ -123,9 +130,6 @@ type dnsConfig struct {
 	// UpstreamTimeout is the timeout for querying upstream servers.
 	UpstreamTimeout timeutil.Duration `yaml:"upstream_timeout"`

-	// ResolveClients enables and disables resolving clients with RDNS.
-	ResolveClients bool `yaml:"resolve_clients"`
-
 	// PrivateNets is the set of IP networks for which the private reverse DNS
 	// resolver should be used.
 	PrivateNets []string `yaml:"private_networks"`
@ -198,7 +202,6 @@ var config = &configuration{
 		FilteringEnabled:           true, // whether or not use filter lists
 		FiltersUpdateIntervalHours: 24,
 		UpstreamTimeout:            timeutil.Duration{Duration: dnsforward.DefaultTimeout},
-		ResolveClients:             true,
 		UsePrivateRDNS:             true,
 	},
 	TLS: tlsConfigSettings{
@ -209,6 +212,15 @@ var config = &configuration{
 	DHCP: &dhcpd.ServerConfig{
 		LocalDomainName: "lan",
 	},
+	Clients: &clientsConfig{
+		Sources: &clientSourcesConf{
+			WHOIS:     true,
+			ARP:       true,
+			RDNS:      true,
+			DHCP:      true,
+			HostsFile: true,
+		},
+	},
 	logSettings: logSettings{
 		LogCompress:   false,
 		LogLocalTime:  false,
@ -404,9 +416,7 @@ func (c *configuration) write() error {
 		s.WriteDiskConfig(&c)
 		dns := &config.DNS
 		dns.FilteringConfig = c
-		dns.LocalPTRResolvers,
-			dns.ResolveClients,
-			dns.UsePrivateRDNS = s.RDNSSettings()
+		dns.LocalPTRResolvers, config.Clients.Sources.RDNS, dns.UsePrivateRDNS = s.RDNSSettings()
 	}

 	if Context.dhcpServer != nil {
@ -415,7 +425,7 @@ func (c *configuration) write() error {
 		config.DHCP = c
 	}

-	config.Clients = Context.clients.forConfig()
+	config.Clients.Persistent = Context.clients.forConfig()

 	configFile := config.getConfigFilename()
 	log.Debug("Writing YAML file: %s", configFile)
--- a/internal/home/dns.go
+++ b/internal/home/dns.go
@ -136,7 +136,10 @@ func initDNSServer() (err error) {
 	}

 	Context.rdns = NewRDNS(Context.dnsServer, &Context.clients, config.DNS.UsePrivateRDNS)
-	Context.whois = initWHOIS(&Context.clients)
+
+	if !config.Clients.Sources.WHOIS {
+		Context.whois = initWHOIS(&Context.clients)
+	}

 	Context.filters.Init()
 	return nil
@ -153,10 +156,11 @@ func onDNSRequest(pctx *proxy.DNSContext) {
 		return
 	}

-	if config.DNS.ResolveClients && !ip.IsLoopback() {
+	srcs := config.Clients.Sources
+	if srcs.RDNS && !ip.IsLoopback() {
 		Context.rdns.Begin(ip)
 	}
-	if !netutil.IsSpecialPurpose(ip) {
+	if srcs.WHOIS && !netutil.IsSpecialPurpose(ip) {
 		Context.whois.Begin(ip)
 	}
 }
@ -239,7 +243,7 @@ func generateServerConfig() (newConf dnsforward.ServerConfig, err error) {
 	newConf.FilterHandler = applyAdditionalFiltering
 	newConf.GetCustomUpstreamByClient = Context.clients.findUpstreams

-	newConf.ResolveClients = dnsConf.ResolveClients
+	newConf.ResolveClients = config.Clients.Sources.RDNS
 	newConf.UsePrivateRDNS = dnsConf.UsePrivateRDNS
 	newConf.LocalPTRResolvers = dnsConf.LocalPTRResolvers
 	newConf.UpstreamTimeout = dnsConf.UpstreamTimeout.Duration
@ -387,10 +391,11 @@ func startDNSServer() error {
 			continue
 		}

-		if config.DNS.ResolveClients && !ip.IsLoopback() {
+		srcs := config.Clients.Sources
+		if srcs.RDNS && !ip.IsLoopback() {
 			Context.rdns.Begin(ip)
 		}
-		if !netutil.IsSpecialPurpose(ip) {
+		if srcs.WHOIS && !netutil.IsSpecialPurpose(ip) {
 			Context.whois.Begin(ip)
 		}
 	}
--- a/internal/home/home.go
+++ b/internal/home/home.go
@ -173,6 +173,11 @@ func setupContext(args options) {

 			os.Exit(0)
 		}
+
+		if !args.noEtcHosts && config.Clients.Sources.HostsFile {
+			err = setupHostsContainer()
+			fatalOnError(err)
+		}
 	}

 	Context.mux = http.NewServeMux()
@ -285,14 +290,12 @@ func setupConfig(args options) (err error) {
 		ConfName: config.getConfigFilename(),
 	})

-	if !args.noEtcHosts {
-		if err = setupHostsContainer(); err != nil {
-			return err
-		}
+	var arpdb aghnet.ARPDB
+	if config.Clients.Sources.ARP {
+		arpdb = aghnet.NewARPDB()
 	}

-	arpdb := aghnet.NewARPDB()
-	Context.clients.Init(config.Clients, Context.dhcpServer, Context.etcHosts, arpdb)
+	Context.clients.Init(config.Clients.Persistent, Context.dhcpServer, Context.etcHosts, arpdb)

 	if args.bindPort != 0 {
 		uc := aghalg.UniqChecker{}
--- a/internal/home/options.go
+++ b/internal/home/options.go
@ -230,13 +230,19 @@ var helpArg = arg{
 }

 var noEtcHostsArg = arg{
-	description:     "Do not use the OS-provided hosts.",
+	description:     "Deprecated.  Do not use the OS-provided hosts.",
 	longName:        "no-etc-hosts",
 	shortName:       "",
 	updateWithValue: nil,
 	updateNoValue:   func(o options) (options, error) { o.noEtcHosts = true; return o, nil },
-	effect:          nil,
-	serialize:       func(o options) []string { return boolSliceOrNil(o.noEtcHosts) },
+	effect: func(_ options, _ string) (f effect, err error) {
+		log.Info(
+			"warning: --no-etc-hosts flag is deprecated and will be removed in the future versions",
+		)
+
+		return nil, nil
+	},
+	serialize: func(o options) []string { return boolSliceOrNil(o.noEtcHosts) },
 }

 var localFrontendArg = arg{
--- a/internal/home/upgrade.go
+++ b/internal/home/upgrade.go
@ -21,9 +21,11 @@ import (
 )

 // currentSchemaVersion is the current schema version.
-const currentSchemaVersion = 13
+const currentSchemaVersion = 14

 // These aliases are provided for convenience.
+//
+// TODO(e.burkov):  Remove any after updating to Go 1.18.
 type (
 	any  = interface{}
 	yarr = []any
@ -86,6 +88,7 @@ func upgradeConfigSchema(oldVersion int, diskConf yobj) (err error) {
 		upgradeSchema10to11,
 		upgradeSchema11to12,
 		upgradeSchema12to13,
+		upgradeSchema13to14,
 	}

 	n := 0
@ -726,7 +729,7 @@ func upgradeSchema12to13(diskConf yobj) (err error) {
 	var dhcp yobj
 	dhcp, ok = dhcpVal.(yobj)
 	if !ok {
-		return fmt.Errorf("unexpected type of dhcp: %T", dnsVal)
+		return fmt.Errorf("unexpected type of dhcp: %T", dhcpVal)
 	}

 	const field = "local_domain_name"
@ -737,6 +740,68 @@ func upgradeSchema12to13(diskConf yobj) (err error) {
 	return nil
 }

+// upgradeSchema13to14 performs the following changes:
+//
+//   # BEFORE:
+//   'clients':
+//   - 'name': 'client-name'
+//     # …
+//
+//   # AFTER:
+//   'clients':
+//     'persistent':
+//     - 'name': 'client-name'
+//       # …
+//     'runtime_sources':
+//       'whois': true
+//       'arp': true
+//       'rdns': true
+//       'dhcp': true
+//       'hosts': true
+//
+func upgradeSchema13to14(diskConf yobj) (err error) {
+	log.Printf("Upgrade yaml: 13 to 14")
+	diskConf["schema_version"] = 14
+
+	clientsVal, ok := diskConf["clients"]
+	if !ok {
+		clientsVal = yarr{}
+	}
+
+	var rdnsSrc bool
+	if dnsVal, dok := diskConf["dns"]; dok {
+		var dnsSettings yobj
+		dnsSettings, ok = dnsVal.(yobj)
+		if !ok {
+			return fmt.Errorf("unexpected type of dns: %T", dnsVal)
+		}
+
+		var rdnsSrcVal any
+		rdnsSrcVal, ok = dnsSettings["resolve_clients"]
+		if ok {
+			rdnsSrc, ok = rdnsSrcVal.(bool)
+			if !ok {
+				return fmt.Errorf("unexpected type of resolve_clients: %T", rdnsSrcVal)
+			}
+
+			delete(dnsSettings, "resolve_clients")
+		}
+	}
+
+	diskConf["clients"] = yobj{
+		"persistent": clientsVal,
+		"runtime_sources": &clientSourcesConf{
+			WHOIS:     true,
+			ARP:       true,
+			RDNS:      rdnsSrc,
+			DHCP:      true,
+			HostsFile: true,
+		},
+	}
+
+	return nil
+}
+
 // TODO(a.garipov): Replace with log.Output when we port it to our logging
 // package.
 func funcName() string {
--- a/internal/home/upgrade_test.go
+++ b/internal/home/upgrade_test.go
@ -513,46 +513,129 @@ func TestUpgradeSchema11to12(t *testing.T) {
 }

 func TestUpgradeSchema12to13(t *testing.T) {
-	t.Run("no_dns", func(t *testing.T) {
-		conf := yobj{}
+	const newSchemaVer = 13

-		err := upgradeSchema12to13(conf)
-		require.NoError(t, err)
-
-		assert.Equal(t, conf["schema_version"], 13)
-	})
-
-	t.Run("no_dhcp", func(t *testing.T) {
-		conf := yobj{
-			"dns": yobj{},
-		}
-
-		err := upgradeSchema12to13(conf)
-		require.NoError(t, err)
-
-		assert.Equal(t, conf["schema_version"], 13)
-	})
-
-	t.Run("good", func(t *testing.T) {
-		conf := yobj{
+	testCases := []struct {
+		in   yobj
+		want yobj
+		name string
+	}{{
+		in:   yobj{},
+		want: yobj{"schema_version": newSchemaVer},
+		name: "no_dns",
+	}, {
+		in: yobj{"dns": yobj{}},
+		want: yobj{
+			"dns":            yobj{},
+			"schema_version": newSchemaVer,
+		},
+		name: "no_dhcp",
+	}, {
+		in: yobj{
 			"dns": yobj{
 				"local_domain_name": "lan",
 			},
 			"dhcp":           yobj{},
-			"schema_version": 12,
-		}
-
-		wantConf := yobj{
+			"schema_version": newSchemaVer - 1,
+		},
+		want: yobj{
 			"dns": yobj{},
 			"dhcp": yobj{
 				"local_domain_name": "lan",
 			},
-			"schema_version": 13,
-		}
+			"schema_version": newSchemaVer,
+		},
+		name: "good",
+	}}

-		err := upgradeSchema12to13(conf)
-		require.NoError(t, err)
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := upgradeSchema12to13(tc.in)
+			require.NoError(t, err)

-		assert.Equal(t, wantConf, conf)
-	})
+			assert.Equal(t, tc.want, tc.in)
+		})
+	}
+}
+
+func TestUpgradeSchema13to14(t *testing.T) {
+	const newSchemaVer = 14
+
+	testClient := &clientObject{
+		Name:              "agh-client",
+		IDs:               []string{"id1"},
+		UseGlobalSettings: true,
+	}
+
+	testCases := []struct {
+		in   yobj
+		want yobj
+		name string
+	}{{
+		in: yobj{},
+		want: yobj{
+			"schema_version": newSchemaVer,
+			// The clients field will be added anyway.
+			"clients": yobj{
+				"persistent": yarr{},
+				"runtime_sources": &clientSourcesConf{
+					WHOIS:     true,
+					ARP:       true,
+					RDNS:      false,
+					DHCP:      true,
+					HostsFile: true,
+				},
+			},
+		},
+		name: "no_clients",
+	}, {
+		in: yobj{
+			"clients": []*clientObject{testClient},
+		},
+		want: yobj{
+			"schema_version": newSchemaVer,
+			"clients": yobj{
+				"persistent": []*clientObject{testClient},
+				"runtime_sources": &clientSourcesConf{
+					WHOIS:     true,
+					ARP:       true,
+					RDNS:      false,
+					DHCP:      true,
+					HostsFile: true,
+				},
+			},
+		},
+		name: "no_dns",
+	}, {
+		in: yobj{
+			"clients": []*clientObject{testClient},
+			"dns": yobj{
+				"resolve_clients": true,
+			},
+		},
+		want: yobj{
+			"schema_version": newSchemaVer,
+			"clients": yobj{
+				"persistent": []*clientObject{testClient},
+				"runtime_sources": &clientSourcesConf{
+					WHOIS:     true,
+					ARP:       true,
+					RDNS:      true,
+					DHCP:      true,
+					HostsFile: true,
+				},
+			},
+			"dns": yobj{},
+		},
+		name: "good",
+	}}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := upgradeSchema13to14(tc.in)
+			require.NoError(t, err)
+
+			assert.Equal(t, tc.want, tc.in)
+		})
+	}
 }