Skip to content

Provide a non-owning constructor for Issuer#362

Merged
cpu merged 1 commit into
rustls:mainfrom
p-avital:patch-1
Jul 4, 2025
Merged

Provide a non-owning constructor for Issuer#362
cpu merged 1 commit into
rustls:mainfrom
p-avital:patch-1

Conversation

@p-avital

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

Copy link
Copy Markdown
Contributor

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 :)

Comment thread rcgen/src/lib.rs Outdated
Comment thread rcgen/src/lib.rs Outdated
@djc

djc commented Jul 4, 2025

Copy link
Copy Markdown
Member

(You might also be interested in #363.)

@djc djc left a comment

Copy link
Copy Markdown
Member

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

p-avital commented Jul 4, 2025

Copy link
Copy Markdown
Contributor Author

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

djc commented Jul 4, 2025

Copy link
Copy Markdown
Member

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

p-avital commented Jul 4, 2025

Copy link
Copy Markdown
Contributor Author

Aaaand squashed :)

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

@est31 est31 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

@cpu cpu left a comment

Copy link
Copy Markdown
Member

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