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

Introduce channel Attributes; Use them to pass the binder target user to the NameResolver #11524

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
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
9 changes: 9 additions & 0 deletions api/src/main/java/io/grpc/Grpc.java
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,15 @@ private Grpc() {
@Documented
public @interface TransportAttr {}

/**
* Annotation for Channel attributes. It follows the annotation semantics defined by {@link
* Attributes}.
*/
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/00000000")
@Retention(RetentionPolicy.SOURCE)
@Documented
public @interface ChannelAttr {}

/**
* Creates a channel builder with a target string and credentials. The target can be either a
* valid {@link NameResolver}-compliant URI, or an authority string.
Expand Down
31 changes: 27 additions & 4 deletions api/src/main/java/io/grpc/NameResolver.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.google.common.base.MoreObjects;
import com.google.common.base.Objects;
import com.google.errorprone.annotations.InlineMe;
import io.grpc.Grpc.ChannelAttr;
import java.lang.annotation.Documented;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
Expand Down Expand Up @@ -281,6 +282,7 @@
private final ProxyDetector proxyDetector;
private final SynchronizationContext syncContext;
private final ServiceConfigParser serviceConfigParser;
@ChannelAttr @Nullable private final Attributes channelAttributes;
@Nullable private final ScheduledExecutorService scheduledExecutorService;
@Nullable private final ChannelLogger channelLogger;
@Nullable private final Executor executor;
Expand All @@ -291,6 +293,7 @@
ProxyDetector proxyDetector,
SynchronizationContext syncContext,
ServiceConfigParser serviceConfigParser,
@ChannelAttr Attributes channelAttributes,
@Nullable ScheduledExecutorService scheduledExecutorService,
@Nullable ChannelLogger channelLogger,
@Nullable Executor executor,
Expand All @@ -299,6 +302,7 @@
this.proxyDetector = checkNotNull(proxyDetector, "proxyDetector not set");
this.syncContext = checkNotNull(syncContext, "syncContext not set");
this.serviceConfigParser = checkNotNull(serviceConfigParser, "serviceConfigParser not set");
this.channelAttributes = checkNotNull(channelAttributes, "channelAttributes not set");
this.scheduledExecutorService = scheduledExecutorService;
this.channelLogger = channelLogger;
this.executor = executor;
Expand Down Expand Up @@ -363,6 +367,12 @@
return serviceConfigParser;
}

/** Returns {@link Attributes} of the associated Channel. */
@ChannelAttr
public Attributes getChannelAttributes() {
return channelAttributes;

Check warning on line 373 in api/src/main/java/io/grpc/NameResolver.java

View check run for this annotation

Codecov / codecov/patch

api/src/main/java/io/grpc/NameResolver.java#L373

Added line #L373 was not covered by tests
}

/**
* Returns the {@link ChannelLogger} for the Channel served by this NameResolver.
*
Expand Down Expand Up @@ -452,6 +462,7 @@
private ProxyDetector proxyDetector;
private SynchronizationContext syncContext;
private ServiceConfigParser serviceConfigParser;
private Attributes channelAttributes = Attributes.EMPTY;
private ScheduledExecutorService scheduledExecutorService;
private ChannelLogger channelLogger;
private Executor executor;
Expand Down Expand Up @@ -510,6 +521,12 @@
return this;
}

/** See {@link Args#getChannelAttributes}. This is a required field, default empty. */
public Builder setChannelAttributes(@ChannelAttr Attributes channelAttributes) {
this.channelAttributes = checkNotNull(channelAttributes);
return this;
}

/**
* See {@link Args#getChannelLogger}.
*
Expand Down Expand Up @@ -548,10 +565,16 @@
* @since 1.21.0
*/
public Args build() {
return
new Args(
defaultPort, proxyDetector, syncContext, serviceConfigParser,
scheduledExecutorService, channelLogger, executor, overrideAuthority);
return new Args(
defaultPort,
proxyDetector,
syncContext,
serviceConfigParser,
channelAttributes,
scheduledExecutorService,
channelLogger,
executor,
overrideAuthority);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import android.content.Intent;
import android.os.Parcel;
import android.os.Parcelable;
import android.os.UserHandle;
import android.os.UserManager;
import androidx.test.core.app.ApplicationProvider;
import androidx.test.ext.junit.runners.AndroidJUnit4;
import com.google.common.io.ByteStreams;
Expand All @@ -50,6 +52,7 @@
import io.grpc.StatusRuntimeException;
import io.grpc.internal.GrpcUtil;
import io.grpc.internal.testing.FakeNameResolverProvider;
import io.grpc.internal.testing.FakeNameResolverProvider.FakeNameResolver;
import io.grpc.stub.ClientCalls;
import io.grpc.stub.MetadataUtils;
import io.grpc.stub.ServerCalls;
Expand All @@ -59,6 +62,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.Executors;
import java.util.concurrent.atomic.AtomicReference;
import org.junit.After;
Expand Down Expand Up @@ -251,6 +255,26 @@ public void testConnectViaIntentFilter() throws Exception {
assertThat(doCall("Hello").get()).isEqualTo("Hello");
}

@Test
public void testTargetUserChannelAttribute() throws Exception {
UserManager userManager = appContext.getSystemService(UserManager.class);
List<UserHandle> userProfiles = userManager.getUserProfiles();
UserHandle someUser = userProfiles.get(0); // Guaranteed to be non-empty.

channel =
BinderChannelBuilder.forTarget(SERVER_TARGET_URI, appContext).bindAsUser(someUser).build();
assertThat(doCall("Hello").get()).isNotNull();

FakeNameResolver fakeNameResolver =
fakeNameResolverProvider.pollNextResolverCreated(5, SECONDS);
assertThat(
fakeNameResolver
.getArgs()
.getChannelAttributes()
.get(ApiConstants.CHANNEL_ATTR_TARGET_USER))
.isEqualTo(someUser);
}

@Test
public void testUncaughtServerException() throws Exception {
// Use a poison parcelable to cause an unexpected Exception in the server's onTransact().
Expand Down
19 changes: 19 additions & 0 deletions binder/src/main/java/io/grpc/binder/ApiConstants.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@
package io.grpc.binder;

import android.content.Intent;
import android.os.UserHandle;
import io.grpc.Attributes;
import io.grpc.ExperimentalApi;
import io.grpc.Grpc.ChannelAttr;

/** Constant parts of the gRPC binder transport public API. */
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/8022")
Expand All @@ -29,4 +32,20 @@ private ApiConstants() {}
* themselves in a {@link android.app.Service#onBind(Intent)} call.
*/
public static final String ACTION_BIND = "grpc.io.action.BIND";

/**
* The target Android user of a binder Channel.
*
* <p>In multi-user Android, the target user for an Intent is unfortunately not part of the intent
* itself. Instead, it's passed around as a separate argument wherever that Intent is needed.
* Following suit, this implementation of the binder transport accepts the target Android user as
* a parameter to BinderChannelBuilder -- unfortunately it's not in the target URI or the
* SocketAddress. Instead, downstream plugins such as {@link io.grpc.NameResolver}s and {@link
* io.grpc.LoadBalancer}s can use this attribute to obtain the Channel's target UserHandle. If the
* attribute is not set, the Channel's target is the Android user hosting the current process (the
* default).
*/
@ChannelAttr
public static final Attributes.Key<UserHandle> CHANNEL_ATTR_TARGET_USER =
Attributes.Key.create("io.grpc.binder.CHANNEL_ATTR_TARGET_USER");
}
4 changes: 4 additions & 0 deletions binder/src/main/java/io/grpc/binder/BinderChannelBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import android.os.UserHandle;
import androidx.annotation.RequiresApi;
import com.google.errorprone.annotations.DoNotCall;
import io.grpc.Attributes;
import io.grpc.ExperimentalApi;
import io.grpc.ForwardingChannelBuilder;
import io.grpc.ManagedChannel;
Expand Down Expand Up @@ -159,6 +160,7 @@ public static BinderChannelBuilder forTarget(String target) {

private final ManagedChannelImplBuilder managedChannelImplBuilder;
private final BinderClientTransportFactory.Builder transportFactoryBuilder;
private final Attributes.Builder channelAttrBuilder = Attributes.newBuilder();

private boolean strictLifecycleManagement;

Expand Down Expand Up @@ -250,6 +252,7 @@ public BinderChannelBuilder securityPolicy(SecurityPolicy securityPolicy) {
@RequiresApi(30)
public BinderChannelBuilder bindAsUser(UserHandle targetUserHandle) {
transportFactoryBuilder.setTargetUserHandle(targetUserHandle);
channelAttrBuilder.set(ApiConstants.CHANNEL_ATTR_TARGET_USER, targetUserHandle);
return this;
}

Expand Down Expand Up @@ -284,6 +287,7 @@ public BinderChannelBuilder idleTimeout(long value, TimeUnit unit) {
public ManagedChannel build() {
transportFactoryBuilder.setOffloadExecutorPool(
managedChannelImplBuilder.getOffloadExecutorPool());
managedChannelImplBuilder.setChannelAttributes(channelAttrBuilder.build());
return super.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,7 @@ ClientStream newSubstream(
this.authorityOverride = builder.authorityOverride;
this.nameResolverArgs =
NameResolver.Args.newBuilder()
.setChannelAttributes(builder.channelAttributes)
.setDefaultPort(builder.getDefaultPort())
.setProxyDetector(proxyDetector)
.setSynchronizationContext(syncContext)
Expand Down
11 changes: 11 additions & 0 deletions core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import io.grpc.CompressorRegistry;
import io.grpc.DecompressorRegistry;
import io.grpc.EquivalentAddressGroup;
import io.grpc.Grpc.ChannelAttr;
import io.grpc.InternalChannelz;
import io.grpc.InternalConfiguratorRegistry;
import io.grpc.ManagedChannel;
Expand Down Expand Up @@ -197,6 +198,8 @@
@Nullable
ProxyDetector proxyDetector;

@ChannelAttr Attributes channelAttributes = Attributes.EMPTY;

private boolean authorityCheckerDisabled;
private boolean statsEnabled = true;
private boolean recordStartedRpcs = true;
Expand Down Expand Up @@ -663,6 +666,14 @@
tracingEnabled = value;
}

/**
* Makes the specified Attributes available to downstream plugins such as resolvers and load
* balancers.
*/
public void setChannelAttributes(@ChannelAttr Attributes channelAttributes) {
this.channelAttributes = channelAttributes;
}

Check warning on line 675 in core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java

View check run for this annotation

Codecov / codecov/patch

core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java#L674-L675

Added lines #L674 - L675 were not covered by tests

/**
* Verifies the authority is valid.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,16 @@
import java.net.URI;
import java.util.Collection;
import java.util.Collections;
import java.util.concurrent.BlockingDeque;
import java.util.concurrent.LinkedBlockingDeque;
import java.util.concurrent.TimeUnit;

/** A name resolver to always resolve the given URI into the given address. */
public final class FakeNameResolverProvider extends NameResolverProvider {

private final URI targetUri;
private final SocketAddress address;
private final BlockingDeque<FakeNameResolver> resolversCreated = new LinkedBlockingDeque<>();

public FakeNameResolverProvider(String targetUri, SocketAddress address) {
this.targetUri = URI.create(targetUri);
Expand All @@ -40,7 +44,9 @@ public FakeNameResolverProvider(String targetUri, SocketAddress address) {
@Override
public NameResolver newNameResolver(URI targetUri, NameResolver.Args args) {
if (targetUri.equals(this.targetUri)) {
return new FakeNameResolver(address);
FakeNameResolver result = new FakeNameResolver(address, args);
resolversCreated.add(result);
return result;
}
return null;
}
Expand All @@ -65,15 +71,26 @@ public Collection<Class<? extends SocketAddress>> getProducedSocketAddressTypes(
return Collections.singleton(address.getClass());
}

/**
* Returns the next FakeNameResolver created by this provider, or null if none were created before
* the timeout lapsed.
*/
public FakeNameResolver pollNextResolverCreated(long timeout, TimeUnit unit)
throws InterruptedException {
return resolversCreated.poll(timeout, unit);
}

/** A single name resolver. */
private static final class FakeNameResolver extends NameResolver {
public static final class FakeNameResolver extends NameResolver {
private static final String AUTHORITY = "fake-authority";

private final SocketAddress address;
private final Args args;
private volatile boolean shutdown;

private FakeNameResolver(SocketAddress address) {
private FakeNameResolver(SocketAddress address, Args args) {
this.address = address;
this.args = args;
}

@Override
Expand All @@ -97,5 +114,9 @@ public String getServiceAuthority() {
public void shutdown() {
shutdown = true;
}

public Args getArgs() {
return this.args;
}
}
}