Validate agent token for duplicate IP addresses

internal/controller/nginx/agent/grpc/interceptor/interceptor.go

@@ -16,6 +16,7 @@ import (
 authv1 "k8s.io/api/authentication/v1"
 corev1 "k8s.io/api/core/v1"
 "k8s.io/apimachinery/pkg/fields"
+"k8s.io/apimachinery/pkg/labels"
 "sigs.k8s.io/controller-runtime/pkg/client"
 
 grpcContext "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/agent/grpc/context"
@@ -160,35 +161,25 @@ func (c ContextSetter) validateToken(ctx context.Context, gi *grpcContext.GrpcIn
 var podList corev1.PodList
 opts := &client.ListOptions{
 FieldSelector: fields.SelectorFromSet(fields.Set{"status.podIP": gi.IPAddress}),
+Namespace: usernameItems[2],
+LabelSelector: labels.Set(map[string]string{
+controller.AppNameLabel: usernameItems[3],
+}).AsSelector(),
 }
 
 if err := c.k8sClient.List(getCtx, &podList, opts); err != nil {
 return nil, status.Error(codes.Internal, fmt.Sprintf("error listing pods: %s", err.Error()))
 }
 
-if len(podList.Items) != 1 {
-msg := fmt.Sprintf("expected one Pod to have IP address %s, found %d", gi.IPAddress, len(podList.Items))
-return nil, status.Error(codes.Internal, msg)
-}
-
-podNS := podList.Items[0].GetNamespace()
-if podNS != usernameItems[2] {
-msg := fmt.Sprintf(
-"token user namespace %q does not match namespace of requesting pod %q", usernameItems[2], podNS,
-)
-return nil, status.Error(codes.Unauthenticated, msg)
-}
-
-scName, ok := podList.Items[0].GetLabels()[controller.AppNameLabel]
-if !ok {
-msg := fmt.Sprintf("could not get app name from %q label; unable to authenticate token", controller.AppNameLabel)
-return nil, status.Error(codes.Unauthenticated, msg)
+runningCount := 0
+for _, pod := range podList.Items {
+if pod.Status.Phase == corev1.PodRunning {
+runningCount++
+}
 }
 
-if scName != usernameItems[3] {
-msg := fmt.Sprintf(
-"token user name %q does not match service account name of requesting pod %q", usernameItems[3], scName,
-)
+if runningCount != 1 {
+msg := fmt.Sprintf("expected a single Running pod with IP address %q, but found %d", gi.IPAddress, runningCount)
 return nil, status.Error(codes.Unauthenticated, msg)
 }
 

internal/controller/nginx/agent/grpc/interceptor/interceptor_test.go

@@ -17,7 +17,9 @@ import (
 corev1 "k8s.io/api/core/v1"
 metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 "sigs.k8s.io/controller-runtime/pkg/client"
+"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
+grpcContext "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/agent/grpc/context"
 "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/controller"
 )
 
@@ -67,6 +69,7 @@ func (m *mockClient) List(_ context.Context, obj client.ObjectList, _ ...client.
 Namespace: m.podNamespace,
 Labels: labels,
 },
+Status: corev1.PodStatus{Phase: corev1.PodRunning},
 },
 }
 
@@ -204,38 +207,6 @@ func TestInterceptor(t *testing.T) {
 expErrCode: codes.Unauthenticated,
 expErrMsg: "must be of the format",
 },
-{
-name: "mismatched namespace in username",
-md: validMetadata,
-peer: validPeerData,
-username: "system:serviceaccount:invalid:gateway-nginx",
-appName: "gateway-nginx",
-podNamespace: "default",
-authenticated: true,
-expErrCode: codes.Unauthenticated,
-expErrMsg: "does not match namespace",
-},
-{
-name: "mismatched name in username",
-md: validMetadata,
-peer: validPeerData,
-username: "system:serviceaccount:default:invalid",
-appName: "gateway-nginx",
-podNamespace: "default",
-authenticated: true,
-expErrCode: codes.Unauthenticated,
-expErrMsg: "does not match service account name",
-},
-{
-name: "missing app name label",
-md: validMetadata,
-peer: validPeerData,
-username: "system:serviceaccount:default:gateway-nginx",
-podNamespace: "default",
-authenticated: true,
-expErrCode: codes.Unauthenticated,
-expErrMsg: "could not get app name",
-},
 }
 
 streamHandler := func(_ any, _ grpc.ServerStream) error {
@@ -261,7 +232,7 @@ func TestInterceptor(t *testing.T) {
 }
 cs := NewContextSetter(mockK8sClient, "ngf-audience")
 
-ctx := context.Background()
+ctx := t.Context()
 if test.md != nil {
 peerCtx := context.Background()
 if test.peer != nil {
@@ -290,3 +261,147 @@ func TestInterceptor(t *testing.T) {
 })
 }
 }
+
+type patchClient struct {
+client.Client
+}
+
+func (p *patchClient) Create(_ context.Context, obj client.Object, _ ...client.CreateOption) error {
+tr, ok := obj.(*authv1.TokenReview)
+if ok {
+tr.Status.Authenticated = true
+tr.Status.User.Username = "system:serviceaccount:default:gateway-nginx"
+}
+return nil
+}
+
+func TestValidateToken_PodListOptions(t *testing.T) {
+t.Parallel()
+
+testCases := []struct {
+pod *corev1.Pod
+gi *grpcContext.GrpcInfo
+name string
+shouldErr bool
+}{
+{
+name: "all match",
+pod: &corev1.Pod{
+ObjectMeta: metav1.ObjectMeta{
+Name: "nginx-pod",
+Namespace: "default",
+Labels: map[string]string{
+controller.AppNameLabel: "gateway-nginx",
+},
+},
+Status: corev1.PodStatus{PodIP: "1.2.3.4", Phase: corev1.PodRunning},
+},
+gi: &grpcContext.GrpcInfo{Token: "dummy-token", IPAddress: "1.2.3.4"},
+shouldErr: false,
+},
+{
+name: "ip matches, namespace does not",
+pod: &corev1.Pod{
+ObjectMeta: metav1.ObjectMeta{
+Name: "nginx-pod",
+Namespace: "other-namespace",
+Labels: map[string]string{
+controller.AppNameLabel: "gateway-nginx",
+},
+},
+Status: corev1.PodStatus{PodIP: "1.2.3.4", Phase: corev1.PodRunning},
+},
+gi: &grpcContext.GrpcInfo{Token: "dummy-token", IPAddress: "1.2.3.4"},
+shouldErr: true,
+},
+{
+name: "ip matches, label value does not match",
+pod: &corev1.Pod{
+ObjectMeta: metav1.ObjectMeta{
+Name: "nginx-pod",
+Namespace: "default",
+Labels: map[string]string{
+controller.AppNameLabel: "not-gateway-nginx",
+},
+},
+Status: corev1.PodStatus{PodIP: "1.2.3.4", Phase: corev1.PodRunning},
+},
+gi: &grpcContext.GrpcInfo{Token: "dummy-token", IPAddress: "1.2.3.4"},
+shouldErr: true,
+},
+{
+name: "ip matches, label does not exist",
+pod: &corev1.Pod{
+ObjectMeta: metav1.ObjectMeta{
+Name: "nginx-pod",
+Namespace: "default",
+Labels: map[string]string{},
+},
+Status: corev1.PodStatus{PodIP: "1.2.3.4", Phase: corev1.PodRunning},
+},
+gi: &grpcContext.GrpcInfo{Token: "dummy-token", IPAddress: "1.2.3.4"},
+shouldErr: true,
+},
+{
+name: "ip does not match",
+pod: &corev1.Pod{
+ObjectMeta: metav1.ObjectMeta{
+Name: "nginx-pod",
+Namespace: "default",
+Labels: map[string]string{
+controller.AppNameLabel: "gateway-nginx",
+},
+},
+Status: corev1.PodStatus{PodIP: "1.2.3.4", Phase: corev1.PodRunning},
+},
+gi: &grpcContext.GrpcInfo{Token: "dummy-token", IPAddress: "9.9.9.9"},
+shouldErr: true,
+},
+{
+name: "all match but pod not running",
+pod: &corev1.Pod{
+ObjectMeta: metav1.ObjectMeta{
+Name: "nginx-pod",
+Namespace: "default",
+Labels: map[string]string{
+controller.AppNameLabel: "gateway-nginx",
+},
+},
+Status: corev1.PodStatus{PodIP: "1.2.3.4", Phase: corev1.PodPending},
+},
+gi: &grpcContext.GrpcInfo{Token: "dummy-token", IPAddress: "1.2.3.4"},
+shouldErr: true,
+},
+}
+
+for _, tc := range testCases {
+t.Run(tc.name, func(t *testing.T) {
+t.Parallel()
+g := NewWithT(t)
+
+fakeClient := fake.NewClientBuilder().
+WithObjects(tc.pod).
+WithIndex(&corev1.Pod{}, "status.podIP", func(obj client.Object) []string {
+pod, ok := obj.(*corev1.Pod)
+g.Expect(ok).To(BeTrue())
+if pod.Status.PodIP != "" {
+return []string{pod.Status.PodIP}
+}
+return nil
+}).
+Build()
+
+patchedClient := &patchClient{fakeClient}
+csPatched := NewContextSetter(patchedClient, "ngf-audience")
+
+resultCtx, err := csPatched.validateToken(t.Context(), tc.gi)
+if tc.shouldErr {
+g.Expect(err).To(HaveOccurred())
+g.Expect(err.Error()).To(ContainSubstring("expected a single Running pod"))
+} else {
+g.Expect(err).ToNot(HaveOccurred())
+g.Expect(resultCtx).ToNot(BeNil())
+}
+})
+}
+}