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.

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
Prev Previous commit
Next Next commit
Simplify annotation validation config
Mike Stephen committed Dec 10, 2020
commit f2bb43b7ee88253b9441ed24708c846572e0c177
175 changes: 29 additions & 146 deletions internal/k8s/validation.go
Original file line number Diff line number Diff line change
@@ -68,33 +68,42 @@ 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.
mikestephen marked this conversation as resolved.
Show resolved Hide resolved
// 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,
},
serverTokensAnnotation: {
validateRequiredAnnotation,
validateBoolAnnotation,
validateServerTokensAnnotation,
},
serverSnippetsAnnotation: {},
locationSnippetsAnnotation: {},
@@ -188,126 +197,7 @@ var (
validateBoolAnnotation,
},
}
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: {
validateRequiredAnnotation,
validateMergeableIngressTypeAnnotation,
},
lbMethodAnnotation: {
validateRequiredAnnotation,
validateNginxPlusLBMethodAnnotation,
},
healthChecksAnnotation: {
validateRequiredAnnotation,
validateBoolAnnotation,
},
healthChecksMandatoryAnnotation: {
validateRelatedAnnotation(healthChecksAnnotation, validateIsTrue),
validateRequiredAnnotation,
validateBoolAnnotation,
},
healthChecksMandatoryQueueAnnotation: {
validateRelatedAnnotation(healthChecksMandatoryAnnotation, validateIsTrue),
validateRequiredAnnotation,
validateUint64Annotation,
},
slowStartAnnotation: {
validateRequiredAnnotation,
validateTimeAnnotation,
},
serverTokensAnnotation: {
validateRequiredAnnotation,
},
serverSnippetsAnnotation: {},
locationSnippetsAnnotation: {},
proxyConnectTimeoutAnnotation: {},
proxyReadTimeoutAnnotation: {},
proxySendTimeoutAnnotation: {},
proxyHideHeadersAnnotation: {},
proxyPassHeadersAnnotation: {},
clientMaxBodySizeAnnotation: {},
redirectToHTTPSAnnotation: {
validateRequiredAnnotation,
validateBoolAnnotation,
},
sslRedirectAnnotation: {
validateRequiredAnnotation,
validateBoolAnnotation,
},
proxyBufferingAnnotation: {
validateRequiredAnnotation,
validateBoolAnnotation,
},
hstsAnnotation: {
validateRequiredAnnotation,
validateBoolAnnotation,
},
hstsMaxAgeAnnotation: {
validateRelatedAnnotation(hstsAnnotation, validateIsTrue),
validateRequiredAnnotation,
validateInt64Annotation,
},
hstsIncludeSubdomainsAnnotation: {
validateRelatedAnnotation(hstsAnnotation, validateIsTrue),
validateRequiredAnnotation,
validateBoolAnnotation,
},
hstsBehindProxyAnnotation: {
validateRelatedAnnotation(hstsAnnotation, validateIsTrue),
validateRequiredAnnotation,
validateBoolAnnotation,
},
proxyBuffersAnnotation: {},
proxyBufferSizeAnnotation: {},
proxyMaxTempFileSizeAnnotation: {},
upstreamZoneSizeAnnotation: {},
jwtRealmAnnotation: {},
jwtKeyAnnotation: {},
jwtTokenAnnotation: {},
jwtLoginURLAnnotation: {},
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,
},
}
nginxPlusAnnotationNames = sortedAnnotationNames(nginxPlusAnnotationValidations)
annotationNames = sortedAnnotationNames(annotationValidations)
)

func sortedAnnotationNames(annotationValidations annotationValidationConfig) []string {
@@ -356,13 +246,6 @@ func validateIngressAnnotations(
) 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{
@@ -383,14 +266,6 @@ func validateIngressAnnotations(

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)
@@ -426,18 +301,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
}
20 changes: 20 additions & 0 deletions internal/k8s/validation_test.go
Original file line number Diff line number Diff line change
@@ -437,6 +437,26 @@ func TestValidateNginxIngressAnnotations(t *testing.T) {
msg: "invalid nginx.com/slow-start annotation",
},

{
annotations: map[string]string{
"nginx.org/server-tokens": "true",
},
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",
},
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",