Fix code review

charts/ingress-nginx/README.md

@@ -294,9 +294,9 @@ As of version `1.26.0` of this chart, by simply not providing any clusterIP valu
 | controller.dnsConfig | object | `{}` | Optionally customize the pod dnsConfig. |
 | controller.dnsPolicy | string | `"ClusterFirst"` | Optionally change this to ClusterFirstWithHostNet in case you have 'hostNetwork: true'. By default, while using host network, name resolution uses the host's DNS. If you wish nginx-controller to keep resolving names inside the k8s network, use ClusterFirstWithHostNet. |
 | controller.electionID | string | `""` | Election ID to use for status update, by default it uses the controller name combined with a suffix of 'leader' |
+| controller.enableAnnotationValidations | bool | `false` |  |
 | controller.enableMimalloc | bool | `true` | Enable mimalloc as a drop-in replacement for malloc. # ref: https://github.com/microsoft/mimalloc # |
 | controller.enableTopologyAwareRouting | bool | `false` | This configuration enables Topology Aware Routing feature, used together with service annotation service.kubernetes.io/topology-aware-hints="auto" Defaults to false |
-| controller.enableValidations | bool | `false` |  |
 | controller.existingPsp | string | `""` | Use an existing PSP instead of creating one |
 | controller.extraArgs | object | `{}` | Additional command line arguments to pass to Ingress-Nginx Controller E.g. to specify the default SSL certificate you can use |
 | controller.extraContainers | list | `[]` | Additional containers to be added to the controller pod. See https://github.com/lemonldap-ng-controller/lemonldap-ng-controller as example. |

charts/ingress-nginx/templates/_params.tpl

@@ -1,6 +1,6 @@
 {{- define "ingress-nginx.params" -}}
 - /nginx-ingress-controller
-{{- if .Values.controller.enableValidations }}
+{{- if .Values.controller.enableAnnotationValidations }}
 - --enable-annotation-validation=true
 {{- end }}
 {{- if .Values.defaultBackend.enabled }}

charts/ingress-nginx/values.yaml

@@ -15,7 +15,7 @@ commonLabels: {}
 
 controller:
   name: controller
-  enableValidations: false
+  enableAnnotationValidations: false
   image:
     ## Keep false as default for now!
     chroot: false

internal/ingress/annotations/http2pushpreload/main.go

@@ -28,7 +28,7 @@ const (
 )
 
 var http2PushPreloadAnnotations = parser.Annotation{
-	Group: "", // TODO: TBD
+	Group: "http2",
 	Annotations: parser.AnnotationFields{
 		http2PushPreloadAnnotation: {
 			Validator:     parser.ValidateBool,

test/e2e/framework/exec.go

@@ -117,11 +117,11 @@ func (f *Framework) newIngressController(namespace string, namespaceOverlay stri
 		isChroot = "false"
 	}
 
-	enableValidations, ok := os.LookupEnv("ENABLE_VALIDATIONS")
+	enableAnnotationValidations, ok := os.LookupEnv("ENABLE_VALIDATIONS")
 	if !ok {
-		enableValidations = "false"
+		enableAnnotationValidations = "false"
 	}
-	cmd := exec.Command("./wait-for-nginx.sh", namespace, namespaceOverlay, isChroot, enableValidations)
+	cmd := exec.Command("./wait-for-nginx.sh", namespace, namespaceOverlay, isChroot, enableAnnotationValidations)
 	out, err := cmd.CombinedOutput()
 	if err != nil {
 		return fmt.Errorf("unexpected error waiting for ingress controller deployment: %v.\nLogs:\n%v", err, string(out))

test/e2e/wait-for-nginx.sh

@@ -69,7 +69,7 @@ else
 # TODO: remove the need to use fullnameOverride
 fullnameOverride: nginx-ingress
 controller:
-  enableValidations: ${ENABLE_VALIDATIONS}
+  enableAnnotationValidations: ${ENABLE_VALIDATIONS}
   image:
     repository: ingress-controller/controller
     chroot: ${IS_CHROOT}