Skip to content

s2a: refactored all the classes into a package #11809

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
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
@@ -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
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
@@ -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();

@@ -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;

@@ -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();
@@ -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 {
@@ -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());
@@ -615,6 +625,7 @@ public void maxHeaderListSizeShouldBeEnforcedOnServer() throws Exception {
Status status = Status.fromThrowable(e);
assertEquals(status.toString(), Status.Code.INTERNAL, status.getCode());
}

}

@Test
@@ -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()
@@ -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)));
@@ -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();

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

private static final class EchoServerStreamListener implements ServerStreamListener {

final ServerStream stream;
final Metadata headers;

@@ -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) {
@@ -996,7 +1012,8 @@ public Attributes transportReady(Attributes transportAttrs) {
}

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

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

private static final class StringMarshaller implements Marshaller<String> {

static final StringMarshaller INSTANCE = new StringMarshaller();

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

private static class NoopProtocolNegotiator implements ProtocolNegotiator {

GrpcHttp2ConnectionHandler grpcHandler;
NoopHandler handler;

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

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

private static final class SocketPicker extends LocalSocketPicker {
@@ -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
@@ -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;
@@ -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;

@@ -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;
@@ -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 =
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
@@ -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;
@@ -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);
}
Original file line number Diff line number Diff line change
@@ -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);
}
Original file line number Diff line number Diff line change
@@ -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();

/**
@@ -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();
}
Original file line number Diff line number Diff line change
@@ -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";
@@ -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";
@@ -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()) {
Original file line number Diff line number Diff line change
@@ -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
@@ -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
Loading