environs: add a certificate argument to Bootstrap

environs: add a certificate argument to Bootstrap



https://code.launchpad.net/~rogpeppe/juju-core/139-bootstrap-cert/+merge/133537

Requires: https://code.launchpad.net/~rogpeppe/juju-core/138-cloudinit-cert/+merge/133516

(do not edit description out of merge proposal)

[rog, 2012-11-08 18:19:57]

Please take a look.

[dave_cheney.net, 2012-11-09 00:24:26]

LGTM.

https://codereview.appspot.com/6819115/diff/2001/environs/interface.go
File environs/interface.go (right):

https://codereview.appspot.com/6819115/diff/2001/environs/interface.go#newcod...
environs/interface.go:136: Bootstrap(uploadTools bool, certAndKey []byte) error
I think the next time we have to add a param to Bootstrap, we should introduce a
BootStrapConfig type.

[rog, 2012-11-09 12:29:48]

https://codereview.appspot.com/6819115/diff/2001/environs/interface.go
File environs/interface.go (right):

https://codereview.appspot.com/6819115/diff/2001/environs/interface.go#newcod...
environs/interface.go:136: Bootstrap(uploadTools bool, certAndKey []byte) error
On 2012/11/09 00:24:26, dfc wrote:
> I think the next time we have to add a param to Bootstrap, we should introduce
a
> BootStrapConfig type.

+1

[rog, 2012-11-12 12:43:30]

Please take a look.

[niemeyer, 2012-11-14 13:59:06]

LGTM

https://codereview.appspot.com/6819115/diff/5001/environs/ec2/ec2.go
File environs/ec2/ec2.go (right):

