refactor: embed Upstreams in UpstreamGroup to make values accessible

Move `startVerifyUpstream` to `upstreams.startVerify` so it's accessible
via `UpstreamGroup` and we don't need to pass `startVerify` to all
resolver constructors that call `NewUpstreamResolver`.

Also has the nice benefit of greatly reducing the usage of `GetConfig`.

config/config.go

@@ -189,44 +189,44 @@ func (b *BootstrappedUpstreamConfig) UnmarshalYAML(unmarshal func(interface{}) e
 //
 //nolint:maligned
 type Config struct {
-	Upstreams           Upstreams           `yaml:"upstreams"`
-	ConnectIPVersion    IPVersion           `yaml:"connectIPVersion"`
-	CustomDNS           CustomDNS           `yaml:"customDNS"`
-	Conditional         ConditionalUpstream `yaml:"conditional"`
-	Blocking            Blocking            `yaml:"blocking"`
-	ClientLookup        ClientLookup        `yaml:"clientLookup"`
-	Caching             CachingConfig       `yaml:"caching"`
-	QueryLog            QueryLogConfig      `yaml:"queryLog"`
-	Prometheus          MetricsConfig       `yaml:"prometheus"`
-	Redis               RedisConfig         `yaml:"redis"`
-	Log                 log.Config          `yaml:"log"`
-	Ports               PortsConfig         `yaml:"ports"`
-	DoHUserAgent        string              `yaml:"dohUserAgent"`
-	MinTLSServeVer      string              `yaml:"minTlsServeVersion" default:"1.2"`
-	StartVerifyUpstream bool                `yaml:"startVerifyUpstream" default:"false"`
-	CertFile            string              `yaml:"certFile"`
-	KeyFile             string              `yaml:"keyFile"`
-	BootstrapDNS        BootstrapDNSConfig  `yaml:"bootstrapDns"`
-	HostsFile           HostsFileConfig     `yaml:"hostsFile"`
-	FQDNOnly            FQDNOnly            `yaml:"fqdnOnly"`
-	Filtering           FilteringConfig     `yaml:"filtering"`
-	EDE                 EDE                 `yaml:"ede"`
-	ECS                 ECS                 `yaml:"ecs"`
-	SUDN                SUDN                `yaml:"specialUseDomains"`
+	Upstreams        Upstreams           `yaml:"upstreams"`
+	ConnectIPVersion IPVersion           `yaml:"connectIPVersion"`
+	CustomDNS        CustomDNS           `yaml:"customDNS"`
+	Conditional      ConditionalUpstream `yaml:"conditional"`
+	Blocking         Blocking            `yaml:"blocking"`
+	ClientLookup     ClientLookup        `yaml:"clientLookup"`
+	Caching          CachingConfig       `yaml:"caching"`
+	QueryLog         QueryLogConfig      `yaml:"queryLog"`
+	Prometheus       MetricsConfig       `yaml:"prometheus"`
+	Redis            RedisConfig         `yaml:"redis"`
+	Log              log.Config          `yaml:"log"`
+	Ports            PortsConfig         `yaml:"ports"`
+	DoHUserAgent     string              `yaml:"dohUserAgent"`
+	MinTLSServeVer   string              `yaml:"minTlsServeVersion" default:"1.2"`
+	CertFile         string              `yaml:"certFile"`
+	KeyFile          string              `yaml:"keyFile"`
+	BootstrapDNS     BootstrapDNSConfig  `yaml:"bootstrapDns"`
+	HostsFile        HostsFileConfig     `yaml:"hostsFile"`
+	FQDNOnly         FQDNOnly            `yaml:"fqdnOnly"`
+	Filtering        FilteringConfig     `yaml:"filtering"`
+	EDE              EDE                 `yaml:"ede"`
+	ECS              ECS                 `yaml:"ecs"`
+	SUDN             SUDN                `yaml:"specialUseDomains"`
 
 	// Deprecated options
 	Deprecated struct {
-		Upstream        *UpstreamGroups `yaml:"upstream"`
-		UpstreamTimeout *Duration       `yaml:"upstreamTimeout"`
-		DisableIPv6     *bool           `yaml:"disableIPv6"`
-		LogLevel        *log.Level      `yaml:"logLevel"`
-		LogFormat       *log.FormatType `yaml:"logFormat"`
-		LogPrivacy      *bool           `yaml:"logPrivacy"`
-		LogTimestamp    *bool           `yaml:"logTimestamp"`
-		DNSPorts        *ListenConfig   `yaml:"port"`
-		HTTPPorts       *ListenConfig   `yaml:"httpPort"`
-		HTTPSPorts      *ListenConfig   `yaml:"httpsPort"`
-		TLSPorts        *ListenConfig   `yaml:"tlsPort"`
+		Upstream            *UpstreamGroups `yaml:"upstream"`
+		UpstreamTimeout     *Duration       `yaml:"upstreamTimeout"`
+		DisableIPv6         *bool           `yaml:"disableIPv6"`
+		LogLevel            *log.Level      `yaml:"logLevel"`
+		LogFormat           *log.FormatType `yaml:"logFormat"`
+		LogPrivacy          *bool           `yaml:"logPrivacy"`
+		LogTimestamp        *bool           `yaml:"logTimestamp"`
+		DNSPorts            *ListenConfig   `yaml:"port"`
+		HTTPPorts           *ListenConfig   `yaml:"httpPort"`
+		HTTPSPorts          *ListenConfig   `yaml:"httpsPort"`
+		TLSPorts            *ListenConfig   `yaml:"tlsPort"`
+		StartVerifyUpstream *bool           `yaml:"startVerifyUpstream"`
 	} `yaml:",inline"`
 }
 
@@ -514,14 +514,15 @@ func (cfg *Config) migrate(logger *logrus.Entry) bool {
 				cfg.Filtering.QueryTypes.Insert(dns.Type(dns.TypeAAAA))
 			}
 		}),
