Merge lp:~dimitern/goamz/update-aws-api-version-to-latest into lp:goamz

[Dimiter Naydenov]

Updated AWS API version used by default to the latest available (2014-10-01)
This simplifies a few places in the code, and also addresses bug #1394186

A few drive-by fixes were done as well, to ensure the ec2test server reacts the same way as live EC2

Summary of more important changes:
- CreateSecurityGroupVPC method is gone (it was a workaround anyway); now CreateSecurityGroup takes an optional vpcId argument like the *VPC() version used to
- When ec2's debug flag is true, now requests are printed before the responses;
- Fixed the serialization format for SecurityGroup's IPPerm component - SourceIPs is now a slice of CIDRIP{IP:string} structs, so it gets properly serialized to match live EC2;
- In the live tests changed the AZ used for test subnets, as the instance types used there (t1-micro and m1-medium) were not available in us-east-1e AZ (which causes live tests to fail randomly)
- Updated ec2test server's error responses in a few cases to match EC2

[Martin Packman (gz)]

Change looks good - the complication is this *is* an api breakage so will require the use of a versioned api bump or similar to have distribution beyond juju. We can just pin as needed with dependencies.tsv so it's fine for our usage.

A few comments inline:

review: Approve

[Michael Foord (mfoord)]

So long as we can live the incompatible changes to the security groups api it looks fine.

review: Approve

[Gustavo Niemeyer (niemeyer)]

Two comments were already made highlighting the issue of incompatibility, and despite it both of them are approving the change, and no further comments were made about it here. I find that slightly surprising, as it's obvious that the change will break existing code for people relying on it, both inside Canonical and outside. Note also that when you bump an API like this, it is possible that the same operations now have different outcomes, and the fact you had to fix the live tests, per the commit log, seems to indicate that this is happening.

So considering that the juju team is responsible for that package, what is your approach to backwards compatibility, and what's the plan now and for the future?

Also, have you reviewed the change log to consider whether this change might break existing clients due to subtle behavior change in existing operations? Tests will most likely not get all of these cases.

[Dimiter Naydenov (dimitern)]

Thanks for all the reviews!

Now that goamz is migrated to github I'm analyzing how upgrading the AWS API version will affect juju-core's use of goamz and will repropose this against gopkg.in/amz.v2-dev branch. If all goes well, v2-dev could be "promoted" to v2 and juju-core can start using it.

[Dimiter Naydenov (dimitern)]

I've done a careful analysis of the following 3 AWS API versions: 2011-12-15 (used for all non-VPC calls), 2013-10-15 (used for VPC calls), 2014-10-01 (current latest version), using the published AWS WSDL schemas to determine how upgrading the version will impact juju-core's usage of goamz.

Fortunately, there are not many changes and very few, easily fixable changes affect juju-core:

VPC calls with no changes in 2014-10-01 vs 2013-10-15:

AssignPrivateIpAddresses
AttachNetworkInterface
CreateNetworkInterface
CreateSecurityGroup
CreateSubnet
CreateVpc
DeleteNetworkInterface
DeleteSubnet
DeleteVpc
DescribeAccountAttributes
DescribeNetworkInterfaces
DescribeSubnets
DescribeVpcs
DetachNetworkInterface
UnassignPrivateIpAddresses

non-VPC calls with no changes in 2014-10-01 vs 2011-12-15:

AuthorizeSecurityGroupIngress
CreateSnapshot
CreateTags
DeleteSnapshot
DescribeAvailabilityZones
DescribeSecurityGroups
DescribeSnapshots
RebootInstances
RevokeSecurityGroupIngress
StartInstances
StopInstances
TerminateInstances

non-VPC calls with changes between 2011-12-15 and 2014-10-01:

DescribeInstances
    request changes:
        maxResults, nextToken (added - not supported or used by juju; pagination support)
    response changes:
        instancesSet>item>productCodes>item>type (added - not supported or used by juju-core)
        instancesSet>item>iamInstanceProfile>* (added whole subtree - not supported or used by juju-core)
        instancesSet>item>ebsOptimized (added - not supported but needed for storage work in juju-core)
        instancesSet>item>sriovNetSupport (added - not supported or used by juju-core)
        instancesSet>item>networkInterfaceSet>item>macAddress (added - already supported since VPC calls landed)
        instancesSet>item>networkInterfaceSet>item>association>* (added whole subtree - not used by juju-core; related to elastic IPs)
        instancesSet>item>networkInterfaceSet>item>privateIpAddressesSet>* (added whole subtree - already supported since VPC calls landed; >item>association>* not supported, but not used as well)
        nextToken (added - not supported or used by juju; for pagination of results)
RunInstances
    request changes:
        blockDeviceMapping>item>ebs>volumeType (added - already supported since storage changes landed)
        blockDeviceMapping>item>ebs>iops (added - already supported since storage changes)
        networkInterfaceSet>item>* - same changes as in DescribeInstances, except:
        networkInterfaceSet>item>associatePublicIpAddress (added - not supported but needed by juju)
        iamInstanceProfile>* (added whole subtree - not supported or used by juju)
        ebsOptimized (added - not supported but possibly needed for juju storage work)
    response changes: the same as for DescribeInstances, except for nextToken

DescribeImages
    request - no changes
    response changes:
        sriovNetSupport (added - not supported or used by juju)
        productCodes, blockDeviceMapping (added - same changes as in RunInstances)

So overall upgrading AWS API version to 2014-10-01 won't require changes in juju-core, and only a few changes to add what's missing in the request/response structs,...

Unmerged revisions

51. By Dimiter Naydenov on 2014-11-27

Upgraded AWS API version to latest - 2014-10-01; live tests pass OK

50. By Dimiter Naydenov on 2014-11-27

All live tests fixed; using the latest API version now

=== modified file 'ec2/ec2.go'
--- ec2/ec2.go	2014-08-11 22:08:47 +0000
+++ ec2/ec2.go	2014-11-27 15:29:26 +0000
@@ -10,6 +10,7 @@
 
 import (
 	"crypto/rand"
+	"encoding/base64"
 	"encoding/hex"
 	"encoding/xml"
 	"fmt"
@@ -21,19 +22,14 @@
 	"strconv"
 	"time"
 
-	"encoding/base64"
 	"launchpad.net/goamz/aws"
 )
 
 const (
 	debug = false
 
-	// legacyAPIVersion is the AWS API version used for all but
-	// VPC-related requests.
-	legacyAPIVersion = "2011-12-15"
-
-	// AWS API version used for VPC-related calls.
-	vpcAPIVersion = "2013-10-15"
+	// apiVersion is the AWS API version used for all requests.
+	apiVersion = "2014-10-01"
 )
 
 // The EC2 type encapsulates operations with a specific EC2 region.
