Skip to content

Commit 62ab46e

Browse files
authored
Add agent config option to set NGINX API URL (#1486)
1 parent e5160cc commit 62ab46e

File tree

15 files changed

+331
-58
lines changed

15 files changed

+331
-58
lines changed

src/core/config/config.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -354,8 +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),
357+
API: &NginxAPI{
358+
URL: Viper.GetString(NginxApiURLKey),
359+
TLS: TLSConfig{Ca: Viper.GetString(NginxApiTlsCa)},
359360
},
360361
}
361362
}

src/core/config/defaults.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,11 @@ var (
6666
NginxClientVersion: 7, // NGINX Plus R25+
6767
ConfigReloadMonitoringPeriod: 10 * time.Second,
6868
TreatWarningsAsErrors: false,
69-
ApiTls: TLSConfig{
70-
Ca: "",
69+
API: &NginxAPI{
70+
URL: "",
71+
TLS: TLSConfig{
72+
Ca: "",
73+
},
7174
},
7275
},
7376
ConfigDirs: "/etc/nginx:/usr/local/etc/nginx:/usr/share/nginx/modules:/etc/nms",
@@ -179,6 +182,7 @@ const (
179182
NginxConfigReloadMonitoringPeriod = NginxKey + agent_config.KeyDelimiter + "config_reload_monitoring_period"
180183
NginxTreatWarningsAsErrors = NginxKey + agent_config.KeyDelimiter + "treat_warnings_as_errors"
181184
NginxApiTlsCa = NginxKey + agent_config.KeyDelimiter + "api_tls_ca"
185+
NginxApiURLKey = NginxKey + agent_config.KeyDelimiter + "api_url"
182186

183187
// viper keys used in config
184188
DataplaneKey = "dataplane"
@@ -325,10 +329,15 @@ var (
325329
Usage: "On nginx -t, treat warnings as failures on configuration application.",
326330
DefaultValue: Defaults.Nginx.TreatWarningsAsErrors,
327331
},
332+
&StringFlag{
333+
Name: NginxApiURLKey,
334+
Usage: "The NGINX Plus API URL.",
335+
DefaultValue: Defaults.Nginx.API.URL,
336+
},
328337
&StringFlag{
329338
Name: NginxApiTlsCa,
330339
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,
340+
DefaultValue: Defaults.Nginx.API.TLS.Ca,
332341
},
333342
// Metrics
334343
&DurationFlag{

src/core/config/types.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,12 @@ type Nginx struct {
155155
NginxClientVersion int `mapstructure:"client_version" yaml:"-"`
156156
ConfigReloadMonitoringPeriod time.Duration `mapstructure:"config_reload_monitoring_period" yaml:"-"`
157157
TreatWarningsAsErrors bool `mapstructure:"treat_warnings_as_errors" yaml:"-"`
158-
ApiTls TLSConfig `mapstructure:"api_tls" yaml:"-"`
158+
API *NginxAPI `mapstructure:"api"`
159+
}
160+
161+
type NginxAPI struct {
162+
URL string `mapstructure:"url"`
163+
TLS TLSConfig `mapstructure:"tls"`
159164
}
160165

161166
type Dataplane struct {

src/core/metrics/sources/nginx_plus.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,8 @@ func NewNginxPlus(baseDimensions *metrics.CommonDim, nginxNamespace, plusNamespa
8787

8888
client := http.DefaultClient
8989

90-
if conf.Nginx.ApiTls.Ca != "" && conf.IsFileAllowed(conf.Nginx.ApiTls.Ca) {
91-
data, err := os.ReadFile(conf.Nginx.ApiTls.Ca)
90+
if conf.Nginx.API != nil && conf.Nginx.API.TLS.Ca != "" && conf.IsFileAllowed(conf.Nginx.API.TLS.Ca) {
91+
data, err := os.ReadFile(conf.Nginx.API.TLS.Ca)
9292
if err != nil {
9393
log.Errorf("Unable to collect NGINX Plus metrics. Failed to read NGINX CA certificate file: %v", err)
9494
return nil

src/core/nginx.go

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -220,20 +220,25 @@ func (n *NginxBinaryType) GetNginxDetailsFromProcess(nginxProcess *Process) *pro
220220
n.statusUrlMutex.RUnlock()
221221

222222
if urlsLength == 0 || nginxStatus == "" {
223-
stubStatusApiUrl, err := sdk.GetStubStatusApiUrl(nginxDetailsFacade.ConfPath, n.config.IgnoreDirectives)
224-
if err != nil {
225-
log.Tracef("Unable to get Stub Status API URL from the configuration: NGINX OSS metrics will be unavailable for this system. please configure a Stub Status API to get NGINX OSS metrics: %v", err)
226-
}
223+
// If NGINX API URL is configured in agent config then use that istead of discovering the URL from the NGINX configuration
224+
if n.config.Nginx.API != nil && n.config.Nginx.API.URL != "" {
225+
nginxDetailsFacade.StatusUrl = n.config.Nginx.API.URL
226+
} else {
227+
stubStatusApiUrl, err := sdk.GetStubStatusApiUrl(nginxDetailsFacade.ConfPath, n.config.IgnoreDirectives)
228+
if err != nil {
229+
log.Tracef("Unable to get Stub Status API URL from the configuration: NGINX OSS metrics will be unavailable for this system. please configure a Stub Status API to get NGINX OSS metrics: %v", err)
230+
}
227231

228-
nginxPlusApiUrl, err := sdk.GetNginxPlusApiUrl(nginxDetailsFacade.ConfPath, n.config.IgnoreDirectives)
229-
if err != nil {
230-
log.Tracef("Unable to get NGINX Plus API URL from the configuration: NGINX Plus metrics will be unavailable for this system. please configure a NGINX Plus API to get NGINX Plus metrics: %v", err)
231-
}
232+
nginxPlusApiUrl, err := sdk.GetNginxPlusApiUrl(nginxDetailsFacade.ConfPath, n.config.IgnoreDirectives)
233+
if err != nil {
234+
log.Tracef("Unable to get NGINX Plus API URL from the configuration: NGINX Plus metrics will be unavailable for this system. please configure a NGINX Plus API to get NGINX Plus metrics: %v", err)
235+
}
232236

233-
if nginxDetailsFacade.Plus.Enabled {
234-
nginxDetailsFacade.StatusUrl = nginxPlusApiUrl
235-
} else {
236-
nginxDetailsFacade.StatusUrl = stubStatusApiUrl
237+
if nginxDetailsFacade.Plus.Enabled {
238+
nginxDetailsFacade.StatusUrl = nginxPlusApiUrl
239+
} else {
240+
nginxDetailsFacade.StatusUrl = stubStatusApiUrl
241+
}
237242
}
238243

239244
n.statusUrlMutex.Lock()

src/core/nginx_test.go

Lines changed: 213 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"path/filepath"
1818
"strings"
1919
"testing"
20+
"time"
2021

2122
"github.com/nginx/agent/sdk/v2/proto"
2223
"github.com/nginx/agent/sdk/v2/zip"
@@ -1499,3 +1500,215 @@ func TestGenerateActionMaps(t *testing.T) {
14991500
})
15001501
}
15011502
}
1503+
1504+
func TestGetNginxDetailsFromProcess(t *testing.T) {
1505+
// Test the core parsing and transformation logic by using the lower-level functions
1506+
testCases := []struct {
1507+
name string
1508+
process *Process
1509+
nginxInfo *nginxInfo
1510+
config *config.Config
1511+
expectedVersion string
1512+
expectedConfPath string
1513+
expectedProcessId string
1514+
expectedPlus bool
1515+
expectedPlusVersion string
1516+
expectedStatusUrl string
1517+
}{
1518+
{
1519+
name: "Test 1: OSS nginx process",
1520+
process: &Process{
1521+
Pid: 1234,
1522+
CreateTime: 1640995200,
1523+
Path: "/usr/sbin/nginx",
1524+
Command: "/usr/sbin/nginx",
1525+
IsMaster: true,
1526+
},
1527+
nginxInfo: &nginxInfo{
1528+
version: "1.20.2",
1529+
plusver: "",
1530+
source: "built by gcc 7.5.0",
1531+
prefix: "/etc/nginx",
1532+
confPath: "/etc/nginx/nginx.conf",
1533+
ssl: []string{"OpenSSL", "1.1.1f", "31 Mar 2020"},
1534+
cfgf: map[string]interface{}{
1535+
"prefix": "/etc/nginx",
1536+
"conf-path": "/etc/nginx/nginx.conf",
1537+
},
1538+
configureArgs: []string{"--prefix=/etc/nginx", "--with-http_ssl_module"},
1539+
mtime: time.Now(),
1540+
},
1541+
config: &config.Config{
1542+
Nginx: config.Nginx{},
1543+
IgnoreDirectives: []string{},
1544+
},
1545+
expectedVersion: "1.20.2",
1546+
expectedConfPath: "/etc/nginx/nginx.conf",
1547+
expectedProcessId: "1234",
1548+
expectedPlus: false,
1549+
expectedPlusVersion: "",
1550+
expectedStatusUrl: "", // Empty because no stub_status configured
1551+
},
1552+
{
1553+
name: "Test 2: Nginx Plus process",
1554+
process: &Process{
1555+
Pid: 5678,
1556+
CreateTime: 1640995300,
1557+
Path: "/usr/sbin/nginx",
1558+
Command: "/usr/sbin/nginx",
1559+
IsMaster: true,
1560+
},
1561+
nginxInfo: &nginxInfo{
1562+
version: "1.21.6",
1563+
plusver: "R26",
1564+
source: "built by gcc 9.4.0",
1565+
prefix: "/etc/nginx",
1566+
confPath: "/etc/nginx/nginx.conf",
1567+
ssl: []string{"OpenSSL", "1.1.1k", "25 Mar 2021"},
1568+
cfgf: map[string]interface{}{
1569+
"prefix": "/etc/nginx",
1570+
"conf-path": "/etc/nginx/nginx.conf",
1571+
},
1572+
configureArgs: []string{"--prefix=/etc/nginx", "--with-http_v2_module"},
1573+
mtime: time.Now(),
1574+
},
1575+
config: &config.Config{
1576+
Nginx: config.Nginx{},
1577+
IgnoreDirectives: []string{},
1578+
},
1579+
expectedVersion: "1.21.6",
1580+
expectedConfPath: "/etc/nginx/nginx.conf",
1581+
expectedProcessId: "5678",
1582+
expectedPlus: true,
1583+
expectedPlusVersion: "R26",
1584+
expectedStatusUrl: "", // Empty because no Plus API configured
1585+
},
1586+
{
1587+
name: "Test 3: Process with custom conf path from command line",
1588+
process: &Process{
1589+
Pid: 9012,
1590+
CreateTime: 1640995400,
1591+
Path: "/opt/nginx/sbin/nginx",
1592+
Command: "/opt/nginx/sbin/nginx -c /custom/nginx.conf",
1593+
IsMaster: true,
1594+
},
1595+
nginxInfo: &nginxInfo{
1596+
version: "1.22.1",
1597+
plusver: "",
1598+
prefix: "/opt/nginx",
1599+
confPath: "/opt/nginx/conf/nginx.conf",
1600+
ssl: []string{"BoringSSL"},
1601+
cfgf: map[string]interface{}{
1602+
"prefix": "/opt/nginx",
1603+
"conf-path": "/opt/nginx/conf/nginx.conf",
1604+
},
1605+
configureArgs: []string{"--prefix=/opt/nginx"},
1606+
mtime: time.Now(),
1607+
},
1608+
config: &config.Config{
1609+
Nginx: config.Nginx{},
1610+
IgnoreDirectives: []string{},
1611+
},
1612+
expectedVersion: "1.22.1",
1613+
expectedConfPath: "/custom/nginx.conf", // Should be overridden by command line
1614+
expectedProcessId: "9012",
1615+
expectedPlus: false,
1616+
expectedPlusVersion: "",
1617+
expectedStatusUrl: "", // Empty because no stub_status configured
1618+
},
1619+
{
1620+
name: "Test 4: Process with configured API URL",
1621+
process: &Process{
1622+
Pid: 1111,
1623+
CreateTime: 1640995500,
1624+
Path: "/usr/sbin/nginx",
1625+
Command: "/usr/sbin/nginx",
1626+
IsMaster: true,
1627+
},
1628+
nginxInfo: &nginxInfo{
1629+
version: "1.20.2",
1630+
plusver: "",
1631+
source: "built by gcc 7.5.0",
1632+
prefix: "/etc/nginx",
1633+
confPath: "/etc/nginx/nginx.conf",
1634+
ssl: []string{"OpenSSL", "1.1.1f", "31 Mar 2020"},
1635+
cfgf: map[string]interface{}{
1636+
"prefix": "/etc/nginx",
1637+
"conf-path": "/etc/nginx/nginx.conf",
1638+
},
1639+
configureArgs: []string{"--prefix=/etc/nginx", "--with-http_ssl_module"},
1640+
mtime: time.Now(),
1641+
},
1642+
config: &config.Config{
1643+
Nginx: config.Nginx{
1644+
API: &config.NginxAPI{
1645+
URL: "http://127.0.0.1:8080/api",
1646+
},
1647+
},
1648+
IgnoreDirectives: []string{},
1649+
},
1650+
expectedVersion: "1.20.2",
1651+
expectedConfPath: "/etc/nginx/nginx.conf",
1652+
expectedProcessId: "1111",
1653+
expectedPlus: false,
1654+
expectedPlusVersion: "",
1655+
expectedStatusUrl: "http://127.0.0.1:8080/api", // From agent config
1656+
},
1657+
}
1658+
1659+
for _, tc := range testCases {
1660+
t.Run(tc.name, func(t *testing.T) {
1661+
// Create a function to test the core logic without file system dependencies
1662+
nginxBinary := &NginxBinaryType{
1663+
config: tc.config,
1664+
statusUrls: make(map[string]string),
1665+
}
1666+
1667+
// Mock the getNginxInfoFrom method by creating a wrapper that returns our test data
1668+
originalPath := tc.process.Path
1669+
1670+
// Create temporary file to satisfy the os.Stat call in the cached path
1671+
tmpFile := filepath.Join(t.TempDir(), "nginx")
1672+
err := os.WriteFile(tmpFile, []byte(""), 0o755)
1673+
require.NoError(t, err)
1674+
1675+
// Set modification time to match our test data
1676+
err = os.Chtimes(tmpFile, tc.nginxInfo.mtime, tc.nginxInfo.mtime)
1677+
require.NoError(t, err)
1678+
1679+
// Update process path and cache the nginx info to avoid nginx -V call
1680+
tc.process.Path = tmpFile
1681+
if nginxBinary.nginxInfoMap == nil {
1682+
nginxBinary.nginxInfoMap = make(map[string]*nginxInfo)
1683+
}
1684+
nginxBinary.nginxInfoMap[tmpFile] = tc.nginxInfo
1685+
1686+
// Call the function under test
1687+
result := nginxBinary.GetNginxDetailsFromProcess(tc.process)
1688+
1689+
// Restore original path for consistent testing
1690+
tc.process.Path = originalPath
1691+
1692+
// Validate core fields
1693+
assert.Equal(t, tc.expectedVersion, result.Version)
1694+
assert.Equal(t, tc.expectedConfPath, result.ConfPath)
1695+
assert.Equal(t, tc.expectedProcessId, result.ProcessId)
1696+
1697+
// Test Plus-specific assertions
1698+
require.NotNil(t, result.Plus)
1699+
assert.Equal(t, tc.expectedPlus, result.Plus.Enabled)
1700+
assert.Equal(t, tc.expectedPlusVersion, result.Plus.Release)
1701+
1702+
// Validate StatusUrl
1703+
assert.Equal(t, tc.expectedStatusUrl, result.StatusUrl)
1704+
1705+
// Validate NginxId is generated
1706+
assert.NotEmpty(t, result.NginxId)
1707+
1708+
// Validate other core fields are set correctly
1709+
assert.Equal(t, fmt.Sprintf("%d", tc.process.Pid), result.ProcessId)
1710+
assert.Equal(t, tc.process.CreateTime, result.StartTime)
1711+
assert.Equal(t, false, result.BuiltFromSource) // Default value
1712+
})
1713+
}
1714+
}

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

Lines changed: 3 additions & 2 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: 12 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)