From 40e44d085fcec11a17e0a96abe015125458b1c30 Mon Sep 17 00:00:00 2001 From: Spencer Ugbo Date: Wed, 16 Jul 2025 14:38:49 +0100 Subject: [PATCH 01/13] Implement test suites to reuse containers for tests --- .../managementplane/config_apply_test.go | 222 ++++++++++-------- .../managementplane/config_upload_test.go | 60 +++-- .../managementplane/file_watcher_test.go | 88 +++---- .../grpc_management_plane_api_test.go | 67 ++---- 4 files changed, 221 insertions(+), 216 deletions(-) diff --git a/test/integration/managementplane/config_apply_test.go b/test/integration/managementplane/config_apply_test.go index 6fcb48096..a2d2bdd32 100644 --- a/test/integration/managementplane/config_apply_test.go +++ b/test/integration/managementplane/config_apply_test.go @@ -15,8 +15,7 @@ import ( "github.com/nginx/agent/v3/test/integration/utils" mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" ) const ( @@ -24,134 +23,165 @@ const ( "number of arguments in \"worker_processes\" directive in /etc/nginx/nginx.conf:1" ) -func TestGrpc_ConfigApply(t *testing.T) { - ctx := context.Background() - teardownTest := utils.SetupConnectionTest(t, false, false, false, - "../../config/agent/nginx-config-with-grpc-client.conf") - defer teardownTest(t) +type ConfigApplyTestSuite struct { + suite.Suite + ctx context.Context + teardownTest func(testing.TB) + nginxInstanceID string +} - nginxInstanceID := utils.VerifyConnection(t, 2, utils.MockManagementPlaneAPIAddress) +type ConfigApplyChunkingTestSuite struct { + suite.Suite + ctx context.Context + teardownTest func(testing.TB) + nginxInstanceID string +} - responses := utils.ManagementPlaneResponses(t, 1, utils.MockManagementPlaneAPIAddress) - assert.Equal(t, mpi.CommandResponse_COMMAND_STATUS_OK, responses[0].GetCommandResponse().GetStatus()) - assert.Equal(t, "Successfully updated all files", responses[0].GetCommandResponse().GetMessage()) +func (s *ConfigApplyTestSuite) SetupSuite() { + s.ctx = context.Background() + s.teardownTest = utils.SetupConnectionTest(s.T(), false, false, false, + "../../config/agent/nginx-config-with-grpc-client.conf") + s.nginxInstanceID = utils.VerifyConnection(s.T(), 2, utils.MockManagementPlaneAPIAddress) +} - t.Run("Test 1: No config changes", func(t *testing.T) { - utils.ClearManagementPlaneResponses(t, utils.MockManagementPlaneAPIAddress) - utils.PerformConfigApply(t, nginxInstanceID, utils.MockManagementPlaneAPIAddress) - responses = utils.ManagementPlaneResponses(t, 1, utils.MockManagementPlaneAPIAddress) - t.Logf("Config apply responses: %v", responses) +func (s *ConfigApplyTestSuite) TearDownSuite() { + s.teardownTest(s.T()) +} - assert.Equal(t, mpi.CommandResponse_COMMAND_STATUS_OK, responses[0].GetCommandResponse().GetStatus()) - assert.Equal(t, "Config apply successful, no files to change", responses[0].GetCommandResponse().GetMessage()) - }) +func (s *ConfigApplyTestSuite) TearDownTest() { + utils.ClearManagementPlaneResponses(s.T(), utils.MockManagementPlaneAPIAddress) +} - t.Run("Test 2: Valid config", func(t *testing.T) { - utils.ClearManagementPlaneResponses(t, utils.MockManagementPlaneAPIAddress) - newConfigFile := "../../config/nginx/nginx-with-test-location.conf" +func (s *ConfigApplyTestSuite) TestConfigApply_Test1_TestSetup() { + responses := utils.ManagementPlaneResponses(s.T(), 1, utils.MockManagementPlaneAPIAddress) + s.Require().Equal(mpi.CommandResponse_COMMAND_STATUS_OK, responses[0].GetCommandResponse().GetStatus()) + s.Require().Equal("Successfully updated all files", responses[0].GetCommandResponse().GetMessage()) +} - if os.Getenv("IMAGE_PATH") == "/nginx-plus/agent" { - newConfigFile = "../../config/nginx/nginx-plus-with-test-location.conf" - } +func (s *ConfigApplyTestSuite) TestConfigApply_Test2_TestNoConfigChanges() { + utils.PerformConfigApply(s.T(), s.nginxInstanceID, utils.MockManagementPlaneAPIAddress) + responses := utils.ManagementPlaneResponses(s.T(), 1, utils.MockManagementPlaneAPIAddress) + s.T().Logf("Config apply responses: %v", responses) - err := utils.MockManagementPlaneGrpcContainer.CopyFileToContainer( - ctx, - newConfigFile, - fmt.Sprintf("/mock-management-plane-grpc/config/%s/etc/nginx/nginx.conf", nginxInstanceID), - 0o666, - ) - require.NoError(t, err) + s.Equal(mpi.CommandResponse_COMMAND_STATUS_OK, responses[0].GetCommandResponse().GetStatus()) + s.Equal("Config apply successful, no files to change", responses[0].GetCommandResponse().GetMessage()) +} - utils.PerformConfigApply(t, nginxInstanceID, utils.MockManagementPlaneAPIAddress) +func (s *ConfigApplyTestSuite) TestConfigApply_Test3_TestValidConfig() { + newConfigFile := "../../config/nginx/nginx-with-test-location.conf" - responses = utils.ManagementPlaneResponses(t, 2, utils.MockManagementPlaneAPIAddress) - t.Logf("Config apply responses: %v", responses) + if os.Getenv("IMAGE_PATH") == "/nginx-plus/agent" { + newConfigFile = "../../config/nginx/nginx-plus-with-test-location.conf" + } + err := utils.MockManagementPlaneGrpcContainer.CopyFileToContainer( + s.ctx, + newConfigFile, + fmt.Sprintf("/mock-management-plane-grpc/config/%s/etc/nginx/nginx.conf", s.nginxInstanceID), + 0o666, + ) + s.Require().NoError(err) - sort.Slice(responses, func(i, j int) bool { - return responses[i].GetCommandResponse().GetMessage() < responses[j].GetCommandResponse().GetMessage() - }) + utils.PerformConfigApply(s.T(), s.nginxInstanceID, utils.MockManagementPlaneAPIAddress) + responses := utils.ManagementPlaneResponses(s.T(), 2, utils.MockManagementPlaneAPIAddress) + s.T().Logf("Config apply responses: %v", responses) - assert.Equal(t, mpi.CommandResponse_COMMAND_STATUS_OK, responses[0].GetCommandResponse().GetStatus()) - assert.Equal(t, "Config apply successful", responses[0].GetCommandResponse().GetMessage()) - assert.Equal(t, mpi.CommandResponse_COMMAND_STATUS_OK, responses[1].GetCommandResponse().GetStatus()) - assert.Equal(t, "Successfully updated all files", responses[1].GetCommandResponse().GetMessage()) + sort.Slice(responses, func(i, j int) bool { + return responses[i].GetCommandResponse().GetMessage() < responses[j].GetCommandResponse().GetMessage() }) - t.Run("Test 3: Invalid config", func(t *testing.T) { - utils.ClearManagementPlaneResponses(t, utils.MockManagementPlaneAPIAddress) - err := utils.MockManagementPlaneGrpcContainer.CopyFileToContainer( - ctx, - "../../config/nginx/invalid-nginx.conf", - fmt.Sprintf("/mock-management-plane-grpc/config/%s/etc/nginx/nginx.conf", nginxInstanceID), - 0o666, - ) - require.NoError(t, err) - - utils.PerformConfigApply(t, nginxInstanceID, utils.MockManagementPlaneAPIAddress) - - responses = utils.ManagementPlaneResponses(t, 2, utils.MockManagementPlaneAPIAddress) - t.Logf("Config apply responses: %v", responses) - - assert.Equal(t, mpi.CommandResponse_COMMAND_STATUS_ERROR, responses[0].GetCommandResponse().GetStatus()) - assert.Equal(t, "Config apply failed, rolling back config", responses[0].GetCommandResponse().GetMessage()) - assert.Equal(t, configApplyErrorMessage, responses[0].GetCommandResponse().GetError()) - assert.Equal(t, mpi.CommandResponse_COMMAND_STATUS_FAILURE, responses[1].GetCommandResponse().GetStatus()) - assert.Equal(t, "Config apply failed, rollback successful", responses[1].GetCommandResponse().GetMessage()) - assert.Equal(t, configApplyErrorMessage, responses[1].GetCommandResponse().GetError()) - }) + s.Equal(mpi.CommandResponse_COMMAND_STATUS_OK, responses[0].GetCommandResponse().GetStatus()) + s.Equal("Config apply successful", responses[0].GetCommandResponse().GetMessage()) + s.Equal(mpi.CommandResponse_COMMAND_STATUS_OK, responses[1].GetCommandResponse().GetStatus()) + s.Equal("Successfully updated all files", responses[1].GetCommandResponse().GetMessage()) +} - t.Run("Test 4: File not in allowed directory", func(t *testing.T) { - utils.ClearManagementPlaneResponses(t, utils.MockManagementPlaneAPIAddress) - utils.PerformInvalidConfigApply(t, nginxInstanceID) +func (s *ConfigApplyTestSuite) TestConfigApply_Test4_TestInvalidConfig() { + err := utils.MockManagementPlaneGrpcContainer.CopyFileToContainer( + s.ctx, + "../../config/nginx/invalid-nginx.conf", + fmt.Sprintf("/mock-management-plane-grpc/config/%s/etc/nginx/nginx.conf", s.nginxInstanceID), + 0o666, + ) + s.Require().NoError(err) - responses = utils.ManagementPlaneResponses(t, 1, utils.MockManagementPlaneAPIAddress) - t.Logf("Config apply responses: %v", responses) + utils.PerformConfigApply(s.T(), s.nginxInstanceID, utils.MockManagementPlaneAPIAddress) - assert.Equal(t, mpi.CommandResponse_COMMAND_STATUS_FAILURE, responses[0].GetCommandResponse().GetStatus()) - assert.Equal(t, "Config apply failed", responses[0].GetCommandResponse().GetMessage()) - assert.Equal( - t, - "file not in allowed directories /unknown/nginx.conf", - responses[0].GetCommandResponse().GetError(), - ) - }) + responses := utils.ManagementPlaneResponses(s.T(), 2, utils.MockManagementPlaneAPIAddress) + s.T().Logf("Config apply responses: %v", responses) + + s.Equal(mpi.CommandResponse_COMMAND_STATUS_ERROR, responses[0].GetCommandResponse().GetStatus()) + s.Equal("Config apply failed, rolling back config", responses[0].GetCommandResponse().GetMessage()) + s.Equal(configApplyErrorMessage, responses[0].GetCommandResponse().GetError()) + s.Equal(mpi.CommandResponse_COMMAND_STATUS_FAILURE, responses[1].GetCommandResponse().GetStatus()) + s.Equal("Config apply failed, rollback successful", responses[1].GetCommandResponse().GetMessage()) + s.Equal(configApplyErrorMessage, responses[1].GetCommandResponse().GetError()) +} + +func (s *ConfigApplyTestSuite) TestConfigApply_Test5_TestFileNotInAllowedDirectory() { + utils.PerformInvalidConfigApply(s.T(), s.nginxInstanceID) + + responses := utils.ManagementPlaneResponses(s.T(), 1, utils.MockManagementPlaneAPIAddress) + s.T().Logf("Config apply responses: %v", responses) + + s.Equal(mpi.CommandResponse_COMMAND_STATUS_FAILURE, responses[0].GetCommandResponse().GetStatus()) + s.Equal("Config apply failed", responses[0].GetCommandResponse().GetMessage()) + s.Equal( + "file not in allowed directories /unknown/nginx.conf", + responses[0].GetCommandResponse().GetError(), + ) +} + +func TestConfigApplyTestSuite(t *testing.T) { + suite.Run(t, new(ConfigApplyTestSuite)) } -func TestGrpc_ConfigApply_Chunking(t *testing.T) { - ctx := context.Background() - teardownTest := utils.SetupConnectionTest(t, false, false, false, +func (s *ConfigApplyChunkingTestSuite) SetupSuite() { + s.ctx = context.Background() + s.teardownTest = utils.SetupConnectionTest(s.T(), false, false, false, "../../config/agent/nginx-config-with-max-file-size.conf") - defer teardownTest(t) + s.nginxInstanceID = utils.VerifyConnection(s.T(), 2, utils.MockManagementPlaneAPIAddress) +} - nginxInstanceID := utils.VerifyConnection(t, 2, utils.MockManagementPlaneAPIAddress) +func (s *ConfigApplyChunkingTestSuite) TearDownSuite() { + s.teardownTest(s.T()) +} - responses := utils.ManagementPlaneResponses(t, 1, utils.MockManagementPlaneAPIAddress) - assert.Equal(t, mpi.CommandResponse_COMMAND_STATUS_OK, responses[0].GetCommandResponse().GetStatus()) - assert.Equal(t, "Successfully updated all files", responses[0].GetCommandResponse().GetMessage()) +func (s *ConfigApplyChunkingTestSuite) TearDownTest() { + utils.ClearManagementPlaneResponses(s.T(), utils.MockManagementPlaneAPIAddress) +} - utils.ClearManagementPlaneResponses(t, utils.MockManagementPlaneAPIAddress) +func (s *ConfigApplyChunkingTestSuite) TestConfigApplyChunking_Test1_TestSetup() { + responses := utils.ManagementPlaneResponses(s.T(), 1, utils.MockManagementPlaneAPIAddress) + s.Require().Equal(mpi.CommandResponse_COMMAND_STATUS_OK, responses[0].GetCommandResponse().GetStatus()) + s.Require().Equal("Successfully updated all files", responses[0].GetCommandResponse().GetMessage()) +} +func (s *ConfigApplyChunkingTestSuite) TestConfigApplyChunking_Test2_TestChunkedConfigApply() { newConfigFile := "../../config/nginx/nginx-1mb-file.conf" err := utils.MockManagementPlaneGrpcContainer.CopyFileToContainer( - ctx, + s.ctx, newConfigFile, - fmt.Sprintf("/mock-management-plane-grpc/config/%s/etc/nginx/nginx.conf", nginxInstanceID), + fmt.Sprintf("/mock-management-plane-grpc/config/%s/etc/nginx/nginx.conf", s.nginxInstanceID), 0o666, ) - require.NoError(t, err) + s.Require().NoError(err) - utils.PerformConfigApply(t, nginxInstanceID, utils.MockManagementPlaneAPIAddress) + utils.PerformConfigApply(s.T(), s.nginxInstanceID, utils.MockManagementPlaneAPIAddress) - responses = utils.ManagementPlaneResponses(t, 2, utils.MockManagementPlaneAPIAddress) - t.Logf("Config apply responses: %v", responses) + responses := utils.ManagementPlaneResponses(s.T(), 2, utils.MockManagementPlaneAPIAddress) + s.T().Logf("Config apply responses: %v", responses) sort.Slice(responses, func(i, j int) bool { return responses[i].GetCommandResponse().GetMessage() < responses[j].GetCommandResponse().GetMessage() }) - assert.Equal(t, mpi.CommandResponse_COMMAND_STATUS_OK, responses[0].GetCommandResponse().GetStatus()) - assert.Equal(t, "Config apply successful", responses[0].GetCommandResponse().GetMessage()) - assert.Equal(t, mpi.CommandResponse_COMMAND_STATUS_OK, responses[1].GetCommandResponse().GetStatus()) - assert.Equal(t, "Successfully updated all files", responses[1].GetCommandResponse().GetMessage()) + s.Equal(mpi.CommandResponse_COMMAND_STATUS_OK, responses[0].GetCommandResponse().GetStatus()) + s.Equal("Config apply successful", responses[0].GetCommandResponse().GetMessage()) + s.Equal(mpi.CommandResponse_COMMAND_STATUS_OK, responses[1].GetCommandResponse().GetStatus()) + s.Equal("Successfully updated all files", responses[1].GetCommandResponse().GetMessage()) +} + +func TestConfigApplyChunkingTestSuite(t *testing.T) { + suite.Run(t, new(ConfigApplyChunkingTestSuite)) } diff --git a/test/integration/managementplane/config_upload_test.go b/test/integration/managementplane/config_upload_test.go index 35d57a628..7bf574391 100644 --- a/test/integration/managementplane/config_upload_test.go +++ b/test/integration/managementplane/config_upload_test.go @@ -6,6 +6,7 @@ package managementplane import ( + "context" "fmt" "net/http" "testing" @@ -14,23 +15,40 @@ import ( "github.com/go-resty/resty/v2" mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" ) -func TestGrpc_ConfigUpload(t *testing.T) { - teardownTest := utils.SetupConnectionTest(t, true, false, false, - "../../config/agent/nginx-config-with-grpc-client.conf") - defer teardownTest(t) +type ConfigUploadMPIFileWatcherTestSuite struct { + suite.Suite + ctx context.Context + teardownTest func(testing.TB) + nginxInstanceID string +} - nginxInstanceID := utils.VerifyConnection(t, 2, utils.MockManagementPlaneAPIAddress) - assert.False(t, t.Failed()) +func (s *ConfigUploadMPIFileWatcherTestSuite) TearDownSuite() { + s.teardownTest(s.T()) +} - responses := utils.ManagementPlaneResponses(t, 1, utils.MockManagementPlaneAPIAddress) +func (s *ConfigUploadMPIFileWatcherTestSuite) TearDownTest() { + utils.ClearManagementPlaneResponses(s.T(), utils.MockManagementPlaneAPIAddress) +} - assert.Equal(t, mpi.CommandResponse_COMMAND_STATUS_OK, responses[0].GetCommandResponse().GetStatus()) - assert.Equal(t, "Successfully updated all files", responses[0].GetCommandResponse().GetMessage()) +func (s *ConfigUploadMPIFileWatcherTestSuite) SetupSuite() { + s.ctx = context.Background() + s.teardownTest = utils.SetupConnectionTest(s.T(), true, false, false, + "../../config/agent/nginx-config-with-grpc-client.conf") + s.nginxInstanceID = utils.VerifyConnection(s.T(), 2, utils.MockManagementPlaneAPIAddress) +} +func (s *ConfigUploadMPIFileWatcherTestSuite) TestConfigUploadMPTFileWatcher_1_TestSetup() { + responses := utils.ManagementPlaneResponses(s.T(), 1, utils.MockManagementPlaneAPIAddress) + s.Equal(mpi.CommandResponse_COMMAND_STATUS_OK, responses[0].GetCommandResponse().GetStatus()) + s.Equal("Successfully updated all files", responses[0].GetCommandResponse().GetMessage()) + + s.False(s.T().Failed()) +} + +func (s *ConfigUploadMPIFileWatcherTestSuite) TestConfigUploadMPTFileWatcher_TestConfigUpload() { request := fmt.Sprintf(`{ "message_meta": { "message_id": "5d0fa83e-351c-4009-90cd-1f2acce2d184", @@ -44,9 +62,9 @@ func TestGrpc_ConfigUpload(t *testing.T) { } } } -}`, nginxInstanceID) +}`, s.nginxInstanceID) - t.Logf("Sending config upload request: %s", request) + s.T().Logf("Sending config upload request: %s", request) client := resty.New() client.SetRetryCount(utils.RetryCount).SetRetryWaitTime(utils.RetryWaitTime).SetRetryMaxWaitTime( @@ -55,13 +73,15 @@ func TestGrpc_ConfigUpload(t *testing.T) { url := fmt.Sprintf("http://%s/api/v1/requests", utils.MockManagementPlaneAPIAddress) resp, err := client.R().EnableTrace().SetBody(request).Post(url) - require.NoError(t, err) - assert.Equal(t, http.StatusOK, resp.StatusCode()) + s.Require().NoError(err) + s.Equal(http.StatusOK, resp.StatusCode()) - responses = utils.ManagementPlaneResponses(t, 2, utils.MockManagementPlaneAPIAddress) + responses := utils.ManagementPlaneResponses(s.T(), 1, utils.MockManagementPlaneAPIAddress) + + s.Equal(mpi.CommandResponse_COMMAND_STATUS_OK, responses[0].GetCommandResponse().GetStatus()) + s.Equal("Successfully updated all files", responses[0].GetCommandResponse().GetMessage()) +} - assert.Equal(t, mpi.CommandResponse_COMMAND_STATUS_OK, responses[0].GetCommandResponse().GetStatus()) - assert.Equal(t, "Successfully updated all files", responses[0].GetCommandResponse().GetMessage()) - assert.Equal(t, mpi.CommandResponse_COMMAND_STATUS_OK, responses[1].GetCommandResponse().GetStatus()) - assert.Equal(t, "Successfully updated all files", responses[1].GetCommandResponse().GetMessage()) +func TestConfigUploadMPIFileWatcherTestSuite(t *testing.T) { + suite.Run(t, new(ConfigUploadMPIFileWatcherTestSuite)) } diff --git a/test/integration/managementplane/file_watcher_test.go b/test/integration/managementplane/file_watcher_test.go index 5d6bb58f8..8f6fdb998 100644 --- a/test/integration/managementplane/file_watcher_test.go +++ b/test/integration/managementplane/file_watcher_test.go @@ -6,70 +6,54 @@ package managementplane import ( - "context" - "testing" - "github.com/nginx/agent/v3/test/integration/utils" mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) -func TestGrpc_FileWatcher(t *testing.T) { - ctx := context.Background() - teardownTest := utils.SetupConnectionTest(t, true, false, false, - "../../config/agent/nginx-config-with-grpc-client.conf") - defer teardownTest(t) - - utils.VerifyConnection(t, 2, utils.MockManagementPlaneAPIAddress) - assert.False(t, t.Failed()) +func (s *ConfigUploadMPIFileWatcherTestSuite) TestFileWatcher_Test1_TestUpdateNGINXConfig() { + err := utils.Container.CopyFileToContainer( + s.ctx, + "../../config/nginx/nginx-with-server-block-access-log.conf", + "/etc/nginx/nginx.conf", + 0o666, + ) + s.Require().NoError(err) - t.Run("Test 1: update nginx config file on data plane", func(t *testing.T) { - err := utils.Container.CopyFileToContainer( - ctx, - "../../config/nginx/nginx-with-server-block-access-log.conf", - "/etc/nginx/nginx.conf", - 0o666, - ) - require.NoError(t, err) + responses := utils.ManagementPlaneResponses(s.T(), 1, utils.MockManagementPlaneAPIAddress) - responses := utils.ManagementPlaneResponses(t, 2, utils.MockManagementPlaneAPIAddress) - assert.Equal(t, mpi.CommandResponse_COMMAND_STATUS_OK, responses[0].GetCommandResponse().GetStatus()) - assert.Equal(t, "Successfully updated all files", responses[0].GetCommandResponse().GetMessage()) - assert.Equal(t, mpi.CommandResponse_COMMAND_STATUS_OK, responses[1].GetCommandResponse().GetStatus()) - assert.Equal(t, "Successfully updated all files", responses[1].GetCommandResponse().GetMessage()) + s.Equal(mpi.CommandResponse_COMMAND_STATUS_OK, responses[0].GetCommandResponse().GetStatus()) + s.Equal("Successfully updated all files", responses[0].GetCommandResponse().GetMessage()) - utils.VerifyUpdateDataPlaneStatus(t, utils.MockManagementPlaneAPIAddress) - }) + utils.VerifyUpdateDataPlaneStatus(s.T(), utils.MockManagementPlaneAPIAddress) +} - t.Run("Test 2: create new nginx config file", func(t *testing.T) { - err := utils.Container.CopyFileToContainer( - ctx, - "../../config/nginx/empty-nginx.conf", - "/etc/nginx/test/test.conf", - 0o666, - ) - require.NoError(t, err) +func (s *ConfigUploadMPIFileWatcherTestSuite) TestFileWatcher_Test2_TestCreateNGINXConfig() { + err := utils.Container.CopyFileToContainer( + s.ctx, + "../../config/nginx/empty-nginx.conf", + "/etc/nginx/test/test.conf", + 0o666, + ) + s.Require().NoError(err) - responses := utils.ManagementPlaneResponses(t, 3, utils.MockManagementPlaneAPIAddress) - assert.Equal(t, mpi.CommandResponse_COMMAND_STATUS_OK, responses[2].GetCommandResponse().GetStatus()) - assert.Equal(t, "Successfully updated all files", responses[2].GetCommandResponse().GetMessage()) + responses := utils.ManagementPlaneResponses(s.T(), 1, utils.MockManagementPlaneAPIAddress) + s.Equal(mpi.CommandResponse_COMMAND_STATUS_OK, responses[0].GetCommandResponse().GetStatus()) + s.Equal("Successfully updated all files", responses[0].GetCommandResponse().GetMessage()) - utils.VerifyUpdateDataPlaneStatus(t, utils.MockManagementPlaneAPIAddress) - }) + utils.VerifyUpdateDataPlaneStatus(s.T(), utils.MockManagementPlaneAPIAddress) +} - t.Run("Test 3: delete nginx config file", func(t *testing.T) { - _, _, err := utils.Container.Exec( - ctx, - []string{"rm", "-rf", "/etc/nginx/test"}, - ) - require.NoError(t, err) +func (s *ConfigUploadMPIFileWatcherTestSuite) TestFileWatcher_Test3_TestDeleteNGINXConfig() { + _, _, err := utils.Container.Exec( + s.ctx, + []string{"rm", "-rf", "/etc/nginx/test"}, + ) + s.Require().NoError(err) - responses := utils.ManagementPlaneResponses(t, 4, utils.MockManagementPlaneAPIAddress) - assert.Equal(t, mpi.CommandResponse_COMMAND_STATUS_OK, responses[3].GetCommandResponse().GetStatus()) - assert.Equal(t, "Successfully updated all files", responses[3].GetCommandResponse().GetMessage()) + responses := utils.ManagementPlaneResponses(s.T(), 1, utils.MockManagementPlaneAPIAddress) + s.Equal(mpi.CommandResponse_COMMAND_STATUS_OK, responses[0].GetCommandResponse().GetStatus()) + s.Equal("Successfully updated all files", responses[0].GetCommandResponse().GetMessage()) - utils.VerifyUpdateDataPlaneStatus(t, utils.MockManagementPlaneAPIAddress) - }) + utils.VerifyUpdateDataPlaneStatus(s.T(), utils.MockManagementPlaneAPIAddress) } diff --git a/test/integration/managementplane/grpc_management_plane_api_test.go b/test/integration/managementplane/grpc_management_plane_api_test.go index 7bee15f28..43feab824 100644 --- a/test/integration/managementplane/grpc_management_plane_api_test.go +++ b/test/integration/managementplane/grpc_management_plane_api_test.go @@ -6,72 +6,43 @@ package managementplane import ( - "context" "fmt" "net" "net/http" - "testing" "time" "github.com/nginx/agent/v3/test/integration/utils" "github.com/go-resty/resty/v2" mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) -func TestGrpc_Reconnection(t *testing.T) { - ctx := context.Background() - teardownTest := utils.SetupConnectionTest(t, false, false, false, - "../../config/agent/nginx-config-with-grpc-client.conf") - defer teardownTest(t) - +func (s *ConfigApplyTestSuite) TestGrpc_Test1_Reconnection() { timeout := 15 * time.Second - originalID := utils.VerifyConnection(t, 2, utils.MockManagementPlaneAPIAddress) - - stopErr := utils.MockManagementPlaneGrpcContainer.Stop(ctx, &timeout) + stopErr := utils.MockManagementPlaneGrpcContainer.Stop(s.ctx, &timeout) - require.NoError(t, stopErr) + s.Require().NoError(stopErr) - startErr := utils.MockManagementPlaneGrpcContainer.Start(ctx) - require.NoError(t, startErr) + startErr := utils.MockManagementPlaneGrpcContainer.Start(s.ctx) + s.Require().NoError(startErr) - ipAddress, err := utils.MockManagementPlaneGrpcContainer.Host(ctx) - require.NoError(t, err) - ports, err := utils.MockManagementPlaneGrpcContainer.Ports(ctx) - require.NoError(t, err) + ipAddress, err := utils.MockManagementPlaneGrpcContainer.Host(s.ctx) + s.Require().NoError(err) + ports, err := utils.MockManagementPlaneGrpcContainer.Ports(s.ctx) + s.Require().NoError(err) utils.MockManagementPlaneAPIAddress = net.JoinHostPort(ipAddress, ports["9093/tcp"][0].HostPort) - currentID := utils.VerifyConnection(t, 2, utils.MockManagementPlaneAPIAddress) - assert.Equal(t, originalID, currentID) + currentID := utils.VerifyConnection(s.T(), 2, utils.MockManagementPlaneAPIAddress) + s.Equal(s.nginxInstanceID, currentID) } // Verify that the agent sends a connection request and an update data plane status request -func TestGrpc_StartUp(t *testing.T) { - teardownTest := utils.SetupConnectionTest(t, true, false, false, - "../../config/agent/nginx-config-with-grpc-client.conf") - defer teardownTest(t) - - utils.VerifyConnection(t, 2, utils.MockManagementPlaneAPIAddress) - assert.False(t, t.Failed()) - utils.VerifyUpdateDataPlaneHealth(t, utils.MockManagementPlaneAPIAddress) +func (s *ConfigUploadMPIFileWatcherTestSuite) TestGrpc_Test2_StartUp() { + utils.VerifyUpdateDataPlaneHealth(s.T(), utils.MockManagementPlaneAPIAddress) } -func TestGrpc_DataplaneHealthRequest(t *testing.T) { - teardownTest := utils.SetupConnectionTest(t, true, false, false, - "../../config/agent/nginx-config-with-grpc-client.conf") - defer teardownTest(t) - - utils.VerifyConnection(t, 2, utils.MockManagementPlaneAPIAddress) - - responses := utils.ManagementPlaneResponses(t, 1, utils.MockManagementPlaneAPIAddress) - assert.Equal(t, mpi.CommandResponse_COMMAND_STATUS_OK, responses[0].GetCommandResponse().GetStatus()) - assert.Equal(t, "Successfully updated all files", responses[0].GetCommandResponse().GetMessage()) - - assert.False(t, t.Failed()) - +func (s *ConfigUploadMPIFileWatcherTestSuite) TestGrpc_Test3_DataplaneHealthRequest() { request := `{ "message_meta": { "message_id": "5d0fa83e-351c-4009-90cd-1f2acce2d184", @@ -88,11 +59,11 @@ func TestGrpc_DataplaneHealthRequest(t *testing.T) { url := fmt.Sprintf("http://%s/api/v1/requests", utils.MockManagementPlaneAPIAddress) resp, err := client.R().EnableTrace().SetBody(request).Post(url) - require.NoError(t, err) - assert.Equal(t, http.StatusOK, resp.StatusCode()) + s.Require().NoError(err) + s.Equal(http.StatusOK, resp.StatusCode()) - responses = utils.ManagementPlaneResponses(t, 2, utils.MockManagementPlaneAPIAddress) + responses := utils.ManagementPlaneResponses(s.T(), 1, utils.MockManagementPlaneAPIAddress) - assert.Equal(t, mpi.CommandResponse_COMMAND_STATUS_OK, responses[1].GetCommandResponse().GetStatus()) - assert.Equal(t, "Successfully sent health status update", responses[1].GetCommandResponse().GetMessage()) + s.Equal(mpi.CommandResponse_COMMAND_STATUS_OK, responses[0].GetCommandResponse().GetStatus()) + s.Equal("Successfully sent health status update", responses[0].GetCommandResponse().GetMessage()) } From 3ec1ae151a5bae51a645364af209c540cbf78f13 Mon Sep 17 00:00:00 2001 From: Spencer Ugbo Date: Thu, 17 Jul 2025 11:44:15 +0100 Subject: [PATCH 02/13] refactor and rename test suites --- .../managementplane/config_apply_test.go | 43 ++++++++----------- .../managementplane/config_upload_test.go | 21 +++++---- .../managementplane/file_watcher_test.go | 6 +-- .../grpc_management_plane_api_test.go | 4 +- 4 files changed, 32 insertions(+), 42 deletions(-) diff --git a/test/integration/managementplane/config_apply_test.go b/test/integration/managementplane/config_apply_test.go index a2d2bdd32..39297e716 100644 --- a/test/integration/managementplane/config_apply_test.go +++ b/test/integration/managementplane/config_apply_test.go @@ -42,6 +42,9 @@ func (s *ConfigApplyTestSuite) SetupSuite() { s.teardownTest = utils.SetupConnectionTest(s.T(), false, false, false, "../../config/agent/nginx-config-with-grpc-client.conf") s.nginxInstanceID = utils.VerifyConnection(s.T(), 2, utils.MockManagementPlaneAPIAddress) + responses := utils.ManagementPlaneResponses(s.T(), 1, utils.MockManagementPlaneAPIAddress) + s.Require().Equal(mpi.CommandResponse_COMMAND_STATUS_OK, responses[0].GetCommandResponse().GetStatus()) + s.Require().Equal("Successfully updated all files", responses[0].GetCommandResponse().GetMessage()) } func (s *ConfigApplyTestSuite) TearDownSuite() { @@ -52,22 +55,18 @@ func (s *ConfigApplyTestSuite) TearDownTest() { utils.ClearManagementPlaneResponses(s.T(), utils.MockManagementPlaneAPIAddress) } -func (s *ConfigApplyTestSuite) TestConfigApply_Test1_TestSetup() { - responses := utils.ManagementPlaneResponses(s.T(), 1, utils.MockManagementPlaneAPIAddress) - s.Require().Equal(mpi.CommandResponse_COMMAND_STATUS_OK, responses[0].GetCommandResponse().GetStatus()) - s.Require().Equal("Successfully updated all files", responses[0].GetCommandResponse().GetMessage()) -} - -func (s *ConfigApplyTestSuite) TestConfigApply_Test2_TestNoConfigChanges() { +func (s *ConfigApplyTestSuite) TestConfigApply_Test1_TestNoConfigChanges() { utils.PerformConfigApply(s.T(), s.nginxInstanceID, utils.MockManagementPlaneAPIAddress) - responses := utils.ManagementPlaneResponses(s.T(), 1, utils.MockManagementPlaneAPIAddress) + responses := utils.ManagementPlaneResponses(s.T(), 2, utils.MockManagementPlaneAPIAddress) s.T().Logf("Config apply responses: %v", responses) s.Equal(mpi.CommandResponse_COMMAND_STATUS_OK, responses[0].GetCommandResponse().GetStatus()) - s.Equal("Config apply successful, no files to change", responses[0].GetCommandResponse().GetMessage()) + s.Equal("Successfully updated all files", responses[0].GetCommandResponse().GetMessage()) + s.Equal(mpi.CommandResponse_COMMAND_STATUS_OK, responses[1].GetCommandResponse().GetStatus()) + s.Equal("Config apply successful, no files to change", responses[1].GetCommandResponse().GetMessage()) } -func (s *ConfigApplyTestSuite) TestConfigApply_Test3_TestValidConfig() { +func (s *ConfigApplyTestSuite) TestConfigApply_Test2_TestValidConfig() { newConfigFile := "../../config/nginx/nginx-with-test-location.conf" if os.Getenv("IMAGE_PATH") == "/nginx-plus/agent" { @@ -95,7 +94,7 @@ func (s *ConfigApplyTestSuite) TestConfigApply_Test3_TestValidConfig() { s.Equal("Successfully updated all files", responses[1].GetCommandResponse().GetMessage()) } -func (s *ConfigApplyTestSuite) TestConfigApply_Test4_TestInvalidConfig() { +func (s *ConfigApplyTestSuite) TestConfigApply_Test3_TestInvalidConfig() { err := utils.MockManagementPlaneGrpcContainer.CopyFileToContainer( s.ctx, "../../config/nginx/invalid-nginx.conf", @@ -117,7 +116,7 @@ func (s *ConfigApplyTestSuite) TestConfigApply_Test4_TestInvalidConfig() { s.Equal(configApplyErrorMessage, responses[1].GetCommandResponse().GetError()) } -func (s *ConfigApplyTestSuite) TestConfigApply_Test5_TestFileNotInAllowedDirectory() { +func (s *ConfigApplyTestSuite) TestConfigApply_Test4_TestFileNotInAllowedDirectory() { utils.PerformInvalidConfigApply(s.T(), s.nginxInstanceID) responses := utils.ManagementPlaneResponses(s.T(), 1, utils.MockManagementPlaneAPIAddress) @@ -131,32 +130,23 @@ func (s *ConfigApplyTestSuite) TestConfigApply_Test5_TestFileNotInAllowedDirecto ) } -func TestConfigApplyTestSuite(t *testing.T) { - suite.Run(t, new(ConfigApplyTestSuite)) -} - func (s *ConfigApplyChunkingTestSuite) SetupSuite() { s.ctx = context.Background() s.teardownTest = utils.SetupConnectionTest(s.T(), false, false, false, "../../config/agent/nginx-config-with-max-file-size.conf") s.nginxInstanceID = utils.VerifyConnection(s.T(), 2, utils.MockManagementPlaneAPIAddress) + responses := utils.ManagementPlaneResponses(s.T(), 1, utils.MockManagementPlaneAPIAddress) + s.Require().Equal(mpi.CommandResponse_COMMAND_STATUS_OK, responses[0].GetCommandResponse().GetStatus()) + s.Require().Equal("Successfully updated all files", responses[0].GetCommandResponse().GetMessage()) } func (s *ConfigApplyChunkingTestSuite) TearDownSuite() { s.teardownTest(s.T()) } -func (s *ConfigApplyChunkingTestSuite) TearDownTest() { +func (s *ConfigApplyChunkingTestSuite) TestConfigApplyChunking() { utils.ClearManagementPlaneResponses(s.T(), utils.MockManagementPlaneAPIAddress) -} -func (s *ConfigApplyChunkingTestSuite) TestConfigApplyChunking_Test1_TestSetup() { - responses := utils.ManagementPlaneResponses(s.T(), 1, utils.MockManagementPlaneAPIAddress) - s.Require().Equal(mpi.CommandResponse_COMMAND_STATUS_OK, responses[0].GetCommandResponse().GetStatus()) - s.Require().Equal("Successfully updated all files", responses[0].GetCommandResponse().GetMessage()) -} - -func (s *ConfigApplyChunkingTestSuite) TestConfigApplyChunking_Test2_TestChunkedConfigApply() { newConfigFile := "../../config/nginx/nginx-1mb-file.conf" err := utils.MockManagementPlaneGrpcContainer.CopyFileToContainer( @@ -182,6 +172,7 @@ func (s *ConfigApplyChunkingTestSuite) TestConfigApplyChunking_Test2_TestChunked s.Equal("Successfully updated all files", responses[1].GetCommandResponse().GetMessage()) } -func TestConfigApplyChunkingTestSuite(t *testing.T) { +func TestConfigApplyTestSuite(t *testing.T) { + suite.Run(t, new(ConfigApplyTestSuite)) suite.Run(t, new(ConfigApplyChunkingTestSuite)) } diff --git a/test/integration/managementplane/config_upload_test.go b/test/integration/managementplane/config_upload_test.go index 7bf574391..0f888d3c4 100644 --- a/test/integration/managementplane/config_upload_test.go +++ b/test/integration/managementplane/config_upload_test.go @@ -18,29 +18,26 @@ import ( "github.com/stretchr/testify/suite" ) -type ConfigUploadMPIFileWatcherTestSuite struct { +type MPITestSuite struct { suite.Suite ctx context.Context teardownTest func(testing.TB) nginxInstanceID string } -func (s *ConfigUploadMPIFileWatcherTestSuite) TearDownSuite() { +func (s *MPITestSuite) TearDownSuite() { s.teardownTest(s.T()) } -func (s *ConfigUploadMPIFileWatcherTestSuite) TearDownTest() { +func (s *MPITestSuite) TearDownTest() { utils.ClearManagementPlaneResponses(s.T(), utils.MockManagementPlaneAPIAddress) } -func (s *ConfigUploadMPIFileWatcherTestSuite) SetupSuite() { +func (s *MPITestSuite) SetupSuite() { s.ctx = context.Background() s.teardownTest = utils.SetupConnectionTest(s.T(), true, false, false, "../../config/agent/nginx-config-with-grpc-client.conf") s.nginxInstanceID = utils.VerifyConnection(s.T(), 2, utils.MockManagementPlaneAPIAddress) -} - -func (s *ConfigUploadMPIFileWatcherTestSuite) TestConfigUploadMPTFileWatcher_1_TestSetup() { responses := utils.ManagementPlaneResponses(s.T(), 1, utils.MockManagementPlaneAPIAddress) s.Equal(mpi.CommandResponse_COMMAND_STATUS_OK, responses[0].GetCommandResponse().GetStatus()) s.Equal("Successfully updated all files", responses[0].GetCommandResponse().GetMessage()) @@ -48,7 +45,7 @@ func (s *ConfigUploadMPIFileWatcherTestSuite) TestConfigUploadMPTFileWatcher_1_T s.False(s.T().Failed()) } -func (s *ConfigUploadMPIFileWatcherTestSuite) TestConfigUploadMPTFileWatcher_TestConfigUpload() { +func (s *MPITestSuite) TestConfigUpload() { request := fmt.Sprintf(`{ "message_meta": { "message_id": "5d0fa83e-351c-4009-90cd-1f2acce2d184", @@ -76,12 +73,14 @@ func (s *ConfigUploadMPIFileWatcherTestSuite) TestConfigUploadMPTFileWatcher_Tes s.Require().NoError(err) s.Equal(http.StatusOK, resp.StatusCode()) - responses := utils.ManagementPlaneResponses(s.T(), 1, utils.MockManagementPlaneAPIAddress) + responses := utils.ManagementPlaneResponses(s.T(), 2, utils.MockManagementPlaneAPIAddress) s.Equal(mpi.CommandResponse_COMMAND_STATUS_OK, responses[0].GetCommandResponse().GetStatus()) s.Equal("Successfully updated all files", responses[0].GetCommandResponse().GetMessage()) + s.Equal(mpi.CommandResponse_COMMAND_STATUS_OK, responses[1].GetCommandResponse().GetStatus()) + s.Equal("Successfully updated all files", responses[1].GetCommandResponse().GetMessage()) } -func TestConfigUploadMPIFileWatcherTestSuite(t *testing.T) { - suite.Run(t, new(ConfigUploadMPIFileWatcherTestSuite)) +func TestMPITestSuite(t *testing.T) { + suite.Run(t, new(MPITestSuite)) } diff --git a/test/integration/managementplane/file_watcher_test.go b/test/integration/managementplane/file_watcher_test.go index 8f6fdb998..3e6824233 100644 --- a/test/integration/managementplane/file_watcher_test.go +++ b/test/integration/managementplane/file_watcher_test.go @@ -11,7 +11,7 @@ import ( mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1" ) -func (s *ConfigUploadMPIFileWatcherTestSuite) TestFileWatcher_Test1_TestUpdateNGINXConfig() { +func (s *MPITestSuite) TestFileWatcher_Test1_TestUpdateNGINXConfig() { err := utils.Container.CopyFileToContainer( s.ctx, "../../config/nginx/nginx-with-server-block-access-log.conf", @@ -28,7 +28,7 @@ func (s *ConfigUploadMPIFileWatcherTestSuite) TestFileWatcher_Test1_TestUpdateNG utils.VerifyUpdateDataPlaneStatus(s.T(), utils.MockManagementPlaneAPIAddress) } -func (s *ConfigUploadMPIFileWatcherTestSuite) TestFileWatcher_Test2_TestCreateNGINXConfig() { +func (s *MPITestSuite) TestFileWatcher_Test2_TestCreateNGINXConfig() { err := utils.Container.CopyFileToContainer( s.ctx, "../../config/nginx/empty-nginx.conf", @@ -44,7 +44,7 @@ func (s *ConfigUploadMPIFileWatcherTestSuite) TestFileWatcher_Test2_TestCreateNG utils.VerifyUpdateDataPlaneStatus(s.T(), utils.MockManagementPlaneAPIAddress) } -func (s *ConfigUploadMPIFileWatcherTestSuite) TestFileWatcher_Test3_TestDeleteNGINXConfig() { +func (s *MPITestSuite) TestFileWatcher_Test3_TestDeleteNGINXConfig() { _, _, err := utils.Container.Exec( s.ctx, []string{"rm", "-rf", "/etc/nginx/test"}, diff --git a/test/integration/managementplane/grpc_management_plane_api_test.go b/test/integration/managementplane/grpc_management_plane_api_test.go index 43feab824..7cfe9050d 100644 --- a/test/integration/managementplane/grpc_management_plane_api_test.go +++ b/test/integration/managementplane/grpc_management_plane_api_test.go @@ -38,11 +38,11 @@ func (s *ConfigApplyTestSuite) TestGrpc_Test1_Reconnection() { } // Verify that the agent sends a connection request and an update data plane status request -func (s *ConfigUploadMPIFileWatcherTestSuite) TestGrpc_Test2_StartUp() { +func (s *MPITestSuite) TestGrpc_Test2_StartUp() { utils.VerifyUpdateDataPlaneHealth(s.T(), utils.MockManagementPlaneAPIAddress) } -func (s *ConfigUploadMPIFileWatcherTestSuite) TestGrpc_Test3_DataplaneHealthRequest() { +func (s *MPITestSuite) TestGrpc_Test3_DataplaneHealthRequest() { request := `{ "message_meta": { "message_id": "5d0fa83e-351c-4009-90cd-1f2acce2d184", From 2e6e8c2028e0f7409addde254567103e1425f1b8 Mon Sep 17 00:00:00 2001 From: Donal Hurley Date: Wed, 16 Jul 2025 11:49:21 +0100 Subject: [PATCH 03/13] Add labels as headers in gRPC connections (#1155) --- internal/command/command_plugin.go | 21 ++++++++++--------- internal/config/config.go | 17 ++++++++++++++++ internal/config/config_test.go | 15 ++++++++++++++ internal/config/types.go | 15 ++++++++++++++ internal/file/file_plugin.go | 26 +++++++++++++----------- test/mock/collector/nginx-agent.conf | 4 ++++ test/mock/grpc/mock_management_server.go | 20 ++++++++++++++---- 7 files changed, 92 insertions(+), 26 deletions(-) diff --git a/internal/command/command_plugin.go b/internal/command/command_plugin.go index 263ac26bf..7ace559ce 100644 --- a/internal/command/command_plugin.go +++ b/internal/command/command_plugin.go @@ -106,28 +106,29 @@ func (cp *CommandPlugin) Info() *bus.Info { func (cp *CommandPlugin) Process(ctx context.Context, msg *bus.Message) { slog.DebugContext(ctx, "Processing command") + ctxWithMetadata := cp.config.NewContextWithLabels(ctx) - if logger.ServerType(ctx) == "" { - ctx = context.WithValue( - ctx, + if logger.ServerType(ctxWithMetadata) == "" { + ctxWithMetadata = context.WithValue( + ctxWithMetadata, logger.ServerTypeContextKey, slog.Any(logger.ServerTypeKey, cp.commandServerType.String()), ) } - if logger.ServerType(ctx) == cp.commandServerType.String() { + if logger.ServerType(ctxWithMetadata) == cp.commandServerType.String() { switch msg.Topic { case bus.ConnectionResetTopic: - cp.processConnectionReset(ctx, msg) + cp.processConnectionReset(ctxWithMetadata, msg) case bus.ResourceUpdateTopic: - cp.processResourceUpdate(ctx, msg) + cp.processResourceUpdate(ctxWithMetadata, msg) case bus.InstanceHealthTopic: - cp.processInstanceHealth(ctx, msg) + cp.processInstanceHealth(ctxWithMetadata, msg) case bus.DataPlaneHealthResponseTopic: - cp.processDataPlaneHealth(ctx, msg) + cp.processDataPlaneHealth(ctxWithMetadata, msg) case bus.DataPlaneResponseTopic: - cp.processDataPlaneResponse(ctx, msg) + cp.processDataPlaneResponse(ctxWithMetadata, msg) default: - slog.DebugContext(ctx, "Command plugin received unknown topic", "topic", msg.Topic) + slog.DebugContext(ctxWithMetadata, "Command plugin received unknown topic", "topic", msg.Topic) } } } diff --git a/internal/config/config.go b/internal/config/config.go index 0447f834e..4ebd75900 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -129,6 +129,7 @@ func ResolveConfig() (*Config, error) { } checkCollectorConfiguration(collector, config) + addLabelsAsOTelHeaders(collector, config.Labels) slog.Debug("Agent config", "config", config) slog.Info("Excluded files from being watched for file changes", "exclude_files", @@ -209,6 +210,22 @@ func defaultCollector(collector *Collector, config *Config) { } } +func addLabelsAsOTelHeaders(collector *Collector, labels map[string]any) { + slog.Debug("Adding labels as headers to collector", "labels", labels) + if collector.Extensions.HeadersSetter != nil { + for key, value := range labels { + valueString, ok := value.(string) + if ok { + collector.Extensions.HeadersSetter.Headers = append(collector.Extensions.HeadersSetter.Headers, Header{ + Action: "insert", + Key: key, + Value: valueString, + }) + } + } + } +} + func setVersion(version, commit string) { RootCommand.Version = version + "-" + commit viperInstance.SetDefault(VersionKey, version) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index f34fa9853..6f3ad0eed 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -9,6 +9,7 @@ import ( "errors" "os" "path" + "sort" "strings" "testing" "time" @@ -63,6 +64,10 @@ func TestResolveConfig(t *testing.T) { actual, err := ResolveConfig() require.NoError(t, err) + sort.Slice(actual.Collector.Extensions.HeadersSetter.Headers, func(i, j int) bool { + headers := actual.Collector.Extensions.HeadersSetter.Headers + return headers[i].Key < headers[j].Key + }) assert.Equal(t, createConfig(), actual) } @@ -1059,6 +1064,16 @@ func createConfig() *Config { Key: "key", Value: "value", }, + { + Action: "insert", + Key: "label1", + Value: "label 1", + }, + { + Action: "insert", + Key: "label2", + Value: "new-value", + }, }, }, }, diff --git a/internal/config/types.go b/internal/config/types.go index b0db71462..ae0558045 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -6,12 +6,15 @@ package config import ( + "context" "errors" "fmt" "path/filepath" "strings" "time" + "google.golang.org/grpc/metadata" + "github.com/google/uuid" ) @@ -432,6 +435,18 @@ func (c *Config) AreReceiversConfigured() bool { len(c.Collector.Receivers.TcplogReceivers) > 0 } +func (c *Config) NewContextWithLabels(ctx context.Context) context.Context { + md := metadata.Pairs() + for key, value := range c.Labels { + valueString, ok := value.(string) + if ok { + md.Set(key, valueString) + } + } + + return metadata.NewOutgoingContext(ctx, md) +} + func isAllowedDir(dir string, allowedDirs []string) bool { if !strings.HasSuffix(dir, "/") && filepath.Ext(dir) == "" { dir += "/" diff --git a/internal/file/file_plugin.go b/internal/file/file_plugin.go index 6818418f3..d94796d2f 100644 --- a/internal/file/file_plugin.go +++ b/internal/file/file_plugin.go @@ -83,34 +83,36 @@ func (fp *FilePlugin) Info() *bus.Info { // nolint: cyclop, revive func (fp *FilePlugin) Process(ctx context.Context, msg *bus.Message) { + ctxWithMetadata := fp.config.NewContextWithLabels(ctx) + if logger.ServerType(ctx) == "" { - ctx = context.WithValue( - ctx, + ctxWithMetadata = context.WithValue( + ctxWithMetadata, logger.ServerTypeContextKey, slog.Any(logger.ServerTypeKey, fp.serverType.String()), ) } - if logger.ServerType(ctx) == fp.serverType.String() { + if logger.ServerType(ctxWithMetadata) == fp.serverType.String() { switch msg.Topic { case bus.ConnectionResetTopic: - fp.handleConnectionReset(ctx, msg) + fp.handleConnectionReset(ctxWithMetadata, msg) case bus.ConnectionCreatedTopic: - slog.DebugContext(ctx, "File plugin received connection created message") + slog.DebugContext(ctxWithMetadata, "File plugin received connection created message") fp.fileManagerService.SetIsConnected(true) case bus.NginxConfigUpdateTopic: - fp.handleNginxConfigUpdate(ctx, msg) + fp.handleNginxConfigUpdate(ctxWithMetadata, msg) case bus.ConfigUploadRequestTopic: - fp.handleConfigUploadRequest(ctx, msg) + fp.handleConfigUploadRequest(ctxWithMetadata, msg) case bus.ConfigApplyRequestTopic: - fp.handleConfigApplyRequest(ctx, msg) + fp.handleConfigApplyRequest(ctxWithMetadata, msg) case bus.ConfigApplyCompleteTopic: - fp.handleConfigApplyComplete(ctx, msg) + fp.handleConfigApplyComplete(ctxWithMetadata, msg) case bus.ConfigApplySuccessfulTopic: - fp.handleConfigApplySuccess(ctx, msg) + fp.handleConfigApplySuccess(ctxWithMetadata, msg) case bus.ConfigApplyFailedTopic: - fp.handleConfigApplyFailedRequest(ctx, msg) + fp.handleConfigApplyFailedRequest(ctxWithMetadata, msg) default: - slog.DebugContext(ctx, "File plugin received unknown topic", "topic", msg.Topic) + slog.DebugContext(ctxWithMetadata, "File plugin received unknown topic", "topic", msg.Topic) } } } diff --git a/test/mock/collector/nginx-agent.conf b/test/mock/collector/nginx-agent.conf index 3194f4c77..dd870c390 100644 --- a/test/mock/collector/nginx-agent.conf +++ b/test/mock/collector/nginx-agent.conf @@ -26,6 +26,10 @@ allowed_directories: - /usr/local/etc/nginx - /usr/share/nginx/modules - /var/run/nginx + +labels: + product-type: mock-product + product-version: v1.0.0 client: http: diff --git a/test/mock/grpc/mock_management_server.go b/test/mock/grpc/mock_management_server.go index ad2b371a1..aff397395 100644 --- a/test/mock/grpc/mock_management_server.go +++ b/test/mock/grpc/mock_management_server.go @@ -52,6 +52,9 @@ var ( Time: keepAliveTime, Timeout: keepAliveTimeout, } + + errMissingMetadata = status.Errorf(codes.InvalidArgument, "missing metadata") + errInvalidToken = status.Errorf(codes.Unauthenticated, "invalid token") ) type MockManagementServer struct { @@ -146,6 +149,7 @@ func serverOptions(agentConfig *config.Config) []grpc.ServerOption { opts = append(opts, grpc.ChainUnaryInterceptor( grpcvalidator.UnaryServerInterceptor(), protovalidateInterceptor.UnaryServerInterceptor(validator), + logHeaders, ), ) } else { @@ -153,6 +157,7 @@ func serverOptions(agentConfig *config.Config) []grpc.ServerOption { grpcvalidator.UnaryServerInterceptor(), protovalidateInterceptor.UnaryServerInterceptor(validator), ensureValidToken, + logHeaders, ), ) } @@ -242,10 +247,6 @@ func reportHealth(healthcheck *health.Server, agentConfig *config.Config) { } func ensureValidToken(ctx context.Context, req any, _ *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (any, error) { - var ( - errMissingMetadata = status.Errorf(codes.InvalidArgument, "missing metadata") - errInvalidToken = status.Errorf(codes.Unauthenticated, "invalid token") - ) md, ok := metadata.FromIncomingContext(ctx) if !ok { return nil, errMissingMetadata @@ -270,3 +271,14 @@ func valid(authorization []string) bool { // for a token matching an arbitrary string. return token == "1234" } + +func logHeaders(ctx context.Context, req any, _ *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (any, error) { + md, ok := metadata.FromIncomingContext(ctx) + if !ok { + return nil, errMissingMetadata + } + + slog.InfoContext(ctx, "Request headers", "headers", md) + + return handler(ctx, req) +} From beccdb27433718a2ee77f5ed1181a72865a74629 Mon Sep 17 00:00:00 2001 From: aphralG <108004222+aphralG@users.noreply.github.com> Date: Thu, 17 Jul 2025 11:05:05 +0100 Subject: [PATCH 04/13] Add support for new redhat and ubuntu os (#1161) --- Makefile | 12 ++++++------ scripts/packages/package-check.sh | 8 ++++---- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Makefile b/Makefile index aaf6b6168..9df9dcca7 100644 --- a/Makefile +++ b/Makefile @@ -17,13 +17,13 @@ GOBIN ?= $$(go env GOPATH)/bin # | OS_RELEASE | OS_VERSION | NOTES | # | ---------------- | ----------------------------------------- | -------------------------------------------------------------- | # | amazonlinux | 2, 2023 | | -# | ubuntu | 20.04, 22.04 24.04 | | +# | ubuntu | 22.04, 24.04 25.04 | | # | debian | bullseye-slim, bookworm-slim | | -# | redhatenterprise | 8, 9 | | -# | rockylinux | 8, 9 | | -# | almalinux | 8, 9 | | -# | alpine | 3.18, 3.19, 3.20, 3.21 3.22 | | -# | oraclelinux | 8, 9 | | +# | redhatenterprise | 8, 9, 10 | | +# | rockylinux | 8, 9, 10 | | +# | almalinux | 8, 9, 10 | | +# | alpine | 3.19, 3.20, 3.21 3.22 | | +# | oraclelinux | 8, 9, 10 | | # | suse | sle15 | | # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # OS_RELEASE ?= ubuntu diff --git a/scripts/packages/package-check.sh b/scripts/packages/package-check.sh index 917d8ce91..c3610b56a 100755 --- a/scripts/packages/package-check.sh +++ b/scripts/packages/package-check.sh @@ -55,18 +55,16 @@ APK=( alpine/v3.21/main/x86_64/nginx-agent-$VERSION.apk alpine/v3.20/main/aarch64/nginx-agent-$VERSION.apk alpine/v3.20/main/x86_64/nginx-agent-$VERSION.apk - alpine/v3.18/main/aarch64/nginx-agent-$VERSION.apk - alpine/v3.18/main/x86_64/nginx-agent-$VERSION.apk alpine/v3.19/main/aarch64/nginx-agent-$VERSION.apk alpine/v3.19/main/x86_64/nginx-agent-$VERSION.apk ) UBUNTU=( - ubuntu/pool/agent/n/nginx-agent/nginx-agent_$VERSION~focal_arm64.deb ubuntu/pool/agent/n/nginx-agent/nginx-agent_$VERSION~jammy_amd64.deb ubuntu/pool/agent/n/nginx-agent/nginx-agent_$VERSION~noble_arm64.deb + ubuntu/pool/agent/n/nginx-agent/nginx-agent_$VERSION~plucky_arm64.deb ubuntu/pool/agent/n/nginx-agent/nginx-agent_$VERSION~jammy_arm64.deb ubuntu/pool/agent/n/nginx-agent/nginx-agent_$VERSION~noble_amd64.deb - ubuntu/pool/agent/n/nginx-agent/nginx-agent_$VERSION~focal_amd64.deb + ubuntu/pool/agent/n/nginx-agent/nginx-agent_$VERSION~plucky_amd64.deb ) DEBIAN=( debian/pool/agent/n/nginx-agent/nginx-agent_$VERSION~bullseye_arm64.deb @@ -85,6 +83,8 @@ SUSE=( sles/15/x86_64/RPMS/nginx-agent-$VERSION.sles15.ngx.x86_64.rpm ) CENTOS=( + centos/10/aarch64/RPMS/nginx-agent-$VERSION.el10.ngx.aarch64.rpm + centos/10/x86_64/RPMS/nginx-agent-$VERSION.el10.ngx.x86_64.rpm centos/9/aarch64/RPMS/nginx-agent-$VERSION.el9.ngx.aarch64.rpm centos/9/x86_64/RPMS/nginx-agent-$VERSION.el9.ngx.x86_64.rpm centos/8/aarch64/RPMS/nginx-agent-$VERSION.el8.ngx.aarch64.rpm From d36e81288d8812826f5bbcd811cdb32bdd8448be Mon Sep 17 00:00:00 2001 From: Sean Breen <101327931+sean-breen@users.noreply.github.com> Date: Thu, 17 Jul 2025 11:07:58 +0100 Subject: [PATCH 05/13] Add check for nginx pid file in test docker images (#1166) * add check for nginx pid file * add alpine default path /var/run/nginx/nginx.pid --- test/docker/entrypoint.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/docker/entrypoint.sh b/test/docker/entrypoint.sh index 24b82bc04..890d90154 100644 --- a/test/docker/entrypoint.sh +++ b/test/docker/entrypoint.sh @@ -33,12 +33,12 @@ echo "starting nginx ..." nginx_pid=$! SECONDS=0 - -while ! ps -ef | grep "nginx: master process" | grep -v grep; do +while [[ ! -f /var/run/nginx.pid ]] && [[ ! -f /var/run/nginx/nginx.pid ]]; do if (( SECONDS > 30 )); then echo "couldn't find nginx master process" exit 1 fi + sleep 1 done cat /etc/nginx-agent/nginx-agent.conf; From 659ac669779be482892bc7ac6084b097d6bea0ef Mon Sep 17 00:00:00 2001 From: aphralG <108004222+aphralG@users.noreply.github.com> Date: Thu, 17 Jul 2025 15:37:59 +0100 Subject: [PATCH 06/13] Fix Agent failing to find NGINX process when NGINX is in debug (#1162) --- pkg/nginxprocess/process.go | 57 ++++++++++++++++++-------------- pkg/nginxprocess/process_test.go | 8 +++++ 2 files changed, 41 insertions(+), 24 deletions(-) diff --git a/pkg/nginxprocess/process.go b/pkg/nginxprocess/process.go index 11173cd05..aebf0fd57 100644 --- a/pkg/nginxprocess/process.go +++ b/pkg/nginxprocess/process.go @@ -31,7 +31,10 @@ type Process struct { func (p *Process) IsWorker() bool { return strings.HasPrefix(p.Cmd, "nginx: worker") } // IsMaster returns true if the process is a NGINX master process. -func (p *Process) IsMaster() bool { return strings.HasPrefix(p.Cmd, "nginx: master") } +func (p *Process) IsMaster() bool { + return strings.HasPrefix(p.Cmd, "nginx: master") || + strings.HasPrefix(p.Cmd, "{nginx-debug} nginx: master") +} // IsShuttingDown returns true if the process is shutting down. This can identify workers that are in the process of a // graceful shutdown. See [changing NGINX configuration] for more details. @@ -53,6 +56,7 @@ type Option interface{ apply(opts *options) } type optionFunc func(*options) +//nolint:ireturn func (f optionFunc) apply(o *options) { f(o) } // WithStatus runs an additional lookup to load the process status. @@ -66,39 +70,44 @@ func convert(ctx context.Context, p *process.Process, o options) (*Process, erro } name, _ := p.NameWithContext(ctx) // slow: shells out to ps - if name != "nginx" { + if name != "nginx" && name != "nginx-debug" { return nil, errNotAnNginxProcess } cmdLine, _ := p.CmdlineWithContext(ctx) // slow: shells out to ps // ignore nginx processes in the middle of an upgrade - if !strings.HasPrefix(cmdLine, "nginx:") || strings.Contains(cmdLine, "upgrade") { + + if strings.Contains(cmdLine, "upgrade") { return nil, errNotAnNginxProcess } - var status string - if o.loadStatus { - flags, _ := p.StatusWithContext(ctx) // slow: shells out to ps - status = strings.Join(flags, " ") - } + if strings.HasPrefix(cmdLine, "nginx:") || strings.HasPrefix(cmdLine, "{nginx-debug} nginx:") { + var status string + if o.loadStatus { + flags, _ := p.StatusWithContext(ctx) // slow: shells out to ps + status = strings.Join(flags, " ") + } - // unconditionally run fast lookups - var created time.Time - if millisSinceEpoch, err := p.CreateTimeWithContext(ctx); err == nil { - created = time.UnixMilli(millisSinceEpoch) + // unconditionally run fast lookups + var created time.Time + if millisSinceEpoch, err := p.CreateTimeWithContext(ctx); err == nil { + created = time.UnixMilli(millisSinceEpoch) + } + ppid, _ := p.PpidWithContext(ctx) + exe, _ := p.ExeWithContext(ctx) + + return &Process{ + PID: p.Pid, + PPID: ppid, + Name: name, + Cmd: cmdLine, + Created: created, + Status: status, + Exe: exe, + }, ctx.Err() } - ppid, _ := p.PpidWithContext(ctx) - exe, _ := p.ExeWithContext(ctx) - - return &Process{ - PID: p.Pid, - PPID: ppid, - Name: name, - Cmd: cmdLine, - Created: created, - Status: status, - Exe: exe, - }, ctx.Err() + + return nil, errNotAnNginxProcess } // List returns a slice of all NGINX processes. Returns a zero-length slice if no NGINX processes are found. diff --git a/pkg/nginxprocess/process_test.go b/pkg/nginxprocess/process_test.go index 5af69b832..fb8d4cbee 100644 --- a/pkg/nginxprocess/process_test.go +++ b/pkg/nginxprocess/process_test.go @@ -87,6 +87,14 @@ func TestProcess_IsNginxMaster(t *testing.T) { cmd: "nginx: cache manager process", want: false, }, + "Test 5: nginx debug master": { + cmd: "{nginx-debug} nginx: master process /usr/sbin/nginx-debug -g daemon off;", + want: true, + }, + "Test 6: nginx debug worker": { + cmd: "{nginx-debug} nginx: worker process;", + want: false, + }, } for name, tc := range testcases { t.Run(name, func(t *testing.T) { From e15d08cd32d1e4b0eb0cd8cd17559bf86571b873 Mon Sep 17 00:00:00 2001 From: aphralG <108004222+aphralG@users.noreply.github.com> Date: Thu, 17 Jul 2025 15:56:59 +0100 Subject: [PATCH 07/13] Parse NGINX Config for Valid NAP Syslog Server (#1165) --- internal/collector/otel_collector_plugin.go | 19 +- .../collector/otel_collector_plugin_test.go | 9 +- .../datasource/config/nginx_config_parser.go | 79 ++++++-- .../config/nginx_config_parser_test.go | 177 ++++++++++++++++-- internal/model/config.go | 18 +- internal/resource/resource_service_test.go | 14 +- .../nginx-with-multiple-syslog-servers.conf | 39 ++++ test/config/nginx_config.go | 20 ++ test/model/config.go | 56 +++++- 9 files changed, 356 insertions(+), 75 deletions(-) create mode 100644 test/config/nginx/nginx-with-multiple-syslog-servers.conf diff --git a/internal/collector/otel_collector_plugin.go b/internal/collector/otel_collector_plugin.go index a1c6a9876..1a8cac572 100644 --- a/internal/collector/otel_collector_plugin.go +++ b/internal/collector/otel_collector_plugin.go @@ -543,17 +543,12 @@ func (oc *Collector) updateExistingNginxOSSReceiver( func (oc *Collector) updateTcplogReceivers(nginxConfigContext *model.NginxConfigContext) bool { newTcplogReceiverAdded := false - if nginxConfigContext.NAPSysLogServers != nil { - napLoop: - for _, napSysLogServer := range nginxConfigContext.NAPSysLogServers { - if oc.doesTcplogReceiverAlreadyExist(napSysLogServer) { - continue napLoop - } - + if nginxConfigContext.NAPSysLogServer != "" { + if !oc.doesTcplogReceiverAlreadyExist(nginxConfigContext.NAPSysLogServer) { oc.config.Collector.Receivers.TcplogReceivers = append( oc.config.Collector.Receivers.TcplogReceivers, config.TcplogReceiver{ - ListenAddress: napSysLogServer, + ListenAddress: nginxConfigContext.NAPSysLogServer, Operators: []config.Operator{ { Type: "add", @@ -621,12 +616,10 @@ func (oc *Collector) configDeletedNapReceivers(nginxConfigContext *model.NginxCo elements[tcplogReceiver.ListenAddress] = true } - if nginxConfigContext.NAPSysLogServers != nil { + if nginxConfigContext.NAPSysLogServer != "" { addressesToDelete := make(map[string]bool) - for _, napAddress := range nginxConfigContext.NAPSysLogServers { - if !elements[napAddress] { - addressesToDelete[napAddress] = true - } + if !elements[nginxConfigContext.NAPSysLogServer] { + addressesToDelete[nginxConfigContext.NAPSysLogServer] = true } return addressesToDelete diff --git a/internal/collector/otel_collector_plugin_test.go b/internal/collector/otel_collector_plugin_test.go index d625d13b7..6073951a1 100644 --- a/internal/collector/otel_collector_plugin_test.go +++ b/internal/collector/otel_collector_plugin_test.go @@ -733,9 +733,7 @@ func TestCollector_updateTcplogReceivers(t *testing.T) { require.NoError(t, err) nginxConfigContext := &model.NginxConfigContext{ - NAPSysLogServers: []string{ - "localhost:151", - }, + NAPSysLogServer: "localhost:151", } assert.Empty(t, conf.Collector.Receivers.TcplogReceivers) @@ -766,9 +764,8 @@ func TestCollector_updateTcplogReceivers(t *testing.T) { }) t.Run("Test 4: New tcplogReceiver added and deleted another", func(tt *testing.T) { - tcplogReceiverDeleted := collector.updateTcplogReceivers(&model.NginxConfigContext{NAPSysLogServers: []string{ - "localhost:152", - }}) + tcplogReceiverDeleted := collector. + updateTcplogReceivers(&model.NginxConfigContext{NAPSysLogServer: "localhost:152"}) assert.True(t, tcplogReceiverDeleted) assert.Len(t, conf.Collector.Receivers.TcplogReceivers, 1) assert.Equal(t, "localhost:152", conf.Collector.Receivers.TcplogReceivers[0].ListenAddress) diff --git a/internal/datasource/config/nginx_config_parser.go b/internal/datasource/config/nginx_config_parser.go index ebdef5441..536751948 100644 --- a/internal/datasource/config/nginx_config_parser.go +++ b/internal/datasource/config/nginx_config_parser.go @@ -47,7 +47,8 @@ const ( type ( NginxConfigParser struct { - agentConfig *config.Config + agentConfig *config.Config + previousNAPSysLogServer string } ) @@ -65,7 +66,8 @@ type ( func NewNginxConfigParser(agentConfig *config.Config) *NginxConfigParser { return &NginxConfigParser{ - agentConfig: agentConfig, + agentConfig: agentConfig, + previousNAPSysLogServer: "", } } @@ -107,6 +109,7 @@ func (ncp *NginxConfigParser) createNginxConfigContext( payload *crossplane.Payload, ) (*model.NginxConfigContext, error) { napSyslogServersFound := make(map[string]bool) + napEnabled := false nginxConfigContext := &model.NginxConfigContext{ InstanceID: instance.GetInstanceMeta().GetInstanceId(), @@ -173,19 +176,11 @@ func (ncp *NginxConfigParser) createNginxConfigContext( } case "app_protect_security_log": if len(directive.Args) > 1 { - syslogArg := directive.Args[1] - re := regexp.MustCompile(`syslog:server=([\S]+)`) - matches := re.FindStringSubmatch(syslogArg) - if len(matches) > 1 { - syslogServer := matches[1] - if !napSyslogServersFound[syslogServer] { - nginxConfigContext.NAPSysLogServers = append( - nginxConfigContext.NAPSysLogServers, - syslogServer, - ) - napSyslogServersFound[syslogServer] = true - slog.DebugContext(ctx, "Found NAP syslog server", "address", syslogServer) - } + napEnabled = true + sysLogServer := ncp.findLocalSysLogServers(directive.Args[1]) + if sysLogServer != "" && !napSyslogServersFound[sysLogServer] { + napSyslogServersFound[sysLogServer] = true + slog.DebugContext(ctx, "Found NAP syslog server", "address", sysLogServer) } } } @@ -207,6 +202,17 @@ func (ncp *NginxConfigParser) createNginxConfigContext( nginxConfigContext.PlusAPI = plusAPI } + if len(napSyslogServersFound) > 0 { + syslogServer := ncp.findAvailableSyslogServers(ctx, napSyslogServersFound) + if syslogServer != "" { + nginxConfigContext.NAPSysLogServer = syslogServer + ncp.previousNAPSysLogServer = syslogServer + } + } else if napEnabled { + slog.WarnContext(ctx, "Could not find available local NGINX App Protect syslog server. "+ + "Security violations will not be collected.") + } + fileMeta, err := files.FileMeta(conf.File) if err != nil { slog.WarnContext(ctx, "Unable to get file metadata", "file_name", conf.File, "error", err) @@ -218,6 +224,49 @@ func (ncp *NginxConfigParser) createNginxConfigContext( return nginxConfigContext, nil } +func (ncp *NginxConfigParser) findAvailableSyslogServers(ctx context.Context, napSyslogServers map[string]bool) string { + if ncp.previousNAPSysLogServer != "" { + if _, ok := napSyslogServers[ncp.previousNAPSysLogServer]; ok { + return ncp.previousNAPSysLogServer + } + } + + for napSyslogServer := range napSyslogServers { + ln, err := net.Listen("tcp", napSyslogServer) + if err != nil { + slog.DebugContext(ctx, "NAP syslog server is not reachable", "address", napSyslogServer, + "error", err) + + continue + } + ln.Close() + + slog.DebugContext(ctx, "Found valid NAP syslog server", "address", napSyslogServer) + + return napSyslogServer + } + + return "" +} + +func (ncp *NginxConfigParser) findLocalSysLogServers(sysLogServer string) string { + re := regexp.MustCompile(`syslog:server=([\S]+)`) + matches := re.FindStringSubmatch(sysLogServer) + if len(matches) > 1 { + host, _, err := net.SplitHostPort(matches[1]) + if err != nil { + return "" + } + + ip := net.ParseIP(host) + if ip.IsLoopback() || strings.EqualFold(host, "localhost") { + return matches[1] + } + } + + return "" +} + func (ncp *NginxConfigParser) parseIncludeDirective(directive *crossplane.Directive) string { var include string if filepath.IsAbs(directive.Args[0]) { diff --git a/internal/datasource/config/nginx_config_parser_test.go b/internal/datasource/config/nginx_config_parser_test.go index 18e85e447..2360de2ce 100644 --- a/internal/datasource/config/nginx_config_parser_test.go +++ b/internal/datasource/config/nginx_config_parser_test.go @@ -9,6 +9,7 @@ import ( "bytes" "context" "fmt" + "net" "net/http" "net/http/httptest" "os" @@ -357,7 +358,7 @@ func TestNginxConfigParser_Parse(t *testing.T) { ltsvAccessLog.Name(), errorLog.Name(), protos.NginxOssInstance([]string{}).GetInstanceMeta().GetInstanceId(), - []string{"127.0.0.1:1515"}, + "127.0.0.1:1515", ), expectedLog: "", allowedDirectories: []string{dir}, @@ -377,7 +378,7 @@ func TestNginxConfigParser_Parse(t *testing.T) { ltsvAccessLog.Name(), errorLog.Name(), protos.NginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(), - []string{"127.0.0.1:1515"}, + "127.0.0.1:1515", ), expectedLog: "", allowedDirectories: []string{dir}, @@ -392,7 +393,7 @@ func TestNginxConfigParser_Parse(t *testing.T) { errorLog.Name(), []*mpi.File{&allowedFileWithMetas}, protos.NginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(), - nil, + "", ), expectedLog: "", allowedDirectories: []string{dir}, @@ -402,13 +403,13 @@ func TestNginxConfigParser_Parse(t *testing.T) { instance: protos.NginxPlusInstance([]string{}), content: testconfig.NginxConfWithSSLCertsWithVariables(), expectedConfigContext: &model.NginxConfigContext{ - StubStatus: &model.APIDetails{}, - PlusAPI: &model.APIDetails{}, - InstanceID: protos.NginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(), - Files: []*mpi.File{}, - AccessLogs: []*model.AccessLog{}, - ErrorLogs: []*model.ErrorLog{}, - NAPSysLogServers: nil, + StubStatus: &model.APIDetails{}, + PlusAPI: &model.APIDetails{}, + InstanceID: protos.NginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(), + Files: []*mpi.File{}, + AccessLogs: []*model.AccessLog{}, + ErrorLogs: []*model.ErrorLog{}, + NAPSysLogServer: "", }, allowedDirectories: []string{dir}, }, @@ -426,7 +427,7 @@ func TestNginxConfigParser_Parse(t *testing.T) { combinedAccessLog.Name(), ltsvAccessLog.Name(), protos.NginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(), - []string{"127.0.0.1:1515"}, + "127.0.0.1:1515", ), expectedLog: "Currently error log outputs to stderr. Log monitoring is disabled while applying a " + "config; log errors to file to enable error monitoring", @@ -446,7 +447,7 @@ func TestNginxConfigParser_Parse(t *testing.T) { combinedAccessLog.Name(), ltsvAccessLog.Name(), protos.NginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(), - []string{"127.0.0.1:1515"}, + "127.0.0.1:1515", ), expectedLog: "Currently error log outputs to stdout. Log monitoring is disabled while applying a " + "config; log errors to file to enable error monitoring", @@ -465,7 +466,7 @@ func TestNginxConfigParser_Parse(t *testing.T) { errorLog.Name(), []*mpi.File{&certFileWithMetas}, protos.NginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(), - nil, + "", ), allowedDirectories: []string{dir}, }, @@ -483,7 +484,7 @@ func TestNginxConfigParser_Parse(t *testing.T) { errorLog.Name(), []*mpi.File{&diffCertFileWithMetas, &certFileWithMetas}, protos.NginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(), - nil, + "", ), allowedDirectories: []string{dir}, }, @@ -501,10 +502,39 @@ func TestNginxConfigParser_Parse(t *testing.T) { errorLog.Name(), []*mpi.File{&certFileWithMetas}, protos.NginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(), - nil, + "", ), allowedDirectories: []string{dir}, }, + { + name: "Test 10: Check with multiple syslog servers", + instance: protos.NginxPlusInstance([]string{}), + content: testconfig.NginxConfigWithMultipleSysLogs(errorLog.Name(), accessLog.Name(), + "192.168.12.34:1517", "my.domain.com:1517", "127.0.0.1:1515"), + expectedConfigContext: modelHelpers.ConfigContextWithSysLog( + accessLog.Name(), + errorLog.Name(), + protos.NginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(), + "127.0.0.1:1515", + ), + allowedDirectories: []string{dir}, + expectedLog: "Found valid NAP syslog server", + }, + { + name: "Test 10: Check with multiple syslog servers", + instance: protos.NginxPlusInstance([]string{}), + content: testconfig.NginxConfigWithMultipleSysLogs(errorLog.Name(), accessLog.Name(), + "192.168.12.34:1517", "my.domain.com:1517", "not.allowed:1515"), + expectedConfigContext: modelHelpers.ConfigContextWithSysLog( + accessLog.Name(), + errorLog.Name(), + protos.NginxPlusInstance([]string{}).GetInstanceMeta().GetInstanceId(), + "", + ), + allowedDirectories: []string{dir}, + expectedLog: "Could not find available local NGINX App Protect syslog server. " + + "Security violations will not be collected.", + }, } for _, test := range tests { @@ -532,6 +562,7 @@ func TestNginxConfigParser_Parse(t *testing.T) { result, parseError := nginxConfig.Parse(ctx, test.instance) require.NoError(t, parseError) + t.Logf("Log: %s", logBuf.String()) helpers.ValidateLog(t, test.expectedLog, logBuf) logBuf.Reset() @@ -548,7 +579,7 @@ func TestNginxConfigParser_Parse(t *testing.T) { assert.Truef(t, protoListEqual(test.expectedConfigContext.Files, result.Files), "Expect %s Got %s", test.expectedConfigContext.Files, result.Files) - assert.Equal(t, test.expectedConfigContext.NAPSysLogServers, result.NAPSysLogServers) + assert.Equal(t, test.expectedConfigContext.NAPSysLogServer, result.NAPSysLogServer) assert.Equal(t, test.expectedConfigContext.PlusAPI, result.PlusAPI) assert.ElementsMatch(t, test.expectedConfigContext.AccessLogs, result.AccessLogs) assert.ElementsMatch(t, test.expectedConfigContext.ErrorLogs, result.ErrorLogs) @@ -582,6 +613,120 @@ func TestNginxConfigParser_sslCert(t *testing.T) { assert.Equal(t, certFile, sslCert.GetFileMeta().GetName()) } +func TestNginxConfigParser_SyslogServerParse(t *testing.T) { + ctx := context.Background() + dir := t.TempDir() + + file := helpers.CreateFileWithErrorCheck(t, dir, "nginx-parse-config.conf") + defer helpers.RemoveFileWithErrorCheck(t, file.Name()) + + errorLog := helpers.CreateFileWithErrorCheck(t, dir, "error.log") + defer helpers.RemoveFileWithErrorCheck(t, errorLog.Name()) + + accessLog := helpers.CreateFileWithErrorCheck(t, dir, "access.log") + defer helpers.RemoveFileWithErrorCheck(t, accessLog.Name()) + + instance := protos.NginxOssInstance([]string{}) + instance.InstanceRuntime.ConfigPath = file.Name() + + tests := []struct { + name string + content string + expectedLog string + expectedSyslogServers string + previousNAPSysLogServer string + portInUse bool + }{ + { + name: "Test 1: Valid port", + expectedSyslogServers: "127.0.0.1:1515", + content: testconfig.NginxConfigWithMultipleSysLogs(errorLog.Name(), accessLog.Name(), + "192.168.12.34:1517", "my.domain.com:1517", "127.0.0.1:1515"), + expectedLog: "Found valid NAP syslog server", + portInUse: false, + }, + { + name: "Test 2: No valid server", + expectedSyslogServers: "", + content: testconfig.NginxConfigWithMultipleSysLogs(errorLog.Name(), accessLog.Name(), + "random.domain:1515", "192.168.12.34:1517", "my.domain.com:1517"), + expectedLog: "Could not find available local NGINX App Protect syslog server. " + + "Security violations will not be collected.", + portInUse: false, + }, + { + name: "Test 3: Port unavailable, use next valid sever", + expectedSyslogServers: "localhost:1516", + content: testconfig.NginxConfigWithMultipleSysLogs(errorLog.Name(), accessLog.Name(), + "192.168.12.34:1517", "127.0.0.1:1515", "localhost:1516"), + expectedLog: "\"Found valid NAP syslog server\" address=localhost:1516", + portInUse: true, + }, + { + name: "Test 4: Port unavailable, no server available", + expectedSyslogServers: "", + content: testconfig.NginxConfigWithMultipleSysLogs(errorLog.Name(), accessLog.Name(), + "my.domain.com:1517", "127.0.0.1:1515", "my.domain.com:1517"), + expectedLog: "NAP syslog server is not reachable", + portInUse: true, + }, + { + name: "Test 5: Server hasn't changed", + expectedSyslogServers: "127.0.0.1:1515", + content: testconfig.NginxConfigWithMultipleSysLogs(errorLog.Name(), accessLog.Name(), + "my.domain.com:1517", "127.0.0.1:1515", "my.domain.com:1517"), + expectedLog: "Found NAP syslog server", + portInUse: true, + previousNAPSysLogServer: "127.0.0.1:1515", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + logBuf := &bytes.Buffer{} + stub.StubLoggerWith(logBuf) + + agentConfig := types.AgentConfig() + agentConfig.AllowedDirectories = []string{dir} + nginxConfig := NewNginxConfigParser(agentConfig) + nginxConfig.previousNAPSysLogServer = test.previousNAPSysLogServer + + writeErr := os.WriteFile(file.Name(), []byte(test.content), 0o600) + require.NoError(t, writeErr) + + if test.portInUse { + ln, err := net.Listen("tcp", "127.0.0.1:1515") + require.NoError(t, err) + defer ln.Close() + } + + result, parseError := nginxConfig.Parse(ctx, instance) + require.NoError(t, parseError) + + t.Logf("Log: %s", logBuf.String()) + helpers.ValidateLog(t, test.expectedLog, logBuf) + logBuf.Reset() + + assert.Equal(t, test.expectedSyslogServers, result.NAPSysLogServer) + }) + } +} + +func TestNginxConfigParser_findValidSysLogServers(t *testing.T) { + servers := []string{ + "syslog:server=192.168.12.34:1517", "syslog:server=my.domain.com:1517", "syslog:server=127.0.0.1:1515", + "syslog:server=localhost:1516", "syslog:server=127.255.255.255:1517", + } + expected := []string{"", "", "127.0.0.1:1515", "localhost:1516", "127.255.255.255:1517"} + ncp := NewNginxConfigParser(types.AgentConfig()) + + for i, server := range servers { + result := ncp.findLocalSysLogServers(server) + + assert.Equal(t, expected[i], result) + } +} + func TestNginxConfigParser_checkLog(t *testing.T) { logBuf := &bytes.Buffer{} stub.StubLoggerWith(logBuf) diff --git a/internal/model/config.go b/internal/model/config.go index ed1d92487..67dea7449 100644 --- a/internal/model/config.go +++ b/internal/model/config.go @@ -12,14 +12,14 @@ import ( ) type NginxConfigContext struct { - StubStatus *APIDetails - PlusAPI *APIDetails - InstanceID string - Files []*v1.File - AccessLogs []*AccessLog - ErrorLogs []*ErrorLog - NAPSysLogServers []string - Includes []string + StubStatus *APIDetails + PlusAPI *APIDetails + InstanceID string + Files []*v1.File + AccessLogs []*AccessLog + ErrorLogs []*ErrorLog + NAPSysLogServer string + Includes []string } type APIDetails struct { @@ -113,7 +113,7 @@ func (ncc *NginxConfigContext) Equal(otherNginxConfigContext *NginxConfigContext return false } - if !reflect.DeepEqual(ncc.NAPSysLogServers, otherNginxConfigContext.NAPSysLogServers) { + if !reflect.DeepEqual(ncc.NAPSysLogServer, otherNginxConfigContext.NAPSysLogServer) { return false } diff --git a/internal/resource/resource_service_test.go b/internal/resource/resource_service_test.go index 0b82f1b18..0123220ac 100644 --- a/internal/resource/resource_service_test.go +++ b/internal/resource/resource_service_test.go @@ -336,13 +336,13 @@ func TestResourceService_ApplyConfig(t *testing.T) { nginxParser := instancefakes.FakeNginxConfigParser{} nginxParser.ParseReturns(&model.NginxConfigContext{ - StubStatus: &model.APIDetails{}, - PlusAPI: &model.APIDetails{}, - InstanceID: test.instanceID, - Files: nil, - AccessLogs: nil, - ErrorLogs: nil, - NAPSysLogServers: nil, + StubStatus: &model.APIDetails{}, + PlusAPI: &model.APIDetails{}, + InstanceID: test.instanceID, + Files: nil, + AccessLogs: nil, + ErrorLogs: nil, + NAPSysLogServer: "", }, nil) resourceService := NewResourceService(ctx, types.AgentConfig()) diff --git a/test/config/nginx/nginx-with-multiple-syslog-servers.conf b/test/config/nginx/nginx-with-multiple-syslog-servers.conf new file mode 100644 index 000000000..01f04b026 --- /dev/null +++ b/test/config/nginx/nginx-with-multiple-syslog-servers.conf @@ -0,0 +1,39 @@ +user nginx; +worker_processes auto; + +error_log %s notice; +pid /var/run/nginx.pid; + +load_module modules/ngx_http_app_protect_module.so; + +events { + worker_connections 1024; +} + +http { + log_format upstream_time '$remote_addr - $remote_user [$time_local]'; + + server { + access_log %s upstream_time; + } +} + +http { + + server { + listen 9093; + server_name lua.example.com; + + ssl_certificate_by_lua_block { + print("Test lua block") + } + } + + server { + + app_protect_security_log "/etc/app_protect/conf/log_default.json" syslog:server=%s; + app_protect_security_log "/etc/app_protect/conf/log_default1.json" syslog:server=%s; + app_protect_security_log "/etc/app_protect/conf/log_default2.json" syslog:server=%s; + app_protect_security_log "/etc/app_protect/conf/log_default3.json" syslog:server=%s; + } +} diff --git a/test/config/nginx_config.go b/test/config/nginx_config.go index e5b3299e4..8eba52461 100644 --- a/test/config/nginx_config.go +++ b/test/config/nginx_config.go @@ -13,6 +13,9 @@ import ( //go:embed nginx/nginx-with-multiple-access-logs.conf var embedNginxConfWithMultipleAccessLogs string +//go:embed nginx/nginx-with-multiple-syslog-servers.conf +var embedNginxConfWithMultipleSysLogs string + //go:embed nginx/nginx-not-allowed-dir.conf var embedNginxConfWithNotAllowedDir string @@ -46,6 +49,23 @@ func NginxConfigWithMultipleAccessLogs( ) } +func NginxConfigWithMultipleSysLogs( + errorLogName, + accessLogName, + syslogServer, + syslogServer3, + syslogServer4 string, +) string { + return fmt.Sprintf( + embedNginxConfWithMultipleSysLogs, + errorLogName, + accessLogName, + syslogServer, + syslogServer3, + syslogServer4, + ) +} + func NginxConfigWithNotAllowedDir(errorLogFile, notAllowedFile, allowedFileDir, accessLogFile string) string { return fmt.Sprintf(embedNginxConfWithNotAllowedDir, errorLogFile, notAllowedFile, allowedFileDir, accessLogFile) } diff --git a/test/model/config.go b/test/model/config.go index 07c6c53c3..661bb2635 100644 --- a/test/model/config.go +++ b/test/model/config.go @@ -29,7 +29,7 @@ func ConfigContextWithNames( ltsvAccessLogName, errorLogName string, instanceID string, - syslogServers []string, + syslogServers string, ) *model.NginxConfigContext { return &model.NginxConfigContext{ StubStatus: &model.APIDetails{ @@ -71,8 +71,8 @@ func ConfigContextWithNames( Permissions: "0600", }, }, - InstanceID: instanceID, - NAPSysLogServers: syslogServers, + InstanceID: instanceID, + NAPSysLogServer: syslogServers, } } @@ -81,7 +81,7 @@ func ConfigContextWithoutErrorLog( combinedAccessLogName, ltsvAccessLogName, instanceID string, - syslogServers []string, + syslogServers string, ) *model.NginxConfigContext { return &model.NginxConfigContext{ StubStatus: &model.APIDetails{ @@ -115,8 +115,8 @@ func ConfigContextWithoutErrorLog( Permissions: "0600", }, }, - InstanceID: instanceID, - NAPSysLogServers: syslogServers, + InstanceID: instanceID, + NAPSysLogServer: syslogServers, } } @@ -125,7 +125,7 @@ func ConfigContextWithFiles( errorLogName string, files []*mpi.File, instanceID string, - syslogServers []string, + syslogServers string, ) *model.NginxConfigContext { return &model.NginxConfigContext{ StubStatus: &model.APIDetails{ @@ -156,7 +156,45 @@ func ConfigContextWithFiles( Permissions: "0600", }, }, - InstanceID: instanceID, - NAPSysLogServers: syslogServers, + InstanceID: instanceID, + NAPSysLogServer: syslogServers, + } +} + +func ConfigContextWithSysLog( + accessLogName, + errorLogName string, + instanceID string, + syslogServers string, +) *model.NginxConfigContext { + return &model.NginxConfigContext{ + StubStatus: &model.APIDetails{ + URL: "", + Listen: "", + Location: "", + }, + PlusAPI: &model.APIDetails{ + URL: "", + Listen: "", + Location: "", + }, + AccessLogs: []*model.AccessLog{ + { + Name: accessLogName, + Format: "$remote_addr - $remote_user [$time_local]", + Readable: true, + Permissions: "0600", + }, + }, + ErrorLogs: []*model.ErrorLog{ + { + Name: errorLogName, + Readable: true, + LogLevel: "notice", + Permissions: "0600", + }, + }, + InstanceID: instanceID, + NAPSysLogServer: syslogServers, } } From a7ea24ff43426aecc13543f3e76c32da126fae4b Mon Sep 17 00:00:00 2001 From: Sean Breen <101327931+sean-breen@users.noreply.github.com> Date: Fri, 18 Jul 2025 14:57:36 +0100 Subject: [PATCH 08/13] Add NAP paths to allowed directories (#1163) * update go verion and golangci-lint * add paths for NAP upon instance discovery * use variable for NAP directory path * add paths when creating NAP instance * add nap paths by default, update agent config during upgrade * add nap by default * nap paths to default agent configuration * add back log message * only add config directory /etc/app_protect * update preinstall.sh * update default config * remove blank line --------- Co-authored-by: Aphral Griffin --- internal/config/config_test.go | 2 +- internal/config/defaults.go | 1 + .../instance/nginx-app-protect-instance-watcher_test.go | 1 - nginx-agent.conf | 1 + scripts/packages/preinstall.sh | 1 + scripts/packages/upgrade-agent-config.sh | 3 ++- 6 files changed, 6 insertions(+), 3 deletions(-) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 6f3ad0eed..9048eb834 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -796,7 +796,7 @@ func agentConfig() *Config { }, AllowedDirectories: []string{ "/etc/nginx/", "/etc/nginx-agent/", "/usr/local/etc/nginx/", "/var/run/nginx/", "/var/log/nginx/", - "/usr/share/nginx/modules/", + "/usr/share/nginx/modules/", "/etc/app_protect/", }, Collector: &Collector{ ConfigPath: "/etc/nginx-agent/nginx-agent-otelcol.yaml", diff --git a/internal/config/defaults.go b/internal/config/defaults.go index 6c4a1ab3d..160677472 100644 --- a/internal/config/defaults.go +++ b/internal/config/defaults.go @@ -106,6 +106,7 @@ func DefaultAllowedDirectories() []string { "/usr/share/nginx/modules", "/var/run/nginx", "/var/log/nginx", + "/etc/app_protect", } } diff --git a/internal/watcher/instance/nginx-app-protect-instance-watcher_test.go b/internal/watcher/instance/nginx-app-protect-instance-watcher_test.go index 48925663d..2dae2d254 100644 --- a/internal/watcher/instance/nginx-app-protect-instance-watcher_test.go +++ b/internal/watcher/instance/nginx-app-protect-instance-watcher_test.go @@ -112,7 +112,6 @@ func TestNginxAppProtectInstanceWatcher_Watch(t *testing.T) { t.Fatalf("Timed out waiting for instance updates") } }) - t.Run("Test 2: Update instance", func(t *testing.T) { _, err = enforcerEngineVersionFile.WriteAt([]byte("6.113.0"), 0) require.NoError(t, err) diff --git a/nginx-agent.conf b/nginx-agent.conf index b67980261..559754f43 100644 --- a/nginx-agent.conf +++ b/nginx-agent.conf @@ -12,6 +12,7 @@ log: allowed_directories: - /etc/nginx + - /etc/app_protect - /usr/local/etc/nginx - /usr/share/nginx/modules - /var/run/nginx diff --git a/scripts/packages/preinstall.sh b/scripts/packages/preinstall.sh index 7bcc02250..530aa2706 100644 --- a/scripts/packages/preinstall.sh +++ b/scripts/packages/preinstall.sh @@ -109,6 +109,7 @@ labels: allowed_directories="${allowed_directories}\n - ${config_dir}" done allowed_directories="${allowed_directories}\n - /var/log/nginx" + allowed_directories="${allowed_directories}\n - /etc/app_protect" echo "Writing new v3 configuration to $v3_config_file" v3_config_contents=" diff --git a/scripts/packages/upgrade-agent-config.sh b/scripts/packages/upgrade-agent-config.sh index 0a3fefe9f..4ea7f1582 100755 --- a/scripts/packages/upgrade-agent-config.sh +++ b/scripts/packages/upgrade-agent-config.sh @@ -52,7 +52,8 @@ for config_dir in $config_dirs; do done allowed_directories="${allowed_directories}\n - /var/log/nginx" - +allowed_directories="${allowed_directories}\n - /etc/app_protect" + v3_config_contents=" # # /etc/nginx-agent/nginx-agent.conf From f3642455825875aee78455b934ac3fc33131b88f Mon Sep 17 00:00:00 2001 From: Donal Hurley Date: Mon, 21 Jul 2025 11:11:27 +0100 Subject: [PATCH 09/13] Fix include directive parsing to handle relative paths (#1174) --- .../datasource/config/nginx_config_parser.go | 9 ++- .../config/nginx_config_parser_test.go | 60 ++++++++++++++++++- internal/watcher/file/file_watcher_service.go | 14 +++-- .../watcher/file/file_watcher_service_test.go | 4 +- 4 files changed, 74 insertions(+), 13 deletions(-) diff --git a/internal/datasource/config/nginx_config_parser.go b/internal/datasource/config/nginx_config_parser.go index 536751948..74ddaf9bf 100644 --- a/internal/datasource/config/nginx_config_parser.go +++ b/internal/datasource/config/nginx_config_parser.go @@ -142,7 +142,7 @@ func (ncp *NginxConfigParser) createNginxConfigContext( func(ctx context.Context, parent, directive *crossplane.Directive) error { switch directive.Directive { case "include": - include := ncp.parseIncludeDirective(directive) + include := ncp.parseIncludeDirective(directive, &conf) nginxConfigContext.Includes = append(nginxConfigContext.Includes, include) case "log_format": @@ -267,12 +267,15 @@ func (ncp *NginxConfigParser) findLocalSysLogServers(sysLogServer string) string return "" } -func (ncp *NginxConfigParser) parseIncludeDirective(directive *crossplane.Directive) string { +func (ncp *NginxConfigParser) parseIncludeDirective( + directive *crossplane.Directive, + configFile *crossplane.Config, +) string { var include string if filepath.IsAbs(directive.Args[0]) { include = directive.Args[0] } else { - include = filepath.Join(filepath.Dir(directive.File), directive.Args[0]) + include = filepath.Join(filepath.Dir(configFile.File), directive.Args[0]) } return include diff --git a/internal/datasource/config/nginx_config_parser_test.go b/internal/datasource/config/nginx_config_parser_test.go index 2360de2ce..5587add3c 100644 --- a/internal/datasource/config/nginx_config_parser_test.go +++ b/internal/datasource/config/nginx_config_parser_test.go @@ -507,7 +507,7 @@ func TestNginxConfigParser_Parse(t *testing.T) { allowedDirectories: []string{dir}, }, { - name: "Test 10: Check with multiple syslog servers", + name: "Test 10: Available NAP syslog server", instance: protos.NginxPlusInstance([]string{}), content: testconfig.NginxConfigWithMultipleSysLogs(errorLog.Name(), accessLog.Name(), "192.168.12.34:1517", "my.domain.com:1517", "127.0.0.1:1515"), @@ -521,7 +521,7 @@ func TestNginxConfigParser_Parse(t *testing.T) { expectedLog: "Found valid NAP syslog server", }, { - name: "Test 10: Check with multiple syslog servers", + name: "Test 11: Unavailable NAP syslog server", instance: protos.NginxPlusInstance([]string{}), content: testconfig.NginxConfigWithMultipleSysLogs(errorLog.Name(), accessLog.Name(), "192.168.12.34:1517", "my.domain.com:1517", "not.allowed:1515"), @@ -1521,6 +1521,62 @@ func TestNginxConfigParser_checkDuplicate(t *testing.T) { } } +func TestNginxConfigParser_parseIncludeDirective(t *testing.T) { + parser := NewNginxConfigParser(types.AgentConfig()) + + tests := []struct { + name string + confFile string + expected string + args []string + }{ + { + name: "Test 1: relative path", + args: []string{"test.conf"}, + confFile: "/etc/nginx/nginx.conf", + expected: "/etc/nginx/test.conf", + }, + { + name: "Test 2: absolute path", + args: []string{"/usr/local/nginx/conf/vhost.conf"}, + confFile: "/etc/nginx/nginx.conf", + expected: "/usr/local/nginx/conf/vhost.conf", + }, + { + name: "Test 3: wildcard", + args: []string{"/etc/nginx/conf.d/*.conf"}, + confFile: "/etc/nginx/nginx.conf", + expected: "/etc/nginx/conf.d/*.conf", + }, + { + name: "Test 4: relative path with subdirectory", + args: []string{"conf.d/default.conf"}, + confFile: "/etc/nginx/nginx.conf", + expected: "/etc/nginx/conf.d/default.conf", + }, + { + name: "Test 5: parent directory reference", + args: []string{"../sites-enabled/*.conf"}, + confFile: "/etc/nginx/conf.d/nginx.conf", + expected: "/etc/nginx/sites-enabled/*.conf", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + include := parser.parseIncludeDirective( + &crossplane.Directive{ + Args: tc.args, + }, + &crossplane.Config{ + File: tc.confFile, + }, + ) + assert.Equal(t, tc.expected, include) + }) + } +} + func protoListEqual(protoListA, protoListB []*mpi.File) bool { for i := range protoListA { res := proto.Equal(protoListA[i], protoListB[i]) diff --git a/internal/watcher/file/file_watcher_service.go b/internal/watcher/file/file_watcher_service.go index dff86b117..10e1a2470 100644 --- a/internal/watcher/file/file_watcher_service.go +++ b/internal/watcher/file/file_watcher_service.go @@ -138,8 +138,12 @@ func (fws *FileWatcherService) addWatchers(ctx context.Context) { } if !slices.Contains(fws.watcher.WatchList(), directory) { - fws.addWatcher(ctx, directory) - fws.filesChanged.Store(true) + err := fws.addWatcher(ctx, directory) + if err != nil { + slog.DebugContext(ctx, "Failed to add file watcher", "directory", directory, "error", err) + } else { + fws.filesChanged.Store(true) + } } } } @@ -208,7 +212,7 @@ func (fws *FileWatcherService) checkForUpdates(ctx context.Context, ch chan<- Fi } } -func (fws *FileWatcherService) addWatcher(ctx context.Context, directory string) { +func (fws *FileWatcherService) addWatcher(ctx context.Context, directory string) error { slog.DebugContext(ctx, "Checking if file watcher needs to be added", "directory", directory) if _, err := os.Stat(directory); errors.Is(err, os.ErrNotExist) { @@ -220,9 +224,7 @@ func (fws *FileWatcherService) addWatcher(ctx context.Context, directory string) slog.DebugContext(ctx, "Adding watcher", "directory", directory) - if err := fws.watcher.Add(directory); err != nil { - slog.WarnContext(ctx, "Failed to add file watcher", "directory", directory, "error", err) - } + return fws.watcher.Add(directory) } func (fws *FileWatcherService) removeWatcher(ctx context.Context, path string) { diff --git a/internal/watcher/file/file_watcher_service_test.go b/internal/watcher/file/file_watcher_service_test.go index 3177c7226..56c35415a 100644 --- a/internal/watcher/file/file_watcher_service_test.go +++ b/internal/watcher/file/file_watcher_service_test.go @@ -62,7 +62,7 @@ func TestFileWatcherService_addWatcher(t *testing.T) { require.NoError(t, err) defer os.Remove(testDirectory) - fileWatcherService.addWatcher(ctx, testDirectory) + require.NoError(t, fileWatcherService.addWatcher(ctx, testDirectory)) directoriesBeingWatched := fileWatcherService.watcher.WatchList() assert.Len(t, directoriesBeingWatched, 1) @@ -79,7 +79,7 @@ func TestFileWatcherService_addWatcher_Error(t *testing.T) { tempDir := t.TempDir() testDirectory := path.Join(tempDir, "test_dir") - fileWatcherService.addWatcher(ctx, testDirectory) + require.Error(t, fileWatcherService.addWatcher(ctx, testDirectory)) directoriesBeingWatched := fileWatcherService.watcher.WatchList() assert.Empty(t, directoriesBeingWatched) From 9f04dbfbeda80c4f4fae51a2c9b6ed9f0eeef480 Mon Sep 17 00:00:00 2001 From: Sean Breen <101327931+sean-breen@users.noreply.github.com> Date: Mon, 21 Jul 2025 15:20:29 +0100 Subject: [PATCH 10/13] add nap logs to default features (#1172) --- internal/config/defaults.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/config/defaults.go b/internal/config/defaults.go index 160677472..40b0767a1 100644 --- a/internal/config/defaults.go +++ b/internal/config/defaults.go @@ -96,6 +96,7 @@ func DefaultFeatures() []string { pkg.FeatureCertificates, pkg.FeatureMetrics, pkg.FeatureFileWatcher, + pkg.FeatureLogsNap, } } From 04828e615d7b674443bd51d7a16ed6c833eb38c8 Mon Sep 17 00:00:00 2001 From: Donal Hurley Date: Tue, 22 Jul 2025 13:57:37 +0100 Subject: [PATCH 11/13] Add support for configuring mutliple OTel pipelines (#1167) --- internal/collector/otel_collector_plugin.go | 91 +++---- .../collector/otel_collector_plugin_test.go | 73 ++--- internal/collector/otelcol.tmpl | 140 +++++----- internal/collector/settings.go | 63 +++-- internal/collector/settings_test.go | 44 +-- internal/config/config.go | 251 +++++++++++++----- internal/config/config_test.go | 183 +++++++++---- internal/config/defaults.go | 9 +- internal/config/flags.go | 9 +- internal/config/testdata/nginx-agent.conf | 88 +++--- internal/config/types.go | 56 ++-- internal/plugin/plugin_manager.go | 2 +- test/config/agent/nginx-agent-otel-load.conf | 23 +- .../test-opentelemetry-collector-agent.yaml | 33 +-- test/types/config.go | 31 ++- 15 files changed, 664 insertions(+), 432 deletions(-) diff --git a/internal/collector/otel_collector_plugin.go b/internal/collector/otel_collector_plugin.go index 1a8cac572..d6d452a41 100644 --- a/internal/collector/otel_collector_plugin.go +++ b/internal/collector/otel_collector_plugin.go @@ -60,7 +60,7 @@ var ( ) // NewCollector is the constructor for the Collector plugin. -func New(conf *config.Config) (*Collector, error) { +func NewCollector(conf *config.Config) (*Collector, error) { initMutex.Lock() defer initMutex.Unlock() @@ -194,7 +194,7 @@ func (oc *Collector) Subscriptions() []string { } // Process receivers and log warning for sub-optimal configurations -func (oc *Collector) processReceivers(ctx context.Context, receivers []config.OtlpReceiver) { +func (oc *Collector) processReceivers(ctx context.Context, receivers map[string]*config.OtlpReceiver) { for _, receiver := range receivers { if receiver.OtlpTLSConfig == nil { slog.WarnContext(ctx, "OTel receiver is configured without TLS. Connections are unencrypted.") @@ -317,12 +317,13 @@ func (oc *Collector) updateResourceProcessor(resourceUpdateContext *v1.Resource) resourceProcessorUpdated := false if oc.config.Collector.Processors.Resource == nil { - oc.config.Collector.Processors.Resource = &config.Resource{ + oc.config.Collector.Processors.Resource = make(map[string]*config.Resource) + oc.config.Collector.Processors.Resource["default"] = &config.Resource{ Attributes: make([]config.ResourceAttribute, 0), } } - if oc.config.Collector.Processors.Resource != nil && + if oc.config.Collector.Processors.Resource["default"] != nil && resourceUpdateContext.GetResourceId() != "" { resourceProcessorUpdated = oc.updateResourceAttributes( []config.ResourceAttribute{ @@ -431,7 +432,7 @@ func (oc *Collector) checkForNewReceivers(ctx context.Context, nginxConfigContex } if oc.config.IsFeatureEnabled(pkgConfig.FeatureLogsNap) { - tcplogReceiversFound := oc.updateTcplogReceivers(nginxConfigContext) + tcplogReceiversFound := oc.updateNginxAppProtectTcplogReceivers(nginxConfigContext) if tcplogReceiversFound { reloadCollector = true } @@ -541,44 +542,46 @@ func (oc *Collector) updateExistingNginxOSSReceiver( return nginxReceiverFound, reloadCollector } -func (oc *Collector) updateTcplogReceivers(nginxConfigContext *model.NginxConfigContext) bool { +func (oc *Collector) updateNginxAppProtectTcplogReceivers(nginxConfigContext *model.NginxConfigContext) bool { newTcplogReceiverAdded := false + + if oc.config.Collector.Receivers.TcplogReceivers == nil { + oc.config.Collector.Receivers.TcplogReceivers = make(map[string]*config.TcplogReceiver) + } + if nginxConfigContext.NAPSysLogServer != "" { if !oc.doesTcplogReceiverAlreadyExist(nginxConfigContext.NAPSysLogServer) { - oc.config.Collector.Receivers.TcplogReceivers = append( - oc.config.Collector.Receivers.TcplogReceivers, - config.TcplogReceiver{ - ListenAddress: nginxConfigContext.NAPSysLogServer, - Operators: []config.Operator{ - { - Type: "add", - Fields: map[string]string{ - "field": "body", - "value": timestampConversionExpression, - }, + oc.config.Collector.Receivers.TcplogReceivers["nginx_app_protect"] = &config.TcplogReceiver{ + ListenAddress: nginxConfigContext.NAPSysLogServer, + Operators: []config.Operator{ + { + Type: "add", + Fields: map[string]string{ + "field": "body", + "value": timestampConversionExpression, }, - { - Type: "syslog_parser", - Fields: map[string]string{ - "protocol": "rfc3164", - }, + }, + { + Type: "syslog_parser", + Fields: map[string]string{ + "protocol": "rfc3164", }, - { - Type: "remove", - Fields: map[string]string{ - "field": "attributes.message", - }, + }, + { + Type: "remove", + Fields: map[string]string{ + "field": "attributes.message", }, - { - Type: "add", - Fields: map[string]string{ - "field": "resource[\"instance.id\"]", - "value": nginxConfigContext.InstanceID, - }, + }, + { + Type: "add", + Fields: map[string]string{ + "field": "resource[\"instance.id\"]", + "value": nginxConfigContext.InstanceID, }, }, }, - ) + } newTcplogReceiverAdded = true } @@ -592,23 +595,13 @@ func (oc *Collector) updateTcplogReceivers(nginxConfigContext *model.NginxConfig func (oc *Collector) areNapReceiversDeleted(nginxConfigContext *model.NginxConfigContext) bool { listenAddressesToBeDeleted := oc.configDeletedNapReceivers(nginxConfigContext) if len(listenAddressesToBeDeleted) != 0 { - oc.deleteNapReceivers(listenAddressesToBeDeleted) + delete(oc.config.Collector.Receivers.TcplogReceivers, "nginx_app_protect") return true } return false } -func (oc *Collector) deleteNapReceivers(listenAddressesToBeDeleted map[string]bool) { - filteredReceivers := (oc.config.Collector.Receivers.TcplogReceivers)[:0] - for _, receiver := range oc.config.Collector.Receivers.TcplogReceivers { - if !listenAddressesToBeDeleted[receiver.ListenAddress] { - filteredReceivers = append(filteredReceivers, receiver) - } - } - oc.config.Collector.Receivers.TcplogReceivers = filteredReceivers -} - func (oc *Collector) configDeletedNapReceivers(nginxConfigContext *model.NginxConfigContext) map[string]bool { elements := make(map[string]bool) @@ -644,16 +637,16 @@ func (oc *Collector) updateResourceAttributes( ) (actionUpdated bool) { actionUpdated = false - if oc.config.Collector.Processors.Resource.Attributes != nil { + if oc.config.Collector.Processors.Resource["default"].Attributes != nil { OUTER: for _, toAdd := range attributesToAdd { - for _, action := range oc.config.Collector.Processors.Resource.Attributes { + for _, action := range oc.config.Collector.Processors.Resource["default"].Attributes { if action.Key == toAdd.Key { continue OUTER } } - oc.config.Collector.Processors.Resource.Attributes = append( - oc.config.Collector.Processors.Resource.Attributes, + oc.config.Collector.Processors.Resource["default"].Attributes = append( + oc.config.Collector.Processors.Resource["default"].Attributes, toAdd, ) actionUpdated = true diff --git a/internal/collector/otel_collector_plugin_test.go b/internal/collector/otel_collector_plugin_test.go index 6073951a1..2fa51cef4 100644 --- a/internal/collector/otel_collector_plugin_test.go +++ b/internal/collector/otel_collector_plugin_test.go @@ -67,7 +67,7 @@ func TestCollector_New(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - collector, err := New(tt.config) + collector, err := NewCollector(tt.config) if tt.expectedError != nil { require.Error(t, err) @@ -114,7 +114,7 @@ func TestCollector_Init(t *testing.T) { conf.Collector.Receivers = config.Receivers{} } - collector, err = New(conf) + collector, err = NewCollector(conf) require.NoError(t, err, "NewCollector should not return an error with valid config") collector.service = createFakeCollector() @@ -133,7 +133,7 @@ func TestCollector_InitAndClose(t *testing.T) { conf := types.OTelConfig(t) conf.Collector.Log.Path = "" - collector, err := New(conf) + collector, err := NewCollector(conf) require.NoError(t, err, "NewCollector should not return an error with valid config") ctx := context.Background() @@ -293,7 +293,7 @@ func TestCollector_ProcessNginxConfigUpdateTopic(t *testing.T) { conf.Collector.Extensions.HeadersSetter = nil conf.Collector.Exporters.PrometheusExporter = nil - collector, err := New(conf) + collector, err := NewCollector(conf) require.NoError(tt, err, "NewCollector should not return an error with valid config") collector.service = createFakeCollector() @@ -349,12 +349,14 @@ func TestCollector_ProcessResourceUpdateTopic(t *testing.T) { Data: protos.HostResource(), }, processors: config.Processors{ - Resource: &config.Resource{ - Attributes: []config.ResourceAttribute{ - { - Key: "resource.id", - Action: "insert", - Value: "1234", + Resource: map[string]*config.Resource{ + "default": { + Attributes: []config.ResourceAttribute{ + { + Key: "resource.id", + Action: "insert", + Value: "1234", + }, }, }, }, @@ -376,7 +378,7 @@ func TestCollector_ProcessResourceUpdateTopic(t *testing.T) { for _, test := range tests { t.Run(test.name, func(tt *testing.T) { - collector, err := New(conf) + collector, err := NewCollector(conf) require.NoError(tt, err, "NewCollector should not return an error with valid config") collector.service = createFakeCollector() @@ -437,7 +439,7 @@ func TestCollector_ProcessResourceUpdateTopicFails(t *testing.T) { for _, test := range tests { t.Run(test.name, func(tt *testing.T) { - collector, err := New(conf) + collector, err := NewCollector(conf) require.NoError(tt, err, "NewCollector should not return an error with valid config") collector.service = createFakeCollector() @@ -559,7 +561,7 @@ func TestCollector_updateExistingNginxOSSReceiver(t *testing.T) { for _, test := range tests { t.Run(test.name, func(tt *testing.T) { conf.Collector.Receivers = test.existingReceivers - collector, err := New(conf) + collector, err := NewCollector(conf) require.NoError(tt, err, "NewCollector should not return an error with valid config") collector.service = createFakeCollector() @@ -650,7 +652,7 @@ func TestCollector_updateExistingNginxPlusReceiver(t *testing.T) { for _, test := range tests { t.Run(test.name, func(tt *testing.T) { conf.Collector.Receivers = test.existingReceivers - collector, err := New(conf) + collector, err := NewCollector(conf) require.NoError(tt, err, "NewCollector should not return an error with valid config") collector.service = createFakeCollector() @@ -705,31 +707,32 @@ func TestCollector_updateResourceAttributes(t *testing.T) { for _, test := range tests { t.Run(test.name, func(tt *testing.T) { - collector, err := New(conf) + collector, err := NewCollector(conf) require.NoError(tt, err, "NewCollector should not return an error with valid config") collector.service = createFakeCollector() // set up Actions - conf.Collector.Processors.Resource = &config.Resource{Attributes: test.setup} + conf.Collector.Processors.Resource = make(map[string]*config.Resource) + conf.Collector.Processors.Resource["default"] = &config.Resource{Attributes: test.setup} reloadRequired := collector.updateResourceAttributes(test.attributes) assert.Equal(tt, test.expectedAttribs, - conf.Collector.Processors.Resource.Attributes) + conf.Collector.Processors.Resource["default"].Attributes) assert.Equal(tt, test.expectedReloadRequired, reloadRequired) }) } } -func TestCollector_updateTcplogReceivers(t *testing.T) { +func TestCollector_updateNginxAppProtectTcplogReceivers(t *testing.T) { conf := types.OTelConfig(t) conf.Collector.Log.Path = "" conf.Collector.Processors.Batch = nil conf.Collector.Processors.Attribute = nil conf.Collector.Processors.Resource = nil conf.Collector.Processors.LogsGzip = nil - collector, err := New(conf) + collector, err := NewCollector(conf) require.NoError(t, err) nginxConfigContext := &model.NginxConfigContext{ @@ -738,38 +741,42 @@ func TestCollector_updateTcplogReceivers(t *testing.T) { assert.Empty(t, conf.Collector.Receivers.TcplogReceivers) - t.Run("Test 1: New TcplogReceiver added", func(tt *testing.T) { - tcplogReceiverAdded := collector.updateTcplogReceivers(nginxConfigContext) + t.Run("Test 1: NewCollector TcplogReceiver added", func(tt *testing.T) { + tcplogReceiverAdded := collector.updateNginxAppProtectTcplogReceivers(nginxConfigContext) assert.True(tt, tcplogReceiverAdded) assert.Len(tt, conf.Collector.Receivers.TcplogReceivers, 1) - assert.Equal(tt, "localhost:151", conf.Collector.Receivers.TcplogReceivers[0].ListenAddress) - assert.Len(tt, conf.Collector.Receivers.TcplogReceivers[0].Operators, 4) + assert.Equal(tt, "localhost:151", conf.Collector.Receivers.TcplogReceivers["nginx_app_protect"].ListenAddress) + assert.Len(tt, conf.Collector.Receivers.TcplogReceivers["nginx_app_protect"].Operators, 4) }) - // Calling updateTcplogReceivers shouldn't update the TcplogReceivers slice + // Calling updateNginxAppProtectTcplogReceivers shouldn't update the TcplogReceivers slice // since there is already a receiver with the same ListenAddress t.Run("Test 2: TcplogReceiver already exists", func(tt *testing.T) { - tcplogReceiverAdded := collector.updateTcplogReceivers(nginxConfigContext) + tcplogReceiverAdded := collector.updateNginxAppProtectTcplogReceivers(nginxConfigContext) assert.False(t, tcplogReceiverAdded) assert.Len(t, conf.Collector.Receivers.TcplogReceivers, 1) - assert.Equal(t, "localhost:151", conf.Collector.Receivers.TcplogReceivers[0].ListenAddress) - assert.Len(t, conf.Collector.Receivers.TcplogReceivers[0].Operators, 4) + assert.Equal(t, "localhost:151", conf.Collector.Receivers.TcplogReceivers["nginx_app_protect"].ListenAddress) + assert.Len(t, conf.Collector.Receivers.TcplogReceivers["nginx_app_protect"].Operators, 4) }) t.Run("Test 3: TcplogReceiver deleted", func(tt *testing.T) { - tcplogReceiverDeleted := collector.updateTcplogReceivers(&model.NginxConfigContext{}) + tcplogReceiverDeleted := collector.updateNginxAppProtectTcplogReceivers(&model.NginxConfigContext{}) assert.True(t, tcplogReceiverDeleted) assert.Empty(t, conf.Collector.Receivers.TcplogReceivers) }) - t.Run("Test 4: New tcplogReceiver added and deleted another", func(tt *testing.T) { - tcplogReceiverDeleted := collector. - updateTcplogReceivers(&model.NginxConfigContext{NAPSysLogServer: "localhost:152"}) + t.Run("Test 4: NewCollector tcplogReceiver added and deleted another", func(tt *testing.T) { + tcplogReceiverDeleted := collector.updateNginxAppProtectTcplogReceivers( + &model.NginxConfigContext{ + NAPSysLogServer: "localhost:152", + }, + ) + assert.True(t, tcplogReceiverDeleted) assert.Len(t, conf.Collector.Receivers.TcplogReceivers, 1) - assert.Equal(t, "localhost:152", conf.Collector.Receivers.TcplogReceivers[0].ListenAddress) - assert.Len(t, conf.Collector.Receivers.TcplogReceivers[0].Operators, 4) + assert.Equal(t, "localhost:152", conf.Collector.Receivers.TcplogReceivers["nginx_app_protect"].ListenAddress) + assert.Len(t, conf.Collector.Receivers.TcplogReceivers["nginx_app_protect"].Operators, 4) }) } diff --git a/internal/collector/otelcol.tmpl b/internal/collector/otelcol.tmpl index 6af00e9d2..b331152c0 100644 --- a/internal/collector/otelcol.tmpl +++ b/internal/collector/otelcol.tmpl @@ -5,6 +5,7 @@ receivers: collection_interval: {{ .Receivers.ContainerMetrics.CollectionInterval }} {{- end}} {{- end }} + {{- if ne .Receivers.HostMetrics nil }} hostmetrics: {{- if .Receivers.HostMetrics.CollectionInterval }} @@ -39,7 +40,8 @@ receivers: network: {{- end }} {{- end }} -{{- end }} +{{- end }} + {{- range $index, $otlpReceiver := .Receivers.OtlpReceivers }} otlp/{{$index}}: protocols: @@ -75,6 +77,7 @@ receivers: {{- end }} {{- end }} {{- end }} + {{- range .Receivers.NginxReceivers }} nginx/{{- .InstanceID -}}: api_details: @@ -92,6 +95,7 @@ receivers: {{- end }} {{- end }} {{- end }} + {{- range .Receivers.NginxPlusReceivers }} nginxplus/{{- .InstanceID -}}: api_details: @@ -102,6 +106,7 @@ receivers: collection_interval: {{ .CollectionInterval }} {{- end }} {{- end }} + {{- range $index, $tcplogReceiver := .Receivers.TcplogReceivers }} tcplog/{{$index}}: listen_address: "{{- .ListenAddress -}}" @@ -116,35 +121,43 @@ receivers: processors: {{- if ne .Processors.Resource nil }} -{{- if .Processors.Resource.Attributes }} - resource: + {{- range $key, $resource := .Processors.Resource }} + {{- if $resource.Attributes }} + resource/{{$key}}: attributes: -{{- range .Processors.Resource.Attributes }} + {{- range $resource.Attributes }} - key: {{ .Key }} action: {{ .Action }} value: {{ .Value }} -{{- end }} -{{- end }} + {{- end }} + {{- end }} + {{- end }} {{- end }} {{- if ne .Processors.Attribute nil }} -{{- if .Processors.Attribute.Actions }} - attributes: + {{- range $key, $attribute := .Processors.Attribute }} + {{- if $attribute.Actions }} + attributes/{{$key}}: actions: -{{- range .Processors.Attribute.Actions }} + {{- range $attribute.Actions }} - key: {{ .Key }} action: {{ .Action }} value: {{ .Value }} -{{- end }} -{{- end }} + {{- end }} + {{- end }} + {{- end }} {{- end }} {{- if ne .Processors.Batch nil }} - batch: - send_batch_size: {{ .Processors.Batch.SendBatchSize }} - timeout: {{ .Processors.Batch.Timeout }} - send_batch_max_size: {{ .Processors.Batch.SendBatchMaxSize }} + {{- range $key, $batch := .Processors.Batch }} + batch/{{$key}}: + send_batch_size: {{ $batch.SendBatchSize }} + timeout: {{ $batch.Timeout }} + send_batch_max_size: {{ $batch.SendBatchMaxSize }} +{{- end }} {{- end }} {{- if ne .Processors.LogsGzip nil }} - logsgzip: {} +{{ range $key, $value := .Processors.LogsGzip }} + logsgzip/{{$key}}: {} +{{- end }} {{- end }} exporters: @@ -173,6 +186,7 @@ exporters: authenticator: {{ .Authenticator -}} {{- end }} {{- end }} + {{- if ne .Exporters.PrometheusExporter nil }} prometheus: endpoint: "{{ .Exporters.PrometheusExporter.Server.Host -}}:{{- .Exporters.PrometheusExporter.Server.Port }}" @@ -241,73 +255,59 @@ service: - headers_setter {{- end}} {{- end}} + pipelines: - {{- if or (ne .Receivers.HostMetrics nil) (ne .Receivers.ContainerMetrics nil) (gt (len .Receivers.OtlpReceivers) 0) (gt (len .Receivers.NginxReceivers) 0) (gt (len .Receivers.NginxPlusReceivers) 0) }} - metrics: + {{- range $pipelineName, $pipeline := .Pipelines.Metrics }} + {{- if or (ne $.Receivers.HostMetrics nil) (ne $.Receivers.ContainerMetrics nil) (gt (len $.Receivers.OtlpReceivers) 0) (gt (len $.Receivers.NginxReceivers) 0) (gt (len $.Receivers.NginxPlusReceivers) 0) }} + metrics/{{$pipelineName}}: receivers: - {{- if ne .Receivers.ContainerMetrics nil }} + {{- range $receiver := $pipeline.Receivers }} + {{- if eq $receiver "host_metrics" }} + {{- if ne $.Receivers.ContainerMetrics nil }} - containermetrics - {{- end }} - {{- if ne .Receivers.HostMetrics nil }} + {{- end }} + {{- if ne $.Receivers.HostMetrics nil }} - hostmetrics - {{- end }} - {{- range $index, $otlpReceiver := .Receivers.OtlpReceivers }} - - otlp/{{$index}} - {{- end }} - {{- range .Receivers.NginxReceivers }} + {{- end }} + {{- else if eq $receiver "nginx_metrics" }} + {{- range $.Receivers.NginxReceivers }} - nginx/{{- .InstanceID -}} - {{- end }} - {{- range .Receivers.NginxPlusReceivers }} + {{- end }} + {{- range $.Receivers.NginxPlusReceivers }} - nginxplus/{{- .InstanceID -}} + {{- end }} + {{- else }} + - {{ $receiver }} + {{- end }} {{- end }} processors: - {{- if ne .Processors.Resource nil }} - {{- if .Processors.Resource.Attributes }} - - resource - {{- end }} - {{- end }} - {{- if ne .Processors.Attribute nil }} - {{- if .Processors.Attribute.Actions }} - - attributes - {{- end }} - {{- end }} - {{- if ne .Processors.Batch nil }} - - batch + {{- range $pipeline.Processors }} + - {{ . }} {{- end }} exporters: - {{- range $index, $otlpExporter := .Exporters.OtlpExporters }} - - otlp/{{$index}} - {{- end }} - {{- if ne .Exporters.PrometheusExporter nil }} - - prometheus - {{- end }} - {{- if ne .Exporters.Debug nil }} - - debug - {{- end }} - {{- end }} - {{- if gt (len .Receivers.TcplogReceivers) 0 }} - logs: + {{- range $pipeline.Exporters }} + - {{ . }} + {{- end }} + {{- end }} + {{- end }} + {{- range $pipelineName, $pipeline := .Pipelines.Logs }} + {{- if gt (len $.Receivers.TcplogReceivers) 0 }} + logs/{{$pipelineName}}: receivers: - {{- range $index, $tcplogReceiver := .Receivers.TcplogReceivers }} - - tcplog/{{$index}} + {{- range $receiver := $pipeline.Receivers }} + {{- if eq $receiver "tcplog/nginx_app_protect" }} + - tcplog/nginx_app_protect: + {{- else }} + - {{ $receiver }} + {{- end }} {{- end }} processors: - {{- if ne .Processors.Resource nil }} - {{- if .Processors.Resource.Attributes }} - - resource - {{- end }} - {{- end }} - {{- if ne .Processors.LogsGzip nil }} - - logsgzip - {{- end }} - {{- if ne .Processors.Batch nil }} - - batch + {{- range $pipeline.Processors }} + - {{ . }} {{- end }} exporters: - {{- range $index, $otlpExporter := .Exporters.OtlpExporters }} - - otlp/{{$index}} + {{- range $pipeline.Exporters }} + - {{ . }} {{- end }} - {{- if ne .Exporters.Debug nil }} - - debug - {{- end }} - {{- end }} + {{- end }} + {{- end }} diff --git a/internal/collector/settings.go b/internal/collector/settings.go index 62e92afbb..e89bb89d6 100644 --- a/internal/collector/settings.go +++ b/internal/collector/settings.go @@ -9,6 +9,7 @@ import ( "log/slog" "os" "path/filepath" + "slices" "text/template" "github.com/nginx/agent/v3/internal/config" @@ -89,43 +90,59 @@ func createFile(confPath string) error { return nil } -// Generates a OTel Collector config to a file by injecting the Metrics Config to a Go template. +// Generates an OTel Collector config to a file by injecting the Metrics Config to a Go template. func writeCollectorConfig(conf *config.Collector) error { - otelcolTemplate, err := template.New(otelTemplatePath).Parse(otelcolTemplate) - if err != nil { - return err + if conf.Processors.Resource["default"] != nil { + addDefaultResourceProcessor(conf.Pipelines.Metrics) + addDefaultResourceProcessor(conf.Pipelines.Logs) } - confPath := filepath.Clean(conf.ConfigPath) + slog.Info("Writing OTel collector config") - // Check if file exists, if not create it. - _, err = os.Stat(confPath) - if err != nil { - if !os.IsNotExist(err) { - return err - } + otelcolTemplate, templateErr := template.New(otelTemplatePath).Parse(otelcolTemplate) + if templateErr != nil { + return templateErr + } - fileErr := createFile(confPath) - if fileErr != nil { - return fileErr - } + confPath := filepath.Clean(conf.ConfigPath) + + // Ensure file exists and has correct permissions + if err := ensureFileExists(confPath); err != nil { + return err } file, err := os.OpenFile(confPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, configFilePermission) + if err != nil { + return err + } defer func() { - err = file.Close() - if err != nil { - slog.Warn("Failed to close file", "file_path", confPath) + fileCloseErr := file.Close() + if fileCloseErr != nil { + slog.Warn("Failed to close file", "file_path", confPath, "error", fileCloseErr) } }() - if err != nil { - return err + + return otelcolTemplate.Execute(file, conf) +} + +func addDefaultResourceProcessor(pipelines map[string]*config.Pipeline) { + for _, pipeline := range pipelines { + if !slices.Contains(pipeline.Processors, "resource/default") { + pipeline.Processors = append(pipeline.Processors, "resource/default") + } } +} - err = otelcolTemplate.Execute(file, conf) +func ensureFileExists(confPath string) error { + _, err := os.Stat(confPath) if err != nil { - return err + if !os.IsNotExist(err) { + return err + } + if createFileErr := createFile(confPath); createFileErr != nil { + return createFileErr + } } - return nil + return os.Chmod(confPath, configFilePermission) } diff --git a/internal/collector/settings_test.go b/internal/collector/settings_test.go index af89bd1e1..f8b8c7be4 100644 --- a/internal/collector/settings_test.go +++ b/internal/collector/settings_test.go @@ -54,12 +54,14 @@ func TestTemplateWrite(t *testing.T) { cfg := types.AgentConfig() actualConfPath := filepath.Join(tmpDir, "nginx-agent-otelcol-test.yaml") cfg.Collector.ConfigPath = actualConfPath - cfg.Collector.Processors.Resource = &config.Resource{ - Attributes: []config.ResourceAttribute{ - { - Key: "resource.id", - Action: "add", - Value: "12345", + cfg.Collector.Processors.Resource = map[string]*config.Resource{ + "default": { + Attributes: []config.ResourceAttribute{ + { + Key: "resource.id", + Action: "add", + Value: "12345", + }, }, }, } @@ -106,8 +108,7 @@ func TestTemplateWrite(t *testing.T) { }, }) // Clear default config and test collector with TLS enabled - cfg.Collector.Receivers.OtlpReceivers = []config.OtlpReceiver{} - cfg.Collector.Receivers.OtlpReceivers = append(cfg.Collector.Receivers.OtlpReceivers, config.OtlpReceiver{ + cfg.Collector.Receivers.OtlpReceivers["default"] = &config.OtlpReceiver{ Server: &config.ServerConfig{ Host: "localhost", Port: 4317, @@ -118,10 +119,10 @@ func TestTemplateWrite(t *testing.T) { Key: "/tmp/key.pem", Ca: "/tmp/ca.pem", }, - }) + } - cfg.Collector.Receivers.TcplogReceivers = []config.TcplogReceiver{ - { + cfg.Collector.Receivers.TcplogReceivers = map[string]*config.TcplogReceiver{ + "default": { ListenAddress: "localhost:151", Operators: []config.Operator{ { @@ -155,13 +156,26 @@ func TestTemplateWrite(t *testing.T) { }, } - cfg.Collector.Exporters.OtlpExporters[0].Authenticator = "headers_setter" + cfg.Collector.Exporters.OtlpExporters["default"].Authenticator = "headers_setter" // nolint: lll - cfg.Collector.Exporters.OtlpExporters[0].Compression = types.AgentConfig().Collector.Exporters.OtlpExporters[0].Compression - cfg.Collector.Exporters.OtlpExporters[0].Server.Port = 1234 - cfg.Collector.Receivers.OtlpReceivers[0].Server.Port = 4317 + cfg.Collector.Exporters.OtlpExporters["default"].Compression = types.AgentConfig().Collector.Exporters.OtlpExporters["default"].Compression + cfg.Collector.Exporters.OtlpExporters["default"].Server.Port = 1234 + cfg.Collector.Receivers.OtlpReceivers["default"].Server.Port = 4317 cfg.Collector.Extensions.Health.Server.Port = 1337 + cfg.Collector.Pipelines.Metrics = make(map[string]*config.Pipeline) + cfg.Collector.Pipelines.Metrics["default"] = &config.Pipeline{ + Receivers: []string{"hostmetrics", "containermetrics", "otlp/default", "nginx/123"}, + Processors: []string{"resource/default", "batch/default"}, + Exporters: []string{"otlp/default", "prometheus", "debug"}, + } + cfg.Collector.Pipelines.Logs = make(map[string]*config.Pipeline) + cfg.Collector.Pipelines.Logs["default"] = &config.Pipeline{ + Receivers: []string{"tcplog/default"}, + Processors: []string{"resource/default", "batch/default"}, + Exporters: []string{"otlp/default", "debug"}, + } + require.NotNil(t, cfg) err := writeCollectorConfig(cfg.Collector) diff --git a/internal/config/config.go b/internal/config/config.go index 4ebd75900..79e661efc 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -32,11 +32,15 @@ import ( ) const ( - ConfigFileName = "nginx-agent.conf" - EnvPrefix = "NGINX_AGENT" - KeyDelimiter = "_" - KeyValueNumber = 2 - AgentDirName = "/etc/nginx-agent/" + ConfigFileName = "nginx-agent.conf" + EnvPrefix = "NGINX_AGENT" + KeyDelimiter = "_" + KeyValueNumber = 2 + AgentDirName = "/etc/nginx-agent/" + DefaultMetricsBatchProcessor = "default_metrics" + DefaultLogsBatchProcessor = "default_logs" + DefaultExporter = "default" + DefaultPipeline = "default" ) var viperInstance = viper.NewWithOptions(viper.KeyDelimiter(KeyDelimiter)) @@ -128,7 +132,7 @@ func ResolveConfig() (*Config, error) { ManifestDir: viperInstance.GetString(ManifestDirPathKey), } - checkCollectorConfiguration(collector, config) + defaultCollector(collector, config) addLabelsAsOTelHeaders(collector, config.Labels) slog.Debug("Agent config", "config", config) @@ -138,34 +142,150 @@ func ResolveConfig() (*Config, error) { return config, nil } -func checkCollectorConfiguration(collector *Collector, config *Config) { - if isOTelExporterConfigured(collector) && config.IsCommandGrpcClientConfigured() && - config.IsCommandAuthConfigured() && - config.IsCommandTLSConfigured() { - slog.Info("No collector configuration found in NGINX Agent config, command server configuration found." + - " Using default collector configuration") - defaultCollector(collector, config) +func defaultCollector(collector *Collector, config *Config) { + // Always add default host metric receiver and default processor + addDefaultHostMetricsReceiver(collector) + addDefaultProcessors(collector) + + // Only add default otlp exporter and pipelines if connected to a management plane + if config.IsCommandGrpcClientConfigured() || config.IsAuxiliaryCommandGrpcClientConfigured() { + addDefaultOtlpExporter(collector, config) + addDefaultPipelines(collector) } } -func defaultCollector(collector *Collector, config *Config) { - token := config.Command.Auth.Token - if config.Command.Auth.TokenPath != "" { - slog.Debug("Reading token from file", "path", config.Command.Auth.TokenPath) - pathToken, err := file.ReadFromFile(config.Command.Auth.TokenPath) +func addDefaultPipelines(collector *Collector) { + if collector.Pipelines.Metrics == nil { + collector.Pipelines.Metrics = make(map[string]*Pipeline) + } + if _, ok := collector.Pipelines.Metrics[DefaultPipeline]; !ok { + collector.Pipelines.Metrics[DefaultPipeline] = &Pipeline{ + Receivers: []string{"host_metrics", "nginx_metrics"}, + Processors: []string{"batch/default_metrics"}, + Exporters: []string{"otlp/default"}, + } + } + + if collector.Pipelines.Logs == nil { + collector.Pipelines.Logs = make(map[string]*Pipeline) + } + if _, ok := collector.Pipelines.Logs[DefaultPipeline]; !ok { + collector.Pipelines.Logs[DefaultPipeline] = &Pipeline{ + Receivers: []string{"tcplog/nginx_app_protect"}, + Processors: []string{"logsgzip/default", "batch/default_logs"}, + Exporters: []string{"otlp/default"}, + } + } +} + +func addDefaultOtlpExporter(collector *Collector, config *Config) { + if collector.Exporters.OtlpExporters == nil { + collector.Exporters.OtlpExporters = make(map[string]*OtlpExporter) + } + + defaultCommandServer := config.Command + + if config.IsAuxiliaryCommandGrpcClientConfigured() { + defaultCommandServer = config.AuxiliaryCommand + } + + if _, ok := collector.Exporters.OtlpExporters[DefaultExporter]; !ok && defaultCommandServer != nil { + collector.Exporters.OtlpExporters[DefaultExporter] = &OtlpExporter{ + Server: defaultCommandServer.Server, + TLS: defaultCommandServer.TLS, + Compression: "", + } + + if defaultCommandServer.Auth != nil { + token := extractTokenFromAuth(defaultCommandServer.Auth) + if token != "" { + addAuthHeader(collector, token) + collector.Exporters.OtlpExporters[DefaultExporter].Authenticator = "headers_setter" + } + } + } +} + +func extractTokenFromAuth(auth *AuthConfig) string { + token := auth.Token + if auth.TokenPath != "" { + slog.Debug("Reading token from file", "path", auth.TokenPath) + tokenFromFile, err := file.ReadFromFile(auth.TokenPath) if err != nil { slog.Error("Error adding token to default collector, "+ "default collector configuration not started", "error", err) - return + return "" + } + token = tokenFromFile + } + + return token +} + +func addAuthHeader(collector *Collector, token string) { + header := []Header{ + { + Action: "insert", + Key: "authorization", + Value: token, + }, + } + + if collector.Extensions.HeadersSetter == nil { + collector.Extensions.HeadersSetter = &HeadersSetter{ + Headers: header, } - token = pathToken + } else { + // nolint: lll + collector.Extensions.HeadersSetter.Headers = append(collector.Extensions.HeadersSetter.Headers, header...) } +} + +func addDefaultProcessors(collector *Collector) { + if collector.Processors.Batch == nil { + collector.Processors.Batch = make(map[string]*Batch) + } + + if _, ok := collector.Processors.Batch[DefaultMetricsBatchProcessor]; !ok { + collector.Processors.Batch[DefaultMetricsBatchProcessor] = &Batch{ + SendBatchSize: DefCollectorMetricsBatchProcessorSendBatchSize, + SendBatchMaxSize: DefCollectorMetricsBatchProcessorSendBatchMaxSize, + Timeout: DefCollectorMetricsBatchProcessorTimeout, + } + } + if _, ok := collector.Processors.Batch[DefaultLogsBatchProcessor]; !ok { + collector.Processors.Batch[DefaultLogsBatchProcessor] = &Batch{ + SendBatchSize: DefCollectorLogsBatchProcessorSendBatchSize, + SendBatchMaxSize: DefCollectorLogsBatchProcessorSendBatchMaxSize, + Timeout: DefCollectorLogsBatchProcessorTimeout, + } + } + + if collector.Processors.LogsGzip == nil { + collector.Processors.LogsGzip = make(map[string]*LogsGzip) + } + if _, ok := collector.Processors.LogsGzip["default"]; !ok { + collector.Processors.LogsGzip["default"] = &LogsGzip{} + } +} +func addDefaultHostMetricsReceiver(collector *Collector) { if host.NewInfo().IsContainer() { + addDefaultContainerHostMetricsReceiver(collector) + } else { + addDefaultVMHostMetricsReceiver(collector) + } +} + +func addDefaultContainerHostMetricsReceiver(collector *Collector) { + if collector.Receivers.ContainerMetrics == nil { collector.Receivers.ContainerMetrics = &ContainerMetricsReceiver{ CollectionInterval: 1 * time.Minute, } + } + + if collector.Receivers.HostMetrics == nil { collector.Receivers.HostMetrics = &HostMetrics{ Scrapers: &HostMetricsScrapers{ Network: &NetworkScraper{}, @@ -173,11 +293,18 @@ func defaultCollector(collector *Collector, config *Config) { CollectionInterval: 1 * time.Minute, InitialDelay: 1 * time.Second, } + } + + if collector.Log == nil { collector.Log = &Log{ Path: "stdout", Level: "info", } - } else { + } +} + +func addDefaultVMHostMetricsReceiver(collector *Collector) { + if collector.Receivers.HostMetrics == nil { collector.Receivers.HostMetrics = &HostMetrics{ Scrapers: &HostMetricsScrapers{ CPU: &CPUScraper{}, @@ -190,24 +317,6 @@ func defaultCollector(collector *Collector, config *Config) { InitialDelay: 1 * time.Second, } } - - collector.Exporters.OtlpExporters = append(collector.Exporters.OtlpExporters, OtlpExporter{ - Server: config.Command.Server, - TLS: config.Command.TLS, - Compression: "", - Authenticator: "headers_setter", - }) - - header := []Header{ - { - Action: "insert", - Key: "authorization", - Value: token, - }, - } - collector.Extensions.HeadersSetter = &HeadersSetter{ - Headers: header, - } } func addLabelsAsOTelHeaders(collector *Collector, labels map[string]any) { @@ -547,24 +656,6 @@ func registerCollectorFlags(fs *flag.FlagSet) { "If the default path doesn't exist, log messages are output to stdout/stderr.", ) - fs.Uint32( - CollectorBatchProcessorSendBatchSizeKey, - DefCollectorBatchProcessorSendBatchSize, - `Number of metric data points after which a batch will be sent regardless of the timeout.`, - ) - - fs.Uint32( - CollectorBatchProcessorSendBatchMaxSizeKey, - DefCollectorBatchProcessorSendBatchMaxSize, - `The upper limit of the batch size.`, - ) - - fs.Duration( - CollectorBatchProcessorTimeoutKey, - DefCollectorBatchProcessorTimeout, - `Time duration after which a batch will be sent regardless of size.`, - ) - fs.String( CollectorExtensionsHealthServerHostKey, DefCollectorExtensionsHealthServerHost, @@ -837,6 +928,7 @@ func resolveCollector(allowedDirs []string) (*Collector, error) { Receivers: receivers, Extensions: resolveExtensions(), Log: resolveCollectorLog(), + Pipelines: resolvePipelines(), } // Check for self-signed certificate true in Agent conf @@ -852,8 +944,33 @@ func resolveCollector(allowedDirs []string) (*Collector, error) { return col, nil } +func resolvePipelines() Pipelines { + var metricsPipelines map[string]*Pipeline + + if viperInstance.IsSet(CollectorMetricsPipelinesKey) { + err := resolveMapStructure(CollectorMetricsPipelinesKey, &metricsPipelines) + if err != nil { + metricsPipelines = nil + } + } + + var logsPipelines map[string]*Pipeline + + if viperInstance.IsSet(CollectorLogsPipelinesKey) { + err := resolveMapStructure(CollectorLogsPipelinesKey, &logsPipelines) + if err != nil { + logsPipelines = nil + } + } + + return Pipelines{ + Metrics: metricsPipelines, + Logs: logsPipelines, + } +} + func resolveExporters() (Exporters, error) { - var otlpExporters []OtlpExporter + var otlpExporters map[string]*OtlpExporter exporters := Exporters{} if viperInstance.IsSet(CollectorDebugExporterKey) { @@ -896,17 +1013,10 @@ func isPrometheusExporterSet() bool { } func resolveProcessors() Processors { - processors := Processors{ - Batch: &Batch{ - SendBatchSize: viperInstance.GetUint32(CollectorBatchProcessorSendBatchSizeKey), - SendBatchMaxSize: viperInstance.GetUint32(CollectorBatchProcessorSendBatchMaxSizeKey), - Timeout: viperInstance.GetDuration(CollectorBatchProcessorTimeoutKey), - }, - LogsGzip: &LogsGzip{}, - } + processors := Processors{} - if viperInstance.IsSet(CollectorAttributeProcessorKey) { - err := resolveMapStructure(CollectorAttributeProcessorKey, &processors.Attribute) + if viperInstance.IsSet(CollectorProcessorsKey) { + err := resolveMapStructure(CollectorProcessorsKey, &processors) if err != nil { return processors } @@ -1184,8 +1294,3 @@ func resolveMapStructure(key string, object any) error { return nil } - -func isOTelExporterConfigured(collector *Collector) bool { - return len(collector.Exporters.OtlpExporters) == 0 && collector.Exporters.PrometheusExporter == nil && - collector.Exporters.Debug == nil -} diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 9048eb834..b8c8a11ce 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -192,11 +192,7 @@ func TestResolveCollector(t *testing.T) { viperInstance.Set(CollectorLogPathKey, expected.Log.Path) viperInstance.Set(CollectorLogLevelKey, expected.Log.Level) viperInstance.Set(CollectorReceiversKey, expected.Receivers) - viperInstance.Set(CollectorBatchProcessorKey, expected.Processors.Batch) - viperInstance.Set(CollectorBatchProcessorSendBatchSizeKey, expected.Processors.Batch.SendBatchSize) - viperInstance.Set(CollectorBatchProcessorSendBatchMaxSizeKey, expected.Processors.Batch.SendBatchMaxSize) - viperInstance.Set(CollectorBatchProcessorTimeoutKey, expected.Processors.Batch.Timeout) - viperInstance.Set(CollectorLogsGzipProcessorKey, expected.Processors.LogsGzip) + viperInstance.Set(CollectorProcessorsKey, expected.Processors) viperInstance.Set(CollectorExportersKey, expected.Exporters) viperInstance.Set(CollectorOtlpExportersKey, expected.Exporters.OtlpExporters) viperInstance.Set(CollectorExtensionsHealthServerHostKey, expected.Extensions.Health.Server.Host) @@ -766,6 +762,79 @@ func TestResolveExtensions_MultipleHeaders(t *testing.T) { } } +func TestAddDefaultOtlpExporter(t *testing.T) { + t.Run("Test 1: Command server only", func(t *testing.T) { + collector := &Collector{} + agentConfig := &Config{ + Command: &Command{ + Server: &ServerConfig{ + Host: "test.com", + Port: 8080, + Type: Grpc, + }, + Auth: &AuthConfig{ + Token: "token", + }, + TLS: &TLSConfig{ + SkipVerify: false, + }, + }, + } + + addDefaultOtlpExporter(collector, agentConfig) + + assert.Equal(t, "test.com", collector.Exporters.OtlpExporters["default"].Server.Host) + assert.Equal(t, 8080, collector.Exporters.OtlpExporters["default"].Server.Port) + assert.False(t, collector.Exporters.OtlpExporters["default"].TLS.SkipVerify) + assert.Equal(t, "headers_setter", collector.Exporters.OtlpExporters["default"].Authenticator) + assert.Equal(t, "insert", collector.Extensions.HeadersSetter.Headers[0].Action) + assert.Equal(t, "authorization", collector.Extensions.HeadersSetter.Headers[0].Key) + assert.Equal(t, "token", collector.Extensions.HeadersSetter.Headers[0].Value) + }) + + t.Run("Test 2: Command and Auxiliary Command servers", func(t *testing.T) { + collector := &Collector{} + agentConfig := &Config{ + Command: &Command{ + Server: &ServerConfig{ + Host: "test.com", + Port: 8080, + Type: Grpc, + }, + Auth: &AuthConfig{ + Token: "token", + }, + TLS: &TLSConfig{ + SkipVerify: false, + }, + }, + AuxiliaryCommand: &Command{ + Server: &ServerConfig{ + Host: "aux-test.com", + Port: 9090, + Type: Grpc, + }, + Auth: &AuthConfig{ + Token: "aux-token", + }, + TLS: &TLSConfig{ + SkipVerify: false, + }, + }, + } + + addDefaultOtlpExporter(collector, agentConfig) + + assert.Equal(t, "aux-test.com", collector.Exporters.OtlpExporters["default"].Server.Host) + assert.Equal(t, 9090, collector.Exporters.OtlpExporters["default"].Server.Port) + assert.False(t, collector.Exporters.OtlpExporters["default"].TLS.SkipVerify) + assert.Equal(t, "headers_setter", collector.Exporters.OtlpExporters["default"].Authenticator) + assert.Equal(t, "insert", collector.Extensions.HeadersSetter.Headers[0].Action) + assert.Equal(t, "authorization", collector.Extensions.HeadersSetter.Headers[0].Key) + assert.Equal(t, "aux-token", collector.Extensions.HeadersSetter.Headers[0].Value) + }) +} + func agentConfig() *Config { return &Config{ UUID: "", @@ -801,8 +870,8 @@ func agentConfig() *Config { Collector: &Collector{ ConfigPath: "/etc/nginx-agent/nginx-agent-otelcol.yaml", Exporters: Exporters{ - OtlpExporters: []OtlpExporter{ - { + OtlpExporters: map[string]*OtlpExporter{ + "default": { Server: &ServerConfig{ Host: "127.0.0.1", Port: 1234, @@ -819,16 +888,20 @@ func agentConfig() *Config { }, }, Processors: Processors{ - Batch: &Batch{ - SendBatchMaxSize: DefCollectorBatchProcessorSendBatchMaxSize, - SendBatchSize: DefCollectorBatchProcessorSendBatchSize, - Timeout: DefCollectorBatchProcessorTimeout, + Batch: map[string]*Batch{ + "default_logs": { + SendBatchMaxSize: DefCollectorLogsBatchProcessorSendBatchMaxSize, + SendBatchSize: DefCollectorLogsBatchProcessorSendBatchSize, + Timeout: DefCollectorLogsBatchProcessorTimeout, + }, + }, + LogsGzip: map[string]*LogsGzip{ + "default": {}, }, - LogsGzip: &LogsGzip{}, }, Receivers: Receivers{ - OtlpReceivers: []OtlpReceiver{ - { + OtlpReceivers: map[string]*OtlpReceiver{ + "default": { Server: &ServerConfig{ Host: "localhost", Port: 4317, @@ -942,8 +1015,8 @@ func createConfig() *Config { Collector: &Collector{ ConfigPath: "/etc/nginx-agent/nginx-agent-otelcol.yaml", Exporters: Exporters{ - OtlpExporters: []OtlpExporter{ - { + OtlpExporters: map[string]*OtlpExporter{ + "default": { Server: &ServerConfig{ Host: "127.0.0.1", Port: 5643, @@ -974,25 +1047,41 @@ func createConfig() *Config { Debug: &DebugExporter{}, }, Processors: Processors{ - Batch: &Batch{ - SendBatchMaxSize: 1, - SendBatchSize: 8199, - Timeout: 30 * time.Second, + Batch: map[string]*Batch{ + "default": { + SendBatchMaxSize: 1, + SendBatchSize: 8199, + Timeout: 30 * time.Second, + }, + "default_metrics": { + SendBatchMaxSize: 1000, + SendBatchSize: 1000, + Timeout: 30 * time.Second, + }, + "default_logs": { + SendBatchMaxSize: 100, + SendBatchSize: 100, + Timeout: 60 * time.Second, + }, }, - Attribute: &Attribute{ - Actions: []Action{ - { - Key: "test", - Action: "insert", - Value: "value", + Attribute: map[string]*Attribute{ + "default": { + Actions: []Action{ + { + Key: "test", + Action: "insert", + Value: "value", + }, }, }, }, - LogsGzip: &LogsGzip{}, + LogsGzip: map[string]*LogsGzip{ + "default": {}, + }, }, Receivers: Receivers{ - OtlpReceivers: []OtlpReceiver{ - { + OtlpReceivers: map[string]*OtlpReceiver{ + "default": { Server: &ServerConfig{ Host: "127.0.0.1", Port: 4317, @@ -1010,26 +1099,6 @@ func createConfig() *Config { }, }, }, - NginxReceivers: []NginxReceiver{ - { - InstanceID: "cd7b8911-c2c5-4daf-b311-dbead151d938", - AccessLogs: []AccessLog{ - { - LogFormat: "$remote_addr - $remote_user [$time_local] \"$request\"" + - " $status $body_bytes_sent \"$http_referer\" \"$http_user_agent\" " + - "\"$http_x_forwarded_for\"", - FilePath: "/var/log/nginx/access-custom.conf", - }, - }, - CollectionInterval: 30 * time.Second, - }, - }, - NginxPlusReceivers: []NginxPlusReceiver{ - { - InstanceID: "cd7b8911-c2c5-4daf-b311-dbead151d939", - CollectionInterval: 30 * time.Second, - }, - }, HostMetrics: &HostMetrics{ CollectionInterval: 10 * time.Second, InitialDelay: 2 * time.Second, @@ -1081,6 +1150,22 @@ func createConfig() *Config { Level: "INFO", Path: "/var/log/nginx-agent/opentelemetry-collector-agent.log", }, + Pipelines: Pipelines{ + Metrics: map[string]*Pipeline{ + "default": { + Receivers: []string{"host_metrics", "nginx_metrics"}, + Processors: []string{"batch/default_metrics"}, + Exporters: []string{"otlp/default"}, + }, + }, + Logs: map[string]*Pipeline{ + "default": { + Receivers: []string{"tcplog/nginx_app_protect"}, + Processors: []string{"logsgzip/default", "batch/default_logs"}, + Exporters: []string{"otlp/default"}, + }, + }, + }, }, Command: &Command{ Server: &ServerConfig{ diff --git a/internal/config/defaults.go b/internal/config/defaults.go index 40b0767a1..c52ebbd5f 100644 --- a/internal/config/defaults.go +++ b/internal/config/defaults.go @@ -73,9 +73,12 @@ const ( DefCollectorTLSCAPath = "/var/lib/nginx-agent/ca.pem" DefCollectorTLSSANNames = "127.0.0.1,::1,localhost" - DefCollectorBatchProcessorSendBatchSize = 1000 - DefCollectorBatchProcessorSendBatchMaxSize = 1000 - DefCollectorBatchProcessorTimeout = 30 * time.Second + DefCollectorMetricsBatchProcessorSendBatchSize = 1000 + DefCollectorMetricsBatchProcessorSendBatchMaxSize = 1000 + DefCollectorMetricsBatchProcessorTimeout = 30 * time.Second + DefCollectorLogsBatchProcessorSendBatchSize = 100 + DefCollectorLogsBatchProcessorSendBatchMaxSize = 100 + DefCollectorLogsBatchProcessorTimeout = 60 * time.Second DefCollectorExtensionsHealthServerHost = "localhost" DefCollectorExtensionsHealthServerPort = 13133 diff --git a/internal/config/flags.go b/internal/config/flags.go index 58a2d8454..189046907 100644 --- a/internal/config/flags.go +++ b/internal/config/flags.go @@ -49,7 +49,6 @@ var ( CollectorConfigPathKey = pre(CollectorRootKey) + "config_path" CollectorExportersKey = pre(CollectorRootKey) + "exporters" - CollectorAttributeProcessorKey = pre(CollectorProcessorsKey) + "attribute" CollectorDebugExporterKey = pre(CollectorExportersKey) + "debug" CollectorPrometheusExporterKey = pre(CollectorExportersKey) + "prometheus" CollectorPrometheusExporterServerHostKey = pre(CollectorPrometheusExporterKey) + "server_host" @@ -62,11 +61,6 @@ var ( CollectorPrometheusExporterTLSServerNameKey = pre(CollectorPrometheusExporterTLSKey) + "server_name" CollectorOtlpExportersKey = pre(CollectorExportersKey) + "otlp" CollectorProcessorsKey = pre(CollectorRootKey) + "processors" - CollectorBatchProcessorKey = pre(CollectorProcessorsKey) + "batch" - CollectorBatchProcessorSendBatchSizeKey = pre(CollectorBatchProcessorKey) + "send_batch_size" - CollectorBatchProcessorSendBatchMaxSizeKey = pre(CollectorBatchProcessorKey) + "send_batch_max_size" - CollectorBatchProcessorTimeoutKey = pre(CollectorBatchProcessorKey) + "timeout" - CollectorLogsGzipProcessorKey = pre(CollectorProcessorsKey) + "logsgzip" CollectorExtensionsKey = pre(CollectorRootKey) + "extensions" CollectorExtensionsHealthKey = pre(CollectorExtensionsKey) + "health" CollectorExtensionsHealthServerHostKey = pre(CollectorExtensionsHealthKey) + "server_host" @@ -79,6 +73,9 @@ var ( CollectorExtensionsHealthTLSServerNameKey = pre(CollectorExtensionsHealthTLSKey) + "server_name" CollectorExtensionsHealthTLSSkipVerifyKey = pre(CollectorExtensionsHealthTLSKey) + "skip_verify" CollectorExtensionsHeadersSetterKey = pre(CollectorExtensionsKey) + "headers_setter" + CollectorPipelinesKey = pre(CollectorRootKey) + "pipelines" + CollectorMetricsPipelinesKey = pre(CollectorPipelinesKey) + "metrics" + CollectorLogsPipelinesKey = pre(CollectorPipelinesKey) + "logs" CollectorReceiversKey = pre(CollectorRootKey) + "receivers" CollectorLogKey = pre(CollectorRootKey) + "log" CollectorLogLevelKey = pre(CollectorLogKey) + "level" diff --git a/internal/config/testdata/nginx-agent.conf b/internal/config/testdata/nginx-agent.conf index 0b4601a55..1aeb11221 100644 --- a/internal/config/testdata/nginx-agent.conf +++ b/internal/config/testdata/nginx-agent.conf @@ -90,46 +90,42 @@ collector: config_path: "/etc/nginx-agent/nginx-agent-otelcol.yaml" receivers: otlp: - - server: - host: "127.0.0.1" - port: 4317 - auth: - token: "secret-receiver-token" - tls: - generate_self_signed_cert: false - server_name: "test-local-server" - skip_verify: true - ca: /tmp/ca.pem - cert: /tmp/cert.pem - key: /tmp/key.pem - nginx: - - instance_id: cd7b8911-c2c5-4daf-b311-dbead151d938 - access_logs: - - file_path: "/var/log/nginx/access-custom.conf" - log_format: "$remote_addr - $remote_user [$time_local] \"$request\" $status $body_bytes_sent \"$http_referer\" \"$http_user_agent\" \"$http_x_forwarded_for\"" - collection_interval: 30s - nginx_plus: - - instance_id: cd7b8911-c2c5-4daf-b311-dbead151d939 - collection_interval: 30s + "default": + server: + host: "127.0.0.1" + port: 4317 + auth: + token: "secret-receiver-token" + tls: + generate_self_signed_cert: false + server_name: "test-local-server" + skip_verify: true + ca: /tmp/ca.pem + cert: /tmp/cert.pem + key: /tmp/key.pem host_metrics: - collection_interval: 10s - initial_delay: 2s - scrapers: - cpu: {} + collection_interval: 10s + initial_delay: 2s + scrapers: + cpu: {} processors: - batch: + batch: + "default": send_batch_max_size: 1 send_batch_size: 8199 timeout: 30s attribute: - actions: - - key: "test" - action: "insert" - value: "value" - logsgzip: {} + "default": + actions: + - key: "test" + action: "insert" + value: "value" + logsgzip: + "default": {} exporters: otlp: - - server: + "default": + server: host: "127.0.0.1" port: 5643 authenticator: "test-saas-token" @@ -140,22 +136,22 @@ collector: key: /path/to/server-key.pem ca: /path/to/server-cert.pem prometheus: - server: - host: "127.0.0.1" - port: 1235 - tls: - server_name: "test-server" - skip_verify: false - cert: /path/to/server-cert.pem - key: /path/to/server-key.pem - ca: /path/to/server-cert.pem + server: + host: "127.0.0.1" + port: 1235 + tls: + server_name: "test-server" + skip_verify: false + cert: /path/to/server-cert.pem + key: /path/to/server-key.pem + ca: /path/to/server-cert.pem debug: {} extensions: headers_setter: - headers: - - action: "action" - key: "key" - value: "value" + headers: + - action: "action" + key: "key" + value: "value" health: server: host: "127.0.0.1" @@ -167,6 +163,6 @@ collector: cert: /path/to/server-cert.pem key: /path/to/server-key.pem ca: /path/to/server-ca.pem - log: + log: level: "INFO" path: "/var/log/nginx-agent/opentelemetry-collector-agent.log" diff --git a/internal/config/types.go b/internal/config/types.go index ae0558045..b6ccebc61 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -107,13 +107,25 @@ type ( Exporters Exporters `yaml:"exporters" mapstructure:"exporters"` Extensions Extensions `yaml:"extensions" mapstructure:"extensions"` Processors Processors `yaml:"processors" mapstructure:"processors"` + Pipelines Pipelines `yaml:"pipelines" mapstructure:"pipelines"` Receivers Receivers `yaml:"receivers" mapstructure:"receivers"` } + Pipelines struct { + Metrics map[string]*Pipeline `yaml:"metrics" mapstructure:"metrics"` + Logs map[string]*Pipeline `yaml:"logs" mapstructure:"logs"` + } + + Pipeline struct { + Receivers []string `yaml:"receivers" mapstructure:"receivers"` + Processors []string `yaml:"processors" mapstructure:"processors"` + Exporters []string `yaml:"exporters" mapstructure:"exporters"` + } + Exporters struct { - Debug *DebugExporter `yaml:"debug" mapstructure:"debug"` - PrometheusExporter *PrometheusExporter `yaml:"prometheus" mapstructure:"prometheus"` - OtlpExporters []OtlpExporter `yaml:"otlp" mapstructure:"otlp"` + Debug *DebugExporter `yaml:"debug" mapstructure:"debug"` + PrometheusExporter *PrometheusExporter `yaml:"prometheus" mapstructure:"prometheus"` + OtlpExporters map[string]*OtlpExporter `yaml:"otlp" mapstructure:"otlp"` } OtlpExporter struct { @@ -156,10 +168,10 @@ type ( // OTel Collector Processors configuration. Processors struct { - Attribute *Attribute `yaml:"attribute" mapstructure:"attribute"` - Resource *Resource `yaml:"resource" mapstructure:"resource"` - Batch *Batch `yaml:"batch" mapstructure:"batch"` - LogsGzip *LogsGzip `yaml:"logsgzip" mapstructure:"logsgzip"` + Attribute map[string]*Attribute `yaml:"attribute" mapstructure:"attribute"` + Resource map[string]*Resource `yaml:"resource" mapstructure:"resource"` + Batch map[string]*Batch `yaml:"batch" mapstructure:"batch"` + LogsGzip map[string]*LogsGzip `yaml:"logsgzip" mapstructure:"logsgzip"` } Attribute struct { @@ -192,12 +204,12 @@ type ( // OTel Collector Receiver configuration. Receivers struct { - ContainerMetrics *ContainerMetricsReceiver `yaml:"container_metrics" mapstructure:"container_metrics"` - HostMetrics *HostMetrics `yaml:"host_metrics" mapstructure:"host_metrics"` - OtlpReceivers []OtlpReceiver `yaml:"otlp" mapstructure:"otlp"` - NginxReceivers []NginxReceiver `yaml:"nginx" mapstructure:"nginx"` - NginxPlusReceivers []NginxPlusReceiver `yaml:"nginx_plus" mapstructure:"nginx_plus"` - TcplogReceivers []TcplogReceiver `yaml:"tcplog" mapstructure:"tcplog"` + ContainerMetrics *ContainerMetricsReceiver `yaml:"container_metrics" mapstructure:"container_metrics"` + HostMetrics *HostMetrics `yaml:"host_metrics" mapstructure:"host_metrics"` + OtlpReceivers map[string]*OtlpReceiver `yaml:"otlp" mapstructure:"otlp"` + TcplogReceivers map[string]*TcplogReceiver `yaml:"tcplog" mapstructure:"tcplog"` + NginxReceivers []NginxReceiver `yaml:"-"` + NginxPlusReceivers []NginxPlusReceiver `yaml:"-"` } OtlpReceiver struct { @@ -379,24 +391,6 @@ func (c *Config) IsAuxiliaryCommandGrpcClientConfigured() bool { c.AuxiliaryCommand.Server.Type == Grpc } -func (c *Config) IsCommandAuthConfigured() bool { - return c.Command.Auth != nil && - (c.Command.Auth.Token != "" || c.Command.Auth.TokenPath != "") -} - -func (c *Config) IsAuxiliaryCommandAuthConfigured() bool { - return c.AuxiliaryCommand.Auth != nil && - (c.AuxiliaryCommand.Auth.Token != "" || c.AuxiliaryCommand.Auth.TokenPath != "") -} - -func (c *Config) IsCommandTLSConfigured() bool { - return c.Command.TLS != nil -} - -func (c *Config) IsAuxiliaryCommandTLSConfigured() bool { - return c.AuxiliaryCommand.TLS != nil -} - func (c *Config) IsFeatureEnabled(feature string) bool { for _, enabledFeature := range c.Features { if enabledFeature == feature { diff --git a/internal/plugin/plugin_manager.go b/internal/plugin/plugin_manager.go index 3f2c3869a..fde011d8b 100644 --- a/internal/plugin/plugin_manager.go +++ b/internal/plugin/plugin_manager.go @@ -96,7 +96,7 @@ func addCollectorPlugin(ctx context.Context, agentConfig *config.Config, plugins return plugins } if agentConfig.IsACollectorExporterConfigured() { - oTelCollector, err := collector.New(agentConfig) + oTelCollector, err := collector.NewCollector(agentConfig) if err == nil { plugins = append(plugins, oTelCollector) } else { diff --git a/test/config/agent/nginx-agent-otel-load.conf b/test/config/agent/nginx-agent-otel-load.conf index 2ee5e8651..4c7590bf3 100644 --- a/test/config/agent/nginx-agent-otel-load.conf +++ b/test/config/agent/nginx-agent-otel-load.conf @@ -20,21 +20,30 @@ allowed_directories: collector: receivers: otlp: - - server: + "default": + server: host: "127.0.0.1" port: 4317 processors: batch: - send_batch_size: 8192 - timeout: 200ms - send_batch_max_size: 0 + "default": + send_batch_size: 8192 + timeout: 200ms + send_batch_max_size: 0 exporters: otlp: - - server: - host: "127.0.0.1" - port: 5643 + "default": + server: + host: "127.0.0.1" + port: 5643 extensions: health: server: host: "127.0.0.1" port: 1337 + pipelines: + metrics: + "default": + receivers: ["otlp/default"] + processors: ["batch/default"] + exporters: ["otlp/default"] diff --git a/test/config/collector/test-opentelemetry-collector-agent.yaml b/test/config/collector/test-opentelemetry-collector-agent.yaml index 9a17adc3f..ed9acec91 100644 --- a/test/config/collector/test-opentelemetry-collector-agent.yaml +++ b/test/config/collector/test-opentelemetry-collector-agent.yaml @@ -18,7 +18,7 @@ receivers: system.memory.limit: enabled: true network: - otlp/0: + otlp/default: protocols: grpc: endpoint: "localhost:4317" @@ -35,7 +35,7 @@ receivers: access_logs: - log_format: "$remote_addr - $remote_user [$time_local] \"$request\" $status $body_bytes_sent \"$http_referer\" \"$http_user_agent\" \"$http_x_forwarded_for\"\"$upstream_cache_status\"" file_path: "/var/log/nginx/access-custom.conf" - tcplog/0: + tcplog/default: listen_address: "localhost:151" operators: - type: add @@ -45,18 +45,18 @@ receivers: field: attributes.message processors: - resource: + resource/default: attributes: - key: resource.id action: add value: 12345 - batch: + batch/default: send_batch_size: 1000 timeout: 30s send_batch_max_size: 1000 exporters: - otlp/0: + otlp/default: endpoint: "127.0.0.1:1234" compression: none timeout: 10s @@ -100,26 +100,27 @@ service: extensions: - health_check - headers_setter + pipelines: - metrics: + metrics/default: receivers: - - containermetrics - hostmetrics - - otlp/0 + - containermetrics + - otlp/default - nginx/123 processors: - - resource - - batch + - resource/default + - batch/default exporters: - - otlp/0 + - otlp/default - prometheus - debug - logs: + logs/default: receivers: - - tcplog/0 + - tcplog/default processors: - - resource - - batch + - resource/default + - batch/default exporters: - - otlp/0 + - otlp/default - debug diff --git a/test/types/config.go b/test/types/config.go index 29775f664..04dec87e5 100644 --- a/test/types/config.go +++ b/test/types/config.go @@ -65,8 +65,8 @@ func AgentConfig() *config.Config { Collector: &config.Collector{ ConfigPath: "/etc/nginx-agent/nginx-agent-otelcol.yaml", Exporters: config.Exporters{ - OtlpExporters: []config.OtlpExporter{ - { + OtlpExporters: map[string]*config.OtlpExporter{ + "default": { Server: &config.ServerConfig{ Host: "127.0.0.1", Port: 0, @@ -76,15 +76,17 @@ func AgentConfig() *config.Config { }, }, Processors: config.Processors{ - Batch: &config.Batch{ - SendBatchSize: config.DefCollectorBatchProcessorSendBatchSize, - SendBatchMaxSize: config.DefCollectorBatchProcessorSendBatchMaxSize, - Timeout: config.DefCollectorBatchProcessorTimeout, + Batch: map[string]*config.Batch{ + "default": { + SendBatchSize: config.DefCollectorMetricsBatchProcessorSendBatchMaxSize, + SendBatchMaxSize: config.DefCollectorMetricsBatchProcessorSendBatchMaxSize, + Timeout: config.DefCollectorMetricsBatchProcessorTimeout, + }, }, }, Receivers: config.Receivers{ - OtlpReceivers: []config.OtlpReceiver{ - { + OtlpReceivers: map[string]*config.OtlpReceiver{ + "default": { Server: &config.ServerConfig{ Host: "127.0.0.1", Port: 0, @@ -128,6 +130,15 @@ func AgentConfig() *config.Config { Level: "INFO", Path: "/var/log/nginx-agent/opentelemetry-collector-agent.log", }, + Pipelines: config.Pipelines{ + Metrics: map[string]*config.Pipeline{ + "default": { + Receivers: []string{"host_metrics"}, + Processors: []string{"batch/default"}, + Exporters: []string{"otlp/default"}, + }, + }, + }, }, Command: &config.Command{ Server: &config.ServerConfig{ @@ -179,11 +190,11 @@ func OTelConfig(t *testing.T) *config.Config { exporterPort, expErr := helpers.RandomPort(t) require.NoError(t, expErr) - ac.Collector.Exporters.OtlpExporters[0].Server.Port = exporterPort + ac.Collector.Exporters.OtlpExporters["default"].Server.Port = exporterPort receiverPort, recErr := helpers.RandomPort(t) require.NoError(t, recErr) - ac.Collector.Receivers.OtlpReceivers[0].Server.Port = receiverPort + ac.Collector.Receivers.OtlpReceivers["default"].Server.Port = receiverPort healthPort, healthErr := helpers.RandomPort(t) require.NoError(t, healthErr) From 13ea749e18fdaa0489537c80367c0cf71dca82db Mon Sep 17 00:00:00 2001 From: aphralG <108004222+aphralG@users.noreply.github.com> Date: Tue, 22 Jul 2025 17:05:12 +0100 Subject: [PATCH 12/13] Fix failing unit test (#1176) --- internal/collector/otel_collector_plugin_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/internal/collector/otel_collector_plugin_test.go b/internal/collector/otel_collector_plugin_test.go index 2fa51cef4..66a58c36b 100644 --- a/internal/collector/otel_collector_plugin_test.go +++ b/internal/collector/otel_collector_plugin_test.go @@ -173,8 +173,9 @@ func TestCollector_ProcessNginxConfigUpdateTopic(t *testing.T) { }, }, receivers: config.Receivers{ - HostMetrics: nil, - OtlpReceivers: nil, + HostMetrics: nil, + OtlpReceivers: nil, + TcplogReceivers: make(map[string]*config.TcplogReceiver), NginxPlusReceivers: []config.NginxPlusReceiver{ { InstanceID: "123", @@ -213,8 +214,9 @@ func TestCollector_ProcessNginxConfigUpdateTopic(t *testing.T) { }, }, receivers: config.Receivers{ - HostMetrics: nil, - OtlpReceivers: nil, + HostMetrics: nil, + OtlpReceivers: nil, + TcplogReceivers: make(map[string]*config.TcplogReceiver), NginxReceivers: []config.NginxReceiver{ { InstanceID: "123", From d6325fef1b2fd8ffab86b60f5a1169759aded800 Mon Sep 17 00:00:00 2001 From: aphralG <108004222+aphralG@users.noreply.github.com> Date: Wed, 23 Jul 2025 09:38:34 +0100 Subject: [PATCH 13/13] Filter NAP logs by Severity (#1169) --- internal/collector/otel_collector_plugin.go | 18 ++++++++++++++++++ .../collector/otel_collector_plugin_test.go | 6 +++--- internal/collector/otelcol.tmpl | 2 +- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/internal/collector/otel_collector_plugin.go b/internal/collector/otel_collector_plugin.go index d6d452a41..953f1838d 100644 --- a/internal/collector/otel_collector_plugin.go +++ b/internal/collector/otel_collector_plugin.go @@ -554,6 +554,24 @@ func (oc *Collector) updateNginxAppProtectTcplogReceivers(nginxConfigContext *mo oc.config.Collector.Receivers.TcplogReceivers["nginx_app_protect"] = &config.TcplogReceiver{ ListenAddress: nginxConfigContext.NAPSysLogServer, Operators: []config.Operator{ + // regex captures the priority number from the log line + { + Type: "regex_parser", + Fields: map[string]string{ + "regex": "^<(?P\\d+)>", + "parse_from": "body", + "parse_to": "attributes", + }, + }, + // filter drops all logs that have a severity above 4 + // https://docs.secureauth.com/0902/en/how-to-read-a-syslog-message.html#severity-code-table + { + Type: "filter", + Fields: map[string]string{ + "expr": "'int(attributes.priority) % 8 > 4'", + "drop_ratio": "1.0", + }, + }, { Type: "add", Fields: map[string]string{ diff --git a/internal/collector/otel_collector_plugin_test.go b/internal/collector/otel_collector_plugin_test.go index 66a58c36b..5623d2eef 100644 --- a/internal/collector/otel_collector_plugin_test.go +++ b/internal/collector/otel_collector_plugin_test.go @@ -749,7 +749,7 @@ func TestCollector_updateNginxAppProtectTcplogReceivers(t *testing.T) { assert.True(tt, tcplogReceiverAdded) assert.Len(tt, conf.Collector.Receivers.TcplogReceivers, 1) assert.Equal(tt, "localhost:151", conf.Collector.Receivers.TcplogReceivers["nginx_app_protect"].ListenAddress) - assert.Len(tt, conf.Collector.Receivers.TcplogReceivers["nginx_app_protect"].Operators, 4) + assert.Len(tt, conf.Collector.Receivers.TcplogReceivers["nginx_app_protect"].Operators, 6) }) // Calling updateNginxAppProtectTcplogReceivers shouldn't update the TcplogReceivers slice @@ -759,7 +759,7 @@ func TestCollector_updateNginxAppProtectTcplogReceivers(t *testing.T) { assert.False(t, tcplogReceiverAdded) assert.Len(t, conf.Collector.Receivers.TcplogReceivers, 1) assert.Equal(t, "localhost:151", conf.Collector.Receivers.TcplogReceivers["nginx_app_protect"].ListenAddress) - assert.Len(t, conf.Collector.Receivers.TcplogReceivers["nginx_app_protect"].Operators, 4) + assert.Len(t, conf.Collector.Receivers.TcplogReceivers["nginx_app_protect"].Operators, 6) }) t.Run("Test 3: TcplogReceiver deleted", func(tt *testing.T) { @@ -778,7 +778,7 @@ func TestCollector_updateNginxAppProtectTcplogReceivers(t *testing.T) { assert.True(t, tcplogReceiverDeleted) assert.Len(t, conf.Collector.Receivers.TcplogReceivers, 1) assert.Equal(t, "localhost:152", conf.Collector.Receivers.TcplogReceivers["nginx_app_protect"].ListenAddress) - assert.Len(t, conf.Collector.Receivers.TcplogReceivers["nginx_app_protect"].Operators, 4) + assert.Len(t, conf.Collector.Receivers.TcplogReceivers["nginx_app_protect"].Operators, 6) }) } diff --git a/internal/collector/otelcol.tmpl b/internal/collector/otelcol.tmpl index b331152c0..572e8c222 100644 --- a/internal/collector/otelcol.tmpl +++ b/internal/collector/otelcol.tmpl @@ -296,7 +296,7 @@ service: receivers: {{- range $receiver := $pipeline.Receivers }} {{- if eq $receiver "tcplog/nginx_app_protect" }} - - tcplog/nginx_app_protect: + - tcplog/nginx_app_protect {{- else }} - {{ $receiver }} {{- end }}