@@ -152,7 +148,10 @@
 	defer r.Body.Close()
 
 	if debug {
-		dump, _ := httputil.DumpResponse(r, true)
+		dump, _ := httputil.DumpRequest(req, true)
+		log.Printf("request:\n")
+		log.Printf("%v\n}\n", string(dump))
+		dump, _ = httputil.DumpResponse(r, true)
 		log.Printf("response:\n")
 		log.Printf("%v\n}\n", string(dump))
 	}
@@ -186,17 +185,9 @@
 }
 
 func makeParams(action string) map[string]string {
-	return makeParamsWithVersion(action, legacyAPIVersion)
-}
-
-func makeParamsVPC(action string) map[string]string {
-	return makeParamsWithVersion(action, vpcAPIVersion)
-}
-
-func makeParamsWithVersion(action, version string) map[string]string {
 	params := make(map[string]string)
 	params["Action"] = action
-	params["Version"] = version
+	params["Version"] = apiVersion
 	return params
 }
 
@@ -309,7 +300,7 @@
 //
 // See http://goo.gl/Mcm3b for more details.
 func (ec2 *EC2) RunInstances(options *RunInstances) (resp *RunInstancesResp, err error) {
-	params := prepareRunParams(*options)
+	params := makeParams("RunInstances")
 	params["ImageId"] = options.ImageId
 	params["InstanceType"] = options.InstanceType
 	var min, max int
@@ -387,16 +378,6 @@
 	return
 }
 
-func prepareRunParams(options RunInstances) map[string]string {
-	if options.SubnetId != "" || len(options.NetworkInterfaces) > 0 {
-		// When either SubnetId or NetworkInterfaces are specified, we
-		// need to use the API version with complete VPC support.
-		return makeParamsVPC("RunInstances")
-	} else {
-		return makeParams("RunInstances")
-	}
-}
-
 func prepareBlockDevices(params map[string]string, blockDevs []BlockDeviceMapping) {
 	for i, b := range blockDevs {
 		n := strconv.Itoa(i + 1)
@@ -731,20 +712,12 @@
 }
 
 // CreateSecurityGroup creates a security group with the provided name
-// and description.
-//
-// See http://goo.gl/Eo7Yl for more details.
-func (ec2 *EC2) CreateSecurityGroup(name, description string) (resp *CreateSecurityGroupResp, err error) {
-	return ec2.CreateSecurityGroupVPC("", name, description)
-}
-
-// CreateSecurityGroupVPC creates a security group in EC2, associated
-// with the given VPC ID. If vpcId is empty, this call is equivalent
-// to CreateSecurityGroup.
-//
-// See http://goo.gl/Eo7Yl for more details.
-func (ec2 *EC2) CreateSecurityGroupVPC(vpcId, name, description string) (resp *CreateSecurityGroupResp, err error) {
-	params := makeParamsVPC("CreateSecurityGroup")
+// and description. If vpcId can be empty to create a EC2-Classic
+// group.
+//
+// See http://goo.gl/Eo7Yl for more details.
+func (ec2 *EC2) CreateSecurityGroup(vpcId, name, description string) (resp *CreateSecurityGroupResp, err error) {
+	params := makeParams("CreateSecurityGroup")
 	params["GroupName"] = name
 	params["GroupDescription"] = description
 	if vpcId != "" {
@@ -780,6 +753,12 @@
 	IPPerms     []IPPerm `xml:"ipPermissions>item"`
 }
 
+// CIDRIP represents a single source address in CIDR notation (e.g.
+// 1.2.3.4/5), used in a security group IPPerm rule.
+type CIDRIP struct {
+	IP string `xml:"cidrIp"`
+}
+
 // IPPerm represents an allowance within an EC2 security group.
 //
 // See http://goo.gl/4oTxv for more details.
@@ -787,7 +766,7 @@
 	Protocol     string              `xml:"ipProtocol"`
 	FromPort     int                 `xml:"fromPort"`
 	ToPort       int                 `xml:"toPort"`
-	SourceIPs    []string            `xml:"ipRanges>item>cidrIp"`
+	SourceIPs    []CIDRIP            `xml:"ipRanges>item"`
 	SourceGroups []UserSecurityGroup `xml:"groups>item"`
 }
 
@@ -902,7 +881,7 @@
 		params[prefix+".FromPort"] = strconv.Itoa(perm.FromPort)
 		params[prefix+".ToPort"] = strconv.Itoa(perm.ToPort)
 		for j, ip := range perm.SourceIPs {
-			params[prefix+".IpRanges."+strconv.Itoa(j+1)+".CidrIp"] = ip
+			params[prefix+".IpRanges."+strconv.Itoa(j+1)+".CidrIp"] = ip.IP
 		}
 		for j, g := range perm.SourceGroups {
 			subprefix := prefix + ".Groups." + strconv.Itoa(j+1)
@@ -1078,7 +1057,7 @@
 //
 // See http://goo.gl/hBc28j for more details.
 func (ec2 *EC2) AccountAttributes(attrNames ...string) (*AccountAttributesResp, error) {
-	params := makeParamsVPC("DescribeAccountAttributes")
+	params := makeParams("DescribeAccountAttributes")
 	for i, attrName := range attrNames {
 		params["AttributeName."+strconv.Itoa(i+1)] = attrName
 	}

=== modified file 'ec2/ec2_test.go'
--- ec2/ec2_test.go	2014-08-11 22:08:47 +0000
+++ ec2/ec2_test.go	2014-11-27 15:29:26 +0000
@@ -3,10 +3,10 @@
 import (
 	"testing"
 
+	. "gopkg.in/check.v1"
 	"launchpad.net/goamz/aws"
 	"launchpad.net/goamz/ec2"
 	"launchpad.net/goamz/testutil"
-	. "gopkg.in/check.v1"
 )
 
 func Test(t *testing.T) {
@@ -128,11 +128,6 @@
 			}},
 		}},
 	}
-	params := ec2.PrepareRunParams(options)
-	c.Assert(params, DeepEquals, map[string]string{
-		"Version": "2013-10-15",
-		"Action":  "RunInstances",
-	})
 	resp, err := s.ec2.RunInstances(&options)
 
 	req := testServer.WaitRequest()
