Skip to content

Fix hostNetwork data plane pod connection issue #4481

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sjberman merged 1 commit into from
Dec 17, 2025
Merged

Fix hostNetwork data plane pod connection issue #4481

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
248 changes: 126 additions & 122 deletions internal/controller/nginx/agent/command.go
View file
Open in desktop

Large diffs are not rendered by default.

318 changes: 58 additions & 260 deletions internal/controller/nginx/agent/command_test.go
View file
Open in desktop

Large diffs are not rendered by default.

20 changes: 10 additions & 10 deletions internal/controller/nginx/agent/file.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func (fs *fileService) GetFile(
ctx context.Context,
req *pb.GetFileRequest,
) (*pb.GetFileResponse, error) {
gi, ok := grpcContext.FromContext(ctx)
grpcInfo, ok := grpcContext.FromContext(ctx)
if !ok {
return nil, agentgrpc.ErrStatusInvalidConnection
}
Expand All @@ -65,7 +65,7 @@ func (fs *fileService) GetFile(
return nil, status.Error(codes.InvalidArgument, "invalid request")
}

contents, err := fs.getFileContents(req, gi.UUID)
contents, err := fs.getFileContents(req, grpcInfo.UUID)
if err != nil {
return nil, err
}
Expand All @@ -84,7 +84,7 @@ func (fs *fileService) GetFileStream(
req *pb.GetFileRequest,
server grpc.ServerStreamingServer[pb.FileDataChunk],
) error {
gi, ok := grpcContext.FromContext(server.Context())
grpcInfo, ok := grpcContext.FromContext(server.Context())
if !ok {
return agentgrpc.ErrStatusInvalidConnection
}
Expand All @@ -93,7 +93,7 @@ func (fs *fileService) GetFileStream(
return status.Error(codes.InvalidArgument, "invalid request")
}

contents, err := fs.getFileContents(req, gi.UUID)
contents, err := fs.getFileContents(req, grpcInfo.UUID)
if err != nil {
return err
}
Expand Down Expand Up @@ -133,11 +133,11 @@ func (fs *fileService) GetFileStream(

func (fs *fileService) getFileContents(req *pb.GetFileRequest, connKey string) ([]byte, error) {
conn := fs.connTracker.GetConnection(connKey)
if conn.PodName == "" {
if conn.InstanceID == "" {
return nil, status.Errorf(codes.NotFound, "connection not found")
}

deployment := fs.nginxDeployments.Get(conn.Parent)
deployment := fs.nginxDeployments.Get(conn.ParentName)
if deployment == nil {
return nil, status.Errorf(codes.NotFound, "deployment not found in store")
}
Expand Down Expand Up @@ -187,17 +187,17 @@ func (fs *fileService) UpdateOverview(
ctx context.Context,
req *pb.UpdateOverviewRequest,
) (*pb.UpdateOverviewResponse, error) {
gi, ok := grpcContext.FromContext(ctx)
grpcInfo, ok := grpcContext.FromContext(ctx)
if !ok {
return &pb.UpdateOverviewResponse{}, agentgrpc.ErrStatusInvalidConnection
}

conn := fs.connTracker.GetConnection(gi.UUID)
if conn.PodName == "" {
conn := fs.connTracker.GetConnection(grpcInfo.UUID)
if conn.InstanceID == "" {
return &pb.UpdateOverviewResponse{}, status.Errorf(codes.NotFound, "connection not found")
}

deployment := fs.nginxDeployments.Get(conn.Parent)
deployment := fs.nginxDeployments.Get(conn.ParentName)
if deployment == nil {
return &pb.UpdateOverviewResponse{}, status.Errorf(codes.NotFound, "deployment not found in store")
}
Expand Down
24 changes: 8 additions & 16 deletions internal/controller/nginx/agent/file_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,8 @@ func TestGetFile(t *testing.T) {

connTracker := &agentgrpcfakes.FakeConnectionsTracker{}
conn := agentgrpc.Connection{
PodName: "nginx-pod",
InstanceID: "12345",
Parent: deploymentName,
ParentName: deploymentName,
}
connTracker.GetConnectionReturns(conn)

Expand Down Expand Up @@ -109,9 +108,8 @@ func TestGetFile_InvalidRequest(t *testing.T) {
deploymentName := types.NamespacedName{Name: "nginx-deployment", Namespace: "default"}
connTracker := &agentgrpcfakes.FakeConnectionsTracker{}
conn := agentgrpc.Connection{
PodName: "nginx-pod",
InstanceID: "12345",
Parent: deploymentName,
ParentName: deploymentName,
}
connTracker.GetConnectionReturns(conn)

Expand Down Expand Up @@ -165,9 +163,8 @@ func TestGetFile_DeploymentNotFound(t *testing.T) {

connTracker := &agentgrpcfakes.FakeConnectionsTracker{}
conn := agentgrpc.Connection{
PodName: "nginx-pod",
InstanceID: "12345",
Parent: deploymentName,
ParentName: deploymentName,
}
connTracker.GetConnectionReturns(conn)

Expand Down Expand Up @@ -198,9 +195,8 @@ func TestGetFile_FileNotFound(t *testing.T) {

connTracker := &agentgrpcfakes.FakeConnectionsTracker{}
conn := agentgrpc.Connection{
PodName: "nginx-pod",
InstanceID: "12345",
Parent: deploymentName,
ParentName: deploymentName,
}
connTracker.GetConnectionReturns(conn)

Expand Down Expand Up @@ -234,9 +230,8 @@ func TestGetFileStream(t *testing.T) {

connTracker := &agentgrpcfakes.FakeConnectionsTracker{}
conn := agentgrpc.Connection{
PodName: "nginx-pod",
InstanceID: "12345",
Parent: deploymentName,
ParentName: deploymentName,
}
connTracker.GetConnectionReturns(conn)

Expand Down Expand Up @@ -312,9 +307,8 @@ func TestGetFileStream_InvalidRequest(t *testing.T) {
deploymentName := types.NamespacedName{Name: "nginx-deployment", Namespace: "default"}
connTracker := &agentgrpcfakes.FakeConnectionsTracker{}
conn := agentgrpc.Connection{
PodName: "nginx-pod",
InstanceID: "12345",
Parent: deploymentName,
ParentName: deploymentName,
}
connTracker.GetConnectionReturns(conn)

Expand Down Expand Up @@ -369,9 +363,8 @@ func TestUpdateOverview(t *testing.T) {

connTracker := &agentgrpcfakes.FakeConnectionsTracker{}
conn := agentgrpc.Connection{
PodName: "nginx-pod",
InstanceID: "12345",
Parent: deploymentName,
ParentName: deploymentName,
}
connTracker.GetConnectionReturns(conn)

Expand Down Expand Up @@ -504,9 +497,8 @@ func TestUpdateOverview_DeploymentNotFound(t *testing.T) {

connTracker := &agentgrpcfakes.FakeConnectionsTracker{}
conn := agentgrpc.Connection{
PodName: "nginx-pod",
InstanceID: "12345",
Parent: deploymentName,
ParentName: deploymentName,
}
connTracker.GetConnectionReturns(conn)

Expand Down
4 changes: 2 additions & 2 deletions internal/controller/nginx/agent/grpc/connections.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ type ConnectionsTracker interface {

// Connection contains the data about a single nginx agent connection.
type Connection struct {
PodName string
InstanceID string
Parent types.NamespacedName
ParentType string
ParentName types.NamespacedName
}

// Ready returns if the connection is ready to be used. In other words, agent
Expand Down
15 changes: 5 additions & 10 deletions internal/controller/nginx/agent/grpc/connections_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,8 @@ func TestGetConnection(t *testing.T) {
tracker := agentgrpc.NewConnectionsTracker()

conn := agentgrpc.Connection{
PodName: "pod1",
InstanceID: "instance1",
Parent: types.NamespacedName{Namespace: "default", Name: "parent1"},
ParentName: types.NamespacedName{Namespace: "default", Name: "parent1"},
}
tracker.Track("key1", conn)

Expand All @@ -34,9 +33,8 @@ func TestConnectionIsReady(t *testing.T) {
g := NewWithT(t)

conn := agentgrpc.Connection{
PodName: "pod1",
InstanceID: "instance1",
Parent: types.NamespacedName{Namespace: "default", Name: "parent1"},
ParentName: types.NamespacedName{Namespace: "default", Name: "parent1"},
}

g.Expect(conn.Ready()).To(BeTrue())
Expand All @@ -47,8 +45,7 @@ func TestConnectionIsNotReady(t *testing.T) {
g := NewWithT(t)

conn := agentgrpc.Connection{
PodName: "pod1",
Parent: types.NamespacedName{Namespace: "default", Name: "parent1"},
ParentName: types.NamespacedName{Namespace: "default", Name: "parent1"},
}

g.Expect(conn.Ready()).To(BeFalse())
Expand All @@ -60,8 +57,7 @@ func TestSetInstanceID(t *testing.T) {

tracker := agentgrpc.NewConnectionsTracker()
conn := agentgrpc.Connection{
PodName: "pod1",
Parent: types.NamespacedName{Namespace: "default", Name: "parent1"},
ParentName: types.NamespacedName{Namespace: "default", Name: "parent1"},
}
tracker.Track("key1", conn)

Expand All @@ -81,9 +77,8 @@ func TestRemoveConnection(t *testing.T) {

tracker := agentgrpc.NewConnectionsTracker()
conn := agentgrpc.Connection{
PodName: "pod1",
InstanceID: "instance1",
Parent: types.NamespacedName{Namespace: "default", Name: "parent1"},
ParentName: types.NamespacedName{Namespace: "default", Name: "parent1"},
}
tracker.Track("key1", conn)

Expand Down
10 changes: 5 additions & 5 deletions internal/controller/nginx/agent/grpc/interceptor/interceptor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,12 @@ func (c ContextSetter) Unary(logger logr.Logger) grpc.UnaryServerInterceptor {
// validateConnection checks that the connection is valid and returns a new
// context containing information used by the gRPC command/file services.
func (c ContextSetter) validateConnection(ctx context.Context) (context.Context, error) {
gi, err := getGrpcInfo(ctx)
grpcInfo, err := getGrpcInfo(ctx)
if err != nil {
return nil, err
}

return c.validateToken(ctx, gi)
return c.validateToken(ctx, grpcInfo)
}

func getGrpcInfo(ctx context.Context) (*grpcContext.GrpcInfo, error) {
Expand All @@ -114,11 +114,11 @@ func getGrpcInfo(ctx context.Context) (*grpcContext.GrpcInfo, error) {
}, nil
}

func (c ContextSetter) validateToken(ctx context.Context, gi *grpcContext.GrpcInfo) (context.Context, error) {
func (c ContextSetter) validateToken(ctx context.Context, grpcInfo *grpcContext.GrpcInfo) (context.Context, error) {
tokenReview := &authv1.TokenReview{
Spec: authv1.TokenReviewSpec{
Audiences: []string{c.audience},
Token: gi.Token,
Token: grpcInfo.Token,
},
}

Expand Down Expand Up @@ -169,5 +169,5 @@ func (c ContextSetter) validateToken(ctx context.Context, gi *grpcContext.GrpcIn
return nil, status.Error(codes.Unauthenticated, msg)
}

return grpcContext.NewGrpcContext(ctx, *gi), nil
return grpcContext.NewGrpcContext(ctx, *grpcInfo), nil
}
14 changes: 7 additions & 7 deletions internal/controller/nginx/agent/grpc/interceptor/interceptor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ func TestValidateToken_PodListOptions(t *testing.T) {

testCases := []struct {
pod *corev1.Pod
gi *grpcContext.GrpcInfo
grpcInfo *grpcContext.GrpcInfo
name string
shouldErr bool
}{
Expand All @@ -269,7 +269,7 @@ func TestValidateToken_PodListOptions(t *testing.T) {
},
Status: corev1.PodStatus{Phase: corev1.PodRunning},
},
gi: &grpcContext.GrpcInfo{Token: "dummy-token"},
grpcInfo: &grpcContext.GrpcInfo{Token: "dummy-token"},
shouldErr: false,
},
{
Expand All @@ -284,7 +284,7 @@ func TestValidateToken_PodListOptions(t *testing.T) {
},
Status: corev1.PodStatus{Phase: corev1.PodRunning},
},
gi: &grpcContext.GrpcInfo{Token: "dummy-token"},
grpcInfo: &grpcContext.GrpcInfo{Token: "dummy-token"},
shouldErr: true,
},
{
Expand All @@ -299,7 +299,7 @@ func TestValidateToken_PodListOptions(t *testing.T) {
},
Status: corev1.PodStatus{Phase: corev1.PodRunning},
},
gi: &grpcContext.GrpcInfo{Token: "dummy-token"},
grpcInfo: &grpcContext.GrpcInfo{Token: "dummy-token"},
shouldErr: true,
},
{
Expand All @@ -312,7 +312,7 @@ func TestValidateToken_PodListOptions(t *testing.T) {
},
Status: corev1.PodStatus{Phase: corev1.PodRunning},
},
gi: &grpcContext.GrpcInfo{Token: "dummy-token"},
grpcInfo: &grpcContext.GrpcInfo{Token: "dummy-token"},
shouldErr: true,
},
{
Expand All @@ -327,7 +327,7 @@ func TestValidateToken_PodListOptions(t *testing.T) {
},
Status: corev1.PodStatus{Phase: corev1.PodPending},
},
gi: &grpcContext.GrpcInfo{Token: "dummy-token"},
grpcInfo: &grpcContext.GrpcInfo{Token: "dummy-token"},
shouldErr: true,
},
}
Expand All @@ -352,7 +352,7 @@ func TestValidateToken_PodListOptions(t *testing.T) {
patchedClient := &patchClient{fakeClient}
csPatched := NewContextSetter(patchedClient, "ngf-audience")

resultCtx, err := csPatched.validateToken(t.Context(), tc.gi)
resultCtx, err := csPatched.validateToken(t.Context(), tc.grpcInfo)
if tc.shouldErr {
g.Expect(err).To(HaveOccurred())
g.Expect(err.Error()).To(ContainSubstring("no running pods"))
Expand Down
11 changes: 11 additions & 0 deletions internal/controller/nginx/types/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,14 @@ const (
// Nginx503Server is used as a backend for services that cannot be resolved (have no IP address).
Nginx503Server = "unix:/var/run/nginx/nginx-503-server.sock"
)

const (
// AgentOwnerNameLabel is the label key used to store the owner name of the nginx agent.
AgentOwnerNameLabel = "owner-name"
// AgentOwnerTypeLabel is the label key used to store the owner type of the nginx agent.
AgentOwnerTypeLabel = "owner-type"
// DaemonSetType is the value used to represent a DaemonSet owner type.
DaemonSetType = "DaemonSet"
// DeploymentType is the value used to represent a Deployment owner type.
DeploymentType = "Deployment"
)
15 changes: 11 additions & 4 deletions internal/controller/provisioner/objects.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (

ngfAPIv1alpha2 "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha2"
"github.com/nginx/nginx-gateway-fabric/v2/internal/controller/config"
nginxTypes "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/types"
"github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/dataplane"
"github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/graph"
"github.com/nginx/nginx-gateway-fabric/v2/internal/framework/controller"
Expand Down Expand Up @@ -451,6 +452,14 @@ func (p *NginxProvisioner) buildNginxConfigMaps(
metricsPort = *port
}

depType := nginxTypes.DeploymentType
if nProxyCfg != nil && nProxyCfg.Kubernetes != nil && nProxyCfg.Kubernetes.DaemonSet != nil {
depType = nginxTypes.DaemonSetType
}

p.cfg.AgentLabels[nginxTypes.AgentOwnerNameLabel] = fmt.Sprintf("%s_%s", objectMeta.Namespace, objectMeta.Name)
p.cfg.AgentLabels[nginxTypes.AgentOwnerTypeLabel] = depType

agentFields := map[string]interface{}{
"Plus": p.cfg.Plus,
"ServiceName": p.cfg.GatewayPodConfig.ServiceName,
Expand Down Expand Up @@ -865,10 +874,8 @@ func (p *NginxProvisioner) buildNginxPodTemplateSpec(
tokenAudience := fmt.Sprintf("%s.%s.svc", p.cfg.GatewayPodConfig.ServiceName, p.cfg.GatewayPodConfig.Namespace)

clusterID := "unknown"
if p.cfg.AgentLabels != nil {
if val, ok := p.cfg.AgentLabels["cluster-id"]; ok {
clusterID = val
}
if val, ok := p.cfg.AgentLabels["cluster-id"]; ok {
clusterID = val
}

spec := corev1.PodTemplateSpec{
Expand Down
Loading
Loading