Refactor createtree command #788
Conversation
RJPercival
force-pushed
the
cmd_createtree_refactor
branch
from
e3ec3ab to
389f383
Compare
cmd/createtree/main_test.go
Outdated
| TreeType: trillian.TreeType_LOG, | ||
| HashStrategy: trillian.HashStrategy_RFC6962_SHA256, | ||
| HashAlgorithm: sigpb.DigitallySigned_SHA256, | ||
| SignatureAlgorithm: sigpb.DigitallySigned_RSA, |
There was a problem hiding this comment.
Suggestion: default to ECDSA? Isn't that what the cool kids use? ;)
Ps: don't forget about nonDefaultTree below.
There was a problem hiding this comment.
I agree that ECDSA should be the default, but is it reasonable to change the default given that this flag has existed for a while? It could surprise some people if they're currently assuming that they'll get RSA trees if they don't provide this flag.
There was a problem hiding this comment.
If we would like to do it, better now than later. I expect that, if you care about the key type, you'll state it explicitly.
It's only a suggestion, though. Don't worry too much about it.
cmd/createtree/pem_test.go
Outdated
| pemPath, pemPassword := "../../testdata/log-rpc-server.privkey.pem", "towel" | ||
|
|
||
| wantTree := *defaultTree | ||
| keyProto, err := ptypes.MarshalAny(&keyspb.PEMKeyFile{ |
There was a problem hiding this comment.
Suggestion: wantTree.PrivateKey = mustMarshalAny(...)
cmd/createtree/pem_test.go
Outdated
| if err != nil { | ||
| t.Fatalf("MarshalAny(PrivateKey): %v", err) | ||
| } | ||
| wantTree.PrivateKey = keyProto |
There was a problem hiding this comment.
Ditto (mustMarshalAny).
So on for other tests.
util/flagsaver/flagsaver_test.go
Outdated
| import ( | ||
| "testing" | ||
| "time" | ||
|
|
There was a problem hiding this comment.
nit: Remove blank line and gofmt.
This makes it easy to save and restore flag values, e.g. during a test. See https://github.com/gflags/gflags/blob/652651b421ca5ac7b722a34a301fb656deca5198/src/gflags.h.in#L251 for prior art (a C++ implementation in GFlags).
The createOpts struct makes it difficult to have some flags/features behind build tags. This change also breaks out the key-format-specific test cases into their own files, so that they can be placed behind a build tag if necessary (e.g. to remove the PKCS#11 dependency).
RJPercival
force-pushed
the
cmd_createtree_refactor
branch
from
027c264 to
d22dc8f
Compare
RJPercival
force-pushed
the
cmd_createtree_refactor
branch
from
d22dc8f to
26f59cf
Compare
3 participants
This simplifies PR #734, which requires putting the PKCS#11 flag used by createtree behind a build tag.