diff --git a/docs/content/configuration/ingress-resources/advanced-configuration-with-annotations.md b/docs/content/configuration/ingress-resources/advanced-configuration-with-annotations.md index 77f505853c..4e0fba9523 100644 --- a/docs/content/configuration/ingress-resources/advanced-configuration-with-annotations.md +++ b/docs/content/configuration/ingress-resources/advanced-configuration-with-annotations.md @@ -83,7 +83,6 @@ Note how the events section includes a Warning event with the Rejected reason. The following Ingress annotations currently have limited or no validation: -- `nginx.org/rewrites`, - `nginx.com/jwt-key`, - `nginx.com/jwt-realm`, - `nginx.com/jwt-token`, @@ -131,7 +130,7 @@ The table below summarizes the available annotations. | ---| ---| ---| ---| --- | |``nginx.org/proxy-hide-headers`` | ``proxy-hide-headers`` | Sets the value of one or more [proxy_hide_header](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_hide_header) directives. Example: ``"nginx.org/proxy-hide-headers": "header-a,header-b"`` | N/A | | |``nginx.org/proxy-pass-headers`` | ``proxy-pass-headers`` | Sets the value of one or more [proxy_pass_header](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_pass_header) directives. Example: ``"nginx.org/proxy-pass-headers": "header-a,header-b"`` | N/A | | -|``nginx.org/rewrites`` | N/A | Configures URI rewriting. | N/A | [Rewrites Support](https://github.com/nginxinc/kubernetes-ingress/tree/v2.2.2/examples/rewrites). | +|``nginx.org/rewrites`` | N/A | Configures URI rewriting using [proxy_pass](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_pass) directive. | N/A | [Rewrites Support](https://github.com/nginxinc/kubernetes-ingress/tree/v2.2.2/examples/rewrites). | {{% /table %}} ### Auth and SSL/TLS diff --git a/internal/configs/parsing_helpers.go b/internal/configs/parsing_helpers.go index 95cde94a47..660fdabc46 100644 --- a/internal/configs/parsing_helpers.go +++ b/internal/configs/parsing_helpers.go @@ -333,28 +333,38 @@ func parseRewrites(service string) (serviceName string, rewrite string, err erro parts := strings.SplitN(strings.TrimSpace(service), " ", 2) if len(parts) != 2 { - return "", "", fmt.Errorf("Invalid rewrite format: %s", service) + return "", "", fmt.Errorf("'%s' is not a valid rewrite format, e.g. 'serviceName=tea-svc rewrite=/'", service) } svcNameParts := strings.Split(parts[0], "=") - if len(svcNameParts) != 2 { - return "", "", fmt.Errorf("Invalid rewrite format: %s", svcNameParts) + if len(svcNameParts) != 2 || svcNameParts[0] != "serviceName" { + return "", "", fmt.Errorf("'%s' is not a valid serviceName format, e.g. 'serviceName=tea-svc'", parts[0]) } rwPathParts := strings.Split(parts[1], "=") - if len(rwPathParts) != 2 { - return "", "", fmt.Errorf("Invalid rewrite format: %s", rwPathParts) + if len(rwPathParts) != 2 || rwPathParts[0] != "rewrite" { + return "", "", fmt.Errorf("'%s' is not a valid rewrite path format, e.g. 'rewrite=/tea'", parts[1]) + } + + if !VerifyPath(rwPathParts[1]) { + return "", "", fmt.Errorf("path must start with '/' and must not include any whitespace character, '{', '}' or '$': '%s'", rwPathParts[1]) } return svcNameParts[1], rwPathParts[1], nil } var ( - threshEx = regexp.MustCompile(`high=([1-9]|[1-9][0-9]|100) low=([1-9]|[1-9][0-9]|100)\b`) - threshExR = regexp.MustCompile(`low=([1-9]|[1-9][0-9]|100) high=([1-9]|[1-9][0-9]|100)\b`) + threshEx = regexp.MustCompile(`high=([1-9]|[1-9][0-9]|100) low=([1-9]|[1-9][0-9]|100)\b`) + threshExR = regexp.MustCompile(`low=([1-9]|[1-9][0-9]|100) high=([1-9]|[1-9][0-9]|100)\b`) + pathRegexp = regexp.MustCompile("^" + `/[^\s{};$]*` + "$") ) // VerifyAppProtectThresholds ensures that threshold values are set correctly func VerifyAppProtectThresholds(value string) bool { return threshEx.MatchString(value) || threshExR.MatchString(value) } + +// VerifyPath ensures that rewrite paths are in the correct format +func VerifyPath(s string) bool { + return pathRegexp.MatchString(s) +} diff --git a/internal/k8s/validation.go b/internal/k8s/validation.go index fc463e0857..f23392c4b4 100644 --- a/internal/k8s/validation.go +++ b/internal/k8s/validation.go @@ -589,8 +589,22 @@ func validateStickyServiceListAnnotation(context *annotationValidationContext) f func validateRewriteListAnnotation(context *annotationValidationContext) field.ErrorList { allErrs := field.ErrorList{} - if _, err := configs.ParseRewriteList(context.value); err != nil { - return append(allErrs, field.Invalid(context.fieldPath, context.value, "must be a semicolon-separated list of rewrites")) + var unknownServices []string + rewrites, err := configs.ParseRewriteList(context.value) + if err != nil { + return append(allErrs, field.Invalid(context.fieldPath, context.value, err.Error())) + } + for rewrite := range rewrites { + if _, exists := context.specServices[rewrite]; !exists { + unknownServices = append(unknownServices, rewrite) + } + } + if len(unknownServices) > 0 { + errorMsg := fmt.Sprintf( + "The following services were not found: %s", + strings.Join(unknownServices, commaDelimiter), + ) + return append(allErrs, field.Invalid(context.fieldPath, context.value, errorMsg)) } return allErrs } diff --git a/internal/k8s/validation_test.go b/internal/k8s/validation_test.go index b6915facec..37638df845 100644 --- a/internal/k8s/validation_test.go +++ b/internal/k8s/validation_test.go @@ -2003,9 +2003,11 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { { annotations: map[string]string{ - "nginx.org/rewrites": "serviceName=service-1 rewrite=rewrite-1", + "nginx.org/rewrites": "serviceName=service-1 rewrite=/rewrite-1", + }, + specServices: map[string]bool{ + "service-1": true, }, - specServices: map[string]bool{}, isPlus: false, appProtectEnabled: false, appProtectDosEnabled: false, @@ -2015,16 +2017,204 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { }, { annotations: map[string]string{ - "nginx.org/rewrites": "serviceName=service-1 rewrite=rewrite-1;serviceName=service-2 rewrite=rewrite-2", + "nginx.org/rewrites": "serviceName=service-1 rewrite=/rewrite-1/", + }, + specServices: map[string]bool{ + "service-1": true, + }, + isPlus: false, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/rewrites annotation, single-value, trailing '/'", + }, + { + annotations: map[string]string{ + "nginx.org/rewrites": "serviceName=service-1 rewrite=/rewrite-1/rewrite", + }, + specServices: map[string]bool{ + "service-1": true, + }, isPlus: false, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/rewrites annotation, single-value, uri levels", + }, + { + annotations: map[string]string{ + "nginx.org/rewrites": "serviceName=service-1 rewrite=rewrite-1", + }, + specServices: map[string]bool{ + "service-1": true, + }, + isPlus: false, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.org/rewrites: Invalid value: "serviceName=service-1 rewrite=rewrite-1": path must start with '/' and must not include any whitespace character, '{', '}' or '$': 'rewrite-1'`, + }, + msg: "invalid nginx.org/rewrites annotation, single-value, no '/' in the beginning", + }, + { + annotations: map[string]string{ + "nginx.org/rewrites": "service-1 rewrite=/rewrite-1", + }, + specServices: map[string]bool{ + "service-1": true, + }, + isPlus: false, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.org/rewrites: Invalid value: "service-1 rewrite=/rewrite-1": 'service-1' is not a valid serviceName format, e.g. 'serviceName=tea-svc'`, + }, + msg: "invalid nginx.org/rewrites annotation, single-value, invalid service name format, 'serviceName' missing", + }, + { + annotations: map[string]string{ + "nginx.org/rewrites": "serviceName1=service-1 rewrite=/rewrite-1", + }, + specServices: map[string]bool{ + "service-1": true, + }, + isPlus: false, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.org/rewrites: Invalid value: "serviceName1=service-1 rewrite=/rewrite-1": 'serviceName1=service-1' is not a valid serviceName format, e.g. 'serviceName=tea-svc'`, + }, + msg: "invalid nginx.org/rewrites annotation, single-value, invalid service name format, 'serviceName' typo", + }, + { + annotations: map[string]string{ + "nginx.org/rewrites": "serviceName=service-1 rewrit=/rewrite-1", + }, + specServices: map[string]bool{ + "service-1": true, + }, + isPlus: false, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.org/rewrites: Invalid value: "serviceName=service-1 rewrit=/rewrite-1": 'rewrit=/rewrite-1' is not a valid rewrite path format, e.g. 'rewrite=/tea'`, + }, + msg: "invalid nginx.org/rewrites annotation, single-value, invalid service name format, 'rewrite' typo ", + }, + { + annotations: map[string]string{ + "nginx.org/rewrites": "serviceName=service-1 rewrite=/rewrite", }, specServices: map[string]bool{}, isPlus: false, appProtectEnabled: false, appProtectDosEnabled: false, internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.org/rewrites: Invalid value: "serviceName=service-1 rewrite=/rewrite": The following services were not found: service-1`, + }, + msg: "invaild nginx.org/rewrites annotation, single-value, service does not exist", + }, + { + annotations: map[string]string{ + "nginx.org/rewrites": "serviceName=service-1 rewrite=/rewrite-{1}", + }, + specServices: map[string]bool{ + "service-1": true, + }, + isPlus: false, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.org/rewrites: Invalid value: "serviceName=service-1 rewrite=/rewrite-{1}": path must start with '/' and must not include any whitespace character, '{', '}' or '$': '/rewrite-{1}'`, + }, + msg: "invalid nginx.org/rewrites annotation, single-value, path containing special characters", + }, + { + annotations: map[string]string{ + "nginx.org/rewrites": "serviceName=service-1 rewrite=/rewr ite", + }, + specServices: map[string]bool{ + "service-1": true, + }, + isPlus: false, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.org/rewrites: Invalid value: "serviceName=service-1 rewrite=/rewr ite": path must start with '/' and must not include any whitespace character, '{', '}' or '$': '/rewr ite'`, + }, + msg: "invalid nginx.org/rewrites annotation, single-value, path containing white spaces", + }, + { + annotations: map[string]string{ + "nginx.org/rewrites": "serviceName=service-1 rewrite=/rewrite/$1", + }, + specServices: map[string]bool{ + "service-1": true, + }, isPlus: false, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.org/rewrites: Invalid value: "serviceName=service-1 rewrite=/rewrite/$1": path must start with '/' and must not include any whitespace character, '{', '}' or '$': '/rewrite/$1'`, + }, + msg: "invaild nginx.org/rewrites annotation, single-value, path containing regex characters", + }, + { + annotations: map[string]string{ + "nginx.org/rewrites": "serviceName=service-1 rewrite=/rewrite-1;serviceName=service-2 rewrite=/rewrite-2", + }, + specServices: map[string]bool{ + "service-1": true, + "service-2": true, + }, + isPlus: false, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, expectedErrors: nil, msg: "valid nginx.org/rewrites annotation, multi-value", }, + { + annotations: map[string]string{ + "nginx.org/rewrites": "serviceName=service-1 rewrite=/rewrite-1;serviceName=service-2 rewrite=/rewrite-2", + }, + specServices: map[string]bool{ + "service-1": true, + }, + isPlus: false, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.org/rewrites: Invalid value: "serviceName=service-1 rewrite=/rewrite-1;serviceName=service-2 rewrite=/rewrite-2": The following services were not found: service-2`, + }, + msg: "valid nginx.org/rewrites annotation, multi-value, service does not exist", + }, + { + annotations: map[string]string{ + "nginx.org/rewrites": "serviceName=service-1 rewrite=rewrite-1;serviceName=service-2 rewrite=/rewrite-2", + }, + specServices: map[string]bool{ + "service-1": true, + "service-2": true, + }, + isPlus: false, + appProtectEnabled: false, + appProtectDosEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.org/rewrites: Invalid value: "serviceName=service-1 rewrite=rewrite-1;serviceName=service-2 rewrite=/rewrite-2": path must start with '/' and must not include any whitespace character, '{', '}' or '$': 'rewrite-1'`, + }, + msg: "invalid nginx.org/rewrites annotation, multi-value without '/' in the beginning", + }, { annotations: map[string]string{ "nginx.org/rewrites": "not_a_rewrite", @@ -2035,7 +2225,7 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { appProtectDosEnabled: false, internalRoutesEnabled: true, expectedErrors: []string{ - `annotations.nginx.org/rewrites: Invalid value: "not_a_rewrite": must be a semicolon-separated list of rewrites`, + `annotations.nginx.org/rewrites: Invalid value: "not_a_rewrite": 'not_a_rewrite' is not a valid rewrite format, e.g. 'serviceName=tea-svc rewrite=/'`, }, msg: "invalid nginx.org/rewrites annotation", },