NGINX Configs to Reference Remote External Files #1389

Akshay2191 · 2025-11-12T15:50:37Z

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.

I have read the CONTRIBUTING document
I have run make install-tools and have attached any dependency changes to this pull request
If applicable, I have added tests that prove my fix is effective or that my feature works
If applicable, I have checked that any relevant tests pass after adding my changes
If applicable, I have updated any relevant documentation (README.md)
If applicable, I have tested my cross-platform changes on Ubuntu 22, Redhat 8, SUSE 15 and FreeBSD 13

codecov · 2025-11-12T16:03:00Z

Codecov Report

❌ Patch coverage is 89.49772% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.51%. Comparing base (b1947ec) to head (bd60017).
⚠️ Report is 21 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/file/file_manager_service.go	87.17%	12 Missing and 8 partials ⚠️
internal/config/config.go	95.23%	1 Missing and 1 partial ⚠️
internal/file/file_service_operator.go	95.23%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1389      +/-   ##
==========================================
+ Coverage   86.31%   86.51%   +0.20%     
==========================================
  Files         102      102              
  Lines       12603    12801     +198     
==========================================
+ Hits        10878    11075     +197     
+ Misses       1249     1242       -7     
- Partials      476      484       +8

Files with missing lines	Coverage Δ
internal/config/defaults.go	`100.00% <ø> (ø)`
internal/config/flags.go	`100.00% <ø> (ø)`
internal/config/types.go	`87.65% <ø> (ø)`
internal/model/config.go	`90.47% <ø> (ø)`
internal/file/file_service_operator.go	`77.32% <95.23%> (+1.18%)`	⬆️
internal/config/config.go	`87.60% <95.23%> (+0.29%)`	⬆️
internal/file/file_manager_service.go	`78.89% <87.17%> (+3.50%)`	⬆️

... and 6 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b1947ec...bd60017. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

internal/config/config.go

internal/config/config_test.go

internal/file/file_manager_service.go

aphralG · 2025-11-20T11:34:09Z

internal/file/file_manager_service.go


 			continue
-		case model.Delete, model.Update:
+		case model.Delete, model.Update, model.ExternalFile:


With the external files are the files still considered updated, added or deleted? Just wondering instead should it be a field added to the file rather than an action this might fix a few of the things the linter had an issue with which required the checking of Adding and updating in the switch statement

Yes, external files still involve Add and Update actions. The challenge is that these actions are only confirmed after a call to the third-party server. Because we must make this external call every time a reference to the third-party file is encountered, we introduced a new flow that triggers based on the presence of the External File URL.

internal/file/file_manager_service.go

internal/file/file_manager_service_test.go

internal/config/config.go

internal/config/defaults.go

dhurley · 2025-11-19T16:33:04Z

internal/file/file_service_operator.go

 	return fso.sendUpdateFileStream(ctx, fileToUpdate, fso.agentConfig.Client.Grpc.FileChunkSize)
 }

