cmd/jujud: accept CA cert flag

cmd/jujud: accept CA cert flag



https://code.launchpad.net/~rogpeppe/juju-core/166-juju-cert-flag/+merge/135907

(do not edit description out of merge proposal)

[rog, 2012-11-23 14:55:02]

Please take a look.

[rog, 2012-11-23 18:43:28]

Please take a look.

[niemeyer, 2012-11-26 20:12:21]

Looking quite nice. Only a few simple ideas:

https://codereview.appspot.com/6842088/diff/7002/cmd/jujud/agent.go
File cmd/jujud/agent.go (right):

https://codereview.appspot.com/6842088/diff/7002/cmd/jujud/agent.go#newcode49
cmd/jujud/agent.go:49: // stateServersVar sets up a gnuflag flag analagously to
FlagSet.*Var methods.
s/analagously/analogous/

https://codereview.appspot.com/6842088/diff/7002/cmd/jujud/agent.go#newcode82
cmd/jujud/agent.go:82: f.StringVar(&c.caCertFile, "ca-cert-file", "", "file name
of CA certificate in PEM format")
s/-file//; (it's --config, not --config-file, etc)
s/file name of/path to/

Also, cmd/filevar.go would make this nicer I think.

https://codereview.appspot.com/6842088/diff/7002/cmd/jujud/agent.go#newcode103
cmd/jujud/agent.go:103: c.CACertPEM, err = ioutil.ReadFile(c.caCertFile)
Why do we have a field for this when we have c.StateInfo.CACert here?

https://codereview.appspot.com/6842088/diff/7002/juju/bootstrap.go
File juju/bootstrap.go (right):

https://codereview.appspot.com/6842088/diff/7002/juju/bootstrap.go#newcode78
juju/bootstrap.go:78: MaxPathLen: 0, // Disallow delegation for now.
:-)

[dave_cheney.net, 2012-11-27 00:44:30]

LGTM

[rog, 2012-11-27 14:19:46]

Please take a look.

https://codereview.appspot.com/6842088/diff/7002/cmd/jujud/agent.go
File cmd/jujud/agent.go (right):

https://codereview.appspot.com/6842088/diff/7002/cmd/jujud/agent.go#newcode49
cmd/jujud/agent.go:49: // stateServersVar sets up a gnuflag flag analagously to
FlagSet.*Var methods.
On 2012/11/26 20:12:21, niemeyer wrote:
> s/analagously/analogous/

Done.

https://codereview.appspot.com/6842088/diff/7002/cmd/jujud/agent.go#newcode82
cmd/jujud/agent.go:82: f.StringVar(&c.caCertFile, "ca-cert-file", "", "file name
of CA certificate in PEM format")
On 2012/11/26 20:12:21, niemeyer wrote:
> s/-file//; (it's --config, not --config-file, etc)
> s/file name of/path to/

done.

> Also, cmd/filevar.go would make this nicer I think.

not done, as it actually makes it not so nice, as reading the certificate file
requires a cmd Context, which we don't have until Run is called, so we'd need to
make each agent aware how a piece of the state info is generated from the flags,
which seems to me like a step backwards.

please say if you think this is what should happen anyway.
(AFAICS the only thing that FileVar offers is that it checks for an empty
string, and reads relative to the context's cwd, the former is trivial and the
latter we do and the latter we don't need for agents)

https://codereview.appspot.com/6842088/diff/7002/cmd/jujud/agent.go#newcode103
cmd/jujud/agent.go:103: c.CACertPEM, err = ioutil.ReadFile(c.caCertFile)
On 2012/11/26 20:12:21, niemeyer wrote:
> Why do we have a field for this when we have c.StateInfo.CACert here?

the branch that adds that field has only just landed, and i've been trying to
minimise prereqs.

[niemeyer, 2012-11-27 14:24:21]

LGTM, thanks!

[rog, 2012-11-27 14:31:08]

*** Submitted:

cmd/jujud: accept CA cert flag

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