Merge lp:~dave-cheney/juju-core/052-environs-ec2-always-request-public-tools-from-us-east-1 into lp:~juju/juju-core/trunk

[Dave Cheney]

environs/ec2: specify public-bucket-region

Fixes LP #1083017

Our public bucket, juju-dist, lives in the us-east-1 region and can only be accessed via that endpoint. If you attempt to do so via another endpoint you get an unhelpful 301 error with an xml blob telling you how to request the bucket. The solution is to add a configuration option, with sensible default, to ec2/config.go to specify the region of the public bucket independantly of the private bucket.

https://codereview.appspot.com/6843112/

[John A Meinel (jameinel)]

Seems ok. I'm not sure if having a configurable "grab the tools from
this region" is better than providing the tools in all regions. If this
is the best way forward, than LGTM.

https://codereview.appspot.com/6843112/diff/2001/environs/ec2/ec2.go
File environs/ec2/ec2.go (left):

https://codereview.appspot.com/6843112/diff/2001/environs/ec2/ec2.go#oldcode162
environs/ec2/ec2.go:162: }
It seems like if you've connected you might want to reuse s3Unlocked if
region == publicBucketRegion.

Beyond that, having to access us-east-1 from outside of it means paying
bandwidth charges, etc. Is it worthwhile to try and put the tools on all
regions?

https://codereview.appspot.com/6843112/

=== modified file 'environs/ec2/config.go'
--- environs/ec2/config.go	2012-11-16 08:04:53 +0000
+++ environs/ec2/config.go	2012-11-26 09:48:38 +0000
@@ -9,17 +9,19 @@
 
 var configChecker = schema.StrictFieldMap(
 	schema.Fields{
-		"access-key":     schema.String(),
-		"secret-key":     schema.String(),
-		"region":         schema.String(),
-		"control-bucket": schema.String(),
-		"public-bucket":  schema.String(),
+		"access-key":           schema.String(),
+		"secret-key":           schema.String(),
+		"region":               schema.String(),
+		"control-bucket":       schema.String(),
+		"public-bucket":        schema.String(),
+		"public-bucket-region": schema.String(),
 	},
 	schema.Defaults{
-		"access-key":    "",
-		"secret-key":    "",
-		"region":        "us-east-1",
-		"public-bucket": "juju-dist",
+		"access-key":           "",
+		"secret-key":           "",
+		"region":               "us-east-1",
+		"public-bucket":        "juju-dist",
+		"public-bucket-region": "us-east-1",
 	},
 )
 
@@ -40,6 +42,10 @@
 	return c.attrs["public-bucket"].(string)
 }
 
+func (c *environConfig) publicBucketRegion() string {
+	return c.attrs["public-bucket-region"].(string)
+}
+
 func (c *environConfig) accessKey() string {
 	return c.attrs["access-key"].(string)
 }
@@ -73,6 +79,9 @@
 	if _, ok := aws.Regions[ecfg.region()]; !ok {
 		return nil, fmt.Errorf("invalid region name %q", ecfg.region())
 	}
+	if _, ok := aws.Regions[ecfg.publicBucketRegion()]; !ok {
+		return nil, fmt.Errorf("invalid public-bucket-region name %q", ecfg.publicBucketRegion())
+	}
 
 	if old != nil {
 		attrs := old.UnknownAttrs()

=== modified file 'environs/ec2/ec2.go'
--- environs/ec2/ec2.go	2012-11-14 14:39:40 +0000
+++ environs/ec2/ec2.go	2012-11-26 09:48:38 +0000
@@ -148,6 +148,7 @@
 
 	auth := aws.Auth{ecfg.accessKey(), ecfg.secretKey()}
 	region := aws.Regions[ecfg.region()]
+	publicBucketRegion := aws.Regions[ecfg.publicBucketRegion()]
 	e.ec2Unlocked = ec2.New(auth, region)
 	e.s3Unlocked = s3.New(auth, region)
 
@@ -158,7 +159,7 @@
 	}
 	if ecfg.publicBucket() != "" {
 		e.publicStorageUnlocked = &storage{
-			bucket: e.s3Unlocked.Bucket(ecfg.publicBucket()),
+			bucket: s3.New(auth, publicBucketRegion).Bucket(ecfg.publicBucket()),
 		}
 	} else {
 		e.publicStorageUnlocked = nil

=== modified file 'environs/ec2/local_test.go'
--- environs/ec2/local_test.go	2012-11-23 18:08:08 +0000
+++ environs/ec2/local_test.go	2012-11-26 09:48:38 +0000
@@ -26,17 +26,18 @@
 		Name: "test",
 	}
 	attrs := map[string]interface{}{
-		"name":            "sample",
-		"type":            "ec2",
-		"region":          "test",
-		"control-bucket":  "test-bucket",
-		"public-bucket":   "public-tools",
-		"admin-secret":    "local-secret",
-		"access-key":      "x",
-		"secret-key":      "x",
-		"authorized-keys": "foo",
-		"ca-cert":         testing.CACertPEM,
-		"ca-private-key":  testing.CAKeyPEM,
+		"name":                 "sample",
+		"type":                 "ec2",
+		"region":               "test",
+		"control-bucket":       "test-bucket",
+		"public-bucket":        "public-tools",
+		"public-bucket-region": "test",
+		"admin-secret":         "local-secret",
+		"access-key":           "x",
+		"secret-key":           "x",
+		"authorized-keys":      "foo",
+		"ca-cert":              testing.CACertPEM,
+		"ca-private-key":       testing.CAKeyPEM,
 	}
 
 	Suite(&localServerSuite{

=== modified file 'juju/bootstrap.go'
--- juju/bootstrap.go	2012-11-23 18:18:38 +0000
+++ juju/bootstrap.go	2012-11-26 09:48:38 +0000
@@ -74,8 +74,8 @@
 		SubjectKeyId:          bigIntHash(priv.N),
 		KeyUsage:              x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign,
 		BasicConstraintsValid: true,
-		IsCA:                  true,
-		MaxPathLen:            0, // Disallow delegation for now.
+		IsCA:       true,
+		MaxPathLen: 0, // Disallow delegation for now.
 	}
 	certDER, err := x509.CreateCertificate(rand.Reader, template, template, &priv.PublicKey, priv)
 	if err != nil {