Skip to content

Provide a non-owning constructor for Issuer #362

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

Merged
merged 1 commit into from
Jul 4, 2025
Merged

Conversation

p-avital
Copy link
Contributor

@p-avital p-avital commented Jul 3, 2025

Hey rcgen folks!

I must say I really like what you've done with the new API changes, but I'm really missing a way to sign certificates without giving the issuer ownership of the KeyPair (especially frustrating since it's not Clone).

Maybe I missed an alternative (maybe there's a newtype around a reference that directly impls SigningKey), but this seems like a straight forward enough change.

Do keep in mind that if you accept this PR, you're probably locking yourself into having these MaybeOwned in the internal type representation (just thought I'd warn you :) ).

Note: feel free to reject this PR if you don't want that extra surface, I'm gonna newtype &KeyPair to get that copy-less use in my project :)

@djc
Copy link
Member

djc commented Jul 4, 2025

(You might also be interested in #363.)

Copy link
Member

@djc djc left a comment

Choose a reason for hiding this comment

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

Thanks! Can you squash your commits, please?

@p-avital
Copy link
Contributor Author

p-avital commented Jul 4, 2025

Gah!
I have no way to do that without from GH (I didn't actually download the repo for such a simple PR) D:
Any chance you can use the Squash & Merge option on GH? You should have it if you unfold the merge button :)

@djc
Copy link
Member

djc commented Jul 4, 2025

Gah! I have no way to do that without from GH (I didn't actually download the repo for such a simple PR) D: Any chance you can use the Squash & Merge option on GH? You should have it if you unfold the merge button :)

We normally use the merge queue, which doesn't make this easy, but we can bypass that if squashing is too hard for you.

@p-avital
Copy link
Contributor Author

p-avital commented Jul 4, 2025

Aaaand squashed :)

@djc djc requested review from cpu and est31 July 4, 2025 12:38
Copy link
Member

@est31 est31 left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Member

@cpu cpu left a comment

Choose a reason for hiding this comment

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

Thank you :-)

@cpu cpu added this pull request to the merge queue Jul 4, 2025
Merged via the queue into rustls:main with commit c2283bb Jul 4, 2025
16 checks passed
@djc djc mentioned this pull request Jul 10, 2025
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.

4 participants