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

Improve testing using generated metadata #1444

Closed
jku opened this issue Jun 14, 2021 · 11 comments · Fixed by #1569
Closed

Improve testing using generated metadata #1444

jku opened this issue Jun 14, 2021 · 11 comments · Fixed by #1569
Assignees
Labels
backlog Issues to address with priority for current development goals testing
Milestone

Comments

@jku
Copy link
Member

jku commented Jun 14, 2021

This is an umbrella issue for a plan I have:

  • Improve Metadata API for the case of Metadata generation (in other words, creating Metadata not deserializing it)
  • Make sure all of it can be done in-memory (so writing things to disk is not needed)
  • Improve testing for Metadata API itself by addding tests with generated data (see e.g. Metadata API: Implement threshold verification #1436: testing signature validation with various thresholds)
  • Improve testing for ngclient/MetadataBundle by addding tests with generated data

The reason I want to do this is that including metadata files in the test data seems hard to scale: We should aim to test de/serialization with test data included with the test (as in #1391), and then generate the test data for the cases where this is impractical (like ngclient MetadataBundle tests).

In the end I believe the improvements will be beneficial for the future repository tools as well, but that's a longer term goal: the above list is something where we could have results in a few weeks.

For background below is what I came up with for generating full metadata for a repository and modifying it just a bit using the current Metadata API (it currently writes to files but just for debugging purposes: to_file() calls can be removed). There's a lot of boilerplate but also quite a few low hanging fruit that would improve the experience:

from collections import OrderedDict
from datetime import datetime
from typing import Tuple

from securesystemslib.keys import generate_ed25519_key
from securesystemslib.signer import SSlibSigner, Signer
from tuf.api.metadata import DelegatedRole, Delegations, Key, MetaFile, Metadata, Role, Root, Snapshot, TargetFile, Targets, Timestamp

# todo should come from metadata
SPEC_VERSION = "1.0.999"

def generate_test_key() -> Tuple[Key, Signer]:
    keydict = generate_ed25519_key()
    # TODO maybe this should be Key.from_securesystemslib_key()
    key = Key(keydict["keyid"], keydict["keytype"], keydict["scheme"], {"public": keydict["keyval"]["public"]})
    signer = SSlibSigner(keydict)

    return (key, signer)

# generate keys
delegate_key, delegate_signer = generate_test_key()
targets_key, targets_signer = generate_test_key()
snapshot_key, snapshot_signer = generate_test_key()
timestamp_key, timestamp_signer = generate_test_key()
root_key, root_signer = generate_test_key()


# Create delegated targets
# TODO Metadata.new_targets_with_defaults(expires: datetime) ?
# TODO Add TargetFile.from_data(data: Union[bytes, BinaryIO]) so this works:
#     delegate = Metadata.new_targets_with_defaults(datetime(3000,1,1))
#     delegate.targets["world"] = TargetFile.from_data(b"hello")
#     delegate.sign(delegate_signer)
targetfile = TargetFile(5, {"sha256": "5891b5b522d5df086d0ff0b110fbd9d21bb4fc7163af34d08286a2e846f6be03"})
delegate_signed = Targets(1, SPEC_VERSION, datetime(3000,1,1), {"world": targetfile}, None)
delegate = Metadata(delegate_signed, OrderedDict())
delegate.sign(delegate_signer)
# TODO: Metadata.to_file should be able to optionally handle versioned file names
delegate.to_file("delegate.json")

# Create targets
# TODO: better API to modify delegations? The correct one is not obvious though.
# One option is to add add_key/remove_key() to delegations as well (see Root)
role = DelegatedRole("delegate", [delegate_key.keyid], 1, True)
delegations = Delegations({delegate_key.keyid: delegate_key}, [role])
targets_signed = Targets(1, SPEC_VERSION, datetime(3000,1,1), {}, delegations)
targets = Metadata(targets_signed, OrderedDict())
targets.sign(targets_signer)
targets.to_file("targets.json")

# create snapshot
# TODO maybe Metafile.from_metadata(targets: Metadata) 
# what Metafile will contain is unclear... it's also not the same as TargetFile.from_data()
# TODO maybe Metadata.new_snapshot_with_defaults(expires: datetime, target_metadatas: list[Metadata])
# (again the issue here is making a decision on what data to include in MetaFile?)
meta = {"targets.json": MetaFile(targets.signed.version)}
snapshot_signed = Snapshot(1, SPEC_VERSION, datetime(3000,1,1), meta)
snapshot = Metadata(snapshot_signed, OrderedDict())
snapshot.sign(snapshot_signer)
snapshot.to_file("snapshot.json")

# create timestamp
# TODO Metadata.new_timestamp_with_defaults(expires: datetime, snapshot: Metadata)
# (again issue is making a decision on what data to include in MetaFile?)
meta = {"snapshot.json": MetaFile(snapshot.signed.version)}
timestamp_signed = Timestamp(1, SPEC_VERSION, datetime(3000,1,1), meta)
timestamp = Metadata(timestamp_signed, OrderedDict())
timestamp.sign(timestamp_signer)
timestamp.to_file("timestamp.json")

# create root
# TODO Metadata.new_root_with_defaults(expires, root_key, timestamp_key, snapshot_key, targets_key)
# TODO alternative: Metadata.new_root_with_defaults(expires) + 
#                               Metadata.set_keys(rolename: list[Key], threshold)
# There's no need to make role management easier as the roles should never change: only keys and role thresholds will
keys = {key.keyid:key for key in [root_key, snapshot_key, timestamp_key, targets_key]}
roles = {
    "root": Role([root_key.keyid], 1),
    "targets": Role([targets_key.keyid], 1),
    "snapshot": Role([snapshot_key.keyid], 1),
    "timestamp": Role([timestamp_key.keyid], 1),
}

root_signed = Root(1, SPEC_VERSION, datetime(3000,1,1), keys, roles, True)
root = Metadata(root_signed, OrderedDict())
root.sign(root_signer)
root.to_file("root.json")


# Test MetadataBundle with the in-memory metadata
# TODO: Add Metadata.to_bytes() to avoid dealing with serializer
#serializer = JSONSerializer()
# UNTESTED: bundle = MetadataBundle(serializer.serialize(root))

# Create root to version 2 with rotated key
root.signed.remove_key("timestamp", timestamp_key.keyid)

timestamp_key, timestamp_signer = generate_test_key()
timestamp.signed.version += 1
timestamp.sign(timestamp_signer)
timestamp.to_file("timestamp.json")

# TODO should add_key() remove the old key from keys if it does not have users 
# anymore like remove _key() does?
root.signed.add_key("timestamp", timestamp_key)
root.signed.version += 1
root.sign(root_signer)
root.to_file("root.json")

# Update bundle to version 2
# UNTESTED: bundle.update_root(serializer.serialize(root))

# create version 3 with higher threshold
root2_key, root2_signer = generate_test_key()
root.signed.add_key("root", root2_key)
root.signed.roles["root"].threshold = 2
root.signed.version += 1

# TODO should signing be a single operation sign(list[signers])?
# alternatively: sign_new_version(list[signers]) that also bumps version?
# alternatively: Metadata can be given a list of signers earlier, always uses them to sign
# when needed? How does repository usually figure out which signers it needs?
root.sign(root_signer)
root.sign(root2_signer, append=True)
root.to_file("root.json")

# Update bundle to version 3
# UNTESTED: bundle.update_root(serializer.serialize(root))

# create version 3 with only one signature
root.signed.version += 1
root.sign(root_signer)
root.to_file("root.json")

# Update bundle to version 3 (this should fail: not enough signatures)
# UNTESTED: bundle.update_root(serializer.serialize(root))

# UNTESTED: bundle.root_update_finished()

(the MetadataBundle testing code is not tested yet as the branch is not up-to-date with develop)

@jku
Copy link
Member Author

jku commented Jun 14, 2021

We can either polish the idea here, or just start picking out the most obvious improvements and filing them as actionable bugs, examples:

  • add Metadata.to_bytes()
  • Add the constructors that seem easiest to design (maybe Targets.new_with_defaults(), TargetFile.from_data())

and then just start adding tests once the most annoying things have been fixed

@jku
Copy link
Member Author

jku commented Jun 14, 2021

some of the individual trickier design issues are pointed out in the code but something that maybe isn't is the interplay of

  • version bumping
  • signing
  • serialization

they are completely unrelated from the API perspective... yet I bet there is streamlining we could do here to make life easier for API user (and the testing code!).

@trishankatdatadog
Copy link
Member

Agree that generating and serving metadata during tests is doing too many things all at once. Hard to read and debug. Test metadata and targets should be generated (ideally once) and served from their own repository/server.

Cc @tedbow and friends, who I believe did some work along these lines...

@jku
Copy link
Member Author

jku commented Jun 21, 2021

Test metadata and targets should be generated (ideally once) and served from their own repository/server.

My thoughts:

  • I really dislike the currently used combination of A) loading external, common test data files and B) then modifying them in test code. This creates a situation where everyone is scared of modifying the common files (because that could change unrelated tests), and where reading a test doesn't really give you a good picture of what happens (because you need to read the external test data file as well)
  • Best option is to include all test data with the test if possible (we're trying to do that in Metadata API: comprehensive de/serialization testing #1391 for serialization), and to keep all data specific to a single test -- this will not work for anything complicated like a full update workflow however
  • For complicated tests that e.g. involve multiple Metadata files and possibly multiple versions of those, I would like to generate the data in the test code, but put real effort into making that manageable. This at least keeps the whole thing in the test code and there's no dependency on "common" test files -- this is also likely to improve Metadata API as we're doing a lot of "repository management" tasks...
  • I would like to avoid putting data on disk as much as possible even at test runtime -- at least up to the ngclient MetadataBundle (or whatever it ends up being called) we should be able to do that 100%. Likewise I'd like to avoid setting up webservers until we have to
  • The most high level tests (like Updater API tests) will have to write to disk, use a webserver and may have to use test data stored in git, just like currently: we should just try to test as muc has we can at lower levels

@tedbow
Copy link

tedbow commented Jun 21, 2021

@trishankatdatadog thanks for getting my attention.

Yes we have done a few different things in https://github.com/php-tuf/php-tuf, which only a client-side implementation of the spec, related to this.
We have no releases yet so we have been able to change more quickly.

TLDR: We currently have a FixtureBuilder class that uses Python TUF to generate test fixtures for our high level functional tests and we commit those test fixtures to the our repo. We may decide later we don't need the actual test metadata files in our repo

We have tentative plans to move all our test fixtures to https://github.com/php-tuf/conformance . Ultimately it might make sense to have these fixtures out of the php-tuf org because they could be used my other repos. Right now it makes more sense to work in 1 repo for us.

For example we have this proposed fixture(new PR): https://github.com/php-tuf/php-tuf/blob/tedbow-fix_terminating_2/fixtures/TUFTestFixtureTerminatingDelegation/__init__.py
which addresses the question here theupdateframework/specification#168
It has a clear explanation of the fixture and the expected outcome when searching for the different targets.
Eventually something like this could be used by multiple clients to ensure they are implementing the spec correctly.

This is how we got to the current situation:

  1. 1st we were generating metadata files with the Python TUF library for our functional high level tests and committing those fixtures to our repo. This resulted in tons of files changes in commits when we were writing these tests that needed fixtures.
  2. To avoid commits with tons of file changes we removed the test metadata files from our repo. @davidstrauss created a github action that would create the fixtures which our functional tests would use. We also saved these test artifacts so people could use this to run the tests locally without setting up Python(since we are a PHP project) and to inspect the actual test fixtures used. At the same time @davidstrauss also put work into making our test fixture generation deterministic(see Make metadata signatures ordered by keyid #1217 )
  3. As our test fixtures became more complicated @phenaproxima refactored the generation into a new FixtureBuilder class to make our generation more understandable. This led to the problem of how can be positive the new class produced identical test fixtures without test fixtures in the repo?
  4. To solve 3) we added back the test fixtures to the repo and added a github action to confirm the committed script we have to generate the test fixtures would also generate exactly the fixtures committed to the repo. This ensures someone doesn't make a small change to the fixture generation and forget to rebuild and commit that change.
  5. Also 4) has the added benefit that when we update our TUF dependency the test fixture generation doesn't change for something we are unaware of.

