Skip to content
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

Deprecates non-inclusive config names #1467

Merged
merged 1 commit into from
Apr 15, 2022
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
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,13 @@ import { ConfigDeprecationLogger } from './types';
import { configDeprecationFactory } from './deprecation_factory';

describe('DeprecationFactory', () => {
const { rename, unused, renameFromRoot, unusedFromRoot } = configDeprecationFactory;
const {
rename,
unused,
renameFromRoot,
unusedFromRoot,
renameFromRootWithoutMap,
} = configDeprecationFactory;

let deprecationMessages: string[];
const logger: ConfigDeprecationLogger = (msg) => deprecationMessages.push(msg);
Expand Down Expand Up @@ -261,6 +267,62 @@ describe('DeprecationFactory', () => {
});
});

describe('renamefromRootWithoutMap', () => {
it('allows use of new property name but gives deprecation warning if old name is used', () => {
const rawConfig = {
myplugin: {
deprecated: 'toberenamed',
valid: 'valid',
},
someOtherPlugin: {
property: 'value',
},
};
const processed = renameFromRootWithoutMap('deprecated', 'renamed')(
rawConfig,
'myplugin',
logger
);
expect(processed).toEqual({
myplugin: {
deprecated: 'toberenamed',
valid: 'valid',
},
someOtherPlugin: {
property: 'value',
},
});
expect(deprecationMessages).toMatchInlineSnapshot(`Array []`);
});

it('deprecates new config and does not log if old property is not present', () => {
const rawConfig = {
myplugin: {
new: 'toBeDeprecated',
valid: 'valid',
},
someOtherPlugin: {
property: 'value',
},
};
const processed = renameFromRootWithoutMap('myplugin.deprecated', 'myplugin.new')(
rawConfig,
'does-not-matter',
logger
);
expect(processed).toEqual({
myplugin: {
deprecated: 'toBeDeprecated',
valid: 'valid',
},
someOtherPlugin: {
property: 'value',
},
});
expect(deprecationMessages.length).toEqual(0);
});
});

