Simplify DnsSrvDiscoverer interface

Refactor DnsSrvDiscoverer constructor to accept service, protocol, and
domain as discrete arguments to better match the specifications outlined
in RFC 2782 and make clearer the concerns and contract between the
discoverer and the caller. The latter should not need to know about
discovery details but only the service to be discovered and the scope of
that discovery (the domain).

Domain is now optional where an omitted value results in discovery
relative to the host's configured search domain, supporting
installations where full DNS based discovery is desired over explicit
configuration. This makes the simplest use case:

    $servers = ( new DnsSrvDiscoverer( 'service' ) )->getServers();

Overall use of the interface is simplified by changing `getServers()` to
unconditionally sort results by priority according to RFC 2872 and
return a simplified result of host/port tuples (the only data relevant
in primary discovery use cases). This makes the handling of discovery
fallback simpler since the caller can construct a default without having
to populate all response record details.

Example

    $dsd = new DnsSrvDiscoverer( $service, $proto, $host );
    $servers = $dsd->getServers() ?: [ [ $host, $defaultPort ] ];

A `getRecords()` method is provided as public to allow a caller to
obtain details on the response (and do its own sorting) should that be
needed.

Added an optional `$resolver` argument to the constructor (which
defaults to `dns_get_record`) to make the class implementation more
testable, and wrote functional tests.

EtcdConfig has been refactored to reflect the new usage of
DnsSrvDiscoverer and now sets the discoverer as a property in the
constructor to allow for mocking in tests.

Implemented tests for etcd DNS SRV discovery within EtcdConfig.

Bug: T296771
Change-Id: Idbd60049853439f96ff6045e01aa03014b4e587f

1 files changed, 145 insertions, 2 deletions

