Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: Add ngclient root key rotation tests #1635

Merged
merged 3 commits into from
Oct 27, 2021

Conversation

jku
Copy link
Member

@jku jku commented Oct 26, 2021

Add 15 key rotation cases to ngclient tests.

Maybe fixes #1607? This only tests root keys but I think this is the tricky part: maybe there should be a small test for other keys as well...

  • The code follows the Code Style Guidelines
  • Tests have been added for the bug fix or new feature
  • Docs have been added for the bug fix or new feature

@coveralls
Copy link

coveralls commented Oct 26, 2021

Pull Request Test Coverage Report for Build 1390950472

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 97.43%

Totals Coverage Status
Change from base Build 1390869159: 0.0%
Covered Lines: 3946
Relevant Lines: 4030

💛 - Coveralls

@jku jku requested a review from sechkova October 26, 2021 07:24
Comment on lines 143 to 146
def add_signer(self, role:str, signer: SSlibSigner):
if role not in self.signers:
self.signers[role] = {}
self.signers[role][signer.key_dict["keyid"]] = signer
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, indent issue here

@jku
Copy link
Member Author

jku commented Oct 26, 2021

I should also fix some linter issues if we're going to enable the linter on tests...

Copy link
Contributor

@sechkova sechkova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks ok to me, I didn't hit the approve button so that you fix the indent and the commits creator :)

# Copyright 2021, New York University and the TUF contributors
# SPDX-License-Identifier: MIT OR Apache-2.0

"""Test ngclient Updater using the repository simulator
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: the docstring needs to be updated.

RootVersion([0,1,2,3,4], 3, [0,2,4]),
RootVersion([0,1,3,4,5], 3, [0,4,1]),
],
"3-of-5, one key rotate fails: not signed with 3 keys from last root" : [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: "from new root"

@sechkova
Copy link
Contributor

ah and yeah, I think we need to add smaller tests for other keys as well, not necessarily with this PR.

Jussi Kukkonen added 3 commits October 27, 2021 18:57
Store signers with their keyids so they are easier to remove.
The signers structure now looks like:
{
  "role1": {
    "keyidA": SSlibSigner,
    "keyidB": SSlibSigner,
  }
}

Add convenience method for adding a signer.

Signed-off-by: Jussi Kukkonen <jkukkonen@vmware.com>
Signed-off-by: Jussi Kukkonen <jkukkonen@vmware.com>
Add one test with 1 subtests for various root key rotation situations.

The test data definition format is a bit tricky but I tried to document
that in the test function docstring.

Signed-off-by: Jussi Kukkonen <jkukkonen@vmware.com>
@jku
Copy link
Member Author

jku commented Oct 27, 2021

The last push is just rebase on recent RepositorySimulator changes.

I should also fix some linter issues if we're going to enable the linter on tests...

Not promising it's 100% there (hard to test without merging with martins PR), but most of the issues should be handled: I'll merge this with the current approval. @MVrachev if there's something exceptionally annoying when you try rebasing, let me know

@jku jku merged commit 589ed9e into theupdateframework:develop Oct 27, 2021
@lukpueh lukpueh mentioned this pull request Dec 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tests: Updater key rotation tests
3 participants