Skip to content

Commit

Permalink
Allow to provide a custom http agent and disable the auth header (#284)
Browse files Browse the repository at this point in the history
  • Loading branch information
slvrtrn authored Jun 19, 2024
1 parent 9bdea17 commit e88d3eb
Show file tree
Hide file tree
Showing 14 changed files with 350 additions and 66 deletions.
31 changes: 31 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,34 @@
# 1.2.0 (Node.js)

## New features

- (Experimental) Added an option to provide a custom HTTP Agent in the client configuration via the `http_agent` option ([#283](https://github.com/ClickHouse/clickhouse-js/issues/283), related: [#278](https://github.com/ClickHouse/clickhouse-js/issues/278)). The following conditions apply if a custom HTTP Agent is provided:
- The `max_open_connections` and `tls` options will have _no effect_ and will be ignored by the client, as it is a part of the underlying HTTP Agent configuration.
- `keep_alive.enabled` will only regulate the default value of the `Connection` header (`true` -> `Connection: keep-alive`, `false` -> `Connection: close`).
- While the idle socket management will still work, it is now possible to disable it completely by setting the `keep_alive.idle_socket_ttl` value to `0`.
- (Experimental) Added a new client configuration option: `set_basic_auth_header`, which disables the `Authorization` header that is set by the client by default for every outgoing HTTP request. One of the possible scenarios when it is necessary to disable this header is when a custom HTTPS agent is used, and the server requires TLS authorization. For example:

```ts
const agent = new https.Agent({
ca: fs.readFileSync('./ca.crt'),
})
const client = createClient({
url: 'https://server.clickhouseconnect.test:8443',
http_agent: agent,
// With a custom HTTPS agent, the client won't use the default HTTPS connection implementation; the headers should be provided manually
http_headers: {
'X-ClickHouse-User': 'default',
'X-ClickHouse-Key': '',
},
// Authorization header conflicts with the TLS headers; disable it.
set_basic_auth_header: false,
})
```

NB: It is currently not possible to set the `set_basic_auth_header` option via the URL params.

If you have feedback on these experimental features, please let us know by creating [an issue](https://github.com/ClickHouse/clickhouse-js/issues) in the repository.

# 1.1.0 (Common, Node.js, Web)

## New features
Expand Down
2 changes: 1 addition & 1 deletion packages/client-common/src/version.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export default '1.1.0'
export default '1.2.0'
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { TestEnv, whenOnEnv } from '@test/utils'
import http from 'http'
import Http from 'http'
import { createClient } from '../../src'

/** HTTPS agent tests are in tls.test.ts as it requires a secure connection. */
describe('[Node.js] custom HTTP agent', () => {
let httpRequestStub: jasmine.Spy<typeof Http.request>
beforeEach(() => {
httpRequestStub = spyOn(Http, 'request').and.callThrough()
})

// disabled with Cloud as it uses a simple HTTP agent
whenOnEnv(TestEnv.LocalSingleNode, TestEnv.LocalCluster).it(
'should use provided http agent instead of the default one',
async () => {
const agent = new http.Agent({
maxFreeSockets: 5,
})
const client = createClient({
http_agent: agent,
})
const rs = await client.query({
query: 'SELECT 42 AS result',
format: 'JSONEachRow',
})
expect(await rs.json()).toEqual([{ result: 42 }])
expect(httpRequestStub).toHaveBeenCalledTimes(1)
const callArgs = httpRequestStub.calls.mostRecent().args
expect(callArgs[1].agent).toBe(agent)
},
)
})
33 changes: 33 additions & 0 deletions packages/client-node/__tests__/tls/tls.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import type { ClickHouseClient } from '@clickhouse/client-common'
import { createTestClient } from '@test/utils'
import * as fs from 'fs'
import Http from 'http'
import https from 'node:https'
import type Stream from 'stream'
import { createClient } from '../../src'

Expand Down Expand Up @@ -135,5 +137,36 @@ describe('[Node.js] TLS connection', () => {
})
expect(await resultSet.text()).toEqual('0\n1\n2\n')
})

describe('Custom HTTPS agent', () => {
let httpRequestStub: jasmine.Spy<typeof Http.request>
beforeEach(() => {
httpRequestStub = spyOn(Http, 'request').and.callThrough()
})

it('should work with a custom HTTPS agent', async () => {
const agent = new https.Agent({
maxFreeSockets: 5,
ca: ca_cert,
})
const client = createClient({
url: 'https://server.clickhouseconnect.test:8443',
http_agent: agent,
http_headers: {
'X-ClickHouse-User': 'default',
'X-ClickHouse-Key': '',
},
set_basic_auth_header: false,
})
const rs = await client.query({
query: 'SELECT 144 AS result',
format: 'JSONEachRow',
})
expect(await rs.json()).toEqual([{ result: 144 }])
expect(httpRequestStub).toHaveBeenCalledTimes(1)
const callArgs = httpRequestStub.calls.mostRecent().args
expect(callArgs[1].agent).toBe(agent)
})
})
})
})
28 changes: 18 additions & 10 deletions packages/client-node/__tests__/unit/node_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type {
} from '@clickhouse/client-common'
import { DefaultLogger, LogWriter } from '@clickhouse/client-common'
import { createClient } from '../../src'
import type { CreateConnectionParams } from '../../src/connection'
import * as c from '../../src/connection/create_connection'

describe('[Node.js] createClient', () => {
Expand Down Expand Up @@ -66,14 +67,16 @@ describe('[Node.js] createClient', () => {
'keep_alive_idle_socket_ttl=1500',
].join('&'),
})
expect(createConnectionStub).toHaveBeenCalledWith(
params,
undefined, // TLS
{
expect(createConnectionStub).toHaveBeenCalledWith({
connection_params: params,
tls: undefined,
keep_alive: {
enabled: true,
idle_socket_ttl: 1500,
},
)
set_basic_auth_header: true,
http_agent: undefined,
} satisfies CreateConnectionParams)
expect(createConnectionStub).toHaveBeenCalledTimes(1)
})

