Add missing formating

internal/ingress/errors/errors.go

@@ -115,6 +115,10 @@ type ValidationError struct {
 	Reason error
 }
 
+type RiskyAnnotationError struct {
+	Reason error
+}
+
 func (e ValidationError) Error() string {
 	return e.Reason.Error()
 }
@@ -132,3 +136,21 @@ func IsValidationError(e error) bool {
 	_, ok := e.(ValidationError)
 	return ok
 }
+
+// NewValidationError returns a new LocationDenied error
+func NewRiskyAnnotations(name string) error {
+	return RiskyAnnotationError{
+		Reason: fmt.Errorf("annotation group %s contains risky annotation based on ingress configuration", name),
+	}
+}
+
+// IsRiskyAnnotationError checks if the err is an error which
+// indicates that some annotation value is invalid
+func IsRiskyAnnotationError(e error) bool {
+	_, ok := e.(ValidationError)
+	return ok
+}
+
+func (e RiskyAnnotationError) Error() string {
+	return e.Reason.Error()
+}

pkg/flags/flags.go

@@ -152,6 +152,9 @@ Requires the update-status parameter.`)
 		annotationsPrefix = flags.String("annotations-prefix", parser.DefaultAnnotationsPrefix,
 			`Prefix of the Ingress annotations specific to the NGINX controller.`)
 
+		disableAnnotationValidation = flags.Bool("disable-annotation-validation", parser.DefaultDisableAnnotationValidation,
+			`Prefix of the Ingress annotations specific to the NGINX controller.`)
+
 		enableSSLChainCompletion = flags.Bool("enable-ssl-chain-completion", false,
 			`Autocomplete SSL certificate chains with missing intermediate CA certificates.
 Certificates uploaded to Kubernetes must have the "Authority Information Access" X.509 v3
@@ -249,6 +252,7 @@ https://blog.maxmind.com/2019/12/18/significant-changes-to-accessing-and-using-g
 	}
 
 	parser.AnnotationsPrefix = *annotationsPrefix
