Skip to content

Commit 55f16fa

Browse files
howardjohnbowei
authored andcommitted
Address comments
1 parent 6987214 commit 55f16fa

15 files changed

Lines changed: 144 additions & 64 deletions

File tree

cmd/atenet/internal/router/config.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -132,9 +132,7 @@ func (c routerConfig) validate() error {
132132
if err := c.ParkedRequest.validate(); err != nil {
133133
return err
134134
}
135-
if c.atenetRouter() == atenetRouterAgentgateway {
136-
return nil
137-
}
135+
138136
if c.ExtProcMaxRequests < 0 {
139137
return fmt.Errorf("--extproc-max-requests must not be negative, got %d (0 derives it from --parked-request-max)", c.ExtProcMaxRequests)
140138
}

cmd/atenet/internal/router/config_test.go

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,15 @@ func TestRouterConfigValidate(t *testing.T) {
2626
wantErr string // substring; empty means valid
2727
}{
2828
{
29-
name: "defaults are valid (auto breaker)",
29+
name: "atenet-router defaults to envoy",
3030
cfg: routerConfig{ExtProcMaxRequests: 0, ParkedRequest: ParkedRequestConfig{Max: defaultParkedRequestMax}},
3131
},
3232
{
33-
name: "agentgateway is valid",
33+
name: "atenet-router set to envoy is valid",
34+
cfg: routerConfig{AtenetRouter: string(atenetRouterEnvoy), ParkedRequest: ParkedRequestConfig{Max: defaultParkedRequestMax}},
35+
},
36+
{
37+
name: "atenet-router set to agentgateway is valid",
3438
cfg: routerConfig{AtenetRouter: string(atenetRouterAgentgateway), ParkedRequest: ParkedRequestConfig{Max: defaultParkedRequestMax}},
3539
},
3640
{
@@ -73,6 +77,25 @@ func TestRouterConfigValidate(t *testing.T) {
7377
}
7478
}
7579

80+
func TestRouterConfigAtenetRouter(t *testing.T) {
81+
tests := []struct {
82+
name string
83+
cfg routerConfig
84+
want atenetRouter
85+
}{
86+
{name: "default", cfg: routerConfig{}, want: atenetRouterEnvoy},
87+
{name: "explicit envoy", cfg: routerConfig{AtenetRouter: string(atenetRouterEnvoy)}, want: atenetRouterEnvoy},
88+
{name: "agentgateway", cfg: routerConfig{AtenetRouter: string(atenetRouterAgentgateway)}, want: atenetRouterAgentgateway},
89+
}
90+
for _, tc := range tests {
91+
t.Run(tc.name, func(t *testing.T) {
92+
if got := tc.cfg.atenetRouter(); got != tc.want {
93+
t.Fatalf("atenetRouter() = %q, want %q", got, tc.want)
94+
}
95+
})
96+
}
97+
}
98+
7699
func TestRouterConfigExtProcMaxRequests(t *testing.T) {
77100
tests := []struct {
78101
name string
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
// Copyright 2026 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package router
16+
17+
import (
18+
"context"
19+
"fmt"
20+
"log/slog"
21+
"net"
22+
"time"
23+
24+
"golang.org/x/sync/errgroup"
25+
)
26+
27+
type dataplaneHealthCheck struct {
28+
url string
29+
expectedBody string
30+
}
31+
32+
func (r atenetRouter) routeViaAuthority() bool {
33+
return r == atenetRouterAgentgateway
34+
}
35+
36+
func (r atenetRouter) healthCheck() dataplaneHealthCheck {
37+
switch r {
38+
case atenetRouterEnvoy:
39+
return dataplaneHealthCheck{url: "http://127.0.0.1:9901/ready", expectedBody: "LIVE"}
40+
case atenetRouterAgentgateway:
41+
return dataplaneHealthCheck{url: "http://127.0.0.1:15021/healthz/ready", expectedBody: "ready"}
42+
default:
43+
return dataplaneHealthCheck{}
44+
}
45+
}
46+
47+
func (s *RouterServer) startDataplane(ctx context.Context, g *errgroup.Group, parkCfg ParkedRequestConfig) error {
48+
switch s.cfg.atenetRouter() {
49+
case atenetRouterEnvoy:
50+
s.startEnvoyDataplane(ctx, g, parkCfg)
51+
case atenetRouterAgentgateway:
52+
// Agentgateway receives all routing configuration from its static file.
53+
default:
54+
return fmt.Errorf("unsupported atenet router %q", s.cfg.atenetRouter())
55+
}
56+
return nil
57+
}
58+
59+
func (s *RouterServer) startEnvoyDataplane(ctx context.Context, g *errgroup.Group, parkCfg ParkedRequestConfig) {
60+
xdsSrv := NewXdsServer(s.cfg.XdsPort)
61+
xdsSrv.SetConfig(s.cfg.HttpPort, s.cfg.ExtprocPort, s.cfg.ExtprocAddr)
62+
setOtlpCollector(ctx, xdsSrv, s.cfg.OtlpCollectorAddress)
63+
64+
xdsSrv.SetExtProcMaxRequests(s.cfg.extProcMaxRequests())
65+
if parkCfg.enabled() {
66+
// Envoy must keep a parked request open at least as long as the router
67+
// will hold it; add a margin so the router surfaces its own 503 first.
68+
xdsSrv.SetExtProcMessageTimeout(parkCfg.Budget + 5*time.Second)
69+
}
70+
71+
xdsSrv.SetTlsConfig(s.cfg.HttpsPort, s.cfg.EnvoyCertPath)
72+
xdsSrv.SetUpstreamTls(s.cfg.UpstreamCredentialBundlePath, s.cfg.UpstreamTrustBundlePath, s.cfg.UpstreamSpiffePrefix)
73+
ctrl := NewController(s.k8sClient, s.clientset, s.cfg, xdsSrv, s.extprocSrv)
74+
75+
// Envoy receives all routing configuration from the local xDS server.
76+
g.Go(func() error {
77+
slog.InfoContext(ctx, "Starting ActorTemplate controller")
78+
return ctrl.Start(ctx)
79+
})
80+
g.Go(func() error {
81+
slog.InfoContext(ctx, "Starting Envoy xDS Server", slog.Int("port", s.cfg.XdsPort))
82+
lis, err := net.Listen("tcp", fmt.Sprintf(":%d", s.cfg.XdsPort))
83+
if err != nil {
84+
return fmt.Errorf("failed to listen on port %d: %w", s.cfg.XdsPort, err)
85+
}
86+
defer lis.Close()
87+
88+
return xdsSrv.Serve(ctx, lis)
89+
})
90+
}

cmd/atenet/internal/router/extproc_in.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ import (
2222
corev3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
2323
)
2424

25+
const authorityHeader = ":authority"
26+
2527
type requestMetadata struct {
2628
headers map[string]string
2729
path string
@@ -44,7 +46,7 @@ func newRequestMetadata(headers []*corev3.HeaderValue) *requestMetadata {
4446
if k == ":path" {
4547
path = val
4648
}
47-
if k == ":authority" || k == "host" {
49+
if k == authorityHeader || k == "host" {
4850
host = val
4951
}
5052
}

cmd/atenet/internal/router/extproc_out.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ func addRoutingMutations(dst, actorHost string, routeViaAuthority bool, mut *ext
7272
mut.SetHeaders = append(mut.SetHeaders, &corev3.HeaderValueOption{
7373
AppendAction: corev3.HeaderValueOption_OVERWRITE_IF_EXISTS_OR_ADD,
7474
Header: &corev3.HeaderValue{
75-
Key: ":authority",
75+
Key: authorityHeader,
7676
RawValue: []byte(dst),
7777
},
7878
})

cmd/atenet/internal/router/extproc_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -439,7 +439,7 @@ func TestAddRoutingMutationsViaAuthority(t *testing.T) {
439439
if got[strings.ToLower(atunnel.OriginalHostHeader)] != "actor-1.team-a.actors.resources.substrate.ate.dev" {
440440
t.Errorf("%s = %q", atunnel.OriginalHostHeader, got[strings.ToLower(atunnel.OriginalHostHeader)])
441441
}
442-
if got[":authority"] != "10.0.0.52:443" {
443-
t.Errorf(":authority = %q", got[":authority"])
442+
if got[authorityHeader] != "10.0.0.52:443" {
443+
t.Errorf("%s = %q", authorityHeader, got[authorityHeader])
444444
}
445445
}

cmd/atenet/internal/router/health.go

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -160,16 +160,8 @@ func (rh *routerHealth) checkDataplane(ctx context.Context) (bool, string) {
160160
timeoutCtx, cancel := context.WithTimeout(ctx, dependencyHealthCheckTimeout)
161161
defer cancel()
162162

163-
var url, expectedBody string
164-
switch rh.cfg.atenetRouter() {
165-
case atenetRouterEnvoy:
166-
url = "http://127.0.0.1:9901/ready"
167-
expectedBody = "LIVE"
168-
case atenetRouterAgentgateway:
169-
url = "http://127.0.0.1:15021/healthz/ready"
170-
expectedBody = "ready"
171-
}
172-
req, err := http.NewRequestWithContext(timeoutCtx, "GET", url, nil)
163+
check := rh.cfg.atenetRouter().healthCheck()
164+
req, err := http.NewRequestWithContext(timeoutCtx, "GET", check.url, nil)
173165
if err != nil {
174166
return false, err.Error()
175167
}
@@ -190,11 +182,11 @@ func (rh *routerHealth) checkDataplane(ctx context.Context) (bool, string) {
190182
}
191183

192184
bodyStr := strings.TrimSpace(string(bodyBytes))
193-
if bodyStr != expectedBody {
194-
return false, fmt.Sprintf("expected %s but got %q", expectedBody, bodyStr)
185+
if bodyStr != check.expectedBody {
186+
return false, fmt.Sprintf("expected %s but got %q", check.expectedBody, bodyStr)
195187
}
196188

197-
return true, expectedBody
189+
return true, check.expectedBody
198190
}
199191

200192
func (rh *routerHealth) checkK8s(ctx context.Context) (bool, string) {

cmd/atenet/internal/router/router.go

Lines changed: 3 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import (
2424
"os"
2525
"os/signal"
2626
"syscall"
27-
"time"
2827

2928
"github.com/spf13/cobra"
3029
"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc"
@@ -196,45 +195,12 @@ func (s *RouterServer) Run(ctx context.Context) error {
196195
if err != nil {
197196
return fmt.Errorf("failed to create parking metrics: %w", err)
198197
}
199-
routeViaAuthority := s.cfg.atenetRouter() == atenetRouterAgentgateway
200-
s.extprocSrv = NewExtProcServer(s.cfg.ExtprocPort, s.apiClient, routeDuration, parkCfg, parkMetrics, routeViaAuthority)
198+
s.extprocSrv = NewExtProcServer(s.cfg.ExtprocPort, s.apiClient, routeDuration, parkCfg, parkMetrics, s.cfg.atenetRouter().routeViaAuthority())
201199
}
202200
s.health = newRouterHealth(s.cfg.HealthInterval, s.clientset, s.apiClient, s.cfg)
203201

204-
switch s.cfg.atenetRouter() {
205-
case atenetRouterEnvoy:
206-
xdsSrv := NewXdsServer(s.cfg.XdsPort)
207-
xdsSrv.SetConfig(s.cfg.HttpPort, s.cfg.ExtprocPort, s.cfg.ExtprocAddr)
208-
setOtlpCollector(ctx, xdsSrv, s.cfg.OtlpCollectorAddress)
209-
210-
xdsSrv.SetExtProcMaxRequests(s.cfg.extProcMaxRequests())
211-
if parkCfg.enabled() {
212-
// Envoy must keep a parked request open at least as long as the router
213-
// will hold it; add a margin so the router surfaces its own 503 first.
214-
xdsSrv.SetExtProcMessageTimeout(parkCfg.Budget + 5*time.Second)
215-
}
216-
217-
xdsSrv.SetTlsConfig(s.cfg.HttpsPort, s.cfg.EnvoyCertPath)
218-
xdsSrv.SetUpstreamTls(s.cfg.UpstreamCredentialBundlePath, s.cfg.UpstreamTrustBundlePath, s.cfg.UpstreamSpiffePrefix)
219-
ctrl := NewController(s.k8sClient, s.clientset, s.cfg, xdsSrv, s.extprocSrv)
220-
221-
// Envoy receives all routing configuration from the local xDS server.
222-
g.Go(func() error {
223-
slog.InfoContext(ctx, "Starting ActorTemplate controller")
224-
return ctrl.Start(ctx)
225-
})
226-
g.Go(func() error {
227-
slog.InfoContext(ctx, "Starting Envoy xDS Server", slog.Int("port", s.cfg.XdsPort))
228-
lis, err := net.Listen("tcp", fmt.Sprintf(":%d", s.cfg.XdsPort))
229-
if err != nil {
230-
return fmt.Errorf("failed to listen on port %d: %w", s.cfg.XdsPort, err)
231-
}
232-
defer lis.Close()
233-
234-
return xdsSrv.Serve(ctx, lis)
235-
})
236-
case atenetRouterAgentgateway:
237-
// Agentgateway receives all routing configuration from its static file.
202+
if err := s.startDataplane(ctx, g, parkCfg); err != nil {
203+
return err
238204
}
239205

240206
// Start periodic service checking logic

cmd/atenet/internal/router/status.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ func (qr *QueryRecorder) AddRouterRequest(
119119
) {
120120
qr.Add(RecordedQuery{
121121
Timestamp: start,
122-
Client: m.headers[":authority"],
122+
Client: m.headers[authorityHeader],
123123
Host: m.host,
124124
Path: redactPath(m.path),
125125
Method: m.headers[":method"],

internal/atunnel/server.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -269,12 +269,12 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
269269

270270
// Do not expose the router-only routing header to actor code. Restore Host
271271
// so dataplanes that route dynamically on worker IP still give the actor its
272-
// stable mesh hostname.
272+
// stable actor DNS name.
273273
r.Header.Del(OriginalHostHeader)
274274
r.Host = actorHost
275275

276276
// ReverseProxy changes the URL destination but intentionally retains Host,
277-
// allowing the actor application to observe its stable mesh hostname.
277+
// allowing the actor application to observe its stable actor DNS name.
278278
s.proxy.ServeHTTP(w, r.WithContext(requestCtx))
279279
}
280280

0 commit comments

Comments
 (0)