From a94d816e8d23d0c015ff2d6ff73434d26ac34778 Mon Sep 17 00:00:00 2001 From: Donal Hurley Date: Wed, 4 Oct 2023 13:32:05 +0100 Subject: [PATCH] Update NGINX plugin to read NGINX config on startup (#489) --- README.md | 5 +++- examples/grafana-metrics/nginx-agent.conf | 17 +----------- hugo/content/getting-started.md | 6 +++-- main.go | 3 +-- src/core/pipe.go | 4 +-- src/plugins/nginx.go | 21 ++++++++++----- src/plugins/nginx_test.go | 27 +++++++------------ test/integration/api/api_test.go | 4 +++ test/integration/api/nginx-agent.conf | 2 +- .../nginx/agent/v2/src/core/pipe.go | 4 +-- .../nginx/agent/v2/src/core/pipe.go | 4 +-- .../nginx/agent/v2/src/plugins/nginx.go | 21 ++++++++++----- 12 files changed, 58 insertions(+), 60 deletions(-) diff --git a/README.md b/README.md index cb8fa7e64b..08ed268bbf 100644 --- a/README.md +++ b/README.md @@ -214,7 +214,10 @@ The NGINX Agent REST interface can be exposed by validating the following lines ```yaml api: - port: 8081 # port to expose REST API + # Set API address to allow remote management + host: 127.0.0.1 + # Set this value to a secure port number to prevent information leaks + port: 8038 # REST TLS parameters cert: ".crt" diff --git a/examples/grafana-metrics/nginx-agent.conf b/examples/grafana-metrics/nginx-agent.conf index d310f1866e..85b8a7313d 100644 --- a/examples/grafana-metrics/nginx-agent.conf +++ b/examples/grafana-metrics/nginx-agent.conf @@ -9,23 +9,8 @@ api: # port to expose http api + host: 0.0.0.0 port: 8081 -# tls options -tls: - # enable tls in the nginx-agent setup for grpcs - # default to enable to connect with tls connection but without client cert for mtls - enable: true - # specify the absolute path to the CA certificate file to use for verifying - # the server certificate (also requires 'skip_verify: false' below) - # by default, this will be the trusted root CAs found in the OS CA store - # ca: /etc/nginx-agent/ca.pem - # specify the absolute path to the client cert, when mtls is enabled - # cert: /etc/nginx-agent/client.crt - # specify the absolute path to the client cert key, when mtls is enabled - # key: /etc/nginx-agent/client.key - # controls whether the server certificate chain and host name are verified. - # for production use, see instructions for configuring TLS - skip_verify: true log: # set log level (panic, fatal, error, info, debug, trace; default "info") level: info diff --git a/hugo/content/getting-started.md b/hugo/content/getting-started.md index 0b1f0a9947..72277a3a29 100644 --- a/hugo/content/getting-started.md +++ b/hugo/content/getting-started.md @@ -74,8 +74,10 @@ The NGINX Agent REST interface can be exposed by validating the following lines ```yaml api: - port: 8081 # port to expose REST API - + # Set API address to allow remote management + host: 127.0.0.1 + # Set this value to a secure port number to prevent information leaks + port: 8038 # REST TLS parameters cert: ".crt" key: ".key" diff --git a/main.go b/main.go index de3e1fb842..31e245ee07 100644 --- a/main.go +++ b/main.go @@ -86,8 +86,7 @@ func main() { corePlugins, extensionPlugins := plugins.LoadPlugins(commander, binary, env, reporter, loadedConfig, eventMeta) pipe := core.InitializePipe(ctx, corePlugins, extensionPlugins, agent_config.DefaultPluginSize) - - defer pipe.Process(core.NewMessage(core.AgentStarted, eventMeta)) + pipe.Process(core.NewMessage(core.AgentStarted, eventMeta)) core.HandleSignals(ctx, commander, loadedConfig, env, pipe, cancel, controller) pipe.Run() diff --git a/src/core/pipe.go b/src/core/pipe.go index 657f572837..1ded89e02c 100644 --- a/src/core/pipe.go +++ b/src/core/pipe.go @@ -194,11 +194,11 @@ func (p *MessagePipe) GetExtensionPlugins() []ExtensionPlugin { func (p *MessagePipe) initPlugins() { for _, r := range p.plugins { - go r.Init(p) + r.Init(p) } for _, r := range p.extensionPlugins { - go r.Init(p) + r.Init(p) } } diff --git a/src/plugins/nginx.go b/src/plugins/nginx.go index 1937ca8dce..5036c71237 100644 --- a/src/plugins/nginx.go +++ b/src/plugins/nginx.go @@ -91,7 +91,6 @@ type NginxConfigValidationResponse struct { func NewNginx(cmdr client.Commander, nginxBinary core.NginxBinary, env core.Environment, loadedConfig *config.Config, processes []*core.Process) *Nginx { isFeatureNginxConfigEnabled := loadedConfig.IsFeatureEnabled(agent_config.FeatureNginxConfig) || loadedConfig.IsFeatureEnabled(agent_config.FeatureNginxConfigAsync) - isNginxAppProtectEnabled := loadedConfig.IsExtensionEnabled(agent_config.NginxAppProtectExtensionPlugin) return &Nginx{ @@ -112,17 +111,25 @@ func (n *Nginx) Init(pipeline core.MessagePipeInterface) { log.Info("NginxBinary initializing") n.messagePipeline = pipeline n.nginxBinary.UpdateNginxDetailsFromProcesses(n.getNginxProccessInfo()) + nginxDetails := n.nginxBinary.GetNginxDetailsMapFromProcesses(n.getNginxProccessInfo()) + + for _, nginxDetail := range nginxDetails { + log.Infof("Reading config in directory %v for nginx instance %v", nginxDetail.GetConfPath(), nginxDetail.GetNginxId()) + _, err := n.nginxBinary.ReadConfig(nginxDetail.GetConfPath(), nginxDetail.GetNginxId(), n.env.GetSystemUUID()) + if err != nil { + log.Errorf("Unable to read nginx config %s: %v", nginxDetail.GetConfPath(), err) + } + } + + n.messagePipeline.Process( + core.NewMessage(core.NginxPluginConfigured, n), + core.NewMessage(core.NginxInstancesFound, nginxDetails), + ) } // Process processes the messages from the messaging pipe func (n *Nginx) Process(message *core.Message) { switch message.Topic() { - case core.AgentStarted: - nginxDetails := n.nginxBinary.GetNginxDetailsMapFromProcesses(n.getNginxProccessInfo()) - n.messagePipeline.Process( - core.NewMessage(core.NginxPluginConfigured, n), - core.NewMessage(core.NginxInstancesFound, nginxDetails), - ) case core.CommNginxConfig: switch cmd := message.Data().(type) { case *proto.Command: diff --git a/src/plugins/nginx_test.go b/src/plugins/nginx_test.go index 7cc9fb7dac..e0768646d0 100644 --- a/src/plugins/nginx_test.go +++ b/src/plugins/nginx_test.go @@ -614,7 +614,6 @@ func TestUploadConfigs(t *testing.T) { } msgTopics := []string{ - core.AgentStarted, core.NginxPluginConfigured, core.NginxInstancesFound, core.DataplaneChanged, @@ -637,14 +636,9 @@ func TestUploadConfigs(t *testing.T) { messagePipe := core.SetupMockMessagePipe(t, context.TODO(), []core.Plugin{pluginUnderTest}, []core.ExtensionPlugin{}) pluginUnderTest.Init(messagePipe) - - // calling Run x 2 means AgentStarted finishes before the DataplaneChanged event gets processed. - // This is the expected order of the real MessagePipe - messagePipe.Process(core.NewMessage(core.AgentStarted, nil)) - messagePipe.Run() - + messagePipe.RunWithoutInit() messagePipe.Process(core.NewMessage(core.DataplaneChanged, nil)) - messagePipe.Run() + messagePipe.RunWithoutInit() binary.AssertExpectations(t) cmdr.AssertExpectations(t) @@ -654,7 +648,6 @@ func TestUploadConfigs(t *testing.T) { func TestDisableUploadConfigs(t *testing.T) { msgTopics := []string{ - core.AgentStarted, core.NginxPluginConfigured, core.NginxInstancesFound, core.DataplaneChanged, @@ -665,6 +658,7 @@ func TestDisableUploadConfigs(t *testing.T) { binary := tutils.NewMockNginxBinary() binary.On("UpdateNginxDetailsFromProcesses", mock.Anything) binary.On("GetNginxDetailsMapFromProcesses", mock.Anything).Return(tutils.GetDetailsMap()) + binary.On("ReadConfig", mock.Anything, mock.Anything, mock.Anything).Return(&proto.NginxConfig{}, nil) cmdr := tutils.NewMockCommandClient() @@ -672,13 +666,10 @@ func TestDisableUploadConfigs(t *testing.T) { messagePipe := core.SetupMockMessagePipe(t, context.TODO(), []core.Plugin{pluginUnderTest}, []core.ExtensionPlugin{}) pluginUnderTest.Init(messagePipe) - // calling Run x 2 means AgentStarted finishes before the DataplaneChanged event gets processed. - // This is the expected order of the real MessagePipe - messagePipe.Process(core.NewMessage(core.AgentStarted, nil)) - messagePipe.Run() + messagePipe.RunWithoutInit() messagePipe.Process(core.NewMessage(core.DataplaneChanged, nil)) - messagePipe.Run() + messagePipe.RunWithoutInit() binary.AssertExpectations(t) @@ -692,6 +683,7 @@ func TestNginxDetailProcUpdate(t *testing.T) { binary := tutils.NewMockNginxBinary() binary.On("GetNginxDetailsMapFromProcesses", mock.Anything).Return(tutils.GetDetailsMap()) binary.On("UpdateNginxDetailsFromProcesses", tutils.GetProcesses()) + binary.On("ReadConfig", mock.Anything, mock.Anything, mock.Anything).Return(&proto.NginxConfig{}, nil) cmdr := tutils.NewMockCommandClient() @@ -699,10 +691,8 @@ func TestNginxDetailProcUpdate(t *testing.T) { messagePipe := core.SetupMockMessagePipe(t, context.TODO(), []core.Plugin{pluginUnderTest}, []core.ExtensionPlugin{}) pluginUnderTest.Init(messagePipe) - messagePipe.Process(core.NewMessage(core.AgentStarted, nil)) - messagePipe.Process(core.NewMessage(core.NginxDetailProcUpdate, tutils.GetProcesses())) - messagePipe.Run() + messagePipe.RunWithoutInit() binary.AssertExpectations(t) cmdr.AssertExpectations(t) @@ -716,7 +706,7 @@ func TestNginxDetailProcUpdate(t *testing.T) { foundMessage = true } } - assert.Len(t, processedMessages, 4) + assert.Len(t, processedMessages, 3) assert.True(t, foundMessage) } @@ -1048,6 +1038,7 @@ func TestBlock_ConfigApply(t *testing.T) { binary := tutils.NewMockNginxBinary() binary.On("UpdateNginxDetailsFromProcesses", env.Processes()) binary.On("GetNginxDetailsMapFromProcesses", env.Processes()).Return(tutils.GetDetailsMap()) + binary.On("ReadConfig", mock.Anything, mock.Anything, mock.Anything).Return(&proto.NginxConfig{}, nil) binary.On("Reload", mock.Anything, mock.Anything).Return(nil) config := tutils.GetMockAgentConfig() diff --git a/test/integration/api/api_test.go b/test/integration/api/api_test.go index 8536683b57..909074da1e 100644 --- a/test/integration/api/api_test.go +++ b/test/integration/api/api_test.go @@ -80,6 +80,10 @@ func TestAPI_Metrics(t *testing.T) { assert.Equal(t, http.StatusOK, resp.StatusCode()) assert.Contains(t, resp.String(), "system_cpu_system") assert.NotContains(t, resp.String(), "test_fail_metric") + // Validate that the agent can call the stub status API + assert.Contains(t, resp.String(), "nginx_http_request_count") + // Validate that the agent can read the NGINX access logs + assert.Contains(t, resp.String(), "nginx_http_status_2xx") metrics := tutils.ProcessResponse(resp) diff --git a/test/integration/api/nginx-agent.conf b/test/integration/api/nginx-agent.conf index 59d136a907..fedca4030b 100644 --- a/test/integration/api/nginx-agent.conf +++ b/test/integration/api/nginx-agent.conf @@ -49,7 +49,7 @@ metrics: bulk_size: 20 # specify metrics poll interval report_interval: 1m - collection_interval: 15s + collection_interval: 1s mode: aggregated # OSS NGINX default config path diff --git a/test/integration/vendor/github.com/nginx/agent/v2/src/core/pipe.go b/test/integration/vendor/github.com/nginx/agent/v2/src/core/pipe.go index 657f572837..1ded89e02c 100644 --- a/test/integration/vendor/github.com/nginx/agent/v2/src/core/pipe.go +++ b/test/integration/vendor/github.com/nginx/agent/v2/src/core/pipe.go @@ -194,11 +194,11 @@ func (p *MessagePipe) GetExtensionPlugins() []ExtensionPlugin { func (p *MessagePipe) initPlugins() { for _, r := range p.plugins { - go r.Init(p) + r.Init(p) } for _, r := range p.extensionPlugins { - go r.Init(p) + r.Init(p) } } diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/core/pipe.go b/test/performance/vendor/github.com/nginx/agent/v2/src/core/pipe.go index 657f572837..1ded89e02c 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/core/pipe.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/core/pipe.go @@ -194,11 +194,11 @@ func (p *MessagePipe) GetExtensionPlugins() []ExtensionPlugin { func (p *MessagePipe) initPlugins() { for _, r := range p.plugins { - go r.Init(p) + r.Init(p) } for _, r := range p.extensionPlugins { - go r.Init(p) + r.Init(p) } } diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/plugins/nginx.go b/test/performance/vendor/github.com/nginx/agent/v2/src/plugins/nginx.go index 1937ca8dce..5036c71237 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/plugins/nginx.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/plugins/nginx.go @@ -91,7 +91,6 @@ type NginxConfigValidationResponse struct { func NewNginx(cmdr client.Commander, nginxBinary core.NginxBinary, env core.Environment, loadedConfig *config.Config, processes []*core.Process) *Nginx { isFeatureNginxConfigEnabled := loadedConfig.IsFeatureEnabled(agent_config.FeatureNginxConfig) || loadedConfig.IsFeatureEnabled(agent_config.FeatureNginxConfigAsync) - isNginxAppProtectEnabled := loadedConfig.IsExtensionEnabled(agent_config.NginxAppProtectExtensionPlugin) return &Nginx{ @@ -112,17 +111,25 @@ func (n *Nginx) Init(pipeline core.MessagePipeInterface) { log.Info("NginxBinary initializing") n.messagePipeline = pipeline n.nginxBinary.UpdateNginxDetailsFromProcesses(n.getNginxProccessInfo()) + nginxDetails := n.nginxBinary.GetNginxDetailsMapFromProcesses(n.getNginxProccessInfo()) + + for _, nginxDetail := range nginxDetails { + log.Infof("Reading config in directory %v for nginx instance %v", nginxDetail.GetConfPath(), nginxDetail.GetNginxId()) + _, err := n.nginxBinary.ReadConfig(nginxDetail.GetConfPath(), nginxDetail.GetNginxId(), n.env.GetSystemUUID()) + if err != nil { + log.Errorf("Unable to read nginx config %s: %v", nginxDetail.GetConfPath(), err) + } + } + + n.messagePipeline.Process( + core.NewMessage(core.NginxPluginConfigured, n), + core.NewMessage(core.NginxInstancesFound, nginxDetails), + ) } // Process processes the messages from the messaging pipe func (n *Nginx) Process(message *core.Message) { switch message.Topic() { - case core.AgentStarted: - nginxDetails := n.nginxBinary.GetNginxDetailsMapFromProcesses(n.getNginxProccessInfo()) - n.messagePipeline.Process( - core.NewMessage(core.NginxPluginConfigured, n), - core.NewMessage(core.NginxInstancesFound, nginxDetails), - ) case core.CommNginxConfig: switch cmd := message.Data().(type) { case *proto.Command: