Skip to content
This repository has been archived by the owner on Sep 26, 2023. It is now read-only.

Commit

Permalink
fix: revert "feat: add api key support (#1436)" (#1617)
Browse files Browse the repository at this point in the history
  • Loading branch information
@arithmetic1728
arithmetic1728 authored Jan 29, 2022
1 parent 173eff4 commit 47f98b8
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 141 deletions.
58 changes: 10 additions & 48 deletions gax/src/main/java/com/google/api/gax/rpc/ClientContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,12 @@
import com.google.api.gax.core.BackgroundResource;
import com.google.api.gax.core.ExecutorAsBackgroundResource;
import com.google.api.gax.core.ExecutorProvider;
import com.google.api.gax.rpc.internal.EnvironmentProvider;
import com.google.api.gax.rpc.internal.QuotaProjectIdHidingCredentials;
import com.google.api.gax.rpc.mtls.MtlsProvider;
import com.google.api.gax.tracing.ApiTracerFactory;
import com.google.api.gax.tracing.BaseApiTracerFactory;
import com.google.auth.Credentials;
import com.google.auto.value.AutoValue;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Sets;
Expand All @@ -67,7 +65,6 @@
@AutoValue
public abstract class ClientContext {
private static final String QUOTA_PROJECT_ID_HEADER_KEY = "x-goog-user-project";
private static final String API_KEY_HEADER_KEY = "x-goog-api-key";

/**
* The objects that need to be closed in order to clean up the resources created in the process of
Expand Down Expand Up @@ -162,32 +159,6 @@ static String getEndpoint(
return endpoint;
}

/**
* Retrieves the API key value and add it to the headers if API key exists. It first tries to
* retrieve the value from the stub settings. If not found, it then tries the load the
* GOOGLE_API_KEY environment variable. An IOException will be thrown if both GOOGLE_API_KEY and
* GOOGLE_APPLICATION_CREDENTIALS environment variables are set.
*/
@VisibleForTesting
static void addApiKeyToHeaders(
StubSettings settings, EnvironmentProvider environmentProvider, Map<String, String> headers)
throws IOException {
if (settings.getApiKey() != null) {
headers.put(API_KEY_HEADER_KEY, settings.getApiKey());
return;
}

String apiKey = environmentProvider.getenv("GOOGLE_API_KEY");
String applicationCredentials = environmentProvider.getenv("GOOGLE_APPLICATION_CREDENTIALS");
if (apiKey != null && applicationCredentials != null) {
throw new IOException(
"Environment variables GOOGLE_API_KEY and GOOGLE_APPLICATION_CREDENTIALS are mutually exclusive");
}
if (apiKey != null) {
headers.put(API_KEY_HEADER_KEY, apiKey);
}
}

/**
* Instantiates the executor, credentials, and transport context based on the given client
* settings.
Expand All @@ -198,21 +169,14 @@ public static ClientContext create(StubSettings settings) throws IOException {
ExecutorProvider backgroundExecutorProvider = settings.getBackgroundExecutorProvider();
final ScheduledExecutorService backgroundExecutor = backgroundExecutorProvider.getExecutor();

Credentials credentials = null;
Map<String, String> headers = getHeadersFromSettingsAndEnvironment(settings, System::getenv);

boolean hasApiKey = headers.containsKey(API_KEY_HEADER_KEY);
if (!hasApiKey) {
credentials = settings.getCredentialsProvider().getCredentials();
Credentials credentials = settings.getCredentialsProvider().getCredentials();

if (settings.getQuotaProjectId() != null) {
// If the quotaProjectId is set, wrap original credentials with correct quotaProjectId as
// QuotaProjectIdHidingCredentials.
// Ensure that a custom set quota project id takes priority over one detected by
// credentials.
// Avoid the backend receiving possibly conflict values of quotaProjectId
credentials = new QuotaProjectIdHidingCredentials(credentials);
}
if (settings.getQuotaProjectId() != null) {
// If the quotaProjectId is set, wrap original credentials with correct quotaProjectId as
// QuotaProjectIdHidingCredentials.
// Ensure that a custom set quota project id takes priority over one detected by credentials.
// Avoid the backend receiving possibly conflict values of quotaProjectId
credentials = new QuotaProjectIdHidingCredentials(credentials);
}

TransportChannelProvider transportChannelProvider = settings.getTransportChannelProvider();
Expand All @@ -222,11 +186,11 @@ public static ClientContext create(StubSettings settings) throws IOException {
if (transportChannelProvider.needsExecutor() && settings.getExecutorProvider() != null) {
transportChannelProvider = transportChannelProvider.withExecutor(backgroundExecutor);
}

Map<String, String> headers = getHeadersFromSettings(settings);
if (transportChannelProvider.needsHeaders()) {
transportChannelProvider = transportChannelProvider.withHeaders(headers);
}
if (!hasApiKey && transportChannelProvider.needsCredentials()) {
if (transportChannelProvider.needsCredentials() && credentials != null) {
transportChannelProvider = transportChannelProvider.withCredentials(credentials);
}
String endpoint =
Expand Down Expand Up @@ -296,8 +260,7 @@ public static ClientContext create(StubSettings settings) throws IOException {
* Getting a header map from HeaderProvider and InternalHeaderProvider from settings with Quota
* Project Id.
*/
private static Map<String, String> getHeadersFromSettingsAndEnvironment(
StubSettings settings, EnvironmentProvider environmentProvider) throws IOException {
private static Map<String, String> getHeadersFromSettings(StubSettings settings) {
// Resolve conflicts when merging headers from multiple sources
Map<String, String> userHeaders = settings.getHeaderProvider().getHeaders();
Map<String, String> internalHeaders = settings.getInternalHeaderProvider().getHeaders();
Expand All @@ -323,7 +286,6 @@ private static Map<String, String> getHeadersFromSettingsAndEnvironment(
effectiveHeaders.putAll(internalHeaders);
effectiveHeaders.putAll(userHeaders);
effectiveHeaders.putAll(conflictResolution);
addApiKeyToHeaders(settings, environmentProvider, effectiveHeaders);

return ImmutableMap.copyOf(effectiveHeaders);
}
Expand Down
20 changes: 0 additions & 20 deletions gax/src/main/java/com/google/api/gax/rpc/StubSettings.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ public abstract class StubSettings<SettingsT extends StubSettings<SettingsT>> {
private final String endpoint;
private final String mtlsEndpoint;
private final String quotaProjectId;
private final String apiKey;
@Nullable private final WatchdogProvider streamWatchdogProvider;
@Nonnull private final Duration streamWatchdogCheckInterval;
@Nonnull private final ApiTracerFactory tracerFactory;
Expand All @@ -100,7 +99,6 @@ protected StubSettings(Builder builder) {
this.mtlsEndpoint = builder.mtlsEndpoint;
this.switchToMtlsEndpointAllowed = builder.switchToMtlsEndpointAllowed;
this.quotaProjectId = builder.quotaProjectId;
this.apiKey = builder.apiKey;
this.streamWatchdogProvider = builder.streamWatchdogProvider;
this.streamWatchdogCheckInterval = builder.streamWatchdogCheckInterval;
this.tracerFactory = builder.tracerFactory;
Expand Down Expand Up @@ -156,10 +154,6 @@ public final String getQuotaProjectId() {
return quotaProjectId;
}

public final String getApiKey() {
return apiKey;
}

@BetaApi("The surface for streaming is not stable yet and may change in the future.")
@Nullable
public final WatchdogProvider getStreamWatchdogProvider() {
Expand Down Expand Up @@ -195,7 +189,6 @@ public String toString() {
.add("mtlsEndpoint", mtlsEndpoint)
.add("switchToMtlsEndpointAllowed", switchToMtlsEndpointAllowed)
.add("quotaProjectId", quotaProjectId)
.add("apiKey", apiKey)
.add("streamWatchdogProvider", streamWatchdogProvider)
.add("streamWatchdogCheckInterval", streamWatchdogCheckInterval)
.add("tracerFactory", tracerFactory)
Expand All @@ -216,7 +209,6 @@ public abstract static class Builder<
private String endpoint;
private String mtlsEndpoint;
private String quotaProjectId;
private String apiKey;
@Nullable private WatchdogProvider streamWatchdogProvider;
@Nonnull private Duration streamWatchdogCheckInterval;
@Nonnull private ApiTracerFactory tracerFactory;
Expand All @@ -242,7 +234,6 @@ protected Builder(StubSettings settings) {
this.mtlsEndpoint = settings.mtlsEndpoint;
this.switchToMtlsEndpointAllowed = settings.switchToMtlsEndpointAllowed;
this.quotaProjectId = settings.quotaProjectId;
this.apiKey = settings.apiKey;
this.streamWatchdogProvider = settings.streamWatchdogProvider;
this.streamWatchdogCheckInterval = settings.streamWatchdogCheckInterval;
this.tracerFactory = settings.tracerFactory;
Expand All @@ -267,7 +258,6 @@ private static String getQuotaProjectIdFromClientContext(ClientContext clientCon
}

protected Builder(ClientContext clientContext) {
this.apiKey = null;
if (clientContext == null) {
this.backgroundExecutorProvider = InstantiatingExecutorProvider.newBuilder().build();
this.transportChannelProvider = null;
Expand Down Expand Up @@ -442,11 +432,6 @@ public B setQuotaProjectId(String quotaProjectId) {
return self();
}

public B setApiKey(String apiKey) {
this.apiKey = apiKey;
return self();
}

/**
* Sets how often the {@link Watchdog} will check ongoing streaming RPCs. Defaults to 10 secs.
* Use {@link Duration#ZERO} to disable.
Expand Down Expand Up @@ -528,10 +513,6 @@ public String getQuotaProjectId() {
return quotaProjectId;
}

public String getApiKey() {
return apiKey;
}

@BetaApi("The surface for streaming is not stable yet and may change in the future.")
@Nonnull
public Duration getStreamWatchdogCheckInterval() {
Expand Down Expand Up @@ -568,7 +549,6 @@ public String toString() {
.add("mtlsEndpoint", mtlsEndpoint)
.add("switchToMtlsEndpointAllowed", switchToMtlsEndpointAllowed)
.add("quotaProjectId", quotaProjectId)
.add("apiKey", apiKey)
.add("streamWatchdogProvider", streamWatchdogProvider)
.add("streamWatchdogCheckInterval", streamWatchdogCheckInterval)
.add("tracerFactory", tracerFactory)
Expand Down
74 changes: 1 addition & 73 deletions gax/src/test/java/com/google/api/gax/rpc/ClientContextTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

Expand All @@ -42,7 +41,6 @@
import com.google.api.gax.core.ExecutorProvider;
import com.google.api.gax.core.FixedCredentialsProvider;
import com.google.api.gax.core.FixedExecutorProvider;
import com.google.api.gax.rpc.internal.EnvironmentProvider;
import com.google.api.gax.rpc.mtls.MtlsProvider;
import com.google.api.gax.rpc.mtls.MtlsProvider.MtlsEndpointUsagePolicy;
import com.google.api.gax.rpc.testing.FakeChannel;
Expand All @@ -56,7 +54,6 @@
import com.google.common.truth.Truth;
import java.io.IOException;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.Executor;
Expand Down Expand Up @@ -179,7 +176,7 @@ public TransportChannelProvider withPoolSize(int size) {

@Override
public TransportChannel getTransportChannel() throws IOException {
if (needsCredentials() && !headers.containsKey("x-goog-api-key")) {
if (needsCredentials()) {
throw new IllegalStateException("Needs Credentials");
}
transport.setExecutor(executor);
Expand Down Expand Up @@ -772,73 +769,4 @@ public void testExecutorSettings() throws Exception {
transportChannel = (FakeTransportChannel) context.getTransportChannel();
assertThat(transportChannel.getExecutor()).isSameInstanceAs(executorProvider.getExecutor());
}

@Test
public void testAddApiKeyToHeadersFromStubSettings() throws IOException {
StubSettings settings = new FakeStubSettings.Builder().setApiKey("stub-setting-key").build();
EnvironmentProvider environmentProvider =
name -> name.equals("GOOGLE_API_KEY") ? "env-key" : null;
Map<String, String> headers = new HashMap<>();
ClientContext.addApiKeyToHeaders(settings, environmentProvider, headers);
assertThat(headers).containsEntry("x-goog-api-key", "stub-setting-key");
}

@Test
public void testAddApiKeyToHeadersFromEnvironmentProvider() throws IOException {
StubSettings settings = new FakeStubSettings.Builder().build();
EnvironmentProvider environmentProvider =
name -> name.equals("GOOGLE_API_KEY") ? "env-key" : null;
Map<String, String> headers = new HashMap<>();
ClientContext.addApiKeyToHeaders(settings, environmentProvider, headers);
assertThat(headers).containsEntry("x-goog-api-key", "env-key");
}

@Test
public void testAddApiKeyToHeadersNoApiKey() throws IOException {
StubSettings settings = new FakeStubSettings.Builder().build();
EnvironmentProvider environmentProvider = name -> null;
Map<String, String> headers = new HashMap<>();
ClientContext.addApiKeyToHeaders(settings, environmentProvider, headers);
assertThat(headers).doesNotContainKey("x-goog-api-key");
}

@Test
public void testAddApiKeyToHeadersThrows() throws IOException {
StubSettings settings = new FakeStubSettings.Builder().build();
EnvironmentProvider environmentProvider =
name -> name.equals("GOOGLE_API_KEY") ? "env-key" : "/path/to/adc/json";
Map<String, String> headers = new HashMap<>();
Exception ex =
assertThrows(
IOException.class,
() -> ClientContext.addApiKeyToHeaders(settings, environmentProvider, headers));
assertThat(ex)
.hasMessageThat()
.contains(
"Environment variables GOOGLE_API_KEY and GOOGLE_APPLICATION_CREDENTIALS are mutually exclusive");
}

@Test
public void testApiKey() throws IOException {
FakeStubSettings.Builder builder = new FakeStubSettings.Builder();

FakeTransportChannel transportChannel = FakeTransportChannel.create(new FakeChannel());
FakeTransportProvider transportProvider =
new FakeTransportProvider(transportChannel, null, true, null, null);
builder.setTransportChannelProvider(transportProvider);

HeaderProvider headerProvider = Mockito.mock(HeaderProvider.class);
Mockito.when(headerProvider.getHeaders()).thenReturn(ImmutableMap.of());
builder.setHeaderProvider(headerProvider);

// Set API key.
builder.setApiKey("key");

ClientContext context = ClientContext.create(builder.build());

// Check API key is in the transport channel's header.
List<BackgroundResource> resources = context.getBackgroundResources();
FakeTransportChannel fakeTransportChannel = (FakeTransportChannel) resources.get(0);
assertThat(fakeTransportChannel.getHeaders()).containsEntry("x-goog-api-key", "key");
}
}

0 comments on commit 47f98b8

Please sign in to comment.