From 4a58377e344d1b66368acac61e90355a412177fa Mon Sep 17 00:00:00 2001 From: Donal Hurley Date: Fri, 20 Jun 2025 11:55:19 +0100 Subject: [PATCH 1/3] Check for ssl in listen directive to determine is an endpoint is using http ot https --- sdk/config_helpers.go | 20 ++++++++++++++----- .../nginx/agent/sdk/v2/config_helpers.go | 20 ++++++++++++++----- .../nginx/agent/sdk/v2/config_helpers.go | 20 ++++++++++++++----- 3 files changed, 45 insertions(+), 15 deletions(-) diff --git a/sdk/config_helpers.go b/sdk/config_helpers.go index 66ce17c64..a0b330d27 100644 --- a/sdk/config_helpers.go +++ b/sdk/config_helpers.go @@ -39,9 +39,10 @@ import ( const ( plusAPIDirective = "api" stubStatusAPIDirective = "stub_status" - apiFormat = "http://%s%s" predefinedAccessLogFormat = "$remote_addr - $remote_user [$time_local] \"$request\" $status $body_bytes_sent \"$http_referer\" \"$http_user_agent\"" httpClientTimeout = 1 * time.Second + httpPrefix = "http://" + httpsPrefix = "https://" ) var readLock = sync.Mutex{} @@ -698,6 +699,7 @@ func parseAddressesFromServerDirective(parent *crossplane.Directive) []string { for _, dir := range parent.Block { hostname := "127.0.0.1" + prefix := httpPrefix switch dir.Directive { case "listen": @@ -718,14 +720,22 @@ func parseAddressesFromServerDirective(parent *crossplane.Directive) []string { hostname = dir.Args[0] } } - hosts = append(hosts, hostname) + + if len(dir.Args) > 1 { + secondArg := dir.Args[1] + if secondArg == "ssl" { + prefix = httpsPrefix + } + } + + hosts = append(hosts, prefix+hostname) case "server_name": if dir.Args[0] == "_" { // default server continue } hostname = dir.Args[0] - hosts = append(hosts, hostname) + hosts = append(hosts, prefix+hostname) } } @@ -952,11 +962,11 @@ func getUrlsForLocationDirective(parent *crossplane.Directive, current *crosspla addresses := parseAddressesFromServerDirective(parent) for _, address := range addresses { - path := parsePathFromLocationDirective(current) + pathFromLocationDirective := parsePathFromLocationDirective(current) switch locChild.Directive { case locationDirectiveName: - urls = append(urls, fmt.Sprintf(apiFormat, address, path)) + urls = append(urls, address+pathFromLocationDirective) } } } diff --git a/test/integration/vendor/github.com/nginx/agent/sdk/v2/config_helpers.go b/test/integration/vendor/github.com/nginx/agent/sdk/v2/config_helpers.go index 66ce17c64..a0b330d27 100644 --- a/test/integration/vendor/github.com/nginx/agent/sdk/v2/config_helpers.go +++ b/test/integration/vendor/github.com/nginx/agent/sdk/v2/config_helpers.go @@ -39,9 +39,10 @@ import ( const ( plusAPIDirective = "api" stubStatusAPIDirective = "stub_status" - apiFormat = "http://%s%s" predefinedAccessLogFormat = "$remote_addr - $remote_user [$time_local] \"$request\" $status $body_bytes_sent \"$http_referer\" \"$http_user_agent\"" httpClientTimeout = 1 * time.Second + httpPrefix = "http://" + httpsPrefix = "https://" ) var readLock = sync.Mutex{} @@ -698,6 +699,7 @@ func parseAddressesFromServerDirective(parent *crossplane.Directive) []string { for _, dir := range parent.Block { hostname := "127.0.0.1" + prefix := httpPrefix switch dir.Directive { case "listen": @@ -718,14 +720,22 @@ func parseAddressesFromServerDirective(parent *crossplane.Directive) []string { hostname = dir.Args[0] } } - hosts = append(hosts, hostname) + + if len(dir.Args) > 1 { + secondArg := dir.Args[1] + if secondArg == "ssl" { + prefix = httpsPrefix + } + } + + hosts = append(hosts, prefix+hostname) case "server_name": if dir.Args[0] == "_" { // default server continue } hostname = dir.Args[0] - hosts = append(hosts, hostname) + hosts = append(hosts, prefix+hostname) } } @@ -952,11 +962,11 @@ func getUrlsForLocationDirective(parent *crossplane.Directive, current *crosspla addresses := parseAddressesFromServerDirective(parent) for _, address := range addresses { - path := parsePathFromLocationDirective(current) + pathFromLocationDirective := parsePathFromLocationDirective(current) switch locChild.Directive { case locationDirectiveName: - urls = append(urls, fmt.Sprintf(apiFormat, address, path)) + urls = append(urls, address+pathFromLocationDirective) } } } diff --git a/test/performance/vendor/github.com/nginx/agent/sdk/v2/config_helpers.go b/test/performance/vendor/github.com/nginx/agent/sdk/v2/config_helpers.go index 66ce17c64..a0b330d27 100644 --- a/test/performance/vendor/github.com/nginx/agent/sdk/v2/config_helpers.go +++ b/test/performance/vendor/github.com/nginx/agent/sdk/v2/config_helpers.go @@ -39,9 +39,10 @@ import ( const ( plusAPIDirective = "api" stubStatusAPIDirective = "stub_status" - apiFormat = "http://%s%s" predefinedAccessLogFormat = "$remote_addr - $remote_user [$time_local] \"$request\" $status $body_bytes_sent \"$http_referer\" \"$http_user_agent\"" httpClientTimeout = 1 * time.Second + httpPrefix = "http://" + httpsPrefix = "https://" ) var readLock = sync.Mutex{} @@ -698,6 +699,7 @@ func parseAddressesFromServerDirective(parent *crossplane.Directive) []string { for _, dir := range parent.Block { hostname := "127.0.0.1" + prefix := httpPrefix switch dir.Directive { case "listen": @@ -718,14 +720,22 @@ func parseAddressesFromServerDirective(parent *crossplane.Directive) []string { hostname = dir.Args[0] } } - hosts = append(hosts, hostname) + + if len(dir.Args) > 1 { + secondArg := dir.Args[1] + if secondArg == "ssl" { + prefix = httpsPrefix + } + } + + hosts = append(hosts, prefix+hostname) case "server_name": if dir.Args[0] == "_" { // default server continue } hostname = dir.Args[0] - hosts = append(hosts, hostname) + hosts = append(hosts, prefix+hostname) } } @@ -952,11 +962,11 @@ func getUrlsForLocationDirective(parent *crossplane.Directive, current *crosspla addresses := parseAddressesFromServerDirective(parent) for _, address := range addresses { - path := parsePathFromLocationDirective(current) + pathFromLocationDirective := parsePathFromLocationDirective(current) switch locChild.Directive { case locationDirectiveName: - urls = append(urls, fmt.Sprintf(apiFormat, address, path)) + urls = append(urls, address+pathFromLocationDirective) } } } From 5cd3241c0088d6f5f911dd93590ffa765587b539 Mon Sep 17 00:00:00 2001 From: Donal Hurley Date: Tue, 24 Jun 2025 14:25:30 +0100 Subject: [PATCH 2/3] Add NGINX CA config option for NGINX Plus API --- src/core/config/config.go | 1 + src/core/config/defaults.go | 7 +++ src/core/config/types.go | 17 ++++++++ src/core/metrics/collectors/nginx.go | 5 ++- src/core/metrics/sources/nginx_plus.go | 43 +++++++++++++++++-- src/core/metrics/sources/nginx_plus_test.go | 4 +- .../nginx/agent/v2/src/core/config/config.go | 1 + .../agent/v2/src/core/config/defaults.go | 7 +++ .../nginx/agent/v2/src/core/config/types.go | 17 ++++++++ .../nginx/agent/v2/src/core/config/config.go | 1 + .../agent/v2/src/core/config/defaults.go | 7 +++ .../nginx/agent/v2/src/core/config/types.go | 17 ++++++++ .../v2/src/core/metrics/collectors/nginx.go | 5 ++- .../v2/src/core/metrics/sources/nginx_plus.go | 43 +++++++++++++++++-- 14 files changed, 165 insertions(+), 10 deletions(-) diff --git a/src/core/config/config.go b/src/core/config/config.go index b8ed3aa10..bd5746032 100644 --- a/src/core/config/config.go +++ b/src/core/config/config.go @@ -354,6 +354,7 @@ func getNginx() Nginx { NginxClientVersion: Viper.GetInt(NginxClientVersion), ConfigReloadMonitoringPeriod: Viper.GetDuration(NginxConfigReloadMonitoringPeriod), TreatWarningsAsErrors: Viper.GetBool(NginxTreatWarningsAsErrors), + Ca: Viper.GetString(NginxCa), } } diff --git a/src/core/config/defaults.go b/src/core/config/defaults.go index efec25090..1b88f790f 100644 --- a/src/core/config/defaults.go +++ b/src/core/config/defaults.go @@ -66,6 +66,7 @@ var ( NginxClientVersion: 7, // NGINX Plus R25+ ConfigReloadMonitoringPeriod: 10 * time.Second, TreatWarningsAsErrors: false, + Ca: "", }, ConfigDirs: "/etc/nginx:/usr/local/etc/nginx:/usr/share/nginx/modules:/etc/nms", IgnoreDirectives: []string{}, @@ -175,6 +176,7 @@ const ( NginxClientVersion = NginxKey + agent_config.KeyDelimiter + "client_version" NginxConfigReloadMonitoringPeriod = NginxKey + agent_config.KeyDelimiter + "config_reload_monitoring_period" NginxTreatWarningsAsErrors = NginxKey + agent_config.KeyDelimiter + "treat_warnings_as_errors" + NginxCa = NginxKey + agent_config.KeyDelimiter + "ca" // viper keys used in config DataplaneKey = "dataplane" @@ -321,6 +323,11 @@ var ( Usage: "On nginx -t, treat warnings as failures on configuration application.", DefaultValue: Defaults.Nginx.TreatWarningsAsErrors, }, + &StringFlag{ + Name: NginxCa, + Usage: "The NGINX Plus CA certificate file location needed to call the NGINX Plus API if SSL is enabled.", + DefaultValue: Defaults.Nginx.Ca, + }, // Metrics &DurationFlag{ Name: MetricsCollectionInterval, diff --git a/src/core/config/types.go b/src/core/config/types.go index 2e359b34b..a4e0c6a6e 100644 --- a/src/core/config/types.go +++ b/src/core/config/types.go @@ -8,6 +8,8 @@ package config import ( + "path/filepath" + "strings" "time" "github.com/nginx/agent/sdk/v2/backoff" @@ -90,6 +92,20 @@ func (c *Config) GetMetricsBackoffSettings() backoff.BackoffSettings { } } +func (c *Config) IsFileAllowed(path string) bool { + if !filepath.IsAbs(path) { + return false + } + + for dir := range c.AllowedDirectoriesMap { + if strings.HasPrefix(path, dir) { + return true + } + } + + return false +} + type Server struct { Host string `mapstructure:"host" yaml:"-"` GrpcPort int `mapstructure:"grpcPort" yaml:"-"` @@ -139,6 +155,7 @@ type Nginx struct { NginxClientVersion int `mapstructure:"client_version" yaml:"-"` ConfigReloadMonitoringPeriod time.Duration `mapstructure:"config_reload_monitoring_period" yaml:"-"` TreatWarningsAsErrors bool `mapstructure:"treat_warnings_as_errors" yaml:"-"` + Ca string `mapstructure:"ca" yaml:"-"` } type Dataplane struct { diff --git a/src/core/metrics/collectors/nginx.go b/src/core/metrics/collectors/nginx.go index f8cea947d..396e094dd 100644 --- a/src/core/metrics/collectors/nginx.go +++ b/src/core/metrics/collectors/nginx.go @@ -61,7 +61,10 @@ func buildSources(dimensions *metrics.CommonDim, binary core.NginxBinary, collec nginxSources = append(nginxSources, sources.NewNginxAccessLog(dimensions, sources.OSSNamespace, binary, sources.OSSNginxType, collectorConf.CollectionInterval)) nginxSources = append(nginxSources, sources.NewNginxErrorLog(dimensions, sources.OSSNamespace, binary, sources.OSSNginxType, collectorConf.CollectionInterval)) } else if collectorConf.PlusAPI != "" { - nginxSources = append(nginxSources, sources.NewNginxPlus(dimensions, sources.OSSNamespace, sources.PlusNamespace, collectorConf.PlusAPI, collectorConf.ClientVersion)) + nginxPlusSource := sources.NewNginxPlus(dimensions, sources.OSSNamespace, sources.PlusNamespace, collectorConf.PlusAPI, collectorConf.ClientVersion, conf) + if nginxPlusSource != nil { + nginxSources = append(nginxSources, nginxPlusSource) + } nginxSources = append(nginxSources, sources.NewNginxAccessLog(dimensions, sources.OSSNamespace, binary, sources.PlusNginxType, collectorConf.CollectionInterval)) nginxSources = append(nginxSources, sources.NewNginxErrorLog(dimensions, sources.OSSNamespace, binary, sources.PlusNginxType, collectorConf.CollectionInterval)) } else { diff --git a/src/core/metrics/sources/nginx_plus.go b/src/core/metrics/sources/nginx_plus.go index 564fefe27..2b6bb1187 100644 --- a/src/core/metrics/sources/nginx_plus.go +++ b/src/core/metrics/sources/nginx_plus.go @@ -9,10 +9,16 @@ package sources import ( "context" + "crypto/tls" + "crypto/x509" "fmt" "math" + "net/http" + "os" "sync" + "github.com/nginx/agent/v2/src/core/config" + "github.com/nginx/agent/sdk/v2/proto" "github.com/nginx/agent/v2/src/core/metrics" @@ -67,6 +73,7 @@ type NginxPlus struct { init sync.Once clientVersion int logger *MetricSourceLogger + client *http.Client } type ExtendedStats struct { @@ -75,14 +82,44 @@ type ExtendedStats struct { streamEndpoints []string } -func NewNginxPlus(baseDimensions *metrics.CommonDim, nginxNamespace, plusNamespace, plusAPI string, clientVersion int) *NginxPlus { +func NewNginxPlus(baseDimensions *metrics.CommonDim, nginxNamespace, plusNamespace, plusAPI string, clientVersion int, conf *config.Config) *NginxPlus { log.Debug("Creating NGINX Plus metrics source") - return &NginxPlus{baseDimensions: baseDimensions, nginxNamespace: nginxNamespace, plusNamespace: plusNamespace, plusAPI: plusAPI, clientVersion: clientVersion, logger: NewMetricSourceLogger()} + + client := http.DefaultClient + + if conf.Nginx.Ca != "" && conf.IsFileAllowed(conf.Nginx.Ca) { + data, err := os.ReadFile(conf.Nginx.Ca) + if err != nil { + log.Errorf("Unable to collect NGINX Plus metrics. Failed to read NGINX CA certificate file: %v", err) + return nil + } + + caCertPool := x509.NewCertPool() + caCertPool.AppendCertsFromPEM(data) + + client = &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + RootCAs: caCertPool, + }, + }, + } + } + + return &NginxPlus{ + baseDimensions: baseDimensions, + nginxNamespace: nginxNamespace, + plusNamespace: plusNamespace, + plusAPI: plusAPI, + clientVersion: clientVersion, + logger: NewMetricSourceLogger(), + client: client, + } } func (c *NginxPlus) Collect(ctx context.Context, m chan<- *metrics.StatsEntityWrapper) { c.init.Do(func() { - cl, err := plusclient.NewNginxClient(c.plusAPI, plusclient.WithMaxAPIVersion()) + cl, err := plusclient.NewNginxClient(c.plusAPI, plusclient.WithMaxAPIVersion(), plusclient.WithHTTPClient(c.client)) if err != nil { c.logger.Log(fmt.Sprintf("Failed to create plus metrics client: %v", err)) SendNginxDownStatus(ctx, c.baseDimensions.ToDimensions(), m) diff --git a/src/core/metrics/sources/nginx_plus_test.go b/src/core/metrics/sources/nginx_plus_test.go index 28224cfa5..129b40300 100644 --- a/src/core/metrics/sources/nginx_plus_test.go +++ b/src/core/metrics/sources/nginx_plus_test.go @@ -570,7 +570,7 @@ func (f *FakeNginxPlus) Collect(ctx context.Context, m chan<- *metrics.StatsEnti } func TestNginxPlusUpdate(t *testing.T) { - nginxPlus := NewNginxPlus(&metrics.CommonDim{}, "test", PlusNamespace, "http://localhost:8080/api", 6) + nginxPlus := NewNginxPlus(&metrics.CommonDim{}, "test", PlusNamespace, "http://localhost:8080/api", 6, &config.Config{}) assert.Equal(t, "", nginxPlus.baseDimensions.InstanceTags) assert.Equal(t, "http://localhost:8080/api", nginxPlus.plusAPI) @@ -867,7 +867,7 @@ func TestNginxPlus_Collect(t *testing.T) { for _, test := range tests { ctx := context.TODO() - f := &FakeNginxPlus{NewNginxPlus(test.baseDimensions, "nginx", "plus", "", 6)} + f := &FakeNginxPlus{NewNginxPlus(test.baseDimensions, "nginx", "plus", "", 6, &config.Config{})} go f.Collect(ctx, test.m) instanceMetrics := <-test.m diff --git a/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/config.go b/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/config.go index b8ed3aa10..bd5746032 100644 --- a/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/config.go +++ b/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/config.go @@ -354,6 +354,7 @@ func getNginx() Nginx { NginxClientVersion: Viper.GetInt(NginxClientVersion), ConfigReloadMonitoringPeriod: Viper.GetDuration(NginxConfigReloadMonitoringPeriod), TreatWarningsAsErrors: Viper.GetBool(NginxTreatWarningsAsErrors), + Ca: Viper.GetString(NginxCa), } } diff --git a/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go b/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go index efec25090..1b88f790f 100644 --- a/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go +++ b/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go @@ -66,6 +66,7 @@ var ( NginxClientVersion: 7, // NGINX Plus R25+ ConfigReloadMonitoringPeriod: 10 * time.Second, TreatWarningsAsErrors: false, + Ca: "", }, ConfigDirs: "/etc/nginx:/usr/local/etc/nginx:/usr/share/nginx/modules:/etc/nms", IgnoreDirectives: []string{}, @@ -175,6 +176,7 @@ const ( NginxClientVersion = NginxKey + agent_config.KeyDelimiter + "client_version" NginxConfigReloadMonitoringPeriod = NginxKey + agent_config.KeyDelimiter + "config_reload_monitoring_period" NginxTreatWarningsAsErrors = NginxKey + agent_config.KeyDelimiter + "treat_warnings_as_errors" + NginxCa = NginxKey + agent_config.KeyDelimiter + "ca" // viper keys used in config DataplaneKey = "dataplane" @@ -321,6 +323,11 @@ var ( Usage: "On nginx -t, treat warnings as failures on configuration application.", DefaultValue: Defaults.Nginx.TreatWarningsAsErrors, }, + &StringFlag{ + Name: NginxCa, + Usage: "The NGINX Plus CA certificate file location needed to call the NGINX Plus API if SSL is enabled.", + DefaultValue: Defaults.Nginx.Ca, + }, // Metrics &DurationFlag{ Name: MetricsCollectionInterval, diff --git a/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/types.go b/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/types.go index 2e359b34b..a4e0c6a6e 100644 --- a/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/types.go +++ b/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/types.go @@ -8,6 +8,8 @@ package config import ( + "path/filepath" + "strings" "time" "github.com/nginx/agent/sdk/v2/backoff" @@ -90,6 +92,20 @@ func (c *Config) GetMetricsBackoffSettings() backoff.BackoffSettings { } } +func (c *Config) IsFileAllowed(path string) bool { + if !filepath.IsAbs(path) { + return false + } + + for dir := range c.AllowedDirectoriesMap { + if strings.HasPrefix(path, dir) { + return true + } + } + + return false +} + type Server struct { Host string `mapstructure:"host" yaml:"-"` GrpcPort int `mapstructure:"grpcPort" yaml:"-"` @@ -139,6 +155,7 @@ type Nginx struct { NginxClientVersion int `mapstructure:"client_version" yaml:"-"` ConfigReloadMonitoringPeriod time.Duration `mapstructure:"config_reload_monitoring_period" yaml:"-"` TreatWarningsAsErrors bool `mapstructure:"treat_warnings_as_errors" yaml:"-"` + Ca string `mapstructure:"ca" yaml:"-"` } type Dataplane struct { diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/config.go b/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/config.go index b8ed3aa10..bd5746032 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/config.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/config.go @@ -354,6 +354,7 @@ func getNginx() Nginx { NginxClientVersion: Viper.GetInt(NginxClientVersion), ConfigReloadMonitoringPeriod: Viper.GetDuration(NginxConfigReloadMonitoringPeriod), TreatWarningsAsErrors: Viper.GetBool(NginxTreatWarningsAsErrors), + Ca: Viper.GetString(NginxCa), } } diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go b/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go index efec25090..1b88f790f 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go @@ -66,6 +66,7 @@ var ( NginxClientVersion: 7, // NGINX Plus R25+ ConfigReloadMonitoringPeriod: 10 * time.Second, TreatWarningsAsErrors: false, + Ca: "", }, ConfigDirs: "/etc/nginx:/usr/local/etc/nginx:/usr/share/nginx/modules:/etc/nms", IgnoreDirectives: []string{}, @@ -175,6 +176,7 @@ const ( NginxClientVersion = NginxKey + agent_config.KeyDelimiter + "client_version" NginxConfigReloadMonitoringPeriod = NginxKey + agent_config.KeyDelimiter + "config_reload_monitoring_period" NginxTreatWarningsAsErrors = NginxKey + agent_config.KeyDelimiter + "treat_warnings_as_errors" + NginxCa = NginxKey + agent_config.KeyDelimiter + "ca" // viper keys used in config DataplaneKey = "dataplane" @@ -321,6 +323,11 @@ var ( Usage: "On nginx -t, treat warnings as failures on configuration application.", DefaultValue: Defaults.Nginx.TreatWarningsAsErrors, }, + &StringFlag{ + Name: NginxCa, + Usage: "The NGINX Plus CA certificate file location needed to call the NGINX Plus API if SSL is enabled.", + DefaultValue: Defaults.Nginx.Ca, + }, // Metrics &DurationFlag{ Name: MetricsCollectionInterval, diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/types.go b/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/types.go index 2e359b34b..a4e0c6a6e 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/types.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/types.go @@ -8,6 +8,8 @@ package config import ( + "path/filepath" + "strings" "time" "github.com/nginx/agent/sdk/v2/backoff" @@ -90,6 +92,20 @@ func (c *Config) GetMetricsBackoffSettings() backoff.BackoffSettings { } } +func (c *Config) IsFileAllowed(path string) bool { + if !filepath.IsAbs(path) { + return false + } + + for dir := range c.AllowedDirectoriesMap { + if strings.HasPrefix(path, dir) { + return true + } + } + + return false +} + type Server struct { Host string `mapstructure:"host" yaml:"-"` GrpcPort int `mapstructure:"grpcPort" yaml:"-"` @@ -139,6 +155,7 @@ type Nginx struct { NginxClientVersion int `mapstructure:"client_version" yaml:"-"` ConfigReloadMonitoringPeriod time.Duration `mapstructure:"config_reload_monitoring_period" yaml:"-"` TreatWarningsAsErrors bool `mapstructure:"treat_warnings_as_errors" yaml:"-"` + Ca string `mapstructure:"ca" yaml:"-"` } type Dataplane struct { diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/core/metrics/collectors/nginx.go b/test/performance/vendor/github.com/nginx/agent/v2/src/core/metrics/collectors/nginx.go index f8cea947d..396e094dd 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/core/metrics/collectors/nginx.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/core/metrics/collectors/nginx.go @@ -61,7 +61,10 @@ func buildSources(dimensions *metrics.CommonDim, binary core.NginxBinary, collec nginxSources = append(nginxSources, sources.NewNginxAccessLog(dimensions, sources.OSSNamespace, binary, sources.OSSNginxType, collectorConf.CollectionInterval)) nginxSources = append(nginxSources, sources.NewNginxErrorLog(dimensions, sources.OSSNamespace, binary, sources.OSSNginxType, collectorConf.CollectionInterval)) } else if collectorConf.PlusAPI != "" { - nginxSources = append(nginxSources, sources.NewNginxPlus(dimensions, sources.OSSNamespace, sources.PlusNamespace, collectorConf.PlusAPI, collectorConf.ClientVersion)) + nginxPlusSource := sources.NewNginxPlus(dimensions, sources.OSSNamespace, sources.PlusNamespace, collectorConf.PlusAPI, collectorConf.ClientVersion, conf) + if nginxPlusSource != nil { + nginxSources = append(nginxSources, nginxPlusSource) + } nginxSources = append(nginxSources, sources.NewNginxAccessLog(dimensions, sources.OSSNamespace, binary, sources.PlusNginxType, collectorConf.CollectionInterval)) nginxSources = append(nginxSources, sources.NewNginxErrorLog(dimensions, sources.OSSNamespace, binary, sources.PlusNginxType, collectorConf.CollectionInterval)) } else { diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/core/metrics/sources/nginx_plus.go b/test/performance/vendor/github.com/nginx/agent/v2/src/core/metrics/sources/nginx_plus.go index 564fefe27..2b6bb1187 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/core/metrics/sources/nginx_plus.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/core/metrics/sources/nginx_plus.go @@ -9,10 +9,16 @@ package sources import ( "context" + "crypto/tls" + "crypto/x509" "fmt" "math" + "net/http" + "os" "sync" + "github.com/nginx/agent/v2/src/core/config" + "github.com/nginx/agent/sdk/v2/proto" "github.com/nginx/agent/v2/src/core/metrics" @@ -67,6 +73,7 @@ type NginxPlus struct { init sync.Once clientVersion int logger *MetricSourceLogger + client *http.Client } type ExtendedStats struct { @@ -75,14 +82,44 @@ type ExtendedStats struct { streamEndpoints []string } -func NewNginxPlus(baseDimensions *metrics.CommonDim, nginxNamespace, plusNamespace, plusAPI string, clientVersion int) *NginxPlus { +func NewNginxPlus(baseDimensions *metrics.CommonDim, nginxNamespace, plusNamespace, plusAPI string, clientVersion int, conf *config.Config) *NginxPlus { log.Debug("Creating NGINX Plus metrics source") - return &NginxPlus{baseDimensions: baseDimensions, nginxNamespace: nginxNamespace, plusNamespace: plusNamespace, plusAPI: plusAPI, clientVersion: clientVersion, logger: NewMetricSourceLogger()} + + client := http.DefaultClient + + if conf.Nginx.Ca != "" && conf.IsFileAllowed(conf.Nginx.Ca) { + data, err := os.ReadFile(conf.Nginx.Ca) + if err != nil { + log.Errorf("Unable to collect NGINX Plus metrics. Failed to read NGINX CA certificate file: %v", err) + return nil + } + + caCertPool := x509.NewCertPool() + caCertPool.AppendCertsFromPEM(data) + + client = &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + RootCAs: caCertPool, + }, + }, + } + } + + return &NginxPlus{ + baseDimensions: baseDimensions, + nginxNamespace: nginxNamespace, + plusNamespace: plusNamespace, + plusAPI: plusAPI, + clientVersion: clientVersion, + logger: NewMetricSourceLogger(), + client: client, + } } func (c *NginxPlus) Collect(ctx context.Context, m chan<- *metrics.StatsEntityWrapper) { c.init.Do(func() { - cl, err := plusclient.NewNginxClient(c.plusAPI, plusclient.WithMaxAPIVersion()) + cl, err := plusclient.NewNginxClient(c.plusAPI, plusclient.WithMaxAPIVersion(), plusclient.WithHTTPClient(c.client)) if err != nil { c.logger.Log(fmt.Sprintf("Failed to create plus metrics client: %v", err)) SendNginxDownStatus(ctx, c.baseDimensions.ToDimensions(), m) From e2f86b1e9ad89c5074353436b3cfcca72b4b2752 Mon Sep 17 00:00:00 2001 From: Donal Hurley Date: Wed, 25 Jun 2025 15:55:04 +0100 Subject: [PATCH 3/3] Address feedback --- src/core/config/config.go | 4 +++- src/core/config/defaults.go | 10 ++++++---- src/core/config/types.go | 2 +- src/core/metrics/sources/nginx_plus.go | 4 ++-- .../nginx/agent/v2/src/core/config/config.go | 4 +++- .../nginx/agent/v2/src/core/config/defaults.go | 10 ++++++---- .../github.com/nginx/agent/v2/src/core/config/types.go | 2 +- .../nginx/agent/v2/src/core/config/config.go | 4 +++- .../nginx/agent/v2/src/core/config/defaults.go | 10 ++++++---- .../github.com/nginx/agent/v2/src/core/config/types.go | 2 +- .../agent/v2/src/core/metrics/sources/nginx_plus.go | 4 ++-- 11 files changed, 34 insertions(+), 22 deletions(-) diff --git a/src/core/config/config.go b/src/core/config/config.go index bd5746032..49402d75c 100644 --- a/src/core/config/config.go +++ b/src/core/config/config.go @@ -354,7 +354,9 @@ func getNginx() Nginx { NginxClientVersion: Viper.GetInt(NginxClientVersion), ConfigReloadMonitoringPeriod: Viper.GetDuration(NginxConfigReloadMonitoringPeriod), TreatWarningsAsErrors: Viper.GetBool(NginxTreatWarningsAsErrors), - Ca: Viper.GetString(NginxCa), + ApiTls: TLSConfig{ + Ca: Viper.GetString(NginxApiTlsCa), + }, } } diff --git a/src/core/config/defaults.go b/src/core/config/defaults.go index 1b88f790f..1fc1bb608 100644 --- a/src/core/config/defaults.go +++ b/src/core/config/defaults.go @@ -66,7 +66,9 @@ var ( NginxClientVersion: 7, // NGINX Plus R25+ ConfigReloadMonitoringPeriod: 10 * time.Second, TreatWarningsAsErrors: false, - Ca: "", + ApiTls: TLSConfig{ + Ca: "", + }, }, ConfigDirs: "/etc/nginx:/usr/local/etc/nginx:/usr/share/nginx/modules:/etc/nms", IgnoreDirectives: []string{}, @@ -176,7 +178,7 @@ const ( NginxClientVersion = NginxKey + agent_config.KeyDelimiter + "client_version" NginxConfigReloadMonitoringPeriod = NginxKey + agent_config.KeyDelimiter + "config_reload_monitoring_period" NginxTreatWarningsAsErrors = NginxKey + agent_config.KeyDelimiter + "treat_warnings_as_errors" - NginxCa = NginxKey + agent_config.KeyDelimiter + "ca" + NginxApiTlsCa = NginxKey + agent_config.KeyDelimiter + "api_tls_ca" // viper keys used in config DataplaneKey = "dataplane" @@ -324,9 +326,9 @@ var ( DefaultValue: Defaults.Nginx.TreatWarningsAsErrors, }, &StringFlag{ - Name: NginxCa, + Name: NginxApiTlsCa, Usage: "The NGINX Plus CA certificate file location needed to call the NGINX Plus API if SSL is enabled.", - DefaultValue: Defaults.Nginx.Ca, + DefaultValue: Defaults.Nginx.ApiTls.Ca, }, // Metrics &DurationFlag{ diff --git a/src/core/config/types.go b/src/core/config/types.go index a4e0c6a6e..4b57a5c6d 100644 --- a/src/core/config/types.go +++ b/src/core/config/types.go @@ -155,7 +155,7 @@ type Nginx struct { NginxClientVersion int `mapstructure:"client_version" yaml:"-"` ConfigReloadMonitoringPeriod time.Duration `mapstructure:"config_reload_monitoring_period" yaml:"-"` TreatWarningsAsErrors bool `mapstructure:"treat_warnings_as_errors" yaml:"-"` - Ca string `mapstructure:"ca" yaml:"-"` + ApiTls TLSConfig `mapstructure:"api_tls" yaml:"-"` } type Dataplane struct { diff --git a/src/core/metrics/sources/nginx_plus.go b/src/core/metrics/sources/nginx_plus.go index 2b6bb1187..1077fdecc 100644 --- a/src/core/metrics/sources/nginx_plus.go +++ b/src/core/metrics/sources/nginx_plus.go @@ -87,8 +87,8 @@ func NewNginxPlus(baseDimensions *metrics.CommonDim, nginxNamespace, plusNamespa client := http.DefaultClient - if conf.Nginx.Ca != "" && conf.IsFileAllowed(conf.Nginx.Ca) { - data, err := os.ReadFile(conf.Nginx.Ca) + if conf.Nginx.ApiTls.Ca != "" && conf.IsFileAllowed(conf.Nginx.ApiTls.Ca) { + data, err := os.ReadFile(conf.Nginx.ApiTls.Ca) if err != nil { log.Errorf("Unable to collect NGINX Plus metrics. Failed to read NGINX CA certificate file: %v", err) return nil diff --git a/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/config.go b/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/config.go index bd5746032..49402d75c 100644 --- a/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/config.go +++ b/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/config.go @@ -354,7 +354,9 @@ func getNginx() Nginx { NginxClientVersion: Viper.GetInt(NginxClientVersion), ConfigReloadMonitoringPeriod: Viper.GetDuration(NginxConfigReloadMonitoringPeriod), TreatWarningsAsErrors: Viper.GetBool(NginxTreatWarningsAsErrors), - Ca: Viper.GetString(NginxCa), + ApiTls: TLSConfig{ + Ca: Viper.GetString(NginxApiTlsCa), + }, } } diff --git a/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go b/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go index 1b88f790f..1fc1bb608 100644 --- a/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go +++ b/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go @@ -66,7 +66,9 @@ var ( NginxClientVersion: 7, // NGINX Plus R25+ ConfigReloadMonitoringPeriod: 10 * time.Second, TreatWarningsAsErrors: false, - Ca: "", + ApiTls: TLSConfig{ + Ca: "", + }, }, ConfigDirs: "/etc/nginx:/usr/local/etc/nginx:/usr/share/nginx/modules:/etc/nms", IgnoreDirectives: []string{}, @@ -176,7 +178,7 @@ const ( NginxClientVersion = NginxKey + agent_config.KeyDelimiter + "client_version" NginxConfigReloadMonitoringPeriod = NginxKey + agent_config.KeyDelimiter + "config_reload_monitoring_period" NginxTreatWarningsAsErrors = NginxKey + agent_config.KeyDelimiter + "treat_warnings_as_errors" - NginxCa = NginxKey + agent_config.KeyDelimiter + "ca" + NginxApiTlsCa = NginxKey + agent_config.KeyDelimiter + "api_tls_ca" // viper keys used in config DataplaneKey = "dataplane" @@ -324,9 +326,9 @@ var ( DefaultValue: Defaults.Nginx.TreatWarningsAsErrors, }, &StringFlag{ - Name: NginxCa, + Name: NginxApiTlsCa, Usage: "The NGINX Plus CA certificate file location needed to call the NGINX Plus API if SSL is enabled.", - DefaultValue: Defaults.Nginx.Ca, + DefaultValue: Defaults.Nginx.ApiTls.Ca, }, // Metrics &DurationFlag{ diff --git a/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/types.go b/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/types.go index a4e0c6a6e..4b57a5c6d 100644 --- a/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/types.go +++ b/test/integration/vendor/github.com/nginx/agent/v2/src/core/config/types.go @@ -155,7 +155,7 @@ type Nginx struct { NginxClientVersion int `mapstructure:"client_version" yaml:"-"` ConfigReloadMonitoringPeriod time.Duration `mapstructure:"config_reload_monitoring_period" yaml:"-"` TreatWarningsAsErrors bool `mapstructure:"treat_warnings_as_errors" yaml:"-"` - Ca string `mapstructure:"ca" yaml:"-"` + ApiTls TLSConfig `mapstructure:"api_tls" yaml:"-"` } type Dataplane struct { diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/config.go b/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/config.go index bd5746032..49402d75c 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/config.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/config.go @@ -354,7 +354,9 @@ func getNginx() Nginx { NginxClientVersion: Viper.GetInt(NginxClientVersion), ConfigReloadMonitoringPeriod: Viper.GetDuration(NginxConfigReloadMonitoringPeriod), TreatWarningsAsErrors: Viper.GetBool(NginxTreatWarningsAsErrors), - Ca: Viper.GetString(NginxCa), + ApiTls: TLSConfig{ + Ca: Viper.GetString(NginxApiTlsCa), + }, } } diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go b/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go index 1b88f790f..1fc1bb608 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go @@ -66,7 +66,9 @@ var ( NginxClientVersion: 7, // NGINX Plus R25+ ConfigReloadMonitoringPeriod: 10 * time.Second, TreatWarningsAsErrors: false, - Ca: "", + ApiTls: TLSConfig{ + Ca: "", + }, }, ConfigDirs: "/etc/nginx:/usr/local/etc/nginx:/usr/share/nginx/modules:/etc/nms", IgnoreDirectives: []string{}, @@ -176,7 +178,7 @@ const ( NginxClientVersion = NginxKey + agent_config.KeyDelimiter + "client_version" NginxConfigReloadMonitoringPeriod = NginxKey + agent_config.KeyDelimiter + "config_reload_monitoring_period" NginxTreatWarningsAsErrors = NginxKey + agent_config.KeyDelimiter + "treat_warnings_as_errors" - NginxCa = NginxKey + agent_config.KeyDelimiter + "ca" + NginxApiTlsCa = NginxKey + agent_config.KeyDelimiter + "api_tls_ca" // viper keys used in config DataplaneKey = "dataplane" @@ -324,9 +326,9 @@ var ( DefaultValue: Defaults.Nginx.TreatWarningsAsErrors, }, &StringFlag{ - Name: NginxCa, + Name: NginxApiTlsCa, Usage: "The NGINX Plus CA certificate file location needed to call the NGINX Plus API if SSL is enabled.", - DefaultValue: Defaults.Nginx.Ca, + DefaultValue: Defaults.Nginx.ApiTls.Ca, }, // Metrics &DurationFlag{ diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/types.go b/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/types.go index a4e0c6a6e..4b57a5c6d 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/types.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/core/config/types.go @@ -155,7 +155,7 @@ type Nginx struct { NginxClientVersion int `mapstructure:"client_version" yaml:"-"` ConfigReloadMonitoringPeriod time.Duration `mapstructure:"config_reload_monitoring_period" yaml:"-"` TreatWarningsAsErrors bool `mapstructure:"treat_warnings_as_errors" yaml:"-"` - Ca string `mapstructure:"ca" yaml:"-"` + ApiTls TLSConfig `mapstructure:"api_tls" yaml:"-"` } type Dataplane struct { diff --git a/test/performance/vendor/github.com/nginx/agent/v2/src/core/metrics/sources/nginx_plus.go b/test/performance/vendor/github.com/nginx/agent/v2/src/core/metrics/sources/nginx_plus.go index 2b6bb1187..1077fdecc 100644 --- a/test/performance/vendor/github.com/nginx/agent/v2/src/core/metrics/sources/nginx_plus.go +++ b/test/performance/vendor/github.com/nginx/agent/v2/src/core/metrics/sources/nginx_plus.go @@ -87,8 +87,8 @@ func NewNginxPlus(baseDimensions *metrics.CommonDim, nginxNamespace, plusNamespa client := http.DefaultClient - if conf.Nginx.Ca != "" && conf.IsFileAllowed(conf.Nginx.Ca) { - data, err := os.ReadFile(conf.Nginx.Ca) + if conf.Nginx.ApiTls.Ca != "" && conf.IsFileAllowed(conf.Nginx.ApiTls.Ca) { + data, err := os.ReadFile(conf.Nginx.ApiTls.Ca) if err != nil { log.Errorf("Unable to collect NGINX Plus metrics. Failed to read NGINX CA certificate file: %v", err) return nil