-		"port":         Move(To("ports.dns", &cfg.Ports)),
-		"httpPort":     Move(To("ports.http", &cfg.Ports)),
-		"httpsPort":    Move(To("ports.https", &cfg.Ports)),
-		"tlsPort":      Move(To("ports.tls", &cfg.Ports)),
-		"logLevel":     Move(To("log.level", &cfg.Log)),
-		"logFormat":    Move(To("log.format", &cfg.Log)),
-		"logPrivacy":   Move(To("log.privacy", &cfg.Log)),
-		"logTimestamp": Move(To("log.timestamp", &cfg.Log)),
+		"port":                Move(To("ports.dns", &cfg.Ports)),
+		"httpPort":            Move(To("ports.http", &cfg.Ports)),
+		"httpsPort":           Move(To("ports.https", &cfg.Ports)),
+		"tlsPort":             Move(To("ports.tls", &cfg.Ports)),
+		"logLevel":            Move(To("log.level", &cfg.Log)),
+		"logFormat":           Move(To("log.format", &cfg.Log)),
+		"logPrivacy":          Move(To("log.privacy", &cfg.Log)),
+		"logTimestamp":        Move(To("log.timestamp", &cfg.Log)),
+		"startVerifyUpstream": Move(To("upstreams.startVerify", &cfg.Upstreams)),
 	})
 
 	usesDepredOpts = cfg.Blocking.migrate(logger) || usesDepredOpts

config/config_test.go