https://codereview.appspot.com/6819115/diff/5001/environs/ec2/ec2.go#newcode210
environs/ec2/ec2.go:210: func (e *environ) Bootstrap(uploadTools bool,
certAndKey []byte) error {
s/certAndKey/stateServerPEM/

https://codereview.appspot.com/6819115/diff/5001/environs/ec2/ec2.go#newcode323
environs/ec2/ec2.go:323: master:    false,
The default is false.

https://codereview.appspot.com/6819115/diff/5001/environs/ec2/ec2.go#newcode351
environs/ec2/ec2.go:351: master     bool
stateServer

https://codereview.appspot.com/6819115/diff/5001/environs/ec2/ec2.go#newcode353
environs/ec2/ec2.go:353: certAndKey []byte
stateServerPEM

https://codereview.appspot.com/6819115/diff/5001/environs/interface.go
File environs/interface.go (right):

https://codereview.appspot.com/6819115/diff/5001/environs/interface.go#newcod...
environs/interface.go:135: // key are specified in PEM format.
s/certAndKey/stateServerPEM/

// The stateServerPEM parameter holds both the private key and the
// respective certificate to be used by the initial state server.

https://codereview.appspot.com/6819115/diff/5001/environs/jujutest/livetests.go
File environs/jujutest/livetests.go (right):

https://codereview.appspot.com/6819115/diff/5001/environs/jujutest/livetests....
environs/jujutest/livetests.go:26: ServerCertAndKey []byte
StateServerPEM

https://codereview.appspot.com/6819115/diff/5001/environs/jujutest/tests.go
File environs/jujutest/tests.go (right):

https://codereview.appspot.com/6819115/diff/5001/environs/jujutest/tests.go#n...
environs/jujutest/tests.go:25: ServerCertAndKey []byte
StateServerPEM

[TheMue, 2012-11-14 14:02:20]

LGTM, only small comments.

https://codereview.appspot.com/6819115/diff/5001/environs/ec2/ec2.go
File environs/ec2/ec2.go (right):

https://codereview.appspot.com/6819115/diff/5001/environs/ec2/ec2.go#newcode352
environs/ec2/ec2.go:352: config     *config.Config
config inside a config? Hmm, maybe startInstanceParams or something like that is
better.

https://codereview.appspot.com/6819115/diff/5001/environs/ec2/live_test.go
File environs/ec2/live_test.go (right):

https://codereview.appspot.com/6819115/diff/5001/environs/ec2/live_test.go#ne...
environs/ec2/live_test.go:60: ServerCertAndKey: []byte("fake cert"), // TODO
What is to do? Didn't we wanted to write 

// TODO(wrtp) Do this and that.

So in case of an unplanned absence a colleague easier could continue.

[rog, 2012-11-14 15:04:14]

*** Submitted:

environs: add a certificate argument to Bootstrap

R=dfc, niemeyer, TheMue
CC=
https://codereview.appspot.com/6819115

https://codereview.appspot.com/6819115/diff/5001/environs/ec2/ec2.go
File environs/ec2/ec2.go (right):

https://codereview.appspot.com/6819115/diff/5001/environs/ec2/ec2.go#newcode210
environs/ec2/ec2.go:210: func (e *environ) Bootstrap(uploadTools bool,
certAndKey []byte) error {
On 2012/11/14 13:59:07, niemeyer wrote:
> s/certAndKey/stateServerPEM/

Done.

https://codereview.appspot.com/6819115/diff/5001/environs/ec2/ec2.go#newcode323
environs/ec2/ec2.go:323: master:    false,
On 2012/11/14 13:59:07, niemeyer wrote:
> The default is false.

Done.

https://codereview.appspot.com/6819115/diff/5001/environs/ec2/ec2.go#newcode351
environs/ec2/ec2.go:351: master     bool
On 2012/11/14 13:59:07, niemeyer wrote:
> stateServer

done, but i'm not sure. being a state server is just one of the potential
attributes of the bootstrap instance, and that's what "master" signifies in this
context. given that the only field it affects in cloudinit.MachineConfig is
StateServer, it's probably ok, but might change back in the future.

https://codereview.appspot.com/6819115/diff/5001/environs/ec2/ec2.go#newcode352
environs/ec2/ec2.go:352: config     *config.Config
On 2012/11/14 14:02:20, TheMue wrote:
> config inside a config? Hmm, maybe startInstanceParams or something like that
is
> better.

Done.

https://codereview.appspot.com/6819115/diff/5001/environs/ec2/ec2.go#newcode353
environs/ec2/ec2.go:353: certAndKey []byte
On 2012/11/14 13:59:07, niemeyer wrote:
> stateServerPEM

Done.

https://codereview.appspot.com/6819115/diff/5001/environs/ec2/live_test.go
File environs/ec2/live_test.go (right):

https://codereview.appspot.com/6819115/diff/5001/environs/ec2/live_test.go#ne...
environs/ec2/live_test.go:60: ServerCertAndKey: []byte("fake cert"), // TODO
On 2012/11/14 14:02:20, TheMue wrote:
> What is to do? Didn't we wanted to write 
> 
> // TODO(wrtp) Do this and that.
> 
> So in case of an unplanned absence a colleague easier could continue.

Done.

https://codereview.appspot.com/6819115/diff/5001/environs/interface.go
File environs/interface.go (right):

https://codereview.appspot.com/6819115/diff/5001/environs/interface.go#newcod...
environs/interface.go:135: // key are specified in PEM format.
On 2012/11/14 13:59:07, niemeyer wrote:
> s/certAndKey/stateServerPEM/
> 
> // The stateServerPEM parameter holds both the private key and the
> // respective certificate to be used by the initial state server.

Done.

https://codereview.appspot.com/6819115/diff/5001/environs/jujutest/livetests.go
File environs/jujutest/livetests.go (right):

https://codereview.appspot.com/6819115/diff/5001/environs/jujutest/livetests....
environs/jujutest/livetests.go:26: ServerCertAndKey []byte
On 2012/11/14 13:59:07, niemeyer wrote:
> StateServerPEM

Done.

https://codereview.appspot.com/6819115/diff/5001/environs/jujutest/tests.go
File environs/jujutest/tests.go (right):

https://codereview.appspot.com/6819115/diff/5001/environs/jujutest/tests.go#n...
environs/jujutest/tests.go:25: ServerCertAndKey []byte
On 2012/11/14 13:59:07, niemeyer wrote:
> StateServerPEM

Done.