Removes unused and TUF-specific content from securesystemslib #165

awwad · 2019-04-03T21:04:00Z

This PR carves out lots of unnecessary content in securesystemslib. It:

removes schemas that were unused across securesystemslib, TUF, and in-toto
removes some functions that were unused across ssl/tuf/in-toto
removes schemas that are specific to TUF (requires changes in TUF that will already be made in TUF PR 846)
marks for future removal a function that is specific to TUF (get_targets_hash)
marks for future removal two compression-related functions and a compression-related exception that are currently unused across ssl/tuf/in-toto
removes some exceptions unused across ssl/tuf/in-toto
removes many exceptions that were TUF-specific (requires changes in TUF, already made in TUF PR 855)
removes RELPATH_SCHEMA and RELPATHS_SCHEMA, which were unnecessary/misleading and are replaced by the existing PATH_SCHEMA and PATHS_SCHEMA (appears not to require changes in in-toto; requires changes in TUF that will already be made in TUF PR 846)

It also removes or corrects relevant tests.

This largely resolves Issue #161, especially given the prior merge of PR #162.

awwad · 2019-04-16T20:04:51Z

@lukpueh
I believe this is ready for review now. Should be very basic. Aside from whatever you spot, I'm curious to know if it affects in-toto. Also LMK if any of the 3 remaining, TODO-marked functions in util.py are of use to you.

Removal of securesystemslib exceptions that are TUF-specific occurs in securesystemslib PR #165 secure-systems-lab/securesystemslib#165 This commit adapts to those changes. Exceptions that are specific to TUF should be in TUF and not in securesystemslib. This commit uses those already-existing TUF exceptions instead of pointing to securesystemslib exceptions that will be removed. For example, securesystemslib has no notion of repositories, so it's ridiculous to have a RepositoryError in securesystemslib and ridiculous for TUF to use securesystemslib.exceptions.RepositoryError. Signed-off-by: Sebastien Awwad <[email protected]>

lukpueh · 2019-08-14T14:54:20Z

Also closes #75

lukpueh

Apologies for the late review. This slipped under my radar. It mostly looks good. I'd like to double check a few more things before merging and update a few others (see TODO below).

Confirming that *Error classes removed from exceptions.py are obsolete or TUF-specific and in the latter case already exist in TUF.
Confirming that *_SCHEMA definitions removed from formats.py are obsolete or TUF-specific and in the latter case already exist in TUF. Except for the following, which are not defined but used in TUF (tip of develop branch):
- KEYDB_SCHEMA
- SIGNATURESTATUS_SCHEMA
- SIGNATURES_SCHEMA
- VERSIONINFO_SCHEMA
Confirming that the functions removed from util.py are obsolete.

TODO:

