瀏覽代碼

Deprecate ambiguous IPv4 and IPv6 target APIs (#117)

Motivation:

The API for creating IPv4/IPv6 resolvable targets is a bit ambiguous:
it's quite easy to assume they will only resolve IPv4 or IPv6 addresses.
Instead they expect to be provided with resolved IPv4 or IPv6 addresses.

Modifications:

- Deprecate these APIs and replace them with more obvious spellings
- Add a debug-only check to validate that the target is a valid
IPv4/IPv6 address

Result:

- Harder to make mistakes
- Easier to diagnose mistakes
George Barnett 4 月之前
父節點
當前提交
9e2bab9ece

+ 1 - 1
Package.swift

@@ -71,7 +71,7 @@ let dependencies: [Package.Dependency] = [
 
 // This adds some build settings which allow us to map "@available(gRPCSwiftNIOTransport 2.x, *)" to
 // the appropriate OS platforms.
-let nextMinorVersion = 0
+let nextMinorVersion = 1
 let availabilitySettings: [SwiftSetting] = (0 ... nextMinorVersion).map { minor in
   let name = "gRPCSwiftNIOTransport"
   let version = "2.\(minor)"

+ 32 - 1
Sources/GRPCNIOTransportCore/Client/Resolver/NameResolver+IPv4.swift

@@ -15,6 +15,7 @@
  */
 
 internal import GRPCCore
+internal import NIOCore
 
 @available(gRPCSwiftNIOTransport 2.0, *)
 extension ResolvableTargets {
@@ -29,6 +30,24 @@ extension ResolvableTargets {
     /// Create a new IPv4 target.
     /// - Parameter addresses: The IPv4 addresses.
     public init(addresses: [SocketAddress.IPv4]) {
+      debugOnly {
+        for address in addresses {
+          do {
+            switch try? NIOCore.SocketAddress(ipAddress: address.host, port: address.port) {
+            case .v4:
+              ()
+            default:
+              assertionFailure(
+                """
+                \(address.host):\(address.port) isn't a valid IPv4 address, did you mean to \
+                use 'dns(host:port:)' instead?
+                """
+              )
+            }
+          }
+        }
+      }
+
       self.addresses = addresses
     }
   }
@@ -38,14 +57,26 @@ extension ResolvableTargets {
 extension ResolvableTarget where Self == ResolvableTargets.IPv4 {
   /// Creates a new resolvable IPv4 target for a single address.
   /// - Parameters:
-  ///   - host: The host address.
+  ///   - host: The resolved host address.
   ///   - port: The port on the host.
   /// - Returns: A ``ResolvableTarget``.
+  @available(*, deprecated, renamed: "ipv4(address:port:)")
   public static func ipv4(host: String, port: Int = 443) -> Self {
     let address = SocketAddress.IPv4(host: host, port: port)
     return Self(addresses: [address])
   }
 
+  /// Creates a new resolvable IPv4 target for a single address.
+  /// - Parameters:
+  ///   - address: The resolved host address.
+  ///   - port: The port on the host.
+  /// - Returns: A ``ResolvableTarget``.
+  @available(gRPCSwiftNIOTransport 2.1, *)
+  public static func ipv4(address: String, port: Int = 443) -> Self {
+    let address = SocketAddress.IPv4(host: address, port: port)
+    return Self(addresses: [address])
+  }
+
   /// Creates a new resolvable IPv4 target from the provided host-port pairs.
   ///
   /// - Parameter pairs: An array of host-port pairs.

+ 31 - 1
Sources/GRPCNIOTransportCore/Client/Resolver/NameResolver+IPv6.swift

@@ -15,6 +15,7 @@
  */
 
 internal import GRPCCore
+internal import NIOCore
 
 @available(gRPCSwiftNIOTransport 2.0, *)
 extension ResolvableTargets {
@@ -29,6 +30,23 @@ extension ResolvableTargets {
     /// Create a new IPv6 target.
     /// - Parameter addresses: The IPv6 addresses.
     public init(addresses: [SocketAddress.IPv6]) {
+      debugOnly {
+        for address in addresses {
+          do {
+            switch try? NIOCore.SocketAddress(ipAddress: address.host, port: address.port) {
+            case .v6:
+              ()
+            default:
+              assertionFailure(
+                """
+                \(address.host):\(address.port) isn't a valid IPv6 address, did you mean to \
+                use 'dns(host:port:)' instead?
+                """
+              )
+            }
+          }
+        }
+      }
       self.addresses = addresses
     }
   }
@@ -38,14 +56,26 @@ extension ResolvableTargets {
 extension ResolvableTarget where Self == ResolvableTargets.IPv6 {
   /// Creates a new resolvable IPv6 target for a single address.
   /// - Parameters:
-  ///   - host: The host address.
+  ///   - host: The resolved host address.
   ///   - port: The port on the host.
   /// - Returns: A ``ResolvableTarget``.
+  @available(*, deprecated, renamed: "ipv6(address:port:)")
   public static func ipv6(host: String, port: Int = 443) -> Self {
     let address = SocketAddress.IPv6(host: host, port: port)
     return Self(addresses: [address])
   }
 
+  /// Creates a new resolvable IPv6 target for a single address.
+  /// - Parameters:
+  ///   - address: The resolved host address.
+  ///   - port: The port on the host.
+  /// - Returns: A ``ResolvableTarget``.
+  @available(gRPCSwiftNIOTransport 2.1, *)
+  public static func ipv6(address: String, port: Int = 443) -> Self {
+    let address = SocketAddress.IPv6(host: address, port: port)
+    return Self(addresses: [address])
+  }
+
   /// Creates a new resolvable IPv6 target from the provided host-port pairs.
   ///
   /// - Parameter pairs: An array of host-port pairs.

+ 25 - 0
Sources/GRPCNIOTransportCore/Internal/DebugOnly.swift

@@ -0,0 +1,25 @@
+/*
+ * Copyright 2025, 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.
+ */
+
+@inlinable
+internal func debugOnly(_ body: () -> Void) {
+  assert(
+    {
+      body()
+      return true
+    }()
+  )
+}

+ 14 - 11
Tests/GRPCNIOTransportCoreTests/Client/Resolver/NameResolverRegistryTests.swift

@@ -142,7 +142,7 @@ final class NameResolverRegistryTests: XCTestCase {
 
   func testMakeResolver() {
     let resolvers = NameResolverRegistry()
-    XCTAssertNil(resolvers.makeResolver(for: .ipv4(host: "foo")))
+    XCTAssertNil(resolvers.makeResolver(for: .ipv4(address: "127.0.0.1")))
   }
 
   func testCustomResolver() async throws {
@@ -192,7 +192,7 @@ final class NameResolverRegistryTests: XCTestCase {
 
   func testIPv4ResolverForSingleHost() async throws {
     let factory = NameResolvers.IPv4()
-    let resolver = factory.resolver(for: .ipv4(host: "foo", port: 1234))
+    let resolver = factory.resolver(for: .ipv4(address: "127.0.0.1", port: 1234))
 
     XCTAssertEqual(resolver.updateMode, .pull)
 
@@ -200,14 +200,17 @@ final class NameResolverRegistryTests: XCTestCase {
     var iterator = resolver.names.makeAsyncIterator()
     for _ in 0 ..< 1000 {
       let result = try await XCTUnwrapAsync { try await iterator.next() }
-      XCTAssertEqual(result.endpoints, [Endpoint(addresses: [.ipv4(host: "foo", port: 1234)])])
+      XCTAssertEqual(
+        result.endpoints,
+        [Endpoint(addresses: [.ipv4(host: "127.0.0.1", port: 1234)])]
+      )
       XCTAssertNil(result.serviceConfig)
     }
   }
 
   func testIPv4ResolverForMultipleHosts() async throws {
     let factory = NameResolvers.IPv4()
-    let resolver = factory.resolver(for: .ipv4(pairs: [("foo", 443), ("bar", 444)]))
+    let resolver = factory.resolver(for: .ipv4(pairs: [("127.0.0.1", 443), ("127.0.0.1", 444)]))
 
     XCTAssertEqual(resolver.updateMode, .pull)
 
@@ -218,8 +221,8 @@ final class NameResolverRegistryTests: XCTestCase {
       XCTAssertEqual(
         result.endpoints,
         [
-          Endpoint(addresses: [.ipv4(host: "foo", port: 443)]),
-          Endpoint(addresses: [.ipv4(host: "bar", port: 444)]),
+          Endpoint(addresses: [.ipv4(host: "127.0.0.1", port: 443)]),
+          Endpoint(addresses: [.ipv4(host: "127.0.0.1", port: 444)]),
         ]
       )
       XCTAssertNil(result.serviceConfig)
@@ -228,7 +231,7 @@ final class NameResolverRegistryTests: XCTestCase {
 
   func testIPv6ResolverForSingleHost() async throws {
     let factory = NameResolvers.IPv6()
-    let resolver = factory.resolver(for: .ipv6(host: "foo", port: 1234))
+    let resolver = factory.resolver(for: .ipv6(address: "::1", port: 1234))
 
     XCTAssertEqual(resolver.updateMode, .pull)
 
@@ -236,14 +239,14 @@ final class NameResolverRegistryTests: XCTestCase {
     var iterator = resolver.names.makeAsyncIterator()
     for _ in 0 ..< 1000 {
       let result = try await XCTUnwrapAsync { try await iterator.next() }
-      XCTAssertEqual(result.endpoints, [Endpoint(addresses: [.ipv6(host: "foo", port: 1234)])])
+      XCTAssertEqual(result.endpoints, [Endpoint(addresses: [.ipv6(host: "::1", port: 1234)])])
       XCTAssertNil(result.serviceConfig)
     }
   }
 
   func testIPv6ResolverForMultipleHosts() async throws {
     let factory = NameResolvers.IPv6()
-    let resolver = factory.resolver(for: .ipv6(pairs: [("foo", 443), ("bar", 444)]))
+    let resolver = factory.resolver(for: .ipv6(pairs: [("::1", 443), ("::1", 444)]))
 
     XCTAssertEqual(resolver.updateMode, .pull)
 
@@ -254,8 +257,8 @@ final class NameResolverRegistryTests: XCTestCase {
       XCTAssertEqual(
         result.endpoints,
         [
-          Endpoint(addresses: [.ipv6(host: "foo", port: 443)]),
-          Endpoint(addresses: [.ipv6(host: "bar", port: 444)]),
+          Endpoint(addresses: [.ipv6(host: "::1", port: 443)]),
+          Endpoint(addresses: [.ipv6(host: "::1", port: 444)]),
         ]
       )
       XCTAssertNil(result.serviceConfig)

+ 1 - 1
Tests/GRPCNIOTransportHTTP2Tests/HTTP2ServerTransport+DebugTests.swift

@@ -74,7 +74,7 @@ struct ChannelDebugCallbackTests {
         try await withGRPCClient(
           transport: self.makeClientTransport(
             kind: clientKind,
-            target: .ipv4(host: address.host, port: address.port),
+            target: .ipv4(address: address.host, port: address.port),
             debug: clientDebug
           )
         ) { client in

+ 1 - 1
Tests/GRPCNIOTransportHTTP2Tests/HTTP2TransportTLSEnabledTests.swift

@@ -673,7 +673,7 @@ struct HTTP2TransportTLSEnabledTests {
         throw TLSEnabledTestsError.unexpectedListeningAddress
       }
 
-      let target: any ResolvableTarget = .ipv4(host: address.host, port: address.port)
+      let target: any ResolvableTarget = .ipv4(address: address.host, port: address.port)
       let clientTransport: NIOClientTransport
       switch clientConfig {
       case .posix(let config):

+ 7 - 7
Tests/GRPCNIOTransportHTTP2Tests/HTTP2TransportTests.swift

@@ -60,9 +60,9 @@ final class HTTP2TransportTests: XCTestCase {
 
         let target: any ResolvableTarget
         if let ipv4 = address.ipv4 {
-          target = .ipv4(host: ipv4.host, port: ipv4.port)
+          target = .ipv4(address: ipv4.host, port: ipv4.port)
         } else if let ipv6 = address.ipv6 {
-          target = .ipv6(host: ipv6.host, port: ipv6.port)
+          target = .ipv6(address: ipv6.host, port: ipv6.port)
         } else if let uds = address.unixDomainSocket {
           target = .unixDomainSocket(path: uds.path)
         } else {
@@ -108,7 +108,7 @@ final class HTTP2TransportTests: XCTestCase {
         let address = try await server.listeningAddress
         let client = try self.makeClient(
           kind: clientKind,
-          target: .ipv4(host: address.host, port: address.port),
+          target: .ipv4(address: address.host, port: address.port),
           compression: .none,
           enabledCompression: .none
         )
@@ -1602,7 +1602,7 @@ final class HTTP2TransportTests: XCTestCase {
 
   func testAuthorityIPv4() async throws {
     try await self.testAuthority(serverAddress: .ipv4(host: "127.0.0.1", port: 0)) { address in
-      return .ipv4(host: "127.0.0.1", port: address.ipv4!.port)
+      return .ipv4(address: "127.0.0.1", port: address.ipv4!.port)
     } expectedAuthority: { address in
       return "127.0.0.1:\(address.ipv4!.port)"
     }
@@ -1613,7 +1613,7 @@ final class HTTP2TransportTests: XCTestCase {
       serverAddress: .ipv4(host: "127.0.0.1", port: 0),
       authorityOverride: "respect-my-authority"
     ) { address in
-      return .ipv4(host: "127.0.0.1", port: address.ipv4!.port)
+      return .ipv4(address: "127.0.0.1", port: address.ipv4!.port)
     } expectedAuthority: { _ in
       return "respect-my-authority"
     }
@@ -1621,7 +1621,7 @@ final class HTTP2TransportTests: XCTestCase {
 
   func testAuthorityIPv6() async throws {
     try await self.testAuthority(serverAddress: .ipv6(host: "::1", port: 0)) { address in
-      return .ipv6(host: "::1", port: address.ipv6!.port)
+      return .ipv6(address: "::1", port: address.ipv6!.port)
     } expectedAuthority: { address in
       return "[::1]:\(address.ipv6!.port)"
     }
@@ -1632,7 +1632,7 @@ final class HTTP2TransportTests: XCTestCase {
       serverAddress: .ipv6(host: "::1", port: 0),
       authorityOverride: "respect-my-authority"
     ) { address in
-      return .ipv6(host: "::1", port: address.ipv6!.port)
+      return .ipv6(address: "::1", port: address.ipv6!.port)
     } expectedAuthority: { _ in
       return "respect-my-authority"
     }