Merge pull request #2112 from justinsb/fix_2110

Perform validation of the security group names

pkg/apis/kops/validation/aws.go

@@ -0,0 +1,51 @@
+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package validation
+
+import (
+	"k8s.io/apimachinery/pkg/util/validation/field"
+	"k8s.io/kops/pkg/apis/kops"
+	"strings"
+)
+
+func awsValidateCluster(c *kops.Cluster) field.ErrorList {
+	return nil
+}
+
+func awsValidateInstanceGroup(ig *kops.InstanceGroup) field.ErrorList {
+	allErrs := field.ErrorList{}
+
+	allErrs = append(allErrs, awsValidateAdditionalSecurityGroups(field.NewPath("spec", "additionalSecurityGroups"), ig.Spec.AdditionalSecurityGroups)...)
+
+	return allErrs
+}
+
+func awsValidateAdditionalSecurityGroups(fieldPath *field.Path, groups []string) field.ErrorList {
+	allErrs := field.ErrorList{}
+
+	for i, s := range groups {
+		if strings.TrimSpace(s) == "" {
+			allErrs = append(allErrs, field.Invalid(fieldPath.Index(i), s, "security group cannot be empty, if specified"))
+			continue
+		}
+		if !strings.HasPrefix(s, "sg-") {
+			allErrs = append(allErrs, field.Invalid(fieldPath.Index(i), s, "security group does not match the expected AWS format"))
+		}
+	}
+
+	return allErrs
+}

pkg/apis/kops/validation/aws_test.go

@@ -0,0 +1,69 @@
+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package validation
+
+import (
+	"k8s.io/kops/pkg/apis/kops"
+	"testing"
+)
+
+func TestValidateInstanceGroupSpec(t *testing.T) {
+	grid := []struct {
+		Input          kops.InstanceGroupSpec
+		ExpectedErrors []string
+	}{
+		{
+			Input: kops.InstanceGroupSpec{
+				AdditionalSecurityGroups: []string{},
+			},
+		},
+		{
+			Input: kops.InstanceGroupSpec{
+				AdditionalSecurityGroups: []string{"sg-1234abcd"},
+			},
+		},
+		{
+			Input: kops.InstanceGroupSpec{
+				AdditionalSecurityGroups: []string{"sg-1234abcd", ""},
+			},
+			ExpectedErrors: []string{"Invalid value::spec.additionalSecurityGroups[1]"},
+		},
+		{
+			Input: kops.InstanceGroupSpec{
+				AdditionalSecurityGroups: []string{" ", ""},
+			},
+			ExpectedErrors: []string{
+				"Invalid value::spec.additionalSecurityGroups[0]",
+				"Invalid value::spec.additionalSecurityGroups[1]",
+			},
+		},
+		{
+			Input: kops.InstanceGroupSpec{
+				AdditionalSecurityGroups: []string{"--invalid"},
+			},
+			ExpectedErrors: []string{"Invalid value::spec.additionalSecurityGroups[0]"},
+		},
+	}
+	for _, g := range grid {
+		ig := &kops.InstanceGroup{
+			Spec: g.Input,
+		}
+		errs := awsValidateInstanceGroup(ig)
+
+		testErrors(t, g.Input, errs, g.ExpectedErrors)
+	}
+}

pkg/apis/kops/validation/legacy.go

@@ -440,6 +440,16 @@ func DeepValidate(c *kops.Cluster, groups []*kops.InstanceGroup, strict bool) er
 		if err != nil {
 			return err
 		}
+
+		// Additional cloud-specific validation rules,
+		// such as making sure that identifiers match the expected formats for the given cloud
+		switch fi.CloudProviderID(c.Spec.CloudProvider) {
+		case fi.CloudProviderAWS:
+			errs := awsValidateInstanceGroup(g)
+			if len(errs) != 0 {
+				return errs[0]
+			}
+		}
 	}
 
 	return nil

pkg/apis/kops/validation/validation_test.go

@@ -93,7 +93,7 @@ func testErrors(t *testing.T, context interface{}, actual field.ErrorList, expec
 
 		for _, expected := range expectedErrors {
 			if !errStrings.Has(expected) {
-				t.Errorf("expected error %v from %q, was not found in %q", expected, context, errStrings.List())
+				t.Errorf("expected error %v from %v, was not found in %q", expected, context, errStrings.List())
 			}
 		}
 	}