Remove service1 / service2 names in consul (#7266)

These names corresponded to single instances of a service, and were primarily used for (a) specifying which interface to bind a gRPC port on and (b) allowing `health-checker` to check individual instances rather than a service as a whole. For (a), change the `--grpc-addr` flags to bind to "all interfaces." For (b), provide a specific IP address and port for health checking. This required adding a `--hostOverride` flag for `health-checker` because the service certificates contain hostname SANs, not IP address SANs. Clarify the situation with nonce services a little bit. Previously we had one nonce "service" in Consul and got nonces from that (i.e. randomly between the two nonce-service instances). Now we have two nonce services in consul, representing multiple datacenters, and one of them is explicitly configured as the "get" service, while both are configured as the "redeem" service. Part of #7245. Note this change does not yet get rid of the rednet/bluenet distinction, nor does it get rid of all use of 10.88.88.88. That will be a followup change.
letsencrypt · Jan 22, 2024 · ce5632b · ce5632b
1 parent f54292e
commit ce5632b
Show file tree

Hide file tree

Showing 9 changed files with 109 additions and 171 deletions.
diff --git a/cmd/nonce-service/main.go b/cmd/nonce-service/main.go
@@ -52,17 +52,23 @@ func derivePrefix(key string, grpcAddr string) (string, error) {
 	if err != nil {
 		return "", fmt.Errorf("parsing gRPC listen address: %w", err)
 	}
+	if host == "" {
+		return "", fmt.Errorf("nonce service gRPC address must include an IP address: got %q", grpcAddr)
+	}
 	if host != "" && port != "" {
 		hostIP := net.ParseIP(host)
 		if hostIP == nil {
-			return "", fmt.Errorf("parsing IP from gRPC listen address: %w", err)
+			return "", fmt.Errorf("gRPC address host part was not an IP address")
+		}
+		if hostIP.IsUnspecified() {
+			return "", fmt.Errorf("nonce service gRPC address must be a specific IP address: got %q", grpcAddr)
 		}
 	}
 	return nonce.DerivePrefix(grpcAddr, key), nil
 }
 
 func main() {
-	grpcAddr := flag.String("addr", "", "gRPC listen address override")
+	grpcAddr := flag.String("addr", "", "gRPC listen address override. Also used to derive the nonce prefix.")
 	debugAddr := flag.String("debug-addr", "", "Debug server address override")
 	configFile := flag.String("config", "", "File path to the configuration file for this service")
 	flag.Parse()

diff --git a/test/config-next/wfe2.json b/test/config-next/wfe2.json
@@ -46,7 +46,7 @@
 		"getNonceService": {
 			"dnsAuthority": "consul.service.consul",
 			"srvLookup": {
-				"service": "nonce",
+				"service": "nonce-taro",
 				"domain": "service.consul"
 			},
 			"timeout": "15s",
@@ -57,11 +57,11 @@
 			"dnsAuthority": "consul.service.consul",
 			"srvLookups": [
 				{
-					"service": "nonce1",
+					"service": "nonce-taro",
 					"domain": "service.consul"
 				},
 				{
-					"service": "nonce2",
+					"service": "nonce-zinc",
 					"domain": "service.consul"
 				}
 			],

diff --git a/test/config/wfe2.json b/test/config/wfe2.json
@@ -48,7 +48,7 @@
 		"getNonceService": {
 			"dnsAuthority": "consul.service.consul",
 			"srvLookup": {
-				"service": "nonce",
+				"service": "nonce-taro",
 				"domain": "service.consul"
 			},
 			"timeout": "15s",
@@ -59,11 +59,11 @@
 			"dnsAuthority": "consul.service.consul",
 			"srvLookups": [
 				{
-					"service": "nonce1",
+					"service": "nonce-taro",
 					"domain": "service.consul"
 				},
 				{
-					"service": "nonce2",
+					"service": "nonce-zinc",
 					"domain": "service.consul"
 				}
 			],

diff --git a/test/consul/config.hcl b/test/consul/config.hcl
@@ -61,22 +61,6 @@ services {
   tags    = ["tcp"] // Required for SRV RR support in gRPC DNS resolution.
 }
 
-services {
-  id      = "ca1"
-  name    = "ca1"
-  address = "10.77.77.77"
-  port    = 9393
-  tags    = ["tcp"] // Required for SRV RR support in gRPC DNS resolution.
-}
-
-services {
-  id      = "ca2"
-  name    = "ca2"
-  address = "10.88.88.88"
-  port    = 9493
-  tags    = ["tcp"] // Required for SRV RR support in gRPC DNS resolution.
-}
-
 services {
   id      = "crl-storer-a"
   name    = "crl-storer"
@@ -117,34 +101,29 @@ services {
   tags    = ["tcp"]
 }
 
+# Unlike most components, we have two completely independent nonce services,
+# simulating two sets of nonce servers running in two different datacenters:
+# taro and zinc.
 services {
-  id      = "nonce-a"
-  name    = "nonce"
+  id      = "nonce-taro-a"
+  name    = "nonce-taro"
   address = "10.77.77.77"
   port    = 9301
   tags    = ["tcp"] // Required for SRV RR support in gRPC DNS resolution.
 }
 
 services {
-  id      = "nonce-b"
-  name    = "nonce"
-  address = "10.88.88.88"
-  port    = 9401
-  tags    = ["tcp"] // Required for SRV RR support in gRPC DNS resolution.
-}
-
-services {
-  id      = "nonce1"
-  name    = "nonce1"
+  id      = "nonce-taro-b"
+  name    = "nonce-taro"
   address = "10.77.77.77"
-  port    = 9301
+  port    = 9501
   tags    = ["tcp"] // Required for SRV RR support in gRPC DNS resolution.
 }
 
 services {
-  id      = "nonce2"
-  name    = "nonce2"
-  address = "10.88.88.88"
+  id      = "nonce-zinc"
+  name    = "nonce-zinc"
+  address = "10.77.77.77"
   port    = 9401
   tags    = ["tcp"] // Required for SRV RR support in gRPC DNS resolution.
 }
@@ -165,22 +144,6 @@ services {
   tags    = ["tcp"] // Required for SRV RR support in gRPC DNS resolution.
 }
 
-services {
-  id      = "publisher1"
-  name    = "publisher1"
-  address = "10.77.77.77"
-  port    = 9391
-  tags    = ["tcp"] // Required for SRV RR support in gRPC DNS resolution.
-}
-
-services {
-  id      = "publisher2"
-  name    = "publisher2"
-  address = "10.88.88.88"
-  port    = 9491
-  tags    = ["tcp"] // Required for SRV RR support in gRPC DNS resolution.
-}
-
 services {
   id      = "ra-a"
   name    = "ra"
@@ -197,22 +160,6 @@ services {
   tags    = ["tcp"] // Required for SRV RR support in gRPC DNS resolution.
 }
 
-services {
-  id      = "ra1"
-  name    = "ra1"
-  address = "10.77.77.77"
-  port    = 9394
-  tags    = ["tcp"] // Required for SRV RR support in gRPC DNS resolution.
-}
-
-services {
-  id      = "ra2"
-  name    = "ra2"
-  address = "10.88.88.88"
-  port    = 9494
-  tags    = ["tcp"] // Required for SRV RR support in gRPC DNS resolution.
-}
-
 services {
   id      = "rva1-a"
   name    = "rva1"
@@ -303,22 +250,6 @@ services {
   ]
 }
 
-services {
-  id      = "sa1"
-  name    = "sa1"
-  address = "10.77.77.77"
-  port    = 9395
-  tags    = ["tcp"] // Required for SRV RR support in gRPC DNS resolution.
-}
-
-services {
-  id      = "sa2"
-  name    = "sa2"
-  address = "10.88.88.88"
-  port    = 9495
-  tags    = ["tcp"] // Required for SRV RR support in gRPC DNS resolution.
-}
-
 services {
   id      = "va-a"
   name    = "va"
@@ -335,22 +266,6 @@ services {
   tags    = ["tcp"] // Required for SRV RR support in gRPC DNS resolution.
 }
 
-services {
-  id      = "va1"
-  name    = "va1"
-  address = "10.77.77.77"
-  port    = 9392
-  tags    = ["tcp"] // Required for SRV RR support in gRPC DNS resolution.
-}
-
-services {
-  id      = "va2"
-  name    = "va2"
-  address = "10.88.88.88"
-  port    = 9492
-  tags    = ["tcp"] // Required for SRV RR support in gRPC DNS resolution.
-}
-
 services {
   id      = "bredis3"
   name    = "redisratelimits"
@@ -395,7 +310,7 @@ services {
 services {
   id      = "case1b"
   name    = "case1b"
-  address = "10.88.88.88"
+  address = "10.77.77.77"
   port    = 9401
   tags    = ["tcp"] // Required for SRV RR support in gRPC DNS resolution.
 }
@@ -405,7 +320,7 @@ services {
 services {
   id      = "case2b"
   name    = "case2b"
-  address = "10.88.88.88"
+  address = "10.77.77.77"
   port    = 9401
   tags    = ["tcp"] // Required for SRV RR support in gRPC DNS resolution.
 }
@@ -436,7 +351,7 @@ services {
 services {
   id      = "case4b"
   name    = "case4b"
-  address = "10.88.88.88"
+  address = "10.77.77.77"
   port    = 9401
   tags    = ["tcp"] // Required for SRV RR support in gRPC DNS resolution.
   checks = [

diff --git a/test/health-checker/main.go b/test/health-checker/main.go
@@ -26,6 +26,7 @@ func main() {
 	// Flag and config parsing and validation.
 	configFile := flag.String("config", "", "Path to the TLS configuration file")
 	serverAddr := flag.String("addr", "", "Address of the gRPC server to check")
+	hostOverride := flag.String("host-override", "", "Hostname to use for TLS certificate validation")
 	flag.Parse()
 	if *configFile == "" {
 		flag.Usage()
@@ -47,6 +48,10 @@ func main() {
 	tlsConfig, err := c.TLS.Load(metrics.NoopRegisterer)
 	cmd.FailOnError(err, "failed to load TLS credentials")
 
+	if *hostOverride != "" {
+		c.GRPC.HostOverride = *hostOverride
+	}
+
 	// GRPC connection prerequisites.
 	clk := cmd.Clock()
 
@@ -58,12 +63,12 @@ func main() {
 	for {
 		select {
 		case <-ticker.C:
-			fmt.Fprintf(os.Stderr, "Connecting to %s health service\n", *serverAddr)
 			_, hostOverride, err := c.GRPC.MakeTargetAndHostOverride()
 			cmd.FailOnError(err, "")
 
 			// Set the hostOverride to match the dNSName in the server certificate.
 			c.GRPC.HostOverride = strings.Replace(hostOverride, ".service.consul", ".boulder", 1)
+			fmt.Fprintf(os.Stderr, "health checking %s (%s)\n", c.GRPC.HostOverride, *serverAddr)
 
 			// Set up the GRPC connection.
 			conn, err := bgrpc.ClientSetup(c.GRPC, tlsConfig, metrics.NoopRegisterer, clk)
@@ -79,9 +84,9 @@ func main() {
 			resp, err := client.Check(ctx2, req)
 			if err != nil {
 				if strings.Contains(err.Error(), "authentication handshake failed") {
-					cmd.Fail(fmt.Sprintf("error connecting to health service %s: %s\n", *serverAddr, err))
+					cmd.Fail(fmt.Sprintf("health checking %s (%s): %s\n", c.GRPC.HostOverride, *serverAddr, err))
 				}
-				fmt.Fprintf(os.Stderr, "got error connecting to health service %s: %s\n", *serverAddr, err)
+				fmt.Fprintf(os.Stderr, "health checking %s (%s): %s\n", c.GRPC.HostOverride, *serverAddr, err)
 			} else if resp.Status == healthpb.HealthCheckResponse_SERVING {
 				return
 			} else {

diff --git a/test/helpers.py b/test/helpers.py
@@ -177,8 +177,9 @@ def waitport(port, prog, perTickCheck=None):
                 raise
     raise(Exception("timed out waiting for debug port %d (%s)" % (port, prog)))
 
-def waithealth(prog, addr):
+def waithealth(prog, port, host_override):
     subprocess.check_call([
         './bin/health-checker',
-        '-addr', addr,
+        '-addr', ("localhost:%d" % (port)),
+        '-host-override', host_override,
         '-config', os.path.join(config_dir, 'health-checker.json')])
diff --git a/test/integration-test.py b/test/integration-test.py
@@ -125,16 +125,16 @@ def check_balance():
     metric is present, which means that backend handled at least one request.
     """
     addresses = [
-        "sa1.service.consul:8003",
-        "sa2.service.consul:8103",
-        "publisher1.service.consul:8009",
-        "publisher2.service.consul:8109",
-        "va1.service.consul:8004",
-        "va2.service.consul:8104",
-        "ca1.service.consul:8001",
-        "ca2.service.consul:8101",
-        "ra1.service.consul:8002",
-        "ra2.service.consul:8102",
+        "localhost:8003", # SA
+        "localhost:8103", # SA
+        "localhost:8009", # publisher
+        "localhost:8109", # publisher
+        "localhost:8004", # VA
+        "localhost:8104", # VA
+        "localhost:8001", # CA
+        "localhost:8101", # CA
+        "localhost:8002", # RA
+        "localhost:8102", # RA
     ]
     for address in addresses:
         metrics = requests.get("http://%s/metrics" % address)

diff --git a/test/integration/testdata/nonce-client.json b/test/integration/testdata/nonce-client.json
@@ -8,7 +8,7 @@
 		"getNonceService": {
 			"dnsAuthority": "consul.service.consul",
 			"srvLookup": {
-				"service": "nonce",
+				"service": "nonce-taro",
 				"domain": "service.consul"
 			},
 			"timeout": "15s",
@@ -19,11 +19,11 @@
 			"dnsAuthority": "consul.service.consul",
 			"srvLookups": [
 				{
-					"service": "nonce1",
+					"service": "nonce-taro",
 					"domain": "service.consul"
 				},
 				{
-					"service": "nonce2",
+					"service": "nonce-zinc",
 					"domain": "service.consul"
 				}
 			],