@@ -503,12 +498,12 @@
 
 func (s *S) TestCreateSecurityGroupExample(c *C) {
 	testServer.Response(200, nil, CreateSecurityGroupExample)
-	resp, err := s.ec2.CreateSecurityGroup("websrv", "Web Servers")
+	resp, err := s.ec2.CreateSecurityGroup("", "websrv", "Web Servers")
 	c.Assert(err, IsNil)
 	s.checkCreateSGResponse(c, resp, "sg-67ad940e", "websrv", "Web Servers", "")
 
 	testServer.Response(200, nil, CreateSecurityGroupExample)
-	resp, err = s.ec2.CreateSecurityGroupVPC("vpc-id", "websrv", "Web Servers")
+	resp, err = s.ec2.CreateSecurityGroup("vpc-id", "websrv", "Web Servers")
 	c.Assert(err, IsNil)
 	s.checkCreateSGResponse(c, resp, "sg-67ad940e", "websrv", "Web Servers", "vpc-id")
 }
@@ -538,7 +533,7 @@
 	c.Assert(g0ipp.Protocol, Equals, "tcp")
 	c.Assert(g0ipp.FromPort, Equals, 80)
 	c.Assert(g0ipp.ToPort, Equals, 80)
-	c.Assert(g0ipp.SourceIPs, DeepEquals, []string{"0.0.0.0/0"})
+	c.Assert(g0ipp.SourceIPs, DeepEquals, sourceIPs("0.0.0.0/0"))
 
 	g1 := resp.Groups[1]
 	c.Assert(g1.OwnerId, Equals, "999988887777")
@@ -632,6 +627,14 @@
 	c.Assert(req.Form["GroupId"], DeepEquals, []string{"sg-67ad940e"})
 }
 
+func sourceIPs(ips ...string) []ec2.CIDRIP {
+	result := make([]ec2.CIDRIP, len(ips))
+	for i, ip := range ips {
+		result[i].IP = ip
+	}
+	return result
+}
+
 func (s *S) TestAuthorizeSecurityGroupExample1(c *C) {
 	testServer.Response(200, nil, AuthorizeSecurityGroupIngressExample)
 
@@ -639,7 +642,7 @@
 		Protocol:  "tcp",
 		FromPort:  80,
 		ToPort:    80,
-		SourceIPs: []string{"205.192.0.0/16", "205.159.0.0/16"},
+		SourceIPs: sourceIPs("205.192.0.0/16", "205.159.0.0/16"),
 	}}
 	resp, err := s.ec2.AuthorizeSecurityGroup(ec2.SecurityGroup{Name: "websrv"}, perms)
 
@@ -664,7 +667,7 @@
 		Protocol:  "tcp",
 		FromPort:  80,
 		ToPort:    80,
-		SourceIPs: []string{"205.192.0.0/16", "205.159.0.0/16"},
+		SourceIPs: sourceIPs("205.192.0.0/16", "205.159.0.0/16"),
 	}}
 	// ignore return and error - we're only want to check the parameter handling.
 	s.ec2.AuthorizeSecurityGroup(ec2.SecurityGroup{Id: "sg-67ad940e", Name: "ignored"}, perms)

=== modified file 'ec2/ec2i_test.go'
--- ec2/ec2i_test.go	2014-08-11 22:08:47 +0000
+++ ec2/ec2i_test.go	2014-11-27 15:29:26 +0000
@@ -4,10 +4,10 @@
 	"crypto/rand"
 	"fmt"
 
+	. "gopkg.in/check.v1"
 	"launchpad.net/goamz/aws"
 	"launchpad.net/goamz/ec2"
 	"launchpad.net/goamz/testutil"
-	. "gopkg.in/check.v1"
 )
 
 // AmazonServer represents an Amazon EC2 server.
@@ -108,13 +108,13 @@
 	s.ec2.DeleteSecurityGroup(ec2.SecurityGroup{Name: name})
 	defer s.ec2.DeleteSecurityGroup(ec2.SecurityGroup{Name: name})
 
-	resp1, err := s.ec2.CreateSecurityGroup(name, descr)
+	resp1, err := s.ec2.CreateSecurityGroup("", name, descr)
 	c.Assert(err, IsNil)
 	c.Assert(resp1.RequestId, Matches, ".+")
 	c.Assert(resp1.Name, Equals, name)
 	c.Assert(resp1.Id, Matches, ".+")
 
-	resp1, err = s.ec2.CreateSecurityGroup(name, descr)
+	resp1, err = s.ec2.CreateSecurityGroup("", name, descr)
 	ec2err, _ := err.(*ec2.Error)
 	c.Assert(resp1, IsNil)
 	c.Assert(ec2err, NotNil)
@@ -124,7 +124,7 @@
 		Protocol:  "tcp",
 		FromPort:  0,
 		ToPort:    1024,
-		SourceIPs: []string{"127.0.0.1/24"},
+		SourceIPs: sourceIPs("127.0.0.0/24"),
 	}}
 
 	resp2, err := s.ec2.AuthorizeSecurityGroup(ec2.SecurityGroup{Name: name}, perms)
@@ -143,7 +143,7 @@
 	c.Assert(g0.IPPerms[0].Protocol, Equals, "tcp")
 	c.Assert(g0.IPPerms[0].FromPort, Equals, 0)
 	c.Assert(g0.IPPerms[0].ToPort, Equals, 1024)
-	c.Assert(g0.IPPerms[0].SourceIPs, DeepEquals, []string{"127.0.0.1/24"})
+	c.Assert(g0.IPPerms[0].SourceIPs, DeepEquals, sourceIPs("127.0.0.0/24"))
 
 	resp2, err = s.ec2.DeleteSecurityGroup(ec2.SecurityGroup{Name: name})
 	c.Assert(err, IsNil)
@@ -178,7 +178,7 @@
 		Protocol:  "tcp",
 		FromPort:  80,
 		ToPort:    80,
-		SourceIPs: []string{"127.0.0.1/32"},
+		SourceIPs: sourceIPs("127.0.0.1/32"),
 	}}
 	errs := make(chan error, len(allRegions))
 	for _, region := range allRegions {

=== modified file 'ec2/ec2t_test.go'
--- ec2/ec2t_test.go	2014-08-11 22:08:47 +0000
+++ ec2/ec2t_test.go	2014-11-27 15:29:26 +0000
@@ -7,11 +7,11 @@
 	"sort"
 	"time"
 
+	. "gopkg.in/check.v1"
 	"launchpad.net/goamz/aws"
 	"launchpad.net/goamz/ec2"
 	"launchpad.net/goamz/ec2/ec2test"
 	"launchpad.net/goamz/testutil"
-	. "gopkg.in/check.v1"
 )
 
 // LocalServer represents a local ec2test fake server.
