Repository: nginx/kubernetes-ingress

Issue:
[Bug]: NIC does not accept day-unit value for controller.mgmt.usageReport.interval
### Version

5.0.0

### What Kubernetes platforms are you running on?

Kind

### Steps to reproduce

Deploy NIC v5.0.0 with `alues.yaml` that has `controller.mgmt.usageReport.interval` set to `30d`

Observed Error:
`E20250630 17:05:38.655714 1 configmaps.go:818] Configmap nginx-plus/ingress-nginx-plus-mgmt: Invalid value for the interval key: got "30d": time: unknown unit "d" in duration "30d". Ignoring.
E20250630 17:05:38.655764 1 configmaps.go:824] Configmap nginx-plus/ingress-nginx-plus-mgmt: Value too low for the interval key, got: 30d, need higher than 60s. Ignoring.`

Expected Outcome:
A value of `30d` for the usage report interval applies successfully in the nginx configuration

### Workaround
Use the hours as the units, eg 30 days as 30*24=720h

Hints from issue comments:
@github-actions: Hi @mrajagopal thanks for reporting! 

 Be sure to check out the [docs](https://docs.nginx.com/nginx-ingress-controller) and the [Contributing Guidelines](https://github.com/nginx/kubernetes-ingress/blob/main/CONTRIBUTING.md) while you wait for a human to take a look at this :slightly_smiling_face:

 Cheers!@AlexFenlon: Hi @mrajagopal, 

Thanks for the bug report, I noticed that [here](https://github.com/nginx/kubernetes-ingress/blob/main/internal/configs/configmaps.go#L912) we are just using the go standard library for [`time.ParseDuration`](https://pkg.go.dev/time#ParseDuration) in our validation for this. As you mentioned, the most is goes up to is hours, not days or months. 
@cpuengr949: When I try 720h in there I get the following.

stderr: "2025/07/07 17:51:37 [notice] 70#70: js vm init njs: 000055B90A8B5F80\n2025/07/07 17:51:37 [emerg] 70#70: interval cannot be greater than 24h in /etc/nginx/nginx.conf:157\n"@mrajagopal: It turns out that the [usage_report](https://nginx.org/en/docs/ngx_mgmt_module.html#usage_report) directive only allows the interval value to be set to a maximum of 24 hours.
Therefore, our recommendation to the customer is to set this to 24h.
 
Hope this clarifies the limits of `controller.mgmt.usageReport.interval` parameter.
I have requested the docs to provide limits for the `interval` value of the `usage_report` directive in the [nginx.org](https://nginx.org/) docs.

@AlexFenlon, please confirm if we have assessed this appropriately.
We ought to improve the logging message that NIC writes to indicate the limits of the `interval` value.
Maybe the schema needs to be update to drop the `day` resolution as it can only be 1 day as the interval when 24h should suffice.@vepatel: @mrajagopal https://github.com/nginx/kubernetes-ingress/pull/8043 drops day altogether

Solution patch:
diff --git a/charts/nginx-ingress/values.schema.json b/charts/nginx-ingress/values.schema.json
index 99c8efa9c..575a6c628 100644
--- a/charts/nginx-ingress/values.schema.json
+++ b/charts/nginx-ingress/values.schema.json
@@ -134,10 +134,11 @@
                 },
                 "interval": {
                   "type": "string",
-                  "pattern": "^[0-9]+[mhd]$",
+                  "pattern": "^[0-9]+[smh]$",
                   "default": "1h",
                   "title": "The usage report interval Schema",
                   "examples": [
+                    "60s",
                     "1m",
                     "1h",
                     "24h"
diff --git a/charts/nginx-ingress/values.yaml b/charts/nginx-ingress/values.yaml
index 95ddf7daa..ddc0adc1b 100644
--- a/charts/nginx-ingress/values.yaml
+++ b/charts/nginx-ingress/values.yaml
@@ -24,7 +24,7 @@ controller:
 
     # usageReport:
     #   endpoint: "product.connect.nginx.com" # Endpoint for usage report
-    #   interval: 1h
+    #   interval: 1h # Interval for usage report, must be between 60s and 24h,
     #   proxyHost: "proxy.example.com:3138" # Proxy server for usage report, with optional port
     #   proxyCredentialsSecretName: "proxy-credentials" # Secret containing proxy credentials, must contain a `username` and `password` field
 
diff --git a/internal/configs/configmaps.go b/internal/configs/configmaps.go
index 3c8436e17..7a1c7cd00 100644
--- a/internal/configs/configmaps.go
+++ b/internal/configs/configmaps.go
@@ -23,6 +23,7 @@ import (
 
 const (
 	minimumInterval     = 60
+	maximumInterval     = 24 * 60 * 60 // interval cannot be greater than 24h
 	zoneSyncDefaultPort = 12345
 	kubeDNSDefault      = "kube-dns.kube-system.svc.cluster.local"
 )
@@ -909,23 +910,34 @@ func ParseMGMTConfigMap(ctx context.Context, cfgm *v1.ConfigMap, eventLog record
 
 	if interval, exists := cfgm.Data["usage-report-interval"]; exists {
 		i := strings.TrimSpace(interval)
-		t, err := time.ParseDuration(i)
-		if err != nil {
-			errorText := fmt.Sprintf("Configmap %s/%s: Invalid value for the interval key: got %q: %v. Ignoring.", cfgm.GetNamespace(), cfgm.GetName(), i, err)
-			nl.Error(l, errorText)
-			eventLog.Event(cfgm, v1.EventTypeWarning, nl.EventReasonInvalidValue, errorText)
-			configWarnings = true
-		}
-		if t.Seconds() < minimumInterval {
-			errorText := fmt.Sprintf("Configmap %s/%s: Value too low for the interval key, got: %v, need higher than %ds. Ignoring.", cfgm.GetNamespace(), cfgm.GetName(), i, minimumInterval)
+
+		// Validate interval: check for unsupported units and parse duration in case json schema validation is not used.
+		if strings.Contains(i, "ms") || strings.Contains(i, "d") {
+			errorText := fmt.Sprintf("Configmap %s/%s: Invalid unit for the interval key: got %q. Only seconds (s), minutes (m), and hours (h) are allowed. Ignoring.", cfgm.GetNamespace(), cfgm.GetName(), i)
 			nl.Error(l, errorText)
 			eventLog.Event(cfgm, v1.EventTypeWarning, nl.EventReasonInvalidValue, errorText)
 			configWarnings = true
-			mgmtCfgParams.Interval = ""
 		} else {
-			mgmtCfgParams.Interval = i
+			t, err := time.ParseDuration(i)
+			if err != nil {
+				errorText := fmt.Sprintf("Configmap %s/%s: Invalid value for the interval key: got %q: %v. Ignoring.", cfgm.GetNamespace(), cfgm.GetName(), i, err)
+				nl.Error(l, errorText)
+				eventLog.Event(cfgm, v1.EventTypeWarning, nl.EventReasonInvalidValue, errorText)
+				configWarnings = true
+			} else if t.Seconds() < minimumInterval {
+				errorText := fmt.Sprintf("Configmap %s/%s: Value too low for the interval key, got: %v, need higher than %ds. Ignoring.", cfgm.GetNamespace(), cfgm.GetName(), i, minimumInterval)
+				nl.Error(l, errorText)
+				eventLog.Event(cfgm, v1.EventTypeWarning, nl.EventReasonInvalidValue, errorText)
+				configWarnings = true
+			} else if t.Seconds() > maximumInterval {
+				errorText := fmt.Sprintf("Configmap %s/%s: Value too high for the interval key, got: %v, maximum allowed is %ds (24h). Ignoring.", cfgm.GetNamespace(), cfgm.GetName(), i, maximumInterval)
+				nl.Error(l, errorText)
+				eventLog.Event(cfgm, v1.EventTypeWarning, nl.EventReasonInvalidValue, errorText)
+				configWarnings = true
+			} else {
+				mgmtCfgParams.Interval = i
+			}
 		}
-
 	}
 	if trustedCertSecretName, exists := cfgm.Data["ssl-trusted-certificate-secret-name"]; exists {
 		mgmtCfgParams.Secrets.TrustedCert = strings.TrimSpace(trustedCertSecretName)



Test patch:
diff --git a/internal/configs/configmaps_test.go b/internal/configs/configmaps_test.go
index 0ddf61c9e..b4afbd661 100644
--- a/internal/configs/configmaps_test.go
+++ b/internal/configs/configmaps_test.go
@@ -683,6 +683,200 @@ func TestParseMGMTConfigMapUsageReportInterval(t *testing.T) {
 	}
 }
 
+func TestParseMGMTConfigMapUsageReportIntervalMaximum(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		configMap      *v1.ConfigMap
+		expectWarnings bool
+		expectInterval string
+		msg            string
+	}{
+		{
+			configMap: &v1.ConfigMap{
+				Data: map[string]string{
+					"license-token-secret-name": "license-token",
+					"usage-report-interval":     "24h",
+				},
+			},
+			expectWarnings: false,
+			expectInterval: "24h",
+			msg:            "24h should be accepted (maximum allowed)",
+		},
+		{
+			configMap: &v1.ConfigMap{
+				Data: map[string]string{
+					"license-token-secret-name": "license-token",
+					"usage-report-interval":     "25h",
+				},
+			},
+			expectWarnings: true,
+			expectInterval: "",
+			msg:            "25h should be rejected (exceeds maximum)",
+		},
+		{
+			configMap: &v1.ConfigMap{
+				Data: map[string]string{
+					"license-token-secret-name": "license-token",
+					"usage-report-interval":     "1440m",
+				},
+			},
+			expectWarnings: false,
+			expectInterval: "1440m",
+			msg:            "1440m (24h) should be accepted",
+		},
+		{
+			configMap: &v1.ConfigMap{
+				Data: map[string]string{
+					"license-token-secret-name": "license-token",
+					"usage-report-interval":     "1441m",
+				},
+			},
+			expectWarnings: true,
+			expectInterval: "",
+			msg:            "1441m (>24h) should be rejected",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.msg, func(t *testing.T) {
+			result, warnings, err := ParseMGMTConfigMap(context.Background(), test.configMap, makeEventLogger())
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			if warnings != test.expectWarnings {
+				t.Errorf("Expected warnings=%v, got warnings=%v", test.expectWarnings, warnings)
+			}
+
+			if result.Interval != test.expectInterval {
+				t.Errorf("Expected interval=%q, got interval=%q", test.expectInterval, result.Interval)
+			}
+		})
+	}
+}
+
+func TestParseMGMTConfigMapUsageReportIntervalEdgeCases(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		configMap      *v1.ConfigMap
+		expectWarnings bool
+		expectInterval string
+		msg            string
+	}{
+		// Test milliseconds (should be rejected due to unsupported unit)
+		{
+			configMap: &v1.ConfigMap{
+				Data: map[string]string{
+					"license-token-secret-name": "license-token",
+					"usage-report-interval":     "1000ms",
+				},
+			},
+			expectWarnings: true,
+			expectInterval: "",
+			msg:            "1000ms should be rejected (ms unit not allowed)",
+		},
+		{
+			configMap: &v1.ConfigMap{
+				Data: map[string]string{
+					"license-token-secret-name": "license-token",
+					"usage-report-interval":     "60000ms",
+				},
+			},
+			expectWarnings: true,
+			expectInterval: "",
+			msg:            "60000ms should be rejected (ms unit not allowed)",
+		},
+		// Test that large ms values are also rejected for unit, not value
+		{
+			configMap: &v1.ConfigMap{
+				Data: map[string]string{
+					"license-token-secret-name": "license-token",
+					"usage-report-interval":     "3600000ms",
+				},
+			},
+			expectWarnings: true,
+			expectInterval: "",
+			msg:            "3600000ms (1h) should be rejected (ms unit not allowed)",
+		},
+		// Test days (should be rejected by Go's ParseDuration)
+		{
+			configMap: &v1.ConfigMap{
+				Data: map[string]string{
+					"license-token-secret-name": "license-token",
+					"usage-report-interval":     "1d",
+				},
+			},
+			expectWarnings: true,
+			expectInterval: "",
+			msg:            "1d should be rejected (Go doesn't support 'd' unit)",
+		},
+		// Test values > 24h (valid in Go but should be rejected by max check)
+		{
+			configMap: &v1.ConfigMap{
+				Data: map[string]string{
+					"license-token-secret-name": "license-token",
+					"usage-report-interval":     "25h",
+				},
+			},
+			expectWarnings: true,
+			expectInterval: "",
+			msg:            "25h should be rejected (exceeds 24h maximum)",
+		},
+		{
+			configMap: &v1.ConfigMap{
+				Data: map[string]string{
+					"license-token-secret-name": "license-token",
+					"usage-report-interval":     "48h",
+				},
+			},
+			expectWarnings: true,
+			expectInterval: "",
+			msg:            "48h should be rejected (exceeds 24h maximum)",
+		},
+		// exactly 86400 seconds (24h)
+		{
+			configMap: &v1.ConfigMap{
+				Data: map[string]string{
+					"license-token-secret-name": "license-token",
+					"usage-report-interval":     "86400s",
+				},
+			},
+			expectWarnings: false,
+			expectInterval: "86400s",
+			msg:            "86400s (24h) should be accepted",
+		},
+		// 86401 seconds (24h + 1s)
+		{
+			configMap: &v1.ConfigMap{
+				Data: map[string]string{
+					"license-token-secret-name": "license-token",
+					"usage-report-interval":     "86401s",
+				},
+			},
+			expectWarnings: true,
+			expectInterval: "",
+			msg:            "86401s (>24h) should be rejected",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.msg, func(t *testing.T) {
+			result, warnings, err := ParseMGMTConfigMap(context.Background(), test.configMap, makeEventLogger())
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			if warnings != test.expectWarnings {
+				t.Errorf("Expected warnings=%v, got warnings=%v", test.expectWarnings, warnings)
+			}
+
+			if result.Interval != test.expectInterval {
+				t.Errorf("Expected interval=%q, got interval=%q", test.expectInterval, result.Interval)
+			}
+		})
+	}
+}
+
 func TestParseMGMTConfigMapResolverIPV6(t *testing.T) {
 	t.Parallel()
 	tests := []struct {