Expand All @@ -91,14 +94,19 @@ describe('[Node.js] createClient', () => {
'keep_alive_idle_socket_ttl=1500',
].join('&'),
})
expect(createConnectionStub).toHaveBeenCalledWith(
{ ...params, url: new URL('https://my.host:8443/my_proxy') },
undefined, // TLS
{
expect(createConnectionStub).toHaveBeenCalledWith({
connection_params: {
...params,
url: new URL('https://my.host:8443/my_proxy'),
},
tls: undefined,
keep_alive: {
enabled: true,
idle_socket_ttl: 1500,
},
)
set_basic_auth_header: true,
http_agent: undefined,
} satisfies CreateConnectionParams)
expect(createConnectionStub).toHaveBeenCalledTimes(1)
})
})
Expand Down
82 changes: 61 additions & 21 deletions packages/client-node/__tests__/unit/node_config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,13 @@ import type {
import { LogWriter } from '@clickhouse/client-common'
import { TestLogger } from '@test/utils'
import { Buffer } from 'buffer'
import http from 'http'
import type { NodeClickHouseClientConfigOptions } from '../../src/config'
import { NodeConfigImpl } from '../../src/config'
import type { NodeBaseConnection } from '../../src/connection'
import type {
CreateConnectionParams,
NodeBaseConnection,
} from '../../src/connection'
import * as c from '../../src/connection/create_connection'

describe('[Node.js] Config implementation details', () => {
Expand Down Expand Up @@ -88,14 +92,16 @@ describe('[Node.js] Config implementation details', () => {
url: new URL('http://localhost:8123'),
}
const res = NodeConfigImpl.make_connection(nodeConfig as any, params)
expect(createConnectionStub).toHaveBeenCalledWith(
params,
undefined, // TLS
{
expect(createConnectionStub).toHaveBeenCalledWith({
connection_params: params,
tls: undefined,
keep_alive: {
enabled: true,
idle_socket_ttl: 2500,
},
)
http_agent: undefined,
set_basic_auth_header: true,
} satisfies CreateConnectionParams)
expect(createConnectionStub).toHaveBeenCalledTimes(1)
expect(res).toEqual(fakeConnection)
})
Expand All @@ -108,17 +114,19 @@ describe('[Node.js] Config implementation details', () => {
},
}
const res = NodeConfigImpl.make_connection(nodeConfig as any, params)
expect(createConnectionStub).toHaveBeenCalledWith(
params,
{
expect(createConnectionStub).toHaveBeenCalledWith({
connection_params: params,
tls: {
type: 'Basic',
ca_cert: Buffer.from('my_ca_cert'),
},
{
keep_alive: {
enabled: true,
idle_socket_ttl: 2500,
},
)
http_agent: undefined,
set_basic_auth_header: true,
} satisfies CreateConnectionParams)
expect(createConnectionStub).toHaveBeenCalledTimes(1)
expect(res).toEqual(fakeConnection)
})
Expand All @@ -133,19 +141,21 @@ describe('[Node.js] Config implementation details', () => {
},
}
const res = NodeConfigImpl.make_connection(nodeConfig as any, params)
expect(createConnectionStub).toHaveBeenCalledWith(
params,
{
expect(createConnectionStub).toHaveBeenCalledWith({
connection_params: params,
tls: {
type: 'Mutual',
ca_cert: Buffer.from('my_ca_cert'),
cert: Buffer.from('my_cert'),
key: Buffer.from('my_key'),
},
{
keep_alive: {
enabled: true,
idle_socket_ttl: 2500,
},
)
http_agent: undefined,
set_basic_auth_header: true,
} satisfies CreateConnectionParams)
expect(createConnectionStub).toHaveBeenCalledTimes(1)
expect(res).toEqual(fakeConnection)
})
Expand All @@ -162,17 +172,47 @@ describe('[Node.js] Config implementation details', () => {
},
}
const res = NodeConfigImpl.make_connection(nodeConfig as any, params)
expect(createConnectionStub).toHaveBeenCalledWith(
params,
{
expect(createConnectionStub).toHaveBeenCalledWith({
connection_params: params,
tls: {
type: 'Basic',
ca_cert: Buffer.from('my_ca_cert'),
},
{
keep_alive: {
enabled: false,
idle_socket_ttl: 42_000,
},
)
http_agent: undefined,
set_basic_auth_header: true,
} satisfies CreateConnectionParams)
expect(createConnectionStub).toHaveBeenCalledTimes(1)
expect(res).toEqual(fakeConnection)
})

it('should create a connection with a custom agent and disabled auth header', async () => {
const agent = new http.Agent({
keepAlive: true,
maxSockets: 2,
})
const nodeConfig: NodeClickHouseClientConfigOptions = {
url: new URL('https://localhost:8123'),
keep_alive: {
enabled: true,
},
set_basic_auth_header: false,
http_agent: agent,
}
const res = NodeConfigImpl.make_connection(nodeConfig as any, params)
expect(createConnectionStub).toHaveBeenCalledWith({
connection_params: params,
tls: undefined,
keep_alive: {
enabled: true,
idle_socket_ttl: 2500,
},
http_agent: agent,
set_basic_auth_header: false,
} satisfies CreateConnectionParams)
expect(createConnectionStub).toHaveBeenCalledTimes(1)
expect(res).toEqual(fakeConnection)
})
Expand Down
Loading

0 comments on commit e88d3eb

Please sign in to comment.