Skip to content

Commit

Permalink
Address PR review feedback
Browse files Browse the repository at this point in the history
Check for key existence, not key truthiness.
  • Loading branch information
jportner committed Jan 11, 2020
1 parent 75d0f70 commit 3120c8f
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 4 deletions.
9 changes: 7 additions & 2 deletions src/core/server/elasticsearch/elasticsearch_config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ describe('deprecations', () => {
});

it('logs a warning if ssl.key is set and ssl.certificate is not', () => {
const { messages } = applyElasticsearchDeprecations({ ssl: { key: 'foo' } });
const { messages } = applyElasticsearchDeprecations({ ssl: { key: '' } });
expect(messages).toMatchInlineSnapshot(`
Array [
"Setting [${CONFIG_PATH}.ssl.key] without [${CONFIG_PATH}.ssl.certificate] is deprecated. This has no effect, you should use both settings to enable TLS client authentication to Elasticsearch.",
Expand All @@ -340,13 +340,18 @@ describe('deprecations', () => {
});

it('logs a warning if ssl.certificate is set and ssl.key is not', () => {
const { messages } = applyElasticsearchDeprecations({ ssl: { certificate: 'foo' } });
const { messages } = applyElasticsearchDeprecations({ ssl: { certificate: '' } });
expect(messages).toMatchInlineSnapshot(`
Array [
"Setting [${CONFIG_PATH}.ssl.certificate] without [${CONFIG_PATH}.ssl.key] is deprecated. This has no effect, you should use both settings to enable TLS client authentication to Elasticsearch.",
]
`);
});

it('does not log a warning if both ssl.key and ssl.certificate are set', () => {
const { messages } = applyElasticsearchDeprecations({ ssl: { key: '', certificate: '' } });
expect(messages).toEqual([]);
});
});

test('#username throws if equal to "elastic", only while running from source', () => {
Expand Down
4 changes: 2 additions & 2 deletions src/core/server/elasticsearch/elasticsearch_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,11 @@ const deprecations: ConfigDeprecationProvider = () => [
`Setting [${fromPath}.username] to "elastic" is deprecated. You should use the "kibana" user instead.`
);
}
if (es.ssl?.key && !es.ssl?.certificate) {
if (es.ssl?.key !== undefined && es.ssl?.certificate === undefined) {
log(
`Setting [${fromPath}.ssl.key] without [${fromPath}.ssl.certificate] is deprecated. This has no effect, you should use both settings to enable TLS client authentication to Elasticsearch.`
);
} else if (es.ssl?.certificate && !es.ssl?.key) {
} else if (es.ssl?.certificate !== undefined && es.ssl?.key === undefined) {
log(
`Setting [${fromPath}.ssl.certificate] without [${fromPath}.ssl.key] is deprecated. This has no effect, you should use both settings to enable TLS client authentication to Elasticsearch.`
);
Expand Down

0 comments on commit 3120c8f

Please sign in to comment.