@@ -230,7 +230,18 @@
 		switch attr.Name {
 		case "supported-platforms":
 			sort.Strings(attr.Values)
-			c.Assert(attr.Values, DeepEquals, []string{"EC2", "VPC"})
+			c.Assert(attr.Values, Not(HasLen), 0)
+			if len(attr.Values) == 2 {
+				c.Assert(attr.Values, DeepEquals, []string{"EC2", "VPC"})
+			} else if len(attr.Values) == 1 {
+				// Some regions have only VPC or EC2 enabled, and
+				// because this test runs both against a local test
+				// server and against the Amazon servers we need to
+				// account for both cases.
+				c.Assert(attr.Values[0], Matches, `(EC2|VPC)`)
+			} else {
+				c.Fatalf("unexpected account attributes: %v", attr.Values)
+			}
 		case "default-vpc":
 			c.Assert(attr.Values, HasLen, 1)
 			c.Assert(attr.Values[0], Not(Equals), "")
@@ -244,8 +255,8 @@
 	defaultVPCId, defaultSubnets := s.getDefaultVPCIdAndSubnets(c)
 	subnet := defaultSubnets[0]
 
-	g0 := s.makeTestGroupVPC(c, defaultVPCId, "goamz-test0", "ec2test group 0")
-	g1 := s.makeTestGroupVPC(c, defaultVPCId, "goamz-test1", "ec2test group 1")
+	g0 := s.makeTestGroup(c, defaultVPCId, "goamz-test0", "ec2test group 0")
+	g1 := s.makeTestGroup(c, defaultVPCId, "goamz-test1", "ec2test group 1")
 	defer s.deleteGroups(c, []ec2.SecurityGroup{g0, g1})
 
 	ip1 := validIPForSubnet(c, subnet, 5)
@@ -345,7 +356,10 @@
 		// the instance might launch in any one.
 		for _, sub := range defaultSubnets {
 			if inst.SubnetId == sub.Id {
-				subnet = &sub
+				subnet.Id = sub.Id
+				subnet.VPCId = sub.VPCId
+				subnet.CIDRBlock = sub.CIDRBlock
+				break
 			}
 		}
 	}
@@ -419,26 +433,23 @@
 	c.Fatalf("%v INSTANCES LEFT RUNNING!!!", ids)
 }
 
-func (s *ServerTests) makeTestGroup(c *C, name, descr string) ec2.SecurityGroup {
-	return s.makeTestGroupVPC(c, "", name, descr)
-}
-
-func (s *ServerTests) makeTestGroupVPC(c *C, vpcId, name, descr string) ec2.SecurityGroup {
+func (s *ServerTests) makeTestGroup(c *C, vpcId, name, descr string) ec2.SecurityGroup {
 	// Clean it up if a previous test left it around.
 	_, err := s.ec2.DeleteSecurityGroup(ec2.SecurityGroup{Name: name})
 	if err != nil && errorCode(err) != "InvalidGroup.NotFound" {
 		c.Fatalf("delete security group: %v", err)
 	}
 
-	resp, err := s.ec2.CreateSecurityGroupVPC(vpcId, name, descr)
+	resp, err := s.ec2.CreateSecurityGroup(vpcId, name, descr)
 	c.Assert(err, IsNil)
-	c.Assert(resp.Name, Equals, name)
+	c.Assert(resp.Id, Matches, "sg-.+")
+	c.Logf("created group %v", resp.SecurityGroup)
 	return resp.SecurityGroup
 }
 
 func (s *ServerTests) TestIPPerms(c *C) {
-	g0 := s.makeTestGroup(c, "goamz-test0", "ec2test group 0")
-	g1 := s.makeTestGroup(c, "goamz-test1", "ec2test group 1")
+	g0 := s.makeTestGroup(c, "", "goamz-test0", "ec2test group 0")
+	g1 := s.makeTestGroup(c, "", "goamz-test1", "ec2test group 1")
 	defer s.deleteGroups(c, []ec2.SecurityGroup{g0, g1})
 
 	resp, err := s.ec2.SecurityGroups([]ec2.SecurityGroup{g0, g1}, nil)
@@ -446,6 +457,10 @@
 	c.Assert(resp.Groups, HasLen, 2)
 	c.Assert(resp.Groups[0].IPPerms, HasLen, 0)
 	c.Assert(resp.Groups[1].IPPerms, HasLen, 0)
+	nameRegexp := fmt.Sprintf("(%s|%s)", regexp.QuoteMeta(g0.Name), regexp.QuoteMeta(g1.Name))
+	for _, rgroup := range resp.Groups {
+		c.Check(rgroup.Name, Matches, nameRegexp)
+	}
 
 	ownerId := resp.Groups[0].OwnerId
 
@@ -455,17 +470,17 @@
 		Protocol:  "tcp",
 		FromPort:  0,
 		ToPort:    1024,
-		SourceIPs: []string{"z127.0.0.1/24"},
+		SourceIPs: sourceIPs("invalid"),
 	}})
 	c.Assert(err, NotNil)
