From 7e34e786cec46874c33edff55ee3699a6e551dad Mon Sep 17 00:00:00 2001 From: Peter Rifel Date: Tue, 7 Jul 2020 21:39:49 -0500 Subject: [PATCH 1/4] cloudmock - use ResourceType constants that are now in aws-sdk-go --- cloudmock/aws/mockec2/natgateway.go | 4 ++-- cloudmock/aws/mockec2/tags.go | 10 ++-------- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/cloudmock/aws/mockec2/natgateway.go b/cloudmock/aws/mockec2/natgateway.go index 82fe5507a2e9a..d9ffddad9ab89 100644 --- a/cloudmock/aws/mockec2/natgateway.go +++ b/cloudmock/aws/mockec2/natgateway.go @@ -128,7 +128,7 @@ func (m *MockEC2) DescribeNatGateways(request *ec2.DescribeNatGatewaysInput) (*e } default: if strings.HasPrefix(*filter.Name, "tag:") { - match = m.hasTag(ResourceTypeNatGateway, *ngw.NatGatewayId, filter) + match = m.hasTag(ec2.ResourceTypeNatgateway, *ngw.NatGatewayId, filter) } else { return nil, fmt.Errorf("unknown filter name: %q", *filter.Name) } @@ -145,7 +145,7 @@ func (m *MockEC2) DescribeNatGateways(request *ec2.DescribeNatGatewaysInput) (*e } copy := *ngw - copy.Tags = m.getTags(ResourceTypeNatGateway, id) + copy.Tags = m.getTags(ec2.ResourceTypeNatgateway, id) ngws = append(ngws, ©) } diff --git a/cloudmock/aws/mockec2/tags.go b/cloudmock/aws/mockec2/tags.go index f0f3a22f039f7..294e9a9b82471 100644 --- a/cloudmock/aws/mockec2/tags.go +++ b/cloudmock/aws/mockec2/tags.go @@ -27,12 +27,6 @@ import ( "k8s.io/klog" ) -const ( - // Not (yet?) in aws-sdk-go - ResourceTypeNatGateway = "nat-gateway" - ResourceTypeAddress = "elastic-ip" -) - func (m *MockEC2) CreateTagsRequest(*ec2.CreateTagsInput) (*request.Request, *ec2.CreateTagsOutput) { panic("Not implemented") } @@ -70,13 +64,13 @@ func (m *MockEC2) addTag(resourceId string, tag *ec2.Tag) { } else if strings.HasPrefix(resourceId, "igw-") { resourceType = ec2.ResourceTypeInternetGateway } else if strings.HasPrefix(resourceId, "nat-") { - resourceType = ResourceTypeNatGateway + resourceType = ec2.ResourceTypeNatgateway } else if strings.HasPrefix(resourceId, "dopt-") { resourceType = ec2.ResourceTypeDhcpOptions } else if strings.HasPrefix(resourceId, "rtb-") { resourceType = ec2.ResourceTypeRouteTable } else if strings.HasPrefix(resourceId, "eipalloc-") { - resourceType = ResourceTypeAddress + resourceType = ec2.ResourceTypeElasticIp } else { klog.Fatalf("Unknown resource-type in create tags: %v", resourceId) } From 844626ae44b2f2d239a0e1c9792d61f6e1be8a82 Mon Sep 17 00:00:00 2001 From: Peter Rifel Date: Tue, 7 Jul 2020 21:40:18 -0500 Subject: [PATCH 2/4] cloudmock - recognize and store tags provided in Create requests --- cloudmock/aws/mockec2/dhcpoptions.go | 5 +++- cloudmock/aws/mockec2/internetgateways.go | 4 +++ cloudmock/aws/mockec2/natgateway.go | 5 ++++ cloudmock/aws/mockec2/routetable.go | 7 +++-- cloudmock/aws/mockec2/securitygroups.go | 7 ++++- cloudmock/aws/mockec2/subnets.go | 2 ++ cloudmock/aws/mockec2/tags.go | 32 +++++++++++++++-------- cloudmock/aws/mockec2/volumes.go | 11 +++----- cloudmock/aws/mockec2/vpcs.go | 5 ++++ 9 files changed, 54 insertions(+), 24 deletions(-) diff --git a/cloudmock/aws/mockec2/dhcpoptions.go b/cloudmock/aws/mockec2/dhcpoptions.go index 42ed71e3a29f3..c3924ccf48f85 100644 --- a/cloudmock/aws/mockec2/dhcpoptions.go +++ b/cloudmock/aws/mockec2/dhcpoptions.go @@ -126,9 +126,10 @@ func (m *MockEC2) CreateDhcpOptions(request *ec2.CreateDhcpOptionsInput) (*ec2.C } n := len(m.DhcpOptions) + 1 + id := fmt.Sprintf("dopt-%d", n) dhcpOptions := &ec2.DhcpOptions{ - DhcpOptionsId: s(fmt.Sprintf("dopt-%d", n)), + DhcpOptionsId: s(id), } for _, o := range request.DhcpConfigurations { @@ -147,6 +148,8 @@ func (m *MockEC2) CreateDhcpOptions(request *ec2.CreateDhcpOptionsInput) (*ec2.C } m.DhcpOptions[*dhcpOptions.DhcpOptionsId] = dhcpOptions + m.addTags(id, tagSpecificationsToTags(request.TagSpecifications, ec2.ResourceTypeDhcpOptions)...) + copy := *dhcpOptions copy.Tags = m.getTags(ec2.ResourceTypeDhcpOptions, *dhcpOptions.DhcpOptionsId) return &ec2.CreateDhcpOptionsOutput{DhcpOptions: ©}, nil diff --git a/cloudmock/aws/mockec2/internetgateways.go b/cloudmock/aws/mockec2/internetgateways.go index dfefd2f7c44f2..930e2682414f4 100644 --- a/cloudmock/aws/mockec2/internetgateways.go +++ b/cloudmock/aws/mockec2/internetgateways.go @@ -66,9 +66,11 @@ func (m *MockEC2) CreateInternetGateway(request *ec2.CreateInternetGatewayInput) klog.Infof("CreateInternetGateway: %v", request) id := m.allocateId("igw") + tags := tagSpecificationsToTags(request.TagSpecifications, ec2.ResourceTypeInternetGateway) igw := &ec2.InternetGateway{ InternetGatewayId: s(id), + Tags: tags, } if m.InternetGateways == nil { @@ -76,6 +78,8 @@ func (m *MockEC2) CreateInternetGateway(request *ec2.CreateInternetGatewayInput) } m.InternetGateways[id] = igw + m.addTags(id, tags...) + response := &ec2.CreateInternetGatewayOutput{ InternetGateway: igw, } diff --git a/cloudmock/aws/mockec2/natgateway.go b/cloudmock/aws/mockec2/natgateway.go index d9ffddad9ab89..2bbdc63345c8e 100644 --- a/cloudmock/aws/mockec2/natgateway.go +++ b/cloudmock/aws/mockec2/natgateway.go @@ -30,9 +30,12 @@ func (m *MockEC2) CreateNatGatewayWithId(request *ec2.CreateNatGatewayInput, id m.mutex.Lock() defer m.mutex.Unlock() + tags := tagSpecificationsToTags(request.TagSpecifications, ec2.ResourceTypeNatgateway) + ngw := &ec2.NatGateway{ NatGatewayId: s(id), SubnetId: request.SubnetId, + Tags: tags, } if request.AllocationId != nil { @@ -57,6 +60,8 @@ func (m *MockEC2) CreateNatGatewayWithId(request *ec2.CreateNatGatewayInput, id } m.NatGateways[*ngw.NatGatewayId] = ngw + m.addTags(id, tags...) + copy := *ngw return &ec2.CreateNatGatewayOutput{ diff --git a/cloudmock/aws/mockec2/routetable.go b/cloudmock/aws/mockec2/routetable.go index 81b3e5175ffcd..907a3e956d018 100644 --- a/cloudmock/aws/mockec2/routetable.go +++ b/cloudmock/aws/mockec2/routetable.go @@ -32,10 +32,9 @@ func (m *MockEC2) AddRouteTable(rt *ec2.RouteTable) { if m.RouteTables == nil { m.RouteTables = make(map[string]*ec2.RouteTable) } - for _, tag := range rt.Tags { - m.addTag(*rt.RouteTableId, tag) - } - rt.Tags = nil + + m.addTags(*rt.RouteTableId, rt.Tags...) + m.RouteTables[*rt.RouteTableId] = rt } diff --git a/cloudmock/aws/mockec2/securitygroups.go b/cloudmock/aws/mockec2/securitygroups.go index 4fde24acd1fa1..d5b9134928570 100644 --- a/cloudmock/aws/mockec2/securitygroups.go +++ b/cloudmock/aws/mockec2/securitygroups.go @@ -41,18 +41,23 @@ func (m *MockEC2) CreateSecurityGroup(request *ec2.CreateSecurityGroupInput) (*e m.securityGroupNumber++ n := m.securityGroupNumber + id := fmt.Sprintf("sg-%d", n) + tags := tagSpecificationsToTags(request.TagSpecifications, ec2.ResourceTypeSecurityGroup) sg := &ec2.SecurityGroup{ GroupName: request.GroupName, - GroupId: s(fmt.Sprintf("sg-%d", n)), + GroupId: s(id), VpcId: request.VpcId, Description: request.Description, + Tags: tags, } if m.SecurityGroups == nil { m.SecurityGroups = make(map[string]*ec2.SecurityGroup) } m.SecurityGroups[*sg.GroupId] = sg + m.addTags(id, tags...) + response := &ec2.CreateSecurityGroupOutput{ GroupId: sg.GroupId, } diff --git a/cloudmock/aws/mockec2/subnets.go b/cloudmock/aws/mockec2/subnets.go index 9935a9f31f0bf..cfc3b0c35c6e9 100644 --- a/cloudmock/aws/mockec2/subnets.go +++ b/cloudmock/aws/mockec2/subnets.go @@ -81,6 +81,8 @@ func (m *MockEC2) CreateSubnetWithId(request *ec2.CreateSubnetInput, id string) main: *subnet, } + m.addTags(id, tagSpecificationsToTags(request.TagSpecifications, ec2.ResourceTypeSubnet)...) + response := &ec2.CreateSubnetOutput{ Subnet: subnet, } diff --git a/cloudmock/aws/mockec2/tags.go b/cloudmock/aws/mockec2/tags.go index 294e9a9b82471..269f18c829356 100644 --- a/cloudmock/aws/mockec2/tags.go +++ b/cloudmock/aws/mockec2/tags.go @@ -43,15 +43,13 @@ func (m *MockEC2) CreateTags(request *ec2.CreateTagsInput) (*ec2.CreateTagsOutpu for _, v := range request.Resources { resourceId := *v - for _, tag := range request.Tags { - m.addTag(resourceId, tag) - } + m.addTags(resourceId, request.Tags...) } response := &ec2.CreateTagsOutput{} return response, nil } -func (m *MockEC2) addTag(resourceId string, tag *ec2.Tag) { +func (m *MockEC2) addTags(resourceId string, tags ...*ec2.Tag) { resourceType := "" if strings.HasPrefix(resourceId, "subnet-") { resourceType = ec2.ResourceTypeSubnet @@ -74,14 +72,15 @@ func (m *MockEC2) addTag(resourceId string, tag *ec2.Tag) { } else { klog.Fatalf("Unknown resource-type in create tags: %v", resourceId) } - - t := &ec2.TagDescription{ - Key: tag.Key, - Value: tag.Value, - ResourceId: s(resourceId), - ResourceType: s(resourceType), + for _, tag := range tags { + t := &ec2.TagDescription{ + Key: tag.Key, + Value: tag.Value, + ResourceId: s(resourceId), + ResourceType: s(resourceType), + } + m.Tags = append(m.Tags, t) } - m.Tags = append(m.Tags, t) } func (m *MockEC2) DescribeTagsRequest(*ec2.DescribeTagsInput) (*request.Request, *ec2.DescribeTagsOutput) { @@ -221,3 +220,14 @@ func SortTags(tags []*ec2.Tag) { } sort.SliceStable(tags, func(i, j int) bool { return keys[i] < keys[j] }) } + +func tagSpecificationsToTags(specifications []*ec2.TagSpecification, resourceType string) []*ec2.Tag { + tags := make([]*ec2.Tag, 0) + for _, specification := range specifications { + if aws.StringValue(specification.ResourceType) != resourceType { + continue + } + tags = append(tags, specification.Tags...) + } + return tags +} diff --git a/cloudmock/aws/mockec2/volumes.go b/cloudmock/aws/mockec2/volumes.go index 0b4c2347eb578..a8b6046848d6b 100644 --- a/cloudmock/aws/mockec2/volumes.go +++ b/cloudmock/aws/mockec2/volumes.go @@ -37,9 +37,10 @@ func (m *MockEC2) CreateVolume(request *ec2.CreateVolumeInput) (*ec2.Volume, err } n := len(m.Volumes) + 1 + id := fmt.Sprintf("vol-%d", n) volume := &ec2.Volume{ - VolumeId: s(fmt.Sprintf("vol-%d", n)), + VolumeId: s(id), AvailabilityZone: request.AvailabilityZone, Encrypted: request.Encrypted, Iops: request.Iops, @@ -49,19 +50,15 @@ func (m *MockEC2) CreateVolume(request *ec2.CreateVolumeInput) (*ec2.Volume, err VolumeType: request.VolumeType, } - for _, tags := range request.TagSpecifications { - for _, tag := range tags.Tags { - m.addTag(*volume.VolumeId, tag) - } - } if m.Volumes == nil { m.Volumes = make(map[string]*ec2.Volume) } m.Volumes[*volume.VolumeId] = volume + m.addTags(id, tagSpecificationsToTags(request.TagSpecifications, ec2.ResourceTypeVolume)...) + copy := *volume copy.Tags = m.getTags(ec2.ResourceTypeVolume, *volume.VolumeId) - // TODO: a few fields // // Information about the volume attachments. // Attachments []*VolumeAttachment `locationName:"attachmentSet" locationNameList:"item" type:"list"` diff --git a/cloudmock/aws/mockec2/vpcs.go b/cloudmock/aws/mockec2/vpcs.go index e522d05bf34bf..a8c8e0d3f418a 100644 --- a/cloudmock/aws/mockec2/vpcs.go +++ b/cloudmock/aws/mockec2/vpcs.go @@ -55,11 +55,14 @@ func (m *MockEC2) CreateVpcWithId(request *ec2.CreateVpcInput, id string) (*ec2. m.mutex.Lock() defer m.mutex.Unlock() + tags := tagSpecificationsToTags(request.TagSpecifications, ec2.ResourceTypeVpc) + vpc := &vpcInfo{ main: ec2.Vpc{ VpcId: s(id), CidrBlock: request.CidrBlock, IsDefault: aws.Bool(false), + Tags: tags, }, attributes: ec2.DescribeVpcAttributeOutput{ EnableDnsHostnames: &ec2.AttributeBooleanValue{Value: aws.Bool(true)}, @@ -72,6 +75,8 @@ func (m *MockEC2) CreateVpcWithId(request *ec2.CreateVpcInput, id string) (*ec2. } m.Vpcs[id] = vpc + m.addTags(id, tags...) + response := &ec2.CreateVpcOutput{ Vpc: &vpc.main, } From fc35f9d59e9e508395f98809ac998ffbdd62efe2 Mon Sep 17 00:00:00 2001 From: Peter Rifel Date: Tue, 7 Jul 2020 22:34:12 -0500 Subject: [PATCH 3/4] fix unit tests --- pkg/resources/aws/aws_test.go | 1 + upup/pkg/fi/cloudup/awstasks/securitygroup_test.go | 1 + 2 files changed, 2 insertions(+) diff --git a/pkg/resources/aws/aws_test.go b/pkg/resources/aws/aws_test.go index 4d5db8f13cad2..9baac02df424f 100644 --- a/pkg/resources/aws/aws_test.go +++ b/pkg/resources/aws/aws_test.go @@ -155,6 +155,7 @@ func TestSharedVolume(t *testing.T) { sharedVolume, err := c.CreateVolume(&ec2.CreateVolumeInput{ TagSpecifications: []*ec2.TagSpecification{ { + ResourceType: aws.String(ec2.ResourceTypeVolume), Tags: []*ec2.Tag{ { Key: aws.String(ownershipTagKey), diff --git a/upup/pkg/fi/cloudup/awstasks/securitygroup_test.go b/upup/pkg/fi/cloudup/awstasks/securitygroup_test.go index ef4f0efbe67f1..047b2d9b3361d 100644 --- a/upup/pkg/fi/cloudup/awstasks/securitygroup_test.go +++ b/upup/pkg/fi/cloudup/awstasks/securitygroup_test.go @@ -149,6 +149,7 @@ func TestSecurityGroupCreate(t *testing.T) { Description: s("Description"), GroupId: sg1.ID, VpcId: vpc1.ID, + Tags: []*ec2.Tag{}, GroupName: s("sg1"), } actual := c.SecurityGroups[*sg1.ID] From b1e7704d09aa6d22ccbfc698b4534ac4409e9fc1 Mon Sep 17 00:00:00 2001 From: Peter Rifel Date: Tue, 7 Jul 2020 23:32:17 -0500 Subject: [PATCH 4/4] Use "lt-" IDs for launch templates and add tagging support --- cloudmock/aws/mockec2/api.go | 6 ++- cloudmock/aws/mockec2/launch_templates.go | 53 +++++++++++++++-------- cloudmock/aws/mockec2/tags.go | 2 + 3 files changed, 42 insertions(+), 19 deletions(-) diff --git a/cloudmock/aws/mockec2/api.go b/cloudmock/aws/mockec2/api.go index 878f591ee72d5..7fc6c0e586b96 100644 --- a/cloudmock/aws/mockec2/api.go +++ b/cloudmock/aws/mockec2/api.go @@ -55,7 +55,8 @@ type MockEC2 struct { InternetGateways map[string]*ec2.InternetGateway - LaunchTemplates map[string]*ec2.ResponseLaunchTemplateData + launchTemplateNumber int + LaunchTemplates map[string]*launchTemplateInfo NatGateways map[string]*ec2.NatGateway @@ -98,6 +99,9 @@ func (m *MockEC2) All() map[string]interface{} { for id, o := range m.InternetGateways { all[id] = o } + for id, o := range m.LaunchTemplates { + all[id] = o + } for id, o := range m.NatGateways { all[id] = o } diff --git a/cloudmock/aws/mockec2/launch_templates.go b/cloudmock/aws/mockec2/launch_templates.go index ee2ac09fe5dd7..16c651c0fa89d 100644 --- a/cloudmock/aws/mockec2/launch_templates.go +++ b/cloudmock/aws/mockec2/launch_templates.go @@ -23,6 +23,11 @@ import ( "github.com/aws/aws-sdk-go/service/ec2" ) +type launchTemplateInfo struct { + data *ec2.ResponseLaunchTemplateData + name *string +} + // DescribeLaunchTemplates mocks the describing the launch templates func (m *MockEC2) DescribeLaunchTemplates(request *ec2.DescribeLaunchTemplatesInput) (*ec2.DescribeLaunchTemplatesOutput, error) { m.mutex.Lock() @@ -34,9 +39,9 @@ func (m *MockEC2) DescribeLaunchTemplates(request *ec2.DescribeLaunchTemplatesIn return o, nil } - for name := range m.LaunchTemplates { + for _, ltInfo := range m.LaunchTemplates { o.LaunchTemplates = append(o.LaunchTemplates, &ec2.LaunchTemplate{ - LaunchTemplateName: aws.String(name), + LaunchTemplateName: ltInfo.name, }) } @@ -54,16 +59,17 @@ func (m *MockEC2) DescribeLaunchTemplateVersions(request *ec2.DescribeLaunchTemp return o, nil } - lt, found := m.LaunchTemplates[aws.StringValue(request.LaunchTemplateName)] - if !found { - return o, nil + for id, ltInfo := range m.LaunchTemplates { + if aws.StringValue(ltInfo.name) != aws.StringValue(request.LaunchTemplateName) { + continue + } + o.LaunchTemplateVersions = append(o.LaunchTemplateVersions, &ec2.LaunchTemplateVersion{ + DefaultVersion: aws.Bool(true), + LaunchTemplateId: aws.String(id), + LaunchTemplateData: ltInfo.data, + LaunchTemplateName: request.LaunchTemplateName, + }) } - o.LaunchTemplateVersions = append(o.LaunchTemplateVersions, &ec2.LaunchTemplateVersion{ - DefaultVersion: aws.Bool(true), - LaunchTemplateData: lt, - LaunchTemplateName: request.LaunchTemplateName, - }) - return o, nil } @@ -72,11 +78,15 @@ func (m *MockEC2) CreateLaunchTemplate(request *ec2.CreateLaunchTemplateInput) ( m.mutex.Lock() defer m.mutex.Unlock() + m.launchTemplateNumber++ + n := m.launchTemplateNumber + id := fmt.Sprintf("lt-%d", n) + if m.LaunchTemplates == nil { - m.LaunchTemplates = make(map[string]*ec2.ResponseLaunchTemplateData) + m.LaunchTemplates = make(map[string]*launchTemplateInfo) } - if m.LaunchTemplates[aws.StringValue(request.LaunchTemplateName)] != nil { - return nil, fmt.Errorf("duplicate LaunchTemplateName %s", aws.StringValue(request.LaunchTemplateName)) + if m.LaunchTemplates[id] != nil { + return nil, fmt.Errorf("duplicate LaunchTemplateName %s", id) } resp := &ec2.ResponseLaunchTemplateData{ DisableApiTermination: request.LaunchTemplateData.DisableApiTermination, @@ -88,9 +98,10 @@ func (m *MockEC2) CreateLaunchTemplate(request *ec2.CreateLaunchTemplateInput) ( SecurityGroups: request.LaunchTemplateData.SecurityGroups, UserData: request.LaunchTemplateData.UserData, } - m.LaunchTemplates[aws.StringValue(request.LaunchTemplateName)] = resp - - // @GOD DAMN AWS request vs response .. fu@@@@@ .. so much typing!!#@#@# + m.LaunchTemplates[id] = &launchTemplateInfo{ + data: resp, + name: request.LaunchTemplateName, + } if request.LaunchTemplateData.Monitoring != nil { resp.Monitoring = &ec2.LaunchTemplatesMonitoring{Enabled: request.LaunchTemplateData.Monitoring.Enabled} @@ -169,6 +180,8 @@ func (m *MockEC2) CreateLaunchTemplate(request *ec2.CreateLaunchTemplateInput) ( }) } } + m.addTags(id, tagSpecificationsToTags(request.TagSpecifications, ec2.ResourceTypeLaunchTemplate)...) + return &ec2.CreateLaunchTemplateOutput{}, nil } @@ -182,7 +195,11 @@ func (m *MockEC2) DeleteLaunchTemplate(request *ec2.DeleteLaunchTemplateInput) ( if m.LaunchTemplates == nil { return o, nil } - delete(m.LaunchTemplates, aws.StringValue(request.LaunchTemplateName)) + for id, lt := range m.LaunchTemplates { + if aws.StringValue(lt.name) == aws.StringValue(request.LaunchTemplateName) { + delete(m.LaunchTemplates, id) + } + } return o, nil } diff --git a/cloudmock/aws/mockec2/tags.go b/cloudmock/aws/mockec2/tags.go index 269f18c829356..5ad9f2c74c8cd 100644 --- a/cloudmock/aws/mockec2/tags.go +++ b/cloudmock/aws/mockec2/tags.go @@ -69,6 +69,8 @@ func (m *MockEC2) addTags(resourceId string, tags ...*ec2.Tag) { resourceType = ec2.ResourceTypeRouteTable } else if strings.HasPrefix(resourceId, "eipalloc-") { resourceType = ec2.ResourceTypeElasticIp + } else if strings.HasPrefix(resourceId, "lt-") { + resourceType = ec2.ResourceTypeLaunchTemplate } else { klog.Fatalf("Unknown resource-type in create tags: %v", resourceId) }