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

s2a: refactored all the classes into a package #11809

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
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
4 changes: 2 additions & 2 deletions examples/example-xds/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ repositories {
}

java {
sourceCompatibility = JavaVersion.VERSION_1_8
targetCompatibility = JavaVersion.VERSION_1_8
sourceCompatibility = JavaVersion.VERSION_23
targetCompatibility = JavaVersion.VERSION_23
}

// IMPORTANT: You probably want the non-SNAPSHOT version of gRPC. Make sure you
Expand Down
44 changes: 33 additions & 11 deletions netty/src/test/java/io/grpc/netty/NettyClientTransportTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,9 @@
*/
@RunWith(JUnit4.class)
public class NettyClientTransportTest {
@Rule public final MockitoRule mocks = MockitoJUnit.rule();

@Rule
public final MockitoRule mocks = MockitoJUnit.rule();

private static final SslContext SSL_CONTEXT = createSslContext();

Expand All @@ -143,7 +145,9 @@ public class NettyClientTransportTest {
private final InternalChannelz channelz = new InternalChannelz();
private Runnable tooManyPingsRunnable = new Runnable() {
// Throwing is useless in this method, because Netty doesn't propagate the exception
@Override public void run() {}
@Override
public void run() {
}
};
private Attributes eagAttributes = Attributes.EMPTY;

Expand Down Expand Up @@ -415,7 +419,7 @@ public void bufferedStreamsShouldBeClosedWhenConnectionTerminates() throws Excep
new Rpc(transport).halfClose().waitForResponse();

// Create 3 streams, but don't half-close. The transport will buffer the second and third.
Rpc[] rpcs = new Rpc[] { new Rpc(transport), new Rpc(transport), new Rpc(transport) };
Rpc[] rpcs = new Rpc[]{new Rpc(transport), new Rpc(transport), new Rpc(transport)};

// Wait for the response for the stream that was actually created.
rpcs[0].waitForResponse();
Expand All @@ -439,15 +443,20 @@ public void bufferedStreamsShouldBeClosedWhenConnectionTerminates() throws Excep
}

public static class CantConstructChannel extends NioSocketChannel {
/** Constructor. It doesn't work. Feel free to try. But it doesn't work. */

/**
* Constructor. It doesn't work. Feel free to try. But it doesn't work.
*/
public CantConstructChannel() {
// Use an Error because we've seen cases of channels failing to construct due to classloading
// problems (like mixing different versions of Netty), and those involve Errors.
throw new CantConstructChannelError();
}
}

private static class CantConstructChannelError extends Error {}
private static class CantConstructChannelError extends Error {

}

@Test
public void failingToConstructChannelShouldFailGracefully() throws Exception {
Expand Down Expand Up @@ -601,6 +610,7 @@ public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise)

@Test
public void maxHeaderListSizeShouldBeEnforcedOnServer() throws Exception {

startServer(100, 1);

NettyClientTransport transport = newTransport(newNegotiator());
Expand All @@ -615,6 +625,7 @@ public void maxHeaderListSizeShouldBeEnforcedOnServer() throws Exception {
Status status = Status.fromThrowable(e);
assertEquals(status.toString(), Status.Code.INTERNAL, status.getCode());
}

}

@Test
Expand Down Expand Up @@ -866,6 +877,7 @@ private static SslContext createSslContext() {
}

private static class Rpc {

static final String MESSAGE = "hello";
static final MethodDescriptor<String, String> METHOD =
MethodDescriptor.<String, String>newBuilder()
Expand All @@ -885,7 +897,8 @@ private static class Rpc {
Rpc(NettyClientTransport transport, Metadata headers) {
stream = transport.newStream(
METHOD, headers, CallOptions.DEFAULT,
new ClientStreamTracer[]{ new ClientStreamTracer() {} });
new ClientStreamTracer[]{new ClientStreamTracer() {
}});
stream.start(listener);
stream.request(1);
stream.writeMessage(new ByteArrayInputStream(MESSAGE.getBytes(UTF_8)));
Expand All @@ -907,6 +920,7 @@ void waitForClose() throws InterruptedException, ExecutionException, TimeoutExce
}

private static final class TestClientStreamListener implements ClientStreamListener {

final SettableFuture<Void> closedFuture = SettableFuture.create();
final SettableFuture<Void> responseFuture = SettableFuture.create();

Expand Down Expand Up @@ -938,6 +952,7 @@ public void onReady() {
}

private static final class EchoServerStreamListener implements ServerStreamListener {

final ServerStream stream;
final Metadata headers;

Expand Down Expand Up @@ -972,9 +987,10 @@ public void closed(Status status) {
}

private final class EchoServerListener implements ServerListener {

final List<NettyServerTransport> transports = new ArrayList<>();
final List<EchoServerStreamListener> streamListeners =
Collections.synchronizedList(new ArrayList<EchoServerStreamListener>());
Collections.synchronizedList(new ArrayList<EchoServerStreamListener>());

@Override
public ServerTransportListener transportCreated(final ServerTransport transport) {
Expand All @@ -996,7 +1012,8 @@ public Attributes transportReady(Attributes transportAttrs) {
}

@Override
public void transportTerminated() {}
public void transportTerminated() {
}
};
}

Expand All @@ -1006,6 +1023,7 @@ public void serverShutdown() {
}

private static final class StringMarshaller implements Marshaller<String> {

static final StringMarshaller INSTANCE = new StringMarshaller();

@Override
Expand Down Expand Up @@ -1042,6 +1060,7 @@ public void fail(ChannelHandlerContext ctx, Throwable cause) {
}

private static class NoopProtocolNegotiator implements ProtocolNegotiator {

GrpcHttp2ConnectionHandler grpcHandler;
NoopHandler handler;

Expand All @@ -1057,7 +1076,8 @@ public AsciiString scheme() {
}

@Override
public void close() {}
public void close() {
}
}

private static final class SocketPicker extends LocalSocketPicker {
Expand All @@ -1072,9 +1092,11 @@ public SocketAddress createSocketAddress(SocketAddress remoteAddress, Attributes
private static final class FakeChannelLogger extends ChannelLogger {

@Override
public void log(ChannelLogLevel level, String message) {}
public void log(ChannelLogLevel level, String message) {
}

@Override
public void log(ChannelLogLevel level, String messageFormat, Object... args) {}
public void log(ChannelLogLevel level, String messageFormat, Object... args) {
}
}
}
16 changes: 13 additions & 3 deletions s2a/src/main/java/io/grpc/s2a/S2AChannelCredentials.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.base.Strings.isNullOrEmpty;

import com.google.common.annotations.VisibleForTesting;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import io.grpc.Channel;
import io.grpc.ChannelCredentials;
Expand All @@ -30,9 +31,10 @@
import io.grpc.internal.SharedResourcePool;
import io.grpc.netty.InternalNettyChannelCredentials;
import io.grpc.netty.InternalProtocolNegotiator;
import io.grpc.s2a.channel.S2AHandshakerServiceChannel;
import io.grpc.s2a.handshaker.S2AIdentity;
import io.grpc.s2a.handshaker.S2AProtocolNegotiatorFactory;
import io.grpc.s2a.internal.channel.S2AHandshakerServiceChannel;
import io.grpc.s2a.internal.handshaker.S2AIdentity;
import io.grpc.s2a.internal.handshaker.S2AProtocolNegotiatorFactory;
import io.grpc.s2a.internal.handshaker.S2AStub;
import javax.annotation.concurrent.NotThreadSafe;
import org.checkerframework.checker.nullness.qual.Nullable;

Expand Down Expand Up @@ -60,6 +62,7 @@ public static final class Builder {
private ObjectPool<Channel> s2aChannelPool;
private ChannelCredentials s2aChannelCredentials;
private @Nullable S2AIdentity localIdentity = null;
private S2AStub stub;

Builder(String s2aAddress) {
this.s2aAddress = s2aAddress;
Expand Down Expand Up @@ -113,6 +116,13 @@ public Builder setS2AChannelCredentials(ChannelCredentials s2aChannelCredentials
return this;
}

@VisibleForTesting
Builder setStub(S2AStub stub) {
checkNotNull(stub);
this.stub = stub;
return this;
}

public ChannelCredentials build() {
checkState(!isNullOrEmpty(s2aAddress), "S2A address must not be null or empty.");
ObjectPool<Channel> s2aChannelPool =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

package io.grpc.s2a.channel;
package io.grpc.s2a.internal.channel;

import com.google.errorprone.annotations.CanIgnoreReturnValue;
import io.grpc.Channel;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

package io.grpc.s2a.channel;
package io.grpc.s2a.internal.channel;

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

package io.grpc.s2a.channel;
package io.grpc.s2a.internal.channel;

import static com.google.common.base.Preconditions.checkNotNull;
import static java.util.concurrent.TimeUnit.SECONDS;
Expand Down Expand Up @@ -122,12 +122,12 @@ public String toString() {
* Manages a channel using a {@link ManagedChannel} instance.
*/
@VisibleForTesting
static class HandshakerServiceChannel extends Channel {
public static class HandshakerServiceChannel extends Channel {
private static final Logger logger =
Logger.getLogger(S2AHandshakerServiceChannel.class.getName());
private final ManagedChannel delegate;

static HandshakerServiceChannel create(ManagedChannel delegate) {
public static HandshakerServiceChannel create(ManagedChannel delegate) {
checkNotNull(delegate);
return new HandshakerServiceChannel(delegate);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@
* limitations under the License.
*/

package io.grpc.s2a.handshaker;
package io.grpc.s2a.internal.handshaker;

import java.io.IOException;

/** Indicates that a connection has been closed. */
@SuppressWarnings("serial") // This class is never serialized.
final class ConnectionClosedException extends IOException {
public final class ConnectionClosedException extends IOException {
public ConnectionClosedException(String errorMessage) {
super(errorMessage);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,17 @@
* limitations under the License.
*/

package io.grpc.s2a.handshaker;
package io.grpc.s2a.internal.handshaker;

import com.google.errorprone.annotations.Immutable;
import io.grpc.s2a.handshaker.S2AIdentity;
import io.grpc.s2a.handshaker.tokenmanager.AccessTokenManager;
//import io.grpc.s2a.internal.handshaker.GetAuthenticationMechanisms;
import io.grpc.s2a.handshaker.AuthenticationMechanism;
import io.grpc.s2a.internal.handshaker.tokenmanager.AccessTokenManager;
import java.util.Optional;

/** Retrieves the authentication mechanism for a given local identity. */
@Immutable
final class GetAuthenticationMechanisms {
public final class GetAuthenticationMechanisms {
private static final Optional<AccessTokenManager> TOKEN_MANAGER = AccessTokenManager.create();

/**
Expand All @@ -32,7 +33,8 @@ final class GetAuthenticationMechanisms {
* @param localIdentity the identity for which to fetch a token.
* @return an {@link AuthenticationMechanism} for the given local identity.
*/
static Optional<AuthenticationMechanism> getAuthMechanism(Optional<S2AIdentity> localIdentity) {
public static Optional<AuthenticationMechanism> getAuthMechanism(
Optional<S2AIdentity> localIdentity) {
if (!TOKEN_MANAGER.isPresent()) {
return Optional.empty();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,22 @@
* limitations under the License.
*/

package io.grpc.s2a.handshaker;
package io.grpc.s2a.internal.handshaker;

import com.google.common.collect.ImmutableSet;
import io.grpc.s2a.handshaker.Ciphersuite;
import io.grpc.s2a.handshaker.TLSVersion;

/** Converts proto messages to Netty strings. */
final class ProtoUtil {
public final class ProtoUtil {
/**
* Converts {@link Ciphersuite} to its {@link String} representation.
*
* @param ciphersuite the {@link Ciphersuite} to be converted.
* @return a {@link String} representing the ciphersuite.
* @throws AssertionError if the {@link Ciphersuite} is not one of the supported ciphersuites.
*/
static String convertCiphersuite(Ciphersuite ciphersuite) {
public static String convertCiphersuite(Ciphersuite ciphersuite) {
switch (ciphersuite) {
case CIPHERSUITE_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256:
return "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256";
Expand All @@ -54,7 +56,7 @@ static String convertCiphersuite(Ciphersuite ciphersuite) {
* @return a {@link String} representation of the TLS version.
* @throws AssertionError if the {@code tlsVersion} is not one of the supported TLS versions.
*/
static String convertTlsProtocolVersion(TLSVersion tlsVersion) {
public static String convertTlsProtocolVersion(TLSVersion tlsVersion) {
switch (tlsVersion) {
case TLS_VERSION_1_3:
return "TLSv1.3";
Expand All @@ -74,7 +76,7 @@ static String convertTlsProtocolVersion(TLSVersion tlsVersion) {
* Builds a set of strings representing all {@link TLSVersion}s between {@code minTlsVersion} and
* {@code maxTlsVersion}.
*/
static ImmutableSet<String> buildTlsProtocolVersionSet(
public static ImmutableSet<String> buildTlsProtocolVersionSet(
TLSVersion minTlsVersion, TLSVersion maxTlsVersion) {
ImmutableSet.Builder<String> tlsVersions = ImmutableSet.<String>builder();
for (TLSVersion tlsVersion : TLSVersion.values()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@
* limitations under the License.
*/

package io.grpc.s2a.handshaker;
package io.grpc.s2a.internal.handshaker;

/** Exception that denotes a runtime error that was encountered when talking to the S2A server. */
@SuppressWarnings("serial") // This class is never serialized.
public class S2AConnectionException extends RuntimeException {
S2AConnectionException(String message) {
public S2AConnectionException(String message) {
super(message);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@
* limitations under the License.
*/

package io.grpc.s2a.handshaker;
package io.grpc.s2a.internal.handshaker;

import static com.google.common.base.Preconditions.checkNotNull;

import com.google.errorprone.annotations.ThreadSafe;
import io.grpc.s2a.handshaker.Identity;

/**
* Stores an identity in such a way that it can be sent to the S2A handshaker service. The identity
Expand Down
Loading