-	c.Check(errorCode(err), Equals, "InvalidPermission.Malformed")
+	c.Check(errorCode(err), Equals, "InvalidParameterValue")
 
 	// Check that AuthorizeSecurityGroup adds the correct authorizations.
 	_, err = s.ec2.AuthorizeSecurityGroup(g0, []ec2.IPPerm{{
 		Protocol:  "tcp",
 		FromPort:  2000,
 		ToPort:    2001,
-		SourceIPs: []string{"127.0.0.0/24"},
+		SourceIPs: sourceIPs("127.0.0.0/24"),
 		SourceGroups: []ec2.UserSecurityGroup{{
 			Name: g1.Name,
 		}, {
@@ -475,48 +490,48 @@
 		Protocol:  "tcp",
 		FromPort:  2000,
 		ToPort:    2001,
-		SourceIPs: []string{"200.1.1.34/32"},
+		SourceIPs: sourceIPs("200.1.1.34/32"),
 	}})
 	c.Assert(err, IsNil)
 
 	resp, err = s.ec2.SecurityGroups([]ec2.SecurityGroup{g0}, nil)
 	c.Assert(err, IsNil)
 	c.Assert(resp.Groups, HasLen, 1)
-	c.Assert(resp.Groups[0].IPPerms, HasLen, 1)
-
-	perm := resp.Groups[0].IPPerms[0]
-	srcg := perm.SourceGroups
-	c.Assert(srcg, HasLen, 2)
-
-	// Normalize so we don't care about returned order.
-	if srcg[0].Name == g1.Name {
-		srcg[0], srcg[1] = srcg[1], srcg[0]
+	c.Assert(resp.Groups[0].IPPerms, HasLen, 3)
+
+	for _, perm := range resp.Groups[0].IPPerms {
+		srcg := perm.SourceGroups
+		if len(srcg) == 0 {
+			// Only SourceIPs should exist.
+			c.Check(perm.SourceIPs, DeepEquals, sourceIPs("127.0.0.0/24", "200.1.1.34/32"))
+			c.Check(srcg, HasLen, 0)
+		} else {
+			// Only SourceGroups should exist, and there should be one
+			// group per IPPerm.
+			c.Check(srcg, HasLen, 1)
+			c.Check(perm.SourceIPs, HasLen, 0)
+
+			idRegexp := fmt.Sprintf("(%s|%s)", regexp.QuoteMeta(g0.Id), regexp.QuoteMeta(g1.Id))
+			c.Check(srcg[0].Id, Matches, idRegexp)
+			c.Check(srcg[0].OwnerId, Equals, ownerId)
+		}
 	}
-	c.Check(srcg[0].Name, Equals, g0.Name)
-	c.Check(srcg[0].Id, Equals, g0.Id)
-	c.Check(srcg[0].OwnerId, Equals, ownerId)
-	c.Check(srcg[1].Name, Equals, g1.Name)
-	c.Check(srcg[1].Id, Equals, g1.Id)
-	c.Check(srcg[1].OwnerId, Equals, ownerId)
-
-	sort.Strings(perm.SourceIPs)
-	c.Check(perm.SourceIPs, DeepEquals, []string{"127.0.0.0/24", "200.1.1.34/32"})
 
 	// Check that we can't delete g1 (because g0 is using it)
 	_, err = s.ec2.DeleteSecurityGroup(g1)
 	c.Assert(err, NotNil)
-	c.Check(errorCode(err), Equals, "InvalidGroup.InUse")
+	c.Check(errorCode(err), Equals, "DependencyViolation")
 
 	_, err = s.ec2.RevokeSecurityGroup(g0, []ec2.IPPerm{{
 		Protocol:     "tcp",
 		FromPort:     2000,
 		ToPort:       2001,
-		SourceGroups: []ec2.UserSecurityGroup{{Id: g1.Id}},
+		SourceGroups: []ec2.UserSecurityGroup{{Id: g1.Id}, {Name: g0.Name}},
 	}, {
 		Protocol:  "tcp",
 		FromPort:  2000,
 		ToPort:    2001,
-		SourceIPs: []string{"200.1.1.34/32"},
+		SourceIPs: sourceIPs("200.1.1.34/32"),
 	}})
 	c.Assert(err, IsNil)
 
@@ -525,14 +540,10 @@
 	c.Assert(resp.Groups, HasLen, 1)
 	c.Assert(resp.Groups[0].IPPerms, HasLen, 1)
 
-	perm = resp.Groups[0].IPPerms[0]
-	srcg = perm.SourceGroups
-	c.Assert(srcg, HasLen, 1)
-	c.Check(srcg[0].Name, Equals, g0.Name)
-	c.Check(srcg[0].Id, Equals, g0.Id)
-	c.Check(srcg[0].OwnerId, Equals, ownerId)
+	perm := resp.Groups[0].IPPerms[0]
+	c.Assert(perm.SourceGroups, HasLen, 0)
 
-	c.Check(perm.SourceIPs, DeepEquals, []string{"127.0.0.0/24"})
+	c.Check(perm.SourceIPs, DeepEquals, sourceIPs("127.0.0.0/24"))
 
 	// We should be able to delete g1 now because we've removed its only use.
 	_, err = s.ec2.DeleteSecurityGroup(g1)
@@ -556,7 +567,7 @@
 	s.ec2.DeleteSecurityGroup(ec2.SecurityGroup{Name: name})
 	defer s.ec2.DeleteSecurityGroup(ec2.SecurityGroup{Name: name})
 
-	resp1, err := s.ec2.CreateSecurityGroup(name, descr)
+	resp1, err := s.ec2.CreateSecurityGroup("", name, descr)
 	c.Assert(err, IsNil)
 	c.Assert(resp1.Name, Equals, name)
 
@@ -564,12 +575,12 @@
 		Protocol:  "tcp",
 		FromPort:  200,
 		ToPort:    1024,
-		SourceIPs: []string{"127.0.0.1/24"},
+		SourceIPs: sourceIPs("127.0.0.1/24"),
 	}, {
 		Protocol:  "tcp",
 		FromPort:  0,
 		ToPort:    100,
-		SourceIPs: []string{"127.0.0.1/24"},
+		SourceIPs: sourceIPs("127.0.0.1/24"),
 	}}
 
 	_, err = s.ec2.AuthorizeSecurityGroup(ec2.SecurityGroup{Name: name}, perms[0:1])
@@ -590,24 +601,34 @@
 	vpcId := vpcResp.VPC.Id
 	defer s.deleteVPCs(c, []string{vpcId})
 
-	subResp := s.createSubnet(c, vpcId, "10.4.1.0/24", "")
+	// Set AZ to us-east-1c explicitly, as t1.micro is not supported
+	// on us-east-1e, if it happens to get picked up randomly during
+	// the live test.
+	subResp := s.createSubnet(c, vpcId, "10.4.1.0/24", "us-east-1c")
 	subId := subResp.Subnet.Id
 	defer s.deleteSubnets(c, []string{subId})
 
 	groupResp, err := s.ec2.CreateSecurityGroup(
+		"",
 		sessionName("testgroup1"),
 		"testgroup one description",
 	)
 	c.Assert(err, IsNil)
 	group1 := groupResp.SecurityGroup
+	c.Assert(group1.Name, Not(Equals), "")
+	c.Assert(group1.Id, Matches, "sg-.+")
+	c.Logf("created group %v", group1)
 
-	groupResp, err = s.ec2.CreateSecurityGroupVPC(
+	groupResp, err = s.ec2.CreateSecurityGroup(
 		vpcId,
 		sessionName("testgroup2"),
 		"testgroup two description vpc",
 	)
 	c.Assert(err, IsNil)
 	group2 := groupResp.SecurityGroup
+	c.Assert(group2.Name, Not(Equals), "")
+	c.Assert(group2.Id, Matches, "sg-.+")
+	c.Logf("created group %v", group2)
 
 	defer s.deleteGroups(c, []ec2.SecurityGroup{group1, group2})
 
@@ -679,24 +700,12 @@
 				{"instance-id", []string{"i-deadbeef12345"}},
 			},
 		}, {
-			about: "check that filtering on group id works",
-			filters: []filterSpec{
-				{"group-id", []string{group1.Id}},
-			},
-			resultIds: ids(0, 1),
-		}, {
 			about: "check that filtering on group id with instance prefix works",
 			filters: []filterSpec{
 				{"instance.group-id", []string{group1.Id}},
 			},
 			resultIds: ids(0, 1),
 		}, {
-			about: "check that filtering on group name works",
-			filters: []filterSpec{
-				{"group-name", []string{group1.Name}},
-			},
-			resultIds: ids(0, 1),
-		}, {
 			about: "check that filtering on group name with instance prefix works",
 			filters: []filterSpec{
 				{"instance.group-name", []string{group1.Name}},
@@ -713,14 +722,14 @@
 			about: "combination filters 1",
 			filters: []filterSpec{
 				{"image-id", []string{imageId, imageId2}},
-				{"group-name", []string{group1.Name}},
+				{"instance.group-name", []string{group1.Name}},
 			},
 			resultIds: ids(0, 1),
 		}, {
 			about: "combination filters 2",
 			filters: []filterSpec{
 				{"image-id", []string{imageId2}},
-				{"group-name", []string{group1.Name}},
+				{"instance.group-name", []string{group1.Name}},
 			},
 		}, {
 			about: "VPC filters in combination",
@@ -740,6 +749,8 @@
 				f.Add(spec.name, spec.values...)
 			}
 		}
