Add tmp dir and gpg key test mixins. #345
Conversation
- Add TmpDirMixin with class methods to create and change to temporary directory, and to change back to original cwd and remove the temporary directory. - Configure relevant test cases to use the mixin in their set up and tear down methods. Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
- Add GPGKeysMixin with classmethod to locate the test gpg rsa keyrings and copy them to the cwd (e.g. a temporary directory when used after TmpDirMixin.set_up_test_dir). - Configure relevant test cases to use the mixin in their set up methods. Note the mixin has a class variable gnupg_home which contains the path to the keyring relative to cwd. This fixes an issue with gpg on windows that has troubles migrating old keyrings if the homedir is an absolute path with drive prefix (e.g. C:/path/to/keyring). Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
|
For documentation purposes, I'd like to share debug output from an appveyor builder that shows how it struggles with absolute paths to non-default homedirs if the gpg keyring needs to be migrated. This directory contains a gpg keyring in an old format. C:\projects\in-toto\tests\gpg_keyrings>ls rsa
7B3ABB26B97B655AB9296BD15B0BD02E1C768C43.ssh pubring.gpg trustdb.gpg
8288EF560ED3795F9DF2C0DB56193089B285DA58.ssh random_seed
8465A1E2E0FB2B40ADB2478E18FB3F537E0C8A17.ssh secring.gpgWhen e.g. trying to sign with a newer version of gpg, it will try to migrate the format of the keys, which fails. Doing the same thing, but passing a relative instead of an absolute path to the keyring, the migration Once migration has happened gpg accepts absolute paths too and can successfully create signatures. It seems that gpg starts the gpg-agent but is then not able to connect to it. Note though that this only happens if migration is necessary (i.e. coming from migrate.c). If migration has already happened, absolute paths are accepted, even if the agent has to be restarted... This PR should sufficiently (for our purposes) work around this issue. |
|
|
||
| # Restore log level to what it was before running in-toto-mock | ||
| logger.setLevel(self._base_log_level) | ||
|
|
||
|
|
There was a problem hiding this comment.
I think there's been some inconsistency here. As I understand it, methods within classes have one blank line before and after. I think we have a mix of single lines and double lines throughout the codebase, and probably something a linter will catch?
There was a problem hiding this comment.
You're right. Seems like pylint does not support that though. Before we investigate other linters, we should overhaul our code style guidelines. IMO the current recommendations for blank lines (5 lines between top-level functions) is overkill. In in-toto we usually use 2 or 3. I think a slight inconsistency is not a very big deal.
|
Thanks for the review, @adityasaky! |
Fixes issue #:
Supersedes #216
Partially fixes #87
Description of the changes being introduced by the pull request:
Adds two test mixins
TmpDirMixinandGPGKeysMixinwith methods and change to a temporary directory, change back to the original cwd and delete the temporary directory, and to copy gpg rsa keys to the cwd.Removes repetitive code from test setup and teardown methods and uses mixins instead
NOTE: the mixin has a class variable
gnupg_homewhich contains the path to the keyring relative to cwd. This fixes an issue with gpg on windows that has troubles migrating old keyrings if the homediris passed as absolute path with drive prefix (e.g.
C:/path/to/keyring).Please verify and check that the pull request fulfills the following
requirements: