Refactor and improve API validation

This commit is contained in:
John Gardiner Myers 2020-05-30 12:59:48 -07:00
parent 9466893436
commit 898f9fa198
3 changed files with 29 additions and 27 deletions

View File

@ -19,9 +19,7 @@ package validation
import (
"fmt"
"net"
"strings"
"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/kops/pkg/apis/kops"
"k8s.io/kops/pkg/apis/kops/util"
@ -48,25 +46,6 @@ func ValidateCluster(c *kops.Cluster, strict bool) field.ErrorList {
return allErrs
}
if c.ObjectMeta.Name == "" {
allErrs = append(allErrs, field.Required(field.NewPath("objectMeta", "name"), "Cluster Name is required (e.g. --name=mycluster.myzone.com)"))
} else {
// Must be a dns name
errs := validation.IsDNS1123Subdomain(c.ObjectMeta.Name)
if len(errs) != 0 {
allErrs = append(allErrs, field.Invalid(field.NewPath("objectMeta", "name"), c.ObjectMeta.Name, fmt.Sprintf("Cluster Name must be a valid DNS name (e.g. --name=mycluster.myzone.com) errors: %s", strings.Join(errs, ", "))))
} else if !strings.Contains(c.ObjectMeta.Name, ".") {
// Tolerate if this is a cluster we are importing for upgrade
if c.ObjectMeta.Annotations[kops.AnnotationNameManagement] != kops.AnnotationValueManagementImported {
allErrs = append(allErrs, field.Invalid(field.NewPath("objectMeta", "name"), c.ObjectMeta.Name, "Cluster Name must be a fully-qualified DNS name (e.g. --name=mycluster.myzone.com)"))
}
}
}
if c.Spec.Assets != nil && c.Spec.Assets.ContainerProxy != nil && c.Spec.Assets.ContainerRegistry != nil {
allErrs = append(allErrs, field.Forbidden(fieldSpec.Child("Assets", "ContainerProxy"), "ContainerProxy cannot be used in conjunction with ContainerRegistry as represent mutually exclusive concepts. Please consult the documentation for details."))
}
requiresSubnets := true
requiresNetworkCIDR := true
requiresSubnetCIDR := true

View File

@ -42,6 +42,23 @@ import (
func newValidateCluster(cluster *kops.Cluster) field.ErrorList {
allErrs := validation.ValidateObjectMeta(&cluster.ObjectMeta, false, validation.NameIsDNSSubdomain, field.NewPath("metadata"))
clusterName := cluster.ObjectMeta.Name
if clusterName == "" {
allErrs = append(allErrs, field.Required(field.NewPath("objectMeta", "name"), "Cluster Name is required (e.g. --name=mycluster.myzone.com)"))
} else {
// Must be a dns name
errs := utilvalidation.IsDNS1123Subdomain(clusterName)
if len(errs) != 0 {
allErrs = append(allErrs, field.Invalid(field.NewPath("objectMeta", "name"), clusterName, fmt.Sprintf("Cluster Name must be a valid DNS name (e.g. --name=mycluster.myzone.com) errors: %s", strings.Join(errs, ", "))))
} else if !strings.Contains(clusterName, ".") {
// Tolerate if this is a cluster we are importing for upgrade
if cluster.ObjectMeta.Annotations[kops.AnnotationNameManagement] != kops.AnnotationValueManagementImported {
allErrs = append(allErrs, field.Invalid(field.NewPath("objectMeta", "name"), clusterName, "Cluster Name must be a fully-qualified DNS name (e.g. --name=mycluster.myzone.com)"))
}
}
}
allErrs = append(allErrs, validateClusterSpec(&cluster.Spec, cluster, field.NewPath("spec"))...)
// Additional cloud-specific validation rules
@ -155,6 +172,12 @@ func validateClusterSpec(spec *kops.ClusterSpec, c *kops.Cluster, fieldPath *fie
allErrs = append(allErrs, validateDockerConfig(spec.Docker, fieldPath.Child("docker"))...)
}
if spec.Assets != nil {
if spec.Assets.ContainerProxy != nil && spec.Assets.ContainerRegistry != nil {
allErrs = append(allErrs, field.Forbidden(fieldPath.Child("assets", "containerProxy"), "containerProxy cannot be used in conjunction with containerRegistry"))
}
}
if spec.RollingUpdate != nil {
allErrs = append(allErrs, validateRollingUpdate(spec.RollingUpdate, fieldPath.Child("rollingUpdate"), false)...)
}
@ -440,18 +463,18 @@ func validateNodeAuthorization(n *kops.NodeAuthorizationSpec, c *kops.Cluster, f
return field.ErrorList{field.Forbidden(fldPath, "node authorization is experimental feature; set `export KOPS_FEATURE_FLAGS=EnableNodeAuthorization`")}
}
authorizerPath := fldPath.Child("nodeAuthorizer")
if c.Spec.NodeAuthorization.NodeAuthorizer == nil {
allErrs = append(allErrs, field.Forbidden(fldPath, "no node authorization policy has been set"))
allErrs = append(allErrs, field.Required(authorizerPath, "no node authorization policy has been set"))
} else {
path := fldPath.Child("nodeAuthorizer")
if c.Spec.NodeAuthorization.NodeAuthorizer.Port < 0 || n.NodeAuthorizer.Port >= 65535 {
allErrs = append(allErrs, field.Invalid(path.Child("port"), n.NodeAuthorizer.Port, "invalid port"))
allErrs = append(allErrs, field.Invalid(authorizerPath.Child("port"), n.NodeAuthorizer.Port, "invalid port"))
}
if c.Spec.NodeAuthorization.NodeAuthorizer.Timeout != nil && n.NodeAuthorizer.Timeout.Duration <= 0 {
allErrs = append(allErrs, field.Invalid(path.Child("timeout"), n.NodeAuthorizer.Timeout, "must be greater than zero"))
allErrs = append(allErrs, field.Invalid(authorizerPath.Child("timeout"), n.NodeAuthorizer.Timeout, "must be greater than zero"))
}
if c.Spec.NodeAuthorization.NodeAuthorizer.TokenTTL != nil && n.NodeAuthorizer.TokenTTL.Duration < 0 {
allErrs = append(allErrs, field.Invalid(path.Child("tokenTTL"), n.NodeAuthorizer.TokenTTL, "must be greater than or equal to zero"))
allErrs = append(allErrs, field.Invalid(authorizerPath.Child("tokenTTL"), n.NodeAuthorizer.TokenTTL, "must be greater than or equal to zero"))
}
// @question: we could probably just default these settings in the model when the node-authorizer is enabled??

View File

@ -174,7 +174,7 @@ func TestValidate_ContainerRegistry_and_ContainerProxy_exclusivity(t *testing.T)
proxy := "https://proxy.example.com/"
c.Spec.Assets.ContainerProxy = &proxy
expectErrorFromValidate(t, c, "ContainerProxy cannot be used in conjunction with ContainerRegistry")
expectErrorFromValidate(t, c, "containerProxy cannot be used in conjunction with containerRegistry")
c.Spec.Assets.ContainerRegistry = nil
expectNoErrorFromValidate(t, c)