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

Import X509 certificate and collections from PEM. #38280

Merged
merged 10 commits into from
Jun 24, 2020

Conversation

vcsjones
Copy link
Member

@vcsjones vcsjones commented Jun 23, 2020

Fixes #31944

This does not have unit tests that exercise the *File import methods. While I'd like those tested, I'm not sure of the best way to do it. Write the file out to a temp location? Or should I add a bunch of files to runtime-assets package and open a PR there? Is there documentation about validating that it works locally?

@Dotnet-GitSync-Bot
Copy link
Collaborator

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost
Copy link

ghost commented Jun 23, 2020

Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq
Notify danmosemsft if you want to be subscribed.

@bartonjs
Copy link
Member

While I'd like those tested, I'm not sure of the best way to do it. Write the file out to a temp location? Or should I add a bunch of files to runtime-assets package and open a PR there?

Start with the temp file route. After everything is happy and stable we can add some things to the testdata package for the verisimilitude of loading a file that existed on disk before the process started.

@vcsjones
Copy link
Member Author

we can add some things to the testdata package

Is there any practical benefit of doing this now if we can write temp files? My recollection from .NET Core 1.0 on Jenkins was that writing to disk was not a good idea because test interruptions would leave data behind and the environment was not ephemeral. If that's no longer true then I don't see much of a value add from adding files to the test data package.

@bartonjs
Copy link
Member

we can add some things to the testdata package

Is there any practical benefit of doing this now if we can write temp files?

Not really. Mostly aesthetic or a "feel good" scenario completion feeling. I think the current model is fine, though.

@bartonjs bartonjs merged commit 7515722 into dotnet:master Jun 24, 2020
@vcsjones vcsjones deleted the pem-x509 branch June 24, 2020 04:17
@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add easy way to create a certificate from a multi-PEM or cert-PEM + key-PEM
3 participants