Configure names for private listeners (#138)

Before this PR, the user is able to specify a hostname for the public
listeners. However, for the private listeners, we compute an internal
name by default. This is not great, especially if you want to change the
names of the internal listeners (e.g., deploy in different
environments).

This PR changes the way the users can configure the listeners:
1) If the user specify a listener config, it has to specify whether the
   listener is public by setting `is_public` knob explicitly.
2) If `is_public` knob is not set, the listener will be private by
   default.
3) For public listeners, the user is required to specify the `hostname`.
4) For private listeners, `hostname` is optional; if not set, we'll
   create a default name:
   <listener_name>.<region>.serviceweaver.internal; if set, the name
   will be:
   <hostname>.<listener_name>.<region>.serviceweaver.internal

.github/workflows/integration.yml

@@ -67,7 +67,7 @@ jobs:
 
         [gke]
         regions= ["us-west1"]
-        listeners.echo = {public_hostname = "echo.serviceweaver.dev"}
+        listeners.echo = {is_public=true, hostname = "echo.serviceweaver.dev"}
 
         ["github.com/ServiceWeaver/weaver-gke/examples/echo/Echoer"]
         Pattern = "${{env.VERSION}}"

examples/echo/weaver.toml

@@ -9,4 +9,4 @@ listeners.echo = {address = "localhost:9000"}
 
 [gke]
 regions = ["us-west1"]
-listeners.echo = {public_hostname = "echo.example.com"}
+listeners.echo = {is_public=true, hostname = "echo.example.com"}

internal/config/config.proto

