diff --git a/src/core/environment.go b/src/core/environment.go index 67d8defab..4fdaaa401 100644 --- a/src/core/environment.go +++ b/src/core/environment.go @@ -39,7 +39,7 @@ import ( //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -generate //counterfeiter:generate -o fake_environment_test.go . Environment -//go:generate sh -c "grep -v agent/product/nginx-agent/v2/core fake_environment_test.go | sed -e s\\/core\\\\.\\/\\/g > fake_environment_fixed.go" +//go:generate sh -c "grep -v github.com/nginx/agent/v2/src/core fake_environment_test.go | sed -e s\\/core\\\\.\\/\\/g > fake_environment_fixed.go" //go:generate mv fake_environment_fixed.go fake_environment_test.go type Environment interface { NewHostInfo(agentVersion string, tags *[]string, configDirs string, clearCache bool) *proto.HostInfo @@ -48,6 +48,8 @@ type Environment interface { GetSystemUUID() (hostId string) ReadDirectory(dir string, ext string) ([]string, error) WriteFiles(backup ConfigApplyMarker, files []*proto.File, prefix string, allowedDirs map[string]struct{}) error + WriteFile(backup ConfigApplyMarker, file *proto.File, confPath string) error + DeleteFile(backup ConfigApplyMarker, fileName string) error Processes() (result []*Process) FileStat(path string) (os.FileInfo, error) DiskDevices() ([]string, error) @@ -236,13 +238,69 @@ func (env *EnvironmentType) WriteFiles(backup ConfigApplyMarker, files []*proto. } for _, file := range files { - if err = writeFile(backup, file, confPath); err != nil { + if err = env.WriteFile(backup, file, confPath); err != nil { return err } } return nil } +// WriteFile writes the provided file content to disk. If the file.GetName() returns an absolute path, it'll be written +// to the path. Otherwise, it'll be written to the path relative to the provided confPath. +func (env *EnvironmentType) WriteFile(backup ConfigApplyMarker, file *proto.File, confPath string) error { + fileFullPath := file.GetName() + if !filepath.IsAbs(fileFullPath) { + fileFullPath = filepath.Join(confPath, fileFullPath) + } + + if err := backup.MarkAndSave(fileFullPath); err != nil { + return err + } + permissions := files.GetFileMode(file.GetPermissions()) + + directory := filepath.Dir(fileFullPath) + _, err := os.Stat(directory) + if os.IsNotExist(err) { + log.Debugf("Creating directory %s with permissions 755", directory) + err = os.MkdirAll(directory, 0o755) + if err != nil { + return err + } + } + + if err := os.WriteFile(fileFullPath, file.GetContents(), permissions); err != nil { + // If the file didn't exist originally and failed to be created + // Then remove that file from the backup so that the rollback doesn't try to delete the file + if _, err := os.Stat(fileFullPath); !errors.Is(err, os.ErrNotExist) { + backup.RemoveFromNotExists(fileFullPath) + } + return err + } + + log.Debugf("Wrote file %s", fileFullPath) + return nil +} + +func (env *EnvironmentType) DeleteFile(backup ConfigApplyMarker, fileName string) error { + if found, foundErr := FileExists(fileName); !found { + if foundErr == nil { + log.Debugf("skip delete for non-existing file: %s", fileName) + return nil + } + // possible perm deny, depends on platform + log.Warnf("file exists returned for %s: %s", fileName, foundErr) + return foundErr + } + if err := backup.MarkAndSave(fileName); err != nil { + return err + } + if err := os.Remove(fileName); err != nil { + return err + } + + return nil +} + func (env *EnvironmentType) IsContainer() bool { const ( dockerEnv = "/.dockerenv" @@ -451,42 +509,6 @@ func allowedFile(path string, allowedDirs map[string]struct{}) bool { return false } -// writeFile writes the provided file content to disk. If the file.GetName() returns an absolute path, it'll be written -// to the path. Otherwise, it'll be written to the path relative to the provided confPath. -func writeFile(backup ConfigApplyMarker, file *proto.File, confPath string) error { - fileFullPath := file.GetName() - if !filepath.IsAbs(fileFullPath) { - fileFullPath = filepath.Join(confPath, fileFullPath) - } - - if err := backup.MarkAndSave(fileFullPath); err != nil { - return err - } - permissions := files.GetFileMode(file.GetPermissions()) - - directory := filepath.Dir(fileFullPath) - _, err := os.Stat(directory) - if os.IsNotExist(err) { - log.Debugf("Creating directory %s with permissions 755", directory) - err = os.MkdirAll(directory, 0o755) - if err != nil { - return err - } - } - - if err := os.WriteFile(fileFullPath, file.GetContents(), permissions); err != nil { - // If the file didn't exist originally and failed to be created - // Then remove that file from the backup so that the rollback doesn't try to delete the file - if _, err := os.Stat(fileFullPath); !errors.Is(err, os.ErrNotExist) { - backup.RemoveFromNotExists(fileFullPath) - } - return err - } - - log.Debugf("Wrote file %s", fileFullPath) - return nil -} - func (env *EnvironmentType) FileStat(path string) (os.FileInfo, error) { // TODO: check if allowed list return os.Stat(path) diff --git a/src/core/environment_test.go b/src/core/environment_test.go index 25a7f9d08..48de869a8 100644 --- a/src/core/environment_test.go +++ b/src/core/environment_test.go @@ -700,6 +700,7 @@ func TestWriteFilesNotAllowed(t *testing.T) { } func TestWriteFile(t *testing.T) { + env := &EnvironmentType{} file := &proto.File{ Name: "/tmp/sub-1/sub-2/write.conf", Contents: []byte("contents"), @@ -707,7 +708,7 @@ func TestWriteFile(t *testing.T) { } backup, err := sdk.NewConfigApplyWithIgnoreDirectives("", nil, []string{}) assert.NoError(t, err) - assert.NoError(t, writeFile(backup, file, "/tmp")) + assert.NoError(t, env.WriteFile(backup, file, "/tmp")) assert.FileExists(t, file.GetName()) contents, err := os.ReadFile(file.GetName()) @@ -718,6 +719,34 @@ func TestWriteFile(t *testing.T) { assert.NoFileExists(t, file.GetName()) } +func TestDeleteFile(t *testing.T) { + env := &EnvironmentType{} + tempDir := t.TempDir() + fileName := tempDir + "/test.txt" + file := &proto.File{ + Name: fileName, + Contents: []byte("contents"), + Permissions: "0777", + } + backup, err := sdk.NewConfigApplyWithIgnoreDirectives("", nil, []string{}) + assert.NoError(t, err) + assert.NoError(t, env.WriteFile(backup, file, "/tmp")) + assert.FileExists(t, file.GetName()) + + contents, err := os.ReadFile(file.GetName()) + assert.NoError(t, err) + assert.Equal(t, file.GetContents(), contents) + + assert.NoError(t, env.DeleteFile(backup, fileName)) + assert.NoFileExists(t, file.GetName()) + + // verify that file being deleted is backed up in case we need to rollback + _, ok := backup.GetNotExists()[fileName] + assert.True(t, ok) + _, ok = backup.GetExisting()[fileName] + assert.False(t, ok) +} + func TestParseOsReleaseFile(t *testing.T) { tests := []struct { name string diff --git a/src/core/fake_environment_test.go b/src/core/fake_environment_test.go index 83ff83ee7..02b4da669 100644 --- a/src/core/fake_environment_test.go +++ b/src/core/fake_environment_test.go @@ -1,9 +1,3 @@ -/** - * Copyright (c) F5, Inc. - * - * This source code is licensed under the Apache License, Version 2.0 license found in the - * LICENSE file in the root directory of this source tree. - */ // Code generated by counterfeiter. DO NOT EDIT. package core @@ -15,6 +9,18 @@ import ( ) type FakeEnvironment struct { + DeleteFileStub func(ConfigApplyMarker, string) error + deleteFileMutex sync.RWMutex + deleteFileArgsForCall []struct { + arg1 ConfigApplyMarker + arg2 string + } + deleteFileReturns struct { + result1 error + } + deleteFileReturnsOnCall map[int]struct { + result1 error + } DiskDevicesStub func() ([]string, error) diskDevicesMutex sync.RWMutex diskDevicesArgsForCall []struct { @@ -132,6 +138,19 @@ type FakeEnvironment struct { result1 []string result2 error } + WriteFileStub func(ConfigApplyMarker, *proto.File, string) error + writeFileMutex sync.RWMutex + writeFileArgsForCall []struct { + arg1 ConfigApplyMarker + arg2 *proto.File + arg3 string + } + writeFileReturns struct { + result1 error + } + writeFileReturnsOnCall map[int]struct { + result1 error + } WriteFilesStub func(ConfigApplyMarker, []*proto.File, string, map[string]struct{}) error writeFilesMutex sync.RWMutex writeFilesArgsForCall []struct { @@ -150,6 +169,68 @@ type FakeEnvironment struct { invocationsMutex sync.RWMutex } +func (fake *FakeEnvironment) DeleteFile(arg1 ConfigApplyMarker, arg2 string) error { + fake.deleteFileMutex.Lock() + ret, specificReturn := fake.deleteFileReturnsOnCall[len(fake.deleteFileArgsForCall)] + fake.deleteFileArgsForCall = append(fake.deleteFileArgsForCall, struct { + arg1 ConfigApplyMarker + arg2 string + }{arg1, arg2}) + stub := fake.DeleteFileStub + fakeReturns := fake.deleteFileReturns + fake.recordInvocation("DeleteFile", []interface{}{arg1, arg2}) + fake.deleteFileMutex.Unlock() + if stub != nil { + return stub(arg1, arg2) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeEnvironment) DeleteFileCallCount() int { + fake.deleteFileMutex.RLock() + defer fake.deleteFileMutex.RUnlock() + return len(fake.deleteFileArgsForCall) +} + +func (fake *FakeEnvironment) DeleteFileCalls(stub func(ConfigApplyMarker, string) error) { + fake.deleteFileMutex.Lock() + defer fake.deleteFileMutex.Unlock() + fake.DeleteFileStub = stub +} + +func (fake *FakeEnvironment) DeleteFileArgsForCall(i int) (ConfigApplyMarker, string) { + fake.deleteFileMutex.RLock() + defer fake.deleteFileMutex.RUnlock() + argsForCall := fake.deleteFileArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2 +} + +func (fake *FakeEnvironment) DeleteFileReturns(result1 error) { + fake.deleteFileMutex.Lock() + defer fake.deleteFileMutex.Unlock() + fake.DeleteFileStub = nil + fake.deleteFileReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeEnvironment) DeleteFileReturnsOnCall(i int, result1 error) { + fake.deleteFileMutex.Lock() + defer fake.deleteFileMutex.Unlock() + fake.DeleteFileStub = nil + if fake.deleteFileReturnsOnCall == nil { + fake.deleteFileReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.deleteFileReturnsOnCall[i] = struct { + result1 error + }{result1} +} + func (fake *FakeEnvironment) DiskDevices() ([]string, error) { fake.diskDevicesMutex.Lock() ret, specificReturn := fake.diskDevicesReturnsOnCall[len(fake.diskDevicesArgsForCall)] @@ -723,6 +804,69 @@ func (fake *FakeEnvironment) ReadDirectoryReturnsOnCall(i int, result1 []string, }{result1, result2} } +func (fake *FakeEnvironment) WriteFile(arg1 ConfigApplyMarker, arg2 *proto.File, arg3 string) error { + fake.writeFileMutex.Lock() + ret, specificReturn := fake.writeFileReturnsOnCall[len(fake.writeFileArgsForCall)] + fake.writeFileArgsForCall = append(fake.writeFileArgsForCall, struct { + arg1 ConfigApplyMarker + arg2 *proto.File + arg3 string + }{arg1, arg2, arg3}) + stub := fake.WriteFileStub + fakeReturns := fake.writeFileReturns + fake.recordInvocation("WriteFile", []interface{}{arg1, arg2, arg3}) + fake.writeFileMutex.Unlock() + if stub != nil { + return stub(arg1, arg2, arg3) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeEnvironment) WriteFileCallCount() int { + fake.writeFileMutex.RLock() + defer fake.writeFileMutex.RUnlock() + return len(fake.writeFileArgsForCall) +} + +func (fake *FakeEnvironment) WriteFileCalls(stub func(ConfigApplyMarker, *proto.File, string) error) { + fake.writeFileMutex.Lock() + defer fake.writeFileMutex.Unlock() + fake.WriteFileStub = stub +} + +func (fake *FakeEnvironment) WriteFileArgsForCall(i int) (ConfigApplyMarker, *proto.File, string) { + fake.writeFileMutex.RLock() + defer fake.writeFileMutex.RUnlock() + argsForCall := fake.writeFileArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 +} + +func (fake *FakeEnvironment) WriteFileReturns(result1 error) { + fake.writeFileMutex.Lock() + defer fake.writeFileMutex.Unlock() + fake.WriteFileStub = nil + fake.writeFileReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeEnvironment) WriteFileReturnsOnCall(i int, result1 error) { + fake.writeFileMutex.Lock() + defer fake.writeFileMutex.Unlock() + fake.WriteFileStub = nil + if fake.writeFileReturnsOnCall == nil { + fake.writeFileReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.writeFileReturnsOnCall[i] = struct { + result1 error + }{result1} +} + func (fake *FakeEnvironment) WriteFiles(arg1 ConfigApplyMarker, arg2 []*proto.File, arg3 string, arg4 map[string]struct{}) error { var arg2Copy []*proto.File if arg2 != nil { @@ -795,6 +939,8 @@ func (fake *FakeEnvironment) WriteFilesReturnsOnCall(i int, result1 error) { func (fake *FakeEnvironment) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() + fake.deleteFileMutex.RLock() + defer fake.deleteFileMutex.RUnlock() fake.diskDevicesMutex.RLock() defer fake.diskDevicesMutex.RUnlock() fake.fileStatMutex.RLock() @@ -815,6 +961,8 @@ func (fake *FakeEnvironment) Invocations() map[string][][]interface{} { defer fake.processesMutex.RUnlock() fake.readDirectoryMutex.RLock() defer fake.readDirectoryMutex.RUnlock() + fake.writeFileMutex.RLock() + defer fake.writeFileMutex.RUnlock() fake.writeFilesMutex.RLock() defer fake.writeFilesMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} diff --git a/src/core/nginx.go b/src/core/nginx.go index 911238715..584dd1d22 100644 --- a/src/core/nginx.go +++ b/src/core/nginx.go @@ -394,39 +394,57 @@ func (n *NginxBinaryType) WriteConfig(config *proto.NginxConfig) (*sdk.ConfigApp return nil, fmt.Errorf("config directory %s not allowed", filepath.Dir(details.ConfPath)) } + unpackMutex.Lock() + defer unpackMutex.Unlock() + + log.Info("Updating NGINX config") + var configApply *sdk.ConfigApply + + filesToUpdate, filesToDelete, allFilesHaveAnAction := generateActionMaps(config.DirectoryMap, n.config.AllowedDirectoriesMap) + + if allFilesHaveAnAction { + configApply, err = n.writeConfigWithWithFileActions(config, details, filesToUpdate, filesToDelete) + } else { + log.Debug("all files in directory map have no action set") + // If the file action is unset for any file then all files are written to disk and a diff is performed to determine what files need to be deleted + configApply, err = n.writeConfigWithNoFileActions(details, config, systemNginxConfig) + } + + if err != nil { + return configApply, err + } + + return configApply, nil +} + +func (n *NginxBinaryType) writeConfigWithNoFileActions(details *proto.NginxDetails, config *proto.NginxConfig, systemNginxConfig *proto.NginxConfig) (*sdk.ConfigApply, error) { confFiles, auxFiles, err := sdk.GetNginxConfigFiles(config) if err != nil { return nil, err } + configApply, err := sdk.NewConfigApplyWithIgnoreDirectives(details.ConfPath, n.config.AllowedDirectoriesMap, n.config.IgnoreDirectives) + if err != nil { + log.Warnf("config_apply error: %s", err) + return configApply, err + } + // Ensure this config request does not remove the process config if !hasConfPath(confFiles, details.ConfPath) { - return nil, fmt.Errorf("should not delete %s", details.ConfPath) + return configApply, fmt.Errorf("should not delete %s", details.ConfPath) } // Ensure all config files are within the allowed list directories. confDir := filepath.Dir(details.ConfPath) if err := ensureFilesAllowed(confFiles, n.config.AllowedDirectoriesMap, confDir); err != nil { - return nil, err + return configApply, err } // Ensure all aux files are within the allowed list directories. if err := ensureFilesAllowed(auxFiles, n.config.AllowedDirectoriesMap, config.GetZaux().GetRootDirectory()); err != nil { - return nil, err - } - - unpackMutex.Lock() - defer unpackMutex.Unlock() - - log.Info("Updating NGINX config") - var configApply *sdk.ConfigApply - configApply, err = sdk.NewConfigApplyWithIgnoreDirectives(details.ConfPath, n.config.AllowedDirectoriesMap, n.config.IgnoreDirectives) - if err != nil { - log.Warnf("config_apply error: %s", err) - return nil, err + return configApply, err } - // TODO: return to Control Plane that there was a rollback err = n.env.WriteFiles(configApply, confFiles, filepath.Dir(details.ConfPath), n.config.AllowedDirectoriesMap) if err != nil { log.Warnf("configuration write failed: %s", err) @@ -458,24 +476,68 @@ func (n *NginxBinaryType) WriteConfig(config *proto.NginxConfig) (*sdk.ConfigApp 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 := n.env.DeleteFile(configApply, file); err != nil { + return configApply, err } - if err = configApply.MarkAndSave(file); err != nil { + + fileDeleted[file] = struct{}{} + } + + return configApply, nil +} + +func (n *NginxBinaryType) writeConfigWithWithFileActions(config *proto.NginxConfig, details *proto.NginxDetails, filesToUpdate map[string]proto.File_Action, filesToDelete map[string]proto.File_Action) (*sdk.ConfigApply, error) { + confFiles, auxFiles, err := sdk.GetNginxConfigFiles(config) + if err != nil { + return nil, err + } + + configApply, err := sdk.NewConfigApplyWithIgnoreDirectives("", n.config.AllowedDirectoriesMap, n.config.IgnoreDirectives) + if err != nil { + log.Warnf("config_apply error: %s", err) + return nil, err + } + + for _, file := range confFiles { + rootDirectoryPath := filepath.Dir(details.ConfPath) + if _, found := filesToUpdate[file.Name]; !found { + log.Debugf("No action found for config file %s.", file.Name) + continue + } + + delete(filesToUpdate, file.Name) + + if err := n.env.WriteFile(configApply, file, rootDirectoryPath); err != nil { + log.Warnf("configuration write failed: %s", err) return configApply, err } - if err = os.Remove(file); err != nil { + } + + for _, file := range auxFiles { + rootDirectoryPath := config.GetZaux().GetRootDirectory() + if _, found := filesToUpdate[file.Name]; !found { + log.Debugf("No action found for aux file %s.", file.Name) + continue + } + + delete(filesToUpdate, file.Name) + + if err := n.env.WriteFile(configApply, file, rootDirectoryPath); err != nil { + log.Warnf("configuration write failed: %s", err) return configApply, err } - fileDeleted[file] = struct{}{} } + for file, action := range filesToUpdate { + log.Warnf("File %s missing from NginxConfig message. Unable to perform action %s.", file, action.String()) + } + + for file := range filesToDelete { + log.Infof("Deleting file: %s", file) + if err := n.env.DeleteFile(configApply, file); err != nil { + return configApply, err + } + } return configApply, nil } @@ -514,6 +576,54 @@ func generateDeleteFromDirectoryMap( return deleteFiles, actionIsSet } +func generateActionMaps( + directoryMap *proto.DirectoryMap, + allowedDirectory map[string]struct{}, +) ( + filesToUpdate map[string]proto.File_Action, + filesToDelete map[string]proto.File_Action, + allFilesHaveAnAction bool, +) { + allFilesHaveAnAction = true + filesToUpdate = make(map[string]proto.File_Action) + filesToDelete = make(map[string]proto.File_Action) + + for _, dir := range directoryMap.Directories { + for _, f := range dir.Files { + path := filepath.Join(dir.Name, f.Name) + + log.Debugf("file %s has action %v", path, f.Action) + + // Can't support relative paths + if !filepath.IsAbs(path) { + continue + } + + if !allowedFile(path, allowedDirectory) { + continue + } + + if f.Action == proto.File_unset { + allFilesHaveAnAction = false + return + } + + if f.Action == proto.File_add || f.Action == proto.File_update { + filesToUpdate[path] = f.Action + continue + } + + if f.Action == proto.File_delete { + filesToDelete[path] = f.Action + continue + } + + } + } + + return filesToUpdate, filesToDelete, allFilesHaveAnAction +} + func (n *NginxBinaryType) ReadConfig(confFile, nginxId, systemId string) (*proto.NginxConfig, error) { configPayload, err := sdk.GetNginxConfigWithIgnoreDirectives(confFile, nginxId, systemId, n.config.AllowedDirectoriesMap, n.config.IgnoreDirectives) if err != nil { diff --git a/src/core/nginx_test.go b/src/core/nginx_test.go index 9e18f6e22..c2c06b512 100644 --- a/src/core/nginx_test.go +++ b/src/core/nginx_test.go @@ -538,7 +538,7 @@ func TestWriteConfigWithFileAction(t *testing.T) { var auxDir *proto.Directory for _, dir := range nginxConfig.DirectoryMap.Directories { for _, f := range dir.Files { - f.Action = proto.File_unchanged + f.Action = proto.File_unset } if filepath.Clean(dir.Name) == filepath.Join(tmpDir, "aux") { auxDir = dir @@ -632,7 +632,7 @@ func TestWriteConfigWithFileActionDeleteWithPermError(t *testing.T) { auxTmpDir := filepath.Join(tmpDir, "aux") for _, dir := range nginxConfig.DirectoryMap.Directories { for _, f := range dir.Files { - f.Action = proto.File_unchanged + f.Action = proto.File_unset } // set aux dir directory map if filepath.Clean(dir.Name) == auxTmpDir { @@ -1399,3 +1399,103 @@ func TestErrorLog(t *testing.T) { }) } } + +func TestGenerateActionMaps(t *testing.T) { + allowedDirectories := make(map[string]struct{}, 0) + allowedDirectories["/testDir/"] = struct{}{} + + testCases := []struct { + name string + directoryMap *proto.DirectoryMap + expectedFilesToUpdate map[string]proto.File_Action + expectedFilesToDelete map[string]proto.File_Action + allFilesHaveAnAction bool + }{ + { + name: "allActionsAreSet", + directoryMap: &proto.DirectoryMap{ + Directories: []*proto.Directory{ + { + Name: "/testDir/", + Files: []*proto.File{ + { + Name: "test-add.conf", + Action: proto.File_add, + }, + { + Name: "test-unchanged.conf", + Action: proto.File_unchanged, + }, + { + Name: "test-update.conf", + Action: proto.File_update, + }, + { + Name: "test-delete.conf", + Action: proto.File_delete, + }, + }, + }, + }, + }, + expectedFilesToUpdate: map[string]proto.File_Action{ + "/testDir/test-add.conf": proto.File_add, + "/testDir/test-update.conf": proto.File_update, + }, + expectedFilesToDelete: map[string]proto.File_Action{ + "/testDir/test-delete.conf": proto.File_delete, + }, + allFilesHaveAnAction: true, + }, + { + name: "NotAllActionsAreSet", + directoryMap: &proto.DirectoryMap{ + Directories: []*proto.Directory{ + { + Name: "/testDir/", + Files: []*proto.File{ + { + Name: "test-unset.conf", + }, + { + Name: "test-delete.conf", + Action: proto.File_delete, + }, + }, + }, + }, + }, + expectedFilesToUpdate: map[string]proto.File_Action{}, + expectedFilesToDelete: map[string]proto.File_Action{}, + allFilesHaveAnAction: false, + }, + { + name: "NoSupportForRelativePaths", + directoryMap: &proto.DirectoryMap{ + Directories: []*proto.Directory{ + { + Name: "testDir/", + Files: []*proto.File{ + { + Name: "test-unsupported.conf", + Action: proto.File_delete, + }, + }, + }, + }, + }, + expectedFilesToUpdate: map[string]proto.File_Action{}, + expectedFilesToDelete: map[string]proto.File_Action{}, + allFilesHaveAnAction: true, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(tt *testing.T) { + filesToUpdate, filesToDelete, allFilesHaveAnAction := generateActionMaps(testCase.directoryMap, allowedDirectories) + assert.Equal(tt, testCase.expectedFilesToUpdate, filesToUpdate) + assert.Equal(tt, testCase.expectedFilesToDelete, filesToDelete) + assert.Equal(tt, testCase.allFilesHaveAnAction, allFilesHaveAnAction) + }) + } +} diff --git a/test/integration/vendor/github.com/nginx/agent/v2/src/core/environment.go b/test/integration/vendor/github.com/nginx/agent/v2/src/core/environment.go index 67d8defab..4fdaaa401 100644 --- a/test/integration/vendor/github.com/nginx/agent/v2/src/core/environment.go +++ b/test/integration/vendor/github.com/nginx/agent/v2/src/core/environment.go @@ -39,7 +39,7 @@ import ( //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -generate //counterfeiter:generate -o fake_environment_test.go . Environment -//go:generate sh -c "grep -v agent/product/nginx-agent/v2/core fake_environment_test.go | sed -e s\\/core\\\\.\\/\\/g > fake_environment_fixed.go" +//go:generate sh -c "grep -v github.com/nginx/agent/v2/src/core fake_environment_test.go | sed -e s\\/core\\\\.\\/\\/g > fake_environment_fixed.go" //go:generate mv fake_environment_fixed.go fake_environment_test.go type Environment interface { NewHostInfo(agentVersion string, tags *[]string, configDirs string, clearCache bool) *proto.HostInfo @@ -48,6 +48,8 @@ type Environment interface { GetSystemUUID() (hostId string) ReadDirectory(dir string, ext string) ([]string, error) WriteFiles(backup ConfigApplyMarker, files []*proto.File, prefix string, allowedDirs map[string]struct{}) error + WriteFile(backup ConfigApplyMarker, file *proto.File, confPath string) error + DeleteFile(backup ConfigApplyMarker, fileName string) error Processes() (result []*Process) FileStat(path string) (os.FileInfo, error) DiskDevices() ([]string, error) @@ -236,13 +238,69 @@ func (env *EnvironmentType) WriteFiles(backup ConfigApplyMarker, files []*proto. } for _, file := range files { - if err = writeFile(backup, file, confPath); err != nil { + if err = env.WriteFile(backup, file, confPath); err != nil { return err } } return nil } +// WriteFile writes the provided file content to disk. If the file.GetName() returns an absolute path, it'll be written +// to the path. Otherwise, it'll be written to the path relative to the provided confPath. +func (env *EnvironmentType) WriteFile(backup ConfigApplyMarker, file *proto.File, confPath string) error { + fileFullPath := file.GetName() + if !filepath.IsAbs(fileFullPath) { + fileFullPath = filepath.Join(confPath, fileFullPath) + } + + if err := backup.MarkAndSave(fileFullPath); err != nil { + return err + } + permissions := files.GetFileMode(file.GetPermissions()) + + directory := filepath.Dir(fileFullPath) + _, err := os.Stat(directory) + if os.IsNotExist(err) { + log.Debugf("Creating directory %s with permissions 755", directory) + err = os.MkdirAll(directory, 0o755) + if err != nil { + return err + } + } + + if err := os.WriteFile(fileFullPath, file.GetContents(), permissions); err != nil { + // If the file didn't exist originally and failed to be created + // Then remove that file from the backup so that the rollback doesn't try to delete the file + if _, err := os.Stat(fileFullPath); !errors.Is(err, os.ErrNotExist) { + backup.RemoveFromNotExists(fileFullPath) + } + return err + } + + log.Debugf("Wrote file %s", fileFullPath) + return nil +} + +func (env *EnvironmentType) DeleteFile(backup ConfigApplyMarker, fileName string) error { + if found, foundErr := FileExists(fileName); !found { + if foundErr == nil { + log.Debugf("skip delete for non-existing file: %s", fileName) + return nil + } + // possible perm deny, depends on platform + log.Warnf("file exists returned for %s: %s", fileName, foundErr) + return foundErr + } + if err := backup.MarkAndSave(fileName); err != nil { + return err + } + if err := os.Remove(fileName); err != nil { + return err + } + + return nil +} + func (env *EnvironmentType) IsContainer() bool { const ( dockerEnv = "/.dockerenv" @@ -451,42 +509,6 @@ func allowedFile(path string, allowedDirs map[string]struct{}) bool { return false } -// writeFile writes the provided file content to disk. If the file.GetName() returns an absolute path, it'll be written -// to the path. Otherwise, it'll be written to the path relative to the provided confPath. -func writeFile(backup ConfigApplyMarker, file *proto.File, confPath string) error { - fileFullPath := file.GetName() - if !filepath.IsAbs(fileFullPath) { - fileFullPath = filepath.Join(confPath, fileFullPath) - } - - if err := backup.MarkAndSave(fileFullPath); err != nil { - return err - } - permissions := files.GetFileMode(file.GetPermissions()) - - directory := filepath.Dir(fileFullPath) - _, err := os.Stat(directory) - if os.IsNotExist(err) { - log.Debugf("Creating directory %s with permissions 755", directory) - err = os.MkdirAll(directory, 0o755) - if err != nil { - return err - } - } - - if err := os.WriteFile(fileFullPath, file.GetContents(), permissions); err != nil { - // If the file didn't exist originally and failed to be created - // Then remove that file from the backup so that the rollback doesn't try to delete the file - if _, err := os.Stat(fileFullPath); !errors.Is(err, os.ErrNotExist) { - backup.RemoveFromNotExists(fileFullPath) - } - return err - } - - log.Debugf("Wrote file %s", fileFullPath) - return nil -} - func (env *EnvironmentType) FileStat(path string) (os.FileInfo, error) { // TODO: check if allowed list return os.Stat(path) diff --git a/test/integration/vendor/github.com/nginx/agent/v2/src/core/nginx.go b/test/integration/vendor/github.com/nginx/agent/v2/src/core/nginx.go index 911238715..584dd1d22 100644 --- a/test/integration/vendor/github.com/nginx/agent/v2/src/core/nginx.go +++ b/test/integration/vendor/github.com/nginx/agent/v2/src/core/nginx.go @@ -394,39 +394,57 @@ func (n *NginxBinaryType) WriteConfig(config *proto.NginxConfig) (*sdk.ConfigApp return nil, fmt.Errorf("config directory %s not allowed", filepath.Dir(details.ConfPath)) } + unpackMutex.Lock() + defer unpackMutex.Unlock() + + log.Info("Updating NGINX config") + var configApply *sdk.ConfigApply + + filesToUpdate, filesToDelete, allFilesHaveAnAction := generateActionMaps(config.DirectoryMap, n.config.AllowedDirectoriesMap) + + if allFilesHaveAnAction { + configApply, err = n.writeConfigWithWithFileActions(config, details, filesToUpdate, filesToDelete) + } else { + log.Debug("all files in directory map have no action set") + // If the file action is unset for any file then all files are written to disk and a diff is performed to determine what files need to be deleted + configApply, err = n.writeConfigWithNoFileActions(details, config, systemNginxConfig) + } + + if err != nil { + return configApply, err + } + + return configApply, nil +} + +func (n *NginxBinaryType) writeConfigWithNoFileActions(details *proto.NginxDetails, config *proto.NginxConfig, systemNginxConfig *proto.NginxConfig) (*sdk.ConfigApply, error) { confFiles, auxFiles, err := sdk.GetNginxConfigFiles(config) if err != nil { return nil, err } + configApply, err := sdk.NewConfigApplyWithIgnoreDirectives(details.ConfPath, n.config.AllowedDirectoriesMap, n.config.IgnoreDirectives) + if err != nil { + log.Warnf("config_apply error: %s", err) + return configApply, err + } + // Ensure this config request does not remove the process config if !hasConfPath(confFiles, details.ConfPath) { - return nil, fmt.Errorf("should not delete %s", details.ConfPath) + return configApply, fmt.Errorf("should not delete %s", details.ConfPath) } // Ensure all config files are within the allowed list directories. confDir := filepath.Dir(details.ConfPath) if err := ensureFilesAllowed(confFiles, n.config.AllowedDirectoriesMap, confDir); err != nil { - return nil, err + return configApply, err } // Ensure all aux files are within the allowed list directories. if err := ensureFilesAllowed(auxFiles, n.config.AllowedDirectoriesMap, config.GetZaux().GetRootDirectory()); err != nil { - return nil, err - } - - unpackMutex.Lock() - defer unpackMutex.Unlock() - - log.Info("Updating NGINX config") - var configApply *sdk.ConfigApply - configApply, err = sdk.NewConfigApplyWithIgnoreDirectives(details.ConfPath, n.config.AllowedDirectoriesMap, n.config.IgnoreDirectives) - if err != nil { - log.Warnf("config_apply error: %s", err) - return nil, err + return configApply, err } - // TODO: return to Control Plane that there was a rollback err = n.env.WriteFiles(configApply, confFiles, filepath.Dir(details.ConfPath), n.config.AllowedDirectoriesMap) if err != nil { log.Warnf("configuration write failed: %s", err) @@ -458,24 +476,68 @@ func (n *NginxBinaryType) WriteConfig(config *proto.NginxConfig) (*sdk.ConfigApp 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 := n.env.DeleteFile(configApply, file); err != nil { + return configApply, err } - if err = configApply.MarkAndSave(file); err != nil { + + fileDeleted[file] = struct{}{} + } + + return configApply, nil +} + +func (n *NginxBinaryType) writeConfigWithWithFileActions(config *proto.NginxConfig, details *proto.NginxDetails, filesToUpdate map[string]proto.File_Action, filesToDelete map[string]proto.File_Action) (*sdk.ConfigApply, error) { + confFiles, auxFiles, err := sdk.GetNginxConfigFiles(config) + if err != nil { + return nil, err + } + + configApply, err := sdk.NewConfigApplyWithIgnoreDirectives("", n.config.AllowedDirectoriesMap, n.config.IgnoreDirectives) + if err != nil { + log.Warnf("config_apply error: %s", err) + return nil, err + } + + for _, file := range confFiles { + rootDirectoryPath := filepath.Dir(details.ConfPath) + if _, found := filesToUpdate[file.Name]; !found { + log.Debugf("No action found for config file %s.", file.Name) + continue + } + + delete(filesToUpdate, file.Name) + + if err := n.env.WriteFile(configApply, file, rootDirectoryPath); err != nil { + log.Warnf("configuration write failed: %s", err) return configApply, err } - if err = os.Remove(file); err != nil { + } + + for _, file := range auxFiles { + rootDirectoryPath := config.GetZaux().GetRootDirectory() + if _, found := filesToUpdate[file.Name]; !found { + log.Debugf("No action found for aux file %s.", file.Name) + continue + } + + delete(filesToUpdate, file.Name) + + if err := n.env.WriteFile(configApply, file, rootDirectoryPath); err != nil { + log.Warnf("configuration write failed: %s", err) return configApply, err } - fileDeleted[file] = struct{}{} } + for file, action := range filesToUpdate { + log.Warnf("File %s missing from NginxConfig message. Unable to perform action %s.", file, action.String()) + } + + for file := range filesToDelete { + log.Infof("Deleting file: %s", file) + if err := n.env.DeleteFile(configApply, file); err != nil { + return configApply, err + } + } return configApply, nil } @@ -514,6 +576,54 @@ func generateDeleteFromDirectoryMap( return deleteFiles, actionIsSet } +func generateActionMaps( + directoryMap *proto.DirectoryMap, + allowedDirectory map[string]struct{}, +) ( + filesToUpdate map[string]proto.File_Action, + filesToDelete map[string]proto.File_Action, + allFilesHaveAnAction bool, +) { + allFilesHaveAnAction = true + filesToUpdate = make(map[string]proto.File_Action) + filesToDelete = make(map[string]proto.File_Action) + + for _, dir := range directoryMap.Directories { + for _, f := range dir.Files { + path := filepath.Join(dir.Name, f.Name) + + log.Debugf("file %s has action %v", path, f.Action) + + // Can't support relative paths + if !filepath.IsAbs(path) { + continue + } + + if !allowedFile(path, allowedDirectory) { + continue + } + + if f.Action == proto.File_unset { + allFilesHaveAnAction = false + return + } + + if f.Action == proto.File_add || f.Action == proto.File_update { + filesToUpdate[path] = f.Action + continue + } + + if f.Action == proto.File_delete { + filesToDelete[path] = f.Action + continue + } + + } + } + + return filesToUpdate, filesToDelete, allFilesHaveAnAction +} + func (n *NginxBinaryType) ReadConfig(confFile, nginxId, systemId string) (*proto.NginxConfig, error) { configPayload, err := sdk.GetNginxConfigWithIgnoreDirectives(confFile, nginxId, systemId, n.config.AllowedDirectoriesMap, n.config.IgnoreDirectives) if err != nil { diff --git a/test/integration/vendor/github.com/nginx/agent/v2/test/utils/environment.go b/test/integration/vendor/github.com/nginx/agent/v2/test/utils/environment.go index af85f0fe6..da6787a24 100644 --- a/test/integration/vendor/github.com/nginx/agent/v2/test/utils/environment.go +++ b/test/integration/vendor/github.com/nginx/agent/v2/test/utils/environment.go @@ -95,6 +95,16 @@ func (m *MockEnvironment) WriteFiles(backup core.ConfigApplyMarker, files []*pro return nil } +func (m *MockEnvironment) WriteFile(backup core.ConfigApplyMarker, file *proto.File, confPath string) error { + m.Called(backup, file, confPath) + return nil +} + +func (m *MockEnvironment) DeleteFile(backup core.ConfigApplyMarker, fileName string) error { + m.Called(backup, fileName) + return nil +} + func (m *MockEnvironment) FileStat(path string) (os.FileInfo, error) { m.Called(path) return os.Stat(path) diff --git a/test/performance/environment_test.go b/test/performance/environment_test.go index f0a85d651..ab99c9856 100644 --- a/test/performance/environment_test.go +++ b/test/performance/environment_test.go @@ -106,6 +106,38 @@ func BenchmarkWriteFiles(b *testing.B) { } } +func BenchmarkWriteFile(b *testing.B) { + file := &proto.File{ + Name: "/tmp/test.conf", + Contents: []byte("contents"), + Permissions: "0644", + } + + backup, err := sdk.NewConfigApplyWithIgnoreDirectives("", nil, []string{}) + assert.NoError(b, err) + b.ResetTimer() + + env := &core.EnvironmentType{} + for i := 0; i < b.N; i++ { + env.WriteFile(backup, file, "/tmp") + os.Remove("/tmp/test.conf") + } +} + +func BenchmarkDeleteFile(b *testing.B) { + fileName := "/tmp/test.conf" + + backup, err := sdk.NewConfigApplyWithIgnoreDirectives("", nil, []string{}) + assert.NoError(b, err) + b.ResetTimer() + + env := &core.EnvironmentType{} + for i := 0; i < b.N; i++ { + os.Create(fileName) + env.DeleteFile(backup, fileName) + } +} + func BenchmarkFileStat(b *testing.B) { tempDir := b.TempDir() tempFile := tempDir + "/test.txt" diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/core/environment.go b/test/performance/vendor/github.com/nginx/agent/v2/src/core/environment.go index 67d8defab..4fdaaa401 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/core/environment.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/core/environment.go @@ -39,7 +39,7 @@ import ( //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -generate //counterfeiter:generate -o fake_environment_test.go . Environment -//go:generate sh -c "grep -v agent/product/nginx-agent/v2/core fake_environment_test.go | sed -e s\\/core\\\\.\\/\\/g > fake_environment_fixed.go" +//go:generate sh -c "grep -v github.com/nginx/agent/v2/src/core fake_environment_test.go | sed -e s\\/core\\\\.\\/\\/g > fake_environment_fixed.go" //go:generate mv fake_environment_fixed.go fake_environment_test.go type Environment interface { NewHostInfo(agentVersion string, tags *[]string, configDirs string, clearCache bool) *proto.HostInfo @@ -48,6 +48,8 @@ type Environment interface { GetSystemUUID() (hostId string) ReadDirectory(dir string, ext string) ([]string, error) WriteFiles(backup ConfigApplyMarker, files []*proto.File, prefix string, allowedDirs map[string]struct{}) error + WriteFile(backup ConfigApplyMarker, file *proto.File, confPath string) error + DeleteFile(backup ConfigApplyMarker, fileName string) error Processes() (result []*Process) FileStat(path string) (os.FileInfo, error) DiskDevices() ([]string, error) @@ -236,13 +238,69 @@ func (env *EnvironmentType) WriteFiles(backup ConfigApplyMarker, files []*proto. } for _, file := range files { - if err = writeFile(backup, file, confPath); err != nil { + if err = env.WriteFile(backup, file, confPath); err != nil { return err } } return nil } +// WriteFile writes the provided file content to disk. If the file.GetName() returns an absolute path, it'll be written +// to the path. Otherwise, it'll be written to the path relative to the provided confPath. +func (env *EnvironmentType) WriteFile(backup ConfigApplyMarker, file *proto.File, confPath string) error { + fileFullPath := file.GetName() + if !filepath.IsAbs(fileFullPath) { + fileFullPath = filepath.Join(confPath, fileFullPath) + } + + if err := backup.MarkAndSave(fileFullPath); err != nil { + return err + } + permissions := files.GetFileMode(file.GetPermissions()) + + directory := filepath.Dir(fileFullPath) + _, err := os.Stat(directory) + if os.IsNotExist(err) { + log.Debugf("Creating directory %s with permissions 755", directory) + err = os.MkdirAll(directory, 0o755) + if err != nil { + return err + } + } + + if err := os.WriteFile(fileFullPath, file.GetContents(), permissions); err != nil { + // If the file didn't exist originally and failed to be created + // Then remove that file from the backup so that the rollback doesn't try to delete the file + if _, err := os.Stat(fileFullPath); !errors.Is(err, os.ErrNotExist) { + backup.RemoveFromNotExists(fileFullPath) + } + return err + } + + log.Debugf("Wrote file %s", fileFullPath) + return nil +} + +func (env *EnvironmentType) DeleteFile(backup ConfigApplyMarker, fileName string) error { + if found, foundErr := FileExists(fileName); !found { + if foundErr == nil { + log.Debugf("skip delete for non-existing file: %s", fileName) + return nil + } + // possible perm deny, depends on platform + log.Warnf("file exists returned for %s: %s", fileName, foundErr) + return foundErr + } + if err := backup.MarkAndSave(fileName); err != nil { + return err + } + if err := os.Remove(fileName); err != nil { + return err + } + + return nil +} + func (env *EnvironmentType) IsContainer() bool { const ( dockerEnv = "/.dockerenv" @@ -451,42 +509,6 @@ func allowedFile(path string, allowedDirs map[string]struct{}) bool { return false } -// writeFile writes the provided file content to disk. If the file.GetName() returns an absolute path, it'll be written -// to the path. Otherwise, it'll be written to the path relative to the provided confPath. -func writeFile(backup ConfigApplyMarker, file *proto.File, confPath string) error { - fileFullPath := file.GetName() - if !filepath.IsAbs(fileFullPath) { - fileFullPath = filepath.Join(confPath, fileFullPath) - } - - if err := backup.MarkAndSave(fileFullPath); err != nil { - return err - } - permissions := files.GetFileMode(file.GetPermissions()) - - directory := filepath.Dir(fileFullPath) - _, err := os.Stat(directory) - if os.IsNotExist(err) { - log.Debugf("Creating directory %s with permissions 755", directory) - err = os.MkdirAll(directory, 0o755) - if err != nil { - return err - } - } - - if err := os.WriteFile(fileFullPath, file.GetContents(), permissions); err != nil { - // If the file didn't exist originally and failed to be created - // Then remove that file from the backup so that the rollback doesn't try to delete the file - if _, err := os.Stat(fileFullPath); !errors.Is(err, os.ErrNotExist) { - backup.RemoveFromNotExists(fileFullPath) - } - return err - } - - log.Debugf("Wrote file %s", fileFullPath) - return nil -} - func (env *EnvironmentType) FileStat(path string) (os.FileInfo, error) { // TODO: check if allowed list return os.Stat(path) 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 911238715..584dd1d22 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 @@ -394,39 +394,57 @@ func (n *NginxBinaryType) WriteConfig(config *proto.NginxConfig) (*sdk.ConfigApp return nil, fmt.Errorf("config directory %s not allowed", filepath.Dir(details.ConfPath)) } + unpackMutex.Lock() + defer unpackMutex.Unlock() + + log.Info("Updating NGINX config") + var configApply *sdk.ConfigApply + + filesToUpdate, filesToDelete, allFilesHaveAnAction := generateActionMaps(config.DirectoryMap, n.config.AllowedDirectoriesMap) + + if allFilesHaveAnAction { + configApply, err = n.writeConfigWithWithFileActions(config, details, filesToUpdate, filesToDelete) + } else { + log.Debug("all files in directory map have no action set") + // If the file action is unset for any file then all files are written to disk and a diff is performed to determine what files need to be deleted + configApply, err = n.writeConfigWithNoFileActions(details, config, systemNginxConfig) + } + + if err != nil { + return configApply, err + } + + return configApply, nil +} + +func (n *NginxBinaryType) writeConfigWithNoFileActions(details *proto.NginxDetails, config *proto.NginxConfig, systemNginxConfig *proto.NginxConfig) (*sdk.ConfigApply, error) { confFiles, auxFiles, err := sdk.GetNginxConfigFiles(config) if err != nil { return nil, err } + configApply, err := sdk.NewConfigApplyWithIgnoreDirectives(details.ConfPath, n.config.AllowedDirectoriesMap, n.config.IgnoreDirectives) + if err != nil { + log.Warnf("config_apply error: %s", err) + return configApply, err + } + // Ensure this config request does not remove the process config if !hasConfPath(confFiles, details.ConfPath) { - return nil, fmt.Errorf("should not delete %s", details.ConfPath) + return configApply, fmt.Errorf("should not delete %s", details.ConfPath) } // Ensure all config files are within the allowed list directories. confDir := filepath.Dir(details.ConfPath) if err := ensureFilesAllowed(confFiles, n.config.AllowedDirectoriesMap, confDir); err != nil { - return nil, err + return configApply, err } // Ensure all aux files are within the allowed list directories. if err := ensureFilesAllowed(auxFiles, n.config.AllowedDirectoriesMap, config.GetZaux().GetRootDirectory()); err != nil { - return nil, err - } - - unpackMutex.Lock() - defer unpackMutex.Unlock() - - log.Info("Updating NGINX config") - var configApply *sdk.ConfigApply - configApply, err = sdk.NewConfigApplyWithIgnoreDirectives(details.ConfPath, n.config.AllowedDirectoriesMap, n.config.IgnoreDirectives) - if err != nil { - log.Warnf("config_apply error: %s", err) - return nil, err + return configApply, err } - // TODO: return to Control Plane that there was a rollback err = n.env.WriteFiles(configApply, confFiles, filepath.Dir(details.ConfPath), n.config.AllowedDirectoriesMap) if err != nil { log.Warnf("configuration write failed: %s", err) @@ -458,24 +476,68 @@ func (n *NginxBinaryType) WriteConfig(config *proto.NginxConfig) (*sdk.ConfigApp 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 := n.env.DeleteFile(configApply, file); err != nil { + return configApply, err } - if err = configApply.MarkAndSave(file); err != nil { + + fileDeleted[file] = struct{}{} + } + + return configApply, nil +} + +func (n *NginxBinaryType) writeConfigWithWithFileActions(config *proto.NginxConfig, details *proto.NginxDetails, filesToUpdate map[string]proto.File_Action, filesToDelete map[string]proto.File_Action) (*sdk.ConfigApply, error) { + confFiles, auxFiles, err := sdk.GetNginxConfigFiles(config) + if err != nil { + return nil, err + } + + configApply, err := sdk.NewConfigApplyWithIgnoreDirectives("", n.config.AllowedDirectoriesMap, n.config.IgnoreDirectives) + if err != nil { + log.Warnf("config_apply error: %s", err) + return nil, err + } + + for _, file := range confFiles { + rootDirectoryPath := filepath.Dir(details.ConfPath) + if _, found := filesToUpdate[file.Name]; !found { + log.Debugf("No action found for config file %s.", file.Name) + continue + } + + delete(filesToUpdate, file.Name) + + if err := n.env.WriteFile(configApply, file, rootDirectoryPath); err != nil { + log.Warnf("configuration write failed: %s", err) return configApply, err } - if err = os.Remove(file); err != nil { + } + + for _, file := range auxFiles { + rootDirectoryPath := config.GetZaux().GetRootDirectory() + if _, found := filesToUpdate[file.Name]; !found { + log.Debugf("No action found for aux file %s.", file.Name) + continue + } + + delete(filesToUpdate, file.Name) + + if err := n.env.WriteFile(configApply, file, rootDirectoryPath); err != nil { + log.Warnf("configuration write failed: %s", err) return configApply, err } - fileDeleted[file] = struct{}{} } + for file, action := range filesToUpdate { + log.Warnf("File %s missing from NginxConfig message. Unable to perform action %s.", file, action.String()) + } + + for file := range filesToDelete { + log.Infof("Deleting file: %s", file) + if err := n.env.DeleteFile(configApply, file); err != nil { + return configApply, err + } + } return configApply, nil } @@ -514,6 +576,54 @@ func generateDeleteFromDirectoryMap( return deleteFiles, actionIsSet } +func generateActionMaps( + directoryMap *proto.DirectoryMap, + allowedDirectory map[string]struct{}, +) ( + filesToUpdate map[string]proto.File_Action, + filesToDelete map[string]proto.File_Action, + allFilesHaveAnAction bool, +) { + allFilesHaveAnAction = true + filesToUpdate = make(map[string]proto.File_Action) + filesToDelete = make(map[string]proto.File_Action) + + for _, dir := range directoryMap.Directories { + for _, f := range dir.Files { + path := filepath.Join(dir.Name, f.Name) + + log.Debugf("file %s has action %v", path, f.Action) + + // Can't support relative paths + if !filepath.IsAbs(path) { + continue + } + + if !allowedFile(path, allowedDirectory) { + continue + } + + if f.Action == proto.File_unset { + allFilesHaveAnAction = false + return + } + + if f.Action == proto.File_add || f.Action == proto.File_update { + filesToUpdate[path] = f.Action + continue + } + + if f.Action == proto.File_delete { + filesToDelete[path] = f.Action + continue + } + + } + } + + return filesToUpdate, filesToDelete, allFilesHaveAnAction +} + func (n *NginxBinaryType) ReadConfig(confFile, nginxId, systemId string) (*proto.NginxConfig, error) { configPayload, err := sdk.GetNginxConfigWithIgnoreDirectives(confFile, nginxId, systemId, n.config.AllowedDirectoriesMap, n.config.IgnoreDirectives) if err != nil { diff --git a/test/performance/vendor/github.com/nginx/agent/v2/test/utils/environment.go b/test/performance/vendor/github.com/nginx/agent/v2/test/utils/environment.go index af85f0fe6..da6787a24 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/test/utils/environment.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/test/utils/environment.go @@ -95,6 +95,16 @@ func (m *MockEnvironment) WriteFiles(backup core.ConfigApplyMarker, files []*pro return nil } +func (m *MockEnvironment) WriteFile(backup core.ConfigApplyMarker, file *proto.File, confPath string) error { + m.Called(backup, file, confPath) + return nil +} + +func (m *MockEnvironment) DeleteFile(backup core.ConfigApplyMarker, fileName string) error { + m.Called(backup, fileName) + return nil +} + func (m *MockEnvironment) FileStat(path string) (os.FileInfo, error) { m.Called(path) return os.Stat(path) diff --git a/test/utils/environment.go b/test/utils/environment.go index af85f0fe6..da6787a24 100644 --- a/test/utils/environment.go +++ b/test/utils/environment.go @@ -95,6 +95,16 @@ func (m *MockEnvironment) WriteFiles(backup core.ConfigApplyMarker, files []*pro return nil } +func (m *MockEnvironment) WriteFile(backup core.ConfigApplyMarker, file *proto.File, confPath string) error { + m.Called(backup, file, confPath) + return nil +} + +func (m *MockEnvironment) DeleteFile(backup core.ConfigApplyMarker, fileName string) error { + m.Called(backup, fileName) + return nil +} + func (m *MockEnvironment) FileStat(path string) (os.FileInfo, error) { m.Called(path) return os.Stat(path)