From c23243b8eb6486d3e880adb7957680e0f003966a Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Wed, 17 Dec 2025 10:20:54 -0700 Subject: [PATCH] Fix hostNetwork data plane pod connection issue Problem: When hostNetwork was enabled, the data plane pod would connect and give us its hostname, which was the name of the node instead of the pod. This would cause issues internally with both tracking and acquiring the pod's owner so we know which config to send to it. Solution: Add the owner's name and type via labels in the agent config. These labels are then sent to the control plane when an agent connects, and can be used to determine which configuration to send. Updated all trackin to use the UUID instead of pod name. --- internal/controller/nginx/agent/command.go | 248 +++++++------- .../controller/nginx/agent/command_test.go | 318 ++++-------------- internal/controller/nginx/agent/file.go | 20 +- internal/controller/nginx/agent/file_test.go | 24 +- .../nginx/agent/grpc/connections.go | 4 +- .../nginx/agent/grpc/connections_test.go | 15 +- .../agent/grpc/interceptor/interceptor.go | 10 +- .../grpc/interceptor/interceptor_test.go | 14 +- internal/controller/nginx/types/types.go | 11 + internal/controller/provisioner/objects.go | 15 +- .../controller/provisioner/objects_test.go | 13 + .../controller/provisioner/provisioner.go | 3 + 12 files changed, 259 insertions(+), 436 deletions(-) diff --git a/internal/controller/nginx/agent/command.go b/internal/controller/nginx/agent/command.go index af4a254a47..fdafa31fbc 100644 --- a/internal/controller/nginx/agent/command.go +++ b/internal/controller/nginx/agent/command.go @@ -17,7 +17,6 @@ import ( "google.golang.org/protobuf/types/known/timestamppb" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" "sigs.k8s.io/controller-runtime/pkg/client" @@ -26,6 +25,7 @@ import ( agentgrpc "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/agent/grpc" grpcContext "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/agent/grpc/context" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/agent/grpc/messenger" + nginxTypes "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/types" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/status" ) @@ -77,7 +77,7 @@ func (cs *commandService) CreateConnection( return nil, errors.New("empty connection request") } - gi, ok := grpcContext.FromContext(ctx) + grpcInfo, ok := grpcContext.FromContext(ctx) if !ok { return nil, agentgrpc.ErrStatusInvalidConnection } @@ -86,8 +86,9 @@ func (cs *commandService) CreateConnection( podName := resource.GetContainerInfo().GetHostname() cs.logger.Info(fmt.Sprintf("Creating connection for nginx pod: %s", podName)) - owner, _, err := cs.getPodOwner(podName) - if err != nil { + name, depType := getAgentDeploymentNameAndType(resource.GetInstances()) + if name == (types.NamespacedName{}) || depType == "" { + err := errors.New("agent labels missing") response := &pb.CreateConnectionResponse{ Response: &pb.CommandResponse{ Status: pb.CommandResponse_COMMAND_STATUS_ERROR, @@ -96,15 +97,15 @@ func (cs *commandService) CreateConnection( }, } cs.logger.Error(err, "error getting pod owner") - return response, grpcStatus.Errorf(codes.Internal, "error getting pod owner %s", err.Error()) + return response, grpcStatus.Errorf(codes.InvalidArgument, "error getting pod owner: %s", err.Error()) } conn := agentgrpc.Connection{ - Parent: owner, - PodName: podName, + ParentName: name, + ParentType: depType, InstanceID: getNginxInstanceID(resource.GetInstances()), } - cs.connTracker.Track(gi.UUID, conn) + cs.connTracker.Track(grpcInfo.UUID, conn) return &pb.CreateConnectionResponse{ Response: &pb.CommandResponse{ @@ -126,27 +127,31 @@ func (cs *commandService) CreateConnection( func (cs *commandService) Subscribe(in pb.CommandService_SubscribeServer) error { ctx := in.Context() - gi, ok := grpcContext.FromContext(ctx) + grpcInfo, ok := grpcContext.FromContext(ctx) if !ok { return agentgrpc.ErrStatusInvalidConnection } - defer cs.connTracker.RemoveConnection(gi.UUID) + defer cs.connTracker.RemoveConnection(grpcInfo.UUID) // wait for the agent to report itself and nginx - conn, deployment, err := cs.waitForConnection(ctx, gi) + conn, deployment, err := cs.waitForConnection(ctx, grpcInfo) if err != nil { cs.logger.Error(err, "error waiting for connection") return err } - defer deployment.RemovePodStatus(conn.PodName) + defer deployment.RemovePodStatus(grpcInfo.UUID) - cs.logger.Info(fmt.Sprintf("Successfully connected to nginx agent %s", conn.PodName)) + cs.logger.Info( + "Successfully connected to nginx agent", + conn.ParentType, conn.ParentName, + "uuid", grpcInfo.UUID, + ) msgr := messenger.New(in) go msgr.Run(ctx) // apply current config before starting event loop - if err := cs.setInitialConfig(ctx, deployment, conn, msgr); err != nil { + if err := cs.setInitialConfig(ctx, &grpcInfo, deployment, conn, msgr); err != nil { return err } @@ -188,7 +193,7 @@ func (cs *commandService) Subscribe(in pb.CommandService_SubscribeServer) error cs.logger.V(1).Info("Sending configuration to agent", "requestType", msg.Type) if err := msgr.Send(ctx, req); err != nil { cs.logger.Error(err, "error sending request to agent") - deployment.SetPodErrorStatus(conn.PodName, err) + deployment.SetPodErrorStatus(grpcInfo.UUID, err) channels.ResponseCh <- struct{}{} return grpcStatus.Error(codes.Internal, err.Error()) @@ -198,8 +203,8 @@ func (cs *commandService) Subscribe(in pb.CommandService_SubscribeServer) error // Only broadcast operations should signal ResponseCh for coordination. pendingBroadcastRequest = &msg case err = <-msgr.Errors(): - cs.logger.Error(err, "connection error", "pod", conn.PodName) - deployment.SetPodErrorStatus(conn.PodName, err) + cs.logger.Error(err, "connection error", conn.ParentType, conn.ParentName, "uuid", grpcInfo.UUID) + deployment.SetPodErrorStatus(grpcInfo.UUID, err) select { case channels.ResponseCh <- struct{}{}: default: @@ -220,9 +225,9 @@ func (cs *commandService) Subscribe(in pb.CommandService_SubscribeServer) error continue } err := fmt.Errorf("msg: %s; error: %s", res.GetMessage(), res.GetError()) - deployment.SetPodErrorStatus(conn.PodName, err) + deployment.SetPodErrorStatus(grpcInfo.UUID, err) } else { - deployment.SetPodErrorStatus(conn.PodName, nil) + deployment.SetPodErrorStatus(grpcInfo.UUID, nil) } // Signal broadcast completion only for tracked broadcast operations. @@ -231,7 +236,11 @@ func (cs *commandService) Subscribe(in pb.CommandService_SubscribeServer) error pendingBroadcastRequest = nil channels.ResponseCh <- struct{}{} } else { - cs.logger.V(1).Info("Received response for non-broadcast request (likely initial config)", "pod", conn.PodName) + cs.logger.V(1).Info( + "Received response for non-broadcast request (likely initial config)", + conn.ParentType, conn.ParentName, + "uuid", grpcInfo.UUID, + ) } } } @@ -239,7 +248,7 @@ func (cs *commandService) Subscribe(in pb.CommandService_SubscribeServer) error func (cs *commandService) waitForConnection( ctx context.Context, - gi grpcContext.GrpcInfo, + grpcInfo grpcContext.GrpcInfo, ) (*agentgrpc.Connection, *Deployment, error) { ticker := time.NewTicker(time.Second) defer ticker.Stop() @@ -258,9 +267,9 @@ func (cs *commandService) waitForConnection( case <-timer.C: return nil, nil, err case <-ticker.C: - if conn := cs.connTracker.GetConnection(gi.UUID); conn.Ready() { + if conn := cs.connTracker.GetConnection(grpcInfo.UUID); conn.Ready() { // connection has been established, now ensure that the deployment exists in the store - if deployment := cs.nginxDeployments.Get(conn.Parent); deployment != nil { + if deployment := cs.nginxDeployments.Get(conn.ParentName); deployment != nil { return &conn, deployment, nil } err = deploymentStoreErr @@ -274,6 +283,7 @@ func (cs *commandService) waitForConnection( // setInitialConfig gets the initial configuration for this connection and applies it. func (cs *commandService) setInitialConfig( ctx context.Context, + grpcInfo *grpcContext.GrpcInfo, deployment *Deployment, conn *agentgrpc.Connection, msgr messenger.Messenger, @@ -281,23 +291,22 @@ func (cs *commandService) setInitialConfig( deployment.FileLock.Lock() defer deployment.FileLock.Unlock() - _, pod, err := cs.getPodOwner(conn.PodName) - if err != nil { - cs.logAndSendErrorStatus(deployment, conn, err) - - return grpcStatus.Error(codes.Internal, err.Error()) - } - if err := cs.validatePodImageVersion(pod, deployment.imageVersion); err != nil { - cs.logAndSendErrorStatus(deployment, conn, err) + if err := cs.validatePodImageVersion(conn.ParentName, conn.ParentType, deployment.imageVersion); err != nil { + cs.logAndSendErrorStatus(grpcInfo, deployment, conn, err) return grpcStatus.Errorf(codes.FailedPrecondition, "nginx image version validation failed: %s", err.Error()) } fileOverviews, configVersion := deployment.GetFileOverviews() - cs.logger.Info("Sending initial configuration to agent", "pod", conn.PodName, "configVersion", configVersion) + cs.logger.Info( + "Sending initial configuration to agent", + conn.ParentType, conn.ParentName, + "uuid", grpcInfo.UUID, + "configVersion", configVersion, + ) if err := msgr.Send(ctx, buildRequest(fileOverviews, conn.InstanceID, configVersion)); err != nil { - cs.logAndSendErrorStatus(deployment, conn, err) + cs.logAndSendErrorStatus(grpcInfo, deployment, conn, err) return grpcStatus.Error(codes.Internal, err.Error()) } @@ -321,7 +330,7 @@ func (cs *commandService) setInitialConfig( true, // poll immediately func(ctx context.Context) (bool, error) { if err := msgr.Send(ctx, buildPlusAPIRequest(action, conn.InstanceID)); err != nil { - cs.logAndSendErrorStatus(deployment, conn, err) + cs.logAndSendErrorStatus(grpcInfo, deployment, conn, err) return false, grpcStatus.Error(codes.Internal, err.Error()) } @@ -350,7 +359,7 @@ func (cs *commandService) setInitialConfig( cancel() } // send the status (error or nil) to the status queue - cs.logAndSendErrorStatus(deployment, conn, errors.Join(errs...)) + cs.logAndSendErrorStatus(grpcInfo, deployment, conn, errors.Join(errs...)) return nil } @@ -393,16 +402,25 @@ func (cs *commandService) waitForInitialConfigApply( // the full Deployment error status to the status queue. This ensures that any other Pod errors that already // exist on the Deployment are not overwritten. // If the error is nil, then we just enqueue the nil value and don't log it, which indicates success. -func (cs *commandService) logAndSendErrorStatus(deployment *Deployment, conn *agentgrpc.Connection, err error) { +func (cs *commandService) logAndSendErrorStatus( + grpcInfo *grpcContext.GrpcInfo, + deployment *Deployment, + conn *agentgrpc.Connection, + err error, +) { if err != nil { cs.logger.Error(err, "error sending request to agent") } else { - cs.logger.Info("Successfully configured nginx for new subscription", "pod", conn.PodName) + cs.logger.Info( + "Successfully configured nginx for new subscription", + conn.ParentType, conn.ParentName, + "uuid", grpcInfo.UUID, + ) } - deployment.SetPodErrorStatus(conn.PodName, err) + deployment.SetPodErrorStatus(grpcInfo.UUID, err) queueObj := &status.QueueObject{ - Deployment: conn.Parent, + Deployment: conn.ParentName, Error: deployment.GetConfigurationStatus(), UpdateType: status.UpdateAll, } @@ -454,99 +472,56 @@ func buildPlusAPIRequest(action *pb.NGINXPlusAction, instanceID string) *pb.Mana } } -func (cs *commandService) getPodOwner(podName string) (types.NamespacedName, *v1.Pod, error) { - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() - - var pods v1.PodList - listOpts := &client.ListOptions{ - FieldSelector: fields.SelectorFromSet(fields.Set{"metadata.name": podName}), - } - if err := cs.k8sReader.List(ctx, &pods, listOpts); err != nil { - return types.NamespacedName{}, nil, fmt.Errorf("error listing pods: %w", err) - } - - if len(pods.Items) == 0 { - return types.NamespacedName{}, nil, fmt.Errorf("no pods found with name %q", podName) - } - - if len(pods.Items) > 1 { - return types.NamespacedName{}, nil, fmt.Errorf("should only be one pod with name %q", podName) - } - pod := &pods.Items[0] - - podOwnerRefs := pod.GetOwnerReferences() - if len(podOwnerRefs) != 1 { - tooManyOwnersError := "expected one owner reference of the nginx Pod, got %d" - return types.NamespacedName{}, nil, fmt.Errorf(tooManyOwnersError, len(podOwnerRefs)) - } - - if podOwnerRefs[0].Kind != "ReplicaSet" && podOwnerRefs[0].Kind != "DaemonSet" { - err := fmt.Errorf("expected pod owner reference to be ReplicaSet or DaemonSet, got %s", podOwnerRefs[0].Kind) - return types.NamespacedName{}, nil, err - } - - if podOwnerRefs[0].Kind == "DaemonSet" { - return types.NamespacedName{Namespace: pod.Namespace, Name: podOwnerRefs[0].Name}, pod, nil - } - - var replicaSet appsv1.ReplicaSet - var replicaSetErr error - if err := wait.PollUntilContextCancel( - ctx, - 500*time.Millisecond, - true, /* poll immediately */ - func(ctx context.Context) (bool, error) { - if err := cs.k8sReader.Get( - ctx, - types.NamespacedName{Namespace: pod.Namespace, Name: podOwnerRefs[0].Name}, - &replicaSet, - ); err != nil { - replicaSetErr = err - return false, nil //nolint:nilerr // error is returned at the end - } - - return true, nil - }, - ); err != nil { - return types.NamespacedName{}, nil, fmt.Errorf("failed to get nginx Pod's ReplicaSet: %w", replicaSetErr) - } - - replicaOwnerRefs := replicaSet.GetOwnerReferences() - if len(replicaOwnerRefs) != 1 { - err := fmt.Errorf("expected one owner reference of the nginx ReplicaSet, got %d", len(replicaOwnerRefs)) - return types.NamespacedName{}, nil, err - } - - return types.NamespacedName{Namespace: pod.Namespace, Name: replicaOwnerRefs[0].Name}, pod, nil -} - // validatePodImageVersion checks if the pod's nginx container image version matches the expected version // from its deployment. Returns an error if versions don't match. func (cs *commandService) validatePodImageVersion( - pod *v1.Pod, + parent types.NamespacedName, + parentType string, expectedImage string, ) error { - var podNginxImage string + var nginxImage string + var found bool + + getNginxContainerImage := func(containers []v1.Container) (string, bool) { + for _, c := range containers { + if c.Name == "nginx" { + return c.Image, true + } + } + return "", false + } + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() - for _, container := range pod.Spec.Containers { - if container.Name == "nginx" { - podNginxImage = container.Image - break + switch parentType { + case nginxTypes.DaemonSetType: + ds := &appsv1.DaemonSet{} + if err := cs.k8sReader.Get(ctx, parent, ds); err != nil { + return fmt.Errorf("failed to get DaemonSet %s: %w", parent.String(), err) } + nginxImage, found = getNginxContainerImage(ds.Spec.Template.Spec.Containers) + case nginxTypes.DeploymentType: + deploy := &appsv1.Deployment{} + if err := cs.k8sReader.Get(ctx, parent, deploy); err != nil { + return fmt.Errorf("failed to get Deployment %s: %w", parent.String(), err) + } + nginxImage, found = getNginxContainerImage(deploy.Spec.Template.Spec.Containers) + default: + return fmt.Errorf("unknown parentType: %s", parentType) } - if podNginxImage == "" { - return fmt.Errorf("nginx container not found in pod %q", pod.Name) + + if !found { + return fmt.Errorf("nginx container not found in %s %q", parentType, parent.Name) } - // Compare images - if podNginxImage != expectedImage { - return fmt.Errorf("nginx image version mismatch: pod has %q but expected %q", podNginxImage, expectedImage) + if nginxImage != expectedImage { + return fmt.Errorf("nginx image version mismatch: has %q but expected %q", nginxImage, expectedImage) } - cs.logger.V(1).Info("Pod nginx image version validated successfully", - "podName", pod.Name, - "image", podNginxImage) + cs.logger.V(1).Info("nginx image version validated successfully", + "parent", parent.String(), + "image", nginxImage) return nil } @@ -562,7 +537,7 @@ func (cs *commandService) UpdateDataPlaneStatus( return nil, errors.New("empty UpdateDataPlaneStatus request") } - gi, ok := grpcContext.FromContext(ctx) + grpcInfo, ok := grpcContext.FromContext(ctx) if !ok { return nil, agentgrpc.ErrStatusInvalidConnection } @@ -572,7 +547,7 @@ func (cs *commandService) UpdateDataPlaneStatus( return nil, grpcStatus.Errorf(codes.InvalidArgument, "request does not contain nginx instanceID") } - cs.connTracker.SetInstanceID(gi.UUID, instanceID) + cs.connTracker.SetInstanceID(grpcInfo.UUID, instanceID) return &pb.UpdateDataPlaneStatusResponse{}, nil } @@ -589,6 +564,35 @@ func getNginxInstanceID(instances []*pb.Instance) string { return "" } +func getAgentDeploymentNameAndType(instances []*pb.Instance) (types.NamespacedName, string) { + var nsName types.NamespacedName + var depType string + + for _, instance := range instances { + instanceType := instance.GetInstanceMeta().GetInstanceType() + if instanceType == pb.InstanceMeta_INSTANCE_TYPE_AGENT { + labels := instance.GetInstanceConfig().GetAgentConfig().GetLabels() + + for _, label := range labels { + fields := label.GetFields() + + if val, ok := fields[nginxTypes.AgentOwnerNameLabel]; ok { + fullName := val.GetStringValue() + parts := strings.SplitN(fullName, "_", 2) + if len(parts) == 2 { + nsName = types.NamespacedName{Namespace: parts[0], Name: parts[1]} + } + } + if val, ok := fields[nginxTypes.AgentOwnerTypeLabel]; ok { + depType = val.GetStringValue() + } + } + } + } + + return nsName, depType +} + // UpdateDataPlaneHealth includes full health information about the data plane as reported by the agent. func (*commandService) UpdateDataPlaneHealth( context.Context, diff --git a/internal/controller/nginx/agent/command_test.go b/internal/controller/nginx/agent/command_test.go index fb14c47ea3..db2fda1cf6 100644 --- a/internal/controller/nginx/agent/command_test.go +++ b/internal/controller/nginx/agent/command_test.go @@ -11,6 +11,7 @@ import ( pb "github.com/nginx/agent/v3/api/grpc/mpi/v1" . "github.com/onsi/gomega" "google.golang.org/grpc" + "google.golang.org/protobuf/types/known/structpb" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -25,6 +26,7 @@ import ( grpcContext "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/agent/grpc/context" agentgrpcfakes "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/agent/grpc/grpcfakes" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/agent/grpc/messenger/messengerfakes" + nginxTypes "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/types" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/status" ) @@ -85,25 +87,19 @@ func createGrpcContextWithCancel() (context.Context, context.CancelFunc) { }), cancel } -func getDefaultPodList() []runtime.Object { - pod := &v1.PodList{ - Items: []v1.Pod{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "nginx-pod", - Namespace: "test", - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "ReplicaSet", - Name: "nginx-replicaset", - }, - }, - }, +func getDefaultResources() []runtime.Object { + deployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nginx-deployment", + Namespace: "test", + }, + Spec: appsv1.DeploymentSpec{ + Template: v1.PodTemplateSpec{ Spec: v1.PodSpec{ Containers: []v1.Container{ { - Image: "nginx:v1.0.0", Name: "nginx", + Image: "nginx:v1.0.0", }, }, }, @@ -111,20 +107,7 @@ func getDefaultPodList() []runtime.Object { }, } - replicaSet := &appsv1.ReplicaSet{ - ObjectMeta: metav1.ObjectMeta{ - Name: "nginx-replicaset", - Namespace: "test", - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "Deployment", - Name: "nginx-deployment", - }, - }, - }, - } - - return []runtime.Object{pod, replicaSet} + return []runtime.Object{deployment} } func TestCreateConnection(t *testing.T) { @@ -154,6 +137,33 @@ func TestCreateConnection(t *testing.T) { InstanceType: pb.InstanceMeta_INSTANCE_TYPE_NGINX, }, }, + { + InstanceMeta: &pb.InstanceMeta{ + InstanceType: pb.InstanceMeta_INSTANCE_TYPE_AGENT, + }, + InstanceConfig: &pb.InstanceConfig{ + Config: &pb.InstanceConfig_AgentConfig{ + AgentConfig: &pb.AgentConfig{ + Labels: []*structpb.Struct{ + { + Fields: map[string]*structpb.Value{ + nginxTypes.AgentOwnerNameLabel: { + Kind: &structpb.Value_StringValue{ + StringValue: "test_nginx-deployment", + }, + }, + nginxTypes.AgentOwnerTypeLabel: { + Kind: &structpb.Value_StringValue{ + StringValue: nginxTypes.DeploymentType, + }, + }, + }, + }, + }, + }, + }, + }, + }, }, }, }, @@ -192,7 +202,7 @@ func TestCreateConnection(t *testing.T) { Response: &pb.CommandResponse{ Status: pb.CommandResponse_COMMAND_STATUS_ERROR, Message: "error getting pod owner", - Error: "no pods found with name \"nginx-pod\"", + Error: "agent labels missing", }, }, errString: "error getting pod owner", @@ -208,7 +218,7 @@ func TestCreateConnection(t *testing.T) { var objs []runtime.Object if test.errString != "error getting pod owner" { - objs = getDefaultPodList() + objs = getDefaultResources() } fakeClient, err := createFakeK8sClient(objs...) g.Expect(err).ToNot(HaveOccurred()) @@ -236,8 +246,8 @@ func TestCreateConnection(t *testing.T) { g.Expect(connTracker.TrackCallCount()).To(Equal(1)) expConn := agentgrpc.Connection{ - Parent: types.NamespacedName{Namespace: "test", Name: "nginx-deployment"}, - PodName: "nginx-pod", + ParentName: types.NamespacedName{Namespace: "test", Name: "nginx-deployment"}, + ParentType: nginxTypes.DeploymentType, InstanceID: "nginx-id", } @@ -303,13 +313,13 @@ func TestSubscribe(t *testing.T) { connTracker := agentgrpcfakes.FakeConnectionsTracker{} conn := agentgrpc.Connection{ - Parent: types.NamespacedName{Namespace: "test", Name: "nginx-deployment"}, - PodName: "nginx-pod", + ParentName: types.NamespacedName{Namespace: "test", Name: "nginx-deployment"}, + ParentType: nginxTypes.DeploymentType, InstanceID: "nginx-id", } connTracker.GetConnectionReturns(conn) - fakeClient, err := createFakeK8sClient(getDefaultPodList()...) + fakeClient, err := createFakeK8sClient(getDefaultResources()...) g.Expect(err).ToNot(HaveOccurred()) store := NewDeploymentStore(&connTracker) @@ -332,7 +342,7 @@ func TestSubscribe(t *testing.T) { broadcaster.SubscribeReturns(subChannels) // set the initial files and actions to be applied by the Subscription - deployment := store.StoreWithBroadcaster(conn.Parent, broadcaster) + deployment := store.StoreWithBroadcaster(conn.ParentName, broadcaster) files := []File{ { Meta: &pb.FileMeta{ @@ -431,7 +441,7 @@ func TestSubscribe(t *testing.T) { g.Eventually(func() map[string]error { return deployment.podStatuses - }).Should(HaveKey("nginx-pod")) + }).Should(HaveKey("1234567")) cancel() @@ -439,7 +449,7 @@ func TestSubscribe(t *testing.T) { return <-errCh }).Should(MatchError(ContainSubstring("context canceled"))) - g.Expect(deployment.podStatuses).ToNot(HaveKey("nginx-pod")) + g.Expect(deployment.podStatuses).ToNot(HaveKey("1234567")) } func TestSubscribe_Reset(t *testing.T) { @@ -448,13 +458,13 @@ func TestSubscribe_Reset(t *testing.T) { connTracker := agentgrpcfakes.FakeConnectionsTracker{} conn := agentgrpc.Connection{ - Parent: types.NamespacedName{Namespace: "test", Name: "nginx-deployment"}, - PodName: "nginx-pod", + ParentName: types.NamespacedName{Namespace: "test", Name: "nginx-deployment"}, + ParentType: nginxTypes.DeploymentType, InstanceID: "nginx-id", } connTracker.GetConnectionReturns(conn) - fakeClient, err := createFakeK8sClient(getDefaultPodList()...) + fakeClient, err := createFakeK8sClient(getDefaultResources()...) g.Expect(err).ToNot(HaveOccurred()) store := NewDeploymentStore(&connTracker) @@ -478,7 +488,7 @@ func TestSubscribe_Reset(t *testing.T) { broadcaster.SubscribeReturns(subChannels) // set the initial files to be applied by the Subscription - deployment := store.StoreWithBroadcaster(conn.Parent, broadcaster) + deployment := store.StoreWithBroadcaster(conn.ParentName, broadcaster) files := []File{ { Meta: &pb.FileMeta{ @@ -612,7 +622,6 @@ func TestSetInitialConfig_Errors(t *testing.T) { setup func(msgr *messengerfakes.FakeMessenger, deployment *Deployment) name string errString string - podList []runtime.Object }{ { name: "error sending initial config", @@ -677,7 +686,7 @@ func TestSetInitialConfig_Errors(t *testing.T) { setup: func(_ *messengerfakes.FakeMessenger, deployment *Deployment) { deployment.SetImageVersion("nginx:v2.0.0") }, - errString: "nginx image version mismatch: pod has \"nginx:v1.0.0\" but expected \"nginx:v2.0.0\"", + errString: "nginx image version mismatch: has \"nginx:v1.0.0\" but expected \"nginx:v2.0.0\"", }, } @@ -689,12 +698,7 @@ func TestSetInitialConfig_Errors(t *testing.T) { connTracker := agentgrpcfakes.FakeConnectionsTracker{} msgr := &messengerfakes.FakeMessenger{} - podList := test.podList - if len(podList) == 0 { - podList = getDefaultPodList() - } - - fakeClient, err := createFakeK8sClient(podList...) + fakeClient, err := createFakeK8sClient(getDefaultResources()...) g.Expect(err).ToNot(HaveOccurred()) cs := newCommandService( @@ -707,9 +711,9 @@ func TestSetInitialConfig_Errors(t *testing.T) { ) conn := &agentgrpc.Connection{ - Parent: types.NamespacedName{Namespace: "test", Name: "nginx-deployment"}, - PodName: "nginx-pod", + ParentName: types.NamespacedName{Namespace: "test", Name: "nginx-deployment"}, InstanceID: "nginx-id", + ParentType: nginxTypes.DeploymentType, } deployment := newDeployment(&broadcastfakes.FakeBroadcaster{}) @@ -719,7 +723,7 @@ func TestSetInitialConfig_Errors(t *testing.T) { test.setup(msgr, deployment) } - err = cs.setInitialConfig(context.Background(), deployment, conn, msgr) + err = cs.setInitialConfig(context.Background(), &grpcContext.GrpcInfo{}, deployment, conn, msgr) g.Expect(err).To(HaveOccurred()) g.Expect(err.Error()).To(ContainSubstring(test.errString)) @@ -727,212 +731,6 @@ func TestSetInitialConfig_Errors(t *testing.T) { } } -func TestGetPodOwner(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - podName string - podList *v1.PodList - replicaSet *appsv1.ReplicaSet - errString string - expected types.NamespacedName - }{ - { - name: "successfully gets pod owner; ReplicaSet", - podName: "nginx-pod", - podList: &v1.PodList{ - Items: []v1.Pod{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "nginx-pod", - Namespace: "test", - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "ReplicaSet", - Name: "nginx-replicaset", - }, - }, - }, - }, - }, - }, - replicaSet: &appsv1.ReplicaSet{ - ObjectMeta: metav1.ObjectMeta{ - Name: "nginx-replicaset", - Namespace: "test", - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "Deployment", - Name: "nginx-deployment", - }, - }, - }, - }, - expected: types.NamespacedName{ - Namespace: "test", - Name: "nginx-deployment", - }, - }, - { - name: "successfully gets pod owner; DaemonSet", - podName: "nginx-pod", - podList: &v1.PodList{ - Items: []v1.Pod{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "nginx-pod", - Namespace: "test", - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "DaemonSet", - Name: "nginx-daemonset", - }, - }, - }, - }, - }, - }, - replicaSet: &appsv1.ReplicaSet{}, - expected: types.NamespacedName{ - Namespace: "test", - Name: "nginx-daemonset", - }, - }, - { - name: "error listing pods", - podName: "nginx-pod", - podList: &v1.PodList{}, - replicaSet: &appsv1.ReplicaSet{}, - errString: "no pods found", - }, - { - name: "multiple pods with same name", - podName: "nginx-pod", - podList: &v1.PodList{ - Items: []v1.Pod{ - {ObjectMeta: metav1.ObjectMeta{Namespace: "test", Name: "nginx-pod"}}, - {ObjectMeta: metav1.ObjectMeta{Namespace: "test2", Name: "nginx-pod"}}, - }, - }, - replicaSet: &appsv1.ReplicaSet{}, - errString: "should only be one pod with name", - }, - { - name: "pod owner reference is not ReplicaSet or DaemonSet", - podName: "nginx-pod", - podList: &v1.PodList{ - Items: []v1.Pod{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "nginx-pod", - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "Owner", - Name: "nginx-owner", - }, - }, - }, - }, - }, - }, - replicaSet: &appsv1.ReplicaSet{}, - errString: "expected pod owner reference to be ReplicaSet or DaemonSet", - }, - { - name: "pod has multiple owners", - podName: "nginx-pod", - podList: &v1.PodList{ - Items: []v1.Pod{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "nginx-pod", - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "ReplicaSet", - Name: "nginx-replicaset", - }, - { - Kind: "ReplicaSet", - Name: "nginx-replicaset2", - }, - }, - }, - }, - }, - }, - replicaSet: &appsv1.ReplicaSet{}, - errString: "expected one owner reference of the nginx Pod", - }, - { - name: "replicaSet has multiple owners", - podName: "nginx-pod", - podList: &v1.PodList{ - Items: []v1.Pod{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "nginx-pod", - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "ReplicaSet", - Name: "nginx-replicaset", - }, - }, - }, - }, - }, - }, - replicaSet: &appsv1.ReplicaSet{ - ObjectMeta: metav1.ObjectMeta{ - Name: "nginx-replicaset", - OwnerReferences: []metav1.OwnerReference{ - { - Kind: "Deployment", - Name: "nginx-deployment", - }, - { - Kind: "Deployment", - Name: "nginx-deployment2", - }, - }, - }, - }, - errString: "expected one owner reference of the nginx ReplicaSet", - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - t.Parallel() - g := NewWithT(t) - - fakeClient, err := createFakeK8sClient(test.podList, test.replicaSet) - g.Expect(err).ToNot(HaveOccurred()) - - cs := newCommandService( - logr.Discard(), - fakeClient, - NewDeploymentStore(nil), - nil, - status.NewQueue(), - nil, - ) - - owner, _, err := cs.getPodOwner(test.podName) - - if test.errString != "" { - g.Expect(err).To(HaveOccurred()) - g.Expect(err.Error()).To(ContainSubstring(test.errString)) - g.Expect(owner).To(Equal(types.NamespacedName{})) - return - } - - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(owner).To(Equal(test.expected)) - }) - } -} - func TestUpdateDataPlaneStatus(t *testing.T) { t.Parallel() diff --git a/internal/controller/nginx/agent/file.go b/internal/controller/nginx/agent/file.go index 035c96b77b..6f1e05a4d1 100644 --- a/internal/controller/nginx/agent/file.go +++ b/internal/controller/nginx/agent/file.go @@ -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 } @@ -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 } @@ -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 } @@ -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 } @@ -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") } @@ -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") } diff --git a/internal/controller/nginx/agent/file_test.go b/internal/controller/nginx/agent/file_test.go index 5e0c52d5bb..d56518ca55 100644 --- a/internal/controller/nginx/agent/file_test.go +++ b/internal/controller/nginx/agent/file_test.go @@ -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) @@ -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) @@ -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) @@ -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) @@ -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) @@ -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) @@ -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) @@ -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) diff --git a/internal/controller/nginx/agent/grpc/connections.go b/internal/controller/nginx/agent/grpc/connections.go index 0bae634ccc..3ae38924d6 100644 --- a/internal/controller/nginx/agent/grpc/connections.go +++ b/internal/controller/nginx/agent/grpc/connections.go @@ -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 diff --git a/internal/controller/nginx/agent/grpc/connections_test.go b/internal/controller/nginx/agent/grpc/connections_test.go index 24280d375f..06e3a47df0 100644 --- a/internal/controller/nginx/agent/grpc/connections_test.go +++ b/internal/controller/nginx/agent/grpc/connections_test.go @@ -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) @@ -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()) @@ -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()) @@ -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) @@ -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) diff --git a/internal/controller/nginx/agent/grpc/interceptor/interceptor.go b/internal/controller/nginx/agent/grpc/interceptor/interceptor.go index b9e6a485f7..b5b8a6c922 100644 --- a/internal/controller/nginx/agent/grpc/interceptor/interceptor.go +++ b/internal/controller/nginx/agent/grpc/interceptor/interceptor.go @@ -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) { @@ -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, }, } @@ -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 } diff --git a/internal/controller/nginx/agent/grpc/interceptor/interceptor_test.go b/internal/controller/nginx/agent/grpc/interceptor/interceptor_test.go index 68f3392a5b..5987da9194 100644 --- a/internal/controller/nginx/agent/grpc/interceptor/interceptor_test.go +++ b/internal/controller/nginx/agent/grpc/interceptor/interceptor_test.go @@ -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 }{ @@ -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, }, { @@ -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, }, { @@ -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, }, { @@ -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, }, { @@ -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, }, } @@ -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")) diff --git a/internal/controller/nginx/types/types.go b/internal/controller/nginx/types/types.go index 6883a4314c..c164e94baa 100644 --- a/internal/controller/nginx/types/types.go +++ b/internal/controller/nginx/types/types.go @@ -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" +) diff --git a/internal/controller/provisioner/objects.go b/internal/controller/provisioner/objects.go index cb4cdb6647..168097b634 100644 --- a/internal/controller/provisioner/objects.go +++ b/internal/controller/provisioner/objects.go @@ -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" @@ -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, @@ -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{ diff --git a/internal/controller/provisioner/objects_test.go b/internal/controller/provisioner/objects_test.go index e9aaa51fef..204b210d21 100644 --- a/internal/controller/provisioner/objects_test.go +++ b/internal/controller/provisioner/objects_test.go @@ -47,6 +47,7 @@ func TestBuildNginxResourceObjects(t *testing.T) { Image: "ngf-image", }, AgentTLSSecretName: agentTLSTestSecretName, + AgentLabels: make(map[string]string), }, baseLabelSelector: metav1.LabelSelector{ MatchLabels: map[string]string{ @@ -253,6 +254,7 @@ func TestBuildNginxResourceObjects_NginxProxyConfig(t *testing.T) { Version: "1.0.0", }, AgentTLSSecretName: agentTLSTestSecretName, + AgentLabels: make(map[string]string), }, baseLabelSelector: metav1.LabelSelector{ MatchLabels: map[string]string{ @@ -494,6 +496,7 @@ func TestBuildNginxResourceObjects_DeploymentReplicasFromHPA(t *testing.T) { Image: "ngf-image", }, AgentTLSSecretName: agentTLSTestSecretName, + AgentLabels: make(map[string]string), }, baseLabelSelector: metav1.LabelSelector{ MatchLabels: map[string]string{"app": "nginx"}, @@ -594,6 +597,7 @@ func TestBuildNginxResourceObjects_Plus(t *testing.T) { SkipVerify: true, }, AgentTLSSecretName: agentTLSTestSecretName, + AgentLabels: make(map[string]string), }, k8sClient: fakeClient, baseLabelSelector: metav1.LabelSelector{ @@ -747,6 +751,7 @@ func TestBuildNginxResourceObjects_DockerSecrets(t *testing.T) { }, NginxDockerSecretNames: []string{dockerTestSecretName, dockerSecretRegistry1Name, dockerSecretRegistry2Name}, AgentTLSSecretName: agentTLSTestSecretName, + AgentLabels: make(map[string]string), }, k8sClient: fakeClient, baseLabelSelector: metav1.LabelSelector{ @@ -838,6 +843,7 @@ func TestBuildNginxResourceObjects_DaemonSet(t *testing.T) { Namespace: ngfNamespace, }, AgentTLSSecretName: agentTLSTestSecretName, + AgentLabels: make(map[string]string), }, k8sClient: fakeClient, baseLabelSelector: metav1.LabelSelector{ @@ -924,6 +930,7 @@ func TestBuildNginxResourceObjects_OpenShift(t *testing.T) { Namespace: ngfNamespace, }, AgentTLSSecretName: agentTLSTestSecretName, + AgentLabels: make(map[string]string), }, k8sClient: fakeClient, baseLabelSelector: metav1.LabelSelector{ @@ -997,6 +1004,7 @@ func TestBuildNginxResourceObjects_DataplaneKeySecret(t *testing.T) { EndpointPort: 443, EndpointTLSSkipVerify: false, }, + AgentLabels: make(map[string]string), }, k8sClient: fakeClient, baseLabelSelector: metav1.LabelSelector{ @@ -1353,6 +1361,7 @@ func TestBuildNginxConfigMaps_WorkerConnections(t *testing.T) { Namespace: "default", ServiceName: "test-service", }, + AgentLabels: make(map[string]string), }, } objectMeta := metav1.ObjectMeta{Name: "test", Namespace: "default"} @@ -1422,6 +1431,8 @@ func TestBuildNginxConfigMaps_AgentFields(t *testing.T) { g.Expect(data).To(ContainSubstring("key1: val1")) g.Expect(data).To(ContainSubstring("key2: val2")) + g.Expect(data).To(ContainSubstring("owner-name: default_test")) + g.Expect(data).To(ContainSubstring("owner-type: Deployment")) g.Expect(data).To(ContainSubstring("host: console.example.com")) g.Expect(data).To(ContainSubstring("port: 443")) g.Expect(data).To(ContainSubstring("skip_verify: false")) @@ -1542,6 +1553,7 @@ func TestBuildNginxResourceObjects_Patches(t *testing.T) { Image: "ngf-image", }, AgentTLSSecretName: agentTLSTestSecretName, + AgentLabels: make(map[string]string), }, baseLabelSelector: metav1.LabelSelector{ MatchLabels: map[string]string{ @@ -1866,6 +1878,7 @@ func TestBuildNginxResourceObjects_InferenceExtension(t *testing.T) { InferenceExtension: true, EndpointPickerDisableTLS: true, EndpointPickerTLSSkipVerify: true, + AgentLabels: make(map[string]string), }, k8sClient: fakeClient, baseLabelSelector: metav1.LabelSelector{ diff --git a/internal/controller/provisioner/provisioner.go b/internal/controller/provisioner/provisioner.go index f7688e9ec1..43d9f5c513 100644 --- a/internal/controller/provisioner/provisioner.go +++ b/internal/controller/provisioner/provisioner.go @@ -146,6 +146,9 @@ func NewNginxProvisioner( cfg.Logger.Error(err, "failed to collect agent labels") } cfg.AgentLabels = agentLabels + if cfg.AgentLabels == nil { + cfg.AgentLabels = make(map[string]string) + } provisioner := &NginxProvisioner{ k8sClient: mgr.GetClient(),