From 8fd0d65800d4c3489e7f4b536c6c7016ed880fe1 Mon Sep 17 00:00:00 2001
From: Amanuel Engeda <74629455+engedaam@users.noreply.github.com>
Date: Thu, 26 Oct 2023 13:49:04 -0700
Subject: [PATCH] chore: Not allowing empty role to be set on EC2NodeClass
 (#4940)

---
 pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml  | 2 ++
 pkg/apis/v1beta1/ec2nodeclass.go                     | 1 +
 pkg/apis/v1beta1/ec2nodeclass_validation_cel_test.go | 5 +++++
 3 files changed, 8 insertions(+)

diff --git a/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml b/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml
index 8045ddb48973..5ea480990412 100644
--- a/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml
+++ b/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml
@@ -272,6 +272,8 @@ spec:
                   profiles on an update.
                 type: string
                 x-kubernetes-validations:
+                - message: role cannot be empty
+                  rule: self != ''
                 - message: immutable field changed
                   rule: self == oldSelf
               securityGroupSelectorTerms:
diff --git a/pkg/apis/v1beta1/ec2nodeclass.go b/pkg/apis/v1beta1/ec2nodeclass.go
index fd3dbebd0094..681ec717ae76 100644
--- a/pkg/apis/v1beta1/ec2nodeclass.go
+++ b/pkg/apis/v1beta1/ec2nodeclass.go
@@ -61,6 +61,7 @@ type EC2NodeClassSpec struct {
 	// Marking this field as immutable avoids concerns around terminating managed instance profiles from running instances.
 	// This field may be made mutable in the future, assuming the correct garbage collection and drift handling is implemented
 	// for the old instance profiles on an update.
+	// +kubebuilder:validation:XValidation:rule="self != ''",message="role cannot be empty"
 	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="immutable field changed"
 	// +required
 	Role string `json:"role"`
diff --git a/pkg/apis/v1beta1/ec2nodeclass_validation_cel_test.go b/pkg/apis/v1beta1/ec2nodeclass_validation_cel_test.go
index 4b5aa4e77cd9..58cf00120d88 100644
--- a/pkg/apis/v1beta1/ec2nodeclass_validation_cel_test.go
+++ b/pkg/apis/v1beta1/ec2nodeclass_validation_cel_test.go
@@ -41,6 +41,7 @@ var _ = Describe("CEL/Validation", func() {
 		nc = &v1beta1.EC2NodeClass{
 			ObjectMeta: metav1.ObjectMeta{Name: strings.ToLower(randomdata.SillyName())},
 			Spec: v1beta1.EC2NodeClassSpec{
+				Role:      "test-role",
 				AMIFamily: &v1beta1.AMIFamilyAL2,
 				SubnetSelectorTerms: []v1beta1.SubnetSelectorTerm{
 					{
@@ -703,6 +704,10 @@ var _ = Describe("CEL/Validation", func() {
 		})
 	})
 	Context("Role Immutability", func() {
+		It("should fail if role is not defined", func() {
+			nc.Spec.Role = ""
+			Expect(env.Client.Create(ctx, nc)).ToNot(Succeed())
+		})
 		It("should fail when updating the role", func() {
 			nc.Spec.Role = "test-role"
 			Expect(env.Client.Create(ctx, nc)).To(Succeed())