@jku
Copy link
Member Author

jku commented Jun 21, 2021

#1458 Add Key.from_securesystemslib:key()?
#1448 Add TargetFile.from_data()
#1459 Create constructor(s) for creating Metadata from scratch

@trishankatdatadog
Copy link
Member

I like the idea of a test metadata/targets repo which multiple implementations can test against...

@jku
Copy link
Member Author

jku commented Aug 9, 2021

Here are some issues with testing the updater (these apply to new and old one):

  • testing is based on json files in git -- these are shared by multiple tests so can't be safely modified. Extending the test set also feels unwise currently since there's no infrastructure to re-generate the set automatically
  • new versions of the repository metadata are created in testing code: there is a lot of boiler plate involved (deserializing, loading keys, signing, serializing) and no consistent way to do this
  • the test cases are very simple (just a few versions) because otherwise the test code gets so complex we no longer understand what happens
  • there is a lot of decrypting keys, reading files, writing files, starting web server processes and making network connections involved -- none of this is core to what the updater does IMO but makes the testing slow: so even if we could massively expand the testing, running the tests would take a long time. I don't mean we shouldn't test the updater e.g. over network at all, just that that isn't necessary for the majority of the test cases

One potential solution is what I think go-tuf does: create an infrastructure the generates a vast amount of test metadata automatically, then build tests to use that metadata. This feels very appropriate for a shared conformance test suite.

