kubernetes
/
kops
mirror of https://github.com/kubernetes/kops.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Return more errors at once during InstanceGroup validation

This commit is contained in:
John Gardiner Myers 2020-01-27 23:17:01 -08:00
parent fbeefee8ee
commit 8f6529879b
7 changed files with 68 additions and 85 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
cmd/kops/create_ig.go
View File

@ -221,7 +221,7 @@ func RunCreateInstanceGroup(f *util.Factory, cmd *cobra.Command, args []string,
return fmt.Errorf("unexpected object type: %T", obj)
}
err = validation.ValidateInstanceGroup(group)
err = validation.ValidateInstanceGroup(group).ToAggregate()
if err != nil {
return err
}

4
cmd/kops/edit_instancegroup.go
View File

@ -152,7 +152,7 @@ func RunEditInstanceGroup(f *util.Factory, cmd *cobra.Command, args []string, ou
return fmt.Errorf("object was not of expected type: %T", newObj)
}
err = validation.ValidateInstanceGroup(newGroup)
err = validation.ValidateInstanceGroup(newGroup).ToAggregate()
if err != nil {
return err
}
@ -175,7 +175,7 @@ func RunEditInstanceGroup(f *util.Factory, cmd *cobra.Command, args []string, ou
return err
}
err = validation.CrossValidateInstanceGroup(fullGroup, fullCluster, true)
err = validation.CrossValidateInstanceGroup(fullGroup, fullCluster, true).ToAggregate()
if err != nil {
return err
}

132
pkg/apis/kops/validation/instancegroup.go
View File

