Delegation #130
Delegation #130
Conversation
Create new fucntions for delegation, Add comments for functions without comments.
topLevelManifests will be changed based on local storage, so that delegated target roles' metadate will be fetched and committed
Added Delegation type to data/types.go
Add checking condition in repo.go/Init Add more choices to db.go/Verify Add Comments
Revised ReadMe
delete copyRepo from generator.go; fix bug in interop_test
-delete extra item; delete related codes in repo.go & test; regenerates testdata;
Pull Request Test Coverage Report for Build 516738297
💛 - Coveralls |
|
Happy New Year to you too, @SimonMen65. Please fix the git history in your PR and let us know when this is ready for review. |
|
@lukpueh Can you be more specific about "fix git history"? I'm confused if you mean there are some lab guidelines I didn't follow or something else. Thanks! |
|
Did you take a look at the commits pane of your PR? Does anything in that commit history look odd to you and/or remind you of something mentioned very prominently in the git history guideline document? |
|
@lukpueh I have read the git history guideline and I think it's ready to review. Leave comments if there's anything wrong. |
|
The TL;DR of the git history guideline document says, "avoid merge commits in your feature branch". If you look at your git history in the commits pane of this PR, you can see that there is a merge commit, which is also the reason why most of your commits appear twice, which is what I meant when I asked you if anything looked odd to you. |
|
Hmm looks strange, because I didn't use "merge" command but used "rebase" command and now it appears to be a merge. |
SimonMen65
force-pushed
the
delegation
branch
from
7e9899a
to
e779461
Compare
delete unneeded code in types.go
SimonMen65
force-pushed
the
delegation
branch
from
22b05ce
to
84979ef
Compare
|
@lukpueh Now I reset to previous versions and it turns out to be wired. In the checks I saw that they collapsed during "format Unix" stage and I have no idea what happened, nor did the raw log had given me any clues. The tests have all passed at my local so it shouldn't be the problem of codes. I've also checked go.sum with theupdateframework:master:go.sum and it seems the same. Hop you can give me a clue of this clash. |
|
Problem solved, simply because didn't format the code; already turned on "Format On Save" mode. I think you can start review the code |
There was a problem hiding this comment.
Great work! I left a couple of comments inline, but I also have some more general comments/questions:
- I'm not sure if this will support nested delegations (for example if targets delegates to role A who further delegates to role B).
- It might be nice to generate some test metadata that includes delegations, just to make sure everything works as expected
|
|
||
| //DelegateAddTargetsWithExpires add targets to non-top target role with expire date | ||
| func (r *Repo) DelegateAddTargetsWithExpires(nameJSON string, paths []string, custom json.RawMessage, expires time.Time) error { | ||
| if !strings.Contains(nameJSON, ".json") { |
There was a problem hiding this comment.
I'd also make sure the '.json' is at the end of the string, here and elsewhere in this file
| return err | ||
| } | ||
|
|
||
| for name := range targ.Roles { |
There was a problem hiding this comment.
What if delegated roles have delegations?
| @@ -208,11 +244,13 @@ func (f TargetFileMeta) HashAlgorithms() []string { | |||
| } | |||
|
|
|||
| type Targets struct { | |||
There was a problem hiding this comment.
should this also include a Delegations JSON object? It seems like keys are added into a Keys JSON dictionary, rather than nested in a Delegations object like these examples:
https://github.com/theupdateframework/tuf/blob/f4ffb9dbaabfa6e0df22091b7a6ca63f0dec9377/tests/repository_data/repository/metadata/targets.json#L10-L11
|
Closing in favor of #175. Thanks for all your hard work, Simon, I think they were useful for that PR! |
Happy New Year Everyone!
In this PR I added the delegation functionality to the project, details are basically described in commit messages.
The branch has passed all tests that & already rebased on the latest theupdateframework:master.
There are 198 files being changed but most of them are regenerated testdata by using client/testdata/go-tuf/regenerate-metadata.sh
The basic idea of adding this functionality is by changing the Target data-struct similar to Root data-struct, which enables Target role to delegate other target roles.
Readme is updated which contains detailed procedure of add/remove delegations, add/remove target files to delegated target roles.