Fix reviews and fcgi e2e

This commit is contained in:
Ricardo Katz 2023-06-18 13:56:48 +00:00 committed by k8s-infra-cherrypick-robot
parent f89e7c4e35
commit 82c27f12a8
5 changed files with 14 additions and 15 deletions

View file

@ -27,7 +27,6 @@ import (
"k8s.io/client-go/tools/cache"
"k8s.io/ingress-nginx/internal/ingress/annotations/parser"
"k8s.io/ingress-nginx/internal/ingress/errors"
ing_errors "k8s.io/ingress-nginx/internal/ingress/errors"
"k8s.io/ingress-nginx/internal/ingress/resolver"
"k8s.io/ingress-nginx/pkg/util/sets"
@ -111,7 +110,7 @@ var authReqAnnotations = parser.Annotation{
Documentation: `This annotation specifies a duration in seconds which an idle keepalive connection to an upstream server will stay open`,
},
authReqCacheDuration: {
Validator: parser.ValidateRegex(*parser.ExtendedChars, false),
Validator: parser.ValidateRegex(*parser.ExtendedCharsRegex, false),
Scope: parser.AnnotationScopeLocation,
Risk: parser.AnnotationRiskMedium,
Documentation: `This annotation allows to specify a caching time for auth responses based on their response codes, e.g. 200 202 30m`,
@ -306,7 +305,7 @@ func (a authReq) Parse(ing *networking.Ingress) (interface{}, error) {
authMethod, err := parser.GetStringAnnotation(authReqMethodAnnotation, ing, a.annotationConfig.Annotations)
if err != nil {
if errors.IsValidationError(err) {
if ing_errors.IsValidationError(err) {
return nil, ing_errors.NewLocationDenied("invalid HTTP method")
}
}
@ -314,7 +313,7 @@ func (a authReq) Parse(ing *networking.Ingress) (interface{}, error) {
// Optional Parameters
signIn, err := parser.GetStringAnnotation(authReqSigninAnnotation, ing, a.annotationConfig.Annotations)
if err != nil {
if errors.IsValidationError(err) {
if ing_errors.IsValidationError(err) {
klog.Warningf("%s value is invalid: %s", authReqSigninAnnotation, err)
}
klog.V(3).InfoS("auth-signin annotation is undefined and will not be set")
@ -322,7 +321,7 @@ func (a authReq) Parse(ing *networking.Ingress) (interface{}, error) {
signInRedirectParam, err := parser.GetStringAnnotation(authReqSigninRedirParamAnnotation, ing, a.annotationConfig.Annotations)
if err != nil {
if errors.IsValidationError(err) {
if ing_errors.IsValidationError(err) {
klog.Warningf("%s value is invalid: %s", authReqSigninRedirParamAnnotation, err)
}
klog.V(3).Infof("auth-signin-redirect-param annotation is undefined and will not be set")
@ -335,7 +334,7 @@ func (a authReq) Parse(ing *networking.Ingress) (interface{}, error) {
authCacheKey, err := parser.GetStringAnnotation(authReqCacheKeyAnnotation, ing, a.annotationConfig.Annotations)
if err != nil {
if errors.IsValidationError(err) {
if ing_errors.IsValidationError(err) {
klog.Warningf("%s value is invalid: %s", authReqCacheKeyAnnotation, err)
}
klog.V(3).InfoS("auth-cache-key annotation is undefined and will not be set")
@ -379,7 +378,7 @@ func (a authReq) Parse(ing *networking.Ingress) (interface{}, error) {
}
durstr, err := parser.GetStringAnnotation(authReqCacheDuration, ing, a.annotationConfig.Annotations)
if err != nil && errors.IsValidationError(err) {
if err != nil && ing_errors.IsValidationError(err) {
return nil, fmt.Errorf("%s contains invalid value", authReqCacheDuration)
}
authCacheDuration, err := ParseStringToCacheDurations(durstr)
@ -389,7 +388,7 @@ func (a authReq) Parse(ing *networking.Ingress) (interface{}, error) {
responseHeaders := []string{}
hstr, err := parser.GetStringAnnotation(authReqResponseHeadersAnnotation, ing, a.annotationConfig.Annotations)
if err != nil && errors.IsValidationError(err) {
if err != nil && ing_errors.IsValidationError(err) {
return nil, ing_errors.NewLocationDenied("validation error")
}
if len(hstr) != 0 {
@ -445,12 +444,12 @@ func (a authReq) Parse(ing *networking.Ingress) (interface{}, error) {
}
requestRedirect, err := parser.GetStringAnnotation(authReqRequestRedirectAnnotation, ing, a.annotationConfig.Annotations)
if err != nil && errors.IsValidationError(err) {
if err != nil && ing_errors.IsValidationError(err) {
return nil, fmt.Errorf("%s is invalid: %w", authReqRequestRedirectAnnotation, err)
}
alwaysSetCookie, err := parser.GetBoolAnnotation(authReqAlwaysSetCookieAnnotation, ing, a.annotationConfig.Annotations)
if err != nil && errors.IsValidationError(err) {
if err != nil && ing_errors.IsValidationError(err) {
return nil, fmt.Errorf("%s is invalid: %w", authReqAlwaysSetCookieAnnotation, err)
}

View file

@ -34,7 +34,7 @@ var globalAuthAnnotations = parser.Annotation{
Validator: parser.ValidateBool,
Scope: parser.AnnotationScopeLocation,
Risk: parser.AnnotationRiskLow,
Documentation: `Defines if the gloabl external authentication should be enabled.`,
Documentation: `Defines if the global external authentication should be enabled.`,
},
},
}

View file

@ -63,7 +63,7 @@ var (
// This combination can also be used on fields that may contain characters like / (as ns/name)
BasicCharsRegex = regexp.MustCompile("^[/" + alphaNumericChars + "]*$")
// ExtendedChars is alphanumeric and ".", "-", "_", "~" and ":" plus "," and spaces, usually used on simple host:port/path composition
ExtendedChars = regexp.MustCompile("^[/" + extendedAlphaNumeric + "]*$")
ExtendedCharsRegex = regexp.MustCompile("^[/" + extendedAlphaNumeric + "]*$")
// CharsWithSpace is like basic chars, but includes the space character
CharsWithSpace = regexp.MustCompile("^[/" + alphaNumericChars + " ]*$")
// NGINXVariable allows entries with alphanumeric characters, -, _ and the special "$"

View file

@ -221,7 +221,7 @@ func (p proxySSL) Parse(ing *networking.Ingress) (interface{}, error) {
config.Protocols, err = parser.GetStringAnnotation(proxySSLProtocolsAnnotation, ing, p.annotationConfig.Annotations)
if err != nil {
if errors.IsValidationError(err) {
klog.Warningf("invalid value passed to proxy-ssl-protocolos, defaulting to %s", defaultProxySSLProtocols)
klog.Warningf("invalid value passed to proxy-ssl-protocols, defaulting to %s", defaultProxySSLProtocols)
}
config.Protocols = defaultProxySSLProtocols
} else {

View file

@ -75,7 +75,7 @@ var _ = framework.DescribeAnnotation("backend-protocol - FastCGI", func() {
Namespace: f.Namespace,
},
Data: map[string]string{
"SCRIPT_FILENAME": "/home/www/scripts/php$fastcgi_script_name",
"SCRIPT_FILENAME": "$fastcgi_script_name",
"REDIRECT_STATUS": "200",
},
}
@ -94,7 +94,7 @@ var _ = framework.DescribeAnnotation("backend-protocol - FastCGI", func() {
f.WaitForNginxServer(host,
func(server string) bool {
return strings.Contains(server, "fastcgi_param SCRIPT_FILENAME \"/home/www/scripts/php$fastcgi_script_name\";") &&
return strings.Contains(server, "fastcgi_param SCRIPT_FILENAME \"$fastcgi_script_name\";") &&
strings.Contains(server, "fastcgi_param REDIRECT_STATUS \"200\";")
})
})