+func (fso *FileServiceOperator) RenameExternalFile(


instead of duplicating the RenameFile function can we update the RenameFile function to just rename the file and have the hash validation in a separate function like this:

func (fso *FileServiceOperator) RenameAndValidateFile( ctx context.Context, source, destination string, ) error { err := fso.RenameFile(ctx, source, destination) if err != nil { return err } return fso.validateFileHash(desination, hash) }

The core issue with the current implementation is that the function call lacks necessary attributes to make a decision. Specifically, we should only proceed with the hash validation if File.GetExternalDataSource() returns an empty value.

internal/file/file_manager_service.go

craigell · 2025-11-25T12:32:04Z

internal/file/file_service_operator_test.go

+		wantErrMsg string
+		wantErr    bool
+	}{
+		{


The following scenarios could increase coverage:

No destination specified

Restricted directory.

Two files to the same destination.

aphralG · 2025-11-26T11:25:00Z

internal/file/file_service_operator_test.go

+	}{
+		{
+			name: "Test 1: success",
+			prepare: func(t *testing.T) (string, string) {


Can this be changed to follow the way other tests are set up and formatted

internal/config/defaults.go

aphralG · 2025-11-26T11:29:04Z

internal/file/file_manager_service_test.go

+}
+
+//nolint:gocognit,revive,govet // cognitive complexity is 22
+func TestFileManagerService_downloadExternalFiles_Cases(t *testing.T) {


Can this test be simplified and changed to match the way other tests are set up and structured

aphralG · 2025-11-26T11:29:59Z

internal/file/file_manager_service_test.go

+	assert.Equal(t, fileName, dstArg, "RenameExternalFile destination argument mismatch")
+}
+
+func TestDownloadFileContent_MaxBytesLimit(t *testing.T) {


Can these tests please be combined

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.

internal/file/file_manager_service_test.go

aphralG · 2025-11-26T11:33:52Z

internal/file/file_manager_service.go

 	return filepath.Join(filepath.Dir(fileName), tempFileName)
 }
+
+func (fms *FileManagerService) downloadExternalFile(ctx context.Context, fileAction *model.FileCache,


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

internal/file/file_manager_service.go

spencerugbo · 2025-12-04T15:08:15Z

internal/file/file_manager_service.go

 	for _, fileAction := range fms.fileActions {
+		var err error
+		fileMeta := fileAction.File.GetFileMeta()
+		tempFilePath := tempFilePath(fileAction.File.GetFileMeta().GetName())


Could fileMeta be used more instead of fileAction.File.GetFileMeta() throughout this for loop?

Suggested change

tempFilePath := tempFilePath(fileAction.File.GetFileMeta().GetName())

tempFilePath := tempFilePath(fileMeta.GetName())

spencerugbo · 2025-12-04T15:28:59Z

internal/config/config.go

 	regexLabelPattern = "^[a-zA-Z0-9]([a-zA-Z0-9-_]{0,254}[a-zA-Z0-9])?$"
 )

+var domainRegex = regexp.MustCompile(


Could we group the two var declarations into one block?

Suggested change

var domainRegex = regexp.MustCompile(

var (

domainRegex = regexp.MustCompile(

`^(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])$`,

)

viperInstance = viper.NewWithOptions(viper.KeyDelimiter(KeyDelimiter))

)

Akshay2191 added 2 commits November 12, 2025 15:29

third party config support

ae7fa14

lint fix

14f1190

Akshay2191 requested a review from a team as a code owner November 12, 2025 15:50

github-actions bot added the chore Pull requests for routine tasks label Nov 12, 2025

Merge branch 'main' into external-file-mgmt-integration

84b2310

Akshay2191 added 8 commits November 13, 2025 12:12

Added a generic file timeout

d31b2df

merging main to resolve conflicts

b7a5c70

added generic timeout to create connection

501e08f

added uT

b7eaef7

Added UT

a83e45b

fix lint issues

529c839

Added UT for Proxy URL

2a51d22

increasing UT coverage

f22834f

Akshay2191 self-assigned this Nov 19, 2025

updated Mock management to test external file config apply

4a2b917

aphralG reviewed Nov 20, 2025

View reviewed changes

dhurley reviewed Nov 21, 2025

View reviewed changes

PR review comments addressed

b7759e1

Akshay2191 requested a review from dhurley November 24, 2025 16:22

worked on PR comments

bd60017

Akshay2191 requested a review from aphralG November 25, 2025 09:06

craigell reviewed Nov 25, 2025

View reviewed changes

aphralG reviewed Nov 26, 2025

View reviewed changes

worked on PR comments

38913b5

spencerugbo reviewed Dec 4, 2025

View reviewed changes

	tempFilePath := tempFilePath(fileAction.File.GetFileMeta().GetName())
	tempFilePath := tempFilePath(fileMeta.GetName())

NGINX Configs to Reference Remote External Files #1389

Are you sure you want to change the base?

NGINX Configs to Reference Remote External Files #1389

Uh oh!

Conversation

Akshay2191 commented Nov 12, 2025

Checklist

Uh oh!

codecov bot commented Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

codecov bot commented Nov 12, 2025 •

edited

Loading