+		c.Logf("\tusing filter: %v", f)
+		c.Logf("\texpecting result ids: %v", t.resultIds)
 		resp, err := s.ec2.Instances(t.instanceIds, f)
 		if t.err != "" {
 			c.Check(err, ErrorMatches, t.err)
@@ -769,11 +780,14 @@
 	vpcId := vpcResp.VPC.Id
 	defer s.deleteVPCs(c, []string{vpcId})
 
-	subResp := s.createSubnet(c, vpcId, "10.6.1.0/24", "")
+	// Set AZ to us-east-1c explicitly, as t1.micro is not supported
+	// on us-east-1e, if it happens to get picked up randomly during
+	// the live test.
+	subResp := s.createSubnet(c, vpcId, "10.6.1.0/24", "us-east-1c")
 	subId := subResp.Subnet.Id
 	defer s.deleteSubnets(c, []string{subId})
 
-	groupResp, err := s.ec2.CreateSecurityGroupVPC(
+	groupResp, err := s.ec2.CreateSecurityGroup(
 		vpcId,
 		sessionName("testgroup1 vpc"),
 		"testgroup description vpc",
@@ -881,9 +895,9 @@
 			// Create the first one as a VPC group.
 			gid += " vpc"
 			desc += " vpc"
-			resp, err = s.ec2.CreateSecurityGroupVPC(vpcId, gid, desc)
+			resp, err = s.ec2.CreateSecurityGroup(vpcId, gid, desc)
 		} else {
-			resp, err = s.ec2.CreateSecurityGroup(gid, desc)
+			resp, err = s.ec2.CreateSecurityGroup("", gid, desc)
 		}
 		c.Assert(err, IsNil)
 		g[i] = resp.SecurityGroup
@@ -900,7 +914,7 @@
 			Protocol:  "tcp",
 			FromPort:  100,
 			ToPort:    200,
-			SourceIPs: []string{"1.2.3.4/32"},
+			SourceIPs: sourceIPs("1.2.3.4/32"),
 		}},
 		{{
 			Protocol:     "tcp",
@@ -958,7 +972,7 @@
 			results: groups(1, 2),
 		}, {
 			about:  "check that specifying a non-existent group id gives an error",
-			groups: append(groups(0), ec2.SecurityGroup{Id: "sg-eeeeeeeee"}),
+			groups: append(groups(0), ec2.SecurityGroup{Id: "sg-eeeeeeee"}),
 			err:    `.*\(InvalidGroup\.NotFound\)`,
 		}, {
 			about: "check that a filter allowed two groups returns both of them",
@@ -977,13 +991,14 @@
 		}, {
 			about: "check that a filter allowing no groups returns none",
 			filters: []filterSpec{
-				{"group-id", []string{"sg-eeeeeeeee"}},
+				{"group-id", []string{"sg-eeeeeeee"}},
 			},
 		},
 		filterCheck("description", "testdescription1", groups(1)),
 		filterCheck("group-name", g[2].Name, groups(2)),
+		filterCheck("group-id", g[2].Id, groups(2)),
 		filterCheck("ip-permission.cidr", "1.2.3.4/32", groups(1)),
-		filterCheck("ip-permission.group-name", g[2].Name, groups(2, 3)),
+		filterCheck("ip-permission.group-id", g[2].Id, groups(2, 3)),
 		filterCheck("ip-permission.protocol", "udp", groups(3)),
 		filterCheck("ip-permission.from-port", "200", groups(2, 3)),
 		filterCheck("ip-permission.to-port", "200", groups(1)),
@@ -999,12 +1014,14 @@
 				f.Add(spec.name, spec.values...)
 			}
 		}
+		c.Logf("\tusing filter: %v", f)
+		c.Logf("\texpecting results: %v", t.results)
 		resp, err := s.ec2.SecurityGroups(t.groups, f)
 		if t.err != "" {
 			c.Check(err, ErrorMatches, t.err)
 			continue
 		}
-		c.Assert(err, IsNil)
+		c.Check(err, IsNil)
 		groups := make(map[string]*ec2.SecurityGroup)
 		for j := range resp.Groups {
 			group := &resp.Groups[j].SecurityGroup
@@ -1025,8 +1042,10 @@
 		c.Check(groups, HasLen, len(t.results))
 		for j, g := range t.results {
 			rg := groups[g.Id]
-			c.Assert(rg, NotNil, Commentf("group %d (%v) not found; got %#v", j, g, groups))
-			c.Check(rg.Name, Equals, g.Name, Commentf("group %d (%v)", j, g))
+			c.Check(rg, NotNil, Commentf("group %d (%v) not found; got %#v", j, g, groups))
+			if rg != nil {
+				c.Check(rg.Name, Equals, g.Name, Commentf("group %d (%v)", j, g))
+			}
 		}
 	}
 }

=== modified file 'ec2/ec2test/server.go'
--- ec2/ec2test/server.go	2014-06-04 05:44:18 +0000
+++ ec2/ec2test/server.go	2014-11-27 15:29:26 +0000
@@ -141,6 +141,10 @@
 		return g.hasPerm(func(k permKey) bool {
 			return k.group != nil && k.group.name == value
 		}), nil
