chore: Exposed the internal endpoints for azurite #1004
Conversation
✅ Deploy Preview for testcontainers-dotnet ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
The https://learn.microsoft.com/en-us/dotnet/api/azure.storage.blobs.blobserviceclient.-ctor?view=azure-dotnet |
There was a problem hiding this comment.
I am fine with the changes per se, but I am wondering if the endpoint lacks the necessary auth information. The documentation related to the connectionString constructor argument says:
A connection string includes the authentication information required for your application to access data in an Azure Storage account at runtime.
I am uncertain about how to pass the auth information to the constructors that utilize, for example, the blobContainerUri argument. Perhaps we should consider expanding the documentation for these constructors to provide a clear explanation of how to use the endpoint getter?
|
AzuriteBuilder now supports the possibility to store own account credentials. Furthermore, it is now possible to communicate with the container via ❗ It may still be necessary to call |
I tried to adjust the pipeline, but there seems to be more dependencies to the |
If it makes things easier, we can add the PEM files to the test repository for now and get the pull request to a proper state. We can sort it out later. Why don't you use |
|
Ok, reworked the PR. Created a certificate with OpenSSL in the openssl req -newkey rsa:2048 -x509 -nodes -keyout server.key -new -out server.cert -sha256 -days 99999 -addext "subjectAltName=IP:127.0.0.1" -subj "/C=CO/ST=ST/L=LO/O=OR/OU=OU/CN=CN"Then uploaded the certificate locally to the certificate store and trusted it. certutil -addstore -enterprise -f Root .\server.certAs well as extended the pipeline to also trust the certificate on ubuntu based systems/runner. Unfortunately this does not seem to work as documented ... Here I would need support . 😞 |
There was a problem hiding this comment.
As well as extended the pipeline to also trust the certificate on ubuntu based systems/runner. Unfortunately this does not seem to work as documented ... Here I would need support . 😞
I did not review the CI step. I do not want to install the certificates in the CI environment because I want to avoid requiring developers to install the certificates on their machines as well to run the tests. I prefer configuring the HttpClientHandler and setting the Azurite client options accordingly. We can inherit from HttpClientHandler and implement one that configures the certificates, like (simplified version):
var serverCertificate = new X509Certificate2(Certificate.PEMPath);
var httpClientHandler = new HttpClientHandler();
httpClientHandler.ClientCertificateOptions = ClientCertificateOption.Manual;
httpClientHandler.ClientCertificates.Add(serverCertificate);
httpClientHandler.ServerCertificateCustomValidationCallback = (_, certificate, chain, _) =>
{
chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck;
chain.ChainPolicy.VerificationFlags = X509VerificationFlags.AllowUnknownCertificateAuthority;
var isVerified = chain.Build(certificate);
var isSignedByExpectedRoot = chain.ChainElements[chain.ChainElements.Count - 1].Certificate.RawData.SequenceEqual(certificate.RawData);
return isVerified && isSignedByExpectedRoot;
};
var options = new BlobClientOptions();
options.Transport = new HttpClientTransport(httpClientHandler);Additionally, I would like to encourage you to split the changes into multiple PRs. This PR includes different changes that are not dependent on each other, making the review more difficult. Specifically, implementing HTTPS necessitates more changes than the initial PR. Please reduce the changes to the bare minimum for each individual feature.
| public AzuriteBuilder WithPemCertificate(string certificatePath, string certificateKeyPath) | ||
| { | ||
| return Merge(DockerResourceConfiguration, new AzuriteConfiguration(certificatePath: certificatePath, certificateKeyPath: certificateKeyPath)) | ||
| .WithBindMount(Path.GetDirectoryName(GetType().Assembly.Location), "/internal") |
There was a problem hiding this comment.
Please do not mount a volume and, instead, utilize the WithResourceMapping(string, string) API or one of its overloaded methods to copy the required files to the container before it starts. Mounting a volume is considered an anti-pattern, it does not work properly in all Docker environments.
.WithResourceMapping(File.ReadAllBytes(clientCertificateFilePath), ClientCertificateFilePath)
.WithResourceMapping(File.ReadAllBytes(clientCertificateFilePath), ClientCertificateKeyFilePath)
.WithCommand("--cert", ClientCertificateFilePath)
.WithCommand("--key", **ClientCertificateKeyFilePath)| // Default command, if not specified the command will be completely overwritten in case of a certificate. | ||
| .WithCommand("azurite", "--blobHost", "0.0.0.0", "--queueHost", "0.0.0.0", "--tableHost", "0.0.0.0"); |
There was a problem hiding this comment.
We should consider the _enabledServices field here. This may require extending th AzuriteService class. We may not initiate every service in the future.
| private static string CleanUpPaths(string value) | ||
| { | ||
| return value.StartsWith("/internal/", StringComparison.OrdinalIgnoreCase) ? value : $"/internal/{value}"; | ||
| } |
There was a problem hiding this comment.
Utilizing WithResourceMapping(string, string) makes this method unnecessary.
| public string CertificatePath { get; } | ||
|
|
||
| public string CertificateKeyPath { get; } |
There was a problem hiding this comment.
I do not think we need to store the cert and key.
|
This pull request has been inactive for some time. I will close it for now. If you wish to continue working on it, please feel free to reopen it. |
What does this PR do?
The service specific endpoints are provided, for free use.
Why is it important?
For better use within tests.
Related issues
None