#806 Part 2: Cleanup KeyStore Implementations #808
Conversation
endophage
force-pushed
the
cleanup_keystore
branch
from
045624d to
4d55f19
Compare
endophage
changed the title
endophage
force-pushed
the
cleanup_keystore
branch
6 times, most recently
from
890112a to
0771aa3
Compare
endophage
changed the title
endophage
force-pushed
the
cleanup_keystore
branch
2 times, most recently
from
bfc90c9 to
1ff0f8c
Compare
| @@ -682,7 +682,7 @@ func (r *NotaryRepository) bootstrapRepo() error { | |||
| logrus.Debugf("Loading trusted collection.") | |||
|
|
|||
| for _, role := range data.BaseRoles { | |||
| jsonBytes, err := r.fileStore.GetMeta(role, store.NoSizeLimit) | |||
| jsonBytes, err := r.fileStore.GetSized(role, store.NoSizeLimit) | |||
There was a problem hiding this comment.
we can do this in a separate PR, but should we phase out store.NoSizeLimit and change these calls to Get() instead of GetSized()?
There was a problem hiding this comment.
Yeah. I figured we'd do that in a different PR. There's already so much in here and that's an important change with meaningful TUF security implications if we change it somewhere we shouldn't, so a smaller PR with just that seems more sensible.
| @@ -26,14 +158,14 @@ func TestAddFile(t *testing.T) { | |||
| expectedFilePath := filepath.Join(tempBaseDir, testName+testExt) | |||
|
|
|||
| // Create our SimpleFileStore | |||
There was a problem hiding this comment.
nit: out of date comments for SimpleFileStore (161, 193, 229)
Signed-off-by: David Lawrence <dclwrnc@gmail.com> (github: endophage)
Signed-off-by: David Lawrence <david.lawrence@docker.com> (github: endophage)
endophage
force-pushed
the
cleanup_keystore
branch
from
1ff0f8c to
1e67617
Compare
| } | ||
|
|
||
| // Error implements error interface | ||
| func (e ErrWrongHash) Error() string { |
There was a problem hiding this comment.
I think all these errors can also be removed, since I can't seem to find anywhere they're being used?
|
LGTM! I think there are some minor cleanups that can happen in a future PR, but this an awesome improvement and simplification of the keystore code! |
|
this is great, parts 1&2 LGTM |
Depends on #807 (the actual diff after merging that PR will be about +1400 -1800)
There are a lot of changes here, my apologies for the size of the commit but a good chunk of it is 1:1 changes that just deal with imports that moved around.
Main items:
/tuf/store/*moved tostorage/trustmanager.KeyFileStoreandtrustmanager.KeyMemoryStoremerged intotrustmanager.GenericKeyStore. They had identical functionality with different underlying storage.trustmanager.SimpleFileStoreunified withtuf.store.FilesystemStoreand now lives atstorage.FilesystemStore.trustmanager.SimpleFileStoretests have been preserved instorage/filestore_test.goand point atFilesystemStore.trustmanager.MemoryStoreunified withtuf.store.MemoryStoreand now lives atstorage.MemoryStore.trustmanager.MemoryStorehad no tests.tuf.utilshaving a dependency (an import) ontrustmanager. Thetrustmanager/x509utils.gocode it was depending on for theutils.CanonicalKeyIDwas standalone (it had dependencies only on golang std lib packages) and has been moved intotuf.utilsGeneral Cleanup:
LocalStorage,GetTarget(legacy from original flynn/go-tuf storage interface, notary does not provide a way to get targets).httpstoretests were doing verification of signatures. Where possible those tests have been moved intotuf.signed. In other places the verification has been replaced with a simple equality tests that more directly testshttpstorebehaviour.tuf/utils/util.goandtuf/utils/utils.gointo a single file of the latter name. Similarly unified test files.