-
Notifications
You must be signed in to change notification settings - Fork 50
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
Automate Signer API docs for RTD #622
Conversation
- add basic docs landing page (index.rst) - copy and adapt docs config from python-tuf (docs/conf.py) - add sphinx make files generated with sphinx-quickstart - add docs- and include in dev- reqirements.txt - gitignore sphinx build directory - add rtd config file Signed-off-by: Lukas Puehringer <[email protected]>
- re-format Signer and Key docstrings - add autodoc docstrings for public dispatch tables - add args docstrings for Signature Signed-off-by: Lukas Puehringer <[email protected]>
Signed-off-by: Lukas Puehringer <[email protected]>
keyid: HEX string used as a unique identifier of the key. | ||
sig: HEX string representing the signature. | ||
unrecognized_fields: Dictionary of all attributes that are not managed | ||
by securesystemslib. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Repeating Args and Attributes seems silly. I'd prefer to only use "Args" here and say "All parameters named below are not just constructor arguments but also instance attributes." as we do in the Key
class.
The problem is we use sig
as arg name and signature
as attribute name. I think we should consolidate them now. Changing the (positional) arg name seems less disrupting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, consolidate now sounds good: No-one outside of securesystemslib should should be calling Signature() so the fallout should be minimal.
This doesn't have to be in this PR though
os: ubuntu-22.04 | ||
apt_packages: | ||
- swig | ||
- softhsm2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
swig
and softhsm2
are needed for pykcs11
, which we install in requirements-docs.txt via requirements-pinned.txt
.
We can probably shuffle requirements* files so that we don't need to install any optional requirements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion makes sense but if rtd is fine with us installing random software... I suppose this is ok
- Switch from rst to markdown (syntax is just so much easier) - Add honest project description (securesystemslib is for TUF and in-toto). - Remove wordy Overview section. Relevant information about crypto backends, and key types and formats should be documented as part of the API on RTD. - Replace legacy interface snippets in Usage section with link to securesystemslib RTD page. Legacy interfaces have functional replacements in the new Signer API, i.e. CryptoSigner for file-based RSA, ed25519, ecdsa keys, and GPGSigner for GPG keys. Signer API docs are still WIP (see secure-systems-lab#622), but already seem more useful than the legacy docs. And we definitely don't want to encourage anyone to use legacy interfaces. - Shorten installation/testing sections. Signed-off-by: Lukas Puehringer <[email protected]>
I've been thinking we should have an example app (like a tool to sign and verify files) to really show why the design makes sense. Sigstore would be the perfect signer implementation to use (because the example would be simplest since there are no private key files to worry about) but that would imply making SigstoreSigner non-experimental which I don't really want yet... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a great start.
I think the make.bat+Makefile
are probably a worse solution than tox -e docs
used in python-tuf because
- tox setup is something we can test on CI
- tox setup is actually less code to maintain
but this is also fine.
os: ubuntu-22.04 | ||
apt_packages: | ||
- swig | ||
- softhsm2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion makes sense but if rtd is fine with us installing random software... I suppose this is ok
keyid: HEX string used as a unique identifier of the key. | ||
sig: HEX string representing the signature. | ||
unrecognized_fields: Dictionary of all attributes that are not managed | ||
by securesystemslib. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, consolidate now sounds good: No-one outside of securesystemslib should should be calling Signature() so the fallout should be minimal.
This doesn't have to be in this PR though
I'll merge this as is, and file a new issue for at least the example |
Description
Adds configuration for Sphinx and Readthedocs, to automate securesystemslib API documentation. This PR includes a minimal landing page and enables docstring-autodocs for interface classes and variables of Signer API only. Additionally, docstrings are modified slightly to make them Sphinx-friendly.
fixes #271 (Note: The ticket is a bit broader than this. I still suggest to close and ticketize remaining tasks as described below)
TODO (in follow-up PRs)
Should Signer subclasses be added? The idea behind the URI system was that the actual implementations can be hidden from the user. But currently, the implementations have some important public methods, e.g.
import_
,generate
, etc., to even construct the URIs. Asked differently, what docs are missing for the Signer API to be fully usable?Ideally, basic usage examples are included in the API docs. See Jussi's blog post and examples added in #604.
At least
dsse
andformats.encode_canonical
.