diff --git a/internal/configs/annotations.go b/internal/configs/annotations.go index 4afff52db8..d568d8b667 100644 --- a/internal/configs/annotations.go +++ b/internal/configs/annotations.go @@ -1,10 +1,6 @@ package configs import ( - "fmt" - "strconv" - "strings" - "github.com/golang/glog" ) @@ -93,29 +89,41 @@ func parseAnnotations(ingEx *IngressEx, baseCfgParams *ConfigParams, isPlus bool if err != nil { glog.Error(err) } - cfgParams.HealthCheckEnabled = healthCheckEnabled + if isPlus { + cfgParams.HealthCheckEnabled = healthCheckEnabled + } else { + glog.Warning("Annotation 'nginx.com/health-checks' requires NGINX Plus") + } } - if healthCheckMandatory, exists, err := GetMapKeyAsBool(ingEx.Ingress.Annotations, "nginx.com/health-checks-mandatory", ingEx.Ingress); exists { - if err != nil { - glog.Error(err) + if cfgParams.HealthCheckEnabled { + if healthCheckMandatory, exists, err := GetMapKeyAsBool(ingEx.Ingress.Annotations, "nginx.com/health-checks-mandatory", ingEx.Ingress); exists { + if err != nil { + glog.Error(err) + } + cfgParams.HealthCheckMandatory = healthCheckMandatory } - cfgParams.HealthCheckMandatory = healthCheckMandatory } - if healthCheckQueue, exists, err := GetMapKeyAsInt64(ingEx.Ingress.Annotations, "nginx.com/health-checks-mandatory-queue", ingEx.Ingress); exists { - if err != nil { - glog.Error(err) + if cfgParams.HealthCheckMandatory { + if healthCheckQueue, exists, err := GetMapKeyAsInt64(ingEx.Ingress.Annotations, "nginx.com/health-checks-mandatory-queue", ingEx.Ingress); exists { + if err != nil { + glog.Error(err) + } + cfgParams.HealthCheckMandatoryQueue = healthCheckQueue } - cfgParams.HealthCheckMandatoryQueue = healthCheckQueue } if slowStart, exists := ingEx.Ingress.Annotations["nginx.com/slow-start"]; exists { - parsedSlowStart, err := ParseTime(slowStart) - if err != nil { - glog.Error(err) + if parsedSlowStart, err := ParseTime(slowStart); err != nil { + glog.Errorf("Ingress %s/%s: Invalid value nginx.org/slow-start: got %q: %v", ingEx.Ingress.GetNamespace(), ingEx.Ingress.GetName(), slowStart, err) + } else { + if isPlus { + cfgParams.SlowStart = parsedSlowStart + } else { + glog.Warning("Annotation 'nginx.com/slow-start' requires NGINX Plus") + } } - cfgParams.SlowStart = parsedSlowStart } if serverTokens, exists, err := GetMapKeyAsBool(ingEx.Ingress.Annotations, "nginx.org/server-tokens", ingEx.Ingress); exists { @@ -275,13 +283,24 @@ func parseAnnotations(ingEx *IngressEx, baseCfgParams *ConfigParams, isPlus bool } } - ports, sslPorts := getServicesPorts(ingEx) - if len(ports) > 0 { - cfgParams.Ports = ports + if values, exists := ingEx.Ingress.Annotations["nginx.org/listen-ports"]; exists { + ports, err := ParsePortList(values) + if err != nil { + glog.Errorf("In %v nginx.org/listen-ports contains invalid declaration: %v, ignoring", ingEx.Ingress.Name, err) + } + if len(ports) > 0 { + cfgParams.Ports = ports + } } - if len(sslPorts) > 0 { - cfgParams.SSLPorts = sslPorts + if values, exists := ingEx.Ingress.Annotations["nginx.org/listen-ports-ssl"]; exists { + sslPorts, err := ParsePortList(values) + if err != nil { + glog.Errorf("In %v nginx.org/listen-ports-ssl contains invalid declaration: %v, ignoring", ingEx.Ingress.Name, err) + } + if len(sslPorts) > 0 { + cfgParams.SSLPorts = sslPorts + } } if keepalive, exists, err := GetMapKeyAsInt(ingEx.Ingress.Annotations, "nginx.org/keepalive", ingEx.Ingress); exists { @@ -351,99 +370,46 @@ func parseAnnotations(ingEx *IngressEx, baseCfgParams *ConfigParams, isPlus bool } func getWebsocketServices(ingEx *IngressEx) map[string]bool { - wsServices := make(map[string]bool) - - if services, exists := ingEx.Ingress.Annotations["nginx.org/websocket-services"]; exists { - for _, svc := range strings.Split(services, ",") { - wsServices[svc] = true - } + if value, exists := ingEx.Ingress.Annotations["nginx.org/websocket-services"]; exists { + return ParseServiceList(value) } - - return wsServices + return nil } func getRewrites(ingEx *IngressEx) map[string]string { - rewrites := make(map[string]string) - - if services, exists := ingEx.Ingress.Annotations["nginx.org/rewrites"]; exists { - for _, svc := range strings.Split(services, ";") { - if serviceName, rewrite, err := parseRewrites(svc); err != nil { - glog.Errorf("In %v nginx.org/rewrites contains invalid declaration: %v, ignoring", ingEx.Ingress.Name, err) - } else { - rewrites[serviceName] = rewrite - } + if value, exists := ingEx.Ingress.Annotations["nginx.org/rewrites"]; exists { + rewrites, err := ParseRewriteList(value) + if err != nil { + glog.Error(err) } + return rewrites } - - return rewrites + return nil } func getSSLServices(ingEx *IngressEx) map[string]bool { - sslServices := make(map[string]bool) - - if services, exists := ingEx.Ingress.Annotations["nginx.org/ssl-services"]; exists { - for _, svc := range strings.Split(services, ",") { - sslServices[svc] = true - } + if value, exists := ingEx.Ingress.Annotations["nginx.org/ssl-services"]; exists { + return ParseServiceList(value) } - - return sslServices + return nil } func getGrpcServices(ingEx *IngressEx) map[string]bool { - grpcServices := make(map[string]bool) - - if services, exists := ingEx.Ingress.Annotations["nginx.org/grpc-services"]; exists { - for _, svc := range strings.Split(services, ",") { - grpcServices[svc] = true - } + if value, exists := ingEx.Ingress.Annotations["nginx.org/grpc-services"]; exists { + return ParseServiceList(value) } - - return grpcServices + return nil } func getSessionPersistenceServices(ingEx *IngressEx) map[string]string { - spServices := make(map[string]string) - - if services, exists := ingEx.Ingress.Annotations["nginx.com/sticky-cookie-services"]; exists { - for _, svc := range strings.Split(services, ";") { - if serviceName, sticky, err := parseStickyService(svc); err != nil { - glog.Errorf("In %v nginx.com/sticky-cookie-services contains invalid declaration: %v, ignoring", ingEx.Ingress.Name, err) - } else { - spServices[serviceName] = sticky - } - } - } - - return spServices -} - -func getServicesPorts(ingEx *IngressEx) ([]int, []int) { - ports := map[string][]int{} - - annotations := []string{ - "nginx.org/listen-ports", - "nginx.org/listen-ports-ssl", - } - - for _, annotation := range annotations { - if values, exists := ingEx.Ingress.Annotations[annotation]; exists { - for _, value := range strings.Split(values, ",") { - if port, err := parsePort(value); err != nil { - glog.Errorf( - "In %v %s contains invalid declaration: %v, ignoring", - ingEx.Ingress.Name, - annotation, - err, - ) - } else { - ports[annotation] = append(ports[annotation], port) - } - } + if value, exists := ingEx.Ingress.Annotations["nginx.com/sticky-cookie-services"]; exists { + services, err := ParseStickyServiceList(value) + if err != nil { + glog.Error(err) } + return services } - - return ports[annotations[0]], ports[annotations[1]] + return nil } func filterMasterAnnotations(annotations map[string]string) []string { @@ -481,57 +447,3 @@ func mergeMasterAnnotationsIntoMinion(minionAnnotations map[string]string, maste } } } - -func parsePort(value string) (int, error) { - port, err := strconv.ParseInt(value, 10, 16) - if err != nil { - return 0, fmt.Errorf( - "Unable to parse port as integer: %s", - err, - ) - } - - if port <= 0 { - return 0, fmt.Errorf( - "Port number should be greater than zero: %q", - port, - ) - } - - return int(port), nil -} - -func parseStickyService(service string) (serviceName string, stickyCookie string, err error) { - parts := strings.SplitN(service, " ", 2) - - if len(parts) != 2 { - return "", "", fmt.Errorf("Invalid sticky-cookie service format: %s", service) - } - - svcNameParts := strings.Split(parts[0], "=") - if len(svcNameParts) != 2 { - return "", "", fmt.Errorf("Invalid sticky-cookie service format: %s", svcNameParts) - } - - return svcNameParts[1], parts[1], nil -} - -func parseRewrites(service string) (serviceName string, rewrite string, err error) { - parts := strings.SplitN(strings.TrimSpace(service), " ", 2) - - if len(parts) != 2 { - return "", "", fmt.Errorf("Invalid rewrite format: %s", service) - } - - svcNameParts := strings.Split(parts[0], "=") - if len(svcNameParts) != 2 { - return "", "", fmt.Errorf("Invalid rewrite format: %s", svcNameParts) - } - - rwPathParts := strings.Split(parts[1], "=") - if len(rwPathParts) != 2 { - return "", "", fmt.Errorf("Invalid rewrite format: %s", rwPathParts) - } - - return svcNameParts[1], rwPathParts[1], nil -} diff --git a/internal/configs/parsing_helpers.go b/internal/configs/parsing_helpers.go index 937486820d..6be33c07d3 100644 --- a/internal/configs/parsing_helpers.go +++ b/internal/configs/parsing_helpers.go @@ -225,6 +225,102 @@ func ParseSize(s string) (string, error) { return "", errors.New("Invalid size string") } +// ParsePortList ensures that the string is a comma-separated list of port numbers +func ParsePortList(s string) ([]int, error) { + var ports []int + for _, value := range strings.Split(s, ",") { + port, err := parsePort(value) + if err != nil { + return nil, err + } + ports = append(ports, port) + } + return ports, nil +} + +func parsePort(value string) (int, error) { + port, err := strconv.ParseInt(value, 10, 16) + if err != nil { + return 0, fmt.Errorf("Unable to parse port as integer: %s", err) + } + + if port <= 0 { + return 0, fmt.Errorf("Port number should be greater than zero: %q", port) + } + + return int(port), nil +} + +// ParseServiceList ensures that the string is a comma-separated list of services +func ParseServiceList(s string) map[string]bool { + services := make(map[string]bool) + for _, part := range strings.Split(s, ",") { + services[part] = true + } + return services +} + +// ParseRewriteList ensures that the string is a semicolon-separated list of services +func ParseRewriteList(s string) (map[string]string, error) { + rewrites := make(map[string]string) + for _, part := range strings.Split(s, ";") { + serviceName, rewrite, err := parseRewrites(part) + if err != nil { + return nil, err + } + rewrites[serviceName] = rewrite + } + return rewrites, nil +} + +// ParseStickyServiceList ensures that the string is a semicolon-separated list of sticky services +func ParseStickyServiceList(s string) (map[string]string, error) { + services := make(map[string]string) + for _, part := range strings.Split(s, ";") { + serviceName, service, err := parseStickyService(part) + if err != nil { + return nil, err + } + services[serviceName] = service + } + return services, nil +} + +func parseStickyService(service string) (serviceName string, stickyCookie string, err error) { + parts := strings.SplitN(service, " ", 2) + + if len(parts) != 2 { + return "", "", fmt.Errorf("Invalid sticky-cookie service format: %s", service) + } + + svcNameParts := strings.Split(parts[0], "=") + if len(svcNameParts) != 2 { + return "", "", fmt.Errorf("Invalid sticky-cookie service format: %s", svcNameParts) + } + + return svcNameParts[1], parts[1], nil +} + +func parseRewrites(service string) (serviceName string, rewrite string, err error) { + parts := strings.SplitN(strings.TrimSpace(service), " ", 2) + + if len(parts) != 2 { + return "", "", fmt.Errorf("Invalid rewrite format: %s", service) + } + + svcNameParts := strings.Split(parts[0], "=") + if len(svcNameParts) != 2 { + return "", "", fmt.Errorf("Invalid rewrite format: %s", svcNameParts) + } + + rwPathParts := strings.Split(parts[1], "=") + if len(rwPathParts) != 2 { + return "", "", fmt.Errorf("Invalid rewrite format: %s", rwPathParts) + } + + 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`) var threshExR = regexp.MustCompile(`low=([1-9]|[1-9][0-9]|100) high=([1-9]|[1-9][0-9]|100)\b`) diff --git a/internal/k8s/configuration.go b/internal/k8s/configuration.go index 971e65ef06..11f161f65b 100644 --- a/internal/k8s/configuration.go +++ b/internal/k8s/configuration.go @@ -307,13 +307,21 @@ type Configuration struct { appPolicyReferenceChecker *appProtectResourceReferenceChecker appLogConfReferenceChecker *appProtectResourceReferenceChecker - isPlus bool + isPlus bool + appProtectEnabled bool + internalRoutesEnabled bool lock sync.RWMutex } // NewConfiguration creates a new Configuration. -func NewConfiguration(hasCorrectIngressClass func(interface{}) bool, isPlus bool, virtualServerValidator *validation.VirtualServerValidator) *Configuration { +func NewConfiguration( + hasCorrectIngressClass func(interface{}) bool, + isPlus bool, + appProtectEnabled bool, + internalRoutesEnabled bool, + virtualServerValidator *validation.VirtualServerValidator, +) *Configuration { return &Configuration{ hosts: make(map[string]Resource), ingresses: make(map[string]*networking.Ingress), @@ -328,6 +336,8 @@ func NewConfiguration(hasCorrectIngressClass func(interface{}) bool, isPlus bool appPolicyReferenceChecker: newAppProtectResourceReferenceChecker(configs.AppProtectPolicyAnnotation), appLogConfReferenceChecker: newAppProtectResourceReferenceChecker(configs.AppProtectLogConfAnnotation), isPlus: isPlus, + appProtectEnabled: appProtectEnabled, + internalRoutesEnabled: internalRoutesEnabled, } } @@ -342,7 +352,7 @@ func (c *Configuration) AddOrUpdateIngress(ing *networking.Ingress) ([]ResourceC if !c.hasCorrectIngressClass(ing) { delete(c.ingresses, key) } else { - validationError = validateIngress(ing, c.isPlus).ToAggregate() + validationError = validateIngress(ing, c.isPlus, c.appProtectEnabled, c.internalRoutesEnabled).ToAggregate() if validationError != nil { delete(c.ingresses, key) } else { diff --git a/internal/k8s/configuration_test.go b/internal/k8s/configuration_test.go index 5f47c79baa..4481cd7b5b 100644 --- a/internal/k8s/configuration_test.go +++ b/internal/k8s/configuration_test.go @@ -16,8 +16,16 @@ func createTestConfiguration() *Configuration { ingressClass: "nginx", useIngressClassOnly: true, } - isPlus := false - return NewConfiguration(lbc.HasCorrectIngressClass, isPlus, validation.NewVirtualServerValidator(isPlus)) + var isPlus bool + var appProtectEnabled bool + var internalRoutesEnabled bool + return NewConfiguration( + lbc.HasCorrectIngressClass, + isPlus, + appProtectEnabled, + internalRoutesEnabled, + validation.NewVirtualServerValidator(isPlus), + ) } func TestAddIngressForRegularIngress(t *testing.T) { diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 8ad39fbb6e..20524f8ddf 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -289,7 +289,12 @@ func NewLoadBalancerController(input NewLoadBalancerControllerInput) *LoadBalanc confClient: input.ConfClient, } - lbc.configuration = NewConfiguration(lbc.HasCorrectIngressClass, input.IsNginxPlus, input.VirtualServerValidator) + lbc.configuration = NewConfiguration( + lbc.HasCorrectIngressClass, + input.IsNginxPlus, + input.AppProtectEnabled, + input.InternalRoutesEnabled, + input.VirtualServerValidator) lbc.secretStore = secrets.NewLocalSecretStore(lbc.configurator) diff --git a/internal/k8s/validation.go b/internal/k8s/validation.go index 0a6bd0c70e..198217d99b 100644 --- a/internal/k8s/validation.go +++ b/internal/k8s/validation.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "sort" + "strings" "github.com/nginxinc/kubernetes-ingress/internal/configs" networking "k8s.io/api/networking/v1beta1" @@ -12,20 +13,61 @@ import ( ) const ( - mergeableIngressTypeAnnotation = "nginx.org/mergeable-ingress-type" - lbMethodAnnotation = "nginx.org/lb-method" - healthChecksAnnotation = "nginx.com/health-checks" - healthChecksMandatoryAnnotation = "nginx.com/health-checks-mandatory" - healthChecksMandatoryQueueAnnotation = "nginx.com/health-checks-mandatory-queue" - slowStartAnnotation = "nginx.com/slow-start" + mergeableIngressTypeAnnotation = "nginx.org/mergeable-ingress-type" + lbMethodAnnotation = "nginx.org/lb-method" + healthChecksAnnotation = "nginx.com/health-checks" + healthChecksMandatoryAnnotation = "nginx.com/health-checks-mandatory" + healthChecksMandatoryQueueAnnotation = "nginx.com/health-checks-mandatory-queue" + slowStartAnnotation = "nginx.com/slow-start" + serverTokensAnnotation = "nginx.org/server-tokens" + serverSnippetsAnnotation = "nginx.org/server-snippets" + locationSnippetsAnnotation = "nginx.org/location-snippets" + proxyConnectTimeoutAnnotation = "nginx.org/proxy-connect-timeout" + proxyReadTimeoutAnnotation = "nginx.org/proxy-read-timeout" + proxySendTimeoutAnnotation = "nginx.org/proxy-send-timeout" + proxyHideHeadersAnnotation = "nginx.org/proxy-hide-headers" + proxyPassHeadersAnnotation = "nginx.org/proxy-pass-headers" + clientMaxBodySizeAnnotation = "nginx.org/client-max-body-size" + redirectToHTTPSAnnotation = "nginx.org/redirect-to-https" + sslRedirectAnnotation = "ingress.kubernetes.io/ssl-redirect" + proxyBufferingAnnotation = "nginx.org/proxy-buffering" + hstsAnnotation = "nginx.org/hsts" + hstsMaxAgeAnnotation = "nginx.org/hsts-max-age" + hstsIncludeSubdomainsAnnotation = "nginx.org/hsts-include-subdomains" + hstsBehindProxyAnnotation = "nginx.org/hsts-behind-proxy" + proxyBuffersAnnotation = "nginx.org/proxy-buffers" + proxyBufferSizeAnnotation = "nginx.org/proxy-buffer-size" + proxyMaxTempFileSizeAnnotation = "nginx.org/proxy-max-temp-file-size" + upstreamZoneSizeAnnotation = "nginx.org/upstream-zone-size" + jwtRealmAnnotation = "nginx.com/jwt-realm" + jwtKeyAnnotation = "nginx.com/jwt-key" + jwtTokenAnnotation = "nginx.com/jwt-token" + jwtLoginURLAnnotation = "nginx.com/jwt-login-url" + listenPortsAnnotation = "nginx.org/listen-ports" + listenPortsSSLAnnotation = "nginx.org/listen-ports-ssl" + keepaliveAnnotation = "nginx.org/keepalive" + maxFailsAnnotation = "nginx.org/max-fails" + maxConnsAnnotation = "nginx.org/max-conns" + failTimeoutAnnotation = "nginx.org/fail-timeout" + appProtectEnableAnnotation = "appprotect.f5.com/app-protect-enable" + appProtectSecurityLogEnableAnnotation = "appprotect.f5.com/app-protect-security-log-enable" + internalRouteAnnotation = "nsm.nginx.com/internal-route" + websocketServicesAnnotation = "nginx.org/websocket-services" + sslServicesAnnotation = "nginx.org/ssl-services" + grpcServicesAnnotation = "nginx.org/grpc-services" + rewritesAnnotation = "nginx.org/rewrites" + stickyCookieServicesAnnotation = "nginx.com/sticky-cookie-services" ) type annotationValidationContext struct { - annotations map[string]string - name string - value string - isPlus bool - fieldPath *field.Path + annotations map[string]string + specServices map[string]bool + name string + value string + isPlus bool + appProtectEnabled bool + internalRoutesEnabled bool + fieldPath *field.Path } type annotationValidationFunc func(context *annotationValidationContext) field.ErrorList @@ -33,65 +75,157 @@ type annotationValidationConfig map[string][]annotationValidationFunc type validatorFunc func(val string) error var ( - // nginxAnnotationValidations defines the various validations which will be applied in order to each ingress - // annotation for nginx. If any specified validation fails, the remaining validations for that annotation will not - // be run. - nginxAnnotationValidations = annotationValidationConfig{ + // annotationValidations defines the various validations which will be applied in order to each ingress annotation. + // If any specified validation fails, the remaining validations for that annotation will not be run. + annotationValidations = annotationValidationConfig{ mergeableIngressTypeAnnotation: { validateRequiredAnnotation, validateMergeableIngressTypeAnnotation, }, lbMethodAnnotation: { validateRequiredAnnotation, - validateNginxLBMethodAnnotation, + validateLBMethodAnnotation, }, healthChecksAnnotation: { validatePlusOnlyAnnotation, + validateRequiredAnnotation, + validateBoolAnnotation, }, healthChecksMandatoryAnnotation: { validatePlusOnlyAnnotation, + validateRelatedAnnotation(healthChecksAnnotation, validateIsTrue), + validateRequiredAnnotation, + validateBoolAnnotation, }, healthChecksMandatoryQueueAnnotation: { validatePlusOnlyAnnotation, + validateRelatedAnnotation(healthChecksMandatoryAnnotation, validateIsTrue), + validateRequiredAnnotation, + validateUint64Annotation, }, slowStartAnnotation: { validatePlusOnlyAnnotation, + validateRequiredAnnotation, + validateTimeAnnotation, }, - } - nginxAnnotationNames = sortedAnnotationNames(nginxAnnotationValidations) - - // nginxPlusAnnotationValidations defines the various validations which will be applied in order to each ingress - // annotation for nginx plus. If any specified validation fails, the remaining validations for that annotation will - // not be run. - nginxPlusAnnotationValidations = annotationValidationConfig{ - mergeableIngressTypeAnnotation: { + serverTokensAnnotation: { validateRequiredAnnotation, - validateMergeableIngressTypeAnnotation, + validateServerTokensAnnotation, }, - lbMethodAnnotation: { + serverSnippetsAnnotation: {}, + locationSnippetsAnnotation: {}, + proxyConnectTimeoutAnnotation: {}, + proxyReadTimeoutAnnotation: {}, + proxySendTimeoutAnnotation: {}, + proxyHideHeadersAnnotation: {}, + proxyPassHeadersAnnotation: {}, + clientMaxBodySizeAnnotation: {}, + redirectToHTTPSAnnotation: { validateRequiredAnnotation, - validateNginxPlusLBMethodAnnotation, + validateBoolAnnotation, }, - healthChecksAnnotation: { + sslRedirectAnnotation: { validateRequiredAnnotation, validateBoolAnnotation, }, - healthChecksMandatoryAnnotation: { - validateRelatedAnnotation(healthChecksAnnotation, validateIsTrue), + proxyBufferingAnnotation: { validateRequiredAnnotation, validateBoolAnnotation, }, - healthChecksMandatoryQueueAnnotation: { - validateRelatedAnnotation(healthChecksMandatoryAnnotation, validateIsTrue), + hstsAnnotation: { + validateRequiredAnnotation, + validateBoolAnnotation, + }, + hstsMaxAgeAnnotation: { + validateRelatedAnnotation(hstsAnnotation, validateIsBool), validateRequiredAnnotation, - validateNonNegativeIntAnnotation, + validateInt64Annotation, }, - slowStartAnnotation: { + hstsIncludeSubdomainsAnnotation: { + validateRelatedAnnotation(hstsAnnotation, validateIsBool), validateRequiredAnnotation, - validateTimeAnnotation, + validateBoolAnnotation, + }, + hstsBehindProxyAnnotation: { + validateRelatedAnnotation(hstsAnnotation, validateIsBool), + validateRequiredAnnotation, + validateBoolAnnotation, + }, + proxyBuffersAnnotation: {}, + proxyBufferSizeAnnotation: {}, + proxyMaxTempFileSizeAnnotation: {}, + upstreamZoneSizeAnnotation: {}, + jwtRealmAnnotation: { + validatePlusOnlyAnnotation, + }, + jwtKeyAnnotation: { + validatePlusOnlyAnnotation, + }, + jwtTokenAnnotation: { + validatePlusOnlyAnnotation, + }, + jwtLoginURLAnnotation: { + validatePlusOnlyAnnotation, + }, + listenPortsAnnotation: { + validateRequiredAnnotation, + validatePortListAnnotation, + }, + listenPortsSSLAnnotation: { + validateRequiredAnnotation, + validatePortListAnnotation, + }, + keepaliveAnnotation: { + validateRequiredAnnotation, + validateIntAnnotation, + }, + maxFailsAnnotation: { + validateRequiredAnnotation, + validateIntAnnotation, + }, + maxConnsAnnotation: { + validateRequiredAnnotation, + validateIntAnnotation, + }, + failTimeoutAnnotation: {}, + appProtectEnableAnnotation: { + validateAppProtectOnlyAnnotation, + validateRequiredAnnotation, + validateBoolAnnotation, + }, + appProtectSecurityLogEnableAnnotation: { + validateAppProtectOnlyAnnotation, + validateRequiredAnnotation, + validateBoolAnnotation, + }, + internalRouteAnnotation: { + validateInternalRoutesOnlyAnnotation, + validateRequiredAnnotation, + validateBoolAnnotation, + }, + websocketServicesAnnotation: { + validateRequiredAnnotation, + validateServiceListAnnotation, + }, + sslServicesAnnotation: { + validateRequiredAnnotation, + validateServiceListAnnotation, + }, + grpcServicesAnnotation: { + validateRequiredAnnotation, + validateServiceListAnnotation, + }, + rewritesAnnotation: { + validateRequiredAnnotation, + validateRewriteListAnnotation, + }, + stickyCookieServicesAnnotation: { + validatePlusOnlyAnnotation, + validateRequiredAnnotation, + validateStickyServiceListAnnotation, }, } - nginxPlusAnnotationNames = sortedAnnotationNames(nginxPlusAnnotationValidations) + annotationNames = sortedAnnotationNames(annotationValidations) ) func sortedAnnotationNames(annotationValidations annotationValidationConfig) []string { @@ -105,9 +239,21 @@ func sortedAnnotationNames(annotationValidations annotationValidationConfig) []s // validateIngress validate an Ingress resource with rules that our Ingress Controller enforces. // Note that the full validation of Ingress resources is done by Kubernetes. -func validateIngress(ing *networking.Ingress, isPlus bool) field.ErrorList { +func validateIngress( + ing *networking.Ingress, + isPlus bool, + appProtectEnabled bool, + internalRoutesEnabled bool, +) field.ErrorList { allErrs := field.ErrorList{} - allErrs = append(allErrs, validateIngressAnnotations(ing.Annotations, isPlus, field.NewPath("annotations"))...) + allErrs = append(allErrs, validateIngressAnnotations( + ing.Annotations, + getSpecServices(ing.Spec), + isPlus, + appProtectEnabled, + internalRoutesEnabled, + field.NewPath("annotations"), + )...) allErrs = append(allErrs, validateIngressSpec(&ing.Spec, field.NewPath("spec"))...) @@ -120,24 +266,27 @@ func validateIngress(ing *networking.Ingress, isPlus bool) field.ErrorList { return allErrs } -func validateIngressAnnotations(annotations map[string]string, isPlus bool, fieldPath *field.Path) field.ErrorList { +func validateIngressAnnotations( + annotations map[string]string, + specServices map[string]bool, + isPlus bool, + appProtectEnabled bool, + internalRoutesEnabled bool, + fieldPath *field.Path, +) field.ErrorList { allErrs := field.ErrorList{} - var annotationNames []string - if isPlus { - annotationNames = nginxPlusAnnotationNames - } else { - annotationNames = nginxAnnotationNames - } - for _, name := range annotationNames { if value, exists := annotations[name]; exists { context := &annotationValidationContext{ - annotations: annotations, - name: name, - value: value, - isPlus: isPlus, - fieldPath: fieldPath.Child(name), + annotations: annotations, + specServices: specServices, + name: name, + value: value, + isPlus: isPlus, + appProtectEnabled: appProtectEnabled, + internalRoutesEnabled: internalRoutesEnabled, + fieldPath: fieldPath.Child(name), } allErrs = append(allErrs, validateIngressAnnotation(context)...) } @@ -148,14 +297,6 @@ func validateIngressAnnotations(annotations map[string]string, isPlus bool, fiel func validateIngressAnnotation(context *annotationValidationContext) field.ErrorList { allErrs := field.ErrorList{} - - var annotationValidations annotationValidationConfig - if context.isPlus { - annotationValidations = nginxPlusAnnotationValidations - } else { - annotationValidations = nginxAnnotationValidations - } - if validationFuncs, exists := annotationValidations[context.name]; exists { for _, validationFunc := range validationFuncs { valErrors := validationFunc(context) @@ -191,18 +332,26 @@ func validateMergeableIngressTypeAnnotation(context *annotationValidationContext return allErrs } -func validateNginxLBMethodAnnotation(context *annotationValidationContext) field.ErrorList { +func validateLBMethodAnnotation(context *annotationValidationContext) field.ErrorList { allErrs := field.ErrorList{} - if _, err := configs.ParseLBMethod(context.value); err != nil { + + parseFunc := configs.ParseLBMethod + if context.isPlus { + parseFunc = configs.ParseLBMethodForPlus + } + + if _, err := parseFunc(context.value); err != nil { return append(allErrs, field.Invalid(context.fieldPath, context.value, err.Error())) } return allErrs } -func validateNginxPlusLBMethodAnnotation(context *annotationValidationContext) field.ErrorList { +func validateServerTokensAnnotation(context *annotationValidationContext) field.ErrorList { allErrs := field.ErrorList{} - if _, err := configs.ParseLBMethodForPlus(context.value); err != nil { - return append(allErrs, field.Invalid(context.fieldPath, context.value, err.Error())) + if !context.isPlus { + if _, err := configs.ParseBool(context.value); err != nil { + return append(allErrs, field.Invalid(context.fieldPath, context.value, "must be a boolean")) + } } return allErrs } @@ -223,10 +372,26 @@ func validatePlusOnlyAnnotation(context *annotationValidationContext) field.Erro return allErrs } +func validateAppProtectOnlyAnnotation(context *annotationValidationContext) field.ErrorList { + allErrs := field.ErrorList{} + if !context.appProtectEnabled { + return append(allErrs, field.Forbidden(context.fieldPath, "annotation requires AppProtect")) + } + return allErrs +} + +func validateInternalRoutesOnlyAnnotation(context *annotationValidationContext) field.ErrorList { + allErrs := field.ErrorList{} + if !context.internalRoutesEnabled { + return append(allErrs, field.Forbidden(context.fieldPath, "annotation requires Internal Routes enabled")) + } + return allErrs +} + func validateBoolAnnotation(context *annotationValidationContext) field.ErrorList { allErrs := field.ErrorList{} if _, err := configs.ParseBool(context.value); err != nil { - return append(allErrs, field.Invalid(context.fieldPath, context.value, "must be a valid boolean")) + return append(allErrs, field.Invalid(context.fieldPath, context.value, "must be a boolean")) } return allErrs } @@ -234,12 +399,12 @@ func validateBoolAnnotation(context *annotationValidationContext) field.ErrorLis func validateTimeAnnotation(context *annotationValidationContext) field.ErrorList { allErrs := field.ErrorList{} if _, err := configs.ParseTime(context.value); err != nil { - return append(allErrs, field.Invalid(context.fieldPath, context.value, "must be a valid time")) + return append(allErrs, field.Invalid(context.fieldPath, context.value, "must be a time")) } return allErrs } -func validateNonNegativeIntAnnotation(context *annotationValidationContext) field.ErrorList { +func validateUint64Annotation(context *annotationValidationContext) field.ErrorList { allErrs := field.ErrorList{} if _, err := configs.ParseUint64(context.value); err != nil { return append(allErrs, field.Invalid(context.fieldPath, context.value, "must be a non-negative integer")) @@ -247,6 +412,70 @@ func validateNonNegativeIntAnnotation(context *annotationValidationContext) fiel return allErrs } +func validateInt64Annotation(context *annotationValidationContext) field.ErrorList { + allErrs := field.ErrorList{} + if _, err := configs.ParseInt64(context.value); err != nil { + return append(allErrs, field.Invalid(context.fieldPath, context.value, "must be an integer")) + } + return allErrs +} + +func validateIntAnnotation(context *annotationValidationContext) field.ErrorList { + allErrs := field.ErrorList{} + if _, err := configs.ParseInt(context.value); err != nil { + return append(allErrs, field.Invalid(context.fieldPath, context.value, "must be an integer")) + } + return allErrs +} + +func validatePortListAnnotation(context *annotationValidationContext) field.ErrorList { + allErrs := field.ErrorList{} + if _, err := configs.ParsePortList(context.value); err != nil { + return append(allErrs, field.Invalid(context.fieldPath, context.value, "must be a comma-separated list of port numbers")) + } + return allErrs +} + +func validateServiceListAnnotation(context *annotationValidationContext) field.ErrorList { + allErrs := field.ErrorList{} + var unknownServices []string + annotationServices := configs.ParseServiceList(context.value) + for svc := range annotationServices { + if _, exists := context.specServices[svc]; !exists { + unknownServices = append(unknownServices, svc) + } + } + if len(unknownServices) > 0 { + errorMsg := fmt.Sprintf( + "must be a comma-separated list of services. The following services were not found: %s", + strings.Join(unknownServices, ","), + ) + return append(allErrs, field.Invalid(context.fieldPath, context.value, errorMsg)) + } + return allErrs +} + +func validateStickyServiceListAnnotation(context *annotationValidationContext) field.ErrorList { + allErrs := field.ErrorList{} + if _, err := configs.ParseStickyServiceList(context.value); err != nil { + return append(allErrs, field.Invalid(context.fieldPath, context.value, "must be a semicolon-separated list of sticky services")) + } + return allErrs +} + +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")) + } + return allErrs +} + +func validateIsBool(v string) error { + _, err := configs.ParseBool(v) + return err +} + func validateIsTrue(v string) error { b, err := configs.ParseBool(v) if err != nil { @@ -317,3 +546,18 @@ func validateMinionSpec(spec *networking.IngressSpec, fieldPath *field.Path) fie return allErrs } + +func getSpecServices(ingressSpec networking.IngressSpec) map[string]bool { + services := make(map[string]bool) + if ingressSpec.Backend != nil { + services[ingressSpec.Backend.ServiceName] = true + } + for _, rule := range ingressSpec.Rules { + if rule.HTTP != nil { + for _, path := range rule.HTTP.Paths { + services[path.Backend.ServiceName] = true + } + } + } + return services +} diff --git a/internal/k8s/validation_test.go b/internal/k8s/validation_test.go index 3b479a230e..904e5de935 100644 --- a/internal/k8s/validation_test.go +++ b/internal/k8s/validation_test.go @@ -13,10 +13,12 @@ import ( func TestValidateIngress(t *testing.T) { tests := []struct { - ing *networking.Ingress - isPlus bool - expectedErrors []string - msg string + ing *networking.Ingress + isPlus bool + appProtectEnabled bool + internalRoutesEnabled bool + expectedErrors []string + msg string }{ { ing: &networking.Ingress{ @@ -28,9 +30,11 @@ func TestValidateIngress(t *testing.T) { }, }, }, - isPlus: false, - expectedErrors: nil, - msg: "valid input", + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid input", }, { ing: &networking.Ingress{ @@ -47,7 +51,9 @@ func TestValidateIngress(t *testing.T) { }, }, }, - isPlus: false, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, expectedErrors: []string{ `annotations.nginx.org/mergeable-ingress-type: Invalid value: "invalid": must be one of: 'master' or 'minion'`, "spec.rules[0].host: Required value", @@ -78,7 +84,9 @@ func TestValidateIngress(t *testing.T) { }, }, }, - isPlus: false, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, expectedErrors: []string{ "spec.rules[0].http.paths: Too many: 1: must have at most 0 items", }, @@ -100,7 +108,9 @@ func TestValidateIngress(t *testing.T) { }, }, }, - isPlus: false, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, expectedErrors: []string{ "spec.rules[0].http.paths: Required value: must include at least one path", }, @@ -109,7 +119,7 @@ func TestValidateIngress(t *testing.T) { } for _, test := range tests { - allErrs := validateIngress(test.ing, test.isPlus) + allErrs := validateIngress(test.ing, test.isPlus, test.appProtectEnabled, test.internalRoutesEnabled) assertion := assertErrors("validateIngress()", test.msg, allErrs, test.expectedErrors) if assertion != "" { t.Error(assertion) @@ -118,16 +128,23 @@ func TestValidateIngress(t *testing.T) { } func TestValidateNginxIngressAnnotations(t *testing.T) { - isPlus := false tests := []struct { - annotations map[string]string - expectedErrors []string - msg string + annotations map[string]string + specServices map[string]bool + isPlus bool + appProtectEnabled bool + internalRoutesEnabled bool + expectedErrors []string + msg string }{ { - annotations: map[string]string{}, - expectedErrors: nil, - msg: "valid no annotations", + annotations: map[string]string{}, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid no annotations", }, { @@ -135,6 +152,10 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { "nginx.org/lb-method": "invalid_method", "nginx.org/mergeable-ingress-type": "invalid", }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, expectedErrors: []string{ `annotations.nginx.org/lb-method: Invalid value: "invalid_method": Invalid load balancing method: "invalid_method"`, `annotations.nginx.org/mergeable-ingress-type: Invalid value: "invalid": must be one of: 'master' or 'minion'`, @@ -146,20 +167,32 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { annotations: map[string]string{ "nginx.org/mergeable-ingress-type": "master", }, - expectedErrors: nil, - msg: "valid input with master annotation", + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid input with master annotation", }, { annotations: map[string]string{ "nginx.org/mergeable-ingress-type": "minion", }, - expectedErrors: nil, - msg: "valid input with minion annotation", + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid input with minion annotation", }, { annotations: map[string]string{ "nginx.org/mergeable-ingress-type": "", }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, expectedErrors: []string{ "annotations.nginx.org/mergeable-ingress-type: Required value", }, @@ -169,6 +202,10 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { annotations: map[string]string{ "nginx.org/mergeable-ingress-type": "abc", }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, expectedErrors: []string{ `annotations.nginx.org/mergeable-ingress-type: Invalid value: "abc": must be one of: 'master' or 'minion'`, }, @@ -179,13 +216,21 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { annotations: map[string]string{ "nginx.org/lb-method": "random", }, - expectedErrors: nil, - msg: "valid nginx.org/lb-method annotation, nginx normal", + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/lb-method annotation, nginx normal", }, { annotations: map[string]string{ "nginx.org/lb-method": "least_time header", }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, expectedErrors: []string{ `annotations.nginx.org/lb-method: Invalid value: "least_time header": Invalid load balancing method: "least_time header"`, }, @@ -195,6 +240,10 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { annotations: map[string]string{ "nginx.org/lb-method": "invalid_method", }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, expectedErrors: []string{ `annotations.nginx.org/lb-method: Invalid value: "invalid_method": Invalid load balancing method: "invalid_method"`, }, @@ -205,246 +254,1271 @@ func TestValidateNginxIngressAnnotations(t *testing.T) { annotations: map[string]string{ "nginx.com/health-checks": "true", }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, expectedErrors: []string{ "annotations.nginx.com/health-checks: Forbidden: annotation requires NGINX Plus", }, msg: "invalid nginx.com/health-checks annotation, nginx plus only", }, + { + annotations: map[string]string{ + "nginx.com/health-checks": "true", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.com/health-checks annotation", + }, + { + annotations: map[string]string{ + "nginx.com/health-checks": "not_a_boolean", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.com/health-checks: Invalid value: "not_a_boolean": must be a boolean`, + }, + msg: "invalid nginx.com/health-checks annotation", + }, { annotations: map[string]string{ "nginx.com/health-checks-mandatory": "true", }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, expectedErrors: []string{ "annotations.nginx.com/health-checks-mandatory: Forbidden: annotation requires NGINX Plus", }, msg: "invalid nginx.com/health-checks-mandatory annotation, nginx plus only", }, + { + annotations: map[string]string{ + "nginx.com/health-checks": "true", + "nginx.com/health-checks-mandatory": "true", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.com/health-checks-mandatory annotation", + }, + { + annotations: map[string]string{ + "nginx.com/health-checks": "true", + "nginx.com/health-checks-mandatory": "not_a_boolean", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.com/health-checks-mandatory: Invalid value: "not_a_boolean": must be a boolean`, + }, + msg: "invalid nginx.com/health-checks-mandatory, must be a boolean", + }, + { + annotations: map[string]string{ + "nginx.com/health-checks-mandatory": "true", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + "annotations.nginx.com/health-checks-mandatory: Forbidden: related annotation nginx.com/health-checks: must be set", + }, + msg: "invalid nginx.com/health-checks-mandatory, related annotation nginx.com/health-checks not set", + }, + { + annotations: map[string]string{ + "nginx.com/health-checks": "false", + "nginx.com/health-checks-mandatory": "true", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + "annotations.nginx.com/health-checks-mandatory: Forbidden: related annotation nginx.com/health-checks: must be true", + }, + msg: "invalid nginx.com/health-checks-mandatory nginx.com/health-checks is not true", + }, { annotations: map[string]string{ "nginx.com/health-checks-mandatory-queue": "true", }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, expectedErrors: []string{ "annotations.nginx.com/health-checks-mandatory-queue: Forbidden: annotation requires NGINX Plus", }, msg: "invalid nginx.com/health-checks-mandatory-queue annotation, nginx plus only", }, + { + annotations: map[string]string{ + "nginx.com/health-checks": "true", + "nginx.com/health-checks-mandatory": "true", + "nginx.com/health-checks-mandatory-queue": "5", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.com/health-checks-mandatory-queue annotation", + }, + { + annotations: map[string]string{ + "nginx.com/health-checks": "true", + "nginx.com/health-checks-mandatory": "true", + "nginx.com/health-checks-mandatory-queue": "not_a_number", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.com/health-checks-mandatory-queue: Invalid value: "not_a_number": must be a non-negative integer`, + }, + msg: "invalid nginx.com/health-checks-mandatory-queue, must be a number", + }, + { + annotations: map[string]string{ + "nginx.com/health-checks-mandatory-queue": "5", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + "annotations.nginx.com/health-checks-mandatory-queue: Forbidden: related annotation nginx.com/health-checks-mandatory: must be set", + }, + msg: "invalid nginx.com/health-checks-mandatory-queue, related annotation nginx.com/health-checks-mandatory not set", + }, + { + annotations: map[string]string{ + "nginx.com/health-checks": "true", + "nginx.com/health-checks-mandatory": "false", + "nginx.com/health-checks-mandatory-queue": "5", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + "annotations.nginx.com/health-checks-mandatory-queue: Forbidden: related annotation nginx.com/health-checks-mandatory: must be true", + }, + msg: "invalid nginx.com/health-checks-mandatory-queue nginx.com/health-checks-mandatory is not true", + }, { annotations: map[string]string{ "nginx.com/slow-start": "true", }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, expectedErrors: []string{ "annotations.nginx.com/slow-start: Forbidden: annotation requires NGINX Plus", }, msg: "invalid nginx.com/slow-start annotation, nginx plus only", }, - } + { + annotations: map[string]string{ + "nginx.com/slow-start": "60s", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.com/slow-start annotation", + }, + { + annotations: map[string]string{ + "nginx.com/slow-start": "not_a_time", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.com/slow-start: Invalid value: "not_a_time": must be a time`, + }, + msg: "invalid nginx.com/slow-start annotation", + }, - for _, test := range tests { - t.Run(test.msg, func(t *testing.T) { - allErrs := validateIngressAnnotations(test.annotations, isPlus, field.NewPath("annotations")) - assertion := assertErrors("validateIngressAnnotations()", test.msg, allErrs, test.expectedErrors) - if assertion != "" { - t.Error(assertion) - } - }) - } -} + { + annotations: map[string]string{ + "nginx.org/server-tokens": "true", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/server-tokens annotation, nginx", + }, + { + annotations: map[string]string{ + "nginx.org/server-tokens": "custom_setting", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/server-tokens annotation, nginx plus", + }, + { + annotations: map[string]string{ + "nginx.org/server-tokens": "custom_setting", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.org/server-tokens: Invalid value: "custom_setting": must be a boolean`, + }, + msg: "invalid nginx.org/server-tokens annotation, must be a boolean", + }, -func TestValidateNginxPlusIngressAnnotations(t *testing.T) { - isPlus := true - tests := []struct { - annotations map[string]string - expectedErrors []string - msg string - }{ { - annotations: map[string]string{}, - expectedErrors: nil, - msg: "valid no annotations", + annotations: map[string]string{ + "nginx.org/server-snippets": "snippet-1", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/server-snippets annotation, single-value", + }, + { + annotations: map[string]string{ + "nginx.org/server-snippets": "snippet-1\nsnippet-2\nsnippet-3", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/server-snippets annotation, multi-value", }, { annotations: map[string]string{ - "nginx.org/lb-method": "invalid_method", - "nginx.com/health-checks": "not_a_boolean", + "nginx.org/location-snippets": "snippet-1", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/location-snippets annotation, single-value", + }, + { + annotations: map[string]string{ + "nginx.org/location-snippets": "snippet-1\nsnippet-2\nsnippet-3", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/location-snippets annotation, multi-value", + }, + + { + annotations: map[string]string{ + "nginx.org/proxy-connect-timeout": "10s", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/proxy-connect-timeout annotation", + }, + + { + annotations: map[string]string{ + "nginx.org/proxy-read-timeout": "10s", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/proxy-read-timeout annotation", + }, + + { + annotations: map[string]string{ + "nginx.org/proxy-send-timeout": "10s", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/proxy-send-timeout annotation", + }, + + { + annotations: map[string]string{ + "nginx.org/proxy-hide-headers": "header-1", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/proxy-hide-headers annotation, single-value", + }, + { + annotations: map[string]string{ + "nginx.org/proxy-hide-headers": "header-1,header-2,header-3", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/proxy-hide-headers annotation, multi-value", + }, + + { + annotations: map[string]string{ + "nginx.org/proxy-pass-headers": "header-1", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/proxy-pass-headers annotation, single-value", + }, + { + annotations: map[string]string{ + "nginx.org/proxy-pass-headers": "header-1,header-2,header-3", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/proxy-pass-headers annotation, multi-value", + }, + + { + annotations: map[string]string{ + "nginx.org/client-max-body-size": "16M", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/client-max-body-size annotation", + }, + + { + annotations: map[string]string{ + "nginx.org/redirect-to-https": "true", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/redirect-to-https annotation", + }, + { + annotations: map[string]string{ + "nginx.org/redirect-to-https": "not_a_boolean", }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, expectedErrors: []string{ - `annotations.nginx.com/health-checks: Invalid value: "not_a_boolean": must be a valid boolean`, - `annotations.nginx.org/lb-method: Invalid value: "invalid_method": Invalid load balancing method: "invalid_method"`, + `annotations.nginx.org/redirect-to-https: Invalid value: "not_a_boolean": must be a boolean`, }, - msg: "invalid multiple annotations messages in alphabetical order", + msg: "invalid nginx.org/redirect-to-https annotation", }, { annotations: map[string]string{ - "nginx.org/mergeable-ingress-type": "master", + "ingress.kubernetes.io/ssl-redirect": "true", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid ingress.kubernetes.io/ssl-redirect annotation", + }, + { + annotations: map[string]string{ + "ingress.kubernetes.io/ssl-redirect": "not_a_boolean", }, - expectedErrors: nil, - msg: "valid input with master annotation", + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.ingress.kubernetes.io/ssl-redirect: Invalid value: "not_a_boolean": must be a boolean`, + }, + msg: "invalid ingress.kubernetes.io/ssl-redirect annotation", }, + { annotations: map[string]string{ - "nginx.org/mergeable-ingress-type": "minion", + "nginx.org/proxy-buffering": "true", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/proxy-buffering annotation", + }, + { + annotations: map[string]string{ + "nginx.org/proxy-buffering": "not_a_boolean", }, - expectedErrors: nil, - msg: "valid input with minion annotation", + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.org/proxy-buffering: Invalid value: "not_a_boolean": must be a boolean`, + }, + msg: "invalid nginx.org/proxy-buffering annotation", }, + { annotations: map[string]string{ - "nginx.org/mergeable-ingress-type": "", + "nginx.org/hsts": "true", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/hsts annotation", + }, + { + annotations: map[string]string{ + "nginx.org/hsts": "not_a_boolean", }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, expectedErrors: []string{ - "annotations.nginx.org/mergeable-ingress-type: Required value", + `annotations.nginx.org/hsts: Invalid value: "not_a_boolean": must be a boolean`, }, - msg: "invalid mergeable type annotation 1", + msg: "invalid nginx.org/hsts annotation", }, + { annotations: map[string]string{ - "nginx.org/mergeable-ingress-type": "abc", + "nginx.org/hsts": "true", + "nginx.org/hsts-max-age": "120", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/hsts-max-age annotation", + }, + { + annotations: map[string]string{ + "nginx.org/hsts": "false", + "nginx.org/hsts-max-age": "120", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/hsts-max-age nginx.org/hsts can be false", + }, + { + annotations: map[string]string{ + "nginx.org/hsts": "true", + "nginx.org/hsts-max-age": "not_a_number", }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, expectedErrors: []string{ - `annotations.nginx.org/mergeable-ingress-type: Invalid value: "abc": must be one of: 'master' or 'minion'`, + `annotations.nginx.org/hsts-max-age: Invalid value: "not_a_number": must be an integer`, }, - msg: "invalid mergeable type annotation 2", + msg: "invalid nginx.org/hsts-max-age, must be a number", + }, + { + annotations: map[string]string{ + "nginx.org/hsts-max-age": "true", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + "annotations.nginx.org/hsts-max-age: Forbidden: related annotation nginx.org/hsts: must be set", + }, + msg: "invalid nginx.org/hsts-max-age, related annotation nginx.org/hsts not set", }, { annotations: map[string]string{ - "nginx.org/lb-method": "least_time header", + "nginx.org/hsts": "true", + "nginx.org/hsts-include-subdomains": "true", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/hsts-include-subdomains annotation", + }, + { + annotations: map[string]string{ + "nginx.org/hsts": "false", + "nginx.org/hsts-include-subdomains": "true", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/hsts-include-subdomains, nginx.org/hsts can be false", + }, + { + annotations: map[string]string{ + "nginx.org/hsts": "true", + "nginx.org/hsts-include-subdomains": "not_a_boolean", }, - expectedErrors: nil, - msg: "valid nginx.org/lb-method annotation", + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.org/hsts-include-subdomains: Invalid value: "not_a_boolean": must be a boolean`, + }, + msg: "invalid nginx.org/hsts-include-subdomains, must be a boolean", }, { annotations: map[string]string{ - "nginx.org/lb-method": "invalid_method", + "nginx.org/hsts-include-subdomains": "true", }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, expectedErrors: []string{ - `annotations.nginx.org/lb-method: Invalid value: "invalid_method": Invalid load balancing method: "invalid_method"`, + "annotations.nginx.org/hsts-include-subdomains: Forbidden: related annotation nginx.org/hsts: must be set", }, - msg: "invalid nginx.org/lb-method annotation, nginx", + msg: "invalid nginx.org/hsts-include-subdomains, related annotation nginx.org/hsts not set", }, { annotations: map[string]string{ - "nginx.com/health-checks": "true", + "nginx.org/hsts": "true", + "nginx.org/hsts-behind-proxy": "true", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/hsts-behind-proxy annotation", + }, + { + annotations: map[string]string{ + "nginx.org/hsts": "false", + "nginx.org/hsts-behind-proxy": "true", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/hsts-behind-proxy, nginx.org/hsts can be false", + }, + { + annotations: map[string]string{ + "nginx.org/hsts": "true", + "nginx.org/hsts-behind-proxy": "not_a_boolean", }, - expectedErrors: nil, - msg: "valid nginx.com/health-checks annotation", + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.org/hsts-behind-proxy: Invalid value: "not_a_boolean": must be a boolean`, + }, + msg: "invalid nginx.org/hsts-behind-proxy, must be a boolean", }, { annotations: map[string]string{ - "nginx.com/health-checks": "not_a_boolean", + "nginx.org/hsts-behind-proxy": "true", }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, expectedErrors: []string{ - `annotations.nginx.com/health-checks: Invalid value: "not_a_boolean": must be a valid boolean`, + "annotations.nginx.org/hsts-behind-proxy: Forbidden: related annotation nginx.org/hsts: must be set", }, - msg: "invalid nginx.com/health-checks annotation", + msg: "invalid nginx.org/hsts-behind-proxy, related annotation nginx.org/hsts not set", }, { annotations: map[string]string{ - "nginx.com/health-checks": "true", - "nginx.com/health-checks-mandatory": "true", + "nginx.org/proxy-buffers": "8 8k", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/proxy-buffers annotation", + }, + + { + annotations: map[string]string{ + "nginx.org/proxy-buffer-size": "16k", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/proxy-buffer-size annotation", + }, + + { + annotations: map[string]string{ + "nginx.org/proxy-max-temp-file-size": "128M", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/proxy-max-temp-file-size annotation", + }, + + { + annotations: map[string]string{ + "nginx.org/upstream-zone-size": "512k", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/upstream-zone-size annotation", + }, + + { + annotations: map[string]string{ + "nginx.com/jwt-realm": "true", }, - expectedErrors: nil, - msg: "valid nginx.com/health-checks-mandatory annotation", + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + "annotations.nginx.com/jwt-realm: Forbidden: annotation requires NGINX Plus", + }, + msg: "invalid nginx.com/jwt-realm annotation, nginx plus only", }, { annotations: map[string]string{ - "nginx.com/health-checks": "true", - "nginx.com/health-checks-mandatory": "not_a_boolean", + "nginx.com/jwt-realm": "my-jwt-realm", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.com/jwt-realm annotation", + }, + + { + annotations: map[string]string{ + "nginx.com/jwt-key": "true", }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, expectedErrors: []string{ - `annotations.nginx.com/health-checks-mandatory: Invalid value: "not_a_boolean": must be a valid boolean`, + "annotations.nginx.com/jwt-key: Forbidden: annotation requires NGINX Plus", }, - msg: "invalid nginx.com/health-checks-mandatory, must be a boolean", + msg: "invalid nginx.com/jwt-key annotation, nginx plus only", }, { annotations: map[string]string{ - "nginx.com/health-checks-mandatory": "true", + "nginx.com/jwt-key": "my-jwk", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.com/jwt-key annotation", + }, + + { + annotations: map[string]string{ + "nginx.com/jwt-token": "true", }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, expectedErrors: []string{ - "annotations.nginx.com/health-checks-mandatory: Forbidden: related annotation nginx.com/health-checks: must be set", + "annotations.nginx.com/jwt-token: Forbidden: annotation requires NGINX Plus", }, - msg: "invalid nginx.com/health-checks-mandatory, related annotation nginx.com/health-checks not set", + msg: "invalid nginx.com/jwt-token annotation, nginx plus only", }, { annotations: map[string]string{ - "nginx.com/health-checks": "false", - "nginx.com/health-checks-mandatory": "true", + "nginx.com/jwt-token": "$cookie_auth_token", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.com/jwt-token annotation", + }, + + { + annotations: map[string]string{ + "nginx.com/jwt-login-url": "true", }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, expectedErrors: []string{ - "annotations.nginx.com/health-checks-mandatory: Forbidden: related annotation nginx.com/health-checks: must be true", + "annotations.nginx.com/jwt-login-url: Forbidden: annotation requires NGINX Plus", }, - msg: "invalid nginx.com/health-checks-mandatory nginx.com/health-checks is not true", + msg: "invalid nginx.com/jwt-login-url annotation, nginx plus only", + }, + { + annotations: map[string]string{ + "nginx.com/jwt-login-url": "https://login.example.com", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.com/jwt-login-url annotation", }, { annotations: map[string]string{ - "nginx.com/health-checks": "true", - "nginx.com/health-checks-mandatory": "true", - "nginx.com/health-checks-mandatory-queue": "5", + "nginx.org/listen-ports": "80,8080,9090", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/listen-ports annotation", + }, + { + annotations: map[string]string{ + "nginx.org/listen-ports": "not_a_port_list", }, - expectedErrors: nil, - msg: "valid nginx.com/health-checks-mandatory-queue annotation", + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.org/listen-ports: Invalid value: "not_a_port_list": must be a comma-separated list of port numbers`, + }, + msg: "invalid nginx.org/listen-ports annotation", }, + { annotations: map[string]string{ - "nginx.com/health-checks": "true", - "nginx.com/health-checks-mandatory": "true", - "nginx.com/health-checks-mandatory-queue": "not_a_number", + "nginx.org/listen-ports-ssl": "443,8443", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/listen-ports-ssl annotation", + }, + { + annotations: map[string]string{ + "nginx.org/listen-ports-ssl": "not_a_port_list", }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, expectedErrors: []string{ - `annotations.nginx.com/health-checks-mandatory-queue: Invalid value: "not_a_number": must be a non-negative integer`, + `annotations.nginx.org/listen-ports-ssl: Invalid value: "not_a_port_list": must be a comma-separated list of port numbers`, }, - msg: "invalid nginx.com/health-checks-mandatory-queue, must be a number", + msg: "invalid nginx.org/listen-ports-ssl annotation", }, + { annotations: map[string]string{ - "nginx.com/health-checks-mandatory-queue": "5", + "nginx.org/keepalive": "1000", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/keepalive annotation", + }, + { + annotations: map[string]string{ + "nginx.org/keepalive": "not_a_number", }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, expectedErrors: []string{ - "annotations.nginx.com/health-checks-mandatory-queue: Forbidden: related annotation nginx.com/health-checks-mandatory: must be set", + `annotations.nginx.org/keepalive: Invalid value: "not_a_number": must be an integer`, }, - msg: "invalid nginx.com/health-checks-mandatory-queue, related annotation nginx.com/health-checks-mandatory not set", + msg: "invalid nginx.org/keepalive annotation", }, + { annotations: map[string]string{ - "nginx.com/health-checks": "true", - "nginx.com/health-checks-mandatory": "false", - "nginx.com/health-checks-mandatory-queue": "5", + "nginx.org/max-fails": "5", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/max-fails annotation", + }, + { + annotations: map[string]string{ + "nginx.org/max-fails": "not_a_number", }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, expectedErrors: []string{ - "annotations.nginx.com/health-checks-mandatory-queue: Forbidden: related annotation nginx.com/health-checks-mandatory: must be true", + `annotations.nginx.org/max-fails: Invalid value: "not_a_number": must be an integer`, }, - msg: "invalid nginx.com/health-checks-mandatory-queue nginx.com/health-checks-mandatory is not true", + msg: "invalid nginx.org/max-fails annotation", }, { annotations: map[string]string{ - "nginx.com/slow-start": "60s", + "nginx.org/max-conns": "10", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/max-conns annotation", + }, + { + annotations: map[string]string{ + "nginx.org/max-conns": "not_a_number", }, - expectedErrors: nil, - msg: "valid nginx.com/slow-start annotation", + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.org/max-conns: Invalid value: "not_a_number": must be an integer`, + }, + msg: "invalid nginx.org/max-conns annotation", }, + { annotations: map[string]string{ - "nginx.com/slow-start": "not_a_time", + "nginx.org/fail-timeout": "10s", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/fail-timeout annotation", + }, + + { + annotations: map[string]string{ + "appprotect.f5.com/app-protect-enable": "true", }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + internalRoutesEnabled: false, expectedErrors: []string{ - `annotations.nginx.com/slow-start: Invalid value: "not_a_time": must be a valid time`, + "annotations.appprotect.f5.com/app-protect-enable: Forbidden: annotation requires AppProtect", }, - msg: "invalid nginx.com/slow-start annotation", + msg: "invalid appprotect.f5.com/app-protect-enable annotation, requires app protect", + }, + { + annotations: map[string]string{ + "appprotect.f5.com/app-protect-enable": "true", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: true, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid appprotect.f5.com/app-protect-enable annotation", + }, + { + annotations: map[string]string{ + "appprotect.f5.com/app-protect-enable": "not_a_boolean", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: true, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.appprotect.f5.com/app-protect-enable: Invalid value: "not_a_boolean": must be a boolean`, + }, + msg: "invalid appprotect.f5.com/app-protect-enable annotation", + }, + + { + annotations: map[string]string{ + "appprotect.f5.com/app-protect-security-log-enable": "true", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + "annotations.appprotect.f5.com/app-protect-security-log-enable: Forbidden: annotation requires AppProtect", + }, + msg: "invalid appprotect.f5.com/app-protect-security-log-enable annotation, requires app protect", + }, + { + annotations: map[string]string{ + "appprotect.f5.com/app-protect-security-log-enable": "true", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: true, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid appprotect.f5.com/app-protect-security-log-enable annotation", + }, + { + annotations: map[string]string{ + "appprotect.f5.com/app-protect-security-log-enable": "not_a_boolean", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: true, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.appprotect.f5.com/app-protect-security-log-enable: Invalid value: "not_a_boolean": must be a boolean`, + }, + msg: "invalid appprotect.f5.com/app-protect-security-log-enable annotation", + }, + + { + annotations: map[string]string{ + "nsm.nginx.com/internal-route": "true", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + "annotations.nsm.nginx.com/internal-route: Forbidden: annotation requires Internal Routes enabled", + }, + msg: "invalid nsm.nginx.com/internal-route annotation, requires internal routes", + }, + { + annotations: map[string]string{ + "nsm.nginx.com/internal-route": "true", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + internalRoutesEnabled: true, + expectedErrors: nil, + msg: "valid nsm.nginx.com/internal-route annotation", + }, + { + annotations: map[string]string{ + "nsm.nginx.com/internal-route": "not_a_boolean", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + internalRoutesEnabled: true, + expectedErrors: []string{ + `annotations.nsm.nginx.com/internal-route: Invalid value: "not_a_boolean": must be a boolean`, + }, + msg: "invalid nsm.nginx.com/internal-route annotation", + }, + + { + annotations: map[string]string{ + "nginx.org/websocket-services": "service-1", + }, + specServices: map[string]bool{ + "service-1": true, + }, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/websocket-services annotation, single-value", + }, + { + annotations: map[string]string{ + "nginx.org/websocket-services": "service-1,service-2", + }, + specServices: map[string]bool{ + "service-1": true, + "service-2": true, + }, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/websocket-services annotation, multi-value", + }, + { + annotations: map[string]string{ + "nginx.org/websocket-services": "service-1,service-2", + }, + specServices: map[string]bool{ + "service-1": true, + }, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.org/websocket-services: Invalid value: "service-1,service-2": must be a comma-separated list of services. The following services were not found: service-2`, + }, + msg: "invalid nginx.org/websocket-services annotation, service does not exist", + }, + + { + annotations: map[string]string{ + "nginx.org/ssl-services": "service-1", + }, + specServices: map[string]bool{ + "service-1": true, + }, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/ssl-services annotation, single-value", + }, + { + annotations: map[string]string{ + "nginx.org/ssl-services": "service-1,service-2", + }, + specServices: map[string]bool{ + "service-1": true, + "service-2": true, + }, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/ssl-services annotation, multi-value", + }, + { + annotations: map[string]string{ + "nginx.org/ssl-services": "service-1,service-2", + }, + specServices: map[string]bool{ + "service-1": true, + }, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.org/ssl-services: Invalid value: "service-1,service-2": must be a comma-separated list of services. The following services were not found: service-2`, + }, + msg: "invalid nginx.org/ssl-services annotation, service does not exist", + }, + + { + annotations: map[string]string{ + "nginx.org/grpc-services": "service-1", + }, + specServices: map[string]bool{ + "service-1": true, + }, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/grpc-services annotation, single-value", + }, + { + annotations: map[string]string{ + "nginx.org/grpc-services": "service-1,service-2", + }, + specServices: map[string]bool{ + "service-1": true, + "service-2": true, + }, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/grpc-services annotation, multi-value", + }, + { + annotations: map[string]string{ + "nginx.org/grpc-services": "service-1,service-2", + }, + specServices: map[string]bool{ + "service-1": true, + }, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.org/grpc-services: Invalid value: "service-1,service-2": must be a comma-separated list of services. The following services were not found: service-2`, + }, + msg: "invalid nginx.org/grpc-services annotation, service does not exist", + }, + + { + annotations: map[string]string{ + "nginx.org/rewrites": "serviceName=service-1 rewrite=rewrite-1", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/rewrites annotation, single-value", + }, + { + annotations: map[string]string{ + "nginx.org/rewrites": "serviceName=service-1 rewrite=rewrite-1;serviceName=service-2 rewrite=rewrite-2", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.org/rewrites annotation, multi-value", + }, + { + annotations: map[string]string{ + "nginx.org/rewrites": "not_a_rewrite", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + internalRoutesEnabled: true, + expectedErrors: []string{ + `annotations.nginx.org/rewrites: Invalid value: "not_a_rewrite": must be a semicolon-separated list of rewrites`, + }, + msg: "invalid nginx.org/rewrites annotation", + }, + + { + annotations: map[string]string{ + "nginx.com/sticky-cookie-services": "true", + }, + specServices: map[string]bool{}, + isPlus: false, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + "annotations.nginx.com/sticky-cookie-services: Forbidden: annotation requires NGINX Plus", + }, + msg: "invalid nginx.com/sticky-cookie-services annotation, nginx plus only", + }, + { + annotations: map[string]string{ + "nginx.com/sticky-cookie-services": "serviceName=service-1 srv_id expires=1h path=/service-1", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.com/sticky-cookie-services annotation, single-value", + }, + { + annotations: map[string]string{ + "nginx.com/sticky-cookie-services": "serviceName=service-1 srv_id expires=1h path=/service-1;serviceName=service-2 srv_id expires=2h path=/service-2", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: nil, + msg: "valid nginx.com/sticky-cookie-services annotation, multi-value", + }, + { + annotations: map[string]string{ + "nginx.com/sticky-cookie-services": "not_a_rewrite", + }, + specServices: map[string]bool{}, + isPlus: true, + appProtectEnabled: false, + internalRoutesEnabled: false, + expectedErrors: []string{ + `annotations.nginx.com/sticky-cookie-services: Invalid value: "not_a_rewrite": must be a semicolon-separated list of sticky services`, + }, + msg: "invalid nginx.com/sticky-cookie-services annotation", }, } for _, test := range tests { t.Run(test.msg, func(t *testing.T) { - allErrs := validateIngressAnnotations(test.annotations, isPlus, field.NewPath("annotations")) + allErrs := validateIngressAnnotations( + test.annotations, + test.specServices, + test.isPlus, + test.appProtectEnabled, + test.internalRoutesEnabled, + field.NewPath("annotations"), + ) assertion := assertErrors("validateIngressAnnotations()", test.msg, allErrs, test.expectedErrors) if assertion != "" { t.Error(assertion)