@@ -770,6 +770,7 @@ bootstrapDns:
 
 func defaultTestFileConfig() {
 	Expect(config.Ports.DNS).Should(Equal(ListenConfig{"55553", ":55554", "[::1]:55555"}))
+	Expect(config.Upstreams.StartVerify).Should(BeFalse())
 	Expect(config.Upstreams.Groups["default"]).Should(HaveLen(3))
 	Expect(config.Upstreams.Groups["default"][0].Host).Should(Equal("8.8.8.8"))
 	Expect(config.Upstreams.Groups["default"][1].Host).Should(Equal("8.8.4.4"))
@@ -798,14 +799,14 @@ func defaultTestFileConfig() {
 
 	Expect(config.DoHUserAgent).Should(Equal("testBlocky"))
 	Expect(config.MinTLSServeVer).Should(Equal("1.3"))
-	Expect(config.StartVerifyUpstream).Should(BeFalse())
 
 	Expect(GetConfig()).Should(Not(BeNil()))
 }
 
 func writeConfigYml(tmpDir *helpertest.TmpFolder) *helpertest.TmpFile {
 	return tmpDir.CreateStringFile("config.yml",
 		"upstreams:",
+		"  startVerify: false",
 		"  groups:",
 		"    default:",
 		"      - tcp+udp:8.8.8.8",
@@ -860,7 +861,7 @@ func writeConfigYml(tmpDir *helpertest.TmpFolder) *helpertest.TmpFile {
 		"logLevel: debug",
 		"dohUserAgent: testBlocky",
 		"minTlsServeVersion: 1.3",
-		"startVerifyUpstream: false")
+	)
 }
 
 func writeConfigDir(tmpDir *helpertest.TmpFolder) error {

config/upstreams.go

@@ -8,9 +8,10 @@ const UpstreamDefaultCfgName = "default"
 
 // Upstreams upstream servers configuration
 type Upstreams struct {
-	Timeout  Duration         `yaml:"timeout" default:"2s"`
-	Groups   UpstreamGroups   `yaml:"groups"`
-	Strategy UpstreamStrategy `yaml:"strategy" default:"parallel_best"`
+	Timeout     Duration         `yaml:"timeout" default:"2s"`
+	Groups      UpstreamGroups   `yaml:"groups"`
+	Strategy    UpstreamStrategy `yaml:"strategy" default:"parallel_best"`
+	StartVerify bool             `yaml:"startVerify" default:"false"`
 }
 
 type UpstreamGroups map[string][]Upstream
@@ -37,21 +38,40 @@ func (c *Upstreams) LogConfig(logger *logrus.Entry) {
 
 // UpstreamGroup represents the config for one group (upstream branch)
 type UpstreamGroup struct {
-	Name      string
-	Upstreams []Upstream
+	Upstreams
+
+	Name string // group name
+}
+
+// NewUpstreamGroup creates an UpstreamGroup with the given name and upstreams.
+//
+// The upstreams from `cfg.Groups` are ignored.
+func NewUpstreamGroup(name string, cfg Upstreams, upstreams []Upstream) UpstreamGroup {
+	group := UpstreamGroup{
+		Name:      name,
+		Upstreams: cfg,
+	}
+
+	group.Groups = UpstreamGroups{name: upstreams}
+
+	return group
+}
+
+func (c *UpstreamGroup) GroupUpstreams() []Upstream {
+	return c.Groups[c.Name]
 }
 
 // IsEnabled implements `config.Configurable`.
 func (c *UpstreamGroup) IsEnabled() bool {
-	return len(c.Upstreams) != 0
+	return len(c.GroupUpstreams()) != 0
 }
 
 // LogConfig implements `config.Configurable`.
 func (c *UpstreamGroup) LogConfig(logger *logrus.Entry) {
 	logger.Info("group: ", c.Name)
 	logger.Info("upstreams:")
 
-	for _, upstream := range c.Upstreams {
+	for _, upstream := range c.GroupUpstreams() {
 		logger.Infof("  - %s", upstream)
 	}
 }

config/upstreams_test.go

@@ -11,7 +11,7 @@ import (
 var _ = Describe("ParallelBestConfig", func() {
 	suiteBeforeEach()
 
-	Context("UpstreamsConfig", func() {
+	Context("Upstreams", func() {
 		var cfg Upstreams
 
 		BeforeEach(func() {
@@ -65,13 +65,13 @@ var _ = Describe("ParallelBestConfig", func() {
 		var cfg UpstreamGroup
 
 		BeforeEach(func() {
-			cfg = UpstreamGroup{
-				Name: UpstreamDefaultCfgName,
-				Upstreams: []Upstream{
-					{Host: "host1"},
-					{Host: "host2"},
-				},
-			}
+			upstreamsCfg, err := WithDefaults[Upstreams]()
+			Expect(err).Should(Succeed())
+
+			cfg = NewUpstreamGroup("test", upstreamsCfg, []Upstream{
+				{Host: "host1"},
+				{Host: "host2"},
+			})
 		})
 
 		Describe("IsEnabled", func() {
@@ -102,7 +102,7 @@ var _ = Describe("ParallelBestConfig", func() {
 				cfg.LogConfig(logger)
 
 				Expect(hook.Calls).ShouldNot(BeEmpty())
-				Expect(hook.Messages).Should(ContainElement(ContainSubstring("group: default")))
+				Expect(hook.Messages).Should(ContainElement(ContainSubstring("group: test")))
 				Expect(hook.Messages).Should(ContainElement(ContainSubstring("upstreams:")))
 				Expect(hook.Messages).Should(ContainElement(ContainSubstring(":host1:")))
 				Expect(hook.Messages).Should(ContainElement(ContainSubstring(":host2:")))

docs/config.yml

@@ -24,9 +24,8 @@ upstreams:
   strategy: parallel_best
   # optional: timeout to query the upstream resolver. Default: 2s
   timeout: 2s
-
-# optional: If true, blocky will fail to start unless at least one upstream server per group is reachable. Default: false
-startVerifyUpstream: true
+  # optional: If true, blocky will fail to start unless at least one upstream server per group is reachable. Default: false
+  startVerify: false
 
 # optional: Determines how blocky will create outgoing connections. This impacts both upstreams, and lists.
 # accepted: dual, v4, v6

docs/configuration.md

@@ -17,7 +17,6 @@ configuration properties as [JSON](config.yml).
 | keyFile             | path                | no        |               | Path to cert and key file for SSL encryption (DoH and DoT); if empty, self-signed certificate is generated |
 | dohUserAgent        | string              | no        |               | HTTP User Agent for DoH upstreams                                                                          |
 | minTlsServeVersion  | string              | no        | 1.2           | Minimum TLS version that the DoT and DoH server use to serve those encrypted DNS requests                  |
-| startVerifyUpstream | bool                | no        | false         | If true, blocky will fail to start unless at least one upstream server per group is reachable.             |
 | connectIPVersion    | enum (dual, v4, v6) | no        | dual          | IP version to use for outgoing connections (dual, v4, v6)                                                  |
 
 !!! example
@@ -70,6 +69,16 @@ All logging options are optional.
 
 ## Upstreams configuration
 
+| Parameter             | Type                                 | Default value | Description                                                                                     |
+| --------------------- | ------------------------------------ | ------------- | ----------------------------------------------------------------------------------------------- |
+| usptreams.groups      | map of name to upstream              | none          | Upstream DNS servers to use, in groups.                                                         |
+| usptreams.startVerify | bool                                 | false         | If true, blocky will fail to start unless at least one upstream server per group is functional. |
+| usptreams.strategy    | enum (parallel_best, random, strict) | parallel_best | Upstream server usage strategy.                                                                 |
+| usptreams.timeout     | bool                                 | true          | Upstream connection timeout.                                                                    |
+
+
+### Upstream Groups
+
 To resolve a DNS query, blocky needs external public or private DNS resolvers. Blocky supports DNS resolvers with
 following network protocols (net part of the resolver URL):
 
@@ -133,6 +142,22 @@ The logic determining what group a client belongs to follows a strict order: IP,
 
 If a client matches multiple client name or CIDR groups, a warning is logged and the first found group is used.
 
+### Upstream connection timeout
+
+Blocky will wait 2 seconds (default value) for the response from the external upstream DNS server. You can change this
+value by setting the `timeout` configuration parameter (in **duration format**).
+
+!!! example
+
+    ```yaml
+    upstreams:
+      timeout: 5s
+      groups:
+        default:
+          - 46.182.19.48
+          - 80.241.218.68
+    ```
+
 ### Upstream strategy
 
 Blocky supports different upstream strategies (default `parallel_best`) that determine how and to which upstream DNS servers requests are forwarded.
@@ -160,21 +185,6 @@ Currently available strategies:
           - 9.8.7.6
     ```
 
-### Upstream lookup timeout
-
-Blocky will wait 2 seconds (default value) for the response from the external upstream DNS server. You can change this
-value by setting the `timeout` configuration parameter (in **duration format**).
-
-!!! example
-
-    ```yaml
-    upstreams:
-      timeout: 5s
-      groups:
-        default:
-          - 46.182.19.48
-          - 80.241.218.68
-    ```
 
 ## Bootstrap DNS configuration
 

e2e/upstream_test.go

@@ -13,8 +13,8 @@ var _ = Describe("Upstream resolver configuration tests", func() {
 	var blocky testcontainers.Container
 	var err error
 
-	Describe("'startVerifyUpstream' parameter handling", func() {
-		When("'startVerifyUpstream' is false and upstream server as IP is not reachable", func() {
+	Describe("'upstreams.startVerify' parameter handling", func() {
+		When("'upstreams.startVerify' is false and upstream server as IP is not reachable", func() {
 			BeforeEach(func() {
 				blocky, err = createBlockyContainer(tmpDir,
 					"log:",
@@ -23,7 +23,7 @@ var _ = Describe("Upstream resolver configuration tests", func() {
 					"  groups:",
 					"    default:",
 					"      - 192.192.192.192",
-					"startVerifyUpstream: false",
+					"  startVerify: false",
 				)
 
 				Expect(err).Should(Succeed())
@@ -34,7 +34,7 @@ var _ = Describe("Upstream resolver configuration tests", func() {
 				Expect(getContainerLogs(blocky)).Should(BeEmpty())
 			})
 		})
-		When("'startVerifyUpstream' is false and upstream server as host name is not reachable", func() {
+		When("'upstreams.startVerify' is false and upstream server as host name is not reachable", func() {
 			BeforeEach(func() {
 				blocky, err = createBlockyContainer(tmpDir,
 					"log:",
@@ -43,7 +43,7 @@ var _ = Describe("Upstream resolver configuration tests", func() {
 					"  groups:",
 					"    default:",
 					"      - some.wrong.host",
-					"startVerifyUpstream: false",
+					"  startVerify: false",
 				)
 
 				Expect(err).Should(Succeed())
@@ -54,14 +54,14 @@ var _ = Describe("Upstream resolver configuration tests", func() {
 				Expect(getContainerLogs(blocky)).Should(BeEmpty())
 			})
 		})
-		When("'startVerifyUpstream' is true and upstream as IP address server is not reachable", func() {
+		When("'upstreams.startVerify' is true and upstream as IP address server is not reachable", func() {
 			BeforeEach(func() {
 				blocky, err = createBlockyContainer(tmpDir,
 					"upstreams:",
 					"  groups:",
 					"    default:",
 					"      - 192.192.192.192",
-					"startVerifyUpstream: true",
+					"  startVerify: true",
 				)
 
 				Expect(err).Should(HaveOccurred())
@@ -75,14 +75,14 @@ var _ = Describe("Upstream resolver configuration tests", func() {
 						ContainSubstring("no valid upstream for group default")))
 			})
 		})
-		When("'startVerifyUpstream' is true and upstream server as host name is not reachable", func() {
+		When("'upstreams.startVerify' is true and upstream server as host name is not reachable", func() {
 			BeforeEach(func() {
 				blocky, err = createBlockyContainer(tmpDir,
 					"upstreams:",
 					"  groups:",
 					"    default:",
 					"      - some.wrong.host",
-					"startVerifyUpstream: true",
+					"  startVerify: true",
 				)
 
 				Expect(err).Should(HaveOccurred())