diff --git a/cmd/atenet/internal/router/extproc.go b/cmd/atenet/internal/router/extproc.go index 7c8c184e2..a1077efe0 100644 --- a/cmd/atenet/internal/router/extproc.go +++ b/cmd/atenet/internal/router/extproc.go @@ -129,7 +129,7 @@ func (s *ExtProcServer) handleRequestHeaders( metadata := newRequestMetadata(reqHeaders.Headers.GetHeaders()) slog.InfoContext(ctx, "Request", slog.String("metadata", metadata.String())) - actorID, err := parseActorID(metadata.host) + actorID, port, err := parseActorID(metadata.host) if err != nil { // Host is invalid, respond with 404. return nil, metadata, "", "", "", invalidHostErr(metadata.host, err) @@ -158,8 +158,7 @@ func (s *ExtProcServer) handleRequestHeaders( "actor %q routing failed", actorID) } - // TODO(bowei) -- handle more than port 80 on the actor. - targetAddr := net.JoinHostPort(workerIP, "80") + targetAddr := net.JoinHostPort(workerIP, port) slog.InfoContext(ctx, "Route ok", slog.String("actorID", actorID), slog.String("targetAddr", targetAddr)) diff --git a/cmd/atenet/internal/router/extproc_in.go b/cmd/atenet/internal/router/extproc_in.go index a394f98da..2c10566f2 100644 --- a/cmd/atenet/internal/router/extproc_in.go +++ b/cmd/atenet/internal/router/extproc_in.go @@ -17,6 +17,8 @@ package router import ( "fmt" "net" + "regexp" + "strconv" "strings" "github.com/agent-substrate/substrate/internal/resources" @@ -61,21 +63,56 @@ func newRequestMetadata(headers []*corev3.HeaderValue) *requestMetadata { } } -func parseActorID(host string) (string, error) { +// defaultActorPort is the actor service port used when the host has no +// "-" prefix. +const defaultActorPort = "80" + +// portPrefixRegex matches a "-" left-most host label. A leading +// numeric run followed by "-" is treated as the target service port on the +// actor. Actor IDs minted for multi-port access must therefore not themselves +// begin with "-"; IDs without that shape keep routing to the default +// port and remain backwards compatible. +var portPrefixRegex = regexp.MustCompile(`^([0-9]+)-(.+)$`) + +// parseActorID extracts the actor ID and target service port from an actor +// host. The default form +// +// . -> port 80 +// +// while a port-prefixed form selects an explicit service port, letting a single +// actor expose multiple services: +// +// -. -> port +// +// Any trailing ":" on the host is the client connection port and is +// stripped before parsing; it does not affect the routed service port. +func parseActorID(host string) (string, string, error) { var err error if strings.Contains(host, ":") { host, _, err = net.SplitHostPort(host) } if err != nil { - return "", err + return "", "", err } - actorID, found := strings.CutSuffix(strings.TrimSuffix(host, "."), "."+resources.ActorDNSSuffix) + label, found := strings.CutSuffix(strings.TrimSuffix(host, "."), "."+resources.ActorDNSSuffix) if !found { - return "", fmt.Errorf("invalid actor_id: must end with %s, got %q", resources.ActorDNSSuffix, host) + return "", "", fmt.Errorf("invalid actor_id: must end with %s, got %q", resources.ActorDNSSuffix, host) } + + port := defaultActorPort + actorID := label + if m := portPrefixRegex.FindStringSubmatch(label); m != nil { + p, perr := strconv.Atoi(m[1]) + if perr != nil || p < 1 || p > 65535 { + return "", "", fmt.Errorf("invalid actor port %q in host %q: must be in 1..65535", m[1], host) + } + port = strconv.Itoa(p) + actorID = m[2] + } + if err := resources.ValidateActorID(actorID); err != nil { - return "", err + return "", "", err } - return actorID, nil + return actorID, port, nil } diff --git a/cmd/atenet/internal/router/extproc_in_test.go b/cmd/atenet/internal/router/extproc_in_test.go index 8fa9ea076..d8644ffa7 100644 --- a/cmd/atenet/internal/router/extproc_in_test.go +++ b/cmd/atenet/internal/router/extproc_in_test.go @@ -137,59 +137,120 @@ func TestRequestMetadata_String(t *testing.T) { func TestParseActorID(t *testing.T) { tests := []struct { - name string - host string - wantID string - wantErr bool + name string + host string + wantID string + wantPort string + wantErr bool }{ { - name: "valid host without port", - host: "my-actor.actors.resources.substrate.ate.dev", - wantID: "my-actor", - wantErr: false, + name: "valid host without port", + host: "my-actor.actors.resources.substrate.ate.dev", + wantID: "my-actor", + wantPort: "80", + wantErr: false, }, { - name: "valid host with port", - host: "my-actor.actors.resources.substrate.ate.dev:8443", - wantID: "my-actor", - wantErr: false, + name: "valid host with port", + host: "my-actor.actors.resources.substrate.ate.dev:8443", + wantID: "my-actor", + wantPort: "80", + wantErr: false, }, { - name: "valid host with trailing dot", - host: "my-actor.actors.resources.substrate.ate.dev.", - wantID: "my-actor", - wantErr: false, + name: "valid host with trailing dot", + host: "my-actor.actors.resources.substrate.ate.dev.", + wantID: "my-actor", + wantPort: "80", + wantErr: false, }, { - name: "valid host with trailing dot and port", - host: "my-actor.actors.resources.substrate.ate.dev.:8080", - wantID: "my-actor", - wantErr: false, + name: "valid host with trailing dot and port", + host: "my-actor.actors.resources.substrate.ate.dev.:8080", + wantID: "my-actor", + wantPort: "80", + wantErr: false, + }, + { + name: "port-prefixed host", + host: "8080-my-actor.actors.resources.substrate.ate.dev", + wantID: "my-actor", + wantPort: "8080", + wantErr: false, + }, + { + name: "port-prefixed host with connection port", + host: "49983-sbx1.actors.resources.substrate.ate.dev:443", + wantID: "sbx1", + wantPort: "49983", + wantErr: false, + }, + { + name: "port-prefixed host with trailing dot", + host: "8080-my-actor.actors.resources.substrate.ate.dev.", + wantID: "my-actor", + wantPort: "8080", + wantErr: false, + }, + { + name: "min port", + host: "1-my-actor.actors.resources.substrate.ate.dev", + wantID: "my-actor", + wantPort: "1", + wantErr: false, + }, + { + name: "max port", + host: "65535-my-actor.actors.resources.substrate.ate.dev", + wantID: "my-actor", + wantPort: "65535", + wantErr: false, + }, + { + name: "port zero is invalid", + host: "0-my-actor.actors.resources.substrate.ate.dev", + wantErr: true, + }, + { + name: "port above range is invalid", + host: "65536-my-actor.actors.resources.substrate.ate.dev", + wantErr: true, + }, + { + name: "leading-zero port is normalized", + host: "0080-my-actor.actors.resources.substrate.ate.dev", + wantID: "my-actor", + wantPort: "80", + wantErr: false, }, { name: "invalid suffix", host: "my-actor.example.com", - wantID: "", wantErr: true, }, { name: "invalid host port format", host: "my-actor.actors.resources.substrate.ate.dev:invalid:port", - wantID: "", wantErr: true, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - gotID, err := parseActorID(tc.host) + gotID, gotPort, err := parseActorID(tc.host) if (err != nil) != tc.wantErr { t.Errorf("parseActorID(%q) error = %v, wantErr %v", tc.host, err, tc.wantErr) return } + if tc.wantErr { + return + } if gotID != tc.wantID { t.Errorf("parseActorID(%q) gotID = %v, want %v", tc.host, gotID, tc.wantID) } + if gotPort != tc.wantPort { + t.Errorf("parseActorID(%q) gotPort = %v, want %v", tc.host, gotPort, tc.wantPort) + } }) } } diff --git a/cmd/ateom-gvisor/main.go b/cmd/ateom-gvisor/main.go index 365447fca..977742040 100644 --- a/cmd/ateom-gvisor/main.go +++ b/cmd/ateom-gvisor/main.go @@ -35,7 +35,6 @@ import ( "github.com/agent-substrate/substrate/internal/serverboot" "github.com/agent-substrate/substrate/internal/version" "github.com/google/nftables" - "github.com/google/nftables/binaryutil" "github.com/google/nftables/expr" "github.com/hashicorp/go-reap" "github.com/spf13/pflag" @@ -609,8 +608,9 @@ func installActorNftablesRules(podIP net.IP) error { // // * postrouting: masquerade actor egress from 169.254.17.2 behind the worker // pod IP so replies route back to the pod. - // * prerouting: DNAT traffic sent to the worker pod IP on TCP/80 to the - // actor veth IP on TCP/80, preserving existing inbound behavior. + // * prerouting: DNAT inbound TCP sent to the worker pod IP to the actor + // veth IP, preserving the destination port so multi-service actors are + // reachable on any port atenet routes to. // * forward: accept forwarded packets between the actor veth and pod eth0. // // This is not the final egress policy path. The later AgentGateway phase @@ -636,23 +636,26 @@ func installActorNftablesRules(podIP net.IP) error { }) // TODO: Support inbound UDP DNAT for actors that expose UDP protocols such // as QUIC. - // TODO: Replace the hard-coded HTTP port with the actor's configured - // inbound ports, either by adding one rule per port or by matching a set. - preroutingExprs := append(ipDestinationEqual(podIP.String()), tcpDestinationPortEqual(80)...) + // + // DNAT every inbound TCP segment destined for the worker pod IP to the actor + // veth IP, preserving the original destination port. atenet selects the + // target service port via the request host (e.g. "8080-") and + // forwards to :; this rule must therefore forward every port to + // the actor, not just 80, so multi-service actors work. The actor decides + // which ports it actually listens on. Only the destination address is + // rewritten (RegProtoMin is left unset), so the port carries through + // unchanged. ateom itself serves on a unix socket, not a pod-IP TCP port, so + // nothing on the control path is shadowed by this catch-all. + preroutingExprs := append(ipDestinationEqual(podIP.String()), tcpProtocol()...) preroutingExprs = append(preroutingExprs, &expr.Immediate{ Register: 1, Data: net.ParseIP(actorVethIP).To4(), }, - &expr.Immediate{ - Register: 2, - Data: binaryutil.BigEndian.PutUint16(80), - }, &expr.NAT{ - Type: expr.NATTypeDestNAT, - Family: unix.NFPROTO_IPV4, - RegAddrMin: 1, - RegProtoMin: 2, + Type: expr.NATTypeDestNAT, + Family: unix.NFPROTO_IPV4, + RegAddrMin: 1, }, ) c.AddRule(&nftables.Rule{ @@ -743,7 +746,7 @@ func ipPayloadEqual(offset uint32, ip string) []expr.Any { } } -func tcpDestinationPortEqual(port uint16) []expr.Any { +func tcpProtocol() []expr.Any { return []expr.Any{ &expr.Meta{Key: expr.MetaKeyL4PROTO, Register: 1}, &expr.Cmp{ @@ -751,17 +754,6 @@ func tcpDestinationPortEqual(port uint16) []expr.Any { Register: 1, Data: []byte{unix.IPPROTO_TCP}, }, - &expr.Payload{ - DestRegister: 1, - Base: expr.PayloadBaseTransportHeader, - Offset: 2, - Len: 2, - }, - &expr.Cmp{ - Op: expr.CmpOpEq, - Register: 1, - Data: binaryutil.BigEndian.PutUint16(port), - }, } }