That approach might work for our updater tests too but I have another approach in mind as well.
I'd like to have a TestingRepo object that:

  • manages a repositorys Metadata (and targets and keys) in memory for testing purposes
  • provides an API for test code to call to modify the Metadata (create new versions, add delegations/targets, etc)
  • handles signing, version changes, expiry, snapshot/timestamp updates etc
  • exposes the metadata to updater by implementing FetcherInterface (the implementation returns results from memory)

This is essentially a TUF repository implementation but specific for testing the client. The Metadata is generated from scratch, is never written to disk by the test code and never needs to be loaded over the network so this side-steps a lot of the boilerplate and unneeded slow steps. Hopefully this is also leads to simpler test code but this would have to be implemented to see if that is the case.

I've tested the Fetcher implementation: that works fine. Open unresolved issues that I can see are:

  • how to handle version bumps -- maybe test code should explicitly ask for new version
  • key generation is easy to include but can be slow -- should avoid generating keys for every test
  • what sort of helper methods are needed -- e.g. update_snasphot,update_timestamp seem obvious but there could be others?
  • signing could be done implicitly on every fetch of metadata but that could be slow as well -- maybe needs to be explicit
  • figuring out which metadata versions to keep stored and how -- e.g. root needs all versions

I think these can be figured out with a prototype. This is, I hope, a simpler problem than a repository CLI but code in https://github.com/vmware-labs/repository-editor-for-tuf/blob/main/tufrepo.py may be useful

@davidstrauss
Copy link

davidstrauss commented Aug 9, 2021

One potential solution is what I think go-tuf does: create an infrastructure the generates a vast amount of test metadata automatically, then build tests to use that metadata. This feels very appropriate for a shared conformance test suite.

We're also doing this for PHP-TUF.

@sechkova sechkova added this to the Sprint 7 milestone Sep 1, 2021
@sechkova sechkova added the backlog Issues to address with priority for current development goals label Sep 1, 2021
@jku
Copy link
Member Author

jku commented Sep 2, 2021

I have a rough prototype of a RepositorySimulator in https://github.com/jku/tuf/commits/updater-tests-with-simulated-repo. I think it's promising. The actual testing looks like this:

    # client: update metadata
    updater = Updater(...)
    updater.refresh()

    # repository: publish new targets version (and snapshot and timestamp)
    repo_sim.targets.version += 1
    repo_sim.update_snapshot()

    # client: update metadata
    updater = Updater(...)
    updater.refresh()

in the current test infra that would have been quite painful

@trishankatdatadog
Copy link
Member

cc @hosseinsia

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Issues to address with priority for current development goals testing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants