Deprecate using elasticsearch.ssl.certificate without elasticsearch.ssl.key and vice versa (#54392)

Author: jportner

src/core/server/config/deprecation/core_deprecations.test.ts

@@ -208,35 +208,4 @@ describe('core deprecations', () => {
       ).toEqual([`worker-src blob:`]);
     });
   });
-
-  describe('elasticsearchUsernameDeprecation', () => {
-    it('logs a warning if elasticsearch.username is set to "elastic"', () => {
-      const { messages } = applyCoreDeprecations({
-        elasticsearch: {
-          username: 'elastic',
-        },
-      });
-      expect(messages).toMatchInlineSnapshot(`
-        Array [
-          "Setting elasticsearch.username to \\"elastic\\" is deprecated. You should use the \\"kibana\\" user instead.",
-        ]
-      `);
-    });
-
-    it('does not log a warning if elasticsearch.username is set to something besides "elastic"', () => {
-      const { messages } = applyCoreDeprecations({
-        elasticsearch: {
-          username: 'otheruser',
-        },
-      });
-      expect(messages).toHaveLength(0);
-    });
-
-    it('does not log a warning if elasticsearch.username is unset', () => {
-      const { messages } = applyCoreDeprecations({
-        elasticsearch: {},
-      });
-      expect(messages).toHaveLength(0);
-    });
-  });
 });

src/core/server/config/deprecation/core_deprecations.ts

@@ -91,16 +91,6 @@ const cspRulesDeprecation: ConfigDeprecation = (settings, fromPath, log) => {
   return settings;
 };
 
