Skip to content

Commit

Permalink
Configure client connections with a builder (#733)
Browse files Browse the repository at this point in the history
Motivation:

Configuring a client without TLS is easy to do, since no tls is the
default in the client connection configuration. This is bad. The API
should make it clearer that the default is insecure.

The current API for configuring clients falls a bit short in a number of
areas. In no particular order:

1. It can't be easily extended without SemVer major changes (since we'd
   have to provide a new init whenever new properties are added),
2. Nested configuration (i.e. TLS) is unnecessarily verbose,
3. The initializer requires properties to be passed in the correct
   order, and
4. TLS is disabled by default (!) and the API doesn't make this clear that
   this is insecure

Modifications:

- create a `ClientConnectionBuilder` whose public API is limited to
  configuring the private properties; this is similar to NIOs bootstraps
- provide static methods on `ClientConnection` to provide a `secure` or
  `insecure` builder
- update tests in a couple of places to demonstrate usage

Result:

- Configuring a connection is less verbose
- Not using TLS is more obviously 'insecure'
  • Loading branch information
glbrntt authored Mar 6, 2020
1 parent 98b7d72 commit 86ea799
Show file tree
Hide file tree
Showing 4 changed files with 206 additions and 24 deletions.
3 changes: 3 additions & 0 deletions Sources/GRPC/ClientConnection.swift
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,9 @@ public class ClientConnection {
}

