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

Handle missing native dependencies in securesystemslib.gpg #206

Merged
merged 5 commits into from
Feb 10, 2020

Conversation

joshuagl
Copy link
Collaborator

@joshuagl joshuagl commented Feb 6, 2020

Fixes issue #:
#179

Description of the changes being introduced by the pull request:

  • Handle missing gpg[2] command in all functions that would call it and raise UnsupportedLibraryError
  • Wrap native module imports in securesystemslib.gpg in try/except logic so that the modules can always be imported cleanly
  • Handle missing native module dependencies in securesystemslib.gpg by raising UnsupportedLibraryError when a function requiring a native module dependency is called
  • Test the securesystemslib.gpg interfaces requiring gpg[2] or native module dependencies from our pure python test environment

Please verify and check that the pull request fulfils the following
requirements
:

  • 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

securesystemslib.gpg.constants checks for the presence of a gpg
binary at import time, save the result of this search in a constant
HAVE_GPG so that we can use it later to present meaningful error
messages when a function requiring gpg is called and the command
isn't available.

Signed-off-by: Joshua Lock <[email protected]>
@joshuagl
Copy link
Collaborator Author

joshuagl commented Feb 6, 2020

Need to fix the .travis.yml changes to correctly remove (and possibly re-add) gpg et al.

@joshuagl joshuagl force-pushed the joshuagl/issue179-gpg branch 3 times, most recently from 856df02 to 283cea3 Compare February 6, 2020 22:02
@coveralls
Copy link

Coverage Status

Coverage remained the same at ?% when pulling 283cea3 on joshuagl:joshuagl/issue179-gpg into d07141c on secure-systems-lab:master.

@coveralls
Copy link

coveralls commented Feb 6, 2020

Coverage Status

Coverage remained the same at ?% when pulling d6c3933 on joshuagl:joshuagl/issue179-gpg into d07141c on secure-systems-lab:master.

Copy link
Member

@lukpueh lukpueh left a comment

Choose a reason for hiding this comment

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

Great work as always! Many thanks, @joshuagl! I have left 3 minuscule comments. Also, IIUC, we should be able to remove this block in aggregate_tests.py now (the call in L57 too), and in test_gpg.py replace
occurrences of
@unittest.skipIf(os.getenv("TEST_SKIP_GPG"), "gpg not found")
with
@unittest.skipIf(not securesystemslib.gpg.constants.HAVE_GPG, "gpg not found")

Right?

.travis.yml Outdated Show resolved Hide resolved
securesystemslib/gpg/util.py Outdated Show resolved Hide resolved
tests/check_public_interfaces.py Show resolved Hide resolved
@joshuagl
Copy link
Collaborator Author

joshuagl commented Feb 7, 2020

Thanks for the review @lukpueh !

Also, IIUC, we should be able to remove this block in aggregate_tests.py now (the call in L57 too), and in test_gpg.py replace
occurrences of
@unittest.skipIf(os.getenv("TEST_SKIP_GPG"), "gpg not found")
with
@unittest.skipIf(not securesystemslib.gpg.constants.HAVE_GPG, "gpg not found")

Right?

Absolutely right. That will be a nice clean up. I'll update the PR with that and your other requested changes on Monday.

Raise an UnsupportedLibraryError exception when functions that rely on
invoking the gpg command are called on a system where gpg is unavailable.

Signed-off-by: Joshua Lock <[email protected]>
Raise UnsupportedLibraryError from each function in the gpg modules
when the native libraries needed by that function aren't available.

Signed-off-by: Joshua Lock <[email protected]>
@joshuagl joshuagl force-pushed the joshuagl/issue179-gpg branch from 283cea3 to 6125013 Compare February 10, 2020 10:49
We don't need to detect the presence of the GPG command and set
environment variables, instead we can use the HAVE_GPG constant which
indicated whether a usable GPG command is present on the system.

Signed-off-by: Joshua Lock <[email protected]>
@joshuagl joshuagl force-pushed the joshuagl/issue179-gpg branch from 6125013 to d6c3933 Compare February 10, 2020 10:53
@joshuagl joshuagl requested a review from lukpueh February 10, 2020 10:56
@joshuagl
Copy link
Collaborator Author

I've update the PR to incorporate your suggested changes @lukpueh . Please take a look when you have the time.

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.

3 participants