From 67cf5202cdf237a3407025253bc36ba72c782c23 Mon Sep 17 00:00:00 2001 From: Amanuel Engeda Date: Tue, 3 Oct 2023 13:22:24 -0700 Subject: [PATCH] CEL validation --- .../karpenter.k8s.aws_ec2nodeclasses.yaml | 95 ++- pkg/apis/v1beta1/ec2nodeclass.go | 44 +- .../ec2nodeclass_validation_cel_test.go | 626 ++++++++++++++++++ .../ec2nodeclass_validation_webhook_test.go | 565 ++++++++++++++++ pkg/apis/v1beta1/suite_test.go | 552 +-------------- pkg/cloudprovider/nodeclaim_test.go | 5 +- pkg/controllers/nodeclass/nodeclass_test.go | 3 +- .../launchtemplate/nodeclass_test.go | 10 + 8 files changed, 1348 insertions(+), 552 deletions(-) create mode 100644 pkg/apis/v1beta1/ec2nodeclass_validation_cel_test.go create mode 100644 pkg/apis/v1beta1/ec2nodeclass_validation_webhook_test.go diff --git a/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml b/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml index a650ca0ed09c..08fa3f26ba61 100644 --- a/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml +++ b/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml @@ -77,9 +77,21 @@ spec: description: Tags is a map of key/value tags used to select subnets Specifying '*' for a value selects all values for a given tag key. + maxProperties: 20 type: object + x-kubernetes-validations: + - message: empty tag keys or values aren't supported + rule: self.all(k, k != '' && self[k] != '') type: object + maxItems: 30 type: array + x-kubernetes-validations: + - message: expected at least one, got none, ['tags', 'id', 'name'] + 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))' blockDeviceMappings: description: BlockDeviceMappings to be applied to provisioned nodes. items: @@ -133,22 +145,35 @@ spec: format: int64 type: integer volumeSize: + allOf: + - pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + - pattern: ^((?:[1-9][0-9]{0,3}|[1-4][0-9]{4}|[5][0-8][0-9]{3}|59000)(G|Gi)|([0-9]||[0-9][0-7]|58)(T|Ti))$ anyOf: - type: integer - type: string - description: "VolumeSize in GiBs. You must specify either - a snapshot ID or a volume size. The following are the - supported volumes sizes for each volume type: \n * gp2 - and gp3: 1-16,384 \n * io1 and io2: 4-16,384 \n * st1 - and sc1: 125-16,384 \n * standard: 1-1,024" - pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + description: "VolumeSize in GBs or TBs. You must specify + either a snapshot ID or a volume size. The following are + the supported volumes sizes for each volume type: \n * + gp2 and gp3: 1-16,384 \n * io1 and io2: 4-16,384 \n * + st1 and sc1: 125-16,384 \n * standard: 1-1,024" x-kubernetes-int-or-string: true volumeType: description: VolumeType of the block device. For more information, see Amazon EBS volume types (https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/EBSVolumeTypes.html) in the Amazon Elastic Compute Cloud User Guide. + enum: + - standard + - io1 + - io2 + - gp2 + - sc1 + - st1 + - gp3 type: string type: object + x-kubernetes-validations: + - message: snapshotID or volumeSize must be defined + rule: has(self.snapshotID) || has(self.volumeSize) rootVolume: description: RootVolume is a flag indicating if this device is mounted as kubelet root dir. You can configure at most @@ -186,6 +211,9 @@ spec: 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." + enum: + - enabled + - disabled type: string httpProtocolIPv6: default: disabled @@ -193,6 +221,9 @@ spec: 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". + enum: + - enabled + - disabled type: string httpPutResponseHopLimit: default: 2 @@ -202,6 +233,8 @@ spec: values are integers from 1 to 64. If metadata options is non-nil, but this parameter is not specified, the default value is 2. format: int64 + maximum: 64 + minimum: 1 type: integer httpTokens: default: required @@ -218,6 +251,9 @@ spec: 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." + enum: + - required + - optional type: string type: object role: @@ -228,6 +264,9 @@ spec: collection and drift handling is implemented for the old instance profiles on an update. type: string + x-kubernetes-validations: + - message: immutable field changed + rule: self == oldSelf securityGroupSelectorTerms: description: SecurityGroupSelectorTerms is a list of or security group selector terms. The terms are ORed. @@ -250,9 +289,25 @@ spec: description: Tags is a map of key/value tags used to select subnets Specifying '*' for a value selects all values for a given tag key. + maxProperties: 20 type: object + x-kubernetes-validations: + - message: empty tag keys or values aren't supported + rule: self.all(k, k != '' && self[k] != '') type: object + maxItems: 30 type: array + x-kubernetes-validations: + - message: securityGroupSelectorTerms cannot be empty + rule: self.size() != 0 + - message: expected at least one, got none, ['tags', 'id', 'name'] + 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 securityGroupSelectorTerms' + rule: '!self.all(x, has(x.id) && (has(x.tags) || has(x.name)))' + - message: '''name'' is mutually exclusive, cannot be set with a combination + of other fields in securityGroupSelectorTerms' + rule: '!self.all(x, has(x.name) && (has(x.tags) || has(x.id)))' subnetSelectorTerms: description: SubnetSelectorTerms is a list of or subnet selector terms. The terms are ORed. @@ -271,15 +326,39 @@ spec: description: Tags is a map of key/value tags used to select subnets Specifying '*' for a value selects all values for a given tag key. + maxProperties: 20 type: object + x-kubernetes-validations: + - message: empty tag keys oe values aren't supported + rule: self.all(k, k != '' && self[k] != '') type: object + maxItems: 30 type: array + x-kubernetes-validations: + - message: subnetSelectorTerms cannot be empty + rule: self.size() != 0 + - message: expected at least one, got none, ['tags', 'id'] + rule: self.all(x, has(x.tags) || has(x.id)) + - message: '''id'' is mutually exclusive, cannot be set with a combination + of other fields in subnetSelectorTerms' + rule: '!self.all(x, has(x.id) && has(x.tags))' tags: additionalProperties: type: string description: Tags to be applied on ec2 resources like instances and launch templates. type: object + x-kubernetes-validations: + - message: empty tag keys aren't supported + rule: self.all(k, k != '') + - message: tag contains in restricted tag matching kubernetes.io/cluster/ + rule: self.all(k, !k.startsWith('kubernetes.io/cluster') ) + - message: tag contains in restricted tag matching karpenter.sh/provisioner-name + rule: self.all(k, k != 'karpenter.sh/provisioner-name') + - message: tag contains in restricted tag matching karpenter.sh/nodepool + rule: self.all(k, k != 'karpenter.sh/nodepool') + - message: tag contains in restricted tag matching karpenter.sh/managed-by + rule: self.all(k, k !='karpenter.sh/managed-by') userData: description: UserData to be applied to the provisioned nodes. It must be in the appropriate format based on the AMIFamily in use. Karpenter @@ -292,6 +371,10 @@ spec: - securityGroupSelectorTerms - subnetSelectorTerms type: object + x-kubernetes-validations: + - message: amiSelectorTerms is required when amiFamily == 'Custom' + rule: 'self.amiFamily == ''Custom'' ? self.amiSelectorTerms.size() != + 0 : true' status: description: EC2NodeClassStatus contains the resolved state of the EC2NodeClass properties: diff --git a/pkg/apis/v1beta1/ec2nodeclass.go b/pkg/apis/v1beta1/ec2nodeclass.go index 036f8af16a2d..1102ec7b97a2 100644 --- a/pkg/apis/v1beta1/ec2nodeclass.go +++ b/pkg/apis/v1beta1/ec2nodeclass.go @@ -27,12 +27,24 @@ 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. + // +kubebuilder:validation:XValidation:message="subnetSelectorTerms cannot be empty",rule="self.size() != 0" + // +kubebuilder:validation:XValidation:message="expected at least one, got none, ['tags', 'id']",rule="self.all(x, has(x.tags) || has(x.id))" + // +kubebuilder:validation:XValidation:message="'id' is mutually exclusive, cannot be set with a combination of other fields in subnetSelectorTerms",rule="!self.all(x, has(x.id) && has(x.tags))" + // +kubebuilder:validation:MaxItems:=30 // +required SubnetSelectorTerms []SubnetSelectorTerm `json:"subnetSelectorTerms" hash:"ignore"` // SecurityGroupSelectorTerms is a list of or security group selector terms. The terms are ORed. + // +kubebuilder:validation:XValidation:message="securityGroupSelectorTerms cannot be empty",rule="self.size() != 0" + // +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 securityGroupSelectorTerms",rule="!self.all(x, has(x.id) && (has(x.tags) || has(x.name)))" + // +kubebuilder:validation:XValidation:message="'name' is mutually exclusive, cannot be set with a combination of other fields in securityGroupSelectorTerms",rule="!self.all(x, has(x.name) && (has(x.tags) || has(x.id)))" + // +kubebuilder:validation:MaxItems:=30 // +required 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:MaxItems:=30 // +optional AMISelectorTerms []AMISelectorTerm `json:"amiSelectorTerms,omitempty" hash:"ignore"` // AMIFamily is the AMI family that instances use. @@ -48,9 +60,15 @@ 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 == oldSelf",message="immutable field changed" // +required Role string `json:"role"` // Tags to be applied on ec2 resources like instances and launch templates. + // +kubebuilder:validation:XValidation:message="empty tag keys aren't supported",rule="self.all(k, k != '')" + // +kubebuilder:validation:XValidation:message="tag contains a restricted tag matching kubernetes.io/cluster/",rule="self.all(k, !k.startsWith('kubernetes.io/cluster') )" + // +kubebuilder:validation:XValidation:message="tag contains a restricted tag matching karpenter.sh/provisioner-name",rule="self.all(k, k != 'karpenter.sh/provisioner-name')" + // +kubebuilder:validation:XValidation:message="tag contains a restricted tag matching karpenter.sh/nodepool",rule="self.all(k, k != 'karpenter.sh/nodepool')" + // +kubebuilder:validation:XValidation:message="tag contains a restricted tag matching karpenter.sh/managed-by",rule="self.all(k, k !='karpenter.sh/managed-by')" // +optional Tags map[string]string `json:"tags,omitempty"` // BlockDeviceMappings to be applied to provisioned nodes. @@ -112,6 +130,8 @@ type EC2NodeClassSpec struct { type SubnetSelectorTerm struct { // Tags is a map of key/value tags used to select subnets // Specifying '*' for a value selects all values for a given tag key. + // +kubebuilder:validation:XValidation:message="empty tag keys or values aren't supported",rule="self.all(k, k != '' && self[k] != '')" + // +kubebuilder:validation:MaxProperties:=20 // +optional Tags map[string]string `json:"tags,omitempty"` // ID is the subnet id in EC2 @@ -125,6 +145,8 @@ type SubnetSelectorTerm struct { type SecurityGroupSelectorTerm struct { // Tags is a map of key/value tags used to select subnets // Specifying '*' for a value selects all values for a given tag key. + // +kubebuilder:validation:XValidation:message="empty tag keys or values aren't supported",rule="self.all(k, k != '' && self[k] != '')" + // +kubebuilder:validation:MaxProperties:=20 // +optional Tags map[string]string `json:"tags,omitempty"` // ID is the security group id in EC2 @@ -141,6 +163,8 @@ type SecurityGroupSelectorTerm struct { type AMISelectorTerm struct { // Tags is a map of key/value tags used to select subnets // Specifying '*' for a value selects all values for a given tag key. + // +kubebuilder:validation:XValidation:message="empty tag keys or values aren't supported",rule="self.all(k, k != '' && self[k] != '')" + // +kubebuilder:validation:MaxProperties:=20 // +optional Tags map[string]string `json:"tags,omitempty"` // ID is the ami id in EC2 @@ -167,12 +191,14 @@ type MetadataOptions struct { // If you specify a value of "disabled", instance metadata will not be accessible // on the node. // +kubebuilder:default=enabled + // +kubebuilder:validation:Enum:={enabled,disabled} // +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 + // +kubebuilder:validation:Enum:={enabled,disabled} // +optional HTTPProtocolIPv6 *string `json:"httpProtocolIPv6,omitempty"` // HTTPPutResponseHopLimit is the desired HTTP PUT response hop limit for @@ -181,6 +207,8 @@ type MetadataOptions struct { // If metadata options is non-nil, but this parameter is not specified, the // default value is 2. // +kubebuilder:default=2 + // +kubebuilder:validation:Minimum:=1 + // +kubebuilder:validation:Maximum:=64 // +optional HTTPPutResponseHopLimit *int64 `json:"httpPutResponseHopLimit,omitempty"` // HTTPTokens determines the state of token usage for instance metadata @@ -198,16 +226,18 @@ type MetadataOptions struct { // role credentials always returns the version 2.0 credentials; the version // 1.0 credentials are not available. // +kubebuilder:default=required + // +kubebuilder:validation:Enum:={required,optional} // +optional HTTPTokens *string `json:"httpTokens,omitempty"` } type BlockDeviceMapping struct { // The device name (for example, /dev/sdh or xvdh). - // +optional + // +required DeviceName *string `json:"deviceName,omitempty"` // EBS contains parameters used to automatically set up EBS volumes when an instance is launched. - // +optional + // +kubebuilder:validation:XValidation:message="snapshotID or volumeSize must be defined",rule="has(self.snapshotID) || has(self.volumeSize)" + // +required EBS *BlockDevice `json:"ebs,omitempty"` // RootVolume is a flag indicating if this device is mounted as kubelet root dir. You can // configure at most one root volume in BlockDeviceMappings. @@ -254,7 +284,7 @@ type BlockDevice struct { // Valid Range: Minimum value of 125. Maximum value of 1000. // +optional Throughput *int64 `json:"throughput,omitempty"` - // VolumeSize in GiBs. You must specify either a snapshot ID or + // VolumeSize in GBs or TBs. You must specify either a snapshot ID or // a volume size. The following are the supported volumes sizes for each volume // type: // @@ -265,11 +295,16 @@ type BlockDevice struct { // * st1 and sc1: 125-16,384 // // * standard: 1-1,024 + // TODO: Add the CEL resources.quantity type after k8s 1.29 + // https://github.com/kubernetes/apiserver/commit/b137c256373aec1c5d5810afbabb8932a19ecd2a#diff-838176caa5882465c9d6061febd456397a3e2b40fb423ed36f0cabb1847ecb4dR190 + // +kubebuilder:validation:Pattern:="^((?:[1-9][0-9]{0,3}|[1-4][0-9]{4}|[5][0-8][0-9]{3}|59000)(Gi)|(?:[1-9][0-9]{0,3}|[1-5][0-9]{4}|[6][0-3][0-9]{3}|64000)(G)|([1-9]||[1-5][0-7]|58)(Ti)|([1-9]||[1-5][0-9]|6[0-3]|64)(T))$" + // +kubebuilder:validation:XIntOrString // +optional - VolumeSize *resource.Quantity `json:"volumeSize,omitempty" hash:"string"` + VolumeSize *resource.Quantity `json:"volumeSize,omitempty"` // VolumeType of the block device. // For more information, see Amazon EBS volume types (https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/EBSVolumeTypes.html) // in the Amazon Elastic Compute Cloud User Guide. + // +kubebuilder:validation:Enum:={standard,io1,io2,gp2,sc1,st1,gp3} // +optional VolumeType *string `json:"volumeType,omitempty"` } @@ -282,6 +317,7 @@ type EC2NodeClass struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` + // +kubebuilder:validation:XValidation:message="amiSelectorTerms is required when amiFamily == 'Custom'",rule="self.amiFamily == 'Custom' ? self.amiSelectorTerms.size() != 0 : true" Spec EC2NodeClassSpec `json:"spec,omitempty"` Status EC2NodeClassStatus `json:"status,omitempty"` diff --git a/pkg/apis/v1beta1/ec2nodeclass_validation_cel_test.go b/pkg/apis/v1beta1/ec2nodeclass_validation_cel_test.go new file mode 100644 index 000000000000..bc1a829fdad7 --- /dev/null +++ b/pkg/apis/v1beta1/ec2nodeclass_validation_cel_test.go @@ -0,0 +1,626 @@ +/* +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 v1beta1_test + +import ( + "strings" + + "github.com/Pallinder/go-randomdata" + "github.com/imdario/mergo" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "knative.dev/pkg/apis" + + "github.com/aws/aws-sdk-go/aws" + + "github.com/aws/karpenter/pkg/apis/v1alpha1" + "github.com/aws/karpenter/pkg/apis/v1beta1" + "github.com/aws/karpenter/pkg/test" +) + +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>") + } + nc = &v1beta1.EC2NodeClass{ + ObjectMeta: metav1.ObjectMeta{Name: strings.ToLower(randomdata.SillyName())}, + Spec: v1beta1.EC2NodeClassSpec{ + AMIFamily: &v1beta1.AMIFamilyAL2, + SubnetSelectorTerms: []v1beta1.SubnetSelectorTerm{ + { + Tags: map[string]string{ + "foo": "bar", + }, + }, + }, + SecurityGroupSelectorTerms: []v1beta1.SecurityGroupSelectorTerm{ + { + Tags: map[string]string{ + "foo": "bar", + }, + }, + }, + }, + } + }) + Context("UserData", func() { + It("should succeed if user data is empty", func() { + Expect(env.Client.Create(ctx, nc)).To(Succeed()) + }) + }) + Context("Tags", func() { + It("should succeed when tags are empty", func() { + nc.Spec.Tags = map[string]string{} + Expect(env.Client.Create(ctx, nc)).To(Succeed()) + }) + It("should succeed if tags aren't in restricted tag keys", func() { + nc.Spec.Tags = map[string]string{ + "karpenter.sh/custom-key": "value", + "karpenter.sh/managed": "true", + "kubernetes.io/role/key": "value", + } + Expect(env.Client.Create(ctx, nc)).To(Succeed()) + }) + It("should fail if tags contain a restricted domain key", func() { + nc.Spec.Tags = map[string]string{ + "karpenter.sh/provisioner-name": "value", + } + Expect(env.Client.Create(ctx, nc)).To(Not(Succeed())) + nc.Spec.Tags = map[string]string{ + "kubernetes.io/cluster/test": "value", + } + Expect(env.Client.Create(ctx, nc)).To(Not(Succeed())) + nc.Spec.Tags = map[string]string{ + "karpenter.sh/managed-by": "test", + } + Expect(env.Client.Create(ctx, nc)).To(Not(Succeed())) + }) + }) + Context("SubnetSelectorTerms", func() { + It("should succeed with a valid subnet selector on tags", func() { + nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ + { + Tags: map[string]string{ + "test": "testvalue", + }, + }, + } + Expect(env.Client.Create(ctx, nc)).To(Succeed()) + }) + It("should succeed with a valid subnet selector on id", func() { + nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ + { + ID: "subnet-12345749", + }, + } + Expect(env.Client.Create(ctx, nc)).To(Succeed()) + }) + It("should fail when subnet selector terms is set to nil", func() { + nc.Spec.SubnetSelectorTerms = nil + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }) + It("should fail when no subnet selector terms exist", func() { + nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{} + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }) + It("should fail when a subnet selector term has no values", func() { + nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ + {}, + } + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }) + It("should fail when a subnet selector term has no tag map values", func() { + nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ + { + Tags: map[string]string{}, + }, + } + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }) + It("should fail when a subnet selector term has a tag map key that is empty", func() { + nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ + { + Tags: map[string]string{ + "test": "", + }, + }, + } + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }) + It("should fail when a subnet selector term has a tag map value that is empty", func() { + nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ + { + Tags: map[string]string{ + "": "testvalue", + }, + }, + } + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }) + It("should fail when the last subnet selector is invalid", func() { + nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ + { + Tags: map[string]string{ + "test": "testvalue", + }, + }, + { + Tags: map[string]string{ + "test2": "testvalue2", + }, + }, + { + Tags: map[string]string{ + "test3": "testvalue3", + }, + }, + { + Tags: map[string]string{ + "": "testvalue4", + }, + }, + } + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }) + It("should fail when specifying id with tags", func() { + nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ + { + ID: "subnet-12345749", + Tags: map[string]string{ + "test": "testvalue", + }, + }, + } + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }) + }) + Context("SecurityGroupSelectorTerms", func() { + It("should succeed with a valid security group selector on tags", func() { + nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + { + Tags: map[string]string{ + "test": "testvalue", + }, + }, + } + Expect(env.Client.Create(ctx, nc)).To(Succeed()) + }) + It("should succeed with a valid security group selector on id", func() { + nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + { + ID: "sg-12345749", + }, + } + Expect(env.Client.Create(ctx, nc)).To(Succeed()) + }) + It("should succeed with a valid security group selector on name", func() { + nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + { + Name: "testname", + }, + } + Expect(env.Client.Create(ctx, nc)).To(Succeed()) + }) + It("should fail when security group selector terms is set to nil", func() { + nc.Spec.SecurityGroupSelectorTerms = nil + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }) + It("should fail when no security group selector terms exist", func() { + nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{} + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }) + It("should fail when a security group selector term has no values", func() { + nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + {}, + } + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }) + It("should fail when a security group selector term has no tag map values", func() { + nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + { + Tags: map[string]string{}, + }, + } + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }) + It("should fail when a security group selector term has a tag map key that is empty", func() { + nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + { + Tags: map[string]string{ + "test": "", + }, + }, + } + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }) + It("should fail when a security group selector term has a tag map value that is empty", func() { + nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + { + Tags: map[string]string{ + "": "testvalue", + }, + }, + } + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }) + It("should fail when the last security group selector is invalid", func() { + nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + { + Tags: map[string]string{ + "test": "testvalue", + }, + }, + { + Tags: map[string]string{ + "test2": "testvalue2", + }, + }, + { + Tags: map[string]string{ + "test3": "testvalue3", + }, + }, + { + Tags: map[string]string{ + "": "testvalue4", + }, + }, + } + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }) + It("should fail when specifying id with tags", func() { + nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + { + ID: "sg-12345749", + Tags: map[string]string{ + "test": "testvalue", + }, + }, + } + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }) + It("should fail when specifying id with name", func() { + nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + { + ID: "sg-12345749", + Name: "my-security-group", + }, + } + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }) + It("should fail when specifying name with tags", func() { + nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + { + Name: "my-security-group", + Tags: map[string]string{ + "test": "testvalue", + }, + }, + } + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }) + }) + Context("AMISelectorTerms", func() { + It("should succeed with a valid ami selector on tags", func() { + nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ + { + Tags: map[string]string{ + "test": "testvalue", + }, + }, + } + Expect(env.Client.Create(ctx, nc)).To(Succeed()) + }) + It("should succeed with a valid ami selector on id", func() { + nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ + { + ID: "ami-12345749", + }, + } + Expect(env.Client.Create(ctx, nc)).To(Succeed()) + }) + It("should succeed with a valid ami selector on name", func() { + nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ + { + Name: "testname", + }, + } + 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{ + {}, + } + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }) + It("should fail when a ami selector term has no tag map values", func() { + nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ + { + Tags: map[string]string{}, + }, + } + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }) + It("should fail when a ami selector term has a tag map key that is empty", func() { + nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ + { + Tags: map[string]string{ + "test": "", + }, + }, + } + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }) + It("should fail when a ami selector term has a tag map value that is empty", func() { + nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ + { + Tags: map[string]string{ + "": "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{ + { + Tags: map[string]string{ + "test": "testvalue", + }, + }, + { + Tags: map[string]string{ + "test2": "testvalue2", + }, + }, + { + Tags: map[string]string{ + "test3": "testvalue3", + }, + }, + { + Tags: map[string]string{ + "": "testvalue4", + }, + }, + } + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }) + It("should fail when specifying id with tags", func() { + nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ + { + ID: "ami-12345749", + Tags: map[string]string{ + "test": "testvalue", + }, + }, + } + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }) + It("should fail when specifying id with name", func() { + nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ + { + ID: "ami-12345749", + Name: "my-custom-ami", + }, + } + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }) + It("should fail when specifying id with owner", func() { + nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ + { + ID: "ami-12345749", + Owner: "123456789", + }, + } + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }) + It("should fail when AMIFamily is Custom and not AMISelectorTerms", func() { + nc.Spec.AMIFamily = &v1alpha1.AMIFamilyCustom + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }) + }) + Context("MetadataOptions", func() { + It("should succeed for valid inputs", func() { + nc.Spec.MetadataOptions = &v1beta1.MetadataOptions{ + HTTPEndpoint: aws.String("disabled"), + HTTPProtocolIPv6: aws.String("enabled"), + HTTPPutResponseHopLimit: aws.Int64(34), + HTTPTokens: aws.String("optional"), + } + Expect(env.Client.Create(ctx, nc)).To(Succeed()) + }) + It("should fail for invalid for HTTPEndpoint", func() { + nc.Spec.MetadataOptions = &v1beta1.MetadataOptions{ + HTTPEndpoint: aws.String("test"), + } + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }) + It("should fail for invalid for HTTPProtocolIPv6", func() { + nc.Spec.MetadataOptions = &v1beta1.MetadataOptions{ + HTTPProtocolIPv6: aws.String("test"), + } + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }) + It("should fail for invalid for HTTPPutResponseHopLimit", func() { + nc.Spec.MetadataOptions = &v1beta1.MetadataOptions{ + HTTPPutResponseHopLimit: aws.Int64(-5), + } + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }) + It("should fail for invalid for HTTPTokens", func() { + nc.Spec.MetadataOptions = &v1beta1.MetadataOptions{ + HTTPTokens: aws.String("test"), + } + Expect(env.Client.Create(ctx, nc)).ToNot(Succeed()) + }) + }) + Context("EC2NodeClass Hash", func() { + var nodeClass *v1beta1.EC2NodeClass + BeforeEach(func() { + nodeClass = test.EC2NodeClass(v1beta1.EC2NodeClass{ + Spec: v1beta1.EC2NodeClassSpec{ + AMIFamily: aws.String(v1alpha1.AMIFamilyAL2), + Context: aws.String("context-1"), + Role: "role-1", + Tags: map[string]string{ + "keyTag-1": "valueTag-1", + "keyTag-2": "valueTag-2", + }, + MetadataOptions: &v1beta1.MetadataOptions{ + HTTPEndpoint: aws.String("test-metadata-1"), + }, + BlockDeviceMappings: []*v1beta1.BlockDeviceMapping{ + { + DeviceName: aws.String("map-device-1"), + }, + { + DeviceName: aws.String("map-device-2"), + }, + }, + UserData: aws.String("userdata-test-1"), + DetailedMonitoring: aws.Bool(false), + }, + }) + }) + DescribeTable("should change hash when static fields are updated", func(changes v1beta1.EC2NodeClass) { + hash := nodeClass.Hash() + Expect(mergo.Merge(nodeClass, changes, mergo.WithOverride)).To(Succeed()) + updatedHash := nodeClass.Hash() + Expect(hash).ToNot(Equal(updatedHash)) + }, + Entry("InstanceProfile Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{Role: "role-2"}}), + Entry("UserData Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{UserData: aws.String("userdata-test-2")}}), + Entry("Tags Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{Tags: map[string]string{"keyTag-test-3": "valueTag-test-3"}}}), + Entry("MetadataOptions Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{MetadataOptions: &v1beta1.MetadataOptions{HTTPEndpoint: aws.String("test-metadata-2")}}}), + Entry("BlockDeviceMappings Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{BlockDeviceMappings: []*v1beta1.BlockDeviceMapping{{DeviceName: aws.String("map-device-test-3")}}}}), + Entry("Context Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{Context: aws.String("context-2")}}), + Entry("DetailedMonitoring Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{DetailedMonitoring: aws.Bool(true)}}), + Entry("AMIFamily Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{AMIFamily: aws.String(v1alpha1.AMIFamilyBottlerocket)}}), + ) + DescribeTable("should not change hash when slices are re-ordered", func(changes v1beta1.EC2NodeClass) { + hash := nodeClass.Hash() + Expect(mergo.Merge(nodeClass, changes, mergo.WithOverride)).To(Succeed()) + updatedHash := nodeClass.Hash() + Expect(hash).To(Equal(updatedHash)) + }, + Entry("Reorder Tags", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{Tags: map[string]string{"keyTag-2": "valueTag-2", "keyTag-1": "valueTag-1"}}}), + Entry("Reorder BlockDeviceMapping", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{BlockDeviceMappings: []*v1beta1.BlockDeviceMapping{{DeviceName: aws.String("map-device-2")}, {DeviceName: aws.String("map-device-1")}}}}), + ) + It("should not change hash when behavior/dynamic fields are updated", func() { + hash := nodeClass.Hash() + + // Update a behavior/dynamic field + nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ + { + Tags: map[string]string{"subnet-test-key": "subnet-test-value"}, + }, + } + nodeClass.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + { + Tags: map[string]string{"sg-test-key": "sg-test-value"}, + }, + } + nodeClass.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ + { + Tags: map[string]string{"ami-test-key": "ami-test-value"}, + }, + } + updatedHash := nodeClass.Hash() + Expect(hash).To(Equal(updatedHash)) + }) + It("should expect two provisioner with the same spec to have the same provisioner hash", func() { + otherNodeClass := test.EC2NodeClass(v1beta1.EC2NodeClass{ + Spec: nodeClass.Spec, + }) + Expect(nodeClass.Hash()).To(Equal(otherNodeClass.Hash())) + }) + }) + Context("BlockDeviceMappings", func() { + It("should fail if more than one root volume is specified", func() { + nodeClass := test.EC2NodeClass(v1beta1.EC2NodeClass{ + Spec: v1beta1.EC2NodeClassSpec{ + BlockDeviceMappings: []*v1beta1.BlockDeviceMapping{ + { + DeviceName: aws.String("map-device-1"), + EBS: &v1beta1.BlockDevice{ + VolumeSize: resource.NewScaledQuantity(50, resource.Giga), + }, + + RootVolume: true, + }, + { + DeviceName: aws.String("map-device-2"), + EBS: &v1beta1.BlockDevice{ + VolumeSize: resource.NewScaledQuantity(50, resource.Giga), + }, + + RootVolume: true, + }, + }, + }, + }) + Expect(nodeClass.Validate(ctx)).To(Not(Succeed())) + }) + It("should fail VolumeSize is less then 1G", func() { + nodeClass := test.EC2NodeClass(v1beta1.EC2NodeClass{ + Spec: v1beta1.EC2NodeClassSpec{ + BlockDeviceMappings: []*v1beta1.BlockDeviceMapping{ + { + DeviceName: aws.String("map-device-1"), + EBS: &v1beta1.BlockDevice{ + VolumeSize: resource.NewScaledQuantity(1, resource.Milli), + }, + RootVolume: false, + }, + }, + }, + }) + Expect(nodeClass.Validate(ctx)).To(Not(Succeed())) + }) + It("should fail VolumeSize is greater then 58T", func() { + nodeClass := test.EC2NodeClass(v1beta1.EC2NodeClass{ + Spec: v1beta1.EC2NodeClassSpec{ + BlockDeviceMappings: []*v1beta1.BlockDeviceMapping{ + { + DeviceName: aws.String("map-device-1"), + EBS: &v1beta1.BlockDevice{ + VolumeSize: resource.NewScaledQuantity(100, resource.Tera), + }, + RootVolume: false, + }, + }, + }, + }) + Expect(nodeClass.Validate(ctx)).To(Not(Succeed())) + }) + }) + Context("Role Immutability", func() { + It("should fail when updating the role", func() { + nc.Spec.Role = "test-role" + Expect(env.Client.Create(ctx, nc)).To(Succeed()) + + updateCtx := apis.WithinUpdate(ctx, nc.DeepCopy()) + nc.Spec.Role = "test-role2" + Expect(nc.Validate(updateCtx)).ToNot(Succeed()) + }) + }) +}) diff --git a/pkg/apis/v1beta1/ec2nodeclass_validation_webhook_test.go b/pkg/apis/v1beta1/ec2nodeclass_validation_webhook_test.go new file mode 100644 index 000000000000..1a7f53836dd8 --- /dev/null +++ b/pkg/apis/v1beta1/ec2nodeclass_validation_webhook_test.go @@ -0,0 +1,565 @@ +/* +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 v1beta1_test + +import ( + "strings" + + "github.com/Pallinder/go-randomdata" + "github.com/imdario/mergo" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "knative.dev/pkg/apis" + + "github.com/aws/aws-sdk-go/aws" + + "github.com/aws/karpenter/pkg/apis/v1alpha1" + "github.com/aws/karpenter/pkg/apis/v1beta1" + "github.com/aws/karpenter/pkg/test" +) + +var _ = Describe("Webhook/Validation", func() { + var nc *v1beta1.EC2NodeClass + + BeforeEach(func() { + nc = &v1beta1.EC2NodeClass{ + ObjectMeta: metav1.ObjectMeta{Name: strings.ToLower(randomdata.SillyName())}, + Spec: v1beta1.EC2NodeClassSpec{ + SubnetSelectorTerms: []v1beta1.SubnetSelectorTerm{ + { + Tags: map[string]string{ + "foo": "bar", + }, + }, + }, + SecurityGroupSelectorTerms: []v1beta1.SecurityGroupSelectorTerm{ + { + Tags: map[string]string{ + "foo": "bar", + }, + }, + }, + }, + } + }) + + Context("UserData", func() { + It("should succeed if user data is empty", func() { + Expect(nc.Validate(ctx)).To(Succeed()) + }) + }) + Context("Tags", func() { + It("should succeed when tags are empty", func() { + nc.Spec.Tags = map[string]string{} + Expect(nc.Validate(ctx)).To(Succeed()) + }) + It("should succeed if tags aren't in restricted tag keys", func() { + nc.Spec.Tags = map[string]string{ + "karpenter.sh/custom-key": "value", + "karpenter.sh/managed": "true", + "kubernetes.io/role/key": "value", + } + Expect(nc.Validate(ctx)).To(Succeed()) + }) + It("should succeed by validating that regex is properly escaped", func() { + nc.Spec.Tags = map[string]string{ + "karpenterzsh/provisioner-name": "value", + } + Expect(nc.Validate(ctx)).To(Succeed()) + nc.Spec.Tags = map[string]string{ + "kubernetesbio/cluster/test": "value", + } + Expect(nc.Validate(ctx)).To(Succeed()) + nc.Spec.Tags = map[string]string{ + "karpenterzsh/managed-by": "test", + } + Expect(nc.Validate(ctx)).To(Succeed()) + }) + It("should fail if tags contain a restricted domain key", func() { + nc.Spec.Tags = map[string]string{ + "karpenter.sh/provisioner-name": "value", + } + Expect(nc.Validate(ctx)).To(Not(Succeed())) + nc.Spec.Tags = map[string]string{ + "kubernetes.io/cluster/test": "value", + } + Expect(nc.Validate(ctx)).To(Not(Succeed())) + nc.Spec.Tags = map[string]string{ + "karpenter.sh/managed-by": "test", + } + Expect(nc.Validate(ctx)).To(Not(Succeed())) + }) + }) + Context("SubnetSelectorTerms", func() { + It("should succeed with a valid subnet selector on tags", func() { + nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ + { + Tags: map[string]string{ + "test": "testvalue", + }, + }, + } + Expect(nc.Validate(ctx)).To(Succeed()) + }) + It("should succeed with a valid subnet selector on id", func() { + nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ + { + ID: "subnet-12345749", + }, + } + Expect(nc.Validate(ctx)).To(Succeed()) + }) + It("should fail when subnet selector terms is set to nil", func() { + nc.Spec.SubnetSelectorTerms = nil + Expect(nc.Validate(ctx)).ToNot(Succeed()) + }) + It("should fail when no subnet selector terms exist", func() { + nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{} + Expect(nc.Validate(ctx)).ToNot(Succeed()) + }) + It("should fail when a subnet selector term has no values", func() { + nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ + {}, + } + Expect(nc.Validate(ctx)).ToNot(Succeed()) + }) + It("should fail when a subnet selector term has no tag map values", func() { + nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ + { + Tags: map[string]string{}, + }, + } + Expect(nc.Validate(ctx)).ToNot(Succeed()) + }) + It("should fail when a subnet selector term has a tag map key that is empty", func() { + nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ + { + Tags: map[string]string{ + "test": "", + }, + }, + } + Expect(nc.Validate(ctx)).ToNot(Succeed()) + }) + It("should fail when a subnet selector term has a tag map value that is empty", func() { + nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ + { + Tags: map[string]string{ + "": "testvalue", + }, + }, + } + Expect(nc.Validate(ctx)).ToNot(Succeed()) + }) + It("should fail when the last subnet selector is invalid", func() { + nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ + { + Tags: map[string]string{ + "test": "testvalue", + }, + }, + { + Tags: map[string]string{ + "test2": "testvalue2", + }, + }, + { + Tags: map[string]string{ + "test3": "testvalue3", + }, + }, + { + Tags: map[string]string{ + "": "testvalue4", + }, + }, + } + Expect(nc.Validate(ctx)).ToNot(Succeed()) + }) + It("should fail when specifying id with tags", func() { + nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ + { + ID: "subnet-12345749", + Tags: map[string]string{ + "test": "testvalue", + }, + }, + } + Expect(nc.Validate(ctx)).ToNot(Succeed()) + }) + }) + Context("SecurityGroupSelectorTerms", func() { + It("should succeed with a valid security group selector on tags", func() { + nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + { + Tags: map[string]string{ + "test": "testvalue", + }, + }, + } + Expect(nc.Validate(ctx)).To(Succeed()) + }) + It("should succeed with a valid security group selector on id", func() { + nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + { + ID: "sg-12345749", + }, + } + Expect(nc.Validate(ctx)).To(Succeed()) + }) + It("should succeed with a valid security group selector on name", func() { + nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + { + Name: "testname", + }, + } + Expect(nc.Validate(ctx)).To(Succeed()) + }) + It("should fail when security group selector terms is set to nil", func() { + nc.Spec.SecurityGroupSelectorTerms = nil + Expect(nc.Validate(ctx)).ToNot(Succeed()) + }) + It("should fail when no security group selector terms exist", func() { + nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{} + Expect(nc.Validate(ctx)).ToNot(Succeed()) + }) + It("should fail when a security group selector term has no values", func() { + nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + {}, + } + Expect(nc.Validate(ctx)).ToNot(Succeed()) + }) + It("should fail when a security group selector term has no tag map values", func() { + nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + { + Tags: map[string]string{}, + }, + } + Expect(nc.Validate(ctx)).ToNot(Succeed()) + }) + It("should fail when a security group selector term has a tag map key that is empty", func() { + nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + { + Tags: map[string]string{ + "test": "", + }, + }, + } + Expect(nc.Validate(ctx)).ToNot(Succeed()) + }) + It("should fail when a security group selector term has a tag map value that is empty", func() { + nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + { + Tags: map[string]string{ + "": "testvalue", + }, + }, + } + Expect(nc.Validate(ctx)).ToNot(Succeed()) + }) + It("should fail when the last security group selector is invalid", func() { + nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + { + Tags: map[string]string{ + "test": "testvalue", + }, + }, + { + Tags: map[string]string{ + "test2": "testvalue2", + }, + }, + { + Tags: map[string]string{ + "test3": "testvalue3", + }, + }, + { + Tags: map[string]string{ + "": "testvalue4", + }, + }, + } + Expect(nc.Validate(ctx)).ToNot(Succeed()) + }) + It("should fail when specifying id with tags", func() { + nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + { + ID: "sg-12345749", + Tags: map[string]string{ + "test": "testvalue", + }, + }, + } + Expect(nc.Validate(ctx)).ToNot(Succeed()) + }) + It("should fail when specifying id with name", func() { + nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + { + ID: "sg-12345749", + Name: "my-security-group", + }, + } + Expect(nc.Validate(ctx)).ToNot(Succeed()) + }) + It("should fail when specifying name with tags", func() { + nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + { + Name: "my-security-group", + Tags: map[string]string{ + "test": "testvalue", + }, + }, + } + Expect(nc.Validate(ctx)).ToNot(Succeed()) + }) + }) + Context("AMISelectorTerms", func() { + It("should succeed with a valid ami selector on tags", func() { + nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ + { + Tags: map[string]string{ + "test": "testvalue", + }, + }, + } + Expect(nc.Validate(ctx)).To(Succeed()) + }) + It("should succeed with a valid ami selector on id", func() { + nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ + { + ID: "sg-12345749", + }, + } + Expect(nc.Validate(ctx)).To(Succeed()) + }) + It("should succeed with a valid ami selector on name", func() { + nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ + { + Name: "testname", + }, + } + Expect(nc.Validate(ctx)).To(Succeed()) + }) + It("should fail when a ami selector term has no values", func() { + nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ + {}, + } + Expect(nc.Validate(ctx)).ToNot(Succeed()) + }) + It("should fail when a ami selector term has no tag map values", func() { + nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ + { + Tags: map[string]string{}, + }, + } + Expect(nc.Validate(ctx)).ToNot(Succeed()) + }) + It("should fail when a ami selector term has a tag map key that is empty", func() { + nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ + { + Tags: map[string]string{ + "test": "", + }, + }, + } + Expect(nc.Validate(ctx)).ToNot(Succeed()) + }) + It("should fail when a ami selector term has a tag map value that is empty", func() { + nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ + { + Tags: map[string]string{ + "": "testvalue", + }, + }, + } + Expect(nc.Validate(ctx)).ToNot(Succeed()) + }) + It("should fail when the last ami selector is invalid", func() { + nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ + { + Tags: map[string]string{ + "test": "testvalue", + }, + }, + { + Tags: map[string]string{ + "test2": "testvalue2", + }, + }, + { + Tags: map[string]string{ + "test3": "testvalue3", + }, + }, + { + Tags: map[string]string{ + "": "testvalue4", + }, + }, + } + Expect(nc.Validate(ctx)).ToNot(Succeed()) + }) + It("should fail when specifying id with tags", func() { + nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ + { + ID: "ami-12345749", + Tags: map[string]string{ + "test": "testvalue", + }, + }, + } + Expect(nc.Validate(ctx)).ToNot(Succeed()) + }) + It("should fail when specifying id with name", func() { + nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ + { + ID: "ami-12345749", + Name: "my-custom-ami", + }, + } + Expect(nc.Validate(ctx)).ToNot(Succeed()) + }) + It("should fail when specifying id with owner", func() { + nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ + { + ID: "ami-12345749", + Owner: "123456789", + }, + } + Expect(nc.Validate(ctx)).ToNot(Succeed()) + }) + }) + Context("EC2NodeClass Hash", func() { + var nodeClass *v1beta1.EC2NodeClass + BeforeEach(func() { + nodeClass = test.EC2NodeClass(v1beta1.EC2NodeClass{ + Spec: v1beta1.EC2NodeClassSpec{ + AMIFamily: aws.String(v1alpha1.AMIFamilyAL2), + Context: aws.String("context-1"), + Role: "role-1", + Tags: map[string]string{ + "keyTag-1": "valueTag-1", + "keyTag-2": "valueTag-2", + }, + MetadataOptions: &v1beta1.MetadataOptions{ + HTTPEndpoint: aws.String("test-metadata-1"), + }, + BlockDeviceMappings: []*v1beta1.BlockDeviceMapping{ + { + DeviceName: aws.String("map-device-1"), + }, + { + DeviceName: aws.String("map-device-2"), + }, + }, + UserData: aws.String("userdata-test-1"), + DetailedMonitoring: aws.Bool(false), + }, + }) + }) + DescribeTable("should change hash when static fields are updated", func(changes v1beta1.EC2NodeClass) { + hash := nodeClass.Hash() + Expect(mergo.Merge(nodeClass, changes, mergo.WithOverride)).To(Succeed()) + updatedHash := nodeClass.Hash() + Expect(hash).ToNot(Equal(updatedHash)) + }, + Entry("InstanceProfile Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{Role: "role-2"}}), + Entry("UserData Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{UserData: aws.String("userdata-test-2")}}), + Entry("Tags Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{Tags: map[string]string{"keyTag-test-3": "valueTag-test-3"}}}), + Entry("MetadataOptions Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{MetadataOptions: &v1beta1.MetadataOptions{HTTPEndpoint: aws.String("test-metadata-2")}}}), + Entry("BlockDeviceMappings Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{BlockDeviceMappings: []*v1beta1.BlockDeviceMapping{{DeviceName: aws.String("map-device-test-3")}}}}), + Entry("Context Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{Context: aws.String("context-2")}}), + Entry("DetailedMonitoring Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{DetailedMonitoring: aws.Bool(true)}}), + Entry("AMIFamily Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{AMIFamily: aws.String(v1alpha1.AMIFamilyBottlerocket)}}), + ) + DescribeTable("should not change hash when slices are re-ordered", func(changes v1beta1.EC2NodeClass) { + hash := nodeClass.Hash() + Expect(mergo.Merge(nodeClass, changes, mergo.WithOverride)).To(Succeed()) + updatedHash := nodeClass.Hash() + Expect(hash).To(Equal(updatedHash)) + }, + Entry("Reorder Tags", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{Tags: map[string]string{"keyTag-2": "valueTag-2", "keyTag-1": "valueTag-1"}}}), + Entry("Reorder BlockDeviceMapping", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{BlockDeviceMappings: []*v1beta1.BlockDeviceMapping{{DeviceName: aws.String("map-device-2")}, {DeviceName: aws.String("map-device-1")}}}}), + ) + It("should not change hash when behavior/dynamic fields are updated", func() { + hash := nodeClass.Hash() + + // Update a behavior/dynamic field + nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ + { + Tags: map[string]string{"subnet-test-key": "subnet-test-value"}, + }, + } + nodeClass.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + { + Tags: map[string]string{"sg-test-key": "sg-test-value"}, + }, + } + nodeClass.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ + { + Tags: map[string]string{"ami-test-key": "ami-test-value"}, + }, + } + updatedHash := nodeClass.Hash() + Expect(hash).To(Equal(updatedHash)) + }) + It("should expect two provisioner with the same spec to have the same provisioner hash", func() { + otherNodeClass := test.EC2NodeClass(v1beta1.EC2NodeClass{ + Spec: nodeClass.Spec, + }) + Expect(nodeClass.Hash()).To(Equal(otherNodeClass.Hash())) + }) + }) + Context("BlockDeviceMappings", func() { + It("should fail if more than one root volume is specified", func() { + nodeClass := test.EC2NodeClass(v1beta1.EC2NodeClass{ + Spec: v1beta1.EC2NodeClassSpec{ + BlockDeviceMappings: []*v1beta1.BlockDeviceMapping{ + { + DeviceName: aws.String("map-device-1"), + EBS: &v1beta1.BlockDevice{ + VolumeSize: resource.NewScaledQuantity(50, resource.Giga), + }, + + RootVolume: true, + }, + { + DeviceName: aws.String("map-device-2"), + EBS: &v1beta1.BlockDevice{ + VolumeSize: resource.NewScaledQuantity(50, resource.Giga), + }, + + RootVolume: true, + }, + }, + }, + }) + Expect(nodeClass.Validate(ctx)).To(Not(Succeed())) + }) + }) + Context("Role Immutability", func() { + It("should fail when updating the role", func() { + nc.Spec.Role = "test-role" + Expect(nc.Validate(ctx)).To(Succeed()) + + updateCtx := apis.WithinUpdate(ctx, nc.DeepCopy()) + nc.Spec.Role = "test-role2" + Expect(nc.Validate(updateCtx)).ToNot(Succeed()) + }) + }) +}) diff --git a/pkg/apis/v1beta1/suite_test.go b/pkg/apis/v1beta1/suite_test.go index 627d6e9e7e4a..869cf13c4775 100644 --- a/pkg/apis/v1beta1/suite_test.go +++ b/pkg/apis/v1beta1/suite_test.go @@ -16,26 +16,23 @@ package v1beta1_test import ( "context" - "strings" "testing" - "github.com/Pallinder/go-randomdata" - "github.com/imdario/mergo" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "k8s.io/apimachinery/pkg/api/resource" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "knative.dev/pkg/apis" . "knative.dev/pkg/logging/testing" - "github.com/aws/aws-sdk-go/aws" + . "github.com/aws/karpenter-core/pkg/test/expectations" - "github.com/aws/karpenter/pkg/apis/v1alpha1" - "github.com/aws/karpenter/pkg/apis/v1beta1" + "github.com/aws/karpenter-core/pkg/operator/scheme" + coretest "github.com/aws/karpenter-core/pkg/test" + "github.com/aws/karpenter/pkg/apis" "github.com/aws/karpenter/pkg/test" ) var ctx context.Context +var env *coretest.Environment +var awsEnv *test.Environment func TestAPIs(t *testing.T) { ctx = TestContextWithLogger(t) @@ -43,534 +40,15 @@ func TestAPIs(t *testing.T) { RunSpecs(t, "Validation") } -var _ = Describe("Validation", func() { - var nc *v1beta1.EC2NodeClass - - BeforeEach(func() { - nc = &v1beta1.EC2NodeClass{ - ObjectMeta: metav1.ObjectMeta{Name: strings.ToLower(randomdata.SillyName())}, - Spec: v1beta1.EC2NodeClassSpec{ - SubnetSelectorTerms: []v1beta1.SubnetSelectorTerm{ - { - Tags: map[string]string{ - "foo": "bar", - }, - }, - }, - SecurityGroupSelectorTerms: []v1beta1.SecurityGroupSelectorTerm{ - { - Tags: map[string]string{ - "foo": "bar", - }, - }, - }, - }, - } - }) - - Context("UserData", func() { - It("should succeed if user data is empty", func() { - Expect(nc.Validate(ctx)).To(Succeed()) - }) - }) - Context("Tags", func() { - It("should succeed when tags are empty", func() { - nc.Spec.Tags = map[string]string{} - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should succeed if tags aren't in restricted tag keys", func() { - nc.Spec.Tags = map[string]string{ - "karpenter.sh/custom-key": "value", - "karpenter.sh/managed": "true", - "kubernetes.io/role/key": "value", - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should succeed by validating that regex is properly escaped", func() { - nc.Spec.Tags = map[string]string{ - "karpenterzsh/provisioner-name": "value", - } - Expect(nc.Validate(ctx)).To(Succeed()) - nc.Spec.Tags = map[string]string{ - "kubernetesbio/cluster/test": "value", - } - Expect(nc.Validate(ctx)).To(Succeed()) - nc.Spec.Tags = map[string]string{ - "karpenterzsh/managed-by": "test", - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should fail if tags contain a restricted domain key", func() { - nc.Spec.Tags = map[string]string{ - "karpenter.sh/provisioner-name": "value", - } - Expect(nc.Validate(ctx)).To(Not(Succeed())) - nc.Spec.Tags = map[string]string{ - "kubernetes.io/cluster/test": "value", - } - Expect(nc.Validate(ctx)).To(Not(Succeed())) - nc.Spec.Tags = map[string]string{ - "karpenter.sh/managed-by": "test", - } - Expect(nc.Validate(ctx)).To(Not(Succeed())) - }) - }) - Context("SubnetSelectorTerms", func() { - It("should succeed with a valid subnet selector on tags", func() { - nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ - { - Tags: map[string]string{ - "test": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should succeed with a valid subnet selector on id", func() { - nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ - { - ID: "subnet-12345749", - }, - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should fail when subnet selector terms is set to nil", func() { - nc.Spec.SubnetSelectorTerms = nil - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when no subnet selector terms exist", func() { - nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{} - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a subnet selector term has no values", func() { - nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ - {}, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a subnet selector term has no tag map values", func() { - nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ - { - Tags: map[string]string{}, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a subnet selector term has a tag map key that is empty", func() { - nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ - { - Tags: map[string]string{ - "test": "", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a subnet selector term has a tag map value that is empty", func() { - nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ - { - Tags: map[string]string{ - "": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when the last subnet selector is invalid", func() { - nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ - { - Tags: map[string]string{ - "test": "testvalue", - }, - }, - { - Tags: map[string]string{ - "test2": "testvalue2", - }, - }, - { - Tags: map[string]string{ - "test3": "testvalue3", - }, - }, - { - Tags: map[string]string{ - "": "testvalue4", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when specifying id with tags", func() { - nc.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ - { - ID: "subnet-12345749", - Tags: map[string]string{ - "test": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - }) - Context("SecurityGroupSelectorTerms", func() { - It("should succeed with a valid security group selector on tags", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ - { - Tags: map[string]string{ - "test": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should succeed with a valid security group selector on id", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ - { - ID: "sg-12345749", - }, - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should succeed with a valid security group selector on name", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ - { - Name: "testname", - }, - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should fail when security group selector terms is set to nil", func() { - nc.Spec.SecurityGroupSelectorTerms = nil - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when no security group selector terms exist", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{} - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a security group selector term has no values", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ - {}, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a security group selector term has no tag map values", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ - { - Tags: map[string]string{}, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a security group selector term has a tag map key that is empty", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ - { - Tags: map[string]string{ - "test": "", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a security group selector term has a tag map value that is empty", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ - { - Tags: map[string]string{ - "": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when the last security group selector is invalid", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ - { - Tags: map[string]string{ - "test": "testvalue", - }, - }, - { - Tags: map[string]string{ - "test2": "testvalue2", - }, - }, - { - Tags: map[string]string{ - "test3": "testvalue3", - }, - }, - { - Tags: map[string]string{ - "": "testvalue4", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when specifying id with tags", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ - { - ID: "sg-12345749", - Tags: map[string]string{ - "test": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when specifying id with name", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ - { - ID: "sg-12345749", - Name: "my-security-group", - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when specifying name with tags", func() { - nc.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ - { - Name: "my-security-group", - Tags: map[string]string{ - "test": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - }) - Context("AMISelectorTerms", func() { - It("should succeed with a valid ami selector on tags", func() { - nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ - { - Tags: map[string]string{ - "test": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should succeed with a valid ami selector on id", func() { - nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ - { - ID: "sg-12345749", - }, - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should succeed with a valid ami selector on name", func() { - nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ - { - Name: "testname", - }, - } - Expect(nc.Validate(ctx)).To(Succeed()) - }) - It("should fail when a ami selector term has no values", func() { - nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ - {}, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a ami selector term has no tag map values", func() { - nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ - { - Tags: map[string]string{}, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a ami selector term has a tag map key that is empty", func() { - nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ - { - Tags: map[string]string{ - "test": "", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when a ami selector term has a tag map value that is empty", func() { - nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ - { - Tags: map[string]string{ - "": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when the last ami selector is invalid", func() { - nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ - { - Tags: map[string]string{ - "test": "testvalue", - }, - }, - { - Tags: map[string]string{ - "test2": "testvalue2", - }, - }, - { - Tags: map[string]string{ - "test3": "testvalue3", - }, - }, - { - Tags: map[string]string{ - "": "testvalue4", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when specifying id with tags", func() { - nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ - { - ID: "ami-12345749", - Tags: map[string]string{ - "test": "testvalue", - }, - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when specifying id with name", func() { - nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ - { - ID: "ami-12345749", - Name: "my-custom-ami", - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - It("should fail when specifying id with owner", func() { - nc.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ - { - ID: "ami-12345749", - Owner: "123456789", - }, - } - Expect(nc.Validate(ctx)).ToNot(Succeed()) - }) - }) - Context("EC2NodeClass Hash", func() { - var nodeClass *v1beta1.EC2NodeClass - BeforeEach(func() { - nodeClass = test.EC2NodeClass(v1beta1.EC2NodeClass{ - Spec: v1beta1.EC2NodeClassSpec{ - AMIFamily: aws.String(v1alpha1.AMIFamilyAL2), - Context: aws.String("context-1"), - Role: "role-1", - Tags: map[string]string{ - "keyTag-1": "valueTag-1", - "keyTag-2": "valueTag-2", - }, - MetadataOptions: &v1beta1.MetadataOptions{ - HTTPEndpoint: aws.String("test-metadata-1"), - }, - BlockDeviceMappings: []*v1beta1.BlockDeviceMapping{ - { - DeviceName: aws.String("map-device-1"), - }, - { - DeviceName: aws.String("map-device-2"), - }, - }, - UserData: aws.String("userdata-test-1"), - DetailedMonitoring: aws.Bool(false), - }, - }) - }) - DescribeTable("should change hash when static fields are updated", func(changes v1beta1.EC2NodeClass) { - hash := nodeClass.Hash() - Expect(mergo.Merge(nodeClass, changes, mergo.WithOverride)).To(Succeed()) - updatedHash := nodeClass.Hash() - Expect(hash).ToNot(Equal(updatedHash)) - }, - Entry("InstanceProfile Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{Role: "role-2"}}), - Entry("UserData Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{UserData: aws.String("userdata-test-2")}}), - Entry("Tags Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{Tags: map[string]string{"keyTag-test-3": "valueTag-test-3"}}}), - Entry("MetadataOptions Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{MetadataOptions: &v1beta1.MetadataOptions{HTTPEndpoint: aws.String("test-metadata-2")}}}), - Entry("BlockDeviceMappings Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{BlockDeviceMappings: []*v1beta1.BlockDeviceMapping{{DeviceName: aws.String("map-device-test-3")}}}}), - Entry("Context Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{Context: aws.String("context-2")}}), - Entry("DetailedMonitoring Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{DetailedMonitoring: aws.Bool(true)}}), - Entry("AMIFamily Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{AMIFamily: aws.String(v1alpha1.AMIFamilyBottlerocket)}}), - ) - DescribeTable("should not change hash when slices are re-ordered", func(changes v1beta1.EC2NodeClass) { - hash := nodeClass.Hash() - Expect(mergo.Merge(nodeClass, changes, mergo.WithOverride)).To(Succeed()) - updatedHash := nodeClass.Hash() - Expect(hash).To(Equal(updatedHash)) - }, - Entry("Reorder Tags", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{Tags: map[string]string{"keyTag-2": "valueTag-2", "keyTag-1": "valueTag-1"}}}), - Entry("Reorder BlockDeviceMapping", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{BlockDeviceMappings: []*v1beta1.BlockDeviceMapping{{DeviceName: aws.String("map-device-2")}, {DeviceName: aws.String("map-device-1")}}}}), - ) - It("should not change hash when behavior/dynamic fields are updated", func() { - hash := nodeClass.Hash() - - // Update a behavior/dynamic field - nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ - { - Tags: map[string]string{"subnet-test-key": "subnet-test-value"}, - }, - } - nodeClass.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ - { - Tags: map[string]string{"sg-test-key": "sg-test-value"}, - }, - } - nodeClass.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ - { - Tags: map[string]string{"ami-test-key": "ami-test-value"}, - }, - } - updatedHash := nodeClass.Hash() - Expect(hash).To(Equal(updatedHash)) - }) - It("should expect two provisioner with the same spec to have the same provisioner hash", func() { - otherNodeClass := test.EC2NodeClass(v1beta1.EC2NodeClass{ - Spec: nodeClass.Spec, - }) - Expect(nodeClass.Hash()).To(Equal(otherNodeClass.Hash())) - }) - }) - Context("BlockDeviceMappings", func() { - It("should fail if more than one root volume is specified", func() { - nodeClass := test.EC2NodeClass(v1beta1.EC2NodeClass{ - Spec: v1beta1.EC2NodeClassSpec{ - BlockDeviceMappings: []*v1beta1.BlockDeviceMapping{ - { - DeviceName: aws.String("map-device-1"), - EBS: &v1beta1.BlockDevice{ - VolumeSize: resource.NewScaledQuantity(50, resource.Giga), - }, - - RootVolume: true, - }, - { - DeviceName: aws.String("map-device-2"), - EBS: &v1beta1.BlockDevice{ - VolumeSize: resource.NewScaledQuantity(50, resource.Giga), - }, +var _ = BeforeSuite(func() { + env = coretest.NewEnvironment(scheme.Scheme, coretest.WithCRDs(apis.CRDs...)) + awsEnv = test.NewEnvironment(ctx, env) +}) - RootVolume: true, - }, - }, - }, - }) - Expect(nodeClass.Validate(ctx)).To(Not(Succeed())) - }) - }) - Context("Role Immutability", func() { - It("should fail when updating the role", func() { - nc.Spec.Role = "test-role" - Expect(nc.Validate(ctx)).To(Succeed()) +var _ = AfterEach(func() { + ExpectCleanedUp(ctx, env.Client) +}) - updateCtx := apis.WithinUpdate(ctx, nc.DeepCopy()) - nc.Spec.Role = "test-role2" - Expect(nc.Validate(updateCtx)).ToNot(Succeed()) - }) - }) +var _ = AfterSuite(func() { + Expect(env.Stop()).To(Succeed(), "Failed to stop environment") }) diff --git a/pkg/cloudprovider/nodeclaim_test.go b/pkg/cloudprovider/nodeclaim_test.go index 22f0dcee5af8..99b0851b4ca1 100644 --- a/pkg/cloudprovider/nodeclaim_test.go +++ b/pkg/cloudprovider/nodeclaim_test.go @@ -321,10 +321,9 @@ var _ = Describe("NodeClaim/CloudProvider", func() { Expect(err).NotTo(HaveOccurred()) Expect(isDrifted).To(Equal(cloudprovider.NodeClassDrift)) }, - Entry("InstanceProfile Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{Role: "role-2"}}), Entry("UserData Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{UserData: aws.String("userdata-test-2")}}), Entry("Tags Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{Tags: map[string]string{"keyTag-test-3": "valueTag-test-3"}}}), - Entry("MetadataOptions Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{MetadataOptions: &v1beta1.MetadataOptions{HTTPEndpoint: aws.String("test-metadata-2")}}}), + Entry("MetadataOptions Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{MetadataOptions: &v1beta1.MetadataOptions{HTTPEndpoint: aws.String("disabled")}}}), Entry("BlockDeviceMappings Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{BlockDeviceMappings: []*v1beta1.BlockDeviceMapping{{DeviceName: aws.String("map-device-test-3")}}}}), Entry("Context Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{Context: aws.String("context-2")}}), Entry("DetailedMonitoring Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{DetailedMonitoring: aws.Bool(true)}}), @@ -345,7 +344,7 @@ var _ = Describe("NodeClaim/CloudProvider", func() { Expect(err).NotTo(HaveOccurred()) Expect(isDrifted).To(BeEmpty()) }, - Entry("AMI Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{AMISelectorTerms: []v1beta1.AMISelectorTerm{{ID: validAMI}}}}), + Entry("AMI Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{AMISelectorTerms: []v1beta1.AMISelectorTerm{{Tags: map[string]string{"*": "*"}}}}}), Entry("Subnet Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{SubnetSelectorTerms: []v1beta1.SubnetSelectorTerm{{ID: "subnet-test1"}}}}), Entry("SecurityGroup Drift", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{SecurityGroupSelectorTerms: []v1beta1.SecurityGroupSelectorTerm{{Tags: map[string]string{"sg-key": "sg-value"}}}}}), ) diff --git a/pkg/controllers/nodeclass/nodeclass_test.go b/pkg/controllers/nodeclass/nodeclass_test.go index 4ba7f2217753..33fc3f44d2ab 100644 --- a/pkg/controllers/nodeclass/nodeclass_test.go +++ b/pkg/controllers/nodeclass/nodeclass_test.go @@ -714,11 +714,10 @@ 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: "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)}}), - Entry("MetadataOptions Drift", &v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{MetadataOptions: &v1beta1.MetadataOptions{HTTPEndpoint: aws.String("test-metadata-2")}}}), + Entry("MetadataOptions Drift", &v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{MetadataOptions: &v1beta1.MetadataOptions{HTTPEndpoint: aws.String("disabled")}}}), Entry("Context Drift", &v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{Context: aws.String("context-2")}}), ) It("should not update the static drift hash when dynamic field is updated", func() { diff --git a/pkg/providers/launchtemplate/nodeclass_test.go b/pkg/providers/launchtemplate/nodeclass_test.go index 0d1f2281aada..7392e6af47a2 100644 --- a/pkg/providers/launchtemplate/nodeclass_test.go +++ b/pkg/providers/launchtemplate/nodeclass_test.go @@ -877,6 +877,11 @@ var _ = Describe("EC2NodeClass/LaunchTemplates", func() { "nodefs.available": "15%", "nodefs.inodesFree": "5%", }, + EvictionSoftGracePeriod: map[string]metav1.Duration{ + "memory.available": {Duration: time.Minute}, + "nodefs.available": {Duration: time.Second * 180}, + "nodefs.inodesFree": {Duration: time.Minute * 5}, + }, } ExpectApplied(ctx, env.Client, nodePool, nodeClass) pod := coretest.UnschedulablePod() @@ -904,6 +909,11 @@ var _ = Describe("EC2NodeClass/LaunchTemplates", func() { "nodefs.available": {Duration: time.Second * 180}, "nodefs.inodesFree": {Duration: time.Minute * 5}, }, + EvictionSoft: map[string]string{ + "memory.available": "10%", + "nodefs.available": "15%", + "nodefs.inodesFree": "5%", + }, } ExpectApplied(ctx, env.Client, nodePool, nodeClass) pod := coretest.UnschedulablePod()