Dos access log dest (#4187)

nginxinc · Sep 7, 2023 · 37ab8d6 · 37ab8d6
1 parent b44efbb
commit 37ab8d6
Show file tree

Hide file tree

Showing 5 changed files with 14 additions and 31 deletions.
diff --git a/docs/content/app-protect-dos/dos-protected.md b/docs/content/app-protect-dos/dos-protected.md
@@ -32,15 +32,16 @@ spec:
 {{% table %}}
 |Field | Description | Type | Required |
 | ---| ---| ---| --- |
-|``enable`` | Enables NGINX App Protect DoS. | ``bool`` | No |
-|``name`` | Name of the protected object, max of 63 characters. | ``string`` | No |
+|``enable`` | Enables NGINX App Protect DoS, Default value: false. | ``bool`` | No |
+|``name`` | Name of the protected object, max of 63 characters. | ``string`` | Yes |
+|``dosAccessLogDest`` | The log destination for the access log with dos log format. Accepted variables are ``syslog:server=<ip-address &#124; localhost &#124; dns-name>:<port>``, ``stderr``, ``<absolute path to file>``. | ``string`` | No |
 |``apDosMonitor.uri`` | The destination to the desired protected object. [App Protect DoS monitor](#dosprotectedresourceapdosmonitor) Default value: None, URL will be extracted from the first request which arrives and taken from "Host" header or from destination ip+port. | ``string`` | No |
 |``apDosMonitor.protocol`` | Determines if the server listens on http1 / http2 / grpc / websocket. [App Protect DoS monitor](#dosprotectedresourceapdosmonitor) Default value: http1. | ``enum`` | No |
 |``apDosMonitor.timeout`` | Determines how long (in seconds) should NGINX App Protect DoS wait for a response. [App Protect DoS monitor](#dosprotectedresourceapdosmonitor) Default value: 10 seconds for http1/http2 and 5 seconds for grpc. | ``int64`` | No |
 |``apDosPolicy`` | The [App Protect DoS policy](#dosprotectedresourceapdospolicy) of the dos. Accepts an optional namespace. | ``string`` | No |
 |``dosSecurityLog.enable`` | Enables security log. | ``bool`` | No |
 |``dosSecurityLog.apDosLogConf`` | The [App Protect DoS log conf](/nginx-ingress-controller/app-protect-dos/configuration/#app-protect-dos-logs) resource. Accepts an optional namespace. | ``string`` | No |
-|``dosSecurityLog.dosLogDest`` | The log destination for the security log. Accepted variables are ``syslog:server=<ip-address | localhost | dns-name>:<port>``,``stderr``,``<absolute path to file>``. Default is``"syslog:server=127.0.0.1:514"``. | ``string`` | No |
+|``dosSecurityLog.dosLogDest`` | The log destination for the security log. Accepted variables are ``syslog:server=<ip-address &#124; localhost &#124; dns-name>:<port>``, ``stderr``, ``<absolute path to file>``. Default is ``"syslog:server=127.0.0.1:514"``. | ``string`` | No |
 {{% /table %}}
 
 ### DosProtectedResource.apDosPolicy

diff --git a/internal/configs/dos.go b/internal/configs/dos.go
@@ -32,7 +32,9 @@ func getAppProtectDosResource(dosEx *DosEx) *appProtectDosResource {
 		dosResource.AppProtectDosMonitorTimeout = protected.Spec.ApDosMonitor.Timeout
 	}
 
-	dosResource.AppProtectDosAccessLogDst = generateDosLogDest(protected.Spec.DosAccessLogDest)
+	if protected.Spec.DosAccessLogDest != "" {
+		dosResource.AppProtectDosAccessLogDst = generateDosLogDest(protected.Spec.DosAccessLogDest)
+	}
 
 	if dosEx.DosPolicy != nil {
 		dosResource.AppProtectDosPolicyFile = appProtectDosPolicyFileName(dosEx.DosPolicy.GetNamespace(), dosEx.DosPolicy.GetName())

diff --git a/pkg/apis/dos/validation/dos.go b/pkg/apis/dos/validation/dos.go
@@ -44,12 +44,11 @@ func ValidateDosProtectedResource(protected *v1beta1.DosProtectedResource) error
 	}
 
 	// dosAccessLogDest
-	if protected.Spec.DosAccessLogDest == "" {
-		return fmt.Errorf("error validating DosProtectedResource: %v missing value for field: %v", protected.Name, "dosAccessLogDest")
-	}
-	err = validateAppProtectDosLogDest(protected.Spec.DosAccessLogDest)
-	if err != nil {
-		return fmt.Errorf("error validating DosProtectedResource: %v invalid field: %v err: %w", protected.Name, "dosAccessLogDest", err)
+	if protected.Spec.DosAccessLogDest != "" {
+		err = validateAppProtectDosLogDest(protected.Spec.DosAccessLogDest)
+		if err != nil {
+			return fmt.Errorf("error validating DosProtectedResource: %v invalid field: %v err: %w", protected.Name, "dosAccessLogDest", err)
+		}
 	}
 
 	// apDosPolicy

diff --git a/pkg/apis/dos/validation/dos_test.go b/pkg/apis/dos/validation/dos_test.go
@@ -29,27 +29,6 @@ func TestValidateDosProtectedResource(t *testing.T) {
 			expectErr: "error validating DosProtectedResource:  missing value for field: name",
 			msg:       "empty spec",
 		},
-		{
-			protected: &v1beta1.DosProtectedResource{
-				Spec: v1beta1.DosProtectedResourceSpec{
-					Name: "name",
-				},
-			},
-			expectErr: "error validating DosProtectedResource:  missing value for field: dosAccessLogDest",
-			msg:       "only name specified",
-		},
-		{
-			protected: &v1beta1.DosProtectedResource{
-				Spec: v1beta1.DosProtectedResourceSpec{
-					Name: "name",
-					ApDosMonitor: &v1beta1.ApDosMonitor{
-						URI: "example.com",
-					},
-				},
-			},
-			expectErr: "error validating DosProtectedResource:  missing value for field: dosAccessLogDest",
-			msg:       "name and apDosMonitor specified",
-		},
 		{
 			protected: &v1beta1.DosProtectedResource{
 				Spec: v1beta1.DosProtectedResourceSpec{

diff --git a/tests/suite/test_dos.py b/tests/suite/test_dos.py
@@ -177,6 +177,8 @@ def test_ap_nginx_config_entries(
             f"app_protect_dos_policy_file /etc/nginx/dos/policies/{test_namespace}_{dos_setup.pol_name}.json;",
             f"app_protect_dos_security_log_enable on;",
             f"app_protect_dos_security_log /etc/nginx/dos/logconfs/{test_namespace}_{dos_setup.log_name}.json syslog:server=syslog-svc.{ingress_controller_prerequisites.namespace}.svc.cluster.local:514;",
+            f"set $loggable '0';",
+            f"access_log syslog:server=127.0.0.1:5561 log_dos if=$loggable;",
         ]
 
         conf_nginx_directive = ["app_protect_dos_api on;", "location = /dashboard-dos.html"]