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

fix: revert "feat: add api key support (#1436)" #1617

Merged
merged 2 commits into from
Jan 29, 2022
Merged

fix: revert "feat: add api key support (#1436)" #1617

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ce573a0
fix: revert "feat: add api key support (#1436)"
arithmetic1728 Jan 28, 2022
9dce2fd
Merge branch 'main' into revert_apikey
arithmetic1728 Jan 28, 2022
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
58 changes: 10 additions & 48 deletions gax/src/main/java/com/google/api/gax/rpc/ClientContext.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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");
}
}