Code review comment for lp://staging/~cmars/juju-core/ecdsa-tls

Revision history for this message
Casey Marshall (cmars) wrote :

Please take a look.

https://codereview.appspot.com/100400043/diff/10001/cert/cert.go
File cert/cert.go (right):

https://codereview.appspot.com/100400043/diff/10001/cert/cert.go#newcode228
cert/cert.go:228: SerialNumber: new(big.Int),
On 2014/05/13 07:21:18, fwereade wrote:
> If we're touching this code, can we please fix this as well?

Done. Will use a random 160-bit value here (max allowed by the spec).

https://codereview.appspot.com/100400043/diff/10001/cert/cert_test.go
File cert/cert_test.go (right):

https://codereview.appspot.com/100400043/diff/10001/cert/cert_test.go#newcode98
cert/cert_test.go:98: //c.Assert(caCert.MaxPathLen, Equals, 0) TODO it
ends up as -1 - check that this is ok.
On 2014/05/13 07:21:19, fwereade wrote:
> I hadn't noticed this before -- would you please figure out what's the
deal
> here?

Known issue, should be fixed in Go 1.3. Commented the test.

https://codereview.appspot.com/100400043/diff/10001/environs/cloudinit.go
File environs/cloudinit.go (right):

https://codereview.appspot.com/100400043/diff/10001/environs/cloudinit.go#newcode146
environs/cloudinit.go:146: return errgo.Annotate(err, "cannot generate
state server certificate")
On 2014/05/13 07:21:19, fwereade wrote:
> We're using juju.errors now, I think? But I haven't checked whether
thumper's
> changes have landed yet, please verify.

Need to merge w/latest trunk.

https://codereview.appspot.com/100400043/

« Back to merge proposal