Implemented validation for provisioner.spec.taints (#502)

* Implemented validation for provisioner.spec.taints * PR Comments
aws · Jul 7, 2021 · 93e01a5 · 93e01a5
1 parent 1bd5174
commit 93e01a5
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 0 deletions.
diff --git a/pkg/apis/provisioning/v1alpha2/provisioner_validation.go b/pkg/apis/provisioning/v1alpha2/provisioner_validation.go
@@ -20,6 +20,8 @@ import (
 
 	"github.com/awslabs/karpenter/pkg/utils/functional"
 	"github.com/awslabs/karpenter/pkg/utils/ptr"
+
+	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/validation"
 	"knative.dev/pkg/apis"
 )
@@ -113,6 +115,7 @@ func (c *Cluster) validate() (errs *apis.FieldError) {
 func (c *Constraints) Validate(ctx context.Context) (errs *apis.FieldError) {
 	errs = errs.Also(
 		c.validateLabels(),
+		c.validateTaints(),
 		c.validateArchitecture(),
 		c.validateOperatingSystem(),
 		c.validateZones(),
@@ -136,6 +139,31 @@ func (c *Constraints) validateLabels() (errs *apis.FieldError) {
 	return errs
 }
 
+func (c *Constraints) validateTaints() (errs *apis.FieldError) {
+	for i, taint := range c.Taints {
+		// Validate Key
+		if len(taint.Key) == 0 {
+			errs = errs.Also(apis.ErrInvalidArrayValue(errs, "taints", i))
+		}
+		for _, err := range validation.IsQualifiedName(taint.Key) {
+			errs = errs.Also(apis.ErrInvalidArrayValue(err, "taints", i))
+		}
+		// Validate Value
+		if len(taint.Value) != 0 {
+			for _, err := range validation.IsQualifiedName(taint.Value) {
+				errs = errs.Also(apis.ErrInvalidArrayValue(err, "taints", i))
+			}
+		}
+		// Validate effect
+		switch taint.Effect {
+		case v1.TaintEffectNoSchedule, v1.TaintEffectPreferNoSchedule, v1.TaintEffectNoExecute, "":
+		default:
+			errs = errs.Also(apis.ErrInvalidArrayValue(taint.Effect, "effect", i))
+		}
+	}
+	return errs
+}
+
 func (c *Constraints) validateArchitecture() (errs *apis.FieldError) {
 	if c.Architecture == nil {
 		return nil

diff --git a/pkg/apis/provisioning/v1alpha2/provisioner_validation_test.go b/pkg/apis/provisioning/v1alpha2/provisioner_validation_test.go
@@ -24,6 +24,7 @@ import (
 	. "github.com/onsi/gomega"
 	"knative.dev/pkg/ptr"
 
+	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -99,6 +100,33 @@ var _ = Describe("Validation", func() {
 			}
 		})
 	})
+	Context("Taints", func() {
+		It("should succeed for valid taints", func() {
+			provisioner.Spec.Taints = []v1.Taint{
+				{Key: "a", Value: "b", Effect: v1.TaintEffectNoSchedule},
+				{Key: "c", Value: "d", Effect: v1.TaintEffectNoExecute},
+				{Key: "e", Value: "f", Effect: v1.TaintEffectPreferNoSchedule},
+				{Key: "key-only", Effect: v1.TaintEffectNoExecute},
+			}
+			Expect(provisioner.Validate(ctx)).To(Succeed())
+		})
+		It("should fail for invalid taint keys", func() {
+			provisioner.Spec.Taints = []v1.Taint{{Key: "???"}}
+			Expect(provisioner.Validate(ctx)).ToNot(Succeed())
+		})
+		It("should fail for missing taint key", func() {
+			provisioner.Spec.Taints = []v1.Taint{{Effect: v1.TaintEffectNoSchedule}}
+			Expect(provisioner.Validate(ctx)).ToNot(Succeed())
+		})
+		It("should fail for invalid taint value", func() {
+			provisioner.Spec.Taints = []v1.Taint{{Key: "invalid-value", Effect: v1.TaintEffectNoSchedule, Value: "???"}}
+			Expect(provisioner.Validate(ctx)).ToNot(Succeed())
+		})
+		It("should fail for invalid taint effect", func() {
+			provisioner.Spec.Taints = []v1.Taint{{Key: "invalid-effect", Effect: "???"}}
+			Expect(provisioner.Validate(ctx)).ToNot(Succeed())
+		})
+	})
 	Context("Zones", func() {
 		SupportedZones = append(SupportedZones, "test-zone-1")
 		It("should succeed if unspecified", func() {