Skip to content

test: Enable TLS for all Integration Tests and run PQC against generated Client - #1005

Open
robertvoinescu-work wants to merge 2 commits into
googleapis:mainfrom
robertvoinescu-work:test/tls-integration-tests
Open

robertvoinescu-work wants to merge 2 commits into
googleapis:mainfrom
robertvoinescu-work:test/tls-integration-tests

Conversation

@robertvoinescu-work

@robertvoinescu-work robertvoinescu-work commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

b/484351289

Validation

  • Linux Tests Run is Green
image
  • Linux PQC Tests Pass
image
  • Windows Test Run is Green
image
  • Windows PQC Tests Pass
image

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request configures the integration tests to run against the GAPIC Showcase server with TLS enabled by default. It refactors the Post-Quantum Cryptography (PQC) tests to inherit from ShowcaseTestBase, configures the gRPC adapter to bypass certificate validation, and updates the test runner script to export the Showcase self-signed certificate via SSL_CERT_FILE. Additionally, several resumable upload tests are temporarily skipped. Feedback is provided regarding a potential issue on Windows environments, where the SSL_CERT_FILE environment variable is ignored by SChannel, which will cause REST integration tests to fail unless the certificate is manually trusted or the tests are programmatically skipped on Windows.

Comment on lines 44 to 47
// RestGrpcAdapter does not forward GrpcChannelOptions.HttpHandler to its internal HttpClient,
// so REST tests require the SSL_CERT_FILE environment variable (set in runintegrationtests.sh)
// to trust Showcase's self-signed certificate.
string name when name.EndsWith("RestTest") => RestGrpcAdapter.Default,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

On Windows, .NET uses SChannel (CryptoAPI) rather than OpenSSL, which means the SSL_CERT_FILE environment variable is ignored. As a result, REST integration tests running on Windows will fail with SSL/TLS trust errors unless the self-signed certificate is manually installed in the Windows Trusted Root Certification Authorities store.

To improve the local developer experience and support running tests directly from IDEs on Linux/macOS without requiring manual environment setup, consider programmatically detecting and setting SSL_CERT_FILE in the constructor or static constructor if it is not already set:

if (Environment.GetEnvironmentVariable("SSL_CERT_FILE") == null)
{
    string currentDir = AppDomain.CurrentDomain.BaseDirectory;
    while (currentDir != null)
    {
        string pemPath = System.IO.Path.Combine(currentDir, "showcase.pem");
        if (System.IO.File.Exists(pemPath))
        {
            Environment.SetEnvironmentVariable("SSL_CERT_FILE", pemPath);
            break;
        }
        currentDir = System.IO.Path.GetDirectoryName(currentDir);
    }
}

Additionally, consider documenting this Windows limitation or programmatically skipping REST tests on Windows if the certificate is not trusted.

@robertvoinescu-work robertvoinescu-work Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not applicable anymore. To get around the windows limitation I've uniformly modified both handlers to accept any certificate without checking it is trusted (SSL_CERT_FILE or within the cert store). This allows us to avoid installing the cert to the root certificate store on windows.

@robertvoinescu-work
robertvoinescu-work force-pushed the test/tls-integration-tests branch 2 times, most recently from a3fd511 to 59dd8ce Compare September 17, 2026 19:20
@robertvoinescu-work

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request updates the integration tests to run entirely over TLS against the GAPIC Showcase server. It refactors the Post-Quantum Cryptography (PQC) tests to share a common base class, updates ShowcaseTestBase to use secure SSL credentials, and introduces reflection-based certificate bypass logic. Additionally, several resumable upload tests are temporarily skipped. Feedback on the changes highlights that the reflection logic in AcceptAnyCertificate is fragile and prone to runtime exceptions if channel or invoker is null, suggesting defensive null checks to ensure robust execution.

Comment thread Google.Api.Generator.IntegrationTests/ShowcaseTestBase.cs Outdated
@robertvoinescu-work

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the integration tests to run entirely over TLS against the GAPIC Showcase server. It updates ShowcaseTestBase to use secure SSL credentials and bypasses certificate validation via reflection on the internal HTTP handler. Additionally, PqcIntegrationTest is refactored into a base class with separate REST and gRPC test classes, and several resumable upload tests are temporarily skipped. Feedback on these changes highlights a compilation issue on .NET Framework due to the use of an unsupported string.Contains overload, and suggests improving error handling in the reflection-based certificate bypass logic to handle null handlers gracefully.

Comment thread Google.Api.Generator.IntegrationTests/PqcIntegrationTest.cs Outdated
Comment thread Google.Api.Generator.IntegrationTests/ShowcaseTestBase.cs
@robertvoinescu-work
robertvoinescu-work marked this pull request as ready for review September 17, 2026 21:46
@robertvoinescu-work
robertvoinescu-work requested a review from a team as a code owner September 17, 2026 21:46
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.

1 participant