You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "sagnghos (via GitHub)" <gi...@apache.org> on 2026/06/11 18:15:44 UTC
[PR] Spanner: Extend SpannerIO to support Spanner OMNI mTLS setup [beam]
sagnghos opened a new pull request, #38928:
URL: https://github.com/apache/beam/pull/38928
This PR introduces capabilities to connect to Spanner Omni endpoints using mTLS and enables the integration test suite to validate against [Spanner Omni](https://docs.cloud.google.com/spanner-omni/overview).
To run IT against Spanner Omni:
- plain-text setup
```
SPANNER_OMNI_USE_PLAIN_TEXT=true SPANNER_OMNI_ENDPOINT=http://localhost:15000 ./gradlew :sdks:java:io:google-cloud-platform:integrationTest --tests "org.apache.beam.sdk.io.gcp.spanner.*IT" --info
```
- TLS setup
```
SPANNER_OMNI_ENDPOINT=http://localhost:15000 ./gradlew :sdks:java:io:google-cloud-platform:integrationTest --tests "org.apache.beam.sdk.io.gcp.spanner.*IT" --info
```
- mTLS setup
```
SPANNER_OMNI_CLIENT_CERT=/tmp/omni/certs/client.crt SPANNER_OMNI_CLIENT_KEY=/tmp/omni/certs/client.key.pkcs8 SPANNER_OMNI_ENDPOINT=http://localhost:15000 ./gradlew :sdks:java:io:google-cloud-platform:integrationTest --tests "org.apache.beam.sdk.io.gcp.spanner.*IT" --info
```
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Go+tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI or the [workflows README](https://github.com/apache/beam/blob/master/.github/workflows/README.md) to see a list of phrases to trigger workflows.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Spanner: Extend SpannerIO to support Spanner Omni mTLS setup [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #38928:
URL: https://github.com/apache/beam/pull/38928#issuecomment-4687659548
Assigning reviewers:
R: @ahmedabu98 for label java.
R: @nielm for label spanner.
Note: If you would like to opt out of this review, comment `assign to next reviewer`.
Available commands:
- `stop reviewer notifications` - opt out of the automated review tooling
- `remind me after tests pass` - tag the comment author after tests pass
- `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)
The PR bot will only process comments in the main thread (not review comments).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Spanner: Extend SpannerIO to support Spanner Omni mTLS setup [beam]
Posted by "sagnghos (via GitHub)" <gi...@apache.org>.
sagnghos commented on code in PR #38928:
URL: https://github.com/apache/beam/pull/38928#discussion_r3398326905
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java:
##########
@@ -249,6 +249,14 @@ static SpannerOptions buildSpannerOptions(SpannerConfig spannerConfig) {
builder.setChannelConfigurator(b -> b.usePlaintext());
builder.setCredentials(NoCredentials.getInstance());
}
+ ValueProvider<String> clientCert = spannerConfig.getClientCertPath();
+ ValueProvider<String> clientKey = spannerConfig.getClientCertKeyPath();
+ if (clientCert != null
+ && clientKey != null
+ && !Strings.isNullOrEmpty(clientCert.get())
+ && !Strings.isNullOrEmpty(clientKey.get())) {
+ builder.useClientCert(clientCert.get(), clientKey.get());
+ }
}
Review Comment:
This is a completely wrong review. The if block ending at 260 is for experimentalHost != null
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Spanner: Extend SpannerIO to support Spanner Omni mTLS setup [beam]
Posted by "sagnghos (via GitHub)" <gi...@apache.org>.
sagnghos commented on code in PR #38928:
URL: https://github.com/apache/beam/pull/38928#discussion_r3398513088
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerTransformRegistrar.java:
##########
@@ -110,6 +112,14 @@ public void setPlainText(@Nullable Boolean plainText) {
this.plainText = plainText;
}
+ public void setClientCertPath(@Nullable String clientCertPath) {
+ this.clientCertPath = clientCertPath;
+ }
+
+ public void setClientCertKeyPath(@Nullable String clientCertKeyPath) {
+ this.clientCertKeyPath = clientCertKeyPath;
+ }
+
void checkMandatoryFields() {
if (projectId.isEmpty()) {
throw new IllegalArgumentException("projectId can't be empty");
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Spanner: Extend SpannerIO to support Spanner Omni mTLS setup [beam]
Posted by "gemini-code-assist[bot] (via GitHub)" <gi...@apache.org>.
gemini-code-assist[bot] commented on code in PR #38928:
URL: https://github.com/apache/beam/pull/38928#discussion_r3398289362
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java:
##########
@@ -367,6 +376,7 @@ public SpannerConfig withExperimentalHost(ValueProvider<String> experimentalHost
return toBuilder()
.setInstanceId(EXPERIMENTAL_HOST_INSTANCE_ID)
.setExperimentalHost(experimentalHost)
+ .setCredentials(ValueProvider.StaticValueProvider.of(NoCredentials.getInstance()))
.build();
Review Comment:
Unconditionally setting `NoCredentials` inside `withExperimentalHost` makes the configuration order-dependent. If a user configures custom credentials first and then calls `withExperimentalHost`, their credentials will be silently overwritten.
Instead of forcing `NoCredentials` here, consider removing this line and defaulting to `NoCredentials` in `SpannerAccessor.buildSpannerOptions` when an `experimentalHost` is present and no custom credentials have been provided.
```java
return toBuilder()
.setInstanceId(EXPERIMENTAL_HOST_INSTANCE_ID)
.setExperimentalHost(experimentalHost)
.build();
```
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerTransformRegistrar.java:
##########
@@ -110,6 +112,14 @@ public void setPlainText(@Nullable Boolean plainText) {
this.plainText = plainText;
}
+ public void setClientCertPath(@Nullable String clientCertPath) {
+ this.clientCertPath = clientCertPath;
+ }
+
+ public void setClientCertKeyPath(@Nullable String clientCertKeyPath) {
+ this.clientCertKeyPath = clientCertKeyPath;
+ }
+
void checkMandatoryFields() {
if (projectId.isEmpty()) {
throw new IllegalArgumentException("projectId can't be empty");
Review Comment:
If a user of the cross-language transform specifies only one of `clientCertPath` or `clientCertKeyPath`, the configuration will be silently ignored because the build methods only apply them if both are non-null.
Add a validation check in `checkMandatoryFields()` to ensure that either both or neither of these fields are specified.
```java
void checkMandatoryFields() {
if (projectId.isEmpty()) {
throw new IllegalArgumentException("projectId can't be empty");
}
if ((clientCertPath != null) != (clientCertKeyPath != null)) {
throw new IllegalArgumentException(
"Both clientCertPath and clientCertKeyPath must be specified together.");
}
}
```
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/MetadataSpannerConfigFactory.java:
##########
@@ -87,6 +87,15 @@ public static SpannerConfig create(
config = config.withUsingPlainTextChannel(plainText.get());
}
+ ValueProvider<String> clientCert = primaryConfig.getClientCertPath();
+ ValueProvider<String> clientKey = primaryConfig.getClientCertKeyPath();
+ if (clientCert != null
+ && clientCert.get() != null
+ && clientKey != null
+ && clientKey.get() != null) {
+ config = config.withClientCert(clientCert.get(), clientKey.get());
+ }
Review Comment:
Instead of calling `.get()` on the `ValueProvider`s, you can pass the `ValueProvider` instances directly to `withClientCert(ValueProvider<String>, ValueProvider<String>)`. This avoids unnecessary evaluation of the providers and is safer if the configuration is ever initialized or validated during pipeline construction.
```java
ValueProvider<String> clientCert = primaryConfig.getClientCertPath();
ValueProvider<String> clientKey = primaryConfig.getClientCertKeyPath();
if (clientCert != null && clientKey != null) {
config = config.withClientCert(clientCert, clientKey);
}
```
##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerTestHelper.java:
##########
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.spanner;
+
+import com.google.cloud.spanner.SpannerOptions;
+import org.apache.beam.sdk.extensions.gcp.options.GcpOptions;
+import org.apache.beam.sdk.options.PipelineOptions;
+import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Strings;
+
+/** Helper methods for Spanner IT tests to support Spanner Omni natively. */
+public class SpannerTestHelper {
+
+ public static String getOmniEndpoint() {
+ return System.getenv("SPANNER_OMNI_ENDPOINT");
+ }
+
+ public static boolean isOmni() {
+ return !Strings.isNullOrEmpty(getOmniEndpoint());
+ }
+
+ public static String getOmniClientCert() {
+ return System.getenv("SPANNER_OMNI_CLIENT_CERT");
+ }
+
+ public static String getOmniClientKey() {
+ return System.getenv("SPANNER_OMNI_CLIENT_KEY");
+ }
+
+ public static boolean isOmniUsePlainText() {
+ return Boolean.parseBoolean(System.getenv("SPANNER_OMNI_USE_PLAIN_TEXT"));
+ }
+
+ public static String getProject(PipelineOptions options, String instanceProjectId) {
+ if (isOmni()) {
+ return "default";
+ }
+ String project = instanceProjectId;
+ if (project == null) {
+ project = options.as(GcpOptions.class).getProject();
+ }
+ return project;
+ }
+
+ public static String getInstanceId(String instanceId) {
+ if (isOmni()) {
+ return "default";
+ }
+ return instanceId;
+ }
+
+ public static SpannerOptions.Builder setUpSpannerOptions(SpannerOptions.Builder builder) {
+ if (isOmni()) {
+ builder.setExperimentalHost(getOmniEndpoint());
+ if (isOmniUsePlainText()) {
+ builder.usePlainText();
+ }
+ String cert = getOmniClientCert();
+ String key = getOmniClientKey();
+ if (!Strings.isNullOrEmpty(cert) && !Strings.isNullOrEmpty(key)) {
+ builder.useClientCert(cert, key);
+ }
+ }
+ return builder;
+ }
Review Comment:
When `isOmni()` is true, we should explicitly set `NoCredentials` on the `SpannerOptions.Builder` so that the direct Java SDK client doesn't attempt to load Google Application Default Credentials (ADC), which might fail or slow down the tests in environments without GCP credentials.
```suggestion
public static SpannerOptions.Builder setUpSpannerOptions(SpannerOptions.Builder builder) {
if (isOmni()) {
builder.setExperimentalHost(getOmniEndpoint());
builder.setCredentials(com.google.cloud.NoCredentials.getInstance());
if (isOmniUsePlainText()) {
builder.usePlainText();
}
String cert = getOmniClientCert();
String key = getOmniClientKey();
if (!Strings.isNullOrEmpty(cert) && !Strings.isNullOrEmpty(key)) {
builder.useClientCert(cert, key);
}
}
return builder;
}
```
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java:
##########
@@ -249,6 +249,14 @@ static SpannerOptions buildSpannerOptions(SpannerConfig spannerConfig) {
builder.setChannelConfigurator(b -> b.usePlaintext());
builder.setCredentials(NoCredentials.getInstance());
}
+ ValueProvider<String> clientCert = spannerConfig.getClientCertPath();
+ ValueProvider<String> clientKey = spannerConfig.getClientCertKeyPath();
+ if (clientCert != null
+ && clientKey != null
+ && !Strings.isNullOrEmpty(clientCert.get())
+ && !Strings.isNullOrEmpty(clientKey.get())) {
+ builder.useClientCert(clientCert.get(), clientKey.get());
+ }
}
Review Comment:
The client certificate configuration is currently placed inside the `if (plainText != null)` block (which ends at line 260). Since `plainText` is `null` by default (unless explicitly configured), this entire block will be skipped for standard mTLS configurations where `plainText` is not set. This prevents the client certificates from being applied to the `SpannerOptions` builder.
To fix this, move the client certificate configuration outside of the `plainText` block so that it is evaluated independently. Additionally, we should validate that both `clientCertPath` and `clientCertKeyPath` are provided together to avoid silent failures.
```java
}
}
ValueProvider<String> clientCert = spannerConfig.getClientCertPath();
ValueProvider<String> clientKey = spannerConfig.getClientCertKeyPath();
boolean hasCert = clientCert != null && !Strings.isNullOrEmpty(clientCert.get());
boolean hasKey = clientKey != null && !Strings.isNullOrEmpty(clientKey.get());
if (hasCert != hasKey) {
throw new IllegalArgumentException(
"Both clientCertPath and clientCertKeyPath must be provided for mTLS.");
}
if (hasCert && hasKey) {
builder.useClientCert(clientCert.get(), clientKey.get());
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Spanner: Extend SpannerIO to support Spanner Omni mTLS setup [beam]
Posted by "gemini-code-assist[bot] (via GitHub)" <gi...@apache.org>.
gemini-code-assist[bot] commented on code in PR #38928:
URL: https://github.com/apache/beam/pull/38928#discussion_r3400784852
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java:
##########
@@ -249,6 +249,14 @@ static SpannerOptions buildSpannerOptions(SpannerConfig spannerConfig) {
builder.setChannelConfigurator(b -> b.usePlaintext());
builder.setCredentials(NoCredentials.getInstance());
}
+ ValueProvider<String> clientCert = spannerConfig.getClientCertPath();
+ ValueProvider<String> clientKey = spannerConfig.getClientCertKeyPath();
+ if (clientCert != null
+ && clientKey != null
+ && !Strings.isNullOrEmpty(clientCert.get())
+ && !Strings.isNullOrEmpty(clientKey.get())) {
+ builder.useClientCert(clientCert.get(), clientKey.get());
+ }
Review Comment:
Since clientCert and clientKey are ValueProviders, they could be RuntimeValueProviders. Calling .get() on them without checking isAccessible() can throw an IllegalStateException during pipeline construction or validation if they are not yet available. Adding isAccessible() checks prevents this issue.
```java
if (clientCert != null
&& clientKey != null
&& clientCert.isAccessible()
&& clientKey.isAccessible()
&& !Strings.isNullOrEmpty(clientCert.get())
&& !Strings.isNullOrEmpty(clientKey.get())) {
builder.useClientCert(clientCert.get(), clientKey.get());
}
```
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java:
##########
@@ -367,6 +376,7 @@ public SpannerConfig withExperimentalHost(ValueProvider<String> experimentalHost
return toBuilder()
.setInstanceId(EXPERIMENTAL_HOST_INSTANCE_ID)
.setExperimentalHost(experimentalHost)
+ .setCredentials(ValueProvider.StaticValueProvider.of(NoCredentials.getInstance()))
.build();
Review Comment:
Unconditionally setting NoCredentials in withExperimentalHost makes the fluent API order-dependent. If a user calls withCredentials(...) before withExperimentalHost(...), their credentials will be silently overwritten with NoCredentials. It is safer to remove this default from SpannerConfig and instead default to NoCredentials in SpannerAccessor.buildSpannerOptions when experimentalHost is present and no credentials are set.
```java
return toBuilder()
.setInstanceId(EXPERIMENTAL_HOST_INSTANCE_ID)
.setExperimentalHost(experimentalHost)
.build();
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Spanner: Extend SpannerIO to support Spanner OMNI mTLS setup [beam]
Posted by "sagnghos (via GitHub)" <gi...@apache.org>.
sagnghos commented on PR #38928:
URL: https://github.com/apache/beam/pull/38928#issuecomment-4683636842
/gemini review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Spanner: Extend SpannerIO to support Spanner Omni mTLS setup [beam]
Posted by "sagnghos (via GitHub)" <gi...@apache.org>.
sagnghos commented on code in PR #38928:
URL: https://github.com/apache/beam/pull/38928#discussion_r3398330721
##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/spanner/SpannerTestHelper.java:
##########
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.beam.sdk.io.gcp.spanner;
+
+import com.google.cloud.spanner.SpannerOptions;
+import org.apache.beam.sdk.extensions.gcp.options.GcpOptions;
+import org.apache.beam.sdk.options.PipelineOptions;
+import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Strings;
+
+/** Helper methods for Spanner IT tests to support Spanner Omni natively. */
+public class SpannerTestHelper {
+
+ public static String getOmniEndpoint() {
+ return System.getenv("SPANNER_OMNI_ENDPOINT");
+ }
+
+ public static boolean isOmni() {
+ return !Strings.isNullOrEmpty(getOmniEndpoint());
+ }
+
+ public static String getOmniClientCert() {
+ return System.getenv("SPANNER_OMNI_CLIENT_CERT");
+ }
+
+ public static String getOmniClientKey() {
+ return System.getenv("SPANNER_OMNI_CLIENT_KEY");
+ }
+
+ public static boolean isOmniUsePlainText() {
+ return Boolean.parseBoolean(System.getenv("SPANNER_OMNI_USE_PLAIN_TEXT"));
+ }
+
+ public static String getProject(PipelineOptions options, String instanceProjectId) {
+ if (isOmni()) {
+ return "default";
+ }
+ String project = instanceProjectId;
+ if (project == null) {
+ project = options.as(GcpOptions.class).getProject();
+ }
+ return project;
+ }
+
+ public static String getInstanceId(String instanceId) {
+ if (isOmni()) {
+ return "default";
+ }
+ return instanceId;
+ }
+
+ public static SpannerOptions.Builder setUpSpannerOptions(SpannerOptions.Builder builder) {
+ if (isOmni()) {
+ builder.setExperimentalHost(getOmniEndpoint());
+ if (isOmniUsePlainText()) {
+ builder.usePlainText();
+ }
+ String cert = getOmniClientCert();
+ String key = getOmniClientKey();
+ if (!Strings.isNullOrEmpty(cert) && !Strings.isNullOrEmpty(key)) {
+ builder.useClientCert(cert, key);
+ }
+ }
+ return builder;
+ }
Review Comment:
This is taken care by the Java Client SDK code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Spanner: Extend SpannerIO to support Spanner Omni mTLS setup [beam]
Posted by "sagnghos (via GitHub)" <gi...@apache.org>.
sagnghos commented on code in PR #38928:
URL: https://github.com/apache/beam/pull/38928#discussion_r3401123843
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerAccessor.java:
##########
@@ -249,6 +249,14 @@ static SpannerOptions buildSpannerOptions(SpannerConfig spannerConfig) {
builder.setChannelConfigurator(b -> b.usePlaintext());
builder.setCredentials(NoCredentials.getInstance());
}
+ ValueProvider<String> clientCert = spannerConfig.getClientCertPath();
+ ValueProvider<String> clientKey = spannerConfig.getClientCertKeyPath();
+ if (clientCert != null
+ && clientKey != null
+ && !Strings.isNullOrEmpty(clientCert.get())
+ && !Strings.isNullOrEmpty(clientKey.get())) {
+ builder.useClientCert(clientCert.get(), clientKey.get());
+ }
Review Comment:
Done
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java:
##########
@@ -367,6 +376,7 @@ public SpannerConfig withExperimentalHost(ValueProvider<String> experimentalHost
return toBuilder()
.setInstanceId(EXPERIMENTAL_HOST_INSTANCE_ID)
.setExperimentalHost(experimentalHost)
+ .setCredentials(ValueProvider.StaticValueProvider.of(NoCredentials.getInstance()))
.build();
Review Comment:
Done
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java:
##########
@@ -367,6 +376,7 @@ public SpannerConfig withExperimentalHost(ValueProvider<String> experimentalHost
return toBuilder()
.setInstanceId(EXPERIMENTAL_HOST_INSTANCE_ID)
.setExperimentalHost(experimentalHost)
+ .setCredentials(ValueProvider.StaticValueProvider.of(NoCredentials.getInstance()))
.build();
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Spanner: Extend SpannerIO to support Spanner Omni mTLS setup [beam]
Posted by "sagnghos (via GitHub)" <gi...@apache.org>.
sagnghos commented on code in PR #38928:
URL: https://github.com/apache/beam/pull/38928#discussion_r3398512254
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/changestreams/MetadataSpannerConfigFactory.java:
##########
@@ -87,6 +87,15 @@ public static SpannerConfig create(
config = config.withUsingPlainTextChannel(plainText.get());
}
+ ValueProvider<String> clientCert = primaryConfig.getClientCertPath();
+ ValueProvider<String> clientKey = primaryConfig.getClientCertKeyPath();
+ if (clientCert != null
+ && clientCert.get() != null
+ && clientKey != null
+ && clientKey.get() != null) {
+ config = config.withClientCert(clientCert.get(), clientKey.get());
+ }
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Spanner: Extend SpannerIO to support Spanner Omni mTLS setup [beam]
Posted by "ahmedabu98 (via GitHub)" <gi...@apache.org>.
ahmedabu98 merged PR #38928:
URL: https://github.com/apache/beam/pull/38928
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Spanner: Extend SpannerIO to support Spanner Omni mTLS setup [beam]
Posted by "sagnghos (via GitHub)" <gi...@apache.org>.
sagnghos commented on code in PR #38928:
URL: https://github.com/apache/beam/pull/38928#discussion_r3398315251
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/spanner/SpannerConfig.java:
##########
@@ -367,6 +376,7 @@ public SpannerConfig withExperimentalHost(ValueProvider<String> experimentalHost
return toBuilder()
.setInstanceId(EXPERIMENTAL_HOST_INSTANCE_ID)
.setExperimentalHost(experimentalHost)
+ .setCredentials(ValueProvider.StaticValueProvider.of(NoCredentials.getInstance()))
.build();
Review Comment:
We need to set it to NoCredentials by default unconditionally as we don't want Cloud credentials to be sent to Omni by mistake which is the default behaviour
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Spanner: Extend SpannerIO to support Spanner Omni mTLS setup [beam]
Posted by "gemini-code-assist[bot] (via GitHub)" <gi...@apache.org>.
gemini-code-assist[bot] commented on PR #38928:
URL: https://github.com/apache/beam/pull/38928#issuecomment-4687588906
## Summary of Changes
Hello, I'm Gemini Code Assist[^1]! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request enables mTLS authentication for Spanner Omni by extending the SpannerConfig and SpannerIO APIs. It provides a standardized way to pass client certificate and key paths, ensuring secure connectivity for Spanner Omni users. Additionally, it refactors the integration test suite to dynamically support Spanner Omni configurations, facilitating easier validation in secure environments.
### Highlights
* **mTLS Support for Spanner Omni**: Added support for configuring client certificates and keys in SpannerConfig to enable mTLS connections to Spanner Omni.
* **API Extensions**: Extended SpannerIO transforms (Read, Write, ReadAll, etc.) to allow users to specify mTLS credentials via new withClientCert methods.
* **Integration Test Improvements**: Introduced SpannerTestHelper to centralize Spanner Omni configuration and updated existing integration tests to support mTLS-enabled environments.
<details>
<summary><b>New Features</b></summary>
🧠You can now enable **Memory** (public preview) to help **Gemini Code Assist** learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. **Click [here](https://codeassist.google/code-review/login) to enable Memory in your admin console.**
</details>
<details>
<summary><b>Using Gemini Code Assist</b></summary>
<br>
The full guide for Gemini Code Assist can be found on our [documentation page](https://developers.google.com/gemini-code-assist/docs/review-github-code), here are some quick tips.
<b>Invoking Gemini</b>
You can request assistance from Gemini at any point by creating a comment using either `/gemini <command>` or `@gemini-code-assist <command>`. Below is a summary of the supported commands on the current page.
Feature | Command | Description
--- | --- | ---
Code Review | `/gemini review` | Performs a code review for the current pull request in its current state.
Pull Request Summary | `/gemini summary` | Provides a summary of the current pull request in its current state.
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help | `/gemini help` | Displays a list of available commands.
<b>Customization</b>
To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a `.gemini/` folder in the base of the repository. Detailed instructions can be found [here](https://developers.google.com/gemini-code-assist/docs/customize-gemini-behavior-github).
<b>Limitations & Feedback</b>
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with :thumbsup: and :thumbsdown: on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up [here](https://google.qualtrics.com/jfe/form/SV_2cyuGuTWsEw84yG).
</details>
[^1]: Review the [Privacy Notices](https://policies.google.com/privacy), [Generative AI Prohibited Use Policy](https://policies.google.com/terms/generative-ai/use-policy), [Terms of Service](https://policies.google.com/terms), and learn how to configure Gemini Code Assist in GitHub [here](https://developers.google.com/gemini-code-assist/docs/customize-gemini-behavior-github). Gemini can make mistakes, so double check it and [use code with caution](https://support.google.com/legal/answer/13505487).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org