diff --git a/tests/phpunit/unit/includes/config/EtcdConfigTest.php b/tests/phpunit/unit/includes/config/EtcdConfigTest.php
index 8b730f0ba2e6..a63b7588208c 100644
--- a/tests/phpunit/unit/includes/config/EtcdConfigTest.php
+++ b/tests/phpunit/unit/includes/config/EtcdConfigTest.php
@@ -4,14 +4,14 @@ use Wikimedia\TestingAccessWrapper;
 
 class EtcdConfigTest extends MediaWikiUnitTestCase {
 
-	private function createConfigMock( array $options = [] ) {
+	private function createConfigMock( array $options = [], ?array $methods = null ) {
 		return $this->getMockBuilder( EtcdConfig::class )
 			->setConstructorArgs( [ $options + [
 				'host' => 'etcd-tcp.example.net',
 				'directory' => '/',
 				'timeout' => 0.1,
 			] ] )
-			->onlyMethods( [ 'fetchAllFromEtcd' ] )
+			->onlyMethods( $methods ?? [ 'fetchAllFromEtcd' ] )
 			->getMock();
 	}
 
@@ -35,6 +35,13 @@ class EtcdConfigTest extends MediaWikiUnitTestCase {
 		return $mock;
 	}
 
+	private function createCallableMock() {
+		return $this
+			->getMockBuilder( \stdClass::class )
+			->addMethods( [ '__invoke' ] )
+			->getMock();
+	}
+
 	/**
 	 * @covers EtcdConfig::has
 	 */
@@ -615,4 +622,140 @@ class EtcdConfigTest extends MediaWikiUnitTestCase {
 			$conf->fetchAllFromEtcdServer( 'etcd-tcp.example.net' )
 		);
 	}
+
+	/**
+	 * @covers EtcdConfig::fetchAllFromEtcdServer
+	 */
+	public function testFetchFromServerWithoutPort() {
+		$conf = $this->getMockBuilder( EtcdConfig::class )
+			->disableOriginalConstructor()
+			->getMock();
+
+		$http = $this->getMockBuilder( MultiHttpClient::class )
+			->disableOriginalConstructor()
+			->getMock();
+
+		$conf = TestingAccessWrapper::newFromObject( $conf );
+		$conf->protocol = 'https';
+		$conf->http = $http;
+
+		$http
+			->expects( $this->once() )
+			->method( 'run' )
+			->with(
+				$this->logicalAnd(
+					$this->arrayHasKey( 'url' ),
+					$this->callback( function ( $request ) {
+						$this->assertStringStartsWith(
+							'https://etcd.example/',
+							$request['url']
+						);
+						return true;
+					} )
+				)
+			);
+
+		$conf->fetchAllFromEtcdServer( 'etcd.example' );
+	}
+
+	/**
+	 * @covers EtcdConfig::fetchAllFromEtcdServer
+	 */
+	public function testFetchFromServerWithPort() {
+		$conf = $this->getMockBuilder( EtcdConfig::class )
+			->disableOriginalConstructor()
+			->getMock();
+
+		$http = $this->getMockBuilder( MultiHttpClient::class )
+			->disableOriginalConstructor()
+			->getMock();
+
+		$conf = TestingAccessWrapper::newFromObject( $conf );
+		$conf->protocol = 'https';
+		$conf->http = $http;
+
+		$http
+			->expects( $this->once() )
+			->method( 'run' )
+			->with(
+				$this->logicalAnd(
+					$this->arrayHasKey( 'url' ),
+					$this->callback( function ( $request ) {
+						$this->assertStringStartsWith(
+							'https://etcd.example:4001/',
+							$request['url']
+						);
+						return true;
+					} )
+				)
+			);
+
+		$conf->fetchAllFromEtcdServer( 'etcd.example', 4001 );
+	}
+
+	/**
+	 * @covers EtcdConfig::fetchAllFromEtcd
+	 */
+	public function testServiceDiscovery() {
+		$conf = $this->createConfigMock(
+			[ 'host' => 'an.example' ],
+			[ 'fetchAllFromEtcdServer' ]
+		);
+		$conf = TestingAccessWrapper::newFromObject( $conf );
+
+		$conf->dsd = TestingAccessWrapper::newFromObject( $conf->dsd );
+		$conf->dsd->resolver = $this->createCallableMock();
+		$conf->dsd->resolver
+			->expects( $this->once() )
+			->method( '__invoke' )
+			->with( '_etcd._tcp.an.example' )
+			->willReturn( [
+				[
+					'target' => 'etcd-target.an.example',
+					'port' => '2379',
+					'pri' => '1',
+					'weight' => '1',
+				],
+			] );
+
+		$conf->expects( $this->once() )
+			->method( 'fetchAllFromEtcdServer' )
+			->with( 'etcd-target.an.example', 2379 )
+			->willReturn( self::createEtcdResponse( [ 'foo' => true ] ) );
+
+		$conf->fetchAllFromEtcd();
+	}
+
+	/**
+	 * @covers EtcdConfig::fetchAllFromEtcd
+	 */
+	public function testServiceDiscoverySrvRecordAsHost() {
+		$conf = $this->createConfigMock(
+			[ 'host' => '_etcd-client-ssl._tcp.an.example' ],
+			[ 'fetchAllFromEtcdServer' ]
+		);
+		$conf = TestingAccessWrapper::newFromObject( $conf );
+
+		$conf->dsd = TestingAccessWrapper::newFromObject( $conf->dsd );
+		$conf->dsd->resolver = $this->createCallableMock();
+		$conf->dsd->resolver
+			->expects( $this->once() )
+			->method( '__invoke' )
+			->with( '_etcd-client-ssl._tcp.an.example' )
+			->willReturn( [
+				[
+					'target' => 'etcd-target.an.example',
+					'port' => '2379',
+					'pri' => '1',
+					'weight' => '1',
+				],
+			] );
+
+		$conf->expects( $this->once() )
+			->method( 'fetchAllFromEtcdServer' )
+			->with( 'etcd-target.an.example', 2379 )
+			->willReturn( self::createEtcdResponse( [ 'foo' => true ] ) );
+
+		$conf->fetchAllFromEtcd();
+	}
 }