Skip to content

Commit 84ca8cd

Browse files
authored
Add check to see if SSL is enabled for Plus API (#1139)
1 parent fb0c657 commit 84ca8cd

File tree

17 files changed

+222
-25
lines changed

17 files changed

+222
-25
lines changed

sdk/config_helpers.go

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,10 @@ import (
3939
const (
4040
plusAPIDirective = "api"
4141
stubStatusAPIDirective = "stub_status"
42-
apiFormat = "http://%s%s"
4342
predefinedAccessLogFormat = "$remote_addr - $remote_user [$time_local] \"$request\" $status $body_bytes_sent \"$http_referer\" \"$http_user_agent\""
4443
httpClientTimeout = 1 * time.Second
44+
httpPrefix = "http://"
45+
httpsPrefix = "https://"
4546
)
4647

4748
var readLock = sync.Mutex{}
@@ -698,6 +699,7 @@ func parseAddressesFromServerDirective(parent *crossplane.Directive) []string {
698699

699700
for _, dir := range parent.Block {
700701
hostname := "127.0.0.1"
702+
prefix := httpPrefix
701703

702704
switch dir.Directive {
703705
case "listen":
@@ -718,14 +720,22 @@ func parseAddressesFromServerDirective(parent *crossplane.Directive) []string {
718720
hostname = dir.Args[0]
719721
}
720722
}
721-
hosts = append(hosts, hostname)
723+
724+
if len(dir.Args) > 1 {
725+
secondArg := dir.Args[1]
726+
if secondArg == "ssl" {
727+
prefix = httpsPrefix
728+
}
729+
}
730+
731+
hosts = append(hosts, prefix+hostname)
722732
case "server_name":
723733
if dir.Args[0] == "_" {
724734
// default server
725735
continue
726736
}
727737
hostname = dir.Args[0]
728-
hosts = append(hosts, hostname)
738+
hosts = append(hosts, prefix+hostname)
729739
}
730740
}
731741

@@ -952,11 +962,11 @@ func getUrlsForLocationDirective(parent *crossplane.Directive, current *crosspla
952962
addresses := parseAddressesFromServerDirective(parent)
953963

954964
for _, address := range addresses {
955-
path := parsePathFromLocationDirective(current)
965+
pathFromLocationDirective := parsePathFromLocationDirective(current)
956966

957967
switch locChild.Directive {
958968
case locationDirectiveName:
959-
urls = append(urls, fmt.Sprintf(apiFormat, address, path))
969+
urls = append(urls, address+pathFromLocationDirective)
960970
}
961971
}
962972
}

src/core/config/config.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,9 @@ func getNginx() Nginx {
354354
NginxClientVersion: Viper.GetInt(NginxClientVersion),
355355
ConfigReloadMonitoringPeriod: Viper.GetDuration(NginxConfigReloadMonitoringPeriod),
356356
TreatWarningsAsErrors: Viper.GetBool(NginxTreatWarningsAsErrors),
357+
ApiTls: TLSConfig{
358+
Ca: Viper.GetString(NginxApiTlsCa),
359+
},
357360
}
358361
}
359362

src/core/config/defaults.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,9 @@ var (
6666
NginxClientVersion: 7, // NGINX Plus R25+
6767
ConfigReloadMonitoringPeriod: 10 * time.Second,
6868
TreatWarningsAsErrors: false,
69+
ApiTls: TLSConfig{
70+
Ca: "",
71+
},
6972
},
7073
ConfigDirs: "/etc/nginx:/usr/local/etc/nginx:/usr/share/nginx/modules:/etc/nms",
7174
IgnoreDirectives: []string{},
@@ -175,6 +178,7 @@ const (
175178
NginxClientVersion = NginxKey + agent_config.KeyDelimiter + "client_version"
176179
NginxConfigReloadMonitoringPeriod = NginxKey + agent_config.KeyDelimiter + "config_reload_monitoring_period"
177180
NginxTreatWarningsAsErrors = NginxKey + agent_config.KeyDelimiter + "treat_warnings_as_errors"
181+
NginxApiTlsCa = NginxKey + agent_config.KeyDelimiter + "api_tls_ca"
178182

179183
// viper keys used in config
180184
DataplaneKey = "dataplane"
@@ -321,6 +325,11 @@ var (
321325
Usage: "On nginx -t, treat warnings as failures on configuration application.",
322326
DefaultValue: Defaults.Nginx.TreatWarningsAsErrors,
323327
},
328+
&StringFlag{
329+
Name: NginxApiTlsCa,
330+
Usage: "The NGINX Plus CA certificate file location needed to call the NGINX Plus API if SSL is enabled.",
331+
DefaultValue: Defaults.Nginx.ApiTls.Ca,
332+
},
324333
// Metrics
325334
&DurationFlag{
326335
Name: MetricsCollectionInterval,

src/core/config/types.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
package config
99

1010
import (
11+
"path/filepath"
12+
"strings"
1113
"time"
1214

1315
"github.com/nginx/agent/sdk/v2/backoff"
@@ -90,6 +92,20 @@ func (c *Config) GetMetricsBackoffSettings() backoff.BackoffSettings {
9092
}
9193
}
9294

95+
func (c *Config) IsFileAllowed(path string) bool {
96+
if !filepath.IsAbs(path) {
97+
return false
98+
}
99+
100+
for dir := range c.AllowedDirectoriesMap {
101+
if strings.HasPrefix(path, dir) {
102+
return true
103+
}
104+
}
105+
106+
return false
107+
}
108+
93109
type Server struct {
94110
Host string `mapstructure:"host" yaml:"-"`
95111
GrpcPort int `mapstructure:"grpcPort" yaml:"-"`
@@ -139,6 +155,7 @@ type Nginx struct {
139155
NginxClientVersion int `mapstructure:"client_version" yaml:"-"`
140156
ConfigReloadMonitoringPeriod time.Duration `mapstructure:"config_reload_monitoring_period" yaml:"-"`
141157
TreatWarningsAsErrors bool `mapstructure:"treat_warnings_as_errors" yaml:"-"`
158+
ApiTls TLSConfig `mapstructure:"api_tls" yaml:"-"`
142159
}
143160

144161
type Dataplane struct {

src/core/metrics/collectors/nginx.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,10 @@ func buildSources(dimensions *metrics.CommonDim, binary core.NginxBinary, collec
6161
nginxSources = append(nginxSources, sources.NewNginxAccessLog(dimensions, sources.OSSNamespace, binary, sources.OSSNginxType, collectorConf.CollectionInterval))
6262
nginxSources = append(nginxSources, sources.NewNginxErrorLog(dimensions, sources.OSSNamespace, binary, sources.OSSNginxType, collectorConf.CollectionInterval))
6363
} else if collectorConf.PlusAPI != "" {
64-
nginxSources = append(nginxSources, sources.NewNginxPlus(dimensions, sources.OSSNamespace, sources.PlusNamespace, collectorConf.PlusAPI, collectorConf.ClientVersion))
64+
nginxPlusSource := sources.NewNginxPlus(dimensions, sources.OSSNamespace, sources.PlusNamespace, collectorConf.PlusAPI, collectorConf.ClientVersion, conf)
65+
if nginxPlusSource != nil {
66+
nginxSources = append(nginxSources, nginxPlusSource)
67+
}
6568
nginxSources = append(nginxSources, sources.NewNginxAccessLog(dimensions, sources.OSSNamespace, binary, sources.PlusNginxType, collectorConf.CollectionInterval))
6669
nginxSources = append(nginxSources, sources.NewNginxErrorLog(dimensions, sources.OSSNamespace, binary, sources.PlusNginxType, collectorConf.CollectionInterval))
6770
} else {

src/core/metrics/sources/nginx_plus.go

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,16 @@ package sources
99

1010
import (
1111
"context"
12+
"crypto/tls"
13+
"crypto/x509"
1214
"fmt"
1315
"math"
16+
"net/http"
17+
"os"
1418
"sync"
1519

20+
"github.com/nginx/agent/v2/src/core/config"
21+
1622
"github.com/nginx/agent/sdk/v2/proto"
1723
"github.com/nginx/agent/v2/src/core/metrics"
1824

@@ -67,6 +73,7 @@ type NginxPlus struct {
6773
init sync.Once
6874
clientVersion int
6975
logger *MetricSourceLogger
76+
client *http.Client
7077
}
7178

7279
type ExtendedStats struct {
@@ -75,14 +82,44 @@ type ExtendedStats struct {
7582
streamEndpoints []string
7683
}
7784

78-
func NewNginxPlus(baseDimensions *metrics.CommonDim, nginxNamespace, plusNamespace, plusAPI string, clientVersion int) *NginxPlus {
85+
func NewNginxPlus(baseDimensions *metrics.CommonDim, nginxNamespace, plusNamespace, plusAPI string, clientVersion int, conf *config.Config) *NginxPlus {
7986
log.Debug("Creating NGINX Plus metrics source")
80-
return &NginxPlus{baseDimensions: baseDimensions, nginxNamespace: nginxNamespace, plusNamespace: plusNamespace, plusAPI: plusAPI, clientVersion: clientVersion, logger: NewMetricSourceLogger()}
87+
88+
client := http.DefaultClient
89+
90+
if conf.Nginx.ApiTls.Ca != "" && conf.IsFileAllowed(conf.Nginx.ApiTls.Ca) {
91+
data, err := os.ReadFile(conf.Nginx.ApiTls.Ca)
92+
if err != nil {
93+
log.Errorf("Unable to collect NGINX Plus metrics. Failed to read NGINX CA certificate file: %v", err)
94+
return nil
95+
}
96+
97+
caCertPool := x509.NewCertPool()
98+
caCertPool.AppendCertsFromPEM(data)
99+
100+
client = &http.Client{
101+
Transport: &http.Transport{
102+
TLSClientConfig: &tls.Config{
103+
RootCAs: caCertPool,
104+
},
105+
},
106+
}
107+
}
108+
109+
return &NginxPlus{
110+
baseDimensions: baseDimensions,
111+
nginxNamespace: nginxNamespace,
112+
plusNamespace: plusNamespace,
113+
plusAPI: plusAPI,
114+
clientVersion: clientVersion,
115+
logger: NewMetricSourceLogger(),
116+
client: client,
117+
}
81118
}
82119

83120
func (c *NginxPlus) Collect(ctx context.Context, m chan<- *metrics.StatsEntityWrapper) {
84121
c.init.Do(func() {
85-
cl, err := plusclient.NewNginxClient(c.plusAPI, plusclient.WithMaxAPIVersion())
122+
cl, err := plusclient.NewNginxClient(c.plusAPI, plusclient.WithMaxAPIVersion(), plusclient.WithHTTPClient(c.client))
86123
if err != nil {
87124
c.logger.Log(fmt.Sprintf("Failed to create plus metrics client: %v", err))
88125
SendNginxDownStatus(ctx, c.baseDimensions.ToDimensions(), m)

src/core/metrics/sources/nginx_plus_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -570,7 +570,7 @@ func (f *FakeNginxPlus) Collect(ctx context.Context, m chan<- *metrics.StatsEnti
570570
}
571571

572572
func TestNginxPlusUpdate(t *testing.T) {
573-
nginxPlus := NewNginxPlus(&metrics.CommonDim{}, "test", PlusNamespace, "http://localhost:8080/api", 6)
573+
nginxPlus := NewNginxPlus(&metrics.CommonDim{}, "test", PlusNamespace, "http://localhost:8080/api", 6, &config.Config{})
574574

575575
assert.Equal(t, "", nginxPlus.baseDimensions.InstanceTags)
576576
assert.Equal(t, "http://localhost:8080/api", nginxPlus.plusAPI)
@@ -867,7 +867,7 @@ func TestNginxPlus_Collect(t *testing.T) {
867867
for _, test := range tests {
868868
ctx := context.TODO()
869869

870-
f := &FakeNginxPlus{NewNginxPlus(test.baseDimensions, "nginx", "plus", "", 6)}
870+
f := &FakeNginxPlus{NewNginxPlus(test.baseDimensions, "nginx", "plus", "", 6, &config.Config{})}
871871
go f.Collect(ctx, test.m)
872872

873873
instanceMetrics := <-test.m

test/integration/vendor/github.com/nginx/agent/sdk/v2/config_helpers.go

Lines changed: 15 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/integration/vendor/github.com/nginx/agent/v2/src/core/config/config.go

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/integration/vendor/github.com/nginx/agent/v2/src/core/config/defaults.go

Lines changed: 9 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)