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

Add a higher level API. #323

Merged
merged 2 commits into from
Nov 19, 2024
Merged

Add a higher level API. #323

merged 2 commits into from
Nov 19, 2024

Conversation

mihaimaruseac
Copy link
Collaborator

@mihaimaruseac mihaimaruseac commented Nov 18, 2024

Summary

Currently, this only supports Sigstore (and only DSSE on verification). I want to push this now so that people can start playing with it while I update the Colab demo and try to make the API more uniform and add support for the missing cases.

With this, the default signing should be as simple as

from model_signing import api as model_signing_api

model_signing_api.sign(model_path, signature_path)

And verification would be as simple as:

from model_signing import api as model_signing_api

model_signing_api.verify(
  model_path, signature_path,
  expected_identity, expected_oidc_provider
)

Will think on how to simplify this even further..

This is part of #172, but not yet closing it

Release Note

NONE

Documentation

NONE

Currently, this only supports Sigstore (and only DSSE on verification).
I want to push this now so that people can start playing with it while I
update the Colab demo and try to make the API more uniform and add
support for the missing cases.

With this, the default signing should be as simple as

```
from model_signing import api as model_signing_api

model_signing_api.sign(model_path, signature_path)
```

And verification would be as simple as:

```
from model_signing import api as model_signing_api

model_signing_api.verify(
  model_path, signature_path,
  expected_identity, expected_oidc_provider
)
```

Will think on how to simplify this even further..

Signed-off-by: Mihai Maruseac <[email protected]>
@mihaimaruseac mihaimaruseac requested review from a team as code owners November 18, 2024 23:47
@mihaimaruseac mihaimaruseac added this to the V1 release milestone Nov 19, 2024
Copy link
Contributor

@spencerschrock spencerschrock left a comment

Choose a reason for hiding this comment

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

Will think on how to simplify this even further.

I think the main thing that stands out to me now is that Signing/Verifying is a high level wrapper that tends to be simple, but the devil is in the Hashing details. The HashingConfig seems like a copy of the serialization API.

I assume the aim is to be explicit about what is backwards compatible by semver? Either way that seems like the rough point to me currently.

src/model_signing/api.py Show resolved Hide resolved
src/model_signing/api.py Outdated Show resolved Hide resolved
src/model_signing/api.py Outdated Show resolved Hide resolved
src/model_signing/api.py Show resolved Hide resolved
src/model_signing/api.py Show resolved Hide resolved
src/model_signing/api.py Show resolved Hide resolved
src/model_signing/api.py Show resolved Hide resolved
src/model_signing/api.py Show resolved Hide resolved
src/model_signing/api.py Show resolved Hide resolved
src/model_signing/api.py Outdated Show resolved Hide resolved
Signed-off-by: Mihai Maruseac <[email protected]>
@mihaimaruseac
Copy link
Collaborator Author

I think the main thing that stands out to me now is that Signing/Verifying is a high level wrapper that tends to be simple, but the devil is in the Hashing details.

That is true. For most usecases, the config should not be used to change anything from the defaults, but we want to allow flexibility.

The HashingConfig seems like a copy of the serialization API.

I'm planning to make it even more uniform in the future, having setters for each field and then a verification method just before the first signing event. But for that I'd first need to make the serialization API more uniform.

I assume the aim is to be explicit about what is backwards compatible by semver? Either way that seems like the rough point to me currently.

Yup. We can then change anything else in the library and as long as these methods have the same interface no user would be broken. In fact, probably we should make everything else private (but need to check with documentation, so let's do this only after we have all documentation done).

Copy link
Contributor

@spencerschrock spencerschrock left a comment

Choose a reason for hiding this comment

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

I'm planning to make it even more uniform in the future

As we iterate here on the stable API, what's the convention in PyPI? Anything under 1.0 can expect breaking changes?

@mihaimaruseac
Copy link
Collaborator Author

Anything under 1.0 can expect breaking changes?

Yeah, we'll follow https://semver.org/#semantic-versioning-specification-semver

Thank you for the review!

@mihaimaruseac mihaimaruseac merged commit 76cfac8 into sigstore:main Nov 19, 2024
21 checks passed
@mihaimaruseac mihaimaruseac deleted the api branch November 19, 2024 18:24
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