From f314e0beebf17b7aef8817de89ad7a8ab93d72da Mon Sep 17 00:00:00 2001 From: zoezhao Date: Fri, 24 Jul 2026 16:54:29 -0700 Subject: [PATCH] atenet: pick up refreshed pod certificate by adding a WatchedDirectory. --- cmd/atenet/internal/router/xds.go | 57 ++++++++++---- cmd/atenet/internal/router/xds_test.go | 100 ++++++++++++++++++++++++- 2 files changed, 142 insertions(+), 15 deletions(-) diff --git a/cmd/atenet/internal/router/xds.go b/cmd/atenet/internal/router/xds.go index 3d52657af..544f17e2e 100644 --- a/cmd/atenet/internal/router/xds.go +++ b/cmd/atenet/internal/router/xds.go @@ -19,6 +19,7 @@ import ( "fmt" "log/slog" "net" + "path/filepath" "strconv" "sync" "time" @@ -52,6 +53,7 @@ import ( endpointgrpc "github.com/envoyproxy/go-control-plane/envoy/service/endpoint/v3" listenergrpc "github.com/envoyproxy/go-control-plane/envoy/service/listener/v3" routegrpc "github.com/envoyproxy/go-control-plane/envoy/service/route/v3" + secretgrpc "github.com/envoyproxy/go-control-plane/envoy/service/secret/v3" typev3 "github.com/envoyproxy/go-control-plane/envoy/type/v3" "github.com/envoyproxy/go-control-plane/pkg/cache/types" cachev3 "github.com/envoyproxy/go-control-plane/pkg/cache/v3" @@ -66,6 +68,7 @@ const ( RouteName = "substrate_routes" ClusterName = "ate-cluster" OtlpClusterName = "otel_collector_cluster" + HTTPSCertSecretName = "https_serving_cert" ) // XdsServer implements an aggregated discovery service server for dynamic Envoy router nodes. @@ -112,6 +115,9 @@ func (x *XdsServer) SetConfig(ingressPort int, extprocPort int, extprocAddr stri func (x *XdsServer) SetTlsConfig(httpsPort int, certPath string) { x.mu.Lock() defer x.mu.Unlock() + if httpsPort > 0 && certPath == "" { + slog.Warn("HTTPS port configured without a certificate path; the HTTPS listener will not be served", slog.Int("port", httpsPort)) + } x.httpsPort = httpsPort x.certPath = certPath } @@ -166,8 +172,10 @@ func (x *XdsServer) UpdateSnapshot() error { listeners := []types.Resource{ x.buildListener(), } - if x.httpsPort > 0 { + var secrets []types.Resource + if x.httpsPort > 0 && x.certPath != "" { listeners = append(listeners, x.buildHttpsListener()) + secrets = append(secrets, x.buildTlsSecret()) } // Snapshot @@ -175,6 +183,7 @@ func (x *XdsServer) UpdateSnapshot() error { resourcev3.ClusterType: clusters, resourcev3.RouteType: routes, resourcev3.ListenerType: listeners, + resourcev3.SecretType: secrets, }) if err != nil { @@ -203,6 +212,7 @@ func (x *XdsServer) Serve(ctx context.Context, lis net.Listener) error { endpointgrpc.RegisterEndpointDiscoveryServiceServer(grpcServer, x.srv) listenergrpc.RegisterListenerDiscoveryServiceServer(grpcServer, x.srv) routegrpc.RegisterRouteDiscoveryServiceServer(grpcServer, x.srv) + secretgrpc.RegisterSecretDiscoveryServiceServer(grpcServer, x.srv) errChan := make(chan error, 1) go func() { @@ -531,8 +541,16 @@ func (x *XdsServer) buildHttpsListener() *listenerv3.Listener { tlsConfig := &tlsv3.DownstreamTlsContext{ CommonTlsContext: &tlsv3.CommonTlsContext{ - TlsCertificates: []*tlsv3.TlsCertificate{ - x.buildTlsCertificate(), + TlsCertificateSdsSecretConfigs: []*tlsv3.SdsSecretConfig{ + { + Name: HTTPSCertSecretName, + SdsConfig: &corev3.ConfigSource{ + ConfigSourceSpecifier: &corev3.ConfigSource_Ads{ + Ads: &corev3.AggregatedConfigSource{}, + }, + ResourceApiVersion: corev3.ApiVersion_V3, + }, + }, }, }, } @@ -571,16 +589,29 @@ func (x *XdsServer) buildHttpsListener() *listenerv3.Listener { } } -func (x *XdsServer) buildTlsCertificate() *tlsv3.TlsCertificate { - return &tlsv3.TlsCertificate{ - CertificateChain: &corev3.DataSource{ - Specifier: &corev3.DataSource_Filename{ - Filename: x.certPath, - }, - }, - PrivateKey: &corev3.DataSource{ - Specifier: &corev3.DataSource_Filename{ - Filename: x.certPath, // Assuming combined file +func (x *XdsServer) buildTlsSecret() *tlsv3.Secret { + return &tlsv3.Secret{ + Name: HTTPSCertSecretName, + Type: &tlsv3.Secret_TlsCertificate{ + TlsCertificate: &tlsv3.TlsCertificate{ + // The pod certificate is projected as a single PEM bundle + // holding both the cert chain and the private key, so both + // DataSources point at the same file. + CertificateChain: &corev3.DataSource{ + Specifier: &corev3.DataSource_Filename{ + Filename: x.certPath, + }, + }, + PrivateKey: &corev3.DataSource{ + Specifier: &corev3.DataSource_Filename{ + Filename: x.certPath, + }, + }, + // By specifying WatchedDirectory, we tell envoy to watch changes to the mounted pod certificate file. + // See documentation in https://pkg.go.dev/github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/tls/v3#:~:text=This%20only%20applies%20when%20a%20%E2%80%9CTlsCertificate%E2%80%9C%20is%20delivered%20by%20SDS + WatchedDirectory: &corev3.WatchedDirectory{ + Path: filepath.Dir(x.certPath), + }, }, }, } diff --git a/cmd/atenet/internal/router/xds_test.go b/cmd/atenet/internal/router/xds_test.go index 9a2c2460f..01cfe917f 100644 --- a/cmd/atenet/internal/router/xds_test.go +++ b/cmd/atenet/internal/router/xds_test.go @@ -24,6 +24,7 @@ import ( clusterv3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3" listenerv3 "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3" routev3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" + tlsv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/tls/v3" cachev3 "github.com/envoyproxy/go-control-plane/pkg/cache/v3" resourcev3 "github.com/envoyproxy/go-control-plane/pkg/resource/v3" ) @@ -139,9 +140,11 @@ func TestXdsServer_UpdateSnapshot(t *testing.T) { } func TestXdsServer_UpdateSnapshot_WithHttps(t *testing.T) { + const certPath = "/run/servicedns.podcert.ate.dev/credential-bundle.pem" + server := NewXdsServer(18000) server.SetConfig(8085, 50053, "127.0.0.1") - server.SetTlsConfig(8443, "") + server.SetTlsConfig(8443, certPath) err := server.UpdateSnapshot() if err != nil { @@ -172,12 +175,105 @@ func TestXdsServer_UpdateSnapshot_WithHttps(t *testing.T) { t.Errorf("Expected port 8443, got %d", sa.GetPortValue()) } - // Verify TLS config + // Verify the TLS config references the serving cert via SDS rather + // than embedding it: inline filename DataSources are read only once + // at listener creation, so rotations would never be picked up. fc := l.GetFilterChains()[0] ts := fc.GetTransportSocket() if ts.GetName() != "envoy.transport_sockets.tls" { t.Errorf("Expected transport socket 'envoy.transport_sockets.tls', got '%s'", ts.GetName()) } + dtc := &tlsv3.DownstreamTlsContext{} + if err := ts.GetTypedConfig().UnmarshalTo(dtc); err != nil { + t.Fatalf("Failed to unmarshal DownstreamTlsContext: %v", err) + } + if got := dtc.GetCommonTlsContext().GetTlsCertificates(); len(got) != 0 { + t.Errorf("Expected no inline TlsCertificates, got %d", len(got)) + } + sds := dtc.GetCommonTlsContext().GetTlsCertificateSdsSecretConfigs() + if len(sds) != 1 { + t.Fatalf("Expected 1 SDS secret config, got %d", len(sds)) + } + if sds[0].GetName() != HTTPSCertSecretName { + t.Errorf("Expected SDS secret name '%s', got '%s'", HTTPSCertSecretName, sds[0].GetName()) + } + if sds[0].GetSdsConfig().GetAds() == nil { + t.Error("Expected SDS config to use the ADS config source") + } + } + + // Verify the Secret resource carries the cert by filename with a watched + // directory, so Envoy re-reads the files when kubelet rotates the + // projected volume. + secretsMap := snap.GetResources(resourcev3.SecretType) + if len(secretsMap) != 1 { + t.Fatalf("Expected 1 secret definition, got %d", len(secretsMap)) + } + raw, exists := secretsMap[HTTPSCertSecretName] + if !exists { + t.Fatalf("Secret '%s' is missing from snapshot secrets", HTTPSCertSecretName) + } + secret := raw.(*tlsv3.Secret) + tlsCert := secret.GetTlsCertificate() + if got := tlsCert.GetCertificateChain().GetFilename(); got != certPath { + t.Errorf("Expected certificate chain filename '%s', got '%s'", certPath, got) + } + if got := tlsCert.GetPrivateKey().GetFilename(); got != certPath { + t.Errorf("Expected private key filename '%s', got '%s'", certPath, got) + } + if got, want := tlsCert.GetWatchedDirectory().GetPath(), "/run/servicedns.podcert.ate.dev"; got != want { + t.Errorf("Expected watched directory '%s', got '%s'", want, got) + } +} + +func TestXdsServer_UpdateSnapshot_HttpsWithoutCertPath(t *testing.T) { + server := NewXdsServer(18000) + server.SetConfig(8085, 50053, "127.0.0.1") + // This is the default flag combination: --port-https set, no + // --envoy-cert-path. An SDS secret with an empty filename would be + // NACKed by Envoy, so the HTTPS listener must be skipped entirely. + server.SetTlsConfig(8443, "") + + if err := server.UpdateSnapshot(); err != nil { + t.Fatalf("UpdateSnapshot failed: %v", err) + } + + res, err := server.snapshot.GetSnapshot(NodeID) + if err != nil { + t.Fatalf("Failed to get snapshot: %v", err) + } + snap, ok := res.(*cachev3.Snapshot) + if !ok { + t.Fatalf("Snapshot doesn't conform to type *cachev3.Snapshot, got %T", res) + } + + listenersMap := snap.GetResources(resourcev3.ListenerType) + if _, exists := listenersMap[IngressHTTPSListener]; exists { + t.Error("HTTPS listener must not be built without a cert path") + } + if len(listenersMap) != 1 { + t.Errorf("Expected only the HTTP listener without a cert path, got %d listeners", len(listenersMap)) + } + if got := snap.GetResources(resourcev3.SecretType); len(got) != 0 { + t.Errorf("Expected no secrets without a cert path, got %d", len(got)) + } +} + +func TestXdsServer_UpdateSnapshot_NoHttps_NoSecrets(t *testing.T) { + server := NewXdsServer(18000) + server.SetConfig(8085, 50053, "127.0.0.1") + + if err := server.UpdateSnapshot(); err != nil { + t.Fatalf("UpdateSnapshot failed: %v", err) + } + + res, err := server.snapshot.GetSnapshot(NodeID) + if err != nil { + t.Fatalf("Failed to get snapshot: %v", err) + } + snap := res.(*cachev3.Snapshot) + if got := snap.GetResources(resourcev3.SecretType); len(got) != 0 { + t.Errorf("Expected no secrets without TLS config, got %d", len(got)) } }