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

Fix compatibility issues in tests and enable Windows CI #518

Merged
merged 6 commits into from
Feb 22, 2023

Conversation

lukpueh
Copy link
Member

@lukpueh lukpueh commented Feb 21, 2023

Description of the changes being introduced by the pull request:

closes #507, #339

Compatibility issues were all related to paths (storage, gpg, hsm) or unavailability of a dependency (sphincs). See commit messages for details.

The PR also generally lowers the min coverage requirement to 90%, which still seems reasonable.

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

* test_interface
Case-handle expected permissions for newly created private key
files (world-readable on Windows, user-readable elsewhere).

* test_storage
 - Refactor exception tests for better readability.
 - Create test directory and explicitly set permissions, to test
   permission errors more reliably.
 - Skip permission tests on Windows, which does not fully support
   `mode`.

* test_util
Remove permission-error test in ensure_parent_dir method to avoid
platform case handling. This is only an internally used thin
wrapper around create_dirs, which is already tested extensively in
test_storage.

Signed-off-by: Lukas Puehringer <[email protected]>
Prior to this change gpg tests failed because securesystemslib.gpg
interface functions cannot handle absoulte paths as homedir on
Windows. This patch works around the issue to fix tests only by not
using absolute paths:

* Use relative path to homedir for gpg tests, or
* Store and use fixtures of raw pubkey data for gpg parsing tests. These test
don't actually need to run the gpg command. The fixtures were created like so:

```
keyids=(F557D0FF451DEF45372591429EA70BD13D883381 \
        E8AC80C924116DABB51D4B987CB07D6D2C199C7C)
dir=tests/gpg_keyrings/rsa

for keyid in ${keyids[@]}; do
  gpg --homedir ${dir} --export ${keyid} > ${dir}/${keyid}.raw;
done
```

Signed-off-by: Lukas Puehringer <[email protected]>
The used trick to assert that HSM tests are not silently skipped
due to an unset environment variable does not seem to work on
Windows.

This patch provides a simple alternative that does not require
advanced escape character usage.

Signed-off-by: Lukas Puehringer <[email protected]>
The required PySPX library is not available on Windows. This patch
configures requirements files to not install PySPX on Windows, and
skips relevant tests on Windows.

Details:
* in test_keys the relevant tests are factored out into a
  separate test class to avoid inline platform case handling in
  multiple places. The refactor also simplifies the tests and removes
  one graceful failure check, which is redundant with
  check_public_interfaces.

* in test_signer the keys need for testing are simply not populated
  on windows.

Signed-off-by: Lukas Puehringer <[email protected]>
Signed-off-by: Lukas Puehringer <[email protected]>
Tests on Windows fail because they don't have 95% coverage (only
~93 %). This commit lowers the requirement to a value that seems
reasonable for all platforms.

Coverage measurement may be fine tuned with secure-systems-lab#208 and changed back
to a higher value if desired.

Signed-off-by: Lukas Puehringer <[email protected]>
@lukpueh lukpueh marked this pull request as ready for review February 21, 2023 12:02
@lukpueh lukpueh merged commit 265115e into secure-systems-lab:master Feb 22, 2023
@lukpueh lukpueh mentioned this pull request Mar 1, 2023
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.

2 participants