Skip to content
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
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.MoreExecutors;
import io.grpc.CallCredentials;
import io.grpc.ManagedChannel;
import io.grpc.StatusRuntimeException;
import io.grpc.internal.WithLogId;
Expand Down Expand Up @@ -87,15 +88,23 @@ public class GrpcClient implements RpcClient {

public GrpcClient(ManagedChannel channel) {
this.channel = channel;
if (channel instanceof WithLogId) {
channelId = ((WithLogId) channel).getLogId().toString();
} else {
channelId = channel.toString();
}
channelId = toChannelId(channel);
asyncStub = VitessGrpc.newStub(channel);
futureStub = VitessGrpc.newFutureStub(channel);
}

public GrpcClient(ManagedChannel channel, CallCredentials credentials) {
this.channel = channel;
channelId = toChannelId(channel);
asyncStub = VitessGrpc.newStub(channel).withCallCredentials(credentials);
futureStub = VitessGrpc.newFutureStub(channel).withCallCredentials(credentials);
}

private String toChannelId(ManagedChannel channel) {
return channel instanceof WithLogId ?
((WithLogId) channel).getLogId().toString() : channel.toString();
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You can call one constructor from another so that only one constructor contains the logic for initialization.

Copy link
Copy Markdown
Contributor Author

@ys8 ys8 Dec 3, 2018

Choose a reason for hiding this comment

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

I tried it first. It didn't look too clean to me. Do you prefer that way? (In fact, having CallCredentials parameter as Nullabe in this case, an extra constructor is technically no longer needed besides the constructor can be more confusing from caller's point of view.)

public GrpcClient(ManagedChannel channel, @Nullable CallCredentials credentials) {
  this.channel = channel;
  channelId = toChannelId(channel);
  asyncStub = credentials != null ? VitessGrpc.newStub(channel).withCallCredentials(credentials) ? VitessGrpc.newStub(channel);
  futureStub = credentials != null ? VitessGrpc.newFutureStub(channel).withCallCredentials(credentials) ? VitessGrpc.newFutureStub(channel);
}

@Override
public void close() throws IOException {
channel.shutdown();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package io.vitess.client.grpc;

import io.grpc.CallCredentials;
import io.grpc.ManagedChannel;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
Expand Down Expand Up @@ -48,6 +50,7 @@
public class GrpcClientFactory implements RpcClientFactory {

private RetryingInterceptorConfig config;
private CallCredentials callCredentials;

public GrpcClientFactory() {
this(RetryingInterceptorConfig.noOpConfig());
Expand All @@ -57,6 +60,11 @@ public GrpcClientFactory(RetryingInterceptorConfig config) {
this.config = config;
}

public GrpcClientFactory setCallCredentials(CallCredentials value) {
callCredentials = value;
return this;
}

/**
* Factory method to construct a gRPC client connection with no transport-layer security.
*
Expand All @@ -67,8 +75,11 @@ public GrpcClientFactory(RetryingInterceptorConfig config) {
*/
@Override
public RpcClient create(Context ctx, String target) {
return new GrpcClient(
NettyChannelBuilder.forTarget(target).negotiationType(NegotiationType.PLAINTEXT).intercept(new RetryingInterceptor(config)).build());
ManagedChannel channel = NettyChannelBuilder.forTarget(target)
.negotiationType(NegotiationType.PLAINTEXT)
.intercept(new RetryingInterceptor(config))
.build();
return callCredentials != null ? new GrpcClient(channel, callCredentials) : new GrpcClient(channel);
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package io.vitess.client.grpc;

import io.grpc.Attributes;
import io.grpc.CallCredentials;
import io.grpc.Metadata;
import io.grpc.MethodDescriptor;
import io.grpc.Status;
import java.util.Objects;
import java.util.concurrent.Executor;

/**
* {@link CallCredentials} that applies plain username and password.
*/
public class StaticAuthCredentials implements CallCredentials {

private static final Metadata.Key<String> USERNAME =
Metadata.Key.of("username", Metadata.ASCII_STRING_MARSHALLER);
private static final Metadata.Key<String> PASSWORD =
Metadata.Key.of("password", Metadata.ASCII_STRING_MARSHALLER);

private final String username;
private final String password;

public StaticAuthCredentials(String username, String password) {
this.username = Objects.requireNonNull(username);
this.password = Objects.requireNonNull(password);
}

@Override
public void applyRequestMetadata(MethodDescriptor<?, ?> method, Attributes attrs,
Executor executor, MetadataApplier applier) {
executor.execute(() -> {
try {
Metadata headers = new Metadata();
headers.put(USERNAME, username);
headers.put(PASSWORD, password);
applier.apply(headers);
} catch (Throwable e) {
applier.fail(Status.UNAUTHENTICATED.withCause(e));
}
});
}

@Override
public void thisUsesUnstableApi() {
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
package io.client.grpc;

import com.google.common.base.Throwables;
import io.grpc.Status;
import io.grpc.StatusRuntimeException;
import io.vitess.client.Context;
import io.vitess.client.RpcClient;
import io.vitess.client.RpcClientTest;
import io.vitess.client.grpc.GrpcClientFactory;
import io.vitess.client.grpc.StaticAuthCredentials;
import io.vitess.proto.Vtgate.GetSrvKeyspaceRequest;
import java.net.ServerSocket;
import java.net.URI;
import java.util.Arrays;
import java.util.concurrent.ExecutionException;
import org.joda.time.Duration;
import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

@RunWith(JUnit4.class)
public class GrpcClientStaticAuthTest extends RpcClientTest {
private static Process vtgateclienttest;
private static int port;

@BeforeClass
public static void setUpBeforeClass() throws Exception {
String vtRoot = System.getenv("VTROOT");
if (vtRoot == null) {
throw new RuntimeException("cannot find env variable VTROOT; make sure to source dev.env");
}
URI staticAuthFile = GrpcClientStaticAuthTest.class.getResource("grpc_static_auth.json").toURI();

ServerSocket socket = new ServerSocket(0);
port = socket.getLocalPort();
socket.close();

vtgateclienttest = new ProcessBuilder(Arrays.asList(
vtRoot + "/bin/vtgateclienttest",
"-logtostderr",
"-grpc_port", Integer.toString(port),
"-service_map", "grpc-vtgateservice",
"-grpc_auth_mode", "static",
"-grpc_auth_static_password_file", staticAuthFile.getPath()
)).inheritIO().start();

Context ctx = Context.getDefault().withDeadlineAfter(Duration.millis(5000L));
client = new GrpcClientFactory()
.setCallCredentials(new StaticAuthCredentials("test-username", "test-password"))
.create(ctx, "localhost:" + port);
}

@AfterClass
public static void tearDownAfterClass() throws Exception {
if (client != null) {
client.close();
}
if (vtgateclienttest != null) {
vtgateclienttest.destroy();
vtgateclienttest.waitFor();
}
}

@Test
public void testWrongPassword() throws Exception {
RpcClient client = new GrpcClientFactory()
.setCallCredentials(new StaticAuthCredentials("test-username", "WRONG-password"))
.create(Context.getDefault(), "localhost:" + port);
try {
client.getSrvKeyspace(Context.getDefault(), GetSrvKeyspaceRequest.getDefaultInstance()).get();
Assert.fail();
} catch (ExecutionException e) {
StatusRuntimeException cause = (StatusRuntimeException) Throwables.getRootCause(e);
Assert.assertSame(cause.getStatus().getCode(), Status.Code.PERMISSION_DENIED);
}
client.close();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[
{
"username": "test-username",
"password": "test-password"
}
]
6 changes: 3 additions & 3 deletions java/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -223,10 +223,10 @@
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<version>3.3</version>
<version>3.8.0</version>
<configuration>
<source>1.7</source>
<target>1.7</target>
<source>1.8</source>
<target>1.8</target>
</configuration>
</plugin>
<!-- Add "dependency:analyze" to "verify" goal. -->
Expand Down