cmd/containerboot: Ingress proxies now only advertise an HTTPS endpoint when it's ready

L7 Ingress proxies now set a new https_endpoint field to their state Secret when
the serve config has been loaded. This gets unset if serve config can not be set.
L7 Ingress proxies now attempt to determine if HTTPS is disabled for the tailnet (by looking
at cert domains in netmap) and log an error.

Signed-off-by: Irbe Krumina <irbe@tailscale.com>

cmd/containerboot/kube.go

@@ -74,7 +74,19 @@ func (kc *kubeClient) storeDeviceEndpoints(ctx context.Context, fqdn string, add
 			kubetypes.KeyDeviceIPs:  deviceIPs,
 		},
 	}
-	return kc.StrategicMergePatchSecret(ctx, secretName, s, "tailscale-container")
+	return kc.StrategicMergePatchSecret(ctx, kc.stateSecret, s, "tailscale-container")
+}
+
+// storeHTTPSEndpoint writes an HTTPS endpoint exposed by this device via 'tailscale serve' to the named Kubernetes
+// Secret. In practice this will be the same value that gets written to 'device_fqdn', but this should only be called
+// when the serve config has been successfully set up.
+func (kc *kubeClient) storeHTTPSEndpoint(ctx context.Context, ep string) error {
+	s := &kubeapi.Secret{
+		Data: map[string][]byte{
+			kubetypes.KeyHTTPSEndpoint: []byte(ep),
+		},
+	}
+	return kc.StrategicMergePatchSecret(ctx, kc.stateSecret, s, "tailscale-container")
 }
 
 // deleteAuthKey deletes the 'authkey' field of the given kube

cmd/containerboot/main.go

@@ -289,12 +289,16 @@ func main() {
 		}
 	}
 
