Merge pull request #9529 from rifelpet/ec2-tag-on-create

Use EC2's tag-on-create for various resources

upup/pkg/fi/cloudup/awstasks/dhcp_options.go

@@ -141,7 +141,9 @@ func (_ *DHCPOptions) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *DHCPOption
 	if a == nil {
 		klog.V(2).Infof("Creating DHCPOptions with Name:%q", *e.Name)
 
-		request := &ec2.CreateDhcpOptionsInput{}
+		request := &ec2.CreateDhcpOptionsInput{
+			TagSpecifications: awsup.EC2TagSpecification(ec2.ResourceTypeDhcpOptions, e.Tags),
+		}
 		if e.DomainNameServers != nil {
 			o := &ec2.NewDhcpConfiguration{
 				Key:    aws.String("domain-name-servers"),

upup/pkg/fi/cloudup/awstasks/ebsvolume.go

@@ -24,7 +24,6 @@ import (
 	"k8s.io/kops/upup/pkg/fi/cloudup/cloudformation"
 	"k8s.io/kops/upup/pkg/fi/cloudup/terraform"
 
-	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/ec2"
 	"k8s.io/klog"
 )
@@ -140,25 +139,13 @@ func (_ *EBSVolume) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *EBSVolume) e
 		klog.V(2).Infof("Creating PersistentVolume with Name:%q", *e.Name)
 
 		request := &ec2.CreateVolumeInput{
-			Size:             e.SizeGB,
-			AvailabilityZone: e.AvailabilityZone,
-			VolumeType:       e.VolumeType,
-			KmsKeyId:         e.KmsKeyId,
-			Encrypted:        e.Encrypted,
-			Iops:             e.VolumeIops,
-		}
-
-		if len(e.Tags) != 0 {
-			request.TagSpecifications = []*ec2.TagSpecification{
-				{ResourceType: aws.String(ec2.ResourceTypeVolume)},
-			}
-
-			for k, v := range e.Tags {
-				request.TagSpecifications[0].Tags = append(request.TagSpecifications[0].Tags, &ec2.Tag{
-					Key:   aws.String(k),
-					Value: aws.String(v),
-				})
-			}
+			Size:              e.SizeGB,
+			AvailabilityZone:  e.AvailabilityZone,
+			VolumeType:        e.VolumeType,
+			KmsKeyId:          e.KmsKeyId,
+			Encrypted:         e.Encrypted,
+			Iops:              e.VolumeIops,
+			TagSpecifications: awsup.EC2TagSpecification(ec2.ResourceTypeVolume, e.Tags),
 		}
 
 		response, err := t.Cloud.EC2().CreateVolume(request)

upup/pkg/fi/cloudup/awstasks/internetgateway.go

@@ -149,7 +149,9 @@ func (_ *InternetGateway) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *Intern
 	if a == nil {
 		klog.V(2).Infof("Creating InternetGateway")
 
-		request := &ec2.CreateInternetGatewayInput{}
+		request := &ec2.CreateInternetGatewayInput{
+			TagSpecifications: awsup.EC2TagSpecification(ec2.ResourceTypeInternetGateway, e.Tags),
+		}
 
 		response, err := t.Cloud.EC2().CreateInternetGateway(request)
 		if err != nil {

upup/pkg/fi/cloudup/awstasks/securitygroup.go

@@ -166,9 +166,10 @@ func (_ *SecurityGroup) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *Security
 		klog.V(2).Infof("Creating SecurityGroup with Name:%q VPC:%q", *e.Name, *e.VPC.ID)
 
 		request := &ec2.CreateSecurityGroupInput{
-			VpcId:       e.VPC.ID,
-			GroupName:   e.Name,
-			Description: e.Description,
+			VpcId:             e.VPC.ID,
+			GroupName:         e.Name,
+			Description:       e.Description,
+			TagSpecifications: awsup.EC2TagSpecification(ec2.ResourceTypeSecurityGroup, e.Tags),
 		}
 
 		response, err := t.Cloud.EC2().CreateSecurityGroup(request)

upup/pkg/fi/cloudup/awstasks/securitygroup_test.go

@@ -149,8 +149,13 @@ func TestSecurityGroupCreate(t *testing.T) {
 			Description: s("Description"),
 			GroupId:     sg1.ID,
 			VpcId:       vpc1.ID,
-			Tags:        []*ec2.Tag{},
-			GroupName:   s("sg1"),
+			Tags: []*ec2.Tag{
+				{
+					Key:   aws.String("Name"),
+					Value: aws.String("sg1"),
+				},
+			},
+			GroupName: s("sg1"),
 		}
 		actual := c.SecurityGroups[*sg1.ID]
 		if !reflect.DeepEqual(actual, expected) {

upup/pkg/fi/cloudup/awstasks/subnet.go

@@ -181,9 +181,10 @@ func (_ *Subnet) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *Subnet) error {
 		klog.V(2).Infof("Creating Subnet with CIDR: %q", *e.CIDR)
 
 		request := &ec2.CreateSubnetInput{
-			CidrBlock:        e.CIDR,
-			AvailabilityZone: e.AvailabilityZone,
-			VpcId:            e.VPC.ID,
+			CidrBlock:         e.CIDR,
+			AvailabilityZone:  e.AvailabilityZone,
+			VpcId:             e.VPC.ID,
+			TagSpecifications: awsup.EC2TagSpecification(ec2.ResourceTypeSubnet, e.Tags),
 		}
 
 		response, err := t.Cloud.EC2().CreateSubnet(request)

upup/pkg/fi/cloudup/awstasks/subnet_test.go

@@ -145,42 +145,39 @@ func TestSharedSubnetCreateDoesNotCreateNew(t *testing.T) {
 	// Pre-create the vpc / subnet
 	vpc, err := c.CreateVpc(&ec2.CreateVpcInput{
 		CidrBlock: aws.String("172.20.0.0/16"),
-	})
-	if err != nil {
-		t.Fatalf("error creating test VPC: %v", err)
-	}
-	_, err = c.CreateTags(&ec2.CreateTagsInput{
-		Resources: []*string{vpc.Vpc.VpcId},
-		Tags: []*ec2.Tag{
+		TagSpecifications: []*ec2.TagSpecification{
 			{
-				Key:   aws.String("Name"),
-				Value: aws.String("ExistingVPC"),
+				ResourceType: aws.String(ec2.ResourceTypeVpc),
+				Tags: []*ec2.Tag{
+					{
+						Key:   aws.String("Name"),
+						Value: aws.String("ExistingVPC"),
+					},
+				},
 			},
 		},
 	})
 	if err != nil {
-		t.Fatalf("error tagging test vpc: %v", err)
+		t.Fatalf("error creating test VPC: %v", err)
 	}
 
 	subnet, err := c.CreateSubnet(&ec2.CreateSubnetInput{
 		VpcId:     vpc.Vpc.VpcId,
 		CidrBlock: aws.String("172.20.1.0/24"),
-	})
-	if err != nil {
-		t.Fatalf("error creating test subnet: %v", err)
-	}
-
-	_, err = c.CreateTags(&ec2.CreateTagsInput{
-		Resources: []*string{subnet.Subnet.SubnetId},
-		Tags: []*ec2.Tag{
+		TagSpecifications: []*ec2.TagSpecification{
 			{
-				Key:   aws.String("Name"),
-				Value: aws.String("ExistingSubnet"),
+				ResourceType: aws.String(ec2.ResourceTypeSubnet),
+				Tags: []*ec2.Tag{
+					{
+						Key:   aws.String("Name"),
+						Value: aws.String("ExistingSubnet"),
+					},
+				},
 			},
 		},
 	})
 	if err != nil {
-		t.Fatalf("error tagging test subnet: %v", err)
+		t.Fatalf("error creating test subnet: %v", err)
 	}
 
 	// We define a function so we can rebuild the tasks, because we modify in-place when running

upup/pkg/fi/cloudup/awstasks/vpc.go

@@ -155,7 +155,8 @@ func (_ *VPC) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *VPC) error {
 		klog.V(2).Infof("Creating VPC with CIDR: %q", *e.CIDR)
 
 		request := &ec2.CreateVpcInput{
-			CidrBlock: e.CIDR,
+			CidrBlock:         e.CIDR,
+			TagSpecifications: awsup.EC2TagSpecification(ec2.ResourceTypeVpc, e.Tags),
 		}
 
 		response, err := t.Cloud.EC2().CreateVpc(request)

upup/pkg/fi/cloudup/awsup/aws_utils.go

@@ -178,3 +178,22 @@ func AWSErrorMessage(err error) string {
 	}
 	return ""
 }
+
+// EC2TagSpecification converts a map of tags to an EC2 TagSpecification
+func EC2TagSpecification(resourceType string, tags map[string]string) []*ec2.TagSpecification {
+	if len(tags) == 0 {
+		return nil
+	}
+	specification := &ec2.TagSpecification{
+		ResourceType: aws.String(resourceType),
+		Tags:         make([]*ec2.Tag, 0),
+	}
+	for k, v := range tags {
+		specification.Tags = append(specification.Tags, &ec2.Tag{
+			Key:   aws.String(k),
+			Value: aws.String(v),
+		})
+	}
+
+	return []*ec2.TagSpecification{specification}
+}

upup/pkg/fi/cloudup/awsup/aws_utils_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package awsup
 
 import (
+	"reflect"
 	"testing"
 
 	"github.com/aws/aws-sdk-go/aws"
@@ -65,3 +66,42 @@ func TestFindRegion(t *testing.T) {
 	}
 
 }
+
+func TestEC2TagSpecification(t *testing.T) {
+	cases := []struct {
+		Name          string
+		ResourceType  string
+		Tags          map[string]string
+		Specification []*ec2.TagSpecification
+	}{
+		{
+			Name: "No tags",
+		},
+		{
+			Name:         "simple tag",
+			ResourceType: "vpc",
+			Tags: map[string]string{
+				"foo": "bar",
+			},
+			Specification: []*ec2.TagSpecification{
+				{
+					ResourceType: aws.String("vpc"),
+					Tags: []*ec2.Tag{
+						{
+							Key:   aws.String("foo"),
+							Value: aws.String("bar"),
+						},
+					},
+				},
+			},
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.Name, func(t *testing.T) {
+			s := EC2TagSpecification(tc.ResourceType, tc.Tags)
+			if !reflect.DeepEqual(s, tc.Specification) {
+				t.Fatalf("tag specifications did not match: %q vs %q", s, tc.Specification)
+			}
+		})
+	}
+}