+	parser.DisableAnnotationValidation = *disableAnnotationValidation
 
 	// check port collisions
 	if !ing_net.IsPortAvailable(*httpPort) {

test/e2e-image/namespace-overlays/validations/values.yaml

@@ -0,0 +1,38 @@
+# TODO: remove the need to use fullnameOverride
+fullnameOverride: nginx-ingress
+controller:
+  image:
+    repository: ingress-controller/controller
+    chroot: true
+    tag: 1.0.0-dev
+    digest:
+    digestChroot:
+  containerPort:
+    http: "1080"
+    https: "1443"
+
+  extraArgs:
+    http-port: "1080"
+    https-port: "1443"
+    # e2e tests do not require information about ingress status
+    update-status: "false"
+
+  scope:
+    enabled: true
+
+  config:
+    worker-processes: "1"
+  service:
+    type: NodePort
+
+  admissionWebhooks:
+    enabled: true
+    certificate: "/usr/local/certificates/cert"
+    key: "/usr/local/certificates/key"
+
+defaultBackend:
+  enabled: false
+
+rbac:
+  create: true
+  scope: true

test/e2e/e2e.go

@@ -47,7 +47,7 @@ import (
 	_ "k8s.io/ingress-nginx/test/e2e/settings"
 	_ "k8s.io/ingress-nginx/test/e2e/settings/modsecurity"
 	_ "k8s.io/ingress-nginx/test/e2e/settings/ocsp"
-	// _ "k8s.io/ingress-nginx/test/e2e/settings/validations" // Test is not working, need to check the cross namespace stuff
+	_ "k8s.io/ingress-nginx/test/e2e/settings/validations"
 
 	_ "k8s.io/ingress-nginx/test/e2e/ssl"
 	_ "k8s.io/ingress-nginx/test/e2e/status"

test/e2e/settings/validations/validations.go

@@ -18,93 +18,69 @@ package annotations
 
 import (
 	"context"
-	"fmt"
-	"net/http"
-	"strings"
 
 	"github.com/onsi/ginkgo/v2"
 	"github.com/stretchr/testify/assert"
-	"golang.org/x/crypto/bcrypt"
 
-	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"k8s.io/ingress-nginx/test/e2e/framework"
 )
 
-func buildSecret(username, password, name, namespace string) *corev1.Secret {
+var _ = framework.IngressNginxDescribeSerial("annotation validations", func() {
-	//out, err := exec.Command("openssl", "passwd", "-crypt", password).CombinedOutput()
+	f := framework.NewDefaultFramework("validations")
-	out, err := bcrypt.GenerateFromPassword([]byte(password), 14)
-	encpass := fmt.Sprintf("%v:%s\n", username, out)
-	assert.Nil(ginkgo.GinkgoT(), err)
 
-	return &corev1.Secret{
+	ginkgo.It("should allow ingress based on their risk on webhooks", func() {
-		ObjectMeta: metav1.ObjectMeta{
-			Name:                       name,
-			Namespace:                  namespace,
-			DeletionGracePeriodSeconds: framework.NewInt64(1),
-		},
-		Data: map[string][]byte{
-			"auth": []byte(encpass),
-		},
-		Type: corev1.SecretTypeOpaque,
-	}
-}
-
-var _ = framework.DescribeAnnotation("annotation validations", func() {
-	f := framework.NewDefaultFramework("annotations-validations")
-
-	ginkgo.BeforeEach(func() {
-		f.NewEchoDeployment()
-		otherns := &corev1.Namespace{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: "otherns",
-			},
-		}
-		_, err := f.KubeClientSet.CoreV1().Namespaces().Create(context.Background(), otherns, metav1.CreateOptions{})
-		assert.Nil(ginkgo.GinkgoT(), err, "creating namespace")
-	})
-
-	ginkgo.AfterEach(func() {
-		err := f.KubeClientSet.CoreV1().Namespaces().Delete(context.Background(), "otherns", metav1.DeleteOptions{})
-		assert.Nil(ginkgo.GinkgoT(), err, "deleting namespace")
-	})
-
-	ginkgo.It("should return status code 401 when authentication is configured but Authorization header is not configured", func() {
 		host := "annotation-validations"
-		// Allow cross namespace consumption
+
-		f.UpdateNginxConfigMapData("allow-cross-namespace-resources", "true")
+		// Low and Medium Risk annotations should be allowed, the rest should be denied
+		f.UpdateNginxConfigMapData("annotations-risk", "Medium")
 		// Sleep a while just to guarantee that the configmap is applied
 		framework.Sleep()
 
-		s := f.EnsureSecret(buildSecret("foo", "bar", "test", "otherns"))
-
 		annotations := map[string]string{
-			"nginx.ingress.kubernetes.io/auth-type":   "basic",
+			"nginx.ingress.kubernetes.io/default-backend":       "default/bla", // low risk
-			"nginx.ingress.kubernetes.io/auth-secret": fmt.Sprintf("%s/%s", s.Namespace, s.Name),
+			"nginx.ingress.kubernetes.io/denylist-source-range": "1.1.1.1/32",  // medium risk
-			"nginx.ingress.kubernetes.io/auth-realm":  "test auth",
 		}
 
+		ginkgo.By("allow ingress with low/medium risk annotations")
 		ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, annotations)
-		f.EnsureIngress(ing)
+		_, err := f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Create(context.TODO(), ing, metav1.CreateOptions{})
+		assert.Nil(ginkgo.GinkgoT(), err, "creating ingress with allowed annotations should not trigger an error")
+
+		ginkgo.By("block ingress with risky annotations")
+		annotations["nginx.ingress.kubernetes.io/modsecurity-transaction-id"] = "bla123"      // High should be blocked
+		annotations["nginx.ingress.kubernetes.io/modsecurity-snippet"] = "some random stuff;" // High should be blocked
+		ing = framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, annotations)
+		_, err = f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Update(context.TODO(), ing, metav1.UpdateOptions{})
+		assert.NotNil(ginkgo.GinkgoT(), err, "creating ingress with risky annotations should trigger an error")
 
-		f.WaitForNginxServer(host,
-			func(server string) bool {
-				return strings.Contains(server, "server_name annotation-validations")
 	})
 
-		f.HTTPTestClient().
+	ginkgo.It("should allow ingress based on their risk on webhooks", func() {
-			GET("/").
+		host := "annotation-validations"
-			WithHeader("Host", host).
+
-			Expect().
+		// Low and Medium Risk annotations should be allowed, the rest should be denied
-			Status(http.StatusUnauthorized).
+		f.UpdateNginxConfigMapData("annotations-risk", "Medium")
-			Body().Contains("401 Authorization Required")
+		// Sleep a while just to guarantee that the configmap is applied
+		framework.Sleep()
+
+		annotations := map[string]string{
+			"nginx.ingress.kubernetes.io/default-backend":       "default/bla", // low risk
+			"nginx.ingress.kubernetes.io/denylist-source-range": "1.1.1.1/32",  // medium risk
+		}
+
+		ginkgo.By("allow ingress with low/medium risk annotations")
+		ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, annotations)
+		_, err := f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Create(context.TODO(), ing, metav1.CreateOptions{})
+		assert.Nil(ginkgo.GinkgoT(), err, "creating ingress with allowed annotations should not trigger an error")
+
+		ginkgo.By("block ingress with risky annotations")
+		annotations["nginx.ingress.kubernetes.io/modsecurity-transaction-id"] = "bla123"      // High should be blocked
+		annotations["nginx.ingress.kubernetes.io/modsecurity-snippet"] = "some random stuff;" // High should be blocked
+		ing = framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, annotations)
+		_, err = f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Update(context.TODO(), ing, metav1.UpdateOptions{})
+		assert.NotNil(ginkgo.GinkgoT(), err, "creating ingress with risky annotations should trigger an error")
 
-		f.HTTPTestClient().
-			GET("/").
-			WithHeader("Host", host).
-			WithBasicAuth("foo", "bar").
-			Expect().
-			Status(http.StatusOK)
 	})
 })