/// Creates a new connection from the given configuration.
///
/// - Important: Users should prefer using `ClientConnection.secure(group:)` to build a connection
/// with TLS, or `ClientConnection.insecure(group:)` to build a connection without TLS.
public init(configuration: Configuration) {
self.configuration = configuration
self.scheme = configuration.tls == nil ? "http" : "https"
Expand Down
194 changes: 194 additions & 0 deletions Sources/GRPC/GRPCChannel/GRPCChannelBuilder.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,194 @@
/*
* Copyright 2020, gRPC Authors All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import NIO
import NIOSSL

extension ClientConnection {
/// Returns an insecure `ClientConnection` builder which is *not configured with TLS*.
public static func insecure(group: EventLoopGroup) -> ClientConnection.Builder {
return Builder(group: group)
}

/// Returns a `ClientConnection` builder configured with TLS.
public static func secure(group: EventLoopGroup) -> ClientConnection.Builder.Secure {
return Builder.Secure(group: group)
}
}

extension ClientConnection {
public class Builder {
private let group: EventLoopGroup
private var maybeTLS: ClientConnection.Configuration.TLS? { return nil }
private var connectionBackoff = ConnectionBackoff()
private var connectionBackoffIsEnabled = true
private var errorDelegate: ClientErrorDelegate?
private var connectivityStateDelegate: ConnectivityStateDelegate?

fileprivate init(group: EventLoopGroup) {
self.group = group
}

public func connect(host: String, port: Int) -> ClientConnection {
let configuration = ClientConnection.Configuration(
target: .hostAndPort(host, port),
eventLoopGroup: self.group,
errorDelegate: self.errorDelegate,
connectivityStateDelegate: self.connectivityStateDelegate,
tls: self.maybeTLS,
connectionBackoff: self.connectionBackoffIsEnabled ? self.connectionBackoff : nil
)
return ClientConnection(configuration: configuration)
}
}
}

extension ClientConnection.Builder {
public class Secure: ClientConnection.Builder {
internal var tls = ClientConnection.Configuration.TLS()
override internal var maybeTLS: ClientConnection.Configuration.TLS? {
return self.tls
}
}
}

extension ClientConnection.Builder {
/// Sets the initial connection backoff. That is, the initial time to wait before re-attempting to
/// establish a connection. Jitter will *not* be applied to the initial backoff. Defaults to
/// 1 second if not set.
@discardableResult
public func withConnectionBackoff(initial amount: TimeAmount) -> Self {
self.connectionBackoff.initialBackoff = .seconds(from: amount)
return self
}

/// Set the maximum connection backoff. That is, the maximum amount of time to wait before
/// re-attempting to establish a connection. Note that this time amount represents the maximum
/// backoff *before* jitter is applied. Defaults to 120 seconds if not set.
@discardableResult
public func withConnectionBackoff(maximum amount: TimeAmount) -> Self {
self.connectionBackoff.maximumBackoff = .seconds(from: amount)
return self
}

/// Backoff is 'jittered' to randomise the amount of time to wait before re-attempting to
/// establish a connection. The jittered backoff will be no more than `jitter ⨯ unjitteredBackoff`
/// from `unjitteredBackoff`. Defaults to 0.2 if not set.
///
/// - Precondition: `0 <= jitter <= 1`
@discardableResult
public func withConnectionBackoff(jitter: Double) -> Self {
self.connectionBackoff.jitter = jitter
return self
}

/// The multiplier for scaling the unjittered backoff between attempts to establish a connection.
/// Defaults to 1.6 if not set.
@discardableResult
public func withConnectionBackoff(multiplier: Double) -> Self {
self.connectionBackoff.multiplier = multiplier
return self
}

/// The minimum timeout to use when attempting to establishing a connection. The connection
/// timeout for each attempt is the larger of the jittered backoff and the minimum connection
/// timeout. Defaults to 20 seconds if not set.
@discardableResult
public func withConnectionTimeout(minimum amount: TimeAmount) -> Self {
self.connectionBackoff.minimumConnectionTimeout = .seconds(from: amount)
return self
}

/// Sets the initial and maximum backoff to given amount. Disables jitter and sets the backoff
/// multiplier to 1.0.
@discardableResult
public func withConnectionBackoff(fixed amount: TimeAmount) -> Self {
let seconds = Double.seconds(from: amount)
self.connectionBackoff.initialBackoff = seconds
self.connectionBackoff.maximumBackoff = seconds
self.connectionBackoff.multiplier = 1.0
self.connectionBackoff.jitter = 0.0
return self
}

/// Sets whether the connection should be re-established automatically if it is dropped. Defaults
/// to `true` if not set.
@discardableResult
public func withConnectionReestablishment(enabled: Bool) -> Self {
self.connectionBackoffIsEnabled = enabled
return self
}
}

extension ClientConnection.Builder {
/// Sets the client error delegate.
@discardableResult
public func withErrorDelegate(_ delegate: ClientErrorDelegate?) -> Self {
self.errorDelegate = delegate
return self
}
}

extension ClientConnection.Builder {
/// Sets the client connectivity state delegate.
@discardableResult
public func withConnectivityStateDelegate(_ delegate: ConnectivityStateDelegate?) -> Self {
self.connectivityStateDelegate = delegate
return self
}
}

extension ClientConnection.Builder.Secure {
/// Sets a server hostname override to be used for the TLS Server Name Indication (SNI) extension.
/// The hostname from `connect(host:port)` is for TLS SNI if this value is not set and hostname
/// verification is enabled.
@discardableResult
public func withTLS(serverHostnameOverride: String?) -> Self {
self.tls.hostnameOverride = serverHostnameOverride
return self
}

/// Sets the sources of certificates to offer during negotiation. No certificates are offered
/// during negotiation by default.
@discardableResult
public func withTLS(certificateChain: [NIOSSLCertificate]) -> Self {
// `.certificate` is the only non-deprecated case in `NIOSSLCertificateSource`
self.tls.certificateChain = certificateChain.map { .certificate($0) }
return self
}

/// Sets the private key associated with the leaf certificate.
@discardableResult
public func withTLS(privateKey: NIOSSLPrivateKey) -> Self {
// `.privateKey` is the only non-deprecated case in `NIOSSLPrivateKeySource`
self.tls.privateKey = .privateKey(privateKey)
return self
}

/// Sets the trust roots to use to validate certificates. This only needs to be provided if you
/// intend to validate certificates. Defaults to the system provided trust store (`.default`) if
/// not set.
@discardableResult
public func withTLS(trustRoots: NIOSSLTrustRoots) -> Self {
self.tls.trustRoots = trustRoots
return self
}
}

fileprivate extension Double {
static func seconds(from amount: TimeAmount) -> Double {
return Double(amount.nanoseconds) / 1_000_000_000
}
}
26 changes: 7 additions & 19 deletions Tests/GRPCTests/ClientTLSTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,6 @@ class ClientTLSHostnameOverrideTests: GRPCTestCase {
return try Server.start(configuration: configuration).wait()
}

func makeConnection(port: Int, tls: ClientConnection.Configuration.TLS) -> ClientConnection {
let configuration: ClientConnection.Configuration = .init(
target: .hostAndPort("localhost", port),
eventLoopGroup: self.eventLoopGroup,
tls: tls
)

return ClientConnection(configuration: configuration)
}

func doTestUnary() throws {
let client = Echo_EchoServiceClient(channel: self.connection)
let get = client.get(.with { $0.text = "foo" })
Expand All @@ -85,12 +75,11 @@ class ClientTLSHostnameOverrideTests: GRPCTestCase {
return
}

let clientTLS: ClientConnection.Configuration.TLS = .init(
trustRoots: .certificates([SampleCertificate.ca.certificate]),
hostnameOverride: "example.com"
)
self.connection = ClientConnection.secure(group: self.eventLoopGroup)
.withTLS(trustRoots: .certificates([SampleCertificate.ca.certificate]))
.withTLS(serverHostnameOverride: "example.com")
.connect(host: "localhost", port: port)

self.connection = self.makeConnection(port: port, tls: clientTLS)
try self.doTestUnary()
}

Expand All @@ -108,11 +97,10 @@ class ClientTLSHostnameOverrideTests: GRPCTestCase {
return
}

let clientTLS: ClientConnection.Configuration.TLS = .init(
trustRoots: .certificates([SampleCertificate.ca.certificate])
)
self.connection = ClientConnection.secure(group: self.eventLoopGroup)
.withTLS(trustRoots: .certificates([SampleCertificate.ca.certificate]))
.connect(host: "localhost", port: port)

self.connection = self.makeConnection(port: port, tls: clientTLS)
try self.doTestUnary()
}
}
7 changes: 2 additions & 5 deletions Tests/GRPCTests/CompressionTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,9 @@ class MessageCompressionTests: GRPCTestCase {
}

func setupClient(encoding: ClientMessageEncoding) {
let configuration = ClientConnection.Configuration(
target: .hostAndPort("localhost", self.server.channel.localAddress!.port!),
eventLoopGroup: self.group
)
self.client = ClientConnection.insecure(group: self.group)
.connect(host: "localhost", port: self.server.channel.localAddress!.port!)

self.client = ClientConnection(configuration: configuration)
self.echo = Echo_EchoServiceClient(
channel: self.client,
defaultCallOptions: CallOptions(messageEncoding: encoding)
Expand Down

0 comments on commit 86ea799

Please sign in to comment.