Skip to content

Support for publish_address as hostname/ip:port #804

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

Merged
merged 3 commits into from
Apr 8, 2019
Merged
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
2 changes: 1 addition & 1 deletion lib/ConnectionPool.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ export default class ConnectionPool {
* @param {object} nodes
* @returns {array} hosts
*/
nodesToHost(nodes: any): any[];
nodesToHost(nodes: any, protocol: string): any[];
/**
* Transforms an url string to a host object
*
Expand Down
21 changes: 18 additions & 3 deletions lib/ConnectionPool.js
Original file line number Diff line number Diff line change
Expand Up @@ -332,18 +332,33 @@ class ConnectionPool {
* @param {object} nodes
* @returns {array} hosts
*/
nodesToHost (nodes) {
nodesToHost (nodes, protocol) {
const ids = Object.keys(nodes)
const hosts = []

for (var i = 0, len = ids.length; i < len; i++) {
const node = nodes[ids[i]]
// If there is no protocol in
// the `publish_address` new URL wil throw
// the `publish_address` new URL will throw
// the publish_address can have two forms:
// - ip:port
// - hostname/ip:port
// if we encounter the second case, we should
// use the hostname instead of the ip
var address = node.http.publish_address
const hostAndIpRegex = /^[a-z0-9_.-]*\/\b\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}\b/gi
const match = address.match(hostAndIpRegex)
if (match !== null) {
const ipRegex = /\b\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}\b/
const ip = address.match(ipRegex)[0]
// extract the hostname, the -1 at the end removes the final /
const hostname = address.slice(0, address.indexOf(ip) - 1)
const port = address.split(':')[1]
address = `${hostname}:${port}`
}
address = address.slice(0, 4) === 'http'
? address
: 'http://' + address
: `${protocol}//${address}`
const roles = node.roles.reduce((acc, role) => {
acc[role] = true
return acc
Expand Down
3 changes: 2 additions & 1 deletion lib/Transport.js
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,8 @@ class Transport {
}

debug('Sniffing ended successfully', result.body)
const hosts = this.connectionPool.nodesToHost(result.body.nodes)
const protocol = result.meta.connection.url.protocol || 'http:'
const hosts = this.connectionPool.nodesToHost(result.body.nodes, protocol)
this.connectionPool.update(hosts)

result.meta.sniff = { hosts, reason }
Expand Down
33 changes: 33 additions & 0 deletions test/behavior/sniff.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,39 @@ test('Should update the connection pool', t => {
})
})

test('Should handle hostnames in publish_address', t => {
t.plan(10)

buildCluster({ hostPublishAddress: true }, ({ nodes, shutdown }) => {
const client = new Client({
node: nodes[Object.keys(nodes)[0]].url
})
t.strictEqual(client.connectionPool.connections.size, 1)

client.on(events.SNIFF, (err, request) => {
t.error(err)
t.strictEqual(
request.meta.sniff.reason,
Transport.sniffReasons.DEFAULT
)
})

// run the sniffer
client.transport.sniff((err, hosts) => {
t.error(err)
t.strictEqual(hosts.length, 4)

for (var i = 0; i < hosts.length; i++) {
// the first node will be an update of the existing one
t.strictEqual(hosts[i].url.hostname, 'localhost')
}

t.strictEqual(client.connectionPool.connections.size, 4)
})
t.teardown(shutdown)
})
})

test('Sniff interval', t => {
t.plan(10)

Expand Down
131 changes: 101 additions & 30 deletions test/unit/connection-pool.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -282,41 +282,112 @@ test('API', t => {
})

t.test('nodesToHost', t => {
const pool = new ConnectionPool({ Connection })
const nodes = {
a1: {
http: {
publish_address: '127.0.0.1:9200'
},
roles: ['master', 'data', 'ingest']
},
a2: {
http: {
publish_address: '127.0.0.1:9202'
t.test('publish_address as ip address', t => {
const pool = new ConnectionPool({ Connection })
const nodes = {
a1: {
http: {
publish_address: '127.0.0.1:9200'
},
roles: ['master', 'data', 'ingest']
},
roles: ['master', 'data', 'ingest']
a2: {
http: {
publish_address: '127.0.0.1:9201'
},
roles: ['master', 'data', 'ingest']
}
}
}

t.deepEqual(pool.nodesToHost(nodes), [{
url: new URL('http://127.0.0.1:9200'),
id: 'a1',
roles: {
master: true,
data: true,
ingest: true,
ml: false
t.deepEqual(pool.nodesToHost(nodes, 'http:'), [{
url: new URL('http://127.0.0.1:9200'),
id: 'a1',
roles: {
master: true,
data: true,
ingest: true,
ml: false
}
}, {
url: new URL('http://127.0.0.1:9201'),
id: 'a2',
roles: {
master: true,
data: true,
ingest: true,
ml: false
}
}])

t.strictEqual(pool.nodesToHost(nodes, 'http:')[0].url.host, '127.0.0.1:9200')
t.strictEqual(pool.nodesToHost(nodes, 'http:')[1].url.host, '127.0.0.1:9201')
t.end()
})

t.test('publish_address as host/ip', t => {
const pool = new ConnectionPool({ Connection })
const nodes = {
a1: {
http: {
publish_address: 'example.com/127.0.0.1:9200'
},
roles: ['master', 'data', 'ingest']
},
a2: {
http: {
publish_address: 'example.com/127.0.0.1:9201'
},
roles: ['master', 'data', 'ingest']
}
}
}, {
url: new URL('http://127.0.0.1:9201'),
id: 'a2',
roles: {
master: true,
data: true,
ingest: true,
ml: false

t.deepEqual(pool.nodesToHost(nodes, 'http:'), [{
url: new URL('http://example.com:9200'),
id: 'a1',
roles: {
master: true,
data: true,
ingest: true,
ml: false
}
}, {
url: new URL('http://example.com:9201'),
id: 'a2',
roles: {
master: true,
data: true,
ingest: true,
ml: false
}
}])

t.strictEqual(pool.nodesToHost(nodes, 'http:')[0].url.host, 'example.com:9200')
t.strictEqual(pool.nodesToHost(nodes, 'http:')[1].url.host, 'example.com:9201')
t.end()
})

t.test('Should use the configure protocol', t => {
const pool = new ConnectionPool({ Connection })
const nodes = {
a1: {
http: {
publish_address: 'example.com/127.0.0.1:9200'
},
roles: ['master', 'data', 'ingest']
},
a2: {
http: {
publish_address: 'example.com/127.0.0.1:9201'
},
roles: ['master', 'data', 'ingest']
}
}
}])

t.strictEqual(pool.nodesToHost(nodes, 'https:')[0].url.protocol, 'https:')
t.strictEqual(pool.nodesToHost(nodes, 'http:')[1].url.protocol, 'http:')
t.end()
})

t.end()
})

Expand Down
6 changes: 4 additions & 2 deletions test/utils/buildCluster.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,14 @@ function buildCluster (options, callback) {

buildServer(options.handler || handler, ({ port }, server) => {
nodes[opts.id] = {
url: `http://localhost:${port}`,
url: `http://127.0.0.1:${port}`,
server
}
sniffResult.nodes[opts.id] = {
http: {
publish_address: `http://localhost:${port}`
publish_address: options.hostPublishAddress
? `localhost/127.0.0.1:${port}`
: `127.0.0.1:${port}`
},
roles: ['master', 'data', 'ingest']
}
Expand Down