NGINX Configs to Reference Remote External Files #1389
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1389 +/- ##
==========================================
+ Coverage 85.31% 85.37% +0.05%
==========================================
Files 102 102
Lines 12974 13165 +191
==========================================
+ Hits 11069 11239 +170
- Misses 1418 1432 +14
- Partials 487 494 +7
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
| wantErrMsg string | ||
| wantErr bool | ||
| }{ | ||
| { |
There was a problem hiding this comment.
The following scenarios could increase coverage:
- No destination specified
- Restricted directory.
- Two files to the same destination.
| }{ | ||
| { | ||
| name: "Test 1: success", | ||
| prepare: func(t *testing.T) (string, string) { |
There was a problem hiding this comment.
Can this be changed to follow the way other tests are set up and formatted
| } | ||
|
|
||
| //nolint:gocognit,revive,govet // cognitive complexity is 22 | ||
| func TestFileManagerService_downloadExternalFiles_Cases(t *testing.T) { |
There was a problem hiding this comment.
Can this test be simplified and changed to match the way other tests are set up and structured
| assert.Equal(t, fileName, dstArg, "RenameExternalFile destination argument mismatch") | ||
| } | ||
|
|
||
| func TestDownloadFileContent_MaxBytesLimit(t *testing.T) { |
There was a problem hiding this comment.
Can these tests please be combined
There was a problem hiding this comment.
I tried to consolidate these divergent test scenarios (MaxBytes vs. Proxy Errors) but it spiked the cognitive complexity to 23 due the very different validation steps. Therefore I kept them seperate.
| return filepath.Join(filepath.Dir(fileName), tempFileName) | ||
| } | ||
|
|
||
| func (fms *FileManagerService) downloadExternalFile(ctx context.Context, fileAction *model.FileCache, |
There was a problem hiding this comment.
Could we add an extra operator maybe like the file operator to handle the external file download as this file is getting a little too large and complex
github-actions
bot
added
dependencies
documentation
# Conflicts: # internal/config/config.go # internal/config/types.go # internal/file/file_manager_service.go
# Conflicts: # internal/file/file_manager_service.go
# Conflicts: # internal/config/config.go # internal/config/types.go # internal/file/file_manager_service.go
Akshay2191
force-pushed
the
external-file-mgmt-integration
branch
from
e50d100 to
2e981e0
Compare
Extends the ConfigApply capability to manage NGINX configurations that reference external resources hosted at remote URLs. The agent now handles downloading of these files into the NGINX configuration directory before applying the new configuration.
Checklist
Before creating a PR, run through this checklist and mark each as complete.
CONTRIBUTINGdocumentmake install-toolsand have attached any dependency changes to this pull requestREADME.md)