+	case "ip-permission.group-id":
+		return g.hasPerm(func(k permKey) bool {
+			return k.group != nil && k.group.id == value
+		}), nil
 	case "ip-permission.from-port":
 		port, err := strconv.Atoi(value)
 		if err != nil {
@@ -176,13 +180,12 @@
 // to g. It groups permissions by port range and protocol.
 func (g *securityGroup) ec2Perms() (perms []ec2.IPPerm) {
 	// The grouping is held in result. We use permKey for convenience,
-	// (ensuring that the group and ipAddr of each key is zero). For
-	// each protocol/port range combination, we build up the permission
-	// set in the associated value.
+	// (ensuring that the ipAddr of each key is zero). For each
+	// protocol/port range combination, we build up the permission set
+	// in the associated value.
 	result := make(map[permKey]*ec2.IPPerm)
 	for k := range g.perms {
 		groupKey := k
-		groupKey.group = nil
 		groupKey.ipAddr = ""
 
 		ec2p := result[groupKey]
@@ -192,7 +195,6 @@
 				FromPort: k.fromPort,
 				ToPort:   k.toPort,
 			}
-			result[groupKey] = ec2p
 		}
 		if k.group != nil {
 			ec2p.SourceGroups = append(ec2p.SourceGroups,
@@ -202,9 +204,11 @@
 					OwnerId: ownerId,
 				})
 		} else {
-			ec2p.SourceIPs = append(ec2p.SourceIPs, k.ipAddr)
+			ec2p.SourceIPs = append(ec2p.SourceIPs, ec2.CIDRIP{k.ipAddr})
 		}
+		result[groupKey] = ec2p
 	}
+
 	for _, ec2p := range result {
 		perms = append(perms, *ec2p)
 	}
@@ -1313,7 +1317,7 @@
 }
 
 var (
-	secGroupPat = regexp.MustCompile(`^sg-[a-z0-9]+$`)
+	secGroupPat = regexp.MustCompile(`^sg-[a-f0-9]+$`)
 	cidrIpPat   = regexp.MustCompile(`^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+/([0-9]+)$`)
 	ownerIdPat  = regexp.MustCompile(`^[0-9]+$`)
 )
@@ -1401,9 +1405,9 @@
 			switch rest {
 			case "CidrIp":
 				if !cidrIpPat.MatchString(val) {
-					fatalf(400, "InvalidPermission.Malformed", "Invalid IP range: %q", val)
+					fatalf(400, "InvalidParameterValue", "Invalid IP range: %q", val)
 				}
-				ec2p.SourceIPs = append(ec2p.SourceIPs, val)
+				ec2p.SourceIPs = append(ec2p.SourceIPs, ec2.CIDRIP{val})
 			default:
 				fatalf(400, "UnknownParameter", "unknown parameter %q", name)
 			}
@@ -1451,7 +1455,7 @@
 		}
 		k.group = nil
 		for _, ip := range p.SourceIPs {
-			k.ipAddr = ip
+			k.ipAddr = ip.IP
 			result = append(result, k)
 		}
 	}
@@ -1471,7 +1475,7 @@
 	for _, r := range srv.reservations {
 		for _, h := range r.groups {
 			if h == g && r.hasRunningMachine() {
-				fatalf(500, "InvalidGroup.InUse", "group is currently in use by a running instance")
+				fatalf(500, "DependencyViolation", "group is currently in use by a running instance")
 			}
 		}
 	}
@@ -1482,7 +1486,7 @@
 		}
 		for k := range sg.perms {
 			if k.group == g {
-				fatalf(500, "InvalidGroup.InUse", "group is currently in use by group %q", sg.id)
+				fatalf(500, "DependencyViolation", "group is currently in use by group %q", sg.id)
 			}
 		}
 	}

=== modified file 'ec2/export_test.go'
--- ec2/export_test.go	2014-08-11 22:08:47 +0000
+++ ec2/export_test.go	2014-11-27 15:29:26 +0000
@@ -15,5 +15,3 @@
 		timeNow = time.Now
 	}
 }
-
-var PrepareRunParams = prepareRunParams

