From 9735883946cb549afee5fa0e4a29b1384031ceef Mon Sep 17 00:00:00 2001 From: Amanuel Engeda <74629455+engedaam@users.noreply.github.com> Date: Tue, 5 Dec 2023 20:35:26 -0600 Subject: [PATCH] chore: Convert CDL values on tags for SelectorTerms (#5237) --- pkg/utils/nodeclass/nodeclass.go | 67 +++++++--- pkg/utils/nodeclass/suite_test.go | 211 ++++++++++++++++++++++++++++-- 2 files changed, 248 insertions(+), 30 deletions(-) diff --git a/pkg/utils/nodeclass/nodeclass.go b/pkg/utils/nodeclass/nodeclass.go index 1f19ae490ba8..321ca4fc701a 100644 --- a/pkg/utils/nodeclass/nodeclass.go +++ b/pkg/utils/nodeclass/nodeclass.go @@ -68,21 +68,23 @@ func NewSubnetSelectorTerms(subnetSelector map[string]string) (terms []v1beta1.S } // Each of these slices needs to be pre-populated with the "0" element so that we can properly generate permutations ids := []string{""} - tags := map[string]string{} + tagSet := []map[string]string{make(map[string]string)} for k, v := range subnetSelector { switch k { case "aws-ids", "aws::ids": ids = lo.Map(strings.Split(v, ","), func(s string, _ int) string { return strings.Trim(s, " ") }) default: - tags[k] = v + tagSet = createSelectorTags(k, v, tagSet) } } // If there are some "special" keys used, we have to represent the old selector as multiple terms for _, id := range ids { - terms = append(terms, v1beta1.SubnetSelectorTerm{ - Tags: tags, - ID: id, - }) + for _, tag := range tagSet { + terms = append(terms, v1beta1.SubnetSelectorTerm{ + Tags: tag, + ID: id, + }) + } } return terms } @@ -93,21 +95,23 @@ func NewSecurityGroupSelectorTerms(securityGroupSelector map[string]string) (ter } // Each of these slices needs to be pre-populated with the "0" element so that we can properly generate permutations ids := []string{""} - tags := map[string]string{} + tagSet := []map[string]string{make(map[string]string)} for k, v := range securityGroupSelector { switch k { case "aws-ids", "aws::ids": ids = lo.Map(strings.Split(v, ","), func(s string, _ int) string { return strings.Trim(s, " ") }) default: - tags[k] = v + tagSet = createSelectorTags(k, v, tagSet) } } // If there are some "special" keys used, we have to represent the old selector as multiple terms for _, id := range ids { - terms = append(terms, v1beta1.SecurityGroupSelectorTerm{ - Tags: tags, - ID: id, - }) + for _, tag := range tagSet { + terms = append(terms, v1beta1.SecurityGroupSelectorTerm{ + Tags: tag, + ID: id, + }) + } } return terms } @@ -120,7 +124,7 @@ func NewAMISelectorTerms(amiSelector map[string]string) (terms []v1beta1.AMISele ids := []string{""} names := []string{""} owners := []string{""} - tags := map[string]string{} + tagSet := []map[string]string{make(map[string]string)} for k, v := range amiSelector { switch k { case "aws-ids", "aws::ids": @@ -130,19 +134,21 @@ func NewAMISelectorTerms(amiSelector map[string]string) (terms []v1beta1.AMISele case "aws::owners": owners = lo.Map(strings.Split(v, ","), func(s string, _ int) string { return strings.Trim(s, " ") }) default: - tags[k] = v + tagSet = createSelectorTags(k, v, tagSet) } } // If there are some "special" keys used, we have to represent the old selector as multiple terms for _, owner := range owners { for _, id := range ids { for _, name := range names { - terms = append(terms, v1beta1.AMISelectorTerm{ - Tags: tags, - ID: id, - Name: name, - Owner: owner, - }) + for _, tag := range tagSet { + terms = append(terms, v1beta1.AMISelectorTerm{ + Tags: tag, + ID: id, + Name: name, + Owner: owner, + }) + } } } } @@ -273,3 +279,24 @@ func HashAnnotation(nodeClass *v1beta1.EC2NodeClass) map[string]string { } return map[string]string{v1beta1.AnnotationNodeClassHash: nodeClass.Hash()} } + +func createSelectorTags(k string, v string, tagSet []map[string]string) (res []map[string]string) { + cdlValue := lo.Map(strings.Split(v, ","), func(s string, _ int) string { return strings.Trim(s, " ") }) + for _, val := range cdlValue { + for _, tag := range tagSet { + tempTag := deepCopyMap(tag) + tempTag[k] = val + res = append(res, tempTag) + } + } + + return res +} + +func deepCopyMap(m map[string]string) map[string]string { + result := map[string]string{} + for k, v := range m { + result[k] = v + } + return result +} diff --git a/pkg/utils/nodeclass/suite_test.go b/pkg/utils/nodeclass/suite_test.go index c5a2dffbfd94..332f577cc639 100644 --- a/pkg/utils/nodeclass/suite_test.go +++ b/pkg/utils/nodeclass/suite_test.go @@ -399,7 +399,7 @@ var _ = Describe("NodeClassUtils", func() { "aws::name": "ami-name1,ami-name2", "aws::owners": "self,amazon", "aws::ids": "ami-1234,ami-5678", - "custom-tag": "custom-value", + "custom-tag": "custom-value1,custom-value2", "custom-tag2": "custom-value2", } nodeClass := nodeclassutil.New(nodeTemplate) @@ -417,14 +417,23 @@ var _ = Describe("NodeClassUtils", func() { // Expect AMISelectorTerms to be exactly what we would expect from the filtering above // This should include all permutations of the filters that could be used by this selector mechanism - Expect(nodeClass.Spec.AMISelectorTerms).To(HaveLen(8)) + Expect(nodeClass.Spec.AMISelectorTerms).To(HaveLen(16)) Expect(nodeClass.Spec.AMISelectorTerms).To(ConsistOf( v1beta1.AMISelectorTerm{ Name: "ami-name1", Owner: "self", ID: "ami-1234", Tags: map[string]string{ - "custom-tag": "custom-value", + "custom-tag": "custom-value1", + "custom-tag2": "custom-value2", + }, + }, + v1beta1.AMISelectorTerm{ + Name: "ami-name1", + Owner: "self", + ID: "ami-1234", + Tags: map[string]string{ + "custom-tag": "custom-value2", "custom-tag2": "custom-value2", }, }, @@ -433,7 +442,16 @@ var _ = Describe("NodeClassUtils", func() { Owner: "self", ID: "ami-5678", Tags: map[string]string{ - "custom-tag": "custom-value", + "custom-tag": "custom-value1", + "custom-tag2": "custom-value2", + }, + }, + v1beta1.AMISelectorTerm{ + Name: "ami-name1", + Owner: "self", + ID: "ami-5678", + Tags: map[string]string{ + "custom-tag": "custom-value2", "custom-tag2": "custom-value2", }, }, @@ -442,7 +460,25 @@ var _ = Describe("NodeClassUtils", func() { Owner: "amazon", ID: "ami-1234", Tags: map[string]string{ - "custom-tag": "custom-value", + "custom-tag": "custom-value1", + "custom-tag2": "custom-value2", + }, + }, + v1beta1.AMISelectorTerm{ + Name: "ami-name1", + Owner: "amazon", + ID: "ami-1234", + Tags: map[string]string{ + "custom-tag": "custom-value2", + "custom-tag2": "custom-value2", + }, + }, + v1beta1.AMISelectorTerm{ + Name: "ami-name1", + Owner: "amazon", + ID: "ami-5678", + Tags: map[string]string{ + "custom-tag": "custom-value1", "custom-tag2": "custom-value2", }, }, @@ -451,7 +487,16 @@ var _ = Describe("NodeClassUtils", func() { Owner: "amazon", ID: "ami-5678", Tags: map[string]string{ - "custom-tag": "custom-value", + "custom-tag": "custom-value2", + "custom-tag2": "custom-value2", + }, + }, + v1beta1.AMISelectorTerm{ + Name: "ami-name2", + Owner: "self", + ID: "ami-1234", + Tags: map[string]string{ + "custom-tag": "custom-value1", "custom-tag2": "custom-value2", }, }, @@ -460,7 +505,7 @@ var _ = Describe("NodeClassUtils", func() { Owner: "self", ID: "ami-1234", Tags: map[string]string{ - "custom-tag": "custom-value", + "custom-tag": "custom-value2", "custom-tag2": "custom-value2", }, }, @@ -469,7 +514,25 @@ var _ = Describe("NodeClassUtils", func() { Owner: "self", ID: "ami-5678", Tags: map[string]string{ - "custom-tag": "custom-value", + "custom-tag": "custom-value1", + "custom-tag2": "custom-value2", + }, + }, + v1beta1.AMISelectorTerm{ + Name: "ami-name2", + Owner: "self", + ID: "ami-5678", + Tags: map[string]string{ + "custom-tag": "custom-value2", + "custom-tag2": "custom-value2", + }, + }, + v1beta1.AMISelectorTerm{ + Name: "ami-name2", + Owner: "amazon", + ID: "ami-1234", + Tags: map[string]string{ + "custom-tag": "custom-value1", "custom-tag2": "custom-value2", }, }, @@ -478,7 +541,7 @@ var _ = Describe("NodeClassUtils", func() { Owner: "amazon", ID: "ami-1234", Tags: map[string]string{ - "custom-tag": "custom-value", + "custom-tag": "custom-value2", "custom-tag2": "custom-value2", }, }, @@ -487,10 +550,138 @@ var _ = Describe("NodeClassUtils", func() { Owner: "amazon", ID: "ami-5678", Tags: map[string]string{ - "custom-tag": "custom-value", + "custom-tag": "custom-value1", "custom-tag2": "custom-value2", }, }, + v1beta1.AMISelectorTerm{ + Name: "ami-name2", + Owner: "amazon", + ID: "ami-5678", + Tags: map[string]string{ + "custom-tag": "custom-value2", + "custom-tag2": "custom-value2", + }, + }, + )) + + Expect(nodeClass.Spec.AMIFamily).To(Equal(nodeTemplate.Spec.AMIFamily)) + Expect(nodeClass.Spec.UserData).To(Equal(nodeTemplate.Spec.UserData)) + 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)) + ExpectMetadataOptionsEqual(nodeTemplate.Spec.MetadataOptions, nodeClass.Spec.MetadataOptions) + Expect(nodeClass.Spec.Context).To(Equal(nodeTemplate.Spec.Context)) + Expect(nodeClass.Spec.LaunchTemplateName).To(Equal(nodeTemplate.Spec.LaunchTemplateName)) + Expect(nodeClass.Spec.InstanceProfile).To(Equal(nodeTemplate.Spec.InstanceProfile)) + + ExpectSubnetStatusEqual(nodeTemplate.Status.Subnets, nodeClass.Status.Subnets) + ExpectSecurityGroupStatusEqual(nodeTemplate.Status.SecurityGroups, nodeClass.Status.SecurityGroups) + ExpectAMIStatusEqual(nodeTemplate.Status.AMIs, nodeClass.Status.AMIs) + }) + It("should convert a AWSNodeTemplate to a EC2NodeClass (with comma delineated list on SubnetSelector)", func() { + nodeTemplate.Spec.SubnetSelector = map[string]string{ + "test-key-1": "test-value-1,test-value-2,test-value-3", + "test-key-2": "test-value-1,test-value-2", + } + + nodeClass := nodeclassutil.New(nodeTemplate) + + for k, v := range nodeTemplate.Annotations { + Expect(nodeClass.Annotations).To(HaveKeyWithValue(k, v)) + } + for k, v := range nodeTemplate.Labels { + Expect(nodeClass.Labels).To(HaveKeyWithValue(k, v)) + } + Expect(nodeClass.Spec.SubnetSelectorTerms).To(HaveLen(6)) + Expect(nodeClass.Spec.SubnetSelectorTerms).To(ConsistOf( + v1beta1.SubnetSelectorTerm{ + Tags: map[string]string{ + "test-key-1": "test-value-1", + "test-key-2": "test-value-1", + }, + }, + v1beta1.SubnetSelectorTerm{ + Tags: map[string]string{ + "test-key-1": "test-value-2", + "test-key-2": "test-value-1", + }, + }, + v1beta1.SubnetSelectorTerm{ + Tags: map[string]string{ + "test-key-1": "test-value-3", + "test-key-2": "test-value-1", + }, + }, + v1beta1.SubnetSelectorTerm{ + Tags: map[string]string{ + "test-key-1": "test-value-1", + "test-key-2": "test-value-2", + }, + }, + v1beta1.SubnetSelectorTerm{ + Tags: map[string]string{ + "test-key-1": "test-value-2", + "test-key-2": "test-value-2", + }, + }, + v1beta1.SubnetSelectorTerm{ + Tags: map[string]string{ + "test-key-1": "test-value-3", + "test-key-2": "test-value-2", + }, + }, + )) + + Expect(nodeClass.Spec.SecurityGroupSelectorTerms).To(HaveLen(1)) + Expect(nodeClass.Spec.SecurityGroupSelectorTerms[0].Tags).To(Equal(nodeTemplate.Spec.SecurityGroupSelector)) + + Expect(nodeClass.Spec.AMIFamily).To(Equal(nodeTemplate.Spec.AMIFamily)) + Expect(nodeClass.Spec.UserData).To(Equal(nodeTemplate.Spec.UserData)) + 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)) + ExpectMetadataOptionsEqual(nodeTemplate.Spec.MetadataOptions, nodeClass.Spec.MetadataOptions) + Expect(nodeClass.Spec.Context).To(Equal(nodeTemplate.Spec.Context)) + Expect(nodeClass.Spec.LaunchTemplateName).To(Equal(nodeTemplate.Spec.LaunchTemplateName)) + Expect(nodeClass.Spec.InstanceProfile).To(Equal(nodeTemplate.Spec.InstanceProfile)) + + ExpectSubnetStatusEqual(nodeTemplate.Status.Subnets, nodeClass.Status.Subnets) + ExpectSecurityGroupStatusEqual(nodeTemplate.Status.SecurityGroups, nodeClass.Status.SecurityGroups) + ExpectAMIStatusEqual(nodeTemplate.Status.AMIs, nodeClass.Status.AMIs) + }) + It("should convert a AWSNodeTemplate to a EC2NodeClass (with comma delineated list on SecurityGroupSelector)", func() { + nodeTemplate.Spec.SecurityGroupSelector = map[string]string{ + "test-key-1": "test-value-1,test-value-2", + "test-key-2": "test-value-1", + } + + nodeClass := nodeclassutil.New(nodeTemplate) + + for k, v := range nodeTemplate.Annotations { + Expect(nodeClass.Annotations).To(HaveKeyWithValue(k, v)) + } + for k, v := range nodeTemplate.Labels { + Expect(nodeClass.Labels).To(HaveKeyWithValue(k, v)) + } + Expect(nodeClass.Spec.SubnetSelectorTerms).To(HaveLen(1)) + Expect(nodeClass.Spec.SubnetSelectorTerms[0].Tags).To(Equal(nodeTemplate.Spec.SubnetSelector)) + Expect(nodeClass.Spec.SecurityGroupSelectorTerms).To(HaveLen(2)) + Expect(nodeClass.Spec.SecurityGroupSelectorTerms).To(ConsistOf( + v1beta1.SecurityGroupSelectorTerm{ + Tags: map[string]string{ + "test-key-1": "test-value-1", + "test-key-2": "test-value-1", + }, + }, + v1beta1.SecurityGroupSelectorTerm{ + Tags: map[string]string{ + "test-key-1": "test-value-2", + "test-key-2": "test-value-1", + }, + }, )) Expect(nodeClass.Spec.AMIFamily).To(Equal(nodeTemplate.Spec.AMIFamily))