Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not use Google's HTTP client to get the default project ID #502

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions common/runtime/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,28 @@
<groupId>org.graalvm.sdk</groupId>
<artifactId>graal-sdk</artifactId>
</dependency>

<!-- Tests -->
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-junit5</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.opentelemetry</groupId>
<artifactId>opentelemetry-opencensus-shim</artifactId>
<scope>test</scope>
</dependency>
</dependencies>

<build>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,23 @@
import static java.util.Collections.emptyList;
import static java.util.Collections.singletonList;

import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.http.HttpClient;
import java.net.http.HttpRequest;
import java.net.http.HttpResponse;
import java.time.Duration;
import java.util.Map;
import java.util.Set;
import java.util.function.Supplier;

import org.eclipse.microprofile.config.spi.ConfigSource;

import com.google.cloud.PlatformInformation;
import com.google.cloud.ServiceDefaults;
import com.google.cloud.ServiceOptions;
import com.google.common.annotations.VisibleForTesting;

import io.smallrye.config.ConfigSourceContext;
import io.smallrye.config.ConfigSourceFactory;
Expand All @@ -17,40 +29,138 @@

public class GcpDefaultsConfigSourceFactory implements ConfigSourceFactory {

private static final String OPENTELEMETRY_CONTEXT_CONTEXT_STORAGE_PROVIDER_SYS_PROP = "io.opentelemetry.context.contextStorageProvider";
private final Supplier<String> defaultProjectIdSupplier;

public GcpDefaultsConfigSourceFactory() {
this(ServiceOptionsHelper::getDefaultProjectId);
}

@VisibleForTesting
GcpDefaultsConfigSourceFactory(Supplier<String> defaultProjectIdSupplier) {
this.defaultProjectIdSupplier = defaultProjectIdSupplier;
}

@Override
public Iterable<ConfigSource> getConfigSources(final ConfigSourceContext context) {
ConfigValue enableMetadataServer = context.getValue("quarkus.google.cloud.enable-metadata-server");
if (enableMetadataServer.getValue() != null) {
if (Converters.getImplicitConverter(Boolean.class).convert(enableMetadataServer.getValue())) {
String previousContextStorageSysProp = null;
try {
// Google HTTP Client under the hood which attempts to record traces via OpenCensus which is wired
// to delegate to OpenTelemetry.
// This can lead to problems with the Quarkus OpenTelemetry extension which expects Vert.x to be running,
// something that is not the case at build time, see https://github.com/quarkusio/quarkus/issues/35500
previousContextStorageSysProp = System.setProperty(OPENTELEMETRY_CONTEXT_CONTEXT_STORAGE_PROVIDER_SYS_PROP,
"default");

String defaultProjectId = ServiceOptions.getDefaultProjectId();
if (defaultProjectId != null) {
return singletonList(
new PropertiesConfigSource(Map.of("quarkus.google.cloud.project-id", defaultProjectId),
"GcpDefaultsConfigSource",
-Integer.MAX_VALUE));
}
} finally {
if (previousContextStorageSysProp == null) {
System.clearProperty(OPENTELEMETRY_CONTEXT_CONTEXT_STORAGE_PROVIDER_SYS_PROP);
} else {
System.setProperty(OPENTELEMETRY_CONTEXT_CONTEXT_STORAGE_PROVIDER_SYS_PROP,
previousContextStorageSysProp);
String defaultProjectId = defaultProjectIdSupplier.get();
if (defaultProjectId != null) {
return singletonList(
new PropertiesConfigSource(Map.of("quarkus.google.cloud.project-id", defaultProjectId),
"GcpDefaultsConfigSource",
-Integer.MAX_VALUE));
}
}
}
return emptyList();
}

/**
* This is a partial copy of {@link ServiceOptions} to prevent the use of Google's HTTP client, which causes
* static initialization trouble via OpenCensus-shim with OpenTelemetry.
*
* <p>
* This helper class is only intended to not use the Google HTTP client but not change any other aspects of
* how the default project ID is retrieved.
*
* <p>
* (The {@link ServiceOptions} class is licensed using ASL2.)
*/
@SuppressWarnings("rawtypes")
static class ServiceOptionsHelper extends ServiceOptions {
@SuppressWarnings("unchecked")
protected ServiceOptionsHelper(Class serviceFactoryClass, Class rpcFactoryClass, Builder builder,
ServiceDefaults serviceDefaults) {
super(serviceFactoryClass, rpcFactoryClass, builder, serviceDefaults);
throw new UnsupportedOperationException();
}

@Override
protected Set<String> getScopes() {
throw new UnsupportedOperationException();
}

@Override
public Builder toBuilder() {
throw new UnsupportedOperationException();
}

public static String getDefaultProjectId() {
// As in the original `ServiceOptions` class
String projectId = System.getProperty("GOOGLE_CLOUD_PROJECT", System.getenv("GOOGLE_CLOUD_PROJECT"));
if (projectId == null) {
projectId = System.getProperty("GCLOUD_PROJECT", System.getenv("GCLOUD_PROJECT"));
}

if (projectId == null) {
projectId = getAppEngineProjectId();
}

if (projectId == null) {
projectId = getServiceAccountProjectId();
}

return projectId != null ? projectId : getGoogleCloudProjectId();
}

protected static String getAppEngineProjectId() {
// As in the original `ServiceOptions` class
String projectId;
if (PlatformInformation.isOnGAEStandard7()) {
projectId = getAppEngineProjectIdFromAppId();
} else {
projectId = System.getenv("GOOGLE_CLOUD_PROJECT");
if (projectId == null) {
projectId = System.getenv("GCLOUD_PROJECT");
}

if (projectId == null) {
projectId = getAppEngineProjectIdFromAppId();
}

if (projectId == null) {
try {
projectId = getAppEngineProjectIdFromMetadataServer();
} catch (IOException var2) {
// projectId = null;
}
}
}

return projectId;
}

/**
* This function has been changed to use the (new) Java HTTP client.
*/
private static String getAppEngineProjectIdFromMetadataServer() throws IOException {
String metadata = "http://metadata.google.internal";
String projectIdURL = "/computeMetadata/v1/project/project-id";

try {
URI uri = new URI(metadata + projectIdURL);
HttpClient client = HttpClient.newBuilder().connectTimeout(Duration.ofMillis(500)).build();
HttpRequest request = HttpRequest.newBuilder().timeout(Duration.ofMillis(500)).GET().uri(uri)
.header("Metadata-Flavor", "Google").build();
HttpResponse.BodyHandler<String> bodyHandler = HttpResponse.BodyHandlers.ofString();
HttpResponse<String> response = client.send(request, bodyHandler);
return headerContainsMetadataFlavor(response) ? response.body() : null;
} catch (URISyntaxException e) {
throw new RuntimeException(e);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
throw new IOException(e);
}
}
return emptyList();

/**
* This function has been adopted for the Java HTTP client.
*/
private static boolean headerContainsMetadataFlavor(HttpResponse<?> response) {
String metadataFlavorValue = response.headers().firstValue("Metadata-Flavor").orElse("");
return "Google".equals(metadataFlavorValue);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package io.quarkiverse.googlecloudservices.common;

import static org.assertj.core.api.Assertions.assertThat;

import org.eclipse.microprofile.config.spi.ConfigSource;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.api.OpenTelemetry;
import io.smallrye.config.ConfigSourceContext;
import io.smallrye.config.ConfigValue;

class GcpDefaultsConfigSourceFactoryTest {
@Test
void configSourceWorks() {
ConfigSourceContext context = Mockito.mock(ConfigSourceContext.class);
Mockito.when(context.getValue("quarkus.google.cloud.enable-metadata-server"))
.thenReturn(ConfigValue.builder().withValue("true").build());

Iterable<ConfigSource> configSources = new GcpDefaultsConfigSourceFactory(() -> "test-project-id")
.getConfigSources(context);
assertThat(configSources).asList().hasSize(1);

ConfigSource configSource = configSources.iterator().next();
assertThat(configSource.getProperties()).containsEntry("quarkus.google.cloud.project-id", "test-project-id");
}

@Test
void metadataServerDisabled() {
ConfigSourceContext context = Mockito.mock(ConfigSourceContext.class);
Mockito.when(context.getValue("quarkus.google.cloud.enable-metadata-server"))
.thenReturn(ConfigValue.builder().withValue("false").build());

Iterable<ConfigSource> configSources = new GcpDefaultsConfigSourceFactory(() -> "test-project-id")
.getConfigSources(context);
assertThat(configSources).isEmpty();
}

/**
* Tests that OpenCensus does not get implicitly initialized and in turn does not "collide" with
* OpenTelemetry, as used in Quarkus.
*/
@Test
void staticOpenCensusOpenTelemetryInit() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the purpose of this test? Does it crash without the change (so it's a non-regression test?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, fails without the prod code change. Added a comment.

try {
GlobalOpenTelemetry.resetForTest();

ConfigSourceContext context = Mockito.mock(ConfigSourceContext.class);
Mockito.when(context.getValue("quarkus.google.cloud.enable-metadata-server"))
.thenReturn(ConfigValue.builder().withValue("true").build());

// Uses the "real" implementation that tries to fetch the default-project-id
Iterable<ConfigSource> configSources = new GcpDefaultsConfigSourceFactory().getConfigSources(context);
assertThat(configSources).asList().hasSizeLessThanOrEqualTo(1);

// This is a pretty ugly way, because it changes static state
GlobalOpenTelemetry.set(OpenTelemetry.noop());
} finally {
GlobalOpenTelemetry.resetForTest();
}
}
}
6 changes: 6 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
<compiler-plugin.version>3.8.1</compiler-plugin.version>
<enforcer-plugin.version>3.4.1</enforcer-plugin.version>
<assertj.version>3.24.2</assertj.version>
<opentelemetry-alpha.version>1.28.0-alpha</opentelemetry-alpha.version>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<opentelemetry-alpha.version>1.28.0-alpha</opentelemetry-alpha.version>
<opentelemetry.version>1.28.0-alpha</opentelemetry.version>

And use the same under

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Took the approach that is used in Quarkus, differing between the "alpha" and "non-alpha" artifacts.

</properties>
<scm>
<connection>scm:git:[email protected]:quarkiverse/quarkus-google-cloud-services.git</connection>
Expand Down Expand Up @@ -63,6 +64,11 @@
<artifactId>assertj-core</artifactId>
<version>${assertj.version}</version>
</dependency>
<dependency>
<groupId>io.opentelemetry</groupId>
<artifactId>opentelemetry-opencensus-shim</artifactId>
<version>${opentelemetry-alpha.version}</version>
</dependency>
</dependencies>
</dependencyManagement>
<build>
Expand Down
Loading