Code review comment for lp://staging/~rogpeppe/juju-core/151-cloudinit-root-cert

Reviewers: mp+134498_code.launchpad.net,

Message:
Please take a look.

Description:
environs/cloudinit: add RootCertPEM

Instances need to know the root CA certificate
so that they can verify the server they're talking to.

https://code.launchpad.net/~rogpeppe/juju-core/151-cloudinit-root-cert/+merge/134498

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/6854054/

Affected files:
   A [revision details]
   M environs/cloudinit/cloudinit.go
   M environs/cloudinit/cloudinit_test.go
   M environs/ec2/ec2.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: environs/cloudinit/cloudinit.go
=== modified file 'environs/cloudinit/cloudinit.go'
--- environs/cloudinit/cloudinit.go 2012-11-14 14:09:56 +0000
+++ environs/cloudinit/cloudinit.go 2012-11-15 16:11:30 +0000
@@ -29,6 +29,10 @@
   // or MongoDB instance.
   StateServer bool

+ // RootCertPEM holds the x509 certificate of the root CA
+ // that signed the state server certificate, in PEM format.
+ RootCertPEM []byte
+
   // StateServerPEM holds the state server certificate and private
   // key in PEM format; it is required when StateServer is set,
   // and ignored otherwise.
@@ -85,7 +89,10 @@
   return base64.StdEncoding.EncodeToString(data)
  }

-const serverPEMPath = "/var/lib/juju/server.pem"
+const (
+ serverPEMPath = "/var/lib/juju/server.pem"
+ rootCertPEMPath = "/var/lib/juju/root.pem"
+)

  func New(cfg *MachineConfig) (*cloudinit.Config, error) {
   if err := verifyConfig(cfg); err != nil {
@@ -115,6 +122,10 @@
    debugFlag = " --debug"
   }

+ addScripts(c,
+ fmt.Sprintf("echo %s > %s", shquote(string(cfg.RootCertPEM)),
rootCertPEMPath),
+ )
+
   if cfg.StateServer {
    addScripts(c,
     fmt.Sprintf("echo %s > %s",
@@ -280,6 +291,9 @@
   if cfg.StateInfo == nil {
    return fmt.Errorf("missing state info")
   }
+ if len(cfg.RootCertPEM) == 0 {
+ return fmt.Errorf("missing root CA certificate PEM")
+ }
   if cfg.StateServer {
    if cfg.InstanceIdAccessor == "" {
     return fmt.Errorf("missing instance id accessor")

Index: environs/cloudinit/cloudinit_test.go
=== modified file 'environs/cloudinit/cloudinit_test.go'
--- environs/cloudinit/cloudinit_test.go 2012-11-14 14:09:56 +0000
+++ environs/cloudinit/cloudinit_test.go 2012-11-15 16:11:30 +0000
@@ -44,6 +44,7 @@
    Tools: newSimpleTools("1.2.3-linux-amd64"),
    StateServer: true,
    StateServerPEM: serverPEM,
+ RootCertPEM: rootCertPEM,
    StateInfo: &state.Info{
     Password: "arble",
    },
@@ -56,6 +57,7 @@
    AuthorizedKeys: "sshkey1",
    DataDir: "/var/lib/juju",
    StateServer: false,
+ RootCertPEM: rootCertPEM,
    Tools: newSimpleTools("1.2.3-linux-amd64"),
    StateInfo: &state.Info{
     Addrs: []string{"state-addr.example.com"},
@@ -260,6 +262,13 @@
   {"missing state server PEM", func(cfg *cloudinit.MachineConfig) {
    cfg.StateServerPEM = []byte{}
   }},
+ {"missing root CA certificate PEM", func(cfg *cloudinit.MachineConfig) {
+ cfg.RootCertPEM = []byte{}
+ }},
+ {"missing root CA certificate PEM", func(cfg *cloudinit.MachineConfig) {
+ cfg.StateServer = false
+ cfg.RootCertPEM = []byte{}
+ }},
   {"missing var directory", func(cfg *cloudinit.MachineConfig) {
    cfg.DataDir = ""
   }},
@@ -303,6 +312,7 @@
   cfg := &cloudinit.MachineConfig{
    StateServer: true,
    StateServerPEM: serverPEM,
+ RootCertPEM: rootCertPEM,
    InstanceIdAccessor: "$instance_id",
    ProviderType: "ec2",
    MachineId: 99,
@@ -349,3 +359,17 @@
  2Vc9NN09ty+HZgxpwqIA1fHVuYJY9GMPG1LnTnZ9INg=
  -----END RSA PRIVATE KEY-----
  `)
+
+var rootCertPEM = []byte(`
+-----BEGIN CERTIFICATE-----
+MIIBmjCCAUagAwIBAgIBADALBgkqhkiG9w0BAQUwJjENMAsGA1UEChMEanVqdTEV
+MBMGA1UEAxMManVqdSB0ZXN0aW5nMB4XDTEyMTExNTE1NDAyMFoXDTIyMTExNTE1
+NDUyMFowJjENMAsGA1UEChMEanVqdTEVMBMGA1UEAxMManVqdSB0ZXN0aW5nMFow
+CwYJKoZIhvcNAQEBA0sAMEgCQQCQGR9Sj2dsNuBF5R7gxcg28EJu0yqxrqILAYkH
+PN/A4OBFIRAeG5Q7O1RddW4pdCAv8Qj3bsRvaxwoxwmcAnD9AgMBAAGjYzBhMA4G
+A1UdDwEB/wQEAwIABDAPBgNVHRMBAf8EBTADAgEBMB0GA1UdDgQWBBT5jDkLWX0S
+xPRuee2ULoDrfiyMMTAfBgNVHSMEGDAWgBT5jDkLWX0SxPRuee2ULoDrfiyMMTAL
+BgkqhkiG9w0BAQUDQQAEpl0ITRktd6uNklg9KduXQqwG/WQ6NVbK48UMkoKHnNya
+lJPzSvbW7xnggA81a6efihNr93uVJqf+NbX0+bO/
+-----END CERTIFICATE-----
+`)

Index: environs/ec2/ec2.go
=== modified file 'environs/ec2/ec2.go'
--- environs/ec2/ec2.go 2012-11-14 14:39:40 +0000
+++ environs/ec2/ec2.go 2012-11-15 16:11:30 +0000
@@ -324,11 +324,11 @@
  }

  func (e *environ) userData(scfg *startInstanceParams) ([]byte, error) {
-
   cfg := &cloudinit.MachineConfig{
    StateServer: scfg.stateServer,
    StateInfo: scfg.info,
    StateServerPEM: scfg.stateServerPEM,
+ RootCertPEM: []byte("fake root cert"), // TODO(rog): use proper
root cert.
    InstanceIdAccessor: "$(curl
http://169.254.169.254/1.0/meta-data/instance-id)",
    ProviderType: "ec2",
    DataDir: "/var/lib/juju",