diff --git a/cmd/nginx-ingress/main.go b/cmd/nginx-ingress/main.go index 40339c46e7..8bc440e1c5 100644 --- a/cmd/nginx-ingress/main.go +++ b/cmd/nginx-ingress/main.go @@ -79,7 +79,7 @@ func main() { appProtectVersion = getAppProtectVersionInfo() } - updateSelfWithVersionInfo(kubeClient, version, nginxVersion, appProtectVersion) + updateSelfWithVersionInfo(kubeClient, version, nginxVersion.String(), appProtectVersion) templateExecutor, templateExecutorV2 := createTemplateExecutors() @@ -118,6 +118,7 @@ func main() { EnableCertManager: *enableCertManager, DynamicSSLReload: *enableDynamicSSLReload, StaticSSLPath: nginxManager.GetSecretsDir(), + NginxVersion: nginxVersion, } processNginxConfig(staticCfgParams, cfgParams, templateExecutor, nginxManager) @@ -146,6 +147,7 @@ func main() { IsPrometheusEnabled: *enablePrometheusMetrics, IsLatencyMetricsEnabled: *enableLatencyMetrics, IsDynamicSSLReloadEnabled: *enableDynamicSSLReload, + NginxVersion: nginxVersion, }) controllerNamespace := os.Getenv("POD_NAMESPACE") @@ -400,17 +402,16 @@ func createNginxManager(managerCollector collectors.ManagerCollector) (nginx.Man return nginxManager, useFakeNginxManager } -func getNginxVersionInfo(nginxManager nginx.Manager) string { - nginxVersion := nginxManager.Version() - isPlus := strings.Contains(nginxVersion, "plus") - glog.Infof("Using %s", nginxVersion) +func getNginxVersionInfo(nginxManager nginx.Manager) nginx.Version { + nginxInfo := nginxManager.Version() + glog.Infof("Using %s", nginxInfo.String()) - if *nginxPlus && !isPlus { + if *nginxPlus && !nginxInfo.IsPlus { glog.Fatal("NGINX Plus flag enabled (-nginx-plus) without NGINX Plus binary") - } else if !*nginxPlus && isPlus { + } else if !*nginxPlus && nginxInfo.IsPlus { glog.Fatal("NGINX Plus binary found without NGINX Plus flag (-nginx-plus)") } - return nginxVersion + return nginxInfo } func getAppProtectVersionInfo() string { diff --git a/internal/configs/config_params.go b/internal/configs/config_params.go index f76a944663..908463024e 100644 --- a/internal/configs/config_params.go +++ b/internal/configs/config_params.go @@ -1,6 +1,9 @@ package configs -import conf_v1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1" +import ( + "github.com/nginxinc/kubernetes-ingress/internal/nginx" + conf_v1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1" +) // ConfigParams holds NGINX configuration parameters that affect the main NGINX config // as well as configs for Ingress resources. @@ -136,6 +139,7 @@ type StaticConfigParams struct { EnableCertManager bool DynamicSSLReload bool StaticSSLPath string + NginxVersion nginx.Version } // GlobalConfigParams holds global configuration parameters. For now, it only holds listeners. diff --git a/internal/configs/configmaps.go b/internal/configs/configmaps.go index 0b653408f9..fbcd63b68d 100644 --- a/internal/configs/configmaps.go +++ b/internal/configs/configmaps.go @@ -581,6 +581,7 @@ func GenerateNginxMainConfig(staticCfgParams *StaticConfigParams, config *Config OIDC: staticCfgParams.EnableOIDC, DynamicSSLReloadEnabled: staticCfgParams.DynamicSSLReload, StaticSSLPath: staticCfgParams.StaticSSLPath, + NginxVersion: staticCfgParams.NginxVersion, } return nginxCfg } diff --git a/internal/configs/configurator.go b/internal/configs/configurator.go index 4d03c03be9..8574589ae4 100644 --- a/internal/configs/configurator.go +++ b/internal/configs/configurator.go @@ -145,6 +145,7 @@ type ConfiguratorParams struct { IsWildcardEnabled bool IsLatencyMetricsEnabled bool IsDynamicSSLReloadEnabled bool + NginxVersion nginx.Version } // NewConfigurator creates a new Configurator. diff --git a/internal/configs/configurator_test.go b/internal/configs/configurator_test.go index a20240f438..6a9720494c 100644 --- a/internal/configs/configurator_test.go +++ b/internal/configs/configurator_test.go @@ -25,6 +25,7 @@ func createTestStaticConfigParams() *StaticConfigParams { NginxStatusAllowCIDRs: []string{"127.0.0.1"}, NginxStatusPort: 8080, StubStatusOverUnixSocketForOSS: false, + NginxVersion: nginx.NewVersion("nginx version: nginx/1.25.3 (nginx-plus-r31)"), } } @@ -53,6 +54,7 @@ func createTestConfigurator(t *testing.T) *Configurator { IsWildcardEnabled: false, IsPrometheusEnabled: false, IsLatencyMetricsEnabled: false, + NginxVersion: nginx.NewVersion("nginx version: nginx/1.25.3 (nginx-plus-r31)"), }) cnf.isReloadsEnabled = true return cnf diff --git a/internal/configs/version1/config.go b/internal/configs/version1/config.go index e1569a7bdb..888c19b336 100644 --- a/internal/configs/version1/config.go +++ b/internal/configs/version1/config.go @@ -1,5 +1,7 @@ package version1 +import "github.com/nginxinc/kubernetes-ingress/internal/nginx" + // UpstreamLabels describes the Prometheus labels for an NGINX upstream. type UpstreamLabels struct { Service string @@ -234,6 +236,7 @@ type MainConfig struct { OIDC bool DynamicSSLReloadEnabled bool StaticSSLPath string + NginxVersion nginx.Version } // NewUpstreamWithDefaultServer creates an upstream with the default server. diff --git a/internal/configs/version1/nginx-plus.tmpl b/internal/configs/version1/nginx-plus.tmpl index cb7cbd8706..0dbef3471f 100644 --- a/internal/configs/version1/nginx-plus.tmpl +++ b/internal/configs/version1/nginx-plus.tmpl @@ -346,3 +346,9 @@ stream { include /etc/nginx/stream-conf.d/*.conf; } + +{{- if (.NginxVersion.PlusGreaterThanOrEqualTo "nginx-plus-r31") }} +mgmt { + usage_report interval=0s; +} +{{- end}} diff --git a/internal/configs/version1/template_test.go b/internal/configs/version1/template_test.go index 395dcd77a8..9172863df2 100644 --- a/internal/configs/version1/template_test.go +++ b/internal/configs/version1/template_test.go @@ -5,6 +5,8 @@ import ( "strings" "testing" "text/template" + + "github.com/nginxinc/kubernetes-ingress/internal/nginx" ) func TestExecuteMainTemplateForNGINXPlus(t *testing.T) { @@ -20,6 +22,19 @@ func TestExecuteMainTemplateForNGINXPlus(t *testing.T) { t.Log(buf.String()) } +func TestExecuteMainTemplateForNGINXPlusR31(t *testing.T) { + t.Parallel() + + tmpl := newNGINXPlusMainTmpl(t) + buf := &bytes.Buffer{} + + err := tmpl.Execute(buf, mainCfgR31) + if err != nil { + t.Error(err) + } + t.Log(buf.String()) +} + func TestExecuteMainTemplateForNGINX(t *testing.T) { t.Parallel() @@ -1291,6 +1306,33 @@ var ( KeepaliveRequests: 100, VariablesHashBucketSize: 256, VariablesHashMaxSize: 1024, + NginxVersion: nginx.NewVersion("nginx version: nginx/1.25.3 (nginx-plus-r31)"), + } + + mainCfgR31 = MainConfig{ + DefaultHTTPListenerPort: 80, + DefaultHTTPSListenerPort: 443, + ServerNamesHashMaxSize: "512", + ServerTokens: "off", + WorkerProcesses: "auto", + WorkerCPUAffinity: "auto", + WorkerShutdownTimeout: "1m", + WorkerConnections: "1024", + WorkerRlimitNofile: "65536", + LogFormat: []string{"$remote_addr", "$remote_user"}, + LogFormatEscaping: "default", + StreamSnippets: []string{"# comment"}, + StreamLogFormat: []string{"$remote_addr", "$remote_user"}, + StreamLogFormatEscaping: "none", + ResolverAddresses: []string{"example.com", "127.0.0.1"}, + ResolverIPV6: false, + ResolverValid: "10s", + ResolverTimeout: "15s", + KeepaliveTimeout: "65s", + KeepaliveRequests: 100, + VariablesHashBucketSize: 256, + VariablesHashMaxSize: 1024, + NginxVersion: nginx.NewVersion("nginx version: nginx/1.25.3 (nginx-plus-r31)"), } mainCfgHTTP2On = MainConfig{ @@ -1317,6 +1359,7 @@ var ( KeepaliveRequests: 100, VariablesHashBucketSize: 256, VariablesHashMaxSize: 1024, + NginxVersion: nginx.NewVersion("nginx version: nginx/1.25.3 (nginx-plus-r31)"), } mainCfgCustomTLSPassthroughPort = MainConfig{ @@ -1342,6 +1385,7 @@ var ( VariablesHashMaxSize: 1024, TLSPassthrough: true, TLSPassthroughPort: 8443, + NginxVersion: nginx.NewVersion("nginx version: nginx/1.25.3 (nginx-plus-r31)"), } mainCfgWithoutTLSPassthrough = MainConfig{ @@ -1367,6 +1411,7 @@ var ( VariablesHashMaxSize: 1024, TLSPassthrough: false, TLSPassthroughPort: 8443, + NginxVersion: nginx.NewVersion("nginx version: nginx/1.25.3 (nginx-plus-r31)"), } mainCfgDefaultTLSPassthroughPort = MainConfig{ @@ -1392,6 +1437,7 @@ var ( VariablesHashMaxSize: 1024, TLSPassthrough: true, TLSPassthroughPort: 443, + NginxVersion: nginx.NewVersion("nginx version: nginx/1.25.3 (nginx-plus-r31)"), } mainCfgCustomDefaultHTTPAndHTTPSListenerPorts = MainConfig{ @@ -1417,6 +1463,7 @@ var ( KeepaliveRequests: 100, VariablesHashBucketSize: 256, VariablesHashMaxSize: 1024, + NginxVersion: nginx.NewVersion("nginx version: nginx/1.25.3 (nginx-plus-r31)"), } mainCfgCustomDefaultHTTPListenerPort = MainConfig{ @@ -1442,6 +1489,7 @@ var ( KeepaliveRequests: 100, VariablesHashBucketSize: 256, VariablesHashMaxSize: 1024, + NginxVersion: nginx.NewVersion("nginx version: nginx/1.25.3 (nginx-plus-r31)"), } mainCfgCustomDefaultHTTPSListenerPort = MainConfig{ @@ -1467,6 +1515,7 @@ var ( KeepaliveRequests: 100, VariablesHashBucketSize: 256, VariablesHashMaxSize: 1024, + NginxVersion: nginx.NewVersion("nginx version: nginx/1.25.3 (nginx-plus-r31)"), } // Vars for Mergable Ingress Master - Minion tests diff --git a/internal/nginx/fake_manager.go b/internal/nginx/fake_manager.go index 5533fb9180..1e42c51ed8 100644 --- a/internal/nginx/fake_manager.go +++ b/internal/nginx/fake_manager.go @@ -101,9 +101,9 @@ func (fm *FakeManager) CreateDHParam(_ string) (string, error) { } // Version provides a fake implementation of Version. -func (*FakeManager) Version() string { +func (*FakeManager) Version() Version { glog.V(3).Info("Printing nginx version") - return "fake version plus" + return Version{} } // Start provides a fake implementation of Start. diff --git a/internal/nginx/manager.go b/internal/nginx/manager.go index b4ac67d110..7c5af351fc 100644 --- a/internal/nginx/manager.go +++ b/internal/nginx/manager.go @@ -8,6 +8,7 @@ import ( "os/exec" "path" "path/filepath" + "regexp" "strconv" "strings" "time" @@ -46,6 +47,19 @@ const ( appProtectDosAgentStartDebugCmd = "/usr/bin/admd -d --standalone --log debug" ) +var ( + re = regexp.MustCompile(`(?P\S+)/(?P\S+)`) + plusre = regexp.MustCompile(`(?P\S+)/(?P\S+).\((?P\S+plus\S+)\)`) +) + +// Version holds the parsed output from `nginx -v` +type Version struct { + raw string + OSS string + IsPlus bool + Plus string +} + // ServerConfig holds the config data for an upstream server in NGINX Plus. type ServerConfig struct { MaxFails int @@ -72,7 +86,7 @@ type Manager interface { CreateDHParam(content string) (string, error) CreateOpenTracingTracerConfig(content string) error Start(done chan error) - Version() string + Version() Version Reload(isEndpointsUpdate bool) error Quit() UpdateConfigVersionFile(openTracing bool) @@ -334,13 +348,13 @@ func (lm *LocalManager) Quit() { } // Version returns NGINX version -func (lm *LocalManager) Version() string { +func (lm *LocalManager) Version() Version { binaryFilename := getBinaryFileName(lm.debug) out, err := exec.Command(binaryFilename, "-v").CombinedOutput() if err != nil { glog.Fatalf("Failed to get nginx version: %v", err) } - return string(out) + return NewVersion(string(out)) } // UpdateConfigVersionFile writes the config version file. @@ -432,6 +446,83 @@ func (lm *LocalManager) CreateOpenTracingTracerConfig(content string) error { return nil } +// Return the raw Nginx version string from `nginx -v` +func (v *Version) String() string { + return v.raw +} + +// PlusGreaterThanOrEqualTo compares the supplied nginx-plus version string with the Version{} struct +func (v Version) PlusGreaterThanOrEqualTo(target string) (bool, error) { + r, p, err := extractPlusVersionValues(v.String()) + if err != nil { + return false, err + } + tr, tp, err := extractPlusVersionValues(target) + if err != nil { + return false, err + } + + return (r > tr || (r == tr && p >= tp)), nil +} + +// NewVersion will take the output from `nginx -v` and explodes it into the `nginx.Version` struct +func NewVersion(line string) Version { + matches := re.FindStringSubmatch(line) + plusmatches := plusre.FindStringSubmatch(line) + nv := Version{ + raw: line, + } + + if len(plusmatches) > 0 { + subNames := plusre.SubexpNames() + nv.IsPlus = true + for i, v := range plusmatches { + switch subNames[i] { + case "plus": + nv.Plus = v + case "version": + nv.OSS = v + } + } + } + + if len(matches) > 0 { + for i, key := range re.SubexpNames() { + val := matches[i] + if key == "version" { + nv.OSS = val + } + } + } + + return nv +} + +// extractPlusVersionValues +func extractPlusVersionValues(input string) (int, int, error) { + var rValue, pValue int + re := regexp.MustCompile(`-r(\d+)(?:-p(\d+))?`) + matches := re.FindStringSubmatch(input) + + if len(matches) < 2 { + return 0, 0, fmt.Errorf("no matches found in the input string") + } + + rValue, err := strconv.Atoi(matches[1]) + if err != nil { + return 0, 0, fmt.Errorf("failed to convert rValue to integer: %w", err) + } + + if len(matches) > 2 && len(matches[2]) > 0 { + pValue, err = strconv.Atoi(matches[2]) + if err != nil { + return 0, 0, fmt.Errorf("failed to convert pValue to integer: %w", err) + } + } + + return rValue, pValue, nil +} + // verifyConfigVersion is used to check if the worker process that the API client is connected // to is using the latest version of nginx config. This way we avoid making changes on // a worker processes that is being shut down. diff --git a/internal/nginx/manager_test.go b/internal/nginx/manager_test.go new file mode 100644 index 0000000000..948326b8ae --- /dev/null +++ b/internal/nginx/manager_test.go @@ -0,0 +1,94 @@ +package nginx + +import ( + "testing" +) + +func TestNginxVersionParsing(t *testing.T) { + t.Parallel() + type testCase struct { + input string + expected Version + } + testCases := []testCase{ + { + input: "nginx version: nginx/1.25.1 (nginx-plus-r30-p1)", + expected: Version{ + raw: "nginx version: nginx/1.25.1 (nginx-plus-r30-p1)", + OSS: "1.25.1", + IsPlus: true, + Plus: "nginx-plus-r30-p1", + }, + }, + { + input: "nginx version: nginx/1.25.3 (nginx-plus-r31)", + expected: Version{ + raw: "nginx version: nginx/1.25.3 (nginx-plus-r31)", + OSS: "1.25.3", + IsPlus: true, + Plus: "nginx-plus-r31", + }, + }, + { + input: "nginx version: nginx/1.25.0", + expected: Version{ + raw: "nginx version: nginx/1.25.0", + OSS: "1.25.0", + IsPlus: false, + Plus: "", + }, + }, + } + for _, tc := range testCases { + t.Run(tc.input, func(t *testing.T) { + actual := NewVersion(tc.input) + if actual != tc.expected { + t.Errorf("expected %v but got %v", tc.expected, actual) + } + }) + } +} + +func TestNginxVersionPlusGreaterThanOrEqualTo(t *testing.T) { + t.Parallel() + type testCase struct { + version Version + input string + expected bool + } + testCases := []testCase{ + { + version: NewVersion("nginx version: nginx/1.25.1 (nginx-plus-r30-p1)"), + input: "nginx-plus-r30-p1", + expected: true, + }, + { + version: NewVersion("nginx version: nginx/1.25.1 (nginx-plus-r30)"), + input: "nginx-plus-r30", + expected: true, + }, + { + version: NewVersion("nginx version: nginx/1.25.1 (nginx-plus-r30-p1)"), + input: "nginx-plus-r30", + expected: true, + }, + { + version: NewVersion("nginx version: nginx/1.25.1 (nginx-plus-r30)"), + input: "nginx-plus-r30-p1", + expected: false, + }, + { + version: NewVersion("nginx version: nginx/1.25.1"), + input: "nginx-plus-r30-p1", + expected: false, + }, + } + for _, tc := range testCases { + t.Run(tc.input, func(t *testing.T) { + actual, _ := tc.version.PlusGreaterThanOrEqualTo(tc.input) + if actual != tc.expected { + t.Errorf("expected %v but got %v", tc.expected, actual) + } + }) + } +}