@@ -43,12 +43,15 @@ message GKEConfig {
   // Options for the application listeners, keyed by listener name.
   // If a listener isn't specified in the map, default options will be used.
   message ListenerOptions {
-    // Public hostname for the listener. If empty, the listener is assumed
-    // to be private.
+    // Is the listener public, i.e., should it be accessible from outside of
+    // the cloud project?
+    bool is_public = 2;
+
+    // Hostname for the listener. It can be empty, iff the listener is private.
     //
     // Public listeners will be configured to receive ingress traffic; all other
     // listeners will be configured only for VPC-internal access.
-    string public_hostname = 1;
+    string hostname = 1;
   }
   map<string, ListenerOptions> listeners = 4;
 

internal/gke/deploy.go

@@ -29,17 +29,17 @@ import (
 	"text/template"
 	"time"
 
-	artifactregistrypb "cloud.google.com/go/artifactregistry/apiv1beta2/artifactregistrypb"
-	computepb "cloud.google.com/go/compute/apiv1/computepb"
+	"cloud.google.com/go/artifactregistry/apiv1beta2/artifactregistrypb"
+	"cloud.google.com/go/compute/apiv1/computepb"
 	container "cloud.google.com/go/container/apiv1"
 	"cloud.google.com/go/container/apiv1/containerpb"
 	"cloud.google.com/go/iam/apiv1/iampb"
 	privateca "cloud.google.com/go/security/privateca/apiv1"
 	"cloud.google.com/go/security/privateca/apiv1/privatecapb"
 	"github.com/ServiceWeaver/weaver"
 	"github.com/ServiceWeaver/weaver-gke/internal/config"
+	"github.com/ServiceWeaver/weaver-gke/internal/nanny"
 	"github.com/ServiceWeaver/weaver-gke/internal/nanny/controller"
-	"github.com/ServiceWeaver/weaver-gke/internal/nanny/distributor"
 	"github.com/ServiceWeaver/weaver-gke/internal/proto"
 	"github.com/ServiceWeaver/weaver/runtime/bin"
 	"github.com/ServiceWeaver/weaver/runtime/graph"
@@ -293,8 +293,11 @@ achieved in one of two ways:
        "http://{{.ExternalGatewayIP}}" in your DNS configuration for "domain.com".
 
 The applications' private listeners will be accessible from inside the
-project's VPC using the schema:
-  - http://<listener_name>.<region>.serviceweaver.internal
+project's VPC in one of the two ways:
+  1. If you specified a hostname in the config for the listener, using the schema:
+     - http://<hostname>.<listener_name>.<region>.serviceweaver.internal
+  2. Otherwise, using the schema:
+     - http://<listener_name>.<region>.serviceweaver.internal
 
 , where <listener_name> is the name the listener was created with in the
 application (i.e., via a call to Listener()). For these names to
@@ -1578,8 +1581,8 @@ func ensureInternalDNS(ctx context.Context, cluster *ClusterInfo, gatewayIP stri
 	if err := patchDNSZone(ctx, cluster.CloudConfig, patchOptions{}, &dns.ManagedZone{
 		Name: managedDNSZoneName,
 		Description: fmt.Sprintf(
-			"Managed zone for domain %s", distributor.InternalDNSDomain),
-		DnsName:    distributor.InternalDNSDomain + ".",
+			"Managed zone for domain %s", nanny.InternalDNSDomain),
+		DnsName:    nanny.InternalDNSDomain + ".",
 		Visibility: "private",
 		PrivateVisibilityConfig: &dns.ManagedZonePrivateVisibilityConfig{
 			Networks: []*dns.ManagedZonePrivateVisibilityConfigNetwork{
@@ -1593,7 +1596,7 @@ func ensureInternalDNS(ctx context.Context, cluster *ClusterInfo, gatewayIP stri
 	}
 
 	// Add the A record to the managed DNS zone.
-	dnsName := fmt.Sprintf("*.%s.%s.", cluster.Region, distributor.InternalDNSDomain)
+	dnsName := fmt.Sprintf("*.%s.%s.", cluster.Region, nanny.InternalDNSDomain)
 	return patchDNSRecordSet(ctx, cluster.CloudConfig, patchOptions{}, managedDNSZoneName, &dns.ResourceRecordSet{
 		Name:    dnsName,
 		Kind:    "dns#resourceRecordSet",

internal/gke/gke.go

@@ -898,7 +898,6 @@ func updateTrafficRoutes(ctx context.Context, cluster *ClusterInfo, logger *slog
 			errs = append(errs, fmt.Errorf("error deleting obsolete route %s: %w", routeName, err))
 		}
 	}
-
 	return errors.Join(errs...)
 }
 

internal/nanny/controller/controller.go

@@ -28,7 +28,6 @@ import (
 
 	"github.com/ServiceWeaver/weaver-gke/internal/endpoints"
 	"github.com/ServiceWeaver/weaver-gke/internal/nanny"
-	"github.com/ServiceWeaver/weaver-gke/internal/nanny/distributor"
 	"github.com/ServiceWeaver/weaver-gke/internal/store"
 	"github.com/ServiceWeaver/weaver/runtime/profiling"
 	"github.com/ServiceWeaver/weaver/runtime/protomsg"
@@ -462,15 +461,7 @@ func appVersionStateToStatus(app string, state *ControllerState, versionState *A
 		}
 		for _, rs := range d.ReplicaSets {
 			for _, l := range rs.Listeners {
-				var hostname string
-				var public bool
-				if opts := cfg.Listeners[l]; opts != nil && opts.PublicHostname != "" {
-					public = true
-					hostname = opts.PublicHostname
-				} else {
-					public = false
-					hostname = fmt.Sprintf("%s.%s.%s", l, loc, distributor.InternalDNSDomain)
-				}
+				hostname, public := nanny.Hostname(l, loc, cfg)
 				ls := listeners[l]
 				if ls == nil {
 					ls = &ListenerStatus{

internal/nanny/controller/controller_test.go

@@ -1301,7 +1301,8 @@ func newVersion(locations []string, appName, id string, rollout string, listener
 			name := strings.Split(l, ".")[0]
 			v.listeners[l] = &nanny.Listener{Name: name}
 			v.listenerOpts[name] = &config.GKEConfig_ListenerOptions{
-				PublicHostname: l,
+				IsPublic: true,
+				Hostname: l,
 			}
 		}
 	}

internal/nanny/distributor/anneal.go

@@ -376,16 +376,6 @@ func (d *Distributor) getReplicaSets(ctx context.Context, app string) error {
 	return errors.Join(errs...)
 }
 
-// hostname returns the hostname for the given listener, and the boolean value
-// indicating whether the given listener is public.
-func (d *Distributor) hostname(lis string, cfg *config.GKEConfig) (string, bool) {
-	if opts := cfg.Listeners[lis]; opts != nil && opts.PublicHostname != "" {
-		return opts.PublicHostname, true
-	}
-	// Private.
-	return fmt.Sprintf("%s.%s.%s", lis, d.region, InternalDNSDomain), false
-}
-
 // ComputeTrafficAssignments computes traffic assignments across all active
 // versions of the applications and returns the earliest time at which a
 // version's desired traffic fraction will change.
@@ -457,7 +447,7 @@ func (d *Distributor) ComputeTrafficAssignments(ctx context.Context, now time.Ti
 		publicTarget := newTarget()
 		privateTarget := newTarget()
 		for _, lis := range v.state.Listeners {
-			host, public := d.hostname(lis.Name, v.state.Config)
+			host, public := nanny.Hostname(lis.Name, d.region, v.state.Config)
 			if public {
 				publicTarget.Listeners[host] = append(publicTarget.Listeners[host], lis)
 			} else {
@@ -562,7 +552,7 @@ func (d *Distributor) detectAppliedTraffic(ctx context.Context, cadence time.Dur
 			expected := nanny.Fraction(v.Schedule)
 			matches := true
 			for _, lis := range v.Listeners {
-				host, _ := d.hostname(lis.Name, v.Config)
+				host, _ := nanny.Hostname(lis.Name, d.region, v.Config)
 				actual := actualTraffic[appVersionHost{app, v.Config.Deployment.Id, host}]
 				if actual >= (expected - maxDivergence) {
 					// Matches the expected traffic.

internal/nanny/distributor/app_test.go

@@ -100,7 +100,8 @@ func newAppVersion(name string, submissionId int, listeners []string, targetFn .
 		if strings.Contains(l, ".") { // public listener
 			name := strings.Split(l, ".")[0]
 			v.listenerOpts[name] = &config.GKEConfig_ListenerOptions{
-				PublicHostname: l,
+				IsPublic: true,
+				Hostname: l,
 			}
 			l = name
 		}

internal/nanny/distributor/distributor.go

@@ -76,9 +76,6 @@ import (
 // We can reuse the state rather than reading it twice.
 
 const (
-	// Internal domain name for applications' private listeners.
-	InternalDNSDomain = "serviceweaver.internal"
-
 	// URL suffixes for various HTTP endpoints exported by the distributor.
 	distributeURL                  = "/distributor/distribute"
 	cleanupURL                     = "/distributor/cleanup"

internal/nanny/distributor/distributor_test.go

@@ -1429,7 +1429,8 @@ func newVersion(appName, id string, submissionId int, listener ...string) versio
 		if strings.Contains(l, ".") { // public listener
 			name := strings.Split(l, ".")[0]
 			v.listenerOpts[name] = &config.GKEConfig_ListenerOptions{
-				PublicHostname: l,
+				IsPublic: true,
+				Hostname: l,
 			}
 			l = name
 		}

internal/nanny/hostname.go

@@ -0,0 +1,43 @@
+// Copyright 2024 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package nanny
+
+import (
+	"fmt"
+
+	"github.com/ServiceWeaver/weaver-gke/internal/config"
+)
+
+// Internal domain name for applications' private listeners.
+const InternalDNSDomain = "serviceweaver.internal"
+
+// Hostname returns the hostname for the given listener, and the boolean value
+// indicating whether the given listener is public.
+func Hostname(lis string, loc string, cfg *config.GKEConfig) (string, bool) {
+	defaultHostname := fmt.Sprintf("%s.%s.%s", lis, loc, InternalDNSDomain)
+	opts := cfg.Listeners[lis]
+	if opts == nil {
+		// Private listener that doesn't have a hostname specified by the user.
+		return defaultHostname, false
+	}
+	if opts.IsPublic {
+		return opts.Hostname, true
+	}
+	if opts.Hostname != "" {
+		// Private listener that has a hostname specified by the user.
+		return fmt.Sprintf("%s.%s", opts.Hostname, defaultHostname), false
+	}
+	return "", false
+}

internal/tool/deploy.go

@@ -118,7 +118,8 @@ func makeGKEConfig(app *protos.AppConfig) (*config.GKEConfig, error) {
 
 	// Use an intermediate struct, so that we can add TOML tags.
 	type lisOpts struct {
-		PublicHostname string `toml:"public_hostname"`
+		IsPublic bool   `toml:"is_public"`
+		Hostname string `toml:"hostname"`
 	}
 	type gkeConfigSchema struct {
 		Regions     []string
@@ -149,9 +150,12 @@ func makeGKEConfig(app *protos.AppConfig) (*config.GKEConfig, error) {
 		for lis, opts := range parsed.Listeners {
 			if _, ok := allListeners[lis]; !ok {
 				return nil, fmt.Errorf("listener %s specified in the config not found in the binary", lis)
-
 			}
-			listeners[lis] = &config.GKEConfig_ListenerOptions{PublicHostname: opts.PublicHostname}
+			// If the listener is public, a hostname is required.
+			if opts.IsPublic && opts.Hostname == "" {
+				return nil, fmt.Errorf("listener %s is public, but has no hostname specified", lis)
+			}
+			listeners[lis] = &config.GKEConfig_ListenerOptions{IsPublic: opts.IsPublic, Hostname: opts.Hostname}
 		}
 	}
 	if err := validateDeployRegions(parsed.Regions); err != nil {

internal/tool/deploy_test.go

@@ -97,19 +97,39 @@ regions = ["us-central1"]
 			},
 		},
 		{
-			name: "listeners",
+			name: "public-listeners",
 			config: `
 [gke]
-listeners.a = {public_hostname="a.com"}
-listeners.b = {public_hostname="b.com"}
+listeners.a = {is_public=true, hostname="a.com"}
+listeners.b = {is_public=true, hostname="b.com"}
 regions = ["us-central1"]
 `,
 			expect: &config.GKEConfig{
 				Image:       defaultBaseImage,
 				MinReplicas: 1,
 				Listeners: map[string]*config.GKEConfig_ListenerOptions{
-					"a": {PublicHostname: "a.com"},
-					"b": {PublicHostname: "b.com"},
+					"a": {IsPublic: true, Hostname: "a.com"},
+					"b": {IsPublic: true, Hostname: "b.com"},
+				},
+				Regions: []string{"us-central1"},
+			},
+		},
+		{
+			name: "public-and-private-listeners",
+			config: `
+[gke]
+listeners.a = {is_public=true, hostname="a.com"}
+listeners.b = {hostname="b.com"}
+listeners.c = {}
+regions = ["us-central1"]
+`,
+			expect: &config.GKEConfig{
+				Image:       defaultBaseImage,
+				MinReplicas: 1,
+				Listeners: map[string]*config.GKEConfig_ListenerOptions{
+					"a": {IsPublic: true, Hostname: "a.com"},
+					"b": {IsPublic: false, Hostname: "b.com"},
+					"c": {IsPublic: false, Hostname: ""},
 				},
 				Regions: []string{"us-central1"},
 			},
@@ -155,10 +175,20 @@ func TestBadGKEConfig(t *testing.T) {
 			name: "missing_listeners",
 			cfg: `
 [gke]
-listeners.c = {public_hostname="c.com"}
+listeners.d = {hostname="d.com"}
+regions = ["us-central1"]
 `,
 			expectedError: "not found in the binary",
 		},
+		{
+			name: "public_listener_no_hostname",
+			cfg: `
+[gke]
+listeners.c = {is_public=true}
+regions = ["us-central1"]
+`,
+			expectedError: "no hostname specified",
+		},
 	} {
 		t.Run(c.name, func(t *testing.T) {
 			d := t.TempDir()