feat: expose ClientTrustConfig as a public class

[SequeI]

Summary

Exposing ClientTrustConfig. Since recent changes to SigningContext.staging/production being moved to ClientTrustConfig, it would be better for this class to be public as it is/will be used in Model-Transparency and such. The API has had massive changes as is, so I think it would be better doing this now for next release.

Resolves #1019

Release Note

Added to changelog.md

The ClientTrustConfig class has been moved from the private _internal package to a new public
module (sigstore.trust). This change formally adds the class to the project's public API,
making it available for use in other projects.

[SequeI]

Noticed the rekorV2 tests are wishy washy

[jku]

Noticed the rekorV2 tests are wishy washy

If you mean that the signing tests using staging rekor2 fail with 50x errors more often then is reasonable: yes this seems to be the case. I'm following up in sigstore/rekor-tiles repo

[SequeI]

sizeable new main commit; rebased pr

[woodruffw]

/gcbrun

[woodruffw]

Thanks @SequeI!

Exposing this as public makes sense to me. Two thoughts:

• What do you think about exposing this via sigstore.models instead? I've tried to centralize more of our data models there, especially when shared between signing and verification. The upside is fewer top-level modules in the sigstore.* namespace; the downside is that module gets chunkier.
• Is there anything else that we should expose from sigstore._internal.trust? I can't think of anything else off the top of my head, but I figure you'd have a better sense as a downstream user.

[woodruffw]

@SequeI Could you avoid force-pushing unless absolutely necessary? It causes a notification to me (and presumably anything one else subscribed on the PR) each time you do 🙂

(It's okay to have a non-clean history on this PR -- we'll clean it up with the actual merge.)

[SequeI]

@woodruffw Apologies! Won't do that anymore

Would it make sense to have a new public module (trust like previously) and just expose trustedRoot and SigningConfig alongside ClientTrustConfig due to it's dependence on it? Putting it into models creates a lot of circular import errors with rekor and rekorv2 unfortunately

[woodruffw]

Putting it into models creates a lot of circular import errors with rekor and rekorv2 unfortunately

Hmm, could you paste those? I think we'd ideally eliminate those potential circularities are part of v4 anyways; they should all be refactorable.

[SequeI]

Sure :

    from sigstore._internal.rekor.client import RekorClient
../../.local/lib/python3.13/site-packages/sigstore/_internal/rekor/client.py:41: in <module>
    from sigstore.models import TransparencyLogEntry
../../.local/lib/python3.13/site-packages/sigstore/models.py:65: in <module>
    from sigstore._internal.trust import SigningConfig, TrustedRoot
../../.local/lib/python3.13/site-packages/sigstore/_internal/trust.py:43: in <module>
    from sigstore._internal.rekor.client_v2 import RekorV2Client
../../.local/lib/python3.13/site-packages/sigstore/_internal/rekor/client_v2.py:41: in <module>
    from sigstore.models import TransparencyLogEntry
E   ImportError: cannot import name 'TransparencyLogEntry' from partially initialized module 'sigstore.models' (most likely due to a circular import) (/home/.local/lib/python3.13/site-packages/sigstore/models.py)

test/unit/conftest.py:40: in <module>
    from sigstore._internal.rekor.client import RekorClient
../../.local/lib/python3.13/site-packages/sigstore/_internal/rekor/client.py:41: in <module>
    from sigstore.models import TransparencyLogEntry
../../.local/lib/python3.13/site-packages/sigstore/models.py:65: in <module>
    from sigstore._internal.trust import SigningConfig, TrustedRoot
../../.local/lib/python3.13/site-packages/sigstore/_internal/trust.py:44: in <module>
    from sigstore._internal.rekor.client import RekorClient
E   ImportError: cannot import name 'RekorClient' from partially initialized module 'sigstore._internal.rekor.client' (most likely due to a circular import) (/home/.local/lib/python3.13/site-packages/sigstore/_internal/rekor/client.py)

I wasn't sure if this would be in the scope of the PR, or if I should fix this etc, but I'll fix these in this case

[woodruffw]

I wasn't sure if this would be in the scope of the PR, or if I should fix this etc, but I'll fix these in this case

Thank you! Yes, I'd consider it in scope, and I appreciate you fixing them 🙂

[SequeI]

Resolved; I had thought it would be much more involved but was just a small issue with RekorV1/V2 placement

[jku]

This change formally adds the class to the project's public API, making it available for use in other projects.

I agree with this goal but I'm not sure exposing just TrustConfig is that useful: this PR in practice also makes TrustedRoot and SigningConfig public (since they are available through TrustConfig properties, and we expect users to uses those properties) but now those two classes still live in _internal making it look in the repo like they are not public API...

Maybe that's not too bad since the only thing we expect users to do with TrustedRoot/SigningConfig is to provide them to other sigstore components as arguments?

[SequeI]

Maybe that's not too bad since the only thing we expect users to do with TrustedRoot/SigningConfig is to provide them to other sigstore components as arguments?

@jku Not sure about other use cases, but the ClientTrustConfig is the only things required in model-transparency and sigstore-a2a to properly use staging/production/trust_config for signing once 4.0.0 releases. I haven't really noticed a use for TrustedRoot or SigningConfig ever since you made trust_config be used everywhere in the project (Within clientrustconfig)

[jku]

I've been avoiding reviewing since both of these feel like code smells:

• ClientTrustConfig is moved to models and SigningConfig/TrustedRoot remain in _internal -- all of them are essentially public API now though
• the import within function

That said, I've tried to come up with better solutions and so far have no better ideas: API publicity was discussed already and the import trick seems required if we want to keep the rekor client selection a SigningConfig implementation detail.

so I'm happy with this. @woodruffw do you want to have another look?

[SequeI]

Any further communication regarding this PR? I don't mind changing anything if it would make better sense for the codebase

[woodruffw]

• ClientTrustConfig is moved to models and SigningConfig/TrustedRoot remain in _internal -- all of them are essentially public API now though

I think we have two options here:

1. We could explicitly mark ClientTrustConfig.trusted_root and ClientTrustConfig.signing_config as private APIs, i.e. with @private like elsewhere in the codebase.
2. We could make these types public as part of sigstore.models as well.

I don't have a strong preference between these -- (1) is probably simpler but we'll likely want to do (2) eventually anyways so maybe we should just get it out of the way.

the import within function

Yeah, I don't love this -- I (personally) find function-scoped imports to be a code smell and much harder to debug (since they modify global import state "late" in execution).

I'm OK with merging here if there's no clean solution, but I'd like to better understand where the import cycle is here so that we can eventually fix it.

[SequeI]

Apologies for the force push, just easiest to do being 20 commits behind main etc.

I have moved over the appropriate classes to sigstore.models, and the main issue regarding circular imports is TransparencyLogEntry.

sigstore._internal.rekor.client import RekorClient
->
from sigstore.models import TransparencyLogEntry
->
from sigstore._internal.rekor.client import RekorClient

This is being hit due to SigningConfig being moved over to sigstore.models and requiring the RekorClient imports now within the file.

I have included a lazy import within the PR for now, just so it works and we can open an issue for this, or if you have any insight since you are much more experienced with this codebase, fix it within this PR.

[jku]

/gcbrun

[jku]

I have filed an issue on the deferred import and will merge this as is. Thanks!