describe('unused', () => {
it('removes the unused property from the config and logs a warning is present', () => {
const rawConfig = {
Expand Down
46 changes: 46 additions & 0 deletions packages/osd-config/src/deprecation/deprecation_factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,37 @@ const _rename = (
return config;
};

const _renameWithoutMap = (
config: Record<string, any>,
rootPath: string,
log: ConfigDeprecationLogger,
oldKey: string,
newKey: string,
silent?: boolean
) => {
const fullOldPath = getPath(rootPath, oldKey);
const oldValue = get(config, fullOldPath);

const fullNewPath = getPath(rootPath, newKey);
const newValue = get(config, fullNewPath);

if (oldValue !== undefined) {
if (!silent) {
log(`"${fullOldPath}" is deprecated and has been replaced by "${fullNewPath}"`);
}

return config;
}

if (newValue === undefined) {
return config;
}

unset(config, fullNewPath);
set(config, fullOldPath, newValue);
return config;
};

const _unused = (
config: Record<string, any>,
rootPath: string,
Expand All @@ -82,6 +113,19 @@ const _unused = (
return config;
};

const renameWithoutMap = (oldKey: string, newKey: string): ConfigDeprecation => (
config,
rootPath,
log
) => _renameWithoutMap(config, rootPath, log, oldKey, newKey);

const renameFromRootWithoutMap = (
oldKey: string,
newKey: string,
silent?: boolean
): ConfigDeprecation => (config, rootPath, log) =>
_renameWithoutMap(config, '', log, oldKey, newKey, silent);

const rename = (oldKey: string, newKey: string): ConfigDeprecation => (config, rootPath, log) =>
_rename(config, rootPath, log, oldKey, newKey);

Expand All @@ -108,6 +152,8 @@ const getPath = (rootPath: string, subPath: string) =>
export const configDeprecationFactory: ConfigDeprecationFactory = {
rename,
renameFromRoot,
renameWithoutMap,
renameFromRootWithoutMap,
unused,
unusedFromRoot,
};
29 changes: 29 additions & 0 deletions packages/osd-config/src/deprecation/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,35 @@ export interface ConfigDeprecationFactory {
* ```
*/
renameFromRoot(oldKey: string, newKey: string, silent?: boolean): ConfigDeprecation;
/**
* Rename a configuration property from inside a plugin's configuration path.
* Will log a deprecation warning if the oldKey was found and deprecation applied.
*
* @example
* Rename 'myplugin.oldKey' to 'myplugin.newKey'
* ```typescript
* const provider: ConfigDeprecationProvider = ({ rename }) => [
* rename('oldKey', 'newKey'),
* ]
* ```
*/
renameWithoutMap(oldKey: string, newKey: string): ConfigDeprecation;
/**
* Rename a configuration property from the root configuration.
* Will log a deprecation warning if the oldKey was found and deprecation applied.
*
* This should be only used when renaming properties from different configuration's path.
* To rename properties from inside a plugin's configuration, use 'rename' instead.
*
* @example
* Rename 'oldplugin.key' to 'newplugin.key'
* ```typescript
* const provider: ConfigDeprecationProvider = ({ renameFromRoot }) => [
* renameFromRoot('oldplugin.key', 'newplugin.key'),
* ]
* ```
*/
renameFromRootWithoutMap(oldKey: string, newKey: string, silent?: boolean): ConfigDeprecation;
/**
* Remove a configuration property from inside a plugin's configuration path.
* Will log a deprecation warning if the unused key was found and deprecation applied.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ describe('core deprecations', () => {
});
expect(messages).toMatchInlineSnapshot(`
Array [
"\\"server.xsrf.whitelist\\" is deprecated and has been replaced by \\"server.xsrf.allowlist\\"",
"It is not recommended to disable xsrf protections for API endpoints via [server.xsrf.whitelist]. Instead, supply the \\"osd-xsrf\\" header.",
]
`);
Expand Down
6 changes: 6 additions & 0 deletions src/core/server/config/deprecation/core_deprecations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ const mapManifestServiceUrlDeprecation: ConfigDeprecation = (settings, fromPath,
export const coreDeprecationProvider: ConfigDeprecationProvider = ({
unusedFromRoot,
renameFromRoot,
renameFromRootWithoutMap,
}) => [
unusedFromRoot('savedObjects.indexCheckTimeout'),
unusedFromRoot('server.xsrf.token'),
Expand Down Expand Up @@ -154,6 +155,11 @@ export const coreDeprecationProvider: ConfigDeprecationProvider = ({
renameFromRoot('cpuacct.cgroup.path.override', 'ops.cGroupOverrides.cpuAcctPath'),
unusedFromRoot('opensearch.preserveHost'),
unusedFromRoot('opensearch.startupTimeout'),
renameFromRootWithoutMap('server.xsrf.whitelist', 'server.xsrf.allowlist'),
renameFromRootWithoutMap(
'server.compression.referrerWhitelist',
'server.compression.referrerAllowlist'
),
configPathDeprecation,
dataPathDeprecation,
rewriteBasePathDeprecation,
Expand Down
1 change: 1 addition & 0 deletions src/core/server/opensearch/opensearch_config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,7 @@ describe('deprecations', () => {
expect(messages).toMatchInlineSnapshot(`
Array [
"\\"elasticsearch.requestHeadersWhitelist\\" is deprecated and has been replaced by \\"opensearch.requestHeadersWhitelist\\"",
"\\"opensearch.requestHeadersWhitelist\\" is deprecated and has been replaced by \\"opensearch.requestHeadersAllowlist\\"",
]
`);
});
Expand Down
6 changes: 5 additions & 1 deletion src/core/server/opensearch/opensearch_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,14 +137,18 @@ export const configSchema = schema.object({
),
});

const deprecations: ConfigDeprecationProvider = ({ renameFromRoot }) => [
const deprecations: ConfigDeprecationProvider = ({ renameFromRoot, renameFromRootWithoutMap }) => [
renameFromRoot('elasticsearch.sniffOnStart', 'opensearch.sniffOnStart'),
renameFromRoot('elasticsearch.sniffInterval', 'opensearch.sniffInterval'),
renameFromRoot('elasticsearch.sniffOnConnectionFault', 'opensearch.sniffOnConnectionFault'),
renameFromRoot('elasticsearch.hosts', 'opensearch.hosts'),
renameFromRoot('elasticsearch.username', 'opensearch.username'),
renameFromRoot('elasticsearch.password', 'opensearch.password'),
renameFromRoot('elasticsearch.requestHeadersWhitelist', 'opensearch.requestHeadersWhitelist'),
renameFromRootWithoutMap(
'opensearch.requestHeadersWhitelist',
'opensearch.requestHeadersAllowlist'
),
renameFromRoot('elasticsearch.customHeaders', 'opensearch.customHeaders'),
renameFromRoot('elasticsearch.shardTimeout', 'opensearch.shardTimeout'),
renameFromRoot('elasticsearch.requestTimeout', 'opensearch.requestTimeout'),
Expand Down