-const elasticsearchUsernameDeprecation: ConfigDeprecation = (settings, _fromPath, log) => {
-  const username: string | undefined = get(settings, 'elasticsearch.username');
-  if (username === 'elastic') {
-    log(
-      `Setting elasticsearch.username to "elastic" is deprecated. You should use the "kibana" user instead.`
-    );
-  }
-  return settings;
-};
-
 export const coreDeprecationProvider: ConfigDeprecationProvider = ({
   unusedFromRoot,
   renameFromRoot,
@@ -120,5 +110,4 @@ export const coreDeprecationProvider: ConfigDeprecationProvider = ({
   dataPathDeprecation,
   rewriteBasePathDeprecation,
   cspRulesDeprecation,
-  elasticsearchUsernameDeprecation,
 ];

src/core/server/elasticsearch/elasticsearch_config.test.ts

@@ -23,19 +23,32 @@ import {
   mockReadPkcs12Truststore,
 } from './elasticsearch_config.test.mocks';
 
-import { ElasticsearchConfig, config, ElasticsearchConfigType } from './elasticsearch_config';
-import { loggingServiceMock } from '../mocks';
-import { Logger } from '../logging';
-
-const createElasticsearchConfig = (rawConfig: ElasticsearchConfigType, log?: Logger) => {
-  if (!log) {
-    log = loggingServiceMock.create().get('config');
-  }
-  return new ElasticsearchConfig(rawConfig, log);
+import { ElasticsearchConfig, config } from './elasticsearch_config';
+import { applyDeprecations, configDeprecationFactory } from '../config/deprecation';
+
+const CONFIG_PATH = 'elasticsearch';
+
+const applyElasticsearchDeprecations = (settings: Record<string, any> = {}) => {
+  const deprecations = config.deprecations!(configDeprecationFactory);
+  const deprecationMessages: string[] = [];
+  const _config: any = {};
+  _config[CONFIG_PATH] = settings;
+  const migrated = applyDeprecations(
+    _config,
+    deprecations.map(deprecation => ({
+      deprecation,
+      path: CONFIG_PATH,
+    })),
+    msg => deprecationMessages.push(msg)
+  );
+  return {
+    messages: deprecationMessages,
+    migrated,
+  };
 };
 
 test('set correct defaults', () => {
-  const configValue = createElasticsearchConfig(config.schema.validate({}));
+  const configValue = new ElasticsearchConfig(config.schema.validate({}));
   expect(configValue).toMatchInlineSnapshot(`
     ElasticsearchConfig {
       "apiVersion": "master",
@@ -70,17 +83,17 @@ test('set correct defaults', () => {
 });
 
 test('#hosts accepts both string and array of strings', () => {
-  let configValue = createElasticsearchConfig(
+  let configValue = new ElasticsearchConfig(
     config.schema.validate({ hosts: 'http://some.host:1234' })
   );
   expect(configValue.hosts).toEqual(['http://some.host:1234']);
 
-  configValue = createElasticsearchConfig(
+  configValue = new ElasticsearchConfig(
     config.schema.validate({ hosts: ['http://some.host:1234'] })
   );
   expect(configValue.hosts).toEqual(['http://some.host:1234']);
 
-  configValue = createElasticsearchConfig(
+  configValue = new ElasticsearchConfig(
     config.schema.validate({
       hosts: ['http://some.host:1234', 'https://some.another.host'],
     })
@@ -89,17 +102,17 @@ test('#hosts accepts both string and array of strings', () => {
 });
 
 test('#requestHeadersWhitelist accepts both string and array of strings', () => {
-  let configValue = createElasticsearchConfig(
+  let configValue = new ElasticsearchConfig(
     config.schema.validate({ requestHeadersWhitelist: 'token' })
   );
   expect(configValue.requestHeadersWhitelist).toEqual(['token']);
 
-  configValue = createElasticsearchConfig(
+  configValue = new ElasticsearchConfig(
     config.schema.validate({ requestHeadersWhitelist: ['token'] })
   );
   expect(configValue.requestHeadersWhitelist).toEqual(['token']);
 
-  configValue = createElasticsearchConfig(
+  configValue = new ElasticsearchConfig(
     config.schema.validate({
       requestHeadersWhitelist: ['token', 'X-Forwarded-Proto'],
     })
@@ -122,37 +135,37 @@ describe('reads files', () => {
   });
 
   it('reads certificate authorities when ssl.keystore.path is specified', () => {
-    const configValue = createElasticsearchConfig(
+    const configValue = new ElasticsearchConfig(
       config.schema.validate({ ssl: { keystore: { path: 'some-path' } } })
     );
     expect(mockReadPkcs12Keystore).toHaveBeenCalledTimes(1);
     expect(configValue.ssl.certificateAuthorities).toEqual(['content-of-some-path.ca']);
   });
 
   it('reads certificate authorities when ssl.truststore.path is specified', () => {
-    const configValue = createElasticsearchConfig(
+    const configValue = new ElasticsearchConfig(
       config.schema.validate({ ssl: { truststore: { path: 'some-path' } } })
     );
     expect(mockReadPkcs12Truststore).toHaveBeenCalledTimes(1);
     expect(configValue.ssl.certificateAuthorities).toEqual(['content-of-some-path']);
   });
 
   it('reads certificate authorities when ssl.certificateAuthorities is specified', () => {
-    let configValue = createElasticsearchConfig(
+    let configValue = new ElasticsearchConfig(
       config.schema.validate({ ssl: { certificateAuthorities: 'some-path' } })
     );
     expect(mockReadFileSync).toHaveBeenCalledTimes(1);
     expect(configValue.ssl.certificateAuthorities).toEqual(['content-of-some-path']);
 
     mockReadFileSync.mockClear();
-    configValue = createElasticsearchConfig(
+    configValue = new ElasticsearchConfig(
       config.schema.validate({ ssl: { certificateAuthorities: ['some-path'] } })
     );
     expect(mockReadFileSync).toHaveBeenCalledTimes(1);
     expect(configValue.ssl.certificateAuthorities).toEqual(['content-of-some-path']);
 
     mockReadFileSync.mockClear();
-    configValue = createElasticsearchConfig(
+    configValue = new ElasticsearchConfig(
       config.schema.validate({
         ssl: { certificateAuthorities: ['some-path', 'another-path'] },
       })
@@ -165,7 +178,7 @@ describe('reads files', () => {
   });
 
   it('reads certificate authorities when ssl.keystore.path, ssl.truststore.path, and ssl.certificateAuthorities are specified', () => {
-    const configValue = createElasticsearchConfig(
+    const configValue = new ElasticsearchConfig(
       config.schema.validate({
         ssl: {
           keystore: { path: 'some-path' },
@@ -185,7 +198,7 @@ describe('reads files', () => {
   });
 
   it('reads a private key and certificate when ssl.keystore.path is specified', () => {
-    const configValue = createElasticsearchConfig(
+    const configValue = new ElasticsearchConfig(
       config.schema.validate({ ssl: { keystore: { path: 'some-path' } } })
     );
     expect(mockReadPkcs12Keystore).toHaveBeenCalledTimes(1);
@@ -194,15 +207,15 @@ describe('reads files', () => {
   });
 
   it('reads a private key when ssl.key is specified', () => {
-    const configValue = createElasticsearchConfig(
+    const configValue = new ElasticsearchConfig(
       config.schema.validate({ ssl: { key: 'some-path' } })
     );
     expect(mockReadFileSync).toHaveBeenCalledTimes(1);
     expect(configValue.ssl.key).toEqual('content-of-some-path');
   });
 
   it('reads a certificate when ssl.certificate is specified', () => {
-    const configValue = createElasticsearchConfig(
+    const configValue = new ElasticsearchConfig(
       config.schema.validate({ ssl: { certificate: 'some-path' } })
     );
     expect(mockReadFileSync).toHaveBeenCalledTimes(1);
@@ -225,52 +238,58 @@ describe('throws when config is invalid', () => {
 
   it('throws if key is invalid', () => {
     const value = { ssl: { key: '/invalid/key' } };
-    expect(() =>
-      createElasticsearchConfig(config.schema.validate(value))
+    expect(
+      () => new ElasticsearchConfig(config.schema.validate(value))
     ).toThrowErrorMatchingInlineSnapshot(
       `"ENOENT: no such file or directory, open '/invalid/key'"`
     );
   });
 
   it('throws if certificate is invalid', () => {
     const value = { ssl: { certificate: '/invalid/cert' } };
-    expect(() =>
-      createElasticsearchConfig(config.schema.validate(value))
+    expect(
+      () => new ElasticsearchConfig(config.schema.validate(value))
     ).toThrowErrorMatchingInlineSnapshot(
       `"ENOENT: no such file or directory, open '/invalid/cert'"`
     );
   });
 
   it('throws if certificateAuthorities is invalid', () => {
     const value = { ssl: { certificateAuthorities: '/invalid/ca' } };
-    expect(() =>
-      createElasticsearchConfig(config.schema.validate(value))
+    expect(
+      () => new ElasticsearchConfig(config.schema.validate(value))
     ).toThrowErrorMatchingInlineSnapshot(`"ENOENT: no such file or directory, open '/invalid/ca'"`);
   });
 
   it('throws if keystore path is invalid', () => {
     const value = { ssl: { keystore: { path: '/invalid/keystore' } } };
-    expect(() =>
-      createElasticsearchConfig(config.schema.validate(value))
+    expect(
+      () => new ElasticsearchConfig(config.schema.validate(value))
     ).toThrowErrorMatchingInlineSnapshot(
       `"ENOENT: no such file or directory, open '/invalid/keystore'"`
     );
   });
 
-  it('throws if keystore does not contain a key or certificate', () => {
+  it('throws if keystore does not contain a key', () => {
     mockReadPkcs12Keystore.mockReturnValueOnce({});
     const value = { ssl: { keystore: { path: 'some-path' } } };
-    expect(() =>
-      createElasticsearchConfig(config.schema.validate(value))
-    ).toThrowErrorMatchingInlineSnapshot(
-      `"Did not find key or certificate in Elasticsearch keystore."`
-    );
+    expect(
+      () => new ElasticsearchConfig(config.schema.validate(value))
+    ).toThrowErrorMatchingInlineSnapshot(`"Did not find key in Elasticsearch keystore."`);
+  });
+
+  it('throws if keystore does not contain a certificate', () => {
+    mockReadPkcs12Keystore.mockReturnValueOnce({ key: 'foo' });
+    const value = { ssl: { keystore: { path: 'some-path' } } };
+    expect(
+      () => new ElasticsearchConfig(config.schema.validate(value))
+    ).toThrowErrorMatchingInlineSnapshot(`"Did not find certificate in Elasticsearch keystore."`);
   });
 
   it('throws if truststore path is invalid', () => {
     const value = { ssl: { keystore: { path: '/invalid/truststore' } } };
-    expect(() =>
-      createElasticsearchConfig(config.schema.validate(value))
+    expect(
+      () => new ElasticsearchConfig(config.schema.validate(value))
     ).toThrowErrorMatchingInlineSnapshot(
       `"ENOENT: no such file or directory, open '/invalid/truststore'"`
     );
@@ -291,31 +310,47 @@ describe('throws when config is invalid', () => {
   });
 });
 
-describe('logs warnings', () => {
-  let logger: ReturnType<typeof loggingServiceMock.create>;
-  let log: Logger;
+describe('deprecations', () => {
+  it('logs a warning if elasticsearch.username is set to "elastic"', () => {
+    const { messages } = applyElasticsearchDeprecations({ username: 'elastic' });
+    expect(messages).toMatchInlineSnapshot(`
+      Array [
+        "Setting [${CONFIG_PATH}.username] to \\"elastic\\" is deprecated. You should use the \\"kibana\\" user instead.",
+      ]
+    `);
+  });
 
-  beforeAll(() => {
-    mockReadFileSync.mockResolvedValue('foo');
+  it('does not log a warning if elasticsearch.username is set to something besides "elastic"', () => {
+    const { messages } = applyElasticsearchDeprecations({ username: 'otheruser' });
+    expect(messages).toHaveLength(0);
   });
 
-  beforeEach(() => {
-    logger = loggingServiceMock.create();
-    log = logger.get('config');
+  it('does not log a warning if elasticsearch.username is unset', () => {
+    const { messages } = applyElasticsearchDeprecations({});
+    expect(messages).toHaveLength(0);
   });
 
-  it('warns if ssl.key is set and ssl.certificate is not', () => {
-    createElasticsearchConfig(config.schema.validate({ ssl: { key: 'some-path' } }), log);
-    expect(loggingServiceMock.collect(logger).warn[0][0]).toMatchInlineSnapshot(
-      `"Detected a key without a certificate; mutual TLS authentication is disabled."`
-    );
+  it('logs a warning if ssl.key is set and ssl.certificate is not', () => {
+    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.",
+      ]
+    `);
   });
 
-  it('warns if ssl.certificate is set and ssl.key is not', () => {
-    createElasticsearchConfig(config.schema.validate({ ssl: { certificate: 'some-path' } }), log);
-    expect(loggingServiceMock.collect(logger).warn[0][0]).toMatchInlineSnapshot(
-      `"Detected a certificate without a key; mutual TLS authentication is disabled."`
-    );
+  it('logs a warning if ssl.certificate is set and ssl.key is not', () => {
+    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([]);
   });
 });