From 2e6dbbccb6aed145ae8c353a1fa553d5e45e2a1a Mon Sep 17 00:00:00 2001 From: bjee19 <139261241+bjee19@users.noreply.github.com> Date: Wed, 5 Nov 2025 10:37:37 -0800 Subject: [PATCH] Fix bug where user mounted files were being removed by agent (#4178) Fix bug where user mounted files were being removed by agent. Problem: User mounted files are being removed by nginx agent. Solution: Mark user mounted files as unmanaged so nginx agent doesn't remove them. To do so we use agent's UpdateOverview function to get all files referenced in the nginx conf. We compare that with the user added volumeMounts and mark any files that were user mounted as unmanaged, ensuring nginx agent doesn't remove them. Testing: Added unit tests and manual testing. --- internal/controller/handler.go | 17 +- internal/controller/handler_test.go | 101 ++++++++++- internal/controller/nginx/agent/agent.go | 6 +- internal/controller/nginx/agent/agent_test.go | 9 +- .../agent/agentfakes/fake_nginx_updater.go | 24 ++- .../controller/nginx/agent/command_test.go | 4 +- internal/controller/nginx/agent/deployment.go | 23 ++- .../controller/nginx/agent/deployment_test.go | 87 ++++++++- internal/controller/nginx/agent/file.go | 35 +++- internal/controller/nginx/agent/file_test.go | 167 +++++++++++++++++- 10 files changed, 446 insertions(+), 27 deletions(-) diff --git a/internal/controller/handler.go b/internal/controller/handler.go index e2c08db82b..35fdec45ff 100644 --- a/internal/controller/handler.go +++ b/internal/controller/handler.go @@ -241,8 +241,20 @@ func (h *eventHandlerImpl) sendNginxConfig(ctx context.Context, logger logr.Logg h.setLatestConfiguration(gw, &cfg) + vm := []v1.VolumeMount{} + if gw.EffectiveNginxProxy != nil && + gw.EffectiveNginxProxy.Kubernetes != nil { + if gw.EffectiveNginxProxy.Kubernetes.Deployment != nil { + vm = gw.EffectiveNginxProxy.Kubernetes.Deployment.Container.VolumeMounts + } + + if gw.EffectiveNginxProxy.Kubernetes.DaemonSet != nil { + vm = gw.EffectiveNginxProxy.Kubernetes.DaemonSet.Container.VolumeMounts + } + } + deployment.FileLock.Lock() - h.updateNginxConf(deployment, cfg) + h.updateNginxConf(deployment, cfg, vm) deployment.FileLock.Unlock() configErr := deployment.GetLatestConfigError() @@ -454,9 +466,10 @@ func (h *eventHandlerImpl) parseAndCaptureEvent(ctx context.Context, logger logr func (h *eventHandlerImpl) updateNginxConf( deployment *agent.Deployment, conf dataplane.Configuration, + volumeMounts []v1.VolumeMount, ) { files := h.cfg.generator.Generate(conf) - h.cfg.nginxUpdater.UpdateConfig(deployment, files) + h.cfg.nginxUpdater.UpdateConfig(deployment, files, volumeMounts) // If using NGINX Plus, update upstream servers using the API. if h.cfg.plus { diff --git a/internal/controller/handler_test.go b/internal/controller/handler_test.go index 4c23a12e55..90f7ffae3c 100644 --- a/internal/controller/handler_test.go +++ b/internal/controller/handler_test.go @@ -22,6 +22,7 @@ import ( gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" ngfAPI "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" + "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha2" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/config" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/licensing/licensingfakes" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/metrics/collectors" @@ -66,7 +67,7 @@ var _ = Describe("eventHandler", func() { Expect(fakeGenerator.GenerateArgsForCall(0)).Should(Equal(expectedConf)) Expect(fakeNginxUpdater.UpdateConfigCallCount()).Should(Equal(1)) - _, files := fakeNginxUpdater.UpdateConfigArgsForCall(0) + _, files, _ := fakeNginxUpdater.UpdateConfigArgsForCall(0) Expect(expectedFiles).To(Equal(files)) Eventually( @@ -642,6 +643,104 @@ var _ = Describe("eventHandler", func() { Expect(handler.GetLatestConfiguration()).To(BeEmpty()) }) + + It("should process events with volume mounts from Deployment", func() { + // Create a gateway with EffectiveNginxProxy containing Deployment VolumeMounts + gatewayWithVolumeMounts := &graph.Graph{ + Gateways: map[types.NamespacedName]*graph.Gateway{ + {Namespace: "test", Name: "gateway"}: { + Valid: true, + Source: &gatewayv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway", + Namespace: "test", + }, + }, + DeploymentName: types.NamespacedName{ + Namespace: "test", + Name: controller.CreateNginxResourceName("gateway", "nginx"), + }, + EffectiveNginxProxy: &graph.EffectiveNginxProxy{ + Kubernetes: &v1alpha2.KubernetesSpec{ + Deployment: &v1alpha2.DeploymentSpec{ + Container: v1alpha2.ContainerSpec{ + VolumeMounts: []v1.VolumeMount{ + { + Name: "test-volume", + MountPath: "/etc/test", + }, + }, + }, + }, + }, + }, + }, + }, + } + + fakeProcessor.ProcessReturns(gatewayWithVolumeMounts) + + e := &events.UpsertEvent{Resource: &gatewayv1.HTTPRoute{}} + batch := []interface{}{e} + + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) + + // Verify that UpdateConfig was called with the volume mounts + Expect(fakeNginxUpdater.UpdateConfigCallCount()).Should(Equal(1)) + _, _, volumeMounts := fakeNginxUpdater.UpdateConfigArgsForCall(0) + Expect(volumeMounts).To(HaveLen(1)) + Expect(volumeMounts[0].Name).To(Equal("test-volume")) + Expect(volumeMounts[0].MountPath).To(Equal("/etc/test")) + }) + + It("should process events with volume mounts from DaemonSet", func() { + // Create a gateway with EffectiveNginxProxy containing DaemonSet VolumeMounts + gatewayWithVolumeMounts := &graph.Graph{ + Gateways: map[types.NamespacedName]*graph.Gateway{ + {Namespace: "test", Name: "gateway"}: { + Valid: true, + Source: &gatewayv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway", + Namespace: "test", + }, + }, + DeploymentName: types.NamespacedName{ + Namespace: "test", + Name: controller.CreateNginxResourceName("gateway", "nginx"), + }, + EffectiveNginxProxy: &graph.EffectiveNginxProxy{ + Kubernetes: &v1alpha2.KubernetesSpec{ + DaemonSet: &v1alpha2.DaemonSetSpec{ + Container: v1alpha2.ContainerSpec{ + VolumeMounts: []v1.VolumeMount{ + { + Name: "daemon-volume", + MountPath: "/var/daemon", + }, + }, + }, + }, + }, + }, + }, + }, + } + + fakeProcessor.ProcessReturns(gatewayWithVolumeMounts) + + e := &events.UpsertEvent{Resource: &gatewayv1.HTTPRoute{}} + batch := []interface{}{e} + + handler.HandleEventBatch(context.Background(), logr.Discard(), batch) + + // Verify that UpdateConfig was called with the volume mounts + Expect(fakeNginxUpdater.UpdateConfigCallCount()).Should(Equal(1)) + _, _, volumeMounts := fakeNginxUpdater.UpdateConfigArgsForCall(0) + Expect(volumeMounts).To(HaveLen(1)) + Expect(volumeMounts[0].Name).To(Equal("daemon-volume")) + Expect(volumeMounts[0].MountPath).To(Equal("/var/daemon")) + }) }) var _ = Describe("getGatewayAddresses", func() { diff --git a/internal/controller/nginx/agent/agent.go b/internal/controller/nginx/agent/agent.go index bdcc9efa9e..53d8a981b0 100644 --- a/internal/controller/nginx/agent/agent.go +++ b/internal/controller/nginx/agent/agent.go @@ -10,6 +10,7 @@ import ( "github.com/go-logr/logr" pb "github.com/nginx/agent/v3/api/grpc/mpi/v1" "google.golang.org/protobuf/types/known/structpb" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/wait" "sigs.k8s.io/controller-runtime/pkg/client" @@ -29,7 +30,7 @@ const retryUpstreamTimeout = 5 * time.Second // NginxUpdater is an interface for updating NGINX using the NGINX agent. type NginxUpdater interface { - UpdateConfig(deployment *Deployment, files []File) + UpdateConfig(deployment *Deployment, files []File, volumeMounts []v1.VolumeMount) UpdateUpstreamServers(deployment *Deployment, conf dataplane.Configuration) } @@ -87,8 +88,9 @@ func NewNginxUpdater( func (n *NginxUpdaterImpl) UpdateConfig( deployment *Deployment, files []File, + volumeMounts []v1.VolumeMount, ) { - msg := deployment.SetFiles(files) + msg := deployment.SetFiles(files, volumeMounts) if msg == nil { n.logger.V(1).Info("No changes to nginx configuration files, not sending to agent") return diff --git a/internal/controller/nginx/agent/agent_test.go b/internal/controller/nginx/agent/agent_test.go index b0646ee939..f0e969846a 100644 --- a/internal/controller/nginx/agent/agent_test.go +++ b/internal/controller/nginx/agent/agent_test.go @@ -9,6 +9,7 @@ import ( pb "github.com/nginx/agent/v3/api/grpc/mpi/v1" . "github.com/onsi/gomega" "google.golang.org/protobuf/types/known/structpb" + v1 "k8s.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client/fake" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/agent/broadcast/broadcastfakes" @@ -63,7 +64,7 @@ func TestUpdateConfig(t *testing.T) { deployment.SetPodErrorStatus("pod1", testErr) } - updater.UpdateConfig(deployment, []File{file}) + updater.UpdateConfig(deployment, []File{file}, []v1.VolumeMount{}) g.Expect(fakeBroadcaster.SendCallCount()).To(Equal(1)) fileContents, _ := deployment.GetFile(file.Meta.Name, file.Meta.Hash) @@ -74,7 +75,7 @@ func TestUpdateConfig(t *testing.T) { // ensure that the error is cleared after the next config is applied deployment.SetPodErrorStatus("pod1", nil) file.Meta.Hash = "5678" - updater.UpdateConfig(deployment, []File{file}) + updater.UpdateConfig(deployment, []File{file}, []v1.VolumeMount{}) g.Expect(deployment.GetLatestConfigError()).ToNot(HaveOccurred()) } else { g.Expect(deployment.GetLatestConfigError()).ToNot(HaveOccurred()) @@ -105,10 +106,10 @@ func TestUpdateConfig_NoChange(t *testing.T) { } // Set the initial files on the deployment - deployment.SetFiles([]File{file}) + deployment.SetFiles([]File{file}, []v1.VolumeMount{}) // Call UpdateConfig with the same files - updater.UpdateConfig(deployment, []File{file}) + updater.UpdateConfig(deployment, []File{file}, []v1.VolumeMount{}) // Verify that no new configuration was sent g.Expect(fakeBroadcaster.SendCallCount()).To(Equal(0)) diff --git a/internal/controller/nginx/agent/agentfakes/fake_nginx_updater.go b/internal/controller/nginx/agent/agentfakes/fake_nginx_updater.go index b838e07274..36d58ace70 100644 --- a/internal/controller/nginx/agent/agentfakes/fake_nginx_updater.go +++ b/internal/controller/nginx/agent/agentfakes/fake_nginx_updater.go @@ -6,14 +6,16 @@ import ( "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/agent" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/dataplane" + v1 "k8s.io/api/core/v1" ) type FakeNginxUpdater struct { - UpdateConfigStub func(*agent.Deployment, []agent.File) + UpdateConfigStub func(*agent.Deployment, []agent.File, []v1.VolumeMount) updateConfigMutex sync.RWMutex updateConfigArgsForCall []struct { arg1 *agent.Deployment arg2 []agent.File + arg3 []v1.VolumeMount } UpdateUpstreamServersStub func(*agent.Deployment, dataplane.Configuration) updateUpstreamServersMutex sync.RWMutex @@ -25,22 +27,28 @@ type FakeNginxUpdater struct { invocationsMutex sync.RWMutex } -func (fake *FakeNginxUpdater) UpdateConfig(arg1 *agent.Deployment, arg2 []agent.File) { +func (fake *FakeNginxUpdater) UpdateConfig(arg1 *agent.Deployment, arg2 []agent.File, arg3 []v1.VolumeMount) { var arg2Copy []agent.File if arg2 != nil { arg2Copy = make([]agent.File, len(arg2)) copy(arg2Copy, arg2) } + var arg3Copy []v1.VolumeMount + if arg3 != nil { + arg3Copy = make([]v1.VolumeMount, len(arg3)) + copy(arg3Copy, arg3) + } fake.updateConfigMutex.Lock() fake.updateConfigArgsForCall = append(fake.updateConfigArgsForCall, struct { arg1 *agent.Deployment arg2 []agent.File - }{arg1, arg2Copy}) + arg3 []v1.VolumeMount + }{arg1, arg2Copy, arg3Copy}) stub := fake.UpdateConfigStub - fake.recordInvocation("UpdateConfig", []interface{}{arg1, arg2Copy}) + fake.recordInvocation("UpdateConfig", []interface{}{arg1, arg2Copy, arg3Copy}) fake.updateConfigMutex.Unlock() if stub != nil { - fake.UpdateConfigStub(arg1, arg2) + fake.UpdateConfigStub(arg1, arg2, arg3) } } @@ -50,17 +58,17 @@ func (fake *FakeNginxUpdater) UpdateConfigCallCount() int { return len(fake.updateConfigArgsForCall) } -func (fake *FakeNginxUpdater) UpdateConfigCalls(stub func(*agent.Deployment, []agent.File)) { +func (fake *FakeNginxUpdater) UpdateConfigCalls(stub func(*agent.Deployment, []agent.File, []v1.VolumeMount)) { fake.updateConfigMutex.Lock() defer fake.updateConfigMutex.Unlock() fake.UpdateConfigStub = stub } -func (fake *FakeNginxUpdater) UpdateConfigArgsForCall(i int) (*agent.Deployment, []agent.File) { +func (fake *FakeNginxUpdater) UpdateConfigArgsForCall(i int) (*agent.Deployment, []agent.File, []v1.VolumeMount) { fake.updateConfigMutex.RLock() defer fake.updateConfigMutex.RUnlock() argsForCall := fake.updateConfigArgsForCall[i] - return argsForCall.arg1, argsForCall.arg2 + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 } func (fake *FakeNginxUpdater) UpdateUpstreamServers(arg1 *agent.Deployment, arg2 dataplane.Configuration) { diff --git a/internal/controller/nginx/agent/command_test.go b/internal/controller/nginx/agent/command_test.go index b7365aa1c8..5acde66376 100644 --- a/internal/controller/nginx/agent/command_test.go +++ b/internal/controller/nginx/agent/command_test.go @@ -342,7 +342,7 @@ func TestSubscribe(t *testing.T) { Contents: []byte("file contents"), }, } - deployment.SetFiles(files) + deployment.SetFiles(files, []v1.VolumeMount{}) deployment.SetImageVersion("nginx:v1.0.0") initialAction := &pb.NGINXPlusAction{ @@ -488,7 +488,7 @@ func TestSubscribe_Reset(t *testing.T) { Contents: []byte("file contents"), }, } - deployment.SetFiles(files) + deployment.SetFiles(files, []v1.VolumeMount{}) deployment.SetImageVersion("nginx:v1.0.0") ctx, cancel := createGrpcContextWithCancel() diff --git a/internal/controller/nginx/agent/deployment.go b/internal/controller/nginx/agent/deployment.go index 93f8b692cf..89f0d65de6 100644 --- a/internal/controller/nginx/agent/deployment.go +++ b/internal/controller/nginx/agent/deployment.go @@ -4,10 +4,12 @@ import ( "context" "errors" "fmt" + "strings" "sync" pb "github.com/nginx/agent/v3/api/grpc/mpi/v1" filesHelper "github.com/nginx/agent/v3/pkg/files" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/agent/broadcast" @@ -58,6 +60,8 @@ type Deployment struct { fileOverviews []*pb.File files []File + latestFileNames []string + FileLock sync.RWMutex errLock sync.RWMutex } @@ -187,7 +191,7 @@ func (d *Deployment) GetFile(name, hash string) ([]byte, string) { // SetFiles updates the nginx files and fileOverviews for the deployment and returns the message to send. // The deployment FileLock MUST already be locked before calling this function. -func (d *Deployment) SetFiles(files []File) *broadcast.NginxAgentMessage { +func (d *Deployment) SetFiles(files []File, volumeMounts []v1.VolumeMount) *broadcast.NginxAgentMessage { d.files = files fileOverviews := make([]*pb.File, 0, len(files)) @@ -195,8 +199,23 @@ func (d *Deployment) SetFiles(files []File) *broadcast.NginxAgentMessage { fileOverviews = append(fileOverviews, &pb.File{FileMeta: file.Meta}) } + // To avoid duplicates, use a set for volume ignore files + volumeIgnoreSet := make(map[string]struct{}, len(d.latestFileNames)) + for _, vm := range volumeMounts { + for _, f := range d.latestFileNames { + if strings.HasPrefix(f, vm.MountPath) { + volumeIgnoreSet[f] = struct{}{} + } + } + } + + volumeIgnoreFiles := make([]string, 0, len(volumeIgnoreSet)) + for f := range volumeIgnoreSet { + volumeIgnoreFiles = append(volumeIgnoreFiles, f) + } + // add ignored files to the overview as 'unmanaged' so agent doesn't touch them - for _, f := range ignoreFiles { + for _, f := range append(ignoreFiles, volumeIgnoreFiles...) { meta := &pb.FileMeta{ Name: f, Permissions: fileMode, diff --git a/internal/controller/nginx/agent/deployment_test.go b/internal/controller/nginx/agent/deployment_test.go index 0560c31600..2307c2cbc8 100644 --- a/internal/controller/nginx/agent/deployment_test.go +++ b/internal/controller/nginx/agent/deployment_test.go @@ -7,6 +7,7 @@ import ( pb "github.com/nginx/agent/v3/api/grpc/mpi/v1" . "github.com/onsi/gomega" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/agent/broadcast" @@ -44,7 +45,7 @@ func TestSetAndGetFiles(t *testing.T) { }, } - msg := deployment.SetFiles(files) + msg := deployment.SetFiles(files, []v1.VolumeMount{}) fileOverviews, configVersion := deployment.GetFileOverviews() g.Expect(msg.Type).To(Equal(broadcast.ConfigApplyRequest)) @@ -61,7 +62,89 @@ func TestSetAndGetFiles(t *testing.T) { g.Expect(wrongHashFile).To(BeNil()) // Set the same files again - msg = deployment.SetFiles(files) + msg = deployment.SetFiles(files, []v1.VolumeMount{}) + g.Expect(msg).To(BeNil()) + + newFileOverviews, _ := deployment.GetFileOverviews() + g.Expect(newFileOverviews).To(Equal(fileOverviews)) +} + +func TestSetAndGetFiles_VolumeIgnoreFiles(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + deployment := newDeployment(&broadcastfakes.FakeBroadcaster{}) + + // Set up latestFileNames that will match with volume mount paths + deployment.latestFileNames = []string{ + "/var/log/nginx/access.log", + "/var/log/nginx/error.log", + "/etc/ssl/certs/cert.pem", + "/etc/nginx/conf.d/default.conf", // This won't match any volume mount + "/one/two/three/etc/ssl", // This won't match any volume mount either + } + + files := []File{ + { + Meta: &pb.FileMeta{ + Name: "test.conf", + Hash: "12345", + }, + Contents: []byte("test content"), + }, + } + + // Create volume mounts that will match some of the latestFileNames + volumeMounts := []v1.VolumeMount{ + { + Name: "log-volume", + MountPath: "/var/log/nginx", + }, + { + Name: "ssl-volume", + MountPath: "/etc/ssl", + }, + } + + msg := deployment.SetFiles(files, volumeMounts) + fileOverviews, configVersion := deployment.GetFileOverviews() + + g.Expect(msg.Type).To(Equal(broadcast.ConfigApplyRequest)) + g.Expect(msg.ConfigVersion).To(Equal(configVersion)) + + // Expected files: 1 managed file + 8 ignoreFiles + 3 volumeIgnoreFiles + // (3 files from latestFileNames that match volume mount paths) + g.Expect(msg.FileOverviews).To(HaveLen(12)) + g.Expect(fileOverviews).To(Equal(msg.FileOverviews)) + + // Verify managed file + file, _ := deployment.GetFile("test.conf", "12345") + g.Expect(file).To(Equal([]byte("test content"))) + + // Check that volume ignore files are present in the unmanaged files + unmanagedFiles := make([]string, 0) + for _, overview := range msg.FileOverviews { + if overview.Unmanaged { + unmanagedFiles = append(unmanagedFiles, overview.FileMeta.Name) + } + } + + // Should contain files that match volume mount paths + g.Expect(unmanagedFiles).To(ContainElement("/var/log/nginx/access.log")) + g.Expect(unmanagedFiles).To(ContainElement("/var/log/nginx/error.log")) + g.Expect(unmanagedFiles).To(ContainElement("/etc/ssl/certs/cert.pem")) + + // Should NOT contain file that doesn't match volume mount paths + g.Expect(unmanagedFiles).ToNot(ContainElement("/etc/nginx/conf.d/default.conf")) + g.Expect(unmanagedFiles).ToNot(ContainElement("/one/two/three/etc/ssl")) + + invalidFile, _ := deployment.GetFile("invalid", "12345") + g.Expect(invalidFile).To(BeNil()) + wrongHashFile, _ := deployment.GetFile("test.conf", "invalid") + g.Expect(wrongHashFile).To(BeNil()) + + // Set the same files again + msg = deployment.SetFiles(files, volumeMounts) g.Expect(msg).To(BeNil()) newFileOverviews, _ := deployment.GetFileOverviews() diff --git a/internal/controller/nginx/agent/file.go b/internal/controller/nginx/agent/file.go index fdcdc2ae8f..b632e2e55f 100644 --- a/internal/controller/nginx/agent/file.go +++ b/internal/controller/nginx/agent/file.go @@ -180,8 +180,39 @@ func (*fileService) GetOverview(context.Context, *pb.GetOverviewRequest) (*pb.Ge } // UpdateOverview is called by agent on startup and whenever any files change on the instance. -// Since directly changing nginx configuration on the instance is not supported, this is a no-op for NGF. -func (*fileService) UpdateOverview(context.Context, *pb.UpdateOverviewRequest) (*pb.UpdateOverviewResponse, error) { +// Since directly changing nginx configuration on the instance is not supported, NGF will send back an empty response. +// However, we do use this call to gather the list of referenced files in the nginx configuration in order to +// mark user mounted files as unmanaged so the agent does not attempt to modify them. +func (fs *fileService) UpdateOverview( + ctx context.Context, + req *pb.UpdateOverviewRequest, +) (*pb.UpdateOverviewResponse, error) { + gi, ok := grpcContext.GrpcInfoFromContext(ctx) + if !ok { + return &pb.UpdateOverviewResponse{}, agentgrpc.ErrStatusInvalidConnection + } + + conn := fs.connTracker.GetConnection(gi.IPAddress) + if conn.PodName == "" { + return &pb.UpdateOverviewResponse{}, status.Errorf(codes.NotFound, "connection not found") + } + + deployment := fs.nginxDeployments.Get(conn.Parent) + if deployment == nil { + return &pb.UpdateOverviewResponse{}, status.Errorf(codes.NotFound, "deployment not found in store") + } + + requestFiles := req.GetOverview().GetFiles() + + fileNames := make([]string, 0, len(requestFiles)) + for _, f := range requestFiles { + fileNames = append(fileNames, f.GetFileMeta().GetName()) + } + + deployment.FileLock.Lock() + deployment.latestFileNames = fileNames + deployment.FileLock.Unlock() + return &pb.UpdateOverviewResponse{}, nil } diff --git a/internal/controller/nginx/agent/file_test.go b/internal/controller/nginx/agent/file_test.go index 3805d145c8..b8ff0dce78 100644 --- a/internal/controller/nginx/agent/file_test.go +++ b/internal/controller/nginx/agent/file_test.go @@ -365,13 +365,176 @@ func TestUpdateOverview(t *testing.T) { t.Parallel() g := NewWithT(t) - fs := newFileService(logr.Discard(), nil, nil) - resp, err := fs.UpdateOverview(t.Context(), &pb.UpdateOverviewRequest{}) + deploymentName := types.NamespacedName{Name: "nginx-deployment", Namespace: "default"} + + connTracker := &agentgrpcfakes.FakeConnectionsTracker{} + conn := agentgrpc.Connection{ + PodName: "nginx-pod", + InstanceID: "12345", + Parent: deploymentName, + } + connTracker.GetConnectionReturns(conn) + + depStore := NewDeploymentStore(connTracker) + dep := depStore.GetOrStore(t.Context(), deploymentName, nil) + + // Create a file larger than defaultChunkSize to ensure multiple chunks are sent + fileContent := make([]byte, defaultChunkSize+100) + for i := range fileContent { + fileContent[i] = byte(i % 256) + } + fileMeta := &pb.FileMeta{ + Name: "bigfile.conf", + Hash: "big-hash", + Size: int64(len(fileContent)), + } + + dep.files = []File{ + { + Meta: fileMeta, + Contents: fileContent, + }, + } + + ctx := grpcContext.NewGrpcContext(t.Context(), grpcContext.GrpcInfo{ + IPAddress: "127.0.0.1", + }) + + fs := newFileService(logr.Discard(), depStore, connTracker) + + resp, err := fs.UpdateOverview(ctx, &pb.UpdateOverviewRequest{ + Overview: &pb.FileOverview{ + Files: []*pb.File{ + { + FileMeta: &pb.FileMeta{ + Name: "nginx.conf", + Hash: "abc123", + Size: 1024, + Permissions: "644", + }, + }, + { + FileMeta: &pb.FileMeta{ + Name: "mime.types", + Hash: "def456", + Size: 2048, + Permissions: "644", + }, + }, + { + FileMeta: &pb.FileMeta{ + Name: "fastcgi.conf", + Hash: "ghi789", + Size: 512, + Permissions: "644", + }, + }, + }, + }, + }) + + // Add assertion to verify deployment.latestFileNames was set + expectedFileNames := []string{"nginx.conf", "mime.types", "fastcgi.conf"} + g.Expect(dep.latestFileNames).To(Equal(expectedFileNames)) g.Expect(err).ToNot(HaveOccurred()) g.Expect(resp).To(Equal(&pb.UpdateOverviewResponse{})) } +func TestUpdateOverview_InvalidConnection(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + fs := newFileService(logr.Discard(), nil, nil) + + req := &pb.UpdateOverviewRequest{ + Overview: &pb.FileOverview{ + Files: []*pb.File{ + { + FileMeta: &pb.FileMeta{ + Name: "nginx.conf", + Hash: "abc123", + }, + }, + }, + }, + } + + // Use regular context without GrpcInfo to trigger invalid connection + resp, err := fs.UpdateOverview(t.Context(), req) + + g.Expect(err).To(Equal(agentgrpc.ErrStatusInvalidConnection)) + g.Expect(resp).To(Equal(&pb.UpdateOverviewResponse{})) +} + +func TestUpdateOverview_ConnectionNotFound(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + fs := newFileService(logr.Discard(), nil, &agentgrpcfakes.FakeConnectionsTracker{}) + + req := &pb.UpdateOverviewRequest{ + Overview: &pb.FileOverview{ + Files: []*pb.File{ + { + FileMeta: &pb.FileMeta{ + Name: "nginx.conf", + Hash: "abc123", + }, + }, + }, + }, + } + + ctx := grpcContext.NewGrpcContext(t.Context(), grpcContext.GrpcInfo{ + IPAddress: "127.0.0.1", + }) + + resp, err := fs.UpdateOverview(ctx, req) + + g.Expect(err).To(Equal(status.Errorf(codes.NotFound, "connection not found"))) + g.Expect(resp).To(Equal(&pb.UpdateOverviewResponse{})) +} + +func TestUpdateOverview_DeploymentNotFound(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + deploymentName := types.NamespacedName{Name: "nginx-deployment", Namespace: "default"} + + connTracker := &agentgrpcfakes.FakeConnectionsTracker{} + conn := agentgrpc.Connection{ + PodName: "nginx-pod", + InstanceID: "12345", + Parent: deploymentName, + } + connTracker.GetConnectionReturns(conn) + + fs := newFileService(logr.Discard(), NewDeploymentStore(connTracker), connTracker) + + req := &pb.UpdateOverviewRequest{ + Overview: &pb.FileOverview{ + Files: []*pb.File{ + { + FileMeta: &pb.FileMeta{ + Name: "nginx.conf", + Hash: "abc123", + }, + }, + }, + }, + } + + ctx := grpcContext.NewGrpcContext(t.Context(), grpcContext.GrpcInfo{ + IPAddress: "127.0.0.1", + }) + + resp, err := fs.UpdateOverview(ctx, req) + + g.Expect(err).To(Equal(status.Errorf(codes.NotFound, "deployment not found in store"))) + g.Expect(resp).To(Equal(&pb.UpdateOverviewResponse{})) +} + func TestUpdateFile(t *testing.T) { t.Parallel() g := NewWithT(t)