+	// Remove any serve config and advertised HTTPS endpoint that may have been set by a previous run of
+	// containerboot, but only if we're providing a new one.
 	if cfg.ServeConfigPath != "" {
-		// Remove any serve config that may have been set by a previous run of
+		log.Printf("serve proxy: unsetting previous config")
-		// containerboot, but only if we're providing a new one.
 		if err := client.SetServeConfig(ctx, new(ipn.ServeConfig)); err != nil {
 			log.Fatalf("failed to unset serve config: %v", err)
 		}
+		if err := kc.storeHTTPSEndpoint(ctx, ""); err != nil {
+			log.Fatalf("failed to update HTTPS endpoint in tailscale state: %v", err)
+		}
 	}
 
 	if hasKubeStateStore(cfg) && isTwoStepConfigAuthOnce(cfg) {
@@ -334,10 +338,12 @@ func main() {
 
 		h             = &healthz{} // http server for the healthz endpoint
 		healthzRunner = sync.OnceFunc(func() { runHealthz(cfg.HealthCheckAddrPort, h) })
+		// triggerWatchServeConfigChanges = sync.OnceFunc(func() {
+		// 	go watchServeConfigChanges(ctx, cfg.ServeConfigPath, certDomainChanged, certDomain, client, kc)
+		// })
+		triggerWatchServeConfigChanges sync.Once
 	)
-	if cfg.ServeConfigPath != "" {
+
-		go watchServeConfigChanges(ctx, cfg.ServeConfigPath, certDomainChanged, certDomain, client)
-	}
 	var nfr linuxfw.NetfilterRunner
 	if isL3Proxy(cfg) {
 		nfr, err = newNetfilterRunner(log.Printf)
@@ -511,8 +517,11 @@ func main() {
 					resetTimer(false)
 					backendAddrs = newBackendAddrs
 				}
-				if cfg.ServeConfigPath != "" && len(n.NetMap.DNS.CertDomains) != 0 {
+				if cfg.ServeConfigPath != "" {
-					cd := n.NetMap.DNS.CertDomains[0]
+					cd := certDomainFromNetmap(n.NetMap)
+					if cd == "" {
+						cd = kubetypes.ValueNoHTTPS
+					}
 					prev := certDomain.Swap(ptr.To(cd))
 					if prev == nil || *prev != cd {
 						select {
@@ -559,6 +568,12 @@ func main() {
 					}
 				}
 
+				if cfg.ServeConfigPath != "" {
+					triggerWatchServeConfigChanges.Do(func() {
+						go watchServeConfigChanges(ctx, cfg.ServeConfigPath, certDomainChanged, certDomain, client, kc)
+					})
+				}
+
 				if cfg.HealthCheckAddrPort != "" {
 					h.Lock()
 					h.hasAddrs = len(addrs) != 0

cmd/containerboot/main_test.go

@@ -102,6 +102,8 @@ func TestContainerBoot(t *testing.T) {
 	argFile := filepath.Join(d, "args")
 	runningSockPath := filepath.Join(d, "tmp/tailscaled.sock")
 
+	capver := fmt.Sprintf("%d", tailcfg.CurrentCapabilityVersion)
+
 	type phase struct {
 		// If non-nil, send this IPN bus notification (and remember it as the
 		// initial update for any future new watchers, then wait for all the
@@ -453,10 +455,11 @@ type phase struct {
 				{
 					Notify: runningNotify,
 					WantKubeSecret: map[string]string{
-						"authkey":     "tskey-key",
+						"authkey":          "tskey-key",
-						"device_fqdn": "test-node.test.ts.net",
+						"device_fqdn":      "test-node.test.ts.net",
-						"device_id":   "myID",
+						"device_id":        "myID",
-						"device_ips":  `["100.64.0.1"]`,
+						"device_ips":       `["100.64.0.1"]`,
+						"tailscale_capver": capver,
 					},
 				},
 			},
@@ -546,9 +549,10 @@ type phase struct {
 						"/usr/bin/tailscale --socket=/tmp/tailscaled.sock set --accept-dns=false",
 					},
 					WantKubeSecret: map[string]string{
-						"device_fqdn": "test-node.test.ts.net",
+						"device_fqdn":      "test-node.test.ts.net",
-						"device_id":   "myID",
+						"device_id":        "myID",
-						"device_ips":  `["100.64.0.1"]`,
+						"device_ips":       `["100.64.0.1"]`,
+						"tailscale_capver": capver,
 					},
 				},
 			},
@@ -575,10 +579,11 @@ type phase struct {
 				{
 					Notify: runningNotify,
 					WantKubeSecret: map[string]string{
-						"authkey":     "tskey-key",
+						"authkey":          "tskey-key",
-						"device_fqdn": "test-node.test.ts.net",
+						"device_fqdn":      "test-node.test.ts.net",
-						"device_id":   "myID",
+						"device_id":        "myID",
-						"device_ips":  `["100.64.0.1"]`,
+						"device_ips":       `["100.64.0.1"]`,
+						"tailscale_capver": capver,
 					},
 				},
 				{
@@ -593,10 +598,11 @@ type phase struct {
 						},
 					},
 					WantKubeSecret: map[string]string{
-						"authkey":     "tskey-key",
+						"authkey":          "tskey-key",
-						"device_fqdn": "new-name.test.ts.net",
+						"device_fqdn":      "new-name.test.ts.net",
-						"device_id":   "newID",
+						"device_id":        "newID",
-						"device_ips":  `["100.64.0.1"]`,
+						"device_ips":       `["100.64.0.1"]`,
+						"tailscale_capver": capver,
 					},
 				},
 			},

cmd/containerboot/serve.go

@@ -19,6 +19,8 @@
 	"github.com/fsnotify/fsnotify"
 	"tailscale.com/client/tailscale"
 	"tailscale.com/ipn"
+	"tailscale.com/kube/kubetypes"
+	"tailscale.com/types/netmap"
 )
 
 // watchServeConfigChanges watches path for changes, and when it sees one, reads
@@ -26,21 +28,21 @@
 // applies it to lc. It exits when ctx is canceled. cdChanged is a channel that
 // is written to when the certDomain changes, causing the serve config to be
 // re-read and applied.
-func watchServeConfigChanges(ctx context.Context, path string, cdChanged <-chan bool, certDomainAtomic *atomic.Pointer[string], lc *tailscale.LocalClient) {
+func watchServeConfigChanges(ctx context.Context, path string, cdChanged <-chan bool, certDomainAtomic *atomic.Pointer[string], lc *tailscale.LocalClient, kc *kubeClient) {
 	if certDomainAtomic == nil {
-		panic("cd must not be nil")
+		panic("certDomainAtomic must not be nil")
 	}
 	var tickChan <-chan time.Time
 	var eventChan <-chan fsnotify.Event
 	if w, err := fsnotify.NewWatcher(); err != nil {
-		log.Printf("failed to create fsnotify watcher, timer-only mode: %v", err)
+		log.Printf("serve proxy: failed to create fsnotify watcher, timer-only mode: %v", err)
 		ticker := time.NewTicker(5 * time.Second)
 		defer ticker.Stop()
 		tickChan = ticker.C
 	} else {
 		defer w.Close()
 		if err := w.Add(filepath.Dir(path)); err != nil {
-			log.Fatalf("failed to add fsnotify watch: %v", err)
+			log.Fatalf("serve proxy: failed to add fsnotify watch: %v", err)
 		}
 		eventChan = w.Events
 	}
@@ -59,24 +61,62 @@ func watchServeConfigChanges(ctx context.Context, path string, cdChanged <-chan
 			// k8s handles these mounts. So just re-read the file and apply it
 			// if it's changed.
 		}
-		if certDomain == "" {
-			continue
-		}
 		sc, err := readServeConfig(path, certDomain)
 		if err != nil {
-			log.Fatalf("failed to read serve config: %v", err)
+			log.Fatalf("serve proxy: failed to read serve config: %v", err)
 		}
 		if prevServeConfig != nil && reflect.DeepEqual(sc, prevServeConfig) {
 			continue
 		}
-		log.Printf("Applying serve config")
+		validateHTTPSServe(certDomain, sc)
-		if err := lc.SetServeConfig(ctx, sc); err != nil {
+		if err := updateServeConfig(ctx, sc, certDomain, kc, lc); err != nil {
-			log.Fatalf("failed to set serve config: %v", err)
+			log.Fatalf("serve proxy: error updating serve config: %v", err)
 		}
 		prevServeConfig = sc
 	}
 }
 
+func certDomainFromNetmap(nm *netmap.NetworkMap) string {
+	if len(nm.DNS.CertDomains) == 0 {
+		return ""
+	}
+	return nm.DNS.CertDomains[0]
+}
+
+func updateServeConfig(ctx context.Context, sc *ipn.ServeConfig, certDomain string, kc *kubeClient, lc *tailscale.LocalClient) error {
+	defer func() {
+		if err := kc.storeHTTPSEndpoint(ctx, certDomain); err != nil {
+			log.Printf("[unexpected]: serve proxy: error storing HTTPS endpoint: %v", err)
+		}
+	}()
+	// TODO(irbekrm): This means that serve config that does not expose HTTPS endpoint will not be set for a tailnet
+	// that does not have HTTPS enabled. We probably want to fix this.
+	if certDomain == kubetypes.ValueNoHTTPS {
+		return nil
+	}
+	log.Printf("serve proxy: applying serve config")
+	return lc.SetServeConfig(ctx, sc)
+}
+
+func validateHTTPSServe(certDomain string, sc *ipn.ServeConfig) {
+	if certDomain != kubetypes.ValueNoHTTPS || !hasHTTPSEndpoint(sc) {
+		return
+	}
+	log.Printf(
+		`serve proxy: this node is configured as a proxy that exposes an HTTPS endpoint to tailnet,
+		(perhaps a Kubernetes operator Ingress proxy) but it is not able to issue TLS certs, so this will likely not work.
+		To make it work, ensure that HTTPS is enabled for you tailnet, see https://tailscale.com/kb/1153/enabling-https or this will not work.`)
+}
+
+func hasHTTPSEndpoint(cfg *ipn.ServeConfig) bool {
+	for _, tcpCfg := range cfg.TCP {
+		if tcpCfg.HTTPS {
+			return true
+		}
+	}
+	return false
+}
+
 // readServeConfig reads the ipn.ServeConfig from path, replacing
 // ${TS_CERT_DOMAIN} with certDomain.
 func readServeConfig(path, certDomain string) (*ipn.ServeConfig, error) {