From e830d20ef1115ad4e3d73d103603da3bfbbdfcad Mon Sep 17 00:00:00 2001 From: Nick Chen Date: Tue, 10 Jan 2023 12:04:21 -0800 Subject: [PATCH] fix: skip delete if file doesn't exists --- src/core/nginx.go | 10 +++ src/core/nginx_test.go | 84 +++++++++++++++++++ .../v2/src/core/metrics/sources/nginx_plus.go | 2 +- .../nginx/agent/v2/src/core/nginx.go | 10 +++ 4 files changed, 105 insertions(+), 1 deletion(-) diff --git a/src/core/nginx.go b/src/core/nginx.go index a68364687..7a76e74cd 100644 --- a/src/core/nginx.go +++ b/src/core/nginx.go @@ -399,6 +399,16 @@ func (n *NginxBinaryType) WriteConfig(config *proto.NginxConfig) (*sdk.ConfigApp if _, ok = fileDeleted[file]; ok { continue } + + if found, foundErr := FileExists(file); !found { + if foundErr == nil { + log.Debugf("skip delete for non-existing file: %s", file) + continue + } + // possible perm deny, depends on platform + log.Warnf("file exists returned for %s: %s", file, foundErr) + return configApply, foundErr + } if err = configApply.MarkAndSave(file); err != nil { return configApply, err } diff --git a/src/core/nginx_test.go b/src/core/nginx_test.go index 76cca36bf..1bf818cd9 100644 --- a/src/core/nginx_test.go +++ b/src/core/nginx_test.go @@ -19,6 +19,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/nginx/agent/sdk/v2/proto" "github.com/nginx/agent/sdk/v2/zip" @@ -555,6 +556,10 @@ func TestWriteConfigWithFileAction(t *testing.T) { Name: "test3.html", Action: proto.File_delete, }) + auxDir.Files = append(auxDir.Files, &proto.File{ + Name: "test4.html", + Action: proto.File_delete, + }) configApply, err := n.WriteConfig(nginxConfig) // Verify configApply @@ -587,6 +592,85 @@ func TestWriteConfigWithFileAction(t *testing.T) { assert.NoError(t, err) } +func TestWriteConfigWithFileActionDeleteWithPermError(t *testing.T) { + tmpDir := t.TempDir() + + allowedDirs := make(map[string]struct{}) + allowedDirs[tmpDir] = struct{}{} + fakeConfig := config.Config{ + AllowedDirectoriesMap: allowedDirs, + } + + env := EnvironmentType{} + n := NewNginxBinary(&env, &fakeConfig) + + n.nginxDetailsMap = map[string]*proto.NginxDetails{ + "151d8728e792f42e129337573a21bb30ab3065d59102f075efc2ded28e713ff8": { + NginxId: "151d8728e792f42e129337573a21bb30ab3065d59102f075efc2ded28e713ff8", + ConfPath: filepath.Join(tmpDir, "/nginx.conf"), + ProcessId: "777", + ProcessPath: "/usr/sbin/nginx", + }, + } + + if err := os.WriteFile(filepath.Join(tmpDir, "nginx.conf"), + []byte(fmt.Sprintf(CONF_TEMPLATE, tmpDir)), 0755); err != nil { + t.Fatalf("failed to write file: %v", err) + } + + if err := os.Mkdir(filepath.Join(tmpDir, "aux"), 0755); err != nil { + t.Fatalf("failed to create aux directory: %v", err) + } + if err := os.WriteFile(filepath.Join(tmpDir, "/aux/test2.html"), []byte(""), 0755); err != nil { + t.Fatalf("failed to write file: %v", err) + } + + nginxConfig, err := buildConfig(tmpDir) + if err != nil { + t.Fatal("failed to create test config") + } + var auxDir *proto.Directory + auxTmpDir := filepath.Join(tmpDir, "aux") + for _, dir := range nginxConfig.DirectoryMap.Directories { + for _, f := range dir.Files { + f.Action = proto.File_unchanged + } + // set aux dir directory map + if filepath.Clean(dir.Name) == auxTmpDir { + auxDir = dir + } + } + if auxDir == nil { + t.Fatalf("no aux dir found") + } + auxDir.Files = append(auxDir.Files, &proto.File{ + Name: "test2.html", + Action: proto.File_delete, + }) + + modDir := &proto.Directory{ + Name: tmpDir, + Files: make([]*proto.File, 0), + } + modDir.Files = append(modDir.Files, &proto.File{ + Name: "test3.html", + Action: proto.File_delete, + }) + nginxConfig.DirectoryMap.Directories = append(nginxConfig.DirectoryMap.Directories, modDir) + + permFile := filepath.Join(tmpDir, "test3.html") + if err = os.WriteFile(permFile, []byte(""), 0755); err != nil { + t.Fatalf("failed to write file: %v", err) + } + require.NoError(t, os.Chmod(permFile, 0000)) + + ca, err := n.WriteConfig(nginxConfig) + // Verify configApply + assert.Error(t, err) + assert.ErrorIs(t, err, os.ErrPermission) + assert.NotNil(t, ca) +} + func TestGetDirectoryMapDiff(t *testing.T) { tests := []struct { name string diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/core/metrics/sources/nginx_plus.go b/test/performance/vendor/github.com/nginx/agent/v2/src/core/metrics/sources/nginx_plus.go index bb4db44c2..6dd9b5d14 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/core/metrics/sources/nginx_plus.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/core/metrics/sources/nginx_plus.go @@ -412,7 +412,7 @@ func (c *NginxPlus) httpUpstreamMetrics(stats, prevStats *plusclient.Stats) []*p peerStateMap[peer.State] = peerStateMap[peer.State] + 1 tempPeer := plusclient.Peer(peer) if prevPeer, ok := prevPeersMap[getHttpUpstreamPeerKey((peer))]; ok { - if peer.Active > prevPeer.Active { + if peer.Active >= prevPeer.Active { tempPeer.Active = peer.Active - prevPeer.Active } if peer.Requests >= prevPeer.Requests { diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/core/nginx.go b/test/performance/vendor/github.com/nginx/agent/v2/src/core/nginx.go index a68364687..7a76e74cd 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/core/nginx.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/core/nginx.go @@ -399,6 +399,16 @@ func (n *NginxBinaryType) WriteConfig(config *proto.NginxConfig) (*sdk.ConfigApp if _, ok = fileDeleted[file]; ok { continue } + + if found, foundErr := FileExists(file); !found { + if foundErr == nil { + log.Debugf("skip delete for non-existing file: %s", file) + continue + } + // possible perm deny, depends on platform + log.Warnf("file exists returned for %s: %s", file, foundErr) + return configApply, foundErr + } if err = configApply.MarkAndSave(file); err != nil { return configApply, err }