review comments - 1

deployments/helm-chart-dos-arbitrator/Chart.yaml

@@ -1,6 +1,6 @@
 name: nginx-appprotect-dos-arbitrator
 version: 0.0.1
-appVersion: 1.0.0
+appVersion: 1.1.0
 apiVersion: v1
 kubeVersion: ">= 1.19.0-0"
 description: NGINX App Protect Dos arbitrator

deployments/helm-chart-dos-arbitrator/README.md

@@ -91,6 +91,6 @@ The following tables lists the configurable parameters of the NGINX App Protect
 Parameter | Description | Default
 --- | --- | ---
 `arbitrator.resources` | The resources of the Arbitrator pods. | limits:<br>cpu: 500m<br>memory: 128Mi
-`appprotectdos.arbitrator.image.repository` | The image repository of the Arbitrator image. | docker-registry.nginx.com/nap-dos/app_protect_dos_arb
+`arbitrator.image.repository` | The image repository of the Arbitrator image. | docker-registry.nginx.com/nap-dos/app_protect_dos_arb
 `arbitrator.image.tag` | The tag of the Arbitrator image. | 1.1.0
 `arbitrator.image.pullPolicy` | The pull policy for the Arbitrator image. | IfNotPresent

deployments/helm-chart/README.md

@@ -14,7 +14,7 @@ This chart deploys the NGINX Ingress controller in your Kubernetes cluster.
     - Alternatively, pull an Ingress controller image with NGINX Plus and push it to your private registry by following the instructions from [here](https://docs.nginx.com/nginx-ingress-controller/installation/pulling-ingress-controller-image).
     - Alternatively, you can build an Ingress controller image with NGINX Plus and push it to your private registry by following the instructions from [here](https://docs.nginx.com/nginx-ingress-controller/installation/building-ingress-controller-image).
     - Update the `controller.image.repository` field of the `values-plus.yaml` accordingly.
-  - If you’d like to use App Protect Dos, please install App Protect Dos Arbitrator helm chart
+  - If you’d like to use App Protect Dos, please install App Protect Dos Arbitrator helm chart. Make sure to install in the same namespace as the NGINX Ingress Controller. Note that if you install multiple NGINX Ingress Controllers in the same namespace, they will need to share the same Arbitrator because it is not possible to install more than one Arbitrator in a single namespace.
 
 
 ## Getting the Chart Sources

deployments/helm-chart/templates/controller-daemonset.yaml

@@ -106,6 +106,12 @@ spec:
           - -nginx-reload-timeout={{ .Values.controller.nginxReloadTimeout }}
           - -enable-app-protect={{ .Values.controller.appprotect.enable }}
           - -enable-app-protect-dos={{ .Values.controller.appprotectdos.enable }}
+  {{- if .Values.controller.appprotectdos.enable }}
+          - -app-protect-dos-debug={{ .Values.controller.appprotectdos.debug }}
+          - -app-protect-dos-max-daemons={{ .Values.controller.appprotectdos.maxWorkers }}
+          - -app-protect-dos-max-workers={{ .Values.controller.appprotectdos.maxDaemons }}
+          - -app-protect-dos-memory={{ .Values.controller.appprotectdos.memory }}
+  {{ end }}
           - -nginx-configmaps=$(POD_NAMESPACE)/{{ include "nginx-ingress.configName" . }}
 {{- if .Values.controller.defaultTLS.secret }}
           - -default-server-tls-secret={{ .Values.controller.defaultTLS.secret }}

deployments/helm-chart/values.yaml

@@ -21,9 +21,13 @@ controller:
   appprotectdos:
     ## Enable the App Protect Dos module in the Ingress Controller.
     enable: false
+    ## Enable debugging for App Protect Dos.
     debug: false
+    ## Max number of nginx processes to support.
     maxWorkers: 0
+    ## Max number of ADMD instances.
     maxDaemons: 0
+    ## RAM memory size to consume in MB.
     memory: 0
 
   ## Enables the Ingress controller pods to use the host's network namespace.

docs/content/app-protect-dos/configuration.md

@@ -8,7 +8,7 @@ toc: true
 ---
 
 This document describes how to configure the NGINX App Protect Dos module
-> Check out the complete [NGINX Ingress Controller with App Protect Dos example resources on GitHub](https://github.com/nginxinc/kubernetes-ingress/tree/v1.12.0/examples/appprotect-dos).
+> Check out the complete [NGINX Ingress Controller with App Protect Dos example resources on GitHub](https://github.com/nginxinc/kubernetes-ingress/tree/v2.0.3/examples/appprotect-dos).
 
 ## Global Configuration
 

docs/content/app-protect-dos/installation.md

@@ -10,7 +10,7 @@ toc: true
 
 This document provides an overview of the steps required to use NGINX App Protect Dos with your NGINX Ingress Controller deployment. You can visit the linked documents to find additional information and instructions.
 
-## Install the app-protect-dos-arb
+## Install the App Protect Dos Arbitrator
 
 - Deploy the app protect dos arbitrator
     ```bash
@@ -44,4 +44,4 @@ Take the steps below to set up and deploy the NGINX Ingress Controller and App P
 3. Enable the App Protect Dos module by adding the `enable-app-protect-dos` [cli argument](/nginx-ingress-controller/configuration/global-configuration/command-line-arguments/#cmdoption-enable-app-protect-dos) to your Deployment or DaemonSet file.
 5. [Deploy the Ingress Controller](/nginx-ingress-controller/installation/installation-with-manifests/#deploy-the-ingress-controller).
 
-For more information, see the [Configuration guide](/nginx-ingress-controller/app-protect-dos/configuration) and the [NGINX Ingress Controller with App Protect Dos examples on GitHub](https://github.com/nginxinc/kubernetes-ingress/tree/v1.12.0/examples/appprotect-dos).
+For more information, see the [Configuration guide](/nginx-ingress-controller/app-protect-dos/configuration) and the [NGINX Ingress Controller with App Protect Dos examples on GitHub](https://github.com/nginxinc/kubernetes-ingress/tree/v2.0.3/examples/appprotect-dos).

docs/content/configuration/dos-protected.md

@@ -43,7 +43,7 @@ spec:
 {{% table %}}
 |Field | Description | Type | Required |
 | ---| ---| ---| --- |
-|``enable`` | Enables NGINX App Protect Dos. | ``bool`` | Yes |
+|``enable`` | Enables NGINX App Protect Dos. | ``bool`` | No |
 |``name`` | Name of the protected object, max of 63 characters. | ``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. [App Protect Dos monitor](#dosprotectedresourceapdosmonitor) Default value: http1. | ``enum`` | No |

docs/content/installation/installation-with-helm-dos-arbitrator.md

@@ -93,6 +93,6 @@ The following tables lists the configurable parameters of the NGINX App Protect
 Parameter | Description | Default
 --- | --- | ---
 `arbitrator.resources` | The resources of the Arbitrator pods. | limits:<br>cpu: 500m<br>memory: 128Mi
-`appprotectdos.arbitrator.image.repository` | The image repository of the Arbitrator image. | docker-registry.nginx.com/nap-dos/app_protect_dos_arb
+`arbitrator.image.repository` | The image repository of the Arbitrator image. | docker-registry.nginx.com/nap-dos/app_protect_dos_arb
 `arbitrator.image.tag` | The tag of the Arbitrator image. | latest
 `arbitrator.image.pullPolicy` | The pull policy for the Arbitrator image. | IfNotPresent

docs/content/installation/installation-with-helm.md

@@ -19,7 +19,7 @@ This document describes how to install the NGINX Ingress Controller in your Kube
     - Alternatively, pull an Ingress controller image with NGINX Plus and push it to your private registry by following the instructions from [here](/nginx-ingress-controller/installation/pulling-ingress-controller-image).
     - Alternatively, you can build an Ingress controller image with NGINX Plus and push it to your private registry by following the instructions from [here](/nginx-ingress-controller/installation/building-ingress-controller-image).
     - Update the `controller.image.repository` field of the `values-plus.yaml` accordingly.
-  - If you’d like to use App Protect Dos, please [install App Protect Dos Arbitrator helm chart](nginx-ingress-controller/installation/installation-with-helm)
+  - If you’d like to use App Protect Dos, please install App Protect Dos Arbitrator helm chart. Make sure to install in the same namespace as the NGINX Ingress Controller. Note that if you install multiple NGINX Ingress Controllers in the same namespace, they will need to share the same Arbitrator because it is not possible to install more than one Arbitrator in a single namespace.
 
 ## Getting the Chart Sources
 

docs/content/installation/installation-with-manifests.md

@@ -108,11 +108,12 @@ If you would like to use the App Protect module, create the following additional
 
 If you would like to use the App Protect Dos module, create the following additional resources:
 
-1. Create a custom resource definition for `APDosPolicy` and `APDosLogConf`:
+1. Create a custom resource definition for `APDosPolicy`, `APDosLogConf` and `DosProtectedResource`:
 
    ```
    $ kubectl apply -f common/crds/appprotectdos.f5.com_apdoslogconfs.yaml
    $ kubectl apply -f common/crds/appprotectdos.f5.com_apdospolicies.yaml
+   $ kubectl apply -f common/crds/appprotectdos.f5.com_dosprotectedresources.yaml
    ```
 
 ## 3. Deploy the Ingress Controller

examples/custom-resources/dos/README.md

@@ -43,7 +43,7 @@ $ kubectl apply -f webapp.yaml
     ```
     $ kubectl apply -f virtual-server.yaml
     ```
-Note the reference to the DOS protected resource in the Policy resource. By specifying the resource it enables DOS protection for the VirtualServer.
+Note the reference to the DOS protected resource in the VirtualServer resource. By specifying the resource it enables DOS protection for the VirtualServer.
 
 ## Step 4 - Test the Application
 

internal/k8s/controller_test.go

@@ -682,7 +682,7 @@ func TestGetPolicies(t *testing.T) {
 
 	expectedPolicies := []*conf_v1.Policy{validPolicy}
 	expectedErrors := []error{
-		errors.New("Policy default/invalid-policy is invalid: spec: Invalid value: \"\": must specify exactly one of: `accessControl`, `rateLimit`, `ingressMTLS`, `egressMTLS`, `jwt`, `oidc`, `waf`, `dos`"),
+		errors.New("Policy default/invalid-policy is invalid: spec: Invalid value: \"\": must specify exactly one of: `accessControl`, `rateLimit`, `ingressMTLS`, `egressMTLS`, `jwt`, `oidc`, `waf`"),
 		errors.New("Policy nginx-ingress/valid-policy doesn't exist"),
 		errors.New("Failed to get policy nginx-ingress/some-policy: GetByKey error"),
 		errors.New("referenced policy default/valid-policy-ingress-class has incorrect ingress class: test-class (controller ingress class: )"),

internal/k8s/reference_checkers.go

@@ -294,7 +294,12 @@ func (rc *dosResourceReferenceChecker) IsReferencedByVirtualServer(namespace str
 	return false
 }
 
-func (rc *dosResourceReferenceChecker) IsReferencedByVirtualServerRoute(_ string, _ string, _ *v1.VirtualServerRoute) bool {
+func (rc *dosResourceReferenceChecker) IsReferencedByVirtualServerRoute(namespace string, name string, vsr *v1.VirtualServerRoute) bool {
+	for _, route := range vsr.Spec.Subroutes {
+		if route.Dos == namespace+"/"+name || (namespace == vsr.Namespace && route.Dos == name) {
+			return true
+		}
+	}
 	return false
 }
 

internal/k8s/validation_test.go

@@ -1562,7 +1562,7 @@ func TestValidateNginxIngressAnnotations(t *testing.T) {
 			appProtectDosEnabled:  true,
 			internalRoutesEnabled: false,
 			expectedErrors:        nil,
-			msg:                   "valid appprotectdos.f5.com/app-protect-dos-enable annotation",
+			msg:                   "valid appprotectdos.f5.com/app-protect-dos-enable annotation with default namespace",
 		},
 		{
 			annotations: map[string]string{
@@ -1574,7 +1574,7 @@ func TestValidateNginxIngressAnnotations(t *testing.T) {
 			appProtectDosEnabled:  true,
 			internalRoutesEnabled: false,
 			expectedErrors:        nil,
-			msg:                   "valid appprotectdos.f5.com/app-protect-dos-enable annotation",
+			msg:                   "valid appprotectdos.f5.com/app-protect-dos-enable annotation with fully specified identifier",
 		},
 		{
 			annotations: map[string]string{
@@ -1588,7 +1588,7 @@ func TestValidateNginxIngressAnnotations(t *testing.T) {
 			expectedErrors: []string{
 				"annotations.appprotectdos.f5.com/app-protect-dos-resource: Invalid value: \"special-chars-&%^\": must be a qualified name",
 			},
-			msg: "valid appprotectdos.f5.com/app-protect-dos-enable annotation",
+			msg: "invalid appprotectdos.f5.com/app-protect-dos-enable annotation with special characters",
 		},
 		{
 			annotations: map[string]string{
@@ -1602,7 +1602,7 @@ func TestValidateNginxIngressAnnotations(t *testing.T) {
 			expectedErrors: []string{
 				"annotations.appprotectdos.f5.com/app-protect-dos-resource: Invalid value: \"too/many/qualifiers\": must be a qualified name",
 			},
-			msg: "valid appprotectdos.f5.com/app-protect-dos-enable annotation",
+			msg: "invalid appprotectdos.f5.com/app-protect-dos-enable annotation with incorrectly qualified identifier",
 		},
 		{
 			annotations: map[string]string{

pkg/apis/configuration/validation/policy.go

@@ -102,7 +102,7 @@ func validatePolicySpec(spec *v1.PolicySpec, fieldPath *field.Path, isPlus, enab
 	if fieldCount != 1 {
 		msg := "must specify exactly one of: `accessControl`, `rateLimit`, `ingressMTLS`, `egressMTLS`"
 		if isPlus {
-			msg = fmt.Sprint(msg, ", `jwt`, `oidc`, `waf`, `dos`")
+			msg = fmt.Sprint(msg, ", `jwt`, `oidc`, `waf`")
 		}
 		allErrs = append(allErrs, field.Invalid(fieldPath, "", msg))
 	}