Skip to content

Commit

Permalink
feat(annotations): introduce enable-custom-http-errors annotation
Browse files Browse the repository at this point in the history
Signed-off-by: GitHub <noreply@github.com>
  • Loading branch information
aslafy-z authored Jul 7, 2024
1 parent 973c1c9 commit 7151e14
Show file tree
Hide file tree
Showing 5 changed files with 140 additions and 31 deletions.
5 changes: 5 additions & 0 deletions docs/user-guide/nginx-configuration/annotations.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ You can add these Kubernetes annotations to specific Ingress objects to customiz
|[nginx.ingress.kubernetes.io/canary-weight-total](#canary)|number|
|[nginx.ingress.kubernetes.io/client-body-buffer-size](#client-body-buffer-size)|string|
|[nginx.ingress.kubernetes.io/configuration-snippet](#configuration-snippet)|string|
|[nginx.ingress.kubernetes.io/enable-custom-http-errors](#custom-http-errors)|"true" or "false"|
|[nginx.ingress.kubernetes.io/custom-http-errors](#custom-http-errors)|[]int|
|[nginx.ingress.kubernetes.io/custom-headers](#custom-headers)|string|
|[nginx.ingress.kubernetes.io/default-backend](#default-backend)|string|
Expand Down Expand Up @@ -332,6 +333,10 @@ Like the [`custom-http-errors`](./configmap.md#custom-http-errors) value in the
Different ingresses can specify different sets of error codes. Even if multiple ingress objects share the same hostname, this annotation can be used to intercept different error codes for each ingress (for example, different error codes to be intercepted for different paths on the same hostname, if each path is on a different ingress).
If `custom-http-errors` is also specified globally, the error values specified in this annotation will override the global value for the given ingress' hostname and path.

If you want to disable this behavior for a specific ingress, you can use the annotation `nginx.ingress.kubernetes.io/enable-custom-http-errors: "false"`.
`nginx.ingress.kubernetes.io/enable-custom-http-errors`:
indicates if the custom http errors feature should be enabled or not to this Ingress rule. The default value is `"true"`.

Example usage:
```
nginx.ingress.kubernetes.io/custom-http-errors: "404,415"
Expand Down
39 changes: 24 additions & 15 deletions internal/ingress/annotations/annotations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,21 +29,22 @@ import (
)

var (
annotationPassthrough = parser.GetAnnotationWithPrefix("ssl-passthrough")
annotationAffinityType = parser.GetAnnotationWithPrefix("affinity")
annotationAffinityMode = parser.GetAnnotationWithPrefix("affinity-mode")
annotationAffinityCanaryBehavior = parser.GetAnnotationWithPrefix("affinity-canary-behavior")
annotationCorsEnabled = parser.GetAnnotationWithPrefix("enable-cors")
annotationCorsAllowMethods = parser.GetAnnotationWithPrefix("cors-allow-methods")
annotationCorsAllowHeaders = parser.GetAnnotationWithPrefix("cors-allow-headers")
annotationCorsExposeHeaders = parser.GetAnnotationWithPrefix("cors-expose-headers")
annotationCorsAllowCredentials = parser.GetAnnotationWithPrefix("cors-allow-credentials")
defaultCorsMethods = "GET, PUT, POST, DELETE, PATCH, OPTIONS"
defaultCorsHeaders = "DNT,Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Range,Authorization"
annotationAffinityCookieName = parser.GetAnnotationWithPrefix("session-cookie-name")
annotationUpstreamHashBy = parser.GetAnnotationWithPrefix("upstream-hash-by")
annotationCustomHTTPErrors = parser.GetAnnotationWithPrefix("custom-http-errors")
annotationCustomHeaders = parser.GetAnnotationWithPrefix("custom-headers")
annotationPassthrough = parser.GetAnnotationWithPrefix("ssl-passthrough")
annotationAffinityType = parser.GetAnnotationWithPrefix("affinity")
annotationAffinityMode = parser.GetAnnotationWithPrefix("affinity-mode")
annotationAffinityCanaryBehavior = parser.GetAnnotationWithPrefix("affinity-canary-behavior")
annotationCorsEnabled = parser.GetAnnotationWithPrefix("enable-cors")
annotationCorsAllowMethods = parser.GetAnnotationWithPrefix("cors-allow-methods")
annotationCorsAllowHeaders = parser.GetAnnotationWithPrefix("cors-allow-headers")
annotationCorsExposeHeaders = parser.GetAnnotationWithPrefix("cors-expose-headers")
annotationCorsAllowCredentials = parser.GetAnnotationWithPrefix("cors-allow-credentials")
defaultCorsMethods = "GET, PUT, POST, DELETE, PATCH, OPTIONS"
defaultCorsHeaders = "DNT,Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Range,Authorization"
annotationAffinityCookieName = parser.GetAnnotationWithPrefix("session-cookie-name")
annotationUpstreamHashBy = parser.GetAnnotationWithPrefix("upstream-hash-by")
annotationCustomHTTPErrorsEnabled = parser.GetAnnotationWithPrefix("enable-custom-http-errors")
annotationCustomHTTPErrors = parser.GetAnnotationWithPrefix("custom-http-errors")
annotationCustomHeaders = parser.GetAnnotationWithPrefix("custom-headers")
)

type mockCfg struct {
Expand Down Expand Up @@ -298,6 +299,14 @@ func TestCustomHTTPErrors(t *testing.T) {
{map[string]string{annotationCustomHTTPErrors: "404"}, []int{404}},
{map[string]string{annotationCustomHTTPErrors: ""}, []int{}},
{map[string]string{annotationCustomHTTPErrors + "_no": "404"}, []int{}},
{map[string]string{annotationCustomHTTPErrorsEnabled: "true", annotationCustomHTTPErrors: "404,415"}, []int{404, 415}},
{map[string]string{annotationCustomHTTPErrorsEnabled: "true", annotationCustomHTTPErrors: "404"}, []int{404}},
{map[string]string{annotationCustomHTTPErrorsEnabled: "true", annotationCustomHTTPErrors: ""}, []int{}},
{map[string]string{annotationCustomHTTPErrorsEnabled: "true", annotationCustomHTTPErrors + "_no": "404"}, []int{}},
{map[string]string{annotationCustomHTTPErrorsEnabled: "false", annotationCustomHTTPErrors: "404,415"}, []int{}},
{map[string]string{annotationCustomHTTPErrorsEnabled: "false", annotationCustomHTTPErrors: "404"}, []int{}},
{map[string]string{annotationCustomHTTPErrorsEnabled: "false", annotationCustomHTTPErrors: ""}, []int{}},
{map[string]string{annotationCustomHTTPErrorsEnabled: "false", annotationCustomHTTPErrors + "_no": "404"}, []int{}},
{map[string]string{}, []int{}},
{nil, []int{}},
}
Expand Down
26 changes: 24 additions & 2 deletions internal/ingress/annotations/customhttperrors/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,16 @@ import (
"strings"

networking "k8s.io/api/networking/v1"
"k8s.io/klog/v2"

"k8s.io/ingress-nginx/internal/ingress/annotations/parser"
"k8s.io/ingress-nginx/internal/ingress/errors"
"k8s.io/ingress-nginx/internal/ingress/resolver"
)

const (
customHTTPErrorsAnnotation = "custom-http-errors"
customHTTPErrorsAnnotation = "custom-http-errors"
customHTTPErrorsEnabledAnnotation = "enable-custom-http-errors"
)

// We accept anything between 400 and 599, on a comma separated.
Expand All @@ -37,13 +40,20 @@ var arrayOfHTTPErrors = regexp.MustCompile(`^(?:[4,5]\d{2},?)*$`)
var customHTTPErrorsAnnotations = parser.Annotation{
Group: "backend",
Annotations: parser.AnnotationFields{
customHTTPErrorsEnabledAnnotation: {
Validator: parser.ValidateBool,
Scope: parser.AnnotationScopeLocation,
Risk: parser.AnnotationRiskLow,
Documentation: `Indicates if the custom http errors feature should be enabled or not for this Ingress rule. The default value is "true".`,
},

customHTTPErrorsAnnotation: {
Validator: parser.ValidateRegex(arrayOfHTTPErrors, true),
Scope: parser.AnnotationScopeLocation,
Risk: parser.AnnotationRiskLow,
Documentation: `If a default backend annotation is specified on the ingress, the errors code specified on this annotation
will be routed to that annotation's default backend service. Otherwise they will be routed to the global default backend.
A comma-separated list of error codes is accepted (anything between 400 and 599, like 403, 503)`,
A comma-separated list of error codes is accepted (anything between 400 and 599, like 403, 503).`,
},
},
}
Expand All @@ -64,6 +74,18 @@ func NewParser(r resolver.Resolver) parser.IngressAnnotation {
// Parse parses the annotations contained in the ingress to use
// custom http errors
func (e customhttperrors) Parse(ing *networking.Ingress) (interface{}, error) {
enabled, err := parser.GetBoolAnnotation(customHTTPErrorsEnabledAnnotation, ing, e.annotationConfig.Annotations)
if err != nil {
if errors.IsValidationError(err) {
klog.Warningf("%s is invalid, defaulting to 'true'", customHTTPErrorsEnabledAnnotation)
}
enabled = true
}

if !enabled {
return []int{}, nil
}

c, err := parser.GetStringAnnotation(customHTTPErrorsAnnotation, ing, e.annotationConfig.Annotations)
if err != nil {
return nil, err
Expand Down
73 changes: 60 additions & 13 deletions internal/ingress/annotations/customhttperrors/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,31 +47,55 @@ func buildIngress() *networking.Ingress {
}
}

func TestParseInvalidAnnotations(t *testing.T) {
func TestParseAnnotations(t *testing.T) {
ing := buildIngress()

_, err := NewParser(&resolver.Mock{}).Parse(ing)
if err == nil {
t.Errorf("expected error parsing ingress with custom-http-errors")
data := map[string]string{}
data[parser.GetAnnotationWithPrefix("custom-http-errors")] = "400,404,500,502"

Check failure on line 53 in internal/ingress/annotations/customhttperrors/main_test.go

View workflow job for this annotation

GitHub Actions / lint

string `400,404,500,502` has 3 occurrences, make it a constant (goconst)
ing.SetAnnotations(data)
i, err := NewParser(&resolver.Mock{}).Parse(ing)
if err != nil {
t.Errorf("unexpected error parsing ingress with custom-http-errors")
}
val, ok := i.([]int)
if !ok {
t.Errorf("expected a []int type")
}
expected := []int{400, 404, 500, 502}
sort.Ints(val)
if !reflect.DeepEqual(expected, val) {
t.Errorf("expected %v but got %v", expected, val)
}
}

func TestParseEnabledAnnotations(t *testing.T) {
ing := buildIngress()

data := map[string]string{}
data[parser.GetAnnotationWithPrefix("custom-http-errors")] = "400,404,abc,502"
data[parser.GetAnnotationWithPrefix("enable-custom-http-errors")] = "true"
data[parser.GetAnnotationWithPrefix("custom-http-errors")] = "400,404,500,502"
ing.SetAnnotations(data)

i, err := NewParser(&resolver.Mock{}).Parse(ing)
if err == nil {
t.Errorf("expected error parsing ingress with custom-http-errors")
if err != nil {
t.Errorf("unexpected error parsing ingress with custom-http-errors")
}
val, ok := i.([]int)
if !ok {
t.Errorf("expected a []int type")
}
if i != nil {
t.Errorf("expected %v but got %v", nil, i)

expected := []int{}

if !reflect.DeepEqual(expected, val) {
t.Errorf("expected %v but got %v", expected, val)
}
}

func TestParseAnnotations(t *testing.T) {
func TestParseDisabledAnnotations(t *testing.T) {
ing := buildIngress()

data := map[string]string{}
data[parser.GetAnnotationWithPrefix("enable-custom-http-errors")] = "false"
data[parser.GetAnnotationWithPrefix("custom-http-errors")] = "400,404,500,502"
ing.SetAnnotations(data)

Expand All @@ -84,8 +108,31 @@ func TestParseAnnotations(t *testing.T) {
t.Errorf("expected a []int type")
}

expected := []int{400, 404, 500, 502}
sort.Ints(val)
expected := []int{}

if !reflect.DeepEqual(expected, val) {
t.Errorf("expected %v but got %v", expected, val)
}
}

func TestParseInvalidAnnotations(t *testing.T) {
ing := buildIngress()

data := map[string]string{}
data[parser.GetAnnotationWithPrefix("enable-custom-http-errors")] = "fakebool"
data[parser.GetAnnotationWithPrefix("custom-http-errors")] = "400,404,fakeint,502"
ing.SetAnnotations(data)

i, err := NewParser(&resolver.Mock{}).Parse(ing)
if err != nil {
t.Errorf("unexpected error parsing ingress with custom-http-errors")
}
val, ok := i.([]int)
if !ok {
t.Errorf("expected a []int type")
}

expected := []int{}

if !reflect.DeepEqual(expected, val) {
t.Errorf("expected %v but got %v", expected, val)
Expand Down
28 changes: 27 additions & 1 deletion test/e2e/annotations/customhttperrors.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ var _ = framework.DescribeAnnotation("custom-http-errors", func() {
f.NewEchoDeployment()
})

ginkgo.It("configures Nginx correctly", func() {
ginkgo.It("configures Nginx correctly when enabled", func() {
host := "customerrors.foo.com"

errorCodes := []string{"404", "500"}
Expand Down Expand Up @@ -119,4 +119,30 @@ var _ = framework.DescribeAnnotation("custom-http-errors", func() {
assert.Contains(ginkgo.GinkgoT(), serverConfig, errorBlockName(fmt.Sprintf("custom-default-backend-%s-%s", f.Namespace, customDefaultBackend), "503"))
assert.Contains(ginkgo.GinkgoT(), serverConfig, fmt.Sprintf("error_page %s = %s", "503", errorBlockName(fmt.Sprintf("custom-default-backend-%s-%s", f.Namespace, customDefaultBackend), "503")))
})

ginkgo.It("configures Nginx correctly when disabled", func() {
host := "customerrors.foo.com"

annotations := map[string]string{
"nginx.ingress.kubernetes.io/enable-custom-http-errors": "false",
"nginx.ingress.kubernetes.io/custom-http-errors": "404, 503",
}

ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, annotations)
f.EnsureIngress(ing)

var serverConfig string
f.WaitForNginxServer(host, func(sc string) bool {
serverConfig = sc
return strings.Contains(serverConfig, fmt.Sprintf("server_name %s", host))
})

f.UpdateNginxConfigMapData("custom-http-errors", "404, 503")

ginkgo.By("not turning on proxy_intercept_errors directive")
assert.NotContains(ginkgo.GinkgoT(), serverConfig, "proxy_intercept_errors on;")

ginkgo.By("not creating error locations")
assert.NotContains(ginkgo.GinkgoT(), serverConfig, fmt.Sprintf("location %s", errorBlockName("upstream-default-backend", "off")))
})
})

0 comments on commit 7151e14

Please sign in to comment.