Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Earlier validation feedback 2 #1277

Merged
merged 13 commits into from
Dec 15, 2020
Merged

Earlier validation feedback 2 #1277

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
591ef58
Add more early ingress annotation validations
Dec 9, 2020
8dc390c
Add further validations
Dec 9, 2020
3fcb93e
Revert changes to annotations.go to better preserve existing function…
Dec 9, 2020
163355a
Add earlier HSTS annotation validation
Dec 10, 2020
ad22af1
Add earlier jwt and proxy buffer annotation validation
Dec 10, 2020
bd30691
Add earlier listen ports annotation validation
Dec 10, 2020
1b69853
Add earlier app protect and internal routes annotation validation
Dec 10, 2020
f2bb43b
Simplify annotation validation config
Dec 10, 2020
98749dd
Add earlier sevices annotation validation
Dec 10, 2020
b3b8652
Change map and slice initialization to follow go idioms
Dec 11, 2020
b046dad
ParseServiceList no longer returns an error
Dec 11, 2020
1c00f49
Validate services in ingress annotations
Dec 11, 2020
36dbc91
hsts annotation validation has been relaxed to comform with existing …
Dec 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
218 changes: 71 additions & 147 deletions internal/configs/annotations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
package configs

import (
"fmt"
"strconv"
"strings"

"github.com/golang/glog"
)

Expand Down Expand Up @@ -93,29 +89,41 @@ func parseAnnotations(ingEx *IngressEx, baseCfgParams *ConfigParams, isPlus bool
if err != nil {
glog.Error(err)
}
cfgParams.HealthCheckEnabled = healthCheckEnabled
if isPlus {
mikestephen marked this conversation as resolved.
Show resolved Hide resolved
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 {
mikestephen marked this conversation as resolved.
Show resolved Hide resolved
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 {
mikestephen marked this conversation as resolved.
Show resolved Hide resolved
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 {
mikestephen marked this conversation as resolved.
Show resolved Hide resolved
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 {
Expand Down Expand Up @@ -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 {
mikestephen marked this conversation as resolved.
Show resolved Hide resolved
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"]; exists {
mikestephen marked this conversation as resolved.
Show resolved Hide resolved
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 {
Expand Down Expand Up @@ -351,99 +370,58 @@ 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 {
services, err := ParseServiceList(value)
if err != nil {
glog.Error(err)
}
return services
}

return wsServices
return make(map[string]bool)
mikestephen marked this conversation as resolved.
Show resolved Hide resolved
}

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 make(map[string]string)
}

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 {
services, err := ParseServiceList(value)
if err != nil {
glog.Error(err)
}
return services
}

return sslServices
return make(map[string]bool)
}

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 {
services, err := ParseServiceList(value)
if err != nil {
glog.Error(err)
}
return services
}

return grpcServices
return make(map[string]bool)
}

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 make(map[string]string)
}

func filterMasterAnnotations(annotations map[string]string) []string {
Expand Down Expand Up @@ -481,57 +459,3 @@ func mergeMasterAnnotationsIntoMinion(minionAnnotations map[string]string, maste
}
}
}

func parsePort(value string) (int, error) {
mikestephen marked this conversation as resolved.
Show resolved Hide resolved
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
}
96 changes: 96 additions & 0 deletions internal/configs/parsing_helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
ports := make([]int, 0)
mikestephen marked this conversation as resolved.
Show resolved Hide resolved
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
mikestephen marked this conversation as resolved.
Show resolved Hide resolved
}

// ParseServiceList ensures that the string is a comma-separated list of services
func ParseServiceList(s string) (map[string]bool, error) {
mikestephen marked this conversation as resolved.
Show resolved Hide resolved
services := make(map[string]bool)
for _, part := range strings.Split(s, ",") {
services[part] = true
}
return services, nil
}

// 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`)

Expand Down
Loading