@ -29,70 +29,58 @@ import (
)
// ValidateInstanceGroup is responsible for validating the configuration of a instancegroup
func ValidateInstanceGroup(g *kops.InstanceGroup) error {
func ValidateInstanceGroup(g *kops.InstanceGroup) field.ErrorList {
allErrs := field.ErrorList{}
if g.ObjectMeta.Name == "" {
return field.Required(field.NewPath("Name"), "")
allErrs = append(allErrs, field.Required(field.NewPath("Name"), ""))
}
switch g.Spec.Role {
case "":
return field.Required(field.NewPath("Role"), "Role must be set")
allErrs = append(allErrs, field.Required(field.NewPath("Role"), "Role must be set"))
case kops.InstanceGroupRoleMaster:
if len(g.Spec.Subnets) == 0 {
allErrs = append(allErrs, field.Required(field.NewPath("Subnets"), "master InstanceGroup must specify at least one Subnet"))
}
case kops.InstanceGroupRoleNode:
case kops.InstanceGroupRoleBastion:
default:
return field.Invalid(field.NewPath("Role"), g.Spec.Role, "Unknown role")
allErrs = append(allErrs, field.Invalid(field.NewPath("Role"), g.Spec.Role, "Unknown role"))
}
if g.Spec.Tenancy != "" {
if g.Spec.Tenancy != "default" && g.Spec.Tenancy != "dedicated" && g.Spec.Tenancy != "host" {
return field.Invalid(field.NewPath("Tenancy"), g.Spec.Tenancy, "Unknown tenancy. Must be Default, Dedicated or Host.")
allErrs = append(allErrs, field.Invalid(field.NewPath("Tenancy"), g.Spec.Tenancy, "Unknown tenancy. Must be Default, Dedicated or Host."))
}
}
if g.Spec.MaxSize != nil && g.Spec.MinSize != nil {
if *g.Spec.MaxSize < *g.Spec.MinSize {
return field.Invalid(field.NewPath("MaxSize"), *g.Spec.MaxSize, "maxSize must be greater than or equal to minSize.")
allErrs = append(allErrs, field.Invalid(field.NewPath("MaxSize"), *g.Spec.MaxSize, "maxSize must be greater than or equal to minSize."))
}
}
if fi.Int32Value(g.Spec.RootVolumeIops) < 0 {
return field.Invalid(field.NewPath("RootVolumeIops"), g.Spec.RootVolumeIops, "RootVolumeIops must be greater than 0")
allErrs = append(allErrs, field.Invalid(field.NewPath("RootVolumeIops"), g.Spec.RootVolumeIops, "RootVolumeIops must be greater than 0"))
}
// @check all the hooks are valid in this instancegroup
for i := range g.Spec.Hooks {
if errs := validateHookSpec(&g.Spec.Hooks[i], field.NewPath("hooks").Index(i)); len(errs) > 0 {
return errs.ToAggregate()
}
allErrs = append(allErrs, validateHookSpec(&g.Spec.Hooks[i], field.NewPath("hooks").Index(i))...)
}
// @check the fileAssets for this instancegroup are valid
for i := range g.Spec.FileAssets {
if errs := validateFileAssetSpec(&g.Spec.FileAssets[i], field.NewPath("fileAssets").Index(i)); len(errs) > 0 {
return errs.ToAggregate()
}
}
if g.IsMaster() {
if len(g.Spec.Subnets) == 0 {
return fmt.Errorf("master InstanceGroup %s did not specify any Subnets", g.ObjectMeta.Name)
}
allErrs = append(allErrs, validateFileAssetSpec(&g.Spec.FileAssets[i], field.NewPath("fileAssets").Index(i))...)
}
if g.Spec.MixedInstancesPolicy != nil {
if errs := validatedMixedInstancesPolicy(field.NewPath(g.Name), g.Spec.MixedInstancesPolicy, g); len(errs) > 0 {
return errs.ToAggregate()
}
allErrs = append(allErrs, validatedMixedInstancesPolicy(field.NewPath(g.Name), g.Spec.MixedInstancesPolicy, g)...)
}
if len(g.Spec.AdditionalUserData) > 0 {
for _, UserDataInfo := range g.Spec.AdditionalUserData {
err := validateExtraUserData(&UserDataInfo)
if err != nil {
return err
}
}
for _, UserDataInfo := range g.Spec.AdditionalUserData {
allErrs = append(allErrs, validateExtraUserData(&UserDataInfo)...)
}
// @step: iterate and check the volume specs
@ -100,13 +88,11 @@ func ValidateInstanceGroup(g *kops.InstanceGroup) error {
devices := make(map[string]bool)
path := field.NewPath("volumes").Index(i)
if err := validateVolumeSpec(path, x); err != nil {
return err
}
allErrs = append(allErrs, validateVolumeSpec(path, x)...)
// @check the device name has not been used already
if _, found := devices[x.Device]; found {
return field.Invalid(path.Child("device"), x.Device, "duplicate device name found in volumes")
allErrs = append(allErrs, field.Invalid(path.Child("device"), x.Device, "duplicate device name found in volumes"))
}
devices[x.Device] = true
@ -117,28 +103,22 @@ func ValidateInstanceGroup(g *kops.InstanceGroup) error {
used := make(map[string]bool)
path := field.NewPath("volumeMounts").Index(i)
if err := validateVolumeMountSpec(path, x); err != nil {
return err
}
allErrs = append(allErrs, validateVolumeMountSpec(path, x)...)
if _, found := used[x.Device]; found {
return field.Invalid(path.Child("device"), x.Device, "duplicate device reference")
allErrs = append(allErrs, field.Invalid(path.Child("device"), x.Device, "duplicate device reference"))
}
if _, found := used[x.Path]; found {
return field.Invalid(path.Child("path"), x.Path, "duplicate mount path specified")
allErrs = append(allErrs, field.Invalid(path.Child("path"), x.Path, "duplicate mount path specified"))
}
}
if err := validateInstanceProfile(g.Spec.IAM, field.NewPath("iam")); err != nil {
return err
}
allErrs = append(allErrs, validateInstanceProfile(g.Spec.IAM, field.NewPath("iam"))...)
if g.Spec.RollingUpdate != nil {
if errs := validateRollingUpdate(g.Spec.RollingUpdate, field.NewPath("rollingUpdate")); len(errs) > 0 {
return errs.ToAggregate()
}
allErrs = append(allErrs, validateRollingUpdate(g.Spec.RollingUpdate, field.NewPath("rollingUpdate"))...)
}
return nil
return allErrs
}
// validatedMixedInstancesPolicy is responsible for validating the user input of a mixed instance policy
@ -179,43 +159,44 @@ func validatedMixedInstancesPolicy(path *field.Path, spec *kops.MixedInstancesPo
}
// validateVolumeSpec is responsible for checking a volume spec is ok
func validateVolumeSpec(path *field.Path, v *kops.VolumeSpec) error {
func validateVolumeSpec(path *field.Path, v *kops.VolumeSpec) field.ErrorList {
allErrs := field.ErrorList{}
if v.Device == "" {
return field.Required(path.Child("device"), "device name required")
allErrs = append(allErrs, field.Required(path.Child("device"), "device name required"))
}
if v.Size <= 0 {
return field.Invalid(path.Child("size"), v.Size, "must be greater than zero")
allErrs = append(allErrs, field.Invalid(path.Child("size"), v.Size, "must be greater than zero"))
}
return nil
return allErrs
}
// validateVolumeMountSpec is responsible for checking the volume mount is ok
func validateVolumeMountSpec(path *field.Path, spec *kops.VolumeMountSpec) error {
func validateVolumeMountSpec(path *field.Path, spec *kops.VolumeMountSpec) field.ErrorList {
allErrs := field.ErrorList{}
if spec.Device == "" {
return field.Required(path.Child("device"), "device name required")
allErrs = append(allErrs, field.Required(path.Child("device"), "device name required"))
}
if spec.Filesystem == "" {
return field.Required(path.Child("filesystem"), "filesystem type required")
allErrs = append(allErrs, field.Required(path.Child("filesystem"), "filesystem type required"))
}
if spec.Path == "" {
return field.Required(path.Child("path"), "mount path required")
allErrs = append(allErrs, field.Required(path.Child("path"), "mount path required"))
}
if !slice.Contains(kops.SupportedFilesystems, spec.Filesystem) {
return field.Invalid(path.Child("filesystem"), spec.Filesystem,
fmt.Sprintf("unsupported filesystem, available types: %s", strings.Join(kops.SupportedFilesystems, ",")))
allErrs = append(allErrs, field.Invalid(path.Child("filesystem"), spec.Filesystem,
fmt.Sprintf("unsupported filesystem, available types: %s", strings.Join(kops.SupportedFilesystems, ","))))
}
return nil
return allErrs
}
// CrossValidateInstanceGroup performs validation of the instance group, including that it is consistent with the Cluster
// It calls ValidateInstanceGroup, so all that validation is included.
func CrossValidateInstanceGroup(g *kops.InstanceGroup, cluster *kops.Cluster, strict bool) error {
err := ValidateInstanceGroup(g)
if err != nil {
return err
}
func CrossValidateInstanceGroup(g *kops.InstanceGroup, cluster *kops.Cluster, strict bool) field.ErrorList {
allErrs := ValidateInstanceGroup(g)
// Check that instance groups are defined in subnets that are defined in the cluster
{
@ -225,25 +206,28 @@ func CrossValidateInstanceGroup(g *kops.InstanceGroup, cluster *kops.Cluster, st
clusterSubnets[s.Name] = s
}
for _, z := range g.Spec.Subnets {
for i, z := range g.Spec.Subnets {
if clusterSubnets[z] == nil {
return fmt.Errorf("InstanceGroup %q is configured in %q, but this is not configured as a Subnet in the cluster", g.ObjectMeta.Name, z)
// TODO field.NotFound(field.NewPath("spec.subnets").Index(i), z) ?
allErrs = append(allErrs, field.Invalid(field.NewPath("spec.subnets").Index(i), z,
fmt.Sprintf("InstanceGroup %q is configured in %q, but this is not configured as a Subnet in the cluster", g.ObjectMeta.Name, z)))
}
}
}
return nil
return allErrs
}
func validateExtraUserData(userData *kops.UserData) error {
func validateExtraUserData(userData *kops.UserData) field.ErrorList {
allErrs := field.ErrorList{}
fieldPath := field.NewPath("AdditionalUserData")
if userData.Name == "" {
return field.Required(fieldPath.Child("Name"), "field must be set")
allErrs = append(allErrs, field.Required(fieldPath.Child("Name"), "field must be set"))
}
if userData.Content == "" {
return field.Required(fieldPath.Child("Content"), "field must be set")
allErrs = append(allErrs, field.Required(fieldPath.Child("Content"), "field must be set"))
}
switch userData.Type {
@ -257,21 +241,23 @@ func validateExtraUserData(userData *kops.UserData) error {
case "text/cloud-boothook":
default:
return field.Invalid(fieldPath.Child("Type"), userData.Type, "Invalid user-data content type")
allErrs = append(allErrs, field.Invalid(fieldPath.Child("Type"), userData.Type, "Invalid user-data content type"))
}
return nil
return allErrs
}
// validateInstanceProfile checks the String values for the AuthProfile
func validateInstanceProfile(v *kops.IAMProfileSpec, fldPath *field.Path) *field.Error {
func validateInstanceProfile(v *kops.IAMProfileSpec, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
if v != nil && v.Profile != nil {
instanceProfileARN := *v.Profile
parsedARN, err := arn.Parse(instanceProfileARN)
if err != nil || !strings.HasPrefix(parsedARN.Resource, "instance-profile") {
return field.Invalid(fldPath.Child("Profile"), instanceProfileARN,
"Instance Group IAM Instance Profile must be a valid aws arn such as arn:aws:iam::123456789012:instance-profile/KopsExampleRole")
allErrs = append(allErrs, field.Invalid(fldPath.Child("Profile"), instanceProfileARN,
"Instance Group IAM Instance Profile must be a valid aws arn such as arn:aws:iam::123456789012:instance-profile/KopsExampleRole"))
}
}
return nil
return allErrs
}

6
pkg/apis/kops/validation/instancegroup_test.go
View File

@ -70,11 +70,7 @@ func TestValidateInstanceProfile(t *testing.T) {
}
for _, g := range grid {
err := validateInstanceProfile(g.Input, field.NewPath("IAMProfile"))
allErrs := field.ErrorList{}
if err != nil {
allErrs = append(allErrs, err)
}
allErrs := validateInstanceProfile(g.Input, field.NewPath("IAMProfile"))
testErrors(t, g.Input, allErrs, g.ExpectedErrors)
if g.ExpectedDetail != "" {

2
pkg/apis/kops/validation/legacy.go
View File

@ -756,7 +756,7 @@ func DeepValidate(c *kops.Cluster, groups []*kops.InstanceGroup, strict bool) er
}
for _, g := range groups {
err := CrossValidateInstanceGroup(g, c, strict)
err := CrossValidateInstanceGroup(g, c, strict).ToAggregate()
if err != nil {
return err
}

4
pkg/client/simple/vfsclientset/instancegroup.go
View File

@ -63,7 +63,7 @@ func NewInstanceGroupMirror(cluster *kopsapi.Cluster, configBase vfs.Path) Insta
defaultReadVersion := v1alpha1.SchemeGroupVersion.WithKind(kind)
r.defaultReadVersion = &defaultReadVersion
r.validate = func(o runtime.Object) error {
return validation.ValidateInstanceGroup(o.(*kopsapi.InstanceGroup))
return validation.ValidateInstanceGroup(o.(*kopsapi.InstanceGroup)).ToAggregate()
}
return r
}
@ -84,7 +84,7 @@ func newInstanceGroupVFS(c *VFSClientset, cluster *kopsapi.Cluster) *InstanceGro
defaultReadVersion := v1alpha1.SchemeGroupVersion.WithKind(kind)
r.defaultReadVersion = &defaultReadVersion
r.validate = func(o runtime.Object) error {
return validation.ValidateInstanceGroup(o.(*kopsapi.InstanceGroup))
return validation.ValidateInstanceGroup(o.(*kopsapi.InstanceGroup)).ToAggregate()
}
return r
}

3
upup/pkg/fi/cloudup/populate_instancegroup_spec.go
View File

@ -63,7 +63,8 @@ var awsDedicatedInstanceExceptions = map[string]bool{
// PopulateInstanceGroupSpec sets default values in the InstanceGroup
// The InstanceGroup is simpler than the cluster spec, so we just populate in place (like the rest of k8s)
func PopulateInstanceGroupSpec(cluster *kops.Cluster, input *kops.InstanceGroup, channel *kops.Channel) (*kops.InstanceGroup, error) {
err := validation.ValidateInstanceGroup(input)
var err error
err = validation.ValidateInstanceGroup(input).ToAggregate()
if err != nil {
return nil, err
}