chore: Restrict owner key in amiSelectorTerms from being set with t…

…ags (#4885)

pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml

@@ -90,8 +90,10 @@ spec:
                   rule: self.all(x, has(x.tags) || has(x.id) || has(x.name))
                 - message: '''id'' is mutually exclusive, cannot be set with a combination
                     of other fields in amiSelectorTerms'
-                  rule: '!self.all(x, has(x.id) && (has(x.tags) || has(x.name)) ||
-                    has(x.owner))'
+                  rule: '!self.all(x, has(x.id) && (has(x.tags) || has(x.name) ||
+                    has(x.owner)))'
+                - message: '''owner'' cannot be set with ''tags'''
+                  rule: '!self.all(x, has(x.owner) && has(x.tags))'
               blockDeviceMappings:
                 description: BlockDeviceMappings to be applied to provisioned nodes.
                 items:

pkg/apis/v1beta1/ec2nodeclass.go

@@ -43,7 +43,8 @@ type EC2NodeClassSpec struct {
 	SecurityGroupSelectorTerms []SecurityGroupSelectorTerm `json:"securityGroupSelectorTerms" hash:"ignore"`
 	// AMISelectorTerms is a list of or ami selector terms. The terms are ORed.
 	// +kubebuilder:validation:XValidation:message="expected at least one, got none, ['tags', 'id', 'name']",rule="self.all(x, has(x.tags) || has(x.id) || has(x.name))"
-	// +kubebuilder:validation:XValidation:message="'id' is mutually exclusive, cannot be set with a combination of other fields in amiSelectorTerms",rule="!self.all(x, has(x.id) && (has(x.tags) || has(x.name)) || has(x.owner))"
+	// +kubebuilder:validation:XValidation:message="'id' is mutually exclusive, cannot be set with a combination of other fields in amiSelectorTerms",rule="!self.all(x, has(x.id) && (has(x.tags) || has(x.name) || has(x.owner)))"
+	// +kubebuilder:validation:XValidation:message="'owner' cannot be set with 'tags'",rule="!self.all(x, has(x.owner) && has(x.tags))"
 	// +kubebuilder:validation:MaxItems:=30
 	// +optional
 	AMISelectorTerms []AMISelectorTerm `json:"amiSelectorTerms,omitempty" hash:"ignore"`

pkg/apis/v1beta1/ec2nodeclass_validation.go

@@ -133,6 +133,8 @@ func (in *AMISelectorTerm) validate() (errs *apis.FieldError) {
 		errs = errs.Also(apis.ErrGeneric("expect at least one, got none", "tags", "id", "name"))
 	} else if in.ID != "" && (len(in.Tags) > 0 || in.Name != "" || in.Owner != "") {
 		errs = errs.Also(apis.ErrGeneric(`"id" is mutually exclusive, cannot be set with a combination of other fields in`))
+	} else if in.Owner != "" && len(in.Tags) > 0 {
+		errs = errs.Also(apis.ErrGeneric(`"owner" cannot be set with "tags" in`))
 	}
 	return errs
 }

pkg/apis/v1beta1/ec2nodeclass_validation_cel_test.go

@@ -35,7 +35,6 @@ var _ = Describe("CEL/Validation", func() {
 	var nc *v1beta1.EC2NodeClass
 
 	BeforeEach(func() {
-		env.Version.Minor()
 		if env.Version.Minor() < 25 {
 			Skip("CEL Validation is for 1.25>")
 		}
@@ -344,6 +343,15 @@ var _ = Describe("CEL/Validation", func() {
 			}
 			Expect(env.Client.Create(ctx, nc)).To(Succeed())
 		})
+		It("should succeed with a valid ami selector on name and owner", func() {
+			nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{
+				{
+					Name:  "testname",
+					Owner: "testowner",
+				},
+			}
+			Expect(env.Client.Create(ctx, nc)).To(Succeed())
+		})
 		It("should fail when a ami selector term has no values", func() {
 			nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{
 				{},
@@ -378,6 +386,17 @@ var _ = Describe("CEL/Validation", func() {
 			}
 			Expect(env.Client.Create(ctx, nc)).ToNot(Succeed())
 		})
+		It("should fail when an ami selector term has an owner key with tags", func() {
+			nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{
+				{
+					Owner: "testowner",
+					Tags: map[string]string{
+						"test": "testvalue",
+					},
+				},
+			}
+			Expect(env.Client.Create(ctx, nc)).ToNot(Succeed())
+		})
 		It("should fail when the last ami selector is invalid", func() {
 			nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{
 				{

pkg/apis/v1beta1/ec2nodeclass_validation_webhook_test.go

@@ -355,6 +355,15 @@ var _ = Describe("Webhook/Validation", func() {
 			}
 			Expect(nc.Validate(ctx)).To(Succeed())
 		})
+		It("should succeed with a valid ami selector on name and owner", func() {
+			nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{
+				{
+					Name:  "testname",
+					Owner: "testowner",
+				},
+			}
+			Expect(nc.Validate(ctx)).To(Succeed())
+		})
 		It("should fail when a ami selector term has no values", func() {
 			nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{
 				{},
@@ -389,6 +398,17 @@ var _ = Describe("Webhook/Validation", func() {
 			}
 			Expect(nc.Validate(ctx)).ToNot(Succeed())
 		})
+		It("should fail when an ami selector term has an owner key with tags", func() {
+			nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{
+				{
+					Owner: "testowner",
+					Tags: map[string]string{
+						"test": "testvalue",
+					},
+				},
+			}
+			Expect(nc.Validate(ctx)).ToNot(Succeed())
+		})
 		It("should fail when the last ami selector is invalid", func() {
 			nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{
 				{

tools/karpenter-convert/pkg/convert/convert.go

@@ -167,7 +167,6 @@ func (o *Context) RunConvert() error {
 func dropFields(buffer bytes.Buffer) string {
 	output := buffer.String()
 	output = strings.Replace(output, "status: {}\n", "", -1)
-	output = strings.Replace(output, "      creationTimestamp: null\n", "", -1)
 	output = strings.Replace(output, "  creationTimestamp: null\n", "", -1)
 	output = strings.Replace(output, "      resources: {}\n", "", -1)
 
@@ -240,9 +239,6 @@ func convertProvisioner(resource runtime.Object, o *Context) runtime.Object {
 		Finalizers:  coreprovisioner.Finalizers,
 	}
 
-	// Reset timestamp if present
-	nodepool.Spec.Template.CreationTimestamp = metav1.Time{}
-
 	// Cleanup the status provided in input
 	nodepool.Status = corev1beta1.NodePoolStatus{}