=== modified file 'ec2/networkinterfaces.go'
--- ec2/networkinterfaces.go	2014-02-12 17:29:13 +0000
+++ ec2/networkinterfaces.go	2014-11-27 15:29:26 +0000
@@ -97,7 +97,7 @@
 //
 // See http://goo.gl/ze3VhA for more details.
 func (ec2 *EC2) CreateNetworkInterface(opts CreateNetworkInterface) (resp *CreateNetworkInterfaceResp, err error) {
-	params := makeParamsVPC("CreateNetworkInterface")
+	params := makeParams("CreateNetworkInterface")
 	params["SubnetId"] = opts.SubnetId
 	for i, ip := range opts.PrivateIPs {
 		prefix := fmt.Sprintf("PrivateIpAddresses.%d.", i+1)
@@ -127,7 +127,7 @@
 //
 // See http://goo.gl/MC1yOj for more details.
 func (ec2 *EC2) DeleteNetworkInterface(id string) (resp *SimpleResp, err error) {
-	params := makeParamsVPC("DeleteNetworkInterface")
+	params := makeParams("DeleteNetworkInterface")
 	params["NetworkInterfaceId"] = id
 	resp = &SimpleResp{}
 	err = ec2.query(params, resp)
@@ -153,7 +153,7 @@
 //
 // See http://goo.gl/2LcXtM for more details.
 func (ec2 *EC2) NetworkInterfaces(ids []string, filter *Filter) (resp *NetworkInterfacesResp, err error) {
-	params := makeParamsVPC("DescribeNetworkInterfaces")
+	params := makeParams("DescribeNetworkInterfaces")
 	for i, id := range ids {
 		params["NetworkInterfaceId."+strconv.Itoa(i+1)] = id
 	}
@@ -180,7 +180,7 @@
 //
 // See http://goo.gl/rEbSii for more details.
 func (ec2 *EC2) AttachNetworkInterface(interfaceId, instanceId string, deviceIndex int) (resp *AttachNetworkInterfaceResp, err error) {
-	params := makeParamsVPC("AttachNetworkInterface")
+	params := makeParams("AttachNetworkInterface")
 	params["NetworkInterfaceId"] = interfaceId
 	params["InstanceId"] = instanceId
 	params["DeviceIndex"] = strconv.Itoa(deviceIndex)
@@ -197,7 +197,7 @@
 //
 // See http://goo.gl/0Xc1px for more details.
 func (ec2 *EC2) DetachNetworkInterface(attachmentId string, force bool) (resp *SimpleResp, err error) {
-	params := makeParamsVPC("DetachNetworkInterface")
+	params := makeParams("DetachNetworkInterface")
 	params["AttachmentId"] = attachmentId
 	if force {
 		// Force is optional.

=== modified file 'ec2/networkinterfaces_test.go'
--- ec2/networkinterfaces_test.go	2014-08-11 22:08:47 +0000
+++ ec2/networkinterfaces_test.go	2014-11-27 15:29:26 +0000
@@ -9,10 +9,10 @@
 package ec2_test
 
 import (
+	"time"
+	. "gopkg.in/check.v1"
 	"launchpad.net/goamz/aws"
 	"launchpad.net/goamz/ec2"
-	. "gopkg.in/check.v1"
-	"time"
 )
 
 // Network interface tests with example responses
@@ -196,11 +196,14 @@
 	vpcId := vpcResp.VPC.Id
 	defer s.deleteVPCs(c, []string{vpcId})
 
-	subResp := s.createSubnet(c, vpcId, "10.3.1.0/24", "")
+	// Set AZ to us-east-1c explicitly, as t1.micro is not supported
+	// on us-east-1e, if it happens to get picked up randomly during
+	// the live test.
+	subResp := s.createSubnet(c, vpcId, "10.3.1.0/24", "us-east-1c")
 	subId := subResp.Subnet.Id
 	defer s.deleteSubnets(c, []string{subId})
 
-	sg := s.makeTestGroupVPC(c, vpcId, "vpc-sg-1", "vpc test group1")
+	sg := s.makeTestGroup(c, vpcId, "vpc-sg-1", "vpc test group1")
 	defer s.deleteGroups(c, []ec2.SecurityGroup{sg})
 
 	instList, err := s.ec2.RunInstances(&ec2.RunInstances{

=== modified file 'ec2/privateips.go'
--- ec2/privateips.go	2014-02-12 17:29:13 +0000
+++ ec2/privateips.go	2014-11-27 15:29:26 +0000
@@ -24,7 +24,7 @@
 //
 // See http://goo.gl/MoeH0L more details.
 func (ec2 *EC2) AssignPrivateIPAddresses(interfaceId string, ipAddresses []string, secondaryIPCount int, allowReassignment bool) (resp *SimpleResp, err error) {
-	params := makeParamsVPC("AssignPrivateIpAddresses")
+	params := makeParams("AssignPrivateIpAddresses")
 	params["NetworkInterfaceId"] = interfaceId
 	if secondaryIPCount > 0 {
 		params["SecondaryPrivateIpAddressCount"] = strconv.Itoa(secondaryIPCount)
@@ -51,7 +51,7 @@
 //
 // See http://goo.gl/RjGZdB for more details.
 func (ec2 *EC2) UnassignPrivateIPAddresses(interfaceId string, ipAddresses []string) (resp *SimpleResp, err error) {
-	params := makeParamsVPC("UnassignPrivateIpAddresses")
+	params := makeParams("UnassignPrivateIpAddresses")
 	params["NetworkInterfaceId"] = interfaceId
 	for i, ip := range ipAddresses {
 		// PrivateIpAddress is zero indexed.

=== modified file 'ec2/privateips_test.go'
--- ec2/privateips_test.go	2014-08-11 22:08:47 +0000
+++ ec2/privateips_test.go	2014-11-27 15:29:26 +0000
@@ -12,9 +12,9 @@
 	"strings"
 	"time"
 
+	. "gopkg.in/check.v1"
 	"launchpad.net/goamz/aws"
 	"launchpad.net/goamz/ec2"
-	. "gopkg.in/check.v1"
 )
 
 // Private IP tests with example responses
@@ -59,7 +59,10 @@
 	vpcId := vpcResp.VPC.Id
 	defer s.deleteVPCs(c, []string{vpcId})
 
-	subResp := s.createSubnet(c, vpcId, "10.7.1.0/24", "")
+	// Set AZ to us-east-1c explicitly, as m1.medium is not supported
+	// on us-east-1e, if it happens to get picked up randomly during
+	// the live test.
+	subResp := s.createSubnet(c, vpcId, "10.7.1.0/24", "us-east-1c")
 	subId := subResp.Subnet.Id
 	defer s.deleteSubnets(c, []string{subId})
 

=== modified file 'ec2/subnets.go'
--- ec2/subnets.go	2014-02-12 13:54:15 +0000
+++ ec2/subnets.go	2014-11-27 15:29:26 +0000
@@ -51,7 +51,7 @@
 //
 // See http://goo.gl/wLPhfI for more details.
 func (ec2 *EC2) CreateSubnet(vpcId, cidrBlock, availZone string) (resp *CreateSubnetResp, err error) {
-	params := makeParamsVPC("CreateSubnet")
+	params := makeParams("CreateSubnet")
 	params["VpcId"] = vpcId
 	params["CidrBlock"] = cidrBlock
 	if availZone != "" {
@@ -70,7 +70,7 @@
 //
 // See http://goo.gl/KmhcBM for more details.
 func (ec2 *EC2) DeleteSubnet(id string) (resp *SimpleResp, err error) {
-	params := makeParamsVPC("DeleteSubnet")
+	params := makeParams("DeleteSubnet")
 	params["SubnetId"] = id
 	resp = &SimpleResp{}
 	err = ec2.query(params, resp)
@@ -94,7 +94,7 @@
 //
 // See http://goo.gl/NTKQVI for more details.
 func (ec2 *EC2) Subnets(ids []string, filter *Filter) (resp *SubnetsResp, err error) {
-	params := makeParamsVPC("DescribeSubnets")
+	params := makeParams("DescribeSubnets")
 	for i, id := range ids {
 		params["SubnetId."+strconv.Itoa(i+1)] = id
 	}

=== modified file 'ec2/vpc.go'
--- ec2/vpc.go	2014-02-12 13:42:33 +0000
+++ ec2/vpc.go	2014-11-27 15:29:26 +0000
@@ -44,7 +44,7 @@
 //
 // See http://goo.gl/nkwjvN for more details.
 func (ec2 *EC2) CreateVPC(CIDRBlock, instanceTenancy string) (resp *CreateVPCResp, err error) {
-	params := makeParamsVPC("CreateVpc")
+	params := makeParams("CreateVpc")
 	params["CidrBlock"] = CIDRBlock
 	if instanceTenancy == "" {
 		instanceTenancy = "default"
@@ -66,7 +66,7 @@
 //
 // See http://goo.gl/bcxtbf for more details.
 func (ec2 *EC2) DeleteVPC(id string) (resp *SimpleResp, err error) {
-	params := makeParamsVPC("DeleteVpc")
+	params := makeParams("DeleteVpc")
 	params["VpcId"] = id
 	resp = &SimpleResp{}
 	err = ec2.query(params, resp)
@@ -90,7 +90,7 @@
 //
 // See http://goo.gl/Y5kHqG for more details.
 func (ec2 *EC2) VPCs(ids []string, filter *Filter) (resp *VPCsResp, err error) {
-	params := makeParamsVPC("DescribeVpcs")
+	params := makeParams("DescribeVpcs")
 	for i, id := range ids {
 		params["VpcId."+strconv.Itoa(i+1)] = id
 	}