Skip to content

Commit 75aeb83

Browse files
committed
Add support for ignoring volume mounted files
1 parent 5c78a60 commit 75aeb83

File tree

10 files changed

+238
-27
lines changed

10 files changed

+238
-27
lines changed

internal/controller/handler.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -241,8 +241,15 @@ func (h *eventHandlerImpl) sendNginxConfig(ctx context.Context, logger logr.Logg
241241

242242
h.setLatestConfiguration(gw, &cfg)
243243

244+
vm := []v1.VolumeMount{}
245+
if gw.EffectiveNginxProxy != nil &&
246+
gw.EffectiveNginxProxy.Kubernetes != nil &&
247+
gw.EffectiveNginxProxy.Kubernetes.Deployment != nil {
248+
vm = gw.EffectiveNginxProxy.Kubernetes.Deployment.Container.VolumeMounts
249+
}
250+
244251
deployment.FileLock.Lock()
245-
h.updateNginxConf(deployment, cfg)
252+
h.updateNginxConf(deployment, cfg, vm)
246253
deployment.FileLock.Unlock()
247254

248255
configErr := deployment.GetLatestConfigError()
@@ -454,9 +461,10 @@ func (h *eventHandlerImpl) parseAndCaptureEvent(ctx context.Context, logger logr
454461
func (h *eventHandlerImpl) updateNginxConf(
455462
deployment *agent.Deployment,
456463
conf dataplane.Configuration,
464+
volumeMounts []v1.VolumeMount,
457465
) {
458466
files := h.cfg.generator.Generate(conf)
459-
h.cfg.nginxUpdater.UpdateConfig(deployment, files)
467+
h.cfg.nginxUpdater.UpdateConfig(deployment, files, volumeMounts)
460468

461469
// If using NGINX Plus, update upstream servers using the API.
462470
if h.cfg.plus {

internal/controller/handler_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ var _ = Describe("eventHandler", func() {
6666
Expect(fakeGenerator.GenerateArgsForCall(0)).Should(Equal(expectedConf))
6767

6868
Expect(fakeNginxUpdater.UpdateConfigCallCount()).Should(Equal(1))
69-
_, files := fakeNginxUpdater.UpdateConfigArgsForCall(0)
69+
_, files, _ := fakeNginxUpdater.UpdateConfigArgsForCall(0)
7070
Expect(expectedFiles).To(Equal(files))
7171

7272
Eventually(

internal/controller/nginx/agent/agent.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/go-logr/logr"
1111
pb "github.com/nginx/agent/v3/api/grpc/mpi/v1"
1212
"google.golang.org/protobuf/types/known/structpb"
13+
v1 "k8s.io/api/core/v1"
1314
"k8s.io/apimachinery/pkg/util/wait"
1415
"sigs.k8s.io/controller-runtime/pkg/client"
1516

@@ -29,7 +30,7 @@ const retryUpstreamTimeout = 5 * time.Second
2930

3031
// NginxUpdater is an interface for updating NGINX using the NGINX agent.
3132
type NginxUpdater interface {
32-
UpdateConfig(deployment *Deployment, files []File)
33+
UpdateConfig(deployment *Deployment, files []File, volumeMounts []v1.VolumeMount)
3334
UpdateUpstreamServers(deployment *Deployment, conf dataplane.Configuration)
3435
}
3536

@@ -87,8 +88,9 @@ func NewNginxUpdater(
8788
func (n *NginxUpdaterImpl) UpdateConfig(
8889
deployment *Deployment,
8990
files []File,
91+
volumeMounts []v1.VolumeMount,
9092
) {
91-
msg := deployment.SetFiles(files)
93+
msg := deployment.SetFiles(files, volumeMounts)
9294
if msg == nil {
9395
n.logger.V(1).Info("No changes to nginx configuration files, not sending to agent")
9496
return

internal/controller/nginx/agent/agent_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
pb "github.com/nginx/agent/v3/api/grpc/mpi/v1"
1010
. "github.com/onsi/gomega"
1111
"google.golang.org/protobuf/types/known/structpb"
12+
v1 "k8s.io/api/core/v1"
1213
"sigs.k8s.io/controller-runtime/pkg/client/fake"
1314

1415
"github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/agent/broadcast/broadcastfakes"
@@ -63,7 +64,7 @@ func TestUpdateConfig(t *testing.T) {
6364
deployment.SetPodErrorStatus("pod1", testErr)
6465
}
6566

66-
updater.UpdateConfig(deployment, []File{file})
67+
updater.UpdateConfig(deployment, []File{file}, []v1.VolumeMount{})
6768

6869
g.Expect(fakeBroadcaster.SendCallCount()).To(Equal(1))
6970
fileContents, _ := deployment.GetFile(file.Meta.Name, file.Meta.Hash)
@@ -74,7 +75,7 @@ func TestUpdateConfig(t *testing.T) {
7475
// ensure that the error is cleared after the next config is applied
7576
deployment.SetPodErrorStatus("pod1", nil)
7677
file.Meta.Hash = "5678"
77-
updater.UpdateConfig(deployment, []File{file})
78+
updater.UpdateConfig(deployment, []File{file}, []v1.VolumeMount{})
7879
g.Expect(deployment.GetLatestConfigError()).ToNot(HaveOccurred())
7980
} else {
8081
g.Expect(deployment.GetLatestConfigError()).ToNot(HaveOccurred())
@@ -105,10 +106,10 @@ func TestUpdateConfig_NoChange(t *testing.T) {
105106
}
106107

107108
// Set the initial files on the deployment
108-
deployment.SetFiles([]File{file})
109+
deployment.SetFiles([]File{file}, []v1.VolumeMount{})
109110

110111
// Call UpdateConfig with the same files
111-
updater.UpdateConfig(deployment, []File{file})
112+
updater.UpdateConfig(deployment, []File{file}, []v1.VolumeMount{})
112113

113114
// Verify that no new configuration was sent
114115
g.Expect(fakeBroadcaster.SendCallCount()).To(Equal(0))

internal/controller/nginx/agent/agentfakes/fake_nginx_updater.go

Lines changed: 16 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/controller/nginx/agent/command_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,7 @@ func TestSubscribe(t *testing.T) {
342342
Contents: []byte("file contents"),
343343
},
344344
}
345-
deployment.SetFiles(files)
345+
deployment.SetFiles(files, []v1.VolumeMount{})
346346
deployment.SetImageVersion("nginx:v1.0.0")
347347

348348
initialAction := &pb.NGINXPlusAction{
@@ -488,7 +488,7 @@ func TestSubscribe_Reset(t *testing.T) {
488488
Contents: []byte("file contents"),
489489
},
490490
}
491-
deployment.SetFiles(files)
491+
deployment.SetFiles(files, []v1.VolumeMount{})
492492
deployment.SetImageVersion("nginx:v1.0.0")
493493

494494
ctx, cancel := createGrpcContextWithCancel()

internal/controller/nginx/agent/deployment.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@ import (
44
"context"
55
"errors"
66
"fmt"
7+
"strings"
78
"sync"
89

910
pb "github.com/nginx/agent/v3/api/grpc/mpi/v1"
1011
filesHelper "github.com/nginx/agent/v3/pkg/files"
12+
v1 "k8s.io/api/core/v1"
1113
"k8s.io/apimachinery/pkg/types"
1214

1315
"github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/agent/broadcast"
@@ -58,6 +60,8 @@ type Deployment struct {
5860
fileOverviews []*pb.File
5961
files []File
6062

63+
latestFileNames []string
64+
6165
FileLock sync.RWMutex
6266
errLock sync.RWMutex
6367
}
@@ -187,16 +191,25 @@ func (d *Deployment) GetFile(name, hash string) ([]byte, string) {
187191

188192
// SetFiles updates the nginx files and fileOverviews for the deployment and returns the message to send.
189193
// The deployment FileLock MUST already be locked before calling this function.
190-
func (d *Deployment) SetFiles(files []File) *broadcast.NginxAgentMessage {
194+
func (d *Deployment) SetFiles(files []File, volumeMounts []v1.VolumeMount) *broadcast.NginxAgentMessage {
191195
d.files = files
192196

193197
fileOverviews := make([]*pb.File, 0, len(files))
194198
for _, file := range files {
195199
fileOverviews = append(fileOverviews, &pb.File{FileMeta: file.Meta})
196200
}
197201

202+
volumeIgnoreFiles := make([]string, 0, len(d.latestFileNames))
203+
for _, f := range d.latestFileNames {
204+
for _, vm := range volumeMounts {
205+
if strings.Contains(f, vm.MountPath) {
206+
volumeIgnoreFiles = append(volumeIgnoreFiles, f)
207+
}
208+
}
209+
}
210+
198211
// add ignored files to the overview as 'unmanaged' so agent doesn't touch them
199-
for _, f := range ignoreFiles {
212+
for _, f := range append(ignoreFiles, volumeIgnoreFiles...) {
200213
meta := &pb.FileMeta{
201214
Name: f,
202215
Permissions: fileMode,

internal/controller/nginx/agent/deployment_test.go

Lines changed: 83 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77

88
pb "github.com/nginx/agent/v3/api/grpc/mpi/v1"
99
. "github.com/onsi/gomega"
10+
v1 "k8s.io/api/core/v1"
1011
"k8s.io/apimachinery/pkg/types"
1112

1213
"github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/agent/broadcast"
@@ -44,7 +45,7 @@ func TestSetAndGetFiles(t *testing.T) {
4445
},
4546
}
4647

47-
msg := deployment.SetFiles(files)
48+
msg := deployment.SetFiles(files, []v1.VolumeMount{})
4849
fileOverviews, configVersion := deployment.GetFileOverviews()
4950

5051
g.Expect(msg.Type).To(Equal(broadcast.ConfigApplyRequest))
@@ -61,7 +62,87 @@ func TestSetAndGetFiles(t *testing.T) {
6162
g.Expect(wrongHashFile).To(BeNil())
6263

6364
// Set the same files again
64-
msg = deployment.SetFiles(files)
65+
msg = deployment.SetFiles(files, []v1.VolumeMount{})
66+
g.Expect(msg).To(BeNil())
67+
68+
newFileOverviews, _ := deployment.GetFileOverviews()
69+
g.Expect(newFileOverviews).To(Equal(fileOverviews))
70+
}
71+
72+
func TestSetAndGetFiles_VolumeIgnoreFiles(t *testing.T) {
73+
t.Parallel()
74+
g := NewWithT(t)
75+
76+
deployment := newDeployment(&broadcastfakes.FakeBroadcaster{})
77+
78+
// Set up latestFileNames that will match with volume mount paths
79+
deployment.latestFileNames = []string{
80+
"/var/log/nginx/access.log",
81+
"/var/log/nginx/error.log",
82+
"/etc/ssl/certs/cert.pem",
83+
"/etc/nginx/conf.d/default.conf", // This won't match any volume mount
84+
}
85+
86+
files := []File{
87+
{
88+
Meta: &pb.FileMeta{
89+
Name: "test.conf",
90+
Hash: "12345",
91+
},
92+
Contents: []byte("test content"),
93+
},
94+
}
95+
96+
// Create volume mounts that will match some of the latestFileNames
97+
volumeMounts := []v1.VolumeMount{
98+
{
99+
Name: "log-volume",
100+
MountPath: "/var/log/nginx",
101+
},
102+
{
103+
Name: "ssl-volume",
104+
MountPath: "/etc/ssl",
105+
},
106+
}
107+
108+
msg := deployment.SetFiles(files, volumeMounts)
109+
fileOverviews, configVersion := deployment.GetFileOverviews()
110+
111+
g.Expect(msg.Type).To(Equal(broadcast.ConfigApplyRequest))
112+
g.Expect(msg.ConfigVersion).To(Equal(configVersion))
113+
114+
// Expected files: 1 managed file + 8 ignoreFiles + 3 volumeIgnoreFiles
115+
// (3 files from latestFileNames that match volume mount paths)
116+
g.Expect(msg.FileOverviews).To(HaveLen(12))
117+
g.Expect(fileOverviews).To(Equal(msg.FileOverviews))
118+
119+
// Verify managed file
120+
file, _ := deployment.GetFile("test.conf", "12345")
121+
g.Expect(file).To(Equal([]byte("test content")))
122+
123+
// Check that volume ignore files are present in the unmanaged files
124+
unmanagedFiles := make([]string, 0)
125+
for _, overview := range msg.FileOverviews {
126+
if overview.Unmanaged {
127+
unmanagedFiles = append(unmanagedFiles, overview.FileMeta.Name)
128+
}
129+
}
130+
131+
// Should contain files that match volume mount paths
132+
g.Expect(unmanagedFiles).To(ContainElement("/var/log/nginx/access.log"))
133+
g.Expect(unmanagedFiles).To(ContainElement("/var/log/nginx/error.log"))
134+
g.Expect(unmanagedFiles).To(ContainElement("/etc/ssl/certs/cert.pem"))
135+
136+
// Should NOT contain file that doesn't match volume mount paths
137+
g.Expect(unmanagedFiles).ToNot(ContainElement("/etc/nginx/conf.d/default.conf"))
138+
139+
invalidFile, _ := deployment.GetFile("invalid", "12345")
140+
g.Expect(invalidFile).To(BeNil())
141+
wrongHashFile, _ := deployment.GetFile("test.conf", "invalid")
142+
g.Expect(wrongHashFile).To(BeNil())
143+
144+
// Set the same files again
145+
msg = deployment.SetFiles(files, volumeMounts)
65146
g.Expect(msg).To(BeNil())
66147

67148
newFileOverviews, _ := deployment.GetFileOverviews()

internal/controller/nginx/agent/file.go

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,8 +180,37 @@ func (*fileService) GetOverview(context.Context, *pb.GetOverviewRequest) (*pb.Ge
180180
}
181181

182182
// UpdateOverview is called by agent on startup and whenever any files change on the instance.
183-
// Since directly changing nginx configuration on the instance is not supported, this is a no-op for NGF.
184-
func (*fileService) UpdateOverview(context.Context, *pb.UpdateOverviewRequest) (*pb.UpdateOverviewResponse, error) {
183+
// Since directly changing nginx configuration on the instance is not supported, NGF will send back an empty response.
184+
// However, we do use this call to gather the list of referenced files in the nginx configuration in order to
185+
// mark user mounted files as unmanaged so the agent does not attempt to modify them.
186+
func (fs *fileService) UpdateOverview(
187+
ctx context.Context,
188+
req *pb.UpdateOverviewRequest,
189+
) (*pb.UpdateOverviewResponse, error) {
190+
gi, ok := grpcContext.GrpcInfoFromContext(ctx)
191+
if !ok {
192+
return &pb.UpdateOverviewResponse{}, agentgrpc.ErrStatusInvalidConnection
193+
}
194+
195+
conn := fs.connTracker.GetConnection(gi.IPAddress)
196+
if conn.PodName == "" {
197+
return &pb.UpdateOverviewResponse{}, status.Errorf(codes.NotFound, "connection not found")
198+
}
199+
200+
deployment := fs.nginxDeployments.Get(conn.Parent)
201+
if deployment == nil {
202+
return &pb.UpdateOverviewResponse{}, status.Errorf(codes.NotFound, "deployment not found in store")
203+
}
204+
205+
if req != nil && req.Overview != nil && req.Overview.Files != nil {
206+
fileNames := make([]string, 0, len(req.Overview.Files))
207+
for _, f := range req.Overview.Files {
208+
fileNames = append(fileNames, f.FileMeta.GetName())
209+
}
210+
211+
deployment.latestFileNames = fileNames
212+
}
213+
185214
return &pb.UpdateOverviewResponse{}, nil
186215
}
187216

0 commit comments

Comments
 (0)