Include concatenated certificate+key PEM file in TLS contexts#18216
Include concatenated certificate+key PEM file in TLS contexts#18216aradalvand wants to merge 4 commits into
Conversation
aradalvand
requested review from
JamesNK,
danegsta,
davidfowl,
karolz-ms and
mitchdenny
as code owners
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 18216Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 18216" |
aradalvand
force-pushed
the
certificate-with-key-pem
branch
from
05b41e1 to
06b9d9f
Compare
aradalvand
force-pushed
the
certificate-with-key-pem
branch
from
06b9d9f to
5cbbe33
Compare
aradalvand
changed the title
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Any guidance on how the cross-language tests are supposed to be fixed? |
aradalvand
force-pushed
the
certificate-with-key-pem
branch
from
9950095 to
0111686
Compare
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
| /// A value provider that will resolve to a path to the the certificate and key concatenated together in PEM format. | ||
| /// </summary> | ||
| [AspireExport] | ||
| public required ReferenceExpression CertificateWithKeyPath { get; init; } |
There was a problem hiding this comment.
There are tests that check for unexpected changes in the polyglot AppHost generated code for all the current supported and experimental languages. You'll need to re-run the CodeGeneration test cases for each of the supported languages and accept the new snapshots with dotnet verify accept -y.
There was a problem hiding this comment.
Is there any way to selectively run the code-generating tests only?
Even if not, how do I exactly find them? Is there a consistent convention? I couldn't find any specific instructions related to this in CONTRIBUTING.md.
There was a problem hiding this comment.
They're a bit awkward and not well documented; it's the Hosting.CodeGeneration.* tests (which test multiple different languages, including ones that are still early experimental) to ensure we don't accidentally regress anything when making API changes.
| /// <summary> | ||
| /// Indicates whether the certificate-with-key path was actually referenced in the resource configuration. | ||
| /// </summary> | ||
| public bool IsCertificateWithKeyPathReferenced => _trackedCertificateWithKeyPathReference?.WasResolved ?? false; |
There was a problem hiding this comment.
You'll also need to update HttpsCertificateExecutionConfigurationExportData with these properties to ensure polyglot code can reference these properties.
There was a problem hiding this comment.
By "these properties", do you mean the Is*Referenced properties? I'm confused. Are you saying these should be added to HttpsCertificateExecutionConfigurationExportData? That class never contained such properties though? Not even previously for KeyPath and PfxPath.
There was a problem hiding this comment.
It's specifically a helper used for code-generation of polyglot AppHosts (so producing the RPC compatible types for things like TypeScript apphosts). It's not an exact 1:1, but there's KeyPathExpression and PfxPathExpression, for example, plus the GetHttpsCertificateData method that populates them.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
2 participants
Description
Fixes #18215
Added a third "certificate-with-key" PEM file and its path reference to all relevant classes, namely
HttpsCertificateConfigurationCallbackAnnotationContext,ContainerFileSystemCallbackHttpsCertificateContext, andHttpsCertificateExecutionConfigurationContext. Also implemented the necessary logic for creating the file insideContainerCreatorandExecutableCreator.I included these properties in the
ExecutionConfigurationGathererTests. And I also did manually test out the whole functionality in the context of my other PR (#18196), and it works. But I couldn't find any automated tests againstContainerCreatororExecutableCreatorto write functional tests for it. So, I'm assuming there are none at the moment.Checklist
<remarks />and<code />elements on your triple slash comments?