Skip to content

Commit

Permalink
Extract JDK compatibility check from BigQuery for reuse
Browse files Browse the repository at this point in the history
This will be used in Snowflake connector too in the next commit.

Co-authored-by: Mateusz "Serafin" Gajewski <[email protected]>
  • Loading branch information
hashhar and wendigo committed Jan 15, 2025
1 parent 318ce93 commit 684bb74
Show file tree
Hide file tree
Showing 3 changed files with 189 additions and 35 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/*
* Licensed 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 io.trino.plugin.base;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Multimap;
import com.google.inject.Binder;

import java.lang.management.ManagementFactory;
import java.util.List;
import java.util.regex.Pattern;

import static java.lang.String.format;
import static java.util.Objects.requireNonNull;

public class JdkCompatibilityChecks
{
private final String inputArguments;

private static final JdkCompatibilityChecks INSTANCE = new JdkCompatibilityChecks(
ManagementFactory.getRuntimeMXBean().getInputArguments());

@VisibleForTesting
JdkCompatibilityChecks(List<String> inputArguments)
{
this.inputArguments = Joiner.on(" ")
.skipNulls()
.join(requireNonNull(inputArguments, "inputArguments is null"));
}

public static void verifyConnectorAccessOpened(Binder binder, String connectorName, Multimap<String, String> modules)
{
INSTANCE.verifyAccessOpened(wrap(binder), format("Connector '%s'", connectorName), modules);
}

@VisibleForTesting
void verifyAccessOpened(ThrowableSettable throwableSettable, String description, Multimap<String, String> modules)
{
ImmutableList.Builder<String> missingJvmArguments = ImmutableList.builder();
for (String fromModule : modules.keySet()) {
for (String toModule : modules.get(fromModule)) {
String requiredJvmArgument = format(".*?--add-opens[\\s=]%s/%s=ALL-UNNAMED.*?", Pattern.quote(fromModule), Pattern.quote(toModule));
if (!inputArguments.matches(requiredJvmArgument)) {
missingJvmArguments.add(format("--add-opens=%s/%s=ALL-UNNAMED", fromModule, toModule));
}
}
}

List<String> requiredJvmArguments = missingJvmArguments.build();
if (!requiredJvmArguments.isEmpty()) {
throwableSettable.setThrowable(new IllegalStateException(format("%s requires additional JVM argument(s). Please add the following to the JVM configuration: '%s'", description, String.join(" ", requiredJvmArguments))));
}
}

private static ThrowableSettable wrap(Binder binder)
{
return throwable -> binder.addError(throwable.getMessage());
}

@VisibleForTesting
interface ThrowableSettable
{
void setThrowable(Throwable throwable);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/*
* Licensed 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 io.trino.plugin.base;

import com.google.common.collect.ImmutableMultimap;
import com.google.inject.Binder;
import com.google.inject.CreationException;
import com.google.inject.Guice;
import com.google.inject.Module;
import org.junit.jupiter.api.Test;

import java.util.List;

import static io.trino.plugin.base.JdkCompatibilityChecks.verifyConnectorAccessOpened;
import static java.util.Objects.requireNonNull;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

public class TestJdkCompatibilityChecks
{
@Test
public void shouldThrowWhenAccessIsNotGranted()
{
ThrowableSettableMock errorSink = new ThrowableSettableMock();
new JdkCompatibilityChecks(List.of()).verifyAccessOpened(errorSink, "Connector 'snowflake'", ImmutableMultimap.of(
"java.base", "java.nio"));

assertThat(errorSink.getThrowable())
.isInstanceOf(IllegalStateException.class)
.hasMessage("Connector 'snowflake' requires additional JVM argument(s). Please add the following to the JVM configuration: '--add-opens=java.base/java.nio=ALL-UNNAMED'");
}

@Test
public void shouldThrowWhenOneOfAccessGrantsIsMissing()
{
ThrowableSettableMock errorSink = new ThrowableSettableMock();
new JdkCompatibilityChecks(List.of("--add-opens", "java.base/java.nio=ALL-UNNAMED")).verifyAccessOpened(errorSink, "Connector 'snowflake'", ImmutableMultimap.of(
"java.base", "java.nio",
"java.base", "java.lang"));

assertThat(errorSink.getThrowable())
.isInstanceOf(IllegalStateException.class)
.hasMessage("Connector 'snowflake' requires additional JVM argument(s). Please add the following to the JVM configuration: '--add-opens=java.base/java.lang=ALL-UNNAMED'");
}

@Test
public void shouldSetErrorOnBinder()
{
assertThatThrownBy(() -> Guice.createInjector(new TestingModule()))
.isInstanceOf(CreationException.class)
.hasMessageContaining("Unable to create injector, see the following errors:")
.hasMessageContaining("1) Connector 'Testing' requires additional JVM argument(s). Please add the following to the JVM configuration: '--add-opens=java.base/java.non.existing=ALL-UNNAMED --add-opens=java.base/java.this.should.fail=ALL-UNNAMED'");
}

@Test
public void shouldNotThrowWhenAllAccessGrantsArePresent()
{
ThrowableSettableMock errorSink = new ThrowableSettableMock();
new JdkCompatibilityChecks(List.of("--add-opens=java.base/java.nio=ALL-UNNAMED", "--add-opens java.base/java.lang=ALL-UNNAMED")).verifyAccessOpened(errorSink, "Connector 'snowflake'", ImmutableMultimap.of(
"java.base", "java.nio",
"java.base", "java.lang"));

assertThat(errorSink.getThrowable()).isNull();
}

private static class TestingModule
implements Module
{
@Override
public void configure(Binder binder)
{
verifyConnectorAccessOpened(binder, "Testing", ImmutableMultimap.of(
"java.base", "java.non.existing",
"java.base", "java.this.should.fail"));
}
}

private static class ThrowableSettableMock
implements JdkCompatibilityChecks.ThrowableSettable
{
private Throwable throwable;

@Override
public void setThrowable(Throwable throwable)
{
this.throwable = requireNonNull(throwable, "throwable is null");
}

public Throwable getThrowable()
{
return throwable;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import com.google.api.gax.rpc.FixedHeaderProvider;
import com.google.api.gax.rpc.HeaderProvider;
import com.google.common.collect.ImmutableMultimap;
import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.inject.Binder;
import com.google.inject.Key;
Expand All @@ -24,6 +25,7 @@
import com.google.inject.multibindings.Multibinder;
import com.google.inject.multibindings.OptionalBinder;
import io.airlift.configuration.AbstractConfigurationAwareModule;
import io.trino.plugin.base.JdkCompatibilityChecks;
import io.trino.plugin.base.logging.FormatInterpolator;
import io.trino.plugin.base.logging.SessionInterpolatedValues;
import io.trino.plugin.base.session.SessionPropertiesProvider;
Expand All @@ -34,11 +36,7 @@
import io.trino.spi.function.table.ConnectorTableFunction;
import io.trino.spi.procedure.Procedure;

import java.lang.management.ManagementFactory;
import java.util.Set;
import java.util.concurrent.ExecutorService;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import static com.google.common.util.concurrent.MoreExecutors.listeningDecorator;
import static com.google.inject.multibindings.Multibinder.newSetBinder;
Expand All @@ -47,10 +45,8 @@
import static io.airlift.configuration.ConditionalModule.conditionalModule;
import static io.airlift.configuration.ConfigBinder.configBinder;
import static io.trino.plugin.base.ClosingBinder.closingBinder;
import static io.trino.plugin.bigquery.BigQueryConfig.ARROW_SERIALIZATION_ENABLED;
import static java.util.concurrent.Executors.newCachedThreadPool;
import static java.util.concurrent.Executors.newFixedThreadPool;
import static java.util.stream.Collectors.toSet;

public class BigQueryConnectorModule
extends AbstractConfigurationAwareModule
Expand Down Expand Up @@ -87,7 +83,10 @@ protected void setup(Binder binder)
install(conditionalModule(
BigQueryConfig.class,
BigQueryConfig::isArrowSerializationEnabled,
ClientModule::verifyPackageAccessAllowed));
innerBinder -> JdkCompatibilityChecks.verifyConnectorAccessOpened(
innerBinder,
"bigquery",
ImmutableMultimap.of("java.base", "java.nio"))));
newSetBinder(binder, ConnectorTableFunction.class).addBinding().toProvider(Query.class).in(Scopes.SINGLETON);
newSetBinder(binder, Procedure.class).addBinding().toProvider(ExecuteProcedure.class).in(Scopes.SINGLETON);
newSetBinder(binder, SessionPropertiesProvider.class).addBinding().to(BigQuerySessionProperties.class).in(Scopes.SINGLETON);
Expand Down Expand Up @@ -141,34 +140,6 @@ public ExecutorService provideExecutor(CatalogName catalogName)
{
return newCachedThreadPool(daemonThreadsNamed("bigquery-" + catalogName + "-%s"));
}

/**
* Apache Arrow requires reflective access to certain Java internals prohibited since Java 17.
* Adds an error to the {@code binder} if required --add-opens is not passed to the JVM.
*/
private static void verifyPackageAccessAllowed(Binder binder)
{
// Match an --add-opens argument that opens a package to unnamed modules.
// The first group is the opened package.
Pattern argPattern = Pattern.compile(
"^--add-opens=(.*)=([A-Za-z0-9_.]+,)*ALL-UNNAMED(,[A-Za-z0-9_.]+)*$");
// We don't need to check for values in separate arguments because
// they are joined with "=" before we get them.

Set<String> openedModules = ManagementFactory.getRuntimeMXBean()
.getInputArguments()
.stream()
.map(argPattern::matcher)
.filter(Matcher::matches)
.map(matcher -> matcher.group(1))
.collect(toSet());

if (!openedModules.contains("java.base/java.nio")) {
binder.addError(
"BigQuery connector requires additional JVM arguments to run when '" + ARROW_SERIALIZATION_ENABLED + "' is enabled. " +
"Please add '--add-opens=java.base/java.nio=ALL-UNNAMED' to the JVM configuration.");
}
}
}

public static class StaticCredentialsModule
Expand Down

0 comments on commit 684bb74

Please sign in to comment.