Skip to content

Commit

Permalink
ConfigService wrongly marks unknown config paths as handled (#36559)
Browse files Browse the repository at this point in the history
* match on path segments not string inclusion

* make function a part of config domain

* improve error message
  • Loading branch information
mshustov committed May 21, 2019
1 parent e2e5fed commit 8c0f8c1
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 3 deletions.
36 changes: 36 additions & 0 deletions src/core/server/config/config.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import { hasConfigPathIntersection } from './config';

describe('hasConfigPathIntersection()', () => {
test('Should return true if leaf is descendent to the root', () => {
expect(hasConfigPathIntersection('a.b', 'a.b')).toBe(true);
expect(hasConfigPathIntersection('a.b.c', 'a')).toBe(true);
expect(hasConfigPathIntersection('a.b.c.d', 'a.b')).toBe(true);
});
test('Should return false if leaf is not descendent to the root', () => {
expect(hasConfigPathIntersection('a.bc', 'a.b')).toBe(false);
expect(hasConfigPathIntersection('a', 'a.b')).toBe(false);
});
test('Should throw if either path is empty', () => {
expect(() => hasConfigPathIntersection('a', '')).toThrow();
expect(() => hasConfigPathIntersection('', 'a')).toThrow();
expect(() => hasConfigPathIntersection('', '')).toThrow();
});
});
13 changes: 13 additions & 0 deletions src/core/server/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,16 @@ export interface Config {
*/
toRaw(): Record<string, any>;
}

const pathDelimiter = '.';
export function hasConfigPathIntersection(leafPath: string, rootPath: string) {
if (!leafPath) {
throw new Error('leafPath cannot be empty');
}
if (!rootPath) {
throw new Error('rootPath cannot be empty');
}
const leafSegments = leafPath.split(pathDelimiter);
const rootSegments = rootPath.split(pathDelimiter);
return rootSegments.every((rootSegment, index) => leafSegments[index] === rootSegment);
}
3 changes: 2 additions & 1 deletion src/core/server/config/config_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { distinctUntilChanged, first, map } from 'rxjs/operators';

import { Config, ConfigPath, Env } from '.';
import { Logger, LoggerFactory } from '../logging';
import { hasConfigPathIntersection } from './config';

/** @internal */
export class ConfigService {
Expand Down Expand Up @@ -180,4 +181,4 @@ const pathToString = (path: ConfigPath) => (Array.isArray(path) ? path.join('.')
* handled paths.
*/
const isPathHandled = (path: string, handledPaths: string[]) =>
handledPaths.some(handledPath => path.startsWith(handledPath));
handledPaths.some(handledPath => hasConfigPathIntersection(path, handledPath));
2 changes: 1 addition & 1 deletion src/core/server/config/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

export { ConfigService } from './config_service';
export { RawConfigService } from './raw_config_service';
export { Config, ConfigPath, isConfigPath } from './config';
export { Config, ConfigPath, isConfigPath, hasConfigPathIntersection } from './config';
export { ObjectToConfigAdapter } from './object_to_config_adapter';
export { CliArgs } from './env';

Expand Down
4 changes: 3 additions & 1 deletion src/legacy/server/config/complete.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import { difference, get, set } from 'lodash';
import { transformDeprecations } from './transform_deprecations';
import { unset, formatListAsProse, getFlattenedObject } from '../../utils';
import { getTransform } from '../../deprecation';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { hasConfigPathIntersection } from '../../../core/server/config/';

const getFlattenedKeys = object => Object.keys(getFlattenedObject(object));

Expand Down Expand Up @@ -68,7 +70,7 @@ async function getUnusedConfigKeys(
return difference(inputKeys, appliedKeys).filter(
unusedConfigKey =>
!coreHandledConfigPaths.some(usedInCoreConfigKey =>
unusedConfigKey.startsWith(usedInCoreConfigKey)
hasConfigPathIntersection(unusedConfigKey, usedInCoreConfigKey)
)
);
}
Expand Down

0 comments on commit 8c0f8c1

Please sign in to comment.