chore: Enforce fields are required for v1beta1/EC2NodeClass (aws#4638)

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

@@ -43,6 +43,13 @@ spec:
             properties:
               amiFamily:
                 description: AMIFamily is the AMI family that instances use.
+                enum:
+                - AL2
+                - Bottlerocket
+                - Ubuntu
+                - Custom
+                - Windows2019
+                - Windows2022
                 type: string
               amiSelectorTerms:
                 description: AMISelectorTerms is a list of or ami selector terms.
@@ -152,6 +159,11 @@ spec:
                   enabled for instances that are launched
                 type: boolean
               metadataOptions:
+                default:
+                  httpEndpoint: enabled
+                  httpProtocolIPv6: disabled
+                  httpPutResponseHopLimit: 2
+                  httpTokens: required
                 description: "MetadataOptions for the generated launch template of
                   provisioned nodes. \n This specifies the exposure of the Instance
                   Metadata Service to provisioned EC2 nodes. For more information,
@@ -163,30 +175,34 @@ spec:
                   disabled, with httpPutResponseLimit of 2, and with httpTokens required."
                 properties:
                   httpEndpoint:
+                    default: enabled
                     description: "HTTPEndpoint enables or disables the HTTP metadata
                       endpoint on provisioned nodes. If metadata options is non-nil,
                       but this parameter is not specified, the default state is \"enabled\".
                       \n If you specify a value of \"disabled\", instance metadata
                       will not be accessible on the node."
                     type: string
                   httpProtocolIPv6:
+                    default: disabled
                     description: HTTPProtocolIPv6 enables or disables the IPv6 endpoint
                       for the instance metadata service on provisioned nodes. If metadata
                       options is non-nil, but this parameter is not specified, the
                       default state is "disabled".
                     type: string
                   httpPutResponseHopLimit:
+                    default: 2
                     description: HTTPPutResponseHopLimit is the desired HTTP PUT response
                       hop limit for instance metadata requests. The larger the number,
                       the further instance metadata requests can travel. Possible
                       values are integers from 1 to 64. If metadata options is non-nil,
-                      but this parameter is not specified, the default value is 1.
+                      but this parameter is not specified, the default value is 2.
                     format: int64
                     type: integer
                   httpTokens:
+                    default: required
                     description: "HTTPTokens determines the state of token usage for
                       instance metadata requests. If metadata options is non-nil,
-                      but this parameter is not specified, the default state is \"optional\".
+                      but this parameter is not specified, the default state is \"required\".
                       \n If the state is optional, one can choose to retrieve instance
                       metadata with or without a signed token header on the request.
                       If one retrieves the IAM role credentials without a token, the
@@ -260,6 +276,11 @@ spec:
                   will merge certain fields into this UserData to ensure nodes are
                   being provisioned with the correct configuration.
                 type: string
+            required:
+            - amiFamily
+            - role
+            - securityGroupSelectorTerms
+            - subnetSelectorTerms
             type: object
           status:
             description: EC2NodeClassStatus contains the resolved state of the EC2NodeClass

pkg/apis/v1beta1/ec2nodeclass.go

@@ -27,25 +27,26 @@ import (
 // This will contain configuration necessary to launch instances in AWS.
 type EC2NodeClassSpec struct {
 	// SubnetSelectorTerms is a list of or subnet selector terms. The terms are ORed.
-	// +optional
+	// +required
 	SubnetSelectorTerms []SubnetSelectorTerm `json:"subnetSelectorTerms" hash:"ignore"`
 	// SecurityGroupSelectorTerms is a list of or security group selector terms. The terms are ORed.
-	// +optional
+	// +required
 	SecurityGroupSelectorTerms []SecurityGroupSelectorTerm `json:"securityGroupSelectorTerms" hash:"ignore"`
 	// AMISelectorTerms is a list of or ami selector terms. The terms are ORed.
 	// +optional
 	AMISelectorTerms []AMISelectorTerm `json:"amiSelectorTerms,omitempty" hash:"ignore"`
 	// AMIFamily is the AMI family that instances use.
-	// +optional
-	AMIFamily *string `json:"amiFamily,omitempty"`
+	// +kubebuilder:validation:Enum:={AL2,Bottlerocket,Ubuntu,Custom,Windows2019,Windows2022}
+	// +required
+	AMIFamily *string `json:"amiFamily"`
 	// UserData to be applied to the provisioned nodes.
 	// It must be in the appropriate format based on the AMIFamily in use. Karpenter will merge certain fields into
 	// this UserData to ensure nodes are being provisioned with the correct configuration.
 	// +optional
 	UserData *string `json:"userData,omitempty"`
 	// Role is the AWS identity that nodes use.
-	// +optional
-	Role *string `json:"role,omitempty"`
+	// +required
+	Role string `json:"role"`
 	// Tags to be applied on ec2 resources like instances and launch templates.
 	// +optional
 	Tags map[string]string `json:"tags,omitempty"`
@@ -69,6 +70,7 @@ type EC2NodeClassSpec struct {
 	// If omitted, defaults to httpEndpoint enabled, with httpProtocolIPv6
 	// disabled, with httpPutResponseLimit of 2, and with httpTokens
 	// required.
+	// +kubebuilder:default={"httpEndpoint":"enabled","httpProtocolIPv6":"disabled","httpPutResponseHopLimit":2,"httpTokens":"required"}
 	// +optional
 	MetadataOptions *MetadataOptions `json:"metadataOptions,omitempty"`
 	// Context is a Reserved field in EC2 APIs
@@ -161,23 +163,26 @@ type MetadataOptions struct {
 	//
 	// If you specify a value of "disabled", instance metadata will not be accessible
 	// on the node.
+	// +kubebuilder:default=enabled
 	// +optional
 	HTTPEndpoint *string `json:"httpEndpoint,omitempty"`
 	// HTTPProtocolIPv6 enables or disables the IPv6 endpoint for the instance metadata
 	// service on provisioned nodes. If metadata options is non-nil, but this parameter
 	// is not specified, the default state is "disabled".
+	// +kubebuilder:default=disabled
 	// +optional
 	HTTPProtocolIPv6 *string `json:"httpProtocolIPv6,omitempty"`
 	// HTTPPutResponseHopLimit is the desired HTTP PUT response hop limit for
 	// instance metadata requests. The larger the number, the further instance
 	// metadata requests can travel. Possible values are integers from 1 to 64.
 	// If metadata options is non-nil, but this parameter is not specified, the
-	// default value is 1.
+	// default value is 2.
+	// +kubebuilder:default=2
 	// +optional
 	HTTPPutResponseHopLimit *int64 `json:"httpPutResponseHopLimit,omitempty"`
 	// HTTPTokens determines the state of token usage for instance metadata
 	// requests. If metadata options is non-nil, but this parameter is not
-	// specified, the default state is "optional".
+	// specified, the default state is "required".
 	//
 	// If the state is optional, one can choose to retrieve instance metadata with
 	// or without a signed token header on the request. If one retrieves the IAM
@@ -189,6 +194,7 @@ type MetadataOptions struct {
 	// instance metadata retrieval requests. In this state, retrieving the IAM
 	// role credentials always returns the version 2.0 credentials; the version
 	// 1.0 credentials are not available.
+	// +kubebuilder:default=required
 	// +optional
 	HTTPTokens *string `json:"httpTokens,omitempty"`
 }

pkg/apis/v1beta1/ec2nodeclass_validation.go

@@ -20,7 +20,6 @@ import (
 	"strings"
 
 	"github.com/aws/aws-sdk-go/service/ec2"
-	"github.com/samber/lo"
 	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	"knative.dev/pkg/apis"
@@ -64,7 +63,6 @@ func (in *EC2NodeClassSpec) validate(_ context.Context) (errs *apis.FieldError)
 		in.validateMetadataOptions().ViaField(metadataOptionsPath),
 		in.validateAMIFamily().ViaField(amiFamilyPath),
 		in.validateBlockDeviceMappings().ViaField(blockDeviceMappingsPath),
-		in.validateUserData().ViaField(userDataPath),
 		in.validateTags().ViaField(tagsPath),
 	)
 }
@@ -249,24 +247,14 @@ func (in *EC2NodeClassSpec) validateVolumeSize(blockDeviceMapping *BlockDeviceMa
 	return nil
 }
 
-func (in *EC2NodeClassSpec) validateUserData() (errs *apis.FieldError) {
-	if in.UserData == nil {
-		return nil
-	}
-	if lo.FromPtr(in.AMIFamily) == AMIFamilyWindows2019 || lo.FromPtr(in.AMIFamily) == AMIFamilyWindows2022 {
-		errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("%s AMIFamily is not currently supported with custom userData", lo.FromPtr(in.AMIFamily)), userDataPath))
-	}
-	return errs
-}
-
 func (in *EC2NodeClassSpec) validateAMIFamily() (errs *apis.FieldError) {
 	if in.AMIFamily == nil {
 		return nil
 	}
 	if *in.AMIFamily == AMIFamilyCustom && len(in.AMISelectorTerms) == 0 {
 		errs = errs.Also(apis.ErrMissingField(amiSelectorTermsPath))
 	}
-	return errs.Also(in.validateStringEnum(*in.AMIFamily, amiFamilyPath, SupportedAMIFamilies))
+	return errs
 }
 
 func (in *EC2NodeClassSpec) validateTags() (errs *apis.FieldError) {

pkg/apis/v1beta1/suite_test.go

@@ -22,10 +22,8 @@ import (
 	"github.com/Pallinder/go-randomdata"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
-	. "knative.dev/pkg/logging/testing"
-	"knative.dev/pkg/ptr"
-
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	. "knative.dev/pkg/logging/testing"
 
 	"github.com/aws/aws-sdk-go/aws"
 
@@ -71,16 +69,6 @@ var _ = Describe("Validation", func() {
 		It("should succeed if user data is empty", func() {
 			Expect(nc.Validate(ctx)).To(Succeed())
 		})
-		It("should fail if Windows2019 AMIFamily is specified", func() {
-			nc.Spec.AMIFamily = &v1alpha1.AMIFamilyWindows2019
-			nc.Spec.UserData = ptr.String("someUserData")
-			Expect(nc.Validate(ctx)).To(Not(Succeed()))
-		})
-		It("should fail if Windows2022 AMIFamily is specified", func() {
-			nc.Spec.AMIFamily = &v1alpha1.AMIFamilyWindows2022
-			nc.Spec.UserData = ptr.String("someUserData")
-			Expect(nc.Validate(ctx)).To(Not(Succeed()))
-		})
 	})
 	Context("Tags", func() {
 		It("should succeed when tags are empty", func() {
@@ -471,7 +459,7 @@ var _ = Describe("Validation", func() {
 				Spec: v1beta1.EC2NodeClassSpec{
 					AMIFamily: aws.String(v1alpha1.AMIFamilyAL2),
 					Context:   aws.String("context-1"),
-					Role:      aws.String("role-1"),
+					Role:      "role-1",
 					Tags: map[string]string{
 						"keyTag-1": "valueTag-1",
 						"keyTag-2": "valueTag-2",
@@ -498,7 +486,7 @@ var _ = Describe("Validation", func() {
 			updatedHash := nodeClass.Hash()
 			Expect(hash).ToNot(Equal(updatedHash))
 		},
-			Entry("InstanceProfile Drift", v1beta1.EC2NodeClassSpec{Role: aws.String("role-2")}),
+			Entry("InstanceProfile Drift", v1beta1.EC2NodeClassSpec{Role: "role-2"}),
 			Entry("UserData Drift", v1beta1.EC2NodeClassSpec{UserData: aws.String("userdata-test-2")}),
 			Entry("Tags Drift", v1beta1.EC2NodeClassSpec{Tags: map[string]string{"keyTag-test-3": "valueTag-test-3"}}),
 			Entry("MetadataOptions Drift", v1beta1.EC2NodeClassSpec{MetadataOptions: &v1beta1.MetadataOptions{HTTPEndpoint: aws.String("test-metadata-2")}}),

pkg/controllers/nodeclass/nodeclass_test.go

@@ -753,7 +753,7 @@ var _ = Describe("NodeClassController", func() {
 		},
 			Entry("AMIFamily Drift", &v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{AMIFamily: aws.String(v1beta1.AMIFamilyBottlerocket)}}),
 			Entry("UserData Drift", &v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{UserData: aws.String("userdata-test-2")}}),
-			Entry("Role Drift", &v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{Role: aws.String("new-role")}}),
+			Entry("Role Drift", &v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{Role: "new-role"}}),
 			Entry("Tags Drift", &v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{Tags: map[string]string{"keyTag-test-3": "valueTag-test-3"}}}),
 			Entry("BlockDeviceMappings Drift", &v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{BlockDeviceMappings: []*v1beta1.BlockDeviceMapping{{DeviceName: aws.String("map-device-test-3")}}}}),
 			Entry("DetailedMonitoring Drift", &v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{DetailedMonitoring: aws.Bool(true)}}),

pkg/test/nodeclass.go

@@ -30,6 +30,12 @@ func EC2NodeClass(overrides ...v1beta1.EC2NodeClass) *v1beta1.EC2NodeClass {
 			panic(fmt.Sprintf("Failed to merge settings: %s", err))
 		}
 	}
+	if options.Spec.AMIFamily == nil {
+		options.Spec.AMIFamily = &v1beta1.AMIFamilyAL2
+	}
+	if options.Spec.Role == "" {
+		options.Spec.Role = "test-role"
+	}
 	if len(options.Spec.SecurityGroupSelectorTerms) == 0 {
 		options.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{
 			{
@@ -39,7 +45,6 @@ func EC2NodeClass(overrides ...v1beta1.EC2NodeClass) *v1beta1.EC2NodeClass {
 			},
 		}
 	}
-
 	if len(options.Spec.SubnetSelectorTerms) == 0 {
 		options.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{
 			{

pkg/utils/nodeclass/suite_test.go

@@ -166,7 +166,7 @@ var _ = Describe("NodeClassUtils", func() {
 		Expect(nodeClass.Spec.AMISelectorTerms[0].Tags).To(Equal(nodeTemplate.Spec.AMISelector))
 		Expect(nodeClass.Spec.AMIFamily).To(Equal(nodeTemplate.Spec.AMIFamily))
 		Expect(nodeClass.Spec.UserData).To(Equal(nodeTemplate.Spec.UserData))
-		Expect(nodeClass.Spec.Role).To(BeNil())
+		Expect(nodeClass.Spec.Role).To(BeEmpty())
 		Expect(nodeClass.Spec.Tags).To(Equal(nodeTemplate.Spec.Tags))
 		ExpectBlockDeviceMappingsEqual(nodeTemplate.Spec.BlockDeviceMappings, nodeClass.Spec.BlockDeviceMappings)
 		Expect(nodeClass.Spec.DetailedMonitoring).To(Equal(nodeTemplate.Spec.DetailedMonitoring))
@@ -234,7 +234,7 @@ var _ = Describe("NodeClassUtils", func() {
 
 		Expect(nodeClass.Spec.AMIFamily).To(Equal(nodeTemplate.Spec.AMIFamily))
 		Expect(nodeClass.Spec.UserData).To(Equal(nodeTemplate.Spec.UserData))
-		Expect(nodeClass.Spec.Role).To(BeNil())
+		Expect(nodeClass.Spec.Role).To(BeEmpty())
 		Expect(nodeClass.Spec.Tags).To(Equal(nodeTemplate.Spec.Tags))
 		ExpectBlockDeviceMappingsEqual(nodeTemplate.Spec.BlockDeviceMappings, nodeClass.Spec.BlockDeviceMappings)
 		Expect(nodeClass.Spec.DetailedMonitoring).To(Equal(nodeTemplate.Spec.DetailedMonitoring))
@@ -283,7 +283,7 @@ var _ = Describe("NodeClassUtils", func() {
 
 		Expect(nodeClass.Spec.AMIFamily).To(Equal(nodeTemplate.Spec.AMIFamily))
 		Expect(nodeClass.Spec.UserData).To(Equal(nodeTemplate.Spec.UserData))
-		Expect(nodeClass.Spec.Role).To(BeNil())
+		Expect(nodeClass.Spec.Role).To(BeEmpty())
 		Expect(nodeClass.Spec.Tags).To(Equal(nodeTemplate.Spec.Tags))
 		ExpectBlockDeviceMappingsEqual(nodeTemplate.Spec.BlockDeviceMappings, nodeClass.Spec.BlockDeviceMappings)
 		Expect(nodeClass.Spec.DetailedMonitoring).To(Equal(nodeTemplate.Spec.DetailedMonitoring))
@@ -397,7 +397,7 @@ var _ = Describe("NodeClassUtils", func() {
 
 		Expect(nodeClass.Spec.AMIFamily).To(Equal(nodeTemplate.Spec.AMIFamily))
 		Expect(nodeClass.Spec.UserData).To(Equal(nodeTemplate.Spec.UserData))
-		Expect(nodeClass.Spec.Role).To(BeNil())
+		Expect(nodeClass.Spec.Role).To(BeEmpty())
 		Expect(nodeClass.Spec.Tags).To(Equal(nodeTemplate.Spec.Tags))
 		ExpectBlockDeviceMappingsEqual(nodeTemplate.Spec.BlockDeviceMappings, nodeClass.Spec.BlockDeviceMappings)
 		Expect(nodeClass.Spec.DetailedMonitoring).To(Equal(nodeTemplate.Spec.DetailedMonitoring))