~~Confirm whether existing TUF PRs add or re-write import paths for the used schemas, and create PRs otherwise~~
- Created PR to add KEYDB_SCHEMA, SIGNATURESTATUS_SCHEMA and VERSIONINFO_SCHEMA back to TUF (see Add TUF-specific schemas removed in sslib theupdateframework/python-tuf#910)
- Re-added SIGNATURES_SCHEMA here
~~Create TUF PR to transfer tuf specific util.get_target_hash and remove here~~
- See Move repository_lib.get_taget_hash back from sslib to tuf theupdateframework/python-tuf#909
~~Remove compression related functions/exception, already marked for removal~~
- deferred for a separate cleanup PR
~~Grep for tuf specific comments and consider removing them~~

Largely resolves Issue #161, especially given the merge of PR #162. See GitHub: #161 #162 Signed-off-by: Sebastien Awwad <[email protected]>

The following functions in securesystemslib.util are not used anywhere, not in securesystemslib, not in TUF, and not in in-toto: - find_delegated_roles - ensure_all_targets_allowed - paths_are_consistent_with_hash_prefixes In addition, the first two are also clearly TUF-specific (and the third is a close call). So I'm removing them all. This commit also removes their tests. Signed-off-by: Sebastien Awwad <[email protected]>

This follows from the commit with subject: "Removes TUF-specific formats from formats.py" It: - removes testing for schemas that were removed by that commit (TUF-specific or otherwise unnecessary schemas) - corrects uses of the deleted RELPATH_SCHEMA to PATH_SCHEMA and RELPATHS_SCHEMA to PATHS_SCHEMA Signed-off-by: Sebastien Awwad <[email protected]>

sys and fnmatch are no longer needed due to the removal of unused functions in prior commits. Signed-off-by: Sebastien Awwad <[email protected]>

Signed-off-by: Sebastien Awwad <[email protected]>

and also marks the DecompressionError exception for possible future removal (alongside the compression-related functions). Signed-off-by: Sebastien Awwad <[email protected]>

coveralls · 2019-09-04T16:20:47Z

Coverage remained the same at 100.0% when pulling 4adbc5a on carve_out_tuf_formats into ae8517d on master.

The function used to last be implemented in securesystemslib and repository_lib.get_taget_hash only served as wrapper. secure-systems-lab/securesystemslib#165 drops the function as TUF-specific. The used constant `securesystemslib.util.HASH_FUNCTION` is replaced with `tuf.settings.DEFAULT_HASH_ALGORITHM`, both of which default to 'sha256'. NOTE: repository_lib.get_taget_hash might be removed altogether in the future (see corresponding code comment). Signed-off-by: Lukas Puehringer <[email protected]>

Also remove now obsolete util.HASH_FUNCTION and corresponding tests. The function is added back to tuf in theupdateframework/python-tuf#909.

Add schemas KEYDB_SCHEMA, SIGNATURESTATUS_SCHEMA and VERSIONINFO_SCHEMA, removed in secure-systems-lab/securesystemslib#165 as TUF specific, and adopt usage accordingly. NOTE: The usefulness of these schemas may be assessed in a different PR.

Add schemas KEYDB_SCHEMA, SIGNATURESTATUS_SCHEMA and VERSIONINFO_SCHEMA, removed in secure-systems-lab/securesystemslib#165 as TUF specific, and adopt usage accordingly. NOTE: The usefulness of these schemas may be assessed in a different PR. Signed-off-by: Lukas Puehringer <[email protected]>

lukpueh · 2019-09-05T11:27:39Z

@awwad, I reviewed your code and made a couple of minor additions (see #165 (review)).
Do you think you have a minute to review my commits on top of yours?

Add schemas KEYDB_SCHEMA, SIGNATURESTATUS_SCHEMA and VERSIONINFO_SCHEMA, removed in secure-systems-lab/securesystemslib#165 as TUF specific, and adopt usage accordingly. NOTE: The usefulness of these schemas may be assessed in a different PR. Signed-off-by: Lukas Puehringer <[email protected]>

adityasaky

Verified *Error, *_SCHEMA, and util.* removed here have been moved to TUF / are no longer used.

Is there conversation around re-adding SIGNATURES_SCHEMA to securesystemslib for clarity?

lukpueh · 2019-09-06T08:23:50Z

Thanks for the review, @adityasaky. SIGNATURES_SCHEMA is currently still used in TUF (@develop) . Maybe it's being removed in one of the pending TUF PRs (haven't checked in that detail). For the time being, I doesn't hurt to keep it in securesystemslib.

adityasaky · 2019-09-06T13:53:53Z

Yep, I saw that! I was just wondering because it was removed then added back in. Definitely doesn't hurt to keep it around. :)

Removal of securesystemslib exceptions that are TUF-specific occurs in securesystemslib PR #165 secure-systems-lab/securesystemslib#165 This commit adapts to those changes. Exceptions that are specific to TUF should be in TUF and not in securesystemslib. This commit uses those already-existing TUF exceptions instead of pointing to securesystemslib exceptions that will be removed. For example, securesystemslib has no notion of repositories, so it's ridiculous to have a RepositoryError in securesystemslib and ridiculous for TUF to use securesystemslib.exceptions.RepositoryError. Signed-off-by: Sebastien Awwad <[email protected]>

awwad changed the title ~~WIP: Removes TUF-specific formats from formats.py~~ Removes TUF-specific formats from formats.py Apr 16, 2019

awwad changed the title ~~Removes TUF-specific formats from formats.py~~ Removes unused and TUF-specific content from securesystemslib Apr 16, 2019

awwad requested a review from lukpueh April 16, 2019 20:03

awwad mentioned this pull request Apr 16, 2019

Adjust to removals from securesystemslib theupdateframework/python-tuf#855

Merged

3 tasks

lukpueh reviewed Aug 14, 2019

View reviewed changes

awwad added 6 commits September 4, 2019 18:16

Removes TUF-specific formats from formats.py

0cfe418

Largely resolves Issue #161, especially given the merge of PR #162. See GitHub: #161 #162 Signed-off-by: Sebastien Awwad <[email protected]>

Remove no-longer-needed imports in util.py

3c7fc34

sys and fnmatch are no longer needed due to the removal of unused functions in prior commits. Signed-off-by: Sebastien Awwad <[email protected]>

minor: add TODOs for possible futher removals from util.py

913f085

Signed-off-by: Sebastien Awwad <[email protected]>

Removes a host of unused or TUF-specific exceptions

d5e0981

and also marks the DecompressionError exception for possible future removal (alongside the compression-related functions). Signed-off-by: Sebastien Awwad <[email protected]>

lukpueh force-pushed the carve_out_tuf_formats branch from 9e21c9b to d0811e0 Compare September 4, 2019 16:18

lukpueh added 2 commits September 5, 2019 11:02

Remove/re-word TUF code comments

560f66c

Add comment to remove TUF-specific TempFile prefix

318f90a

lukpueh force-pushed the carve_out_tuf_formats branch from d0811e0 to 318f90a Compare September 5, 2019 09:05

lukpueh mentioned this pull request Sep 5, 2019

Move repository_lib.get_taget_hash back from sslib to tuf theupdateframework/python-tuf#909

Merged

3 tasks

lukpueh added 2 commits September 5, 2019 12:11

Remove TUF-specific util.get_target_hash function

532fc20

Also remove now obsolete util.HASH_FUNCTION and corresponding tests. The function is added back to tuf in theupdateframework/python-tuf#909.

Re-add mildly useful SIGNATURES_SCHEMA

4adbc5a

lukpueh mentioned this pull request Sep 5, 2019

Add TUF-specific schemas removed in sslib theupdateframework/python-tuf#910

Merged

3 tasks

adityasaky approved these changes Sep 5, 2019

View reviewed changes

lukpueh merged commit e0dd6c7 into master Sep 6, 2019

This was referenced Sep 6, 2019

Boundary between securesystemslib and TUF is off. #161

Closed

Remove references to TUF #75

Closed

lukpueh mentioned this pull request Sep 9, 2019

Schema revision #176

Merged

3 tasks

joshuagl mentioned this pull request Sep 13, 2019

Use TUF specific formats as they have been removed from securesystemslib theupdateframework/python-tuf#912

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Removes unused and TUF-specific content from securesystemslib #165

Removes unused and TUF-specific content from securesystemslib #165

awwad commented Apr 3, 2019 •

edited

Loading

awwad commented Apr 16, 2019 •

edited

Loading

lukpueh commented Aug 14, 2019

lukpueh left a comment •

edited

Loading

coveralls commented Sep 4, 2019 •

edited

Loading

lukpueh commented Sep 5, 2019

adityasaky left a comment •

edited

Loading

lukpueh commented Sep 6, 2019

adityasaky commented Sep 6, 2019

Removes unused and TUF-specific content from securesystemslib #165

Removes unused and TUF-specific content from securesystemslib #165

Conversation

awwad commented Apr 3, 2019 • edited Loading

awwad commented Apr 16, 2019 • edited Loading

lukpueh commented Aug 14, 2019

lukpueh left a comment • edited Loading

Choose a reason for hiding this comment

coveralls commented Sep 4, 2019 • edited Loading

lukpueh commented Sep 5, 2019

adityasaky left a comment • edited Loading

Choose a reason for hiding this comment

lukpueh commented Sep 6, 2019

adityasaky commented Sep 6, 2019

awwad commented Apr 3, 2019 •

edited

Loading

awwad commented Apr 16, 2019 •

edited

Loading

lukpueh left a comment •

edited

Loading

coveralls commented Sep 4, 2019 •

edited

Loading

adityasaky left a comment •

edited

Loading