diff --git a/cmd/k8s-operator/ingress-for-pg.go b/cmd/k8s-operator/ingress-for-pg.go index 4dcaf7c6d..e90187d58 100644 --- a/cmd/k8s-operator/ingress-for-pg.go +++ b/cmd/k8s-operator/ingress-for-pg.go @@ -222,13 +222,14 @@ func (a *IngressPGReconciler) maybeProvision(ctx context.Context, hostname strin }, }, } + serviceName := tailcfg.ServiceName("svc:" + hostname) var gotCfg *ipn.ServiceConfig if cfg != nil && cfg.Services != nil { - gotCfg = cfg.Services[hostname] + gotCfg = cfg.Services[serviceName] } if !reflect.DeepEqual(gotCfg, ingCfg) { logger.Infof("Updating serve config") - mak.Set(&cfg.Services, hostname, ingCfg) + mak.Set(&cfg.Services, serviceName, ingCfg) cfgBytes, err := json.Marshal(cfg) if err != nil { return fmt.Errorf("error marshaling serve config: %w", err) @@ -309,7 +310,7 @@ func (a *IngressPGReconciler) maybeCleanupProxyGroup(ctx context.Context, proxyG found := false for _, i := range ingList.Items { ingressHostname := hostnameForIngress(&i) - if ingressHostname == vipHostname { + if ingressHostname == vipHostname.WithoutPrefix() { found = true break } @@ -317,7 +318,7 @@ func (a *IngressPGReconciler) maybeCleanupProxyGroup(ctx context.Context, proxyG if !found { logger.Infof("VIPService %q is not owned by any Ingress, cleaning up", vipHostname) - svc, err := a.getVIPService(ctx, vipHostname, logger) + svc, err := a.getVIPService(ctx, vipHostname.WithoutPrefix(), logger) if err != nil { errResp := &tailscale.ErrResponse{} if errors.As(err, &errResp) && errResp.Status == http.StatusNotFound { @@ -329,7 +330,7 @@ func (a *IngressPGReconciler) maybeCleanupProxyGroup(ctx context.Context, proxyG } if isVIPServiceForAnyIngress(svc) { logger.Infof("cleaning up orphaned VIPService %q", vipHostname) - if err := a.tsClient.deleteVIPServiceByName(ctx, vipHostname); err != nil { + if err := a.tsClient.deleteVIPServiceByName(ctx, vipHostname.WithoutPrefix()); err != nil { errResp := &tailscale.ErrResponse{} if !errors.As(err, &errResp) || errResp.Status != http.StatusNotFound { return fmt.Errorf("deleting VIPService %q: %w", vipHostname, err) @@ -374,11 +375,12 @@ func (a *IngressPGReconciler) maybeCleanup(ctx context.Context, hostname string, if err != nil { return fmt.Errorf("error getting ProxyGroup serve config: %w", err) } + serviceName := tailcfg.ServiceName("svc:" + hostname) // VIPService is always first added to serve config and only then created in the Tailscale API, so if it is not // found in the serve config, we can assume that there is no VIPService. TODO(irbekrm): once we have ingress // ProxyGroup, we will probably add currently exposed VIPServices to its status. At that point, we can use the // status rather than checking the serve config each time. - if cfg == nil || cfg.Services == nil || cfg.Services[hostname] == nil { + if cfg == nil || cfg.Services == nil || cfg.Services[serviceName] == nil { return nil } logger.Infof("Ensuring that VIPService %q configuration is cleaned up", hostname) @@ -390,7 +392,7 @@ func (a *IngressPGReconciler) maybeCleanup(ctx context.Context, hostname string, // 3. Remove the VIPService from the serve config for the ProxyGroup. logger.Infof("Removing VIPService %q from serve config for ProxyGroup %q", hostname, pg) - delete(cfg.Services, hostname) + delete(cfg.Services, serviceName) cfgBytes, err := json.Marshal(cfg) if err != nil { return fmt.Errorf("error marshaling serve config: %w", err) diff --git a/cmd/k8s-operator/ingress-for-pg_test.go b/cmd/k8s-operator/ingress-for-pg_test.go index 2cd340962..9ef36f696 100644 --- a/cmd/k8s-operator/ingress-for-pg_test.go +++ b/cmd/k8s-operator/ingress-for-pg_test.go @@ -137,7 +137,7 @@ func TestIngressPGReconciler(t *testing.T) { t.Fatalf("unmarshaling serve config: %v", err) } - if cfg.Services["my-svc"] == nil { + if cfg.Services["svc:my-svc"] == nil { t.Error("expected serve config to contain VIPService configuration") } diff --git a/cmd/tailscale/cli/advertise.go b/cmd/tailscale/cli/advertise.go index 00b5024f0..83d1a35aa 100644 --- a/cmd/tailscale/cli/advertise.go +++ b/cmd/tailscale/cli/advertise.go @@ -66,7 +66,7 @@ func parseServiceNames(servicesArg string) ([]string, error) { if servicesArg != "" { services = strings.Split(servicesArg, ",") for _, svc := range services { - err := tailcfg.CheckServiceName(svc) + err := tailcfg.ServiceName(svc).Validate() if err != nil { return nil, fmt.Errorf("service %q: %s", svc, err) } diff --git a/ipn/ipn_clone.go b/ipn/ipn_clone.go index 34d7ba9a6..47cca71d0 100644 --- a/ipn/ipn_clone.go +++ b/ipn/ipn_clone.go @@ -106,7 +106,7 @@ func (src *ServeConfig) Clone() *ServeConfig { } } if dst.Services != nil { - dst.Services = map[string]*ServiceConfig{} + dst.Services = map[tailcfg.ServiceName]*ServiceConfig{} for k, v := range src.Services { if v == nil { dst.Services[k] = nil @@ -133,7 +133,7 @@ func (src *ServeConfig) Clone() *ServeConfig { var _ServeConfigCloneNeedsRegeneration = ServeConfig(struct { TCP map[uint16]*TCPPortHandler Web map[HostPort]*WebServerConfig - Services map[string]*ServiceConfig + Services map[tailcfg.ServiceName]*ServiceConfig AllowFunnel map[HostPort]bool Foreground map[string]*ServeConfig ETag string diff --git a/ipn/ipn_view.go b/ipn/ipn_view.go index 9cd5a466a..41b4ddbc8 100644 --- a/ipn/ipn_view.go +++ b/ipn/ipn_view.go @@ -195,7 +195,7 @@ func (v ServeConfigView) Web() views.MapFn[HostPort, *WebServerConfig, WebServer }) } -func (v ServeConfigView) Services() views.MapFn[string, *ServiceConfig, ServiceConfigView] { +func (v ServeConfigView) Services() views.MapFn[tailcfg.ServiceName, *ServiceConfig, ServiceConfigView] { return views.MapFnOf(v.ж.Services, func(t *ServiceConfig) ServiceConfigView { return t.View() }) @@ -216,7 +216,7 @@ func (v ServeConfigView) ETag() string { return v.ж.ETag } var _ServeConfigViewNeedsRegeneration = ServeConfig(struct { TCP map[uint16]*TCPPortHandler Web map[HostPort]*WebServerConfig - Services map[string]*ServiceConfig + Services map[tailcfg.ServiceName]*ServiceConfig AllowFunnel map[HostPort]bool Foreground map[string]*ServeConfig ETag string diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 470824fde..2bd46b852 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -3417,13 +3417,13 @@ func generateInterceptVIPServicesTCPPortFunc(svcAddrPorts map[netip.Addr]func(ui // setVIPServicesTCPPortsIntercepted populates b.shouldInterceptVIPServicesTCPPortAtomic with an // efficient func for ShouldInterceptTCPPort to use, which is called on every incoming packet. -func (b *LocalBackend) setVIPServicesTCPPortsIntercepted(svcPorts map[string][]uint16) { +func (b *LocalBackend) setVIPServicesTCPPortsIntercepted(svcPorts map[tailcfg.ServiceName][]uint16) { b.mu.Lock() defer b.mu.Unlock() b.setVIPServicesTCPPortsInterceptedLocked(svcPorts) } -func (b *LocalBackend) setVIPServicesTCPPortsInterceptedLocked(svcPorts map[string][]uint16) { +func (b *LocalBackend) setVIPServicesTCPPortsInterceptedLocked(svcPorts map[tailcfg.ServiceName][]uint16) { if len(svcPorts) == 0 { b.shouldInterceptVIPServicesTCPPortAtomic.Store(func(netip.AddrPort) bool { return false }) return @@ -6025,7 +6025,7 @@ func (b *LocalBackend) reloadServeConfigLocked(prefs ipn.PrefsView) { // b.mu must be held. func (b *LocalBackend) setTCPPortsInterceptedFromNetmapAndPrefsLocked(prefs ipn.PrefsView) { handlePorts := make([]uint16, 0, 4) - var vipServicesPorts map[string][]uint16 + var vipServicesPorts map[tailcfg.ServiceName][]uint16 if prefs.Valid() && prefs.RunSSH() && envknob.CanSSHD() { handlePorts = append(handlePorts, 22) @@ -7815,7 +7815,7 @@ func (b *LocalBackend) vipServiceHash(services []*tailcfg.VIPService) string { func (b *LocalBackend) vipServicesFromPrefsLocked(prefs ipn.PrefsView) []*tailcfg.VIPService { // keyed by service name - var services map[string]*tailcfg.VIPService + var services map[tailcfg.ServiceName]*tailcfg.VIPService if !b.serveConfig.Valid() { return nil } @@ -7828,12 +7828,13 @@ func (b *LocalBackend) vipServicesFromPrefsLocked(prefs ipn.PrefsView) []*tailcf } for _, s := range prefs.AdvertiseServices().All() { - if services == nil || services[s] == nil { - mak.Set(&services, s, &tailcfg.VIPService{ - Name: s, + sn := tailcfg.ServiceName(s) + if services == nil || services[sn] == nil { + mak.Set(&services, sn, &tailcfg.VIPService{ + Name: sn, }) } - services[s].Active = true + services[sn].Active = true } return slicesx.MapValues(services) diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index f851bb0f8..b1a79860d 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -2715,7 +2715,7 @@ func TestTCPHandlerForDstWithVIPService(t *testing.T) { err = b.setServeConfigLocked( &ipn.ServeConfig{ - Services: map[string]*ipn.ServiceConfig{ + Services: map[tailcfg.ServiceName]*ipn.ServiceConfig{ "svc:foo": { TCP: map[uint16]*ipn.TCPPortHandler{ 882: {HTTP: true}, @@ -4747,7 +4747,7 @@ func TestGetVIPServices(t *testing.T) { "served-only", []string{}, &ipn.ServeConfig{ - Services: map[string]*ipn.ServiceConfig{ + Services: map[tailcfg.ServiceName]*ipn.ServiceConfig{ "svc:abc": {Tun: true}, }, }, @@ -4762,7 +4762,7 @@ func TestGetVIPServices(t *testing.T) { "served-and-advertised", []string{"svc:abc"}, &ipn.ServeConfig{ - Services: map[string]*ipn.ServiceConfig{ + Services: map[tailcfg.ServiceName]*ipn.ServiceConfig{ "svc:abc": {Tun: true}, }, }, @@ -4778,7 +4778,7 @@ func TestGetVIPServices(t *testing.T) { "served-and-advertised-different-service", []string{"svc:def"}, &ipn.ServeConfig{ - Services: map[string]*ipn.ServiceConfig{ + Services: map[tailcfg.ServiceName]*ipn.ServiceConfig{ "svc:abc": {Tun: true}, }, }, @@ -4797,7 +4797,7 @@ func TestGetVIPServices(t *testing.T) { "served-with-port-ranges-one-range-single", []string{}, &ipn.ServeConfig{ - Services: map[string]*ipn.ServiceConfig{ + Services: map[tailcfg.ServiceName]*ipn.ServiceConfig{ "svc:abc": {TCP: map[uint16]*ipn.TCPPortHandler{ 80: {HTTPS: true}, }}, @@ -4814,7 +4814,7 @@ func TestGetVIPServices(t *testing.T) { "served-with-port-ranges-one-range-multiple", []string{}, &ipn.ServeConfig{ - Services: map[string]*ipn.ServiceConfig{ + Services: map[tailcfg.ServiceName]*ipn.ServiceConfig{ "svc:abc": {TCP: map[uint16]*ipn.TCPPortHandler{ 80: {HTTPS: true}, 81: {HTTPS: true}, @@ -4833,7 +4833,7 @@ func TestGetVIPServices(t *testing.T) { "served-with-port-ranges-multiple-ranges", []string{}, &ipn.ServeConfig{ - Services: map[string]*ipn.ServiceConfig{ + Services: map[tailcfg.ServiceName]*ipn.ServiceConfig{ "svc:abc": {TCP: map[uint16]*ipn.TCPPortHandler{ 80: {HTTPS: true}, 81: {HTTPS: true}, @@ -4866,7 +4866,7 @@ func TestGetVIPServices(t *testing.T) { } got := lb.vipServicesFromPrefsLocked(prefs.View()) slices.SortFunc(got, func(a, b *tailcfg.VIPService) int { - return strings.Compare(a.Name, b.Name) + return strings.Compare(a.Name.String(), b.Name.String()) }) if !reflect.DeepEqual(tt.want, got) { t.Logf("want:") diff --git a/ipn/ipnlocal/serve.go b/ipn/ipnlocal/serve.go index a5247dd8c..63cb2ef55 100644 --- a/ipn/ipnlocal/serve.go +++ b/ipn/ipnlocal/serve.go @@ -55,7 +55,7 @@ var serveHTTPContextKey ctxkey.Key[*serveHTTPContext] type serveHTTPContext struct { SrcAddr netip.AddrPort - ForVIPService string // VIP service name, empty string means local + ForVIPService tailcfg.ServiceName // "" means local DestPort uint16 // provides funnel-specific context, nil if not funneled @@ -1006,7 +1006,7 @@ func allNumeric(s string) bool { return s != "" } -func (b *LocalBackend) webServerConfig(hostname string, forVIPService string, port uint16) (c ipn.WebServerConfigView, ok bool) { +func (b *LocalBackend) webServerConfig(hostname string, forVIPService tailcfg.ServiceName, port uint16) (c ipn.WebServerConfigView, ok bool) { key := ipn.HostPort(fmt.Sprintf("%s:%v", hostname, port)) b.mu.Lock() @@ -1021,7 +1021,7 @@ func (b *LocalBackend) webServerConfig(hostname string, forVIPService string, po return b.serveConfig.FindWeb(key) } -func (b *LocalBackend) getTLSServeCertForPort(port uint16, forVIPService string) func(hi *tls.ClientHelloInfo) (*tls.Certificate, error) { +func (b *LocalBackend) getTLSServeCertForPort(port uint16, forVIPService tailcfg.ServiceName) func(hi *tls.ClientHelloInfo) (*tls.Certificate, error) { return func(hi *tls.ClientHelloInfo) (*tls.Certificate, error) { if hi == nil || hi.ServerName == "" { return nil, errors.New("no SNI ServerName") diff --git a/ipn/ipnlocal/serve_test.go b/ipn/ipnlocal/serve_test.go index f2ea8e5cd..eb8169390 100644 --- a/ipn/ipnlocal/serve_test.go +++ b/ipn/ipnlocal/serve_test.go @@ -354,7 +354,7 @@ func TestServeConfigServices(t *testing.T) { { name: "one-incorrectly-configured-service", conf: &ipn.ServeConfig{ - Services: map[string]*ipn.ServiceConfig{ + Services: map[tailcfg.ServiceName]*ipn.ServiceConfig{ "svc:foo": { TCP: map[uint16]*ipn.TCPPortHandler{ 80: {HTTP: true}, @@ -369,7 +369,7 @@ func TestServeConfigServices(t *testing.T) { // one correctly configured service with packet should be intercepted name: "one-service-intercept-packet", conf: &ipn.ServeConfig{ - Services: map[string]*ipn.ServiceConfig{ + Services: map[tailcfg.ServiceName]*ipn.ServiceConfig{ "svc:foo": { TCP: map[uint16]*ipn.TCPPortHandler{ 80: {HTTP: true}, @@ -388,7 +388,7 @@ func TestServeConfigServices(t *testing.T) { // one correctly configured service with packet should not be intercepted name: "one-service-not-intercept-packet", conf: &ipn.ServeConfig{ - Services: map[string]*ipn.ServiceConfig{ + Services: map[tailcfg.ServiceName]*ipn.ServiceConfig{ "svc:foo": { TCP: map[uint16]*ipn.TCPPortHandler{ 80: {HTTP: true}, @@ -406,10 +406,10 @@ func TestServeConfigServices(t *testing.T) { intercepted: false, }, { - //multiple correctly configured service with packet should be intercepted + // multiple correctly configured service with packet should be intercepted name: "multiple-service-intercept-packet", conf: &ipn.ServeConfig{ - Services: map[string]*ipn.ServiceConfig{ + Services: map[tailcfg.ServiceName]*ipn.ServiceConfig{ "svc:foo": { TCP: map[uint16]*ipn.TCPPortHandler{ 80: {HTTP: true}, @@ -437,7 +437,7 @@ func TestServeConfigServices(t *testing.T) { // multiple correctly configured service with packet should not be intercepted name: "multiple-service-not-intercept-packet", conf: &ipn.ServeConfig{ - Services: map[string]*ipn.ServiceConfig{ + Services: map[tailcfg.ServiceName]*ipn.ServiceConfig{ "svc:foo": { TCP: map[uint16]*ipn.TCPPortHandler{ 80: {HTTP: true}, diff --git a/ipn/serve.go b/ipn/serve.go index 4c2d2f158..ac92287bd 100644 --- a/ipn/serve.go +++ b/ipn/serve.go @@ -57,7 +57,7 @@ type ServeConfig struct { // Services maps from service name (in the form "svc:dns-label") to a ServiceConfig. // Which describes the L3, L4, and L7 forwarding information for the service. - Services map[string]*ServiceConfig `json:",omitempty"` + Services map[tailcfg.ServiceName]*ServiceConfig `json:",omitempty"` // AllowFunnel is the set of SNI:port values for which funnel // traffic is allowed, from trusted ingress peers. @@ -618,7 +618,7 @@ func (v ServeConfigView) Webs() iter.Seq2[HostPort, WebServerConfigView] { } // FindServiceTCP return the TCPPortHandlerView for the given service name and port. -func (v ServeConfigView) FindServiceTCP(svcName string, port uint16) (res TCPPortHandlerView, ok bool) { +func (v ServeConfigView) FindServiceTCP(svcName tailcfg.ServiceName, port uint16) (res TCPPortHandlerView, ok bool) { svcCfg, ok := v.Services().GetOk(svcName) if !ok { return res, ok @@ -626,7 +626,7 @@ func (v ServeConfigView) FindServiceTCP(svcName string, port uint16) (res TCPPor return svcCfg.TCP().GetOk(port) } -func (v ServeConfigView) FindServiceWeb(svcName string, hp HostPort) (res WebServerConfigView, ok bool) { +func (v ServeConfigView) FindServiceWeb(svcName tailcfg.ServiceName, hp HostPort) (res WebServerConfigView, ok bool) { if svcCfg, ok := v.Services().GetOk(svcName); ok { if res, ok := svcCfg.Web().GetOk(hp); ok { return res, ok diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index e1259b3f5..c921a0c7d 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -717,21 +717,6 @@ func CheckTag(tag string) error { return nil } -// CheckServiceName validates svc for use as a service name. -// We only allow valid DNS labels, since the expectation is that these will be -// used as parts of domain names. -func CheckServiceName(svc string) error { - var ok bool - svc, ok = strings.CutPrefix(svc, "svc:") - if !ok { - return errors.New("services must start with 'svc:'") - } - if svc == "" { - return errors.New("service names must not be empty") - } - return dnsname.ValidLabel(svc) -} - // CheckRequestTags checks that all of h.RequestTags are valid. func (h *Hostinfo) CheckRequestTags() error { if h == nil { @@ -897,16 +882,51 @@ type Hostinfo struct { // require changes to Hostinfo.Equal. } +// ServiceName is the name of a service, of the form `svc:dns-label`. Services +// represent some kind of application provided for users of the tailnet with a +// MagicDNS name and possibly dedicated IP addresses. Currently (2024-01-21), +// the only type of service is [VIPService]. +// This is not related to the older [Service] used in [Hostinfo.Services]. +type ServiceName string + +// Validate validates if the service name is formatted correctly. +// We only allow valid DNS labels, since the expectation is that these will be +// used as parts of domain names. +func (sn ServiceName) Validate() error { + bareName, ok := strings.CutPrefix(string(sn), "svc:") + if !ok { + return errors.New("services must start with 'svc:'") + } + if bareName == "" { + return errors.New("service names must not be empty") + } + return dnsname.ValidLabel(bareName) +} + +// String implements [fmt.Stringer]. +func (sn ServiceName) String() string { + return string(sn) +} + +// WithoutPrefix is the name of the service without the `svc:` prefix, used for +// DNS names. If the name does not include the prefix (which means +// [ServiceName.Validate] would return an error) then it returns "". +func (sn ServiceName) WithoutPrefix() string { + bareName, ok := strings.CutPrefix(string(sn), "svc:") + if !ok { + return "" + } + return bareName +} + // VIPService represents a service created on a tailnet from the // perspective of a node providing that service. These services // have an virtual IP (VIP) address pair distinct from the node's IPs. type VIPService struct { - // Name is the name of the service, of the form `svc:dns-label`. - // See CheckServiceName for a validation func. - // Name uniquely identifies a service on a particular tailnet, - // and so also corresponds uniquely to the pair of IP addresses - // belonging to the VIP service. - Name string + // Name is the name of the service. The Name uniquely identifies a service + // on a particular tailnet, and so also corresponds uniquely to the pair of + // IP addresses belonging to the VIP service. + Name ServiceName // Ports specify which ProtoPorts are made available by this node // on the service's IPs. @@ -2972,10 +2992,10 @@ type EarlyNoise struct { // vs NodeKey) const LBHeader = "Ts-Lb" -// ServiceIPMappings maps service names (strings that conform to -// [CheckServiceName]) to lists of IP addresses. This is used as the value of -// the [NodeAttrServiceHost] capability, to inform service hosts what IP -// addresses they need to listen on for each service that they are advertising. +// ServiceIPMappings maps ServiceName to lists of IP addresses. This is used +// as the value of the [NodeAttrServiceHost] capability, to inform service hosts +// what IP addresses they need to listen on for each service that they are +// advertising. // // This is of the form: // @@ -2988,4 +3008,4 @@ const LBHeader = "Ts-Lb" // provided in AllowedIPs, but this lets the client know which services // correspond to those IPs. Any services that don't correspond to a service // this client is hosting can be ignored. -type ServiceIPMappings map[string][]netip.Addr +type ServiceIPMappings map[ServiceName][]netip.Addr diff --git a/types/netmap/netmap.go b/types/netmap/netmap.go index 1482e534e..ab22eec3e 100644 --- a/types/netmap/netmap.go +++ b/types/netmap/netmap.go @@ -427,10 +427,10 @@ const ( ) // IPServiceMappings maps IP addresses to service names. This is the inverse of -// [ServiceIPMappings], and is used to inform clients which services is an VIP -// address associated with. This is set to b.ipVIPServiceMap every time the -// netmap is updated. This is used to reduce the cost for looking up the service -// name for the dst IP address in the netStack packet processing workflow. +// [tailcfg.ServiceIPMappings], and is used to inform track which service a VIP +// is associated with. This is set to b.ipVIPServiceMap every time the netmap is +// updated. This is used to reduce the cost for looking up the service name for +// the dst IP address in the netStack packet processing workflow. // // This is of the form: // @@ -440,4 +440,4 @@ const ( // "100.102.42.3": "svc:web", // "fd7a:115c:a1e0::abcd": "svc:web", // } -type IPServiceMappings map[netip.Addr]string +type IPServiceMappings map[netip.Addr]tailcfg.ServiceName