diff --git a/src/cli_plugin/install/kibana.js b/src/cli_plugin/install/kibana.js index 675cc180a9d94..7cc18f55ab73b 100644 --- a/src/cli_plugin/install/kibana.js +++ b/src/cli_plugin/install/kibana.js @@ -42,8 +42,7 @@ export async function rebuildCache(settings, logger) { '--env.name=production', '--optimize.useBundleCache=false', '--server.autoListen=false', - '--plugins.initialize=false', - '--uiSettings.enabled=false' + '--plugins.initialize=false' ]; const proc = execa(process.execPath, kibanaArgs, { diff --git a/src/core_plugins/kibana/public/management/sections/settings/__snapshots__/advanced_settings.test.js.snap b/src/core_plugins/kibana/public/management/sections/settings/__snapshots__/advanced_settings.test.js.snap index bfde665a6e5dc..c72d3e2f64c58 100644 --- a/src/core_plugins/kibana/public/management/sections/settings/__snapshots__/advanced_settings.test.js.snap +++ b/src/core_plugins/kibana/public/management/sections/settings/__snapshots__/advanced_settings.test.js.snap @@ -76,7 +76,7 @@ exports[`AdvancedSettings should render normally 1`] = ` categoryCounts={ Object { "elasticsearch": 2, - "general": 7, + "general": 11, } } clear={[Function]} @@ -96,6 +96,7 @@ exports[`AdvancedSettings should render normally 1`] = ` "description": "Description for Test array setting", "displayName": "Test array setting", "isCustom": undefined, + "isOverridden": false, "name": "test:array:setting", "options": undefined, "readonly": false, @@ -111,6 +112,7 @@ exports[`AdvancedSettings should render normally 1`] = ` "description": "Description for Test boolean setting", "displayName": "Test boolean setting", "isCustom": undefined, + "isOverridden": false, "name": "test:boolean:setting", "options": undefined, "readonly": false, @@ -128,6 +130,7 @@ exports[`AdvancedSettings should render normally 1`] = ` "description": "Description for Test custom string setting", "displayName": "Test custom string setting", "isCustom": undefined, + "isOverridden": false, "name": "test:customstring:setting", "options": undefined, "readonly": false, @@ -143,12 +146,83 @@ exports[`AdvancedSettings should render normally 1`] = ` "description": "Description for Test image setting", "displayName": "Test image setting", "isCustom": undefined, + "isOverridden": false, "name": "test:image:setting", "options": undefined, "readonly": false, "type": "image", "value": undefined, }, + Object { + "ariaName": "test is overridden json", + "category": Array [ + "general", + ], + "defVal": "{ + \\"foo\\": \\"bar\\" +}", + "description": "Description for overridden json", + "displayName": "An overridden json", + "isCustom": undefined, + "isOverridden": true, + "name": "test:isOverridden:json", + "options": undefined, + "readonly": false, + "type": "json", + "value": undefined, + }, + Object { + "ariaName": "test is overridden number", + "category": Array [ + "general", + ], + "defVal": 1234, + "description": "Description for overridden number", + "displayName": "An overridden number", + "isCustom": undefined, + "isOverridden": true, + "name": "test:isOverridden:number", + "options": undefined, + "readonly": false, + "type": "number", + "value": undefined, + }, + Object { + "ariaName": "test is overridden select", + "category": Array [ + "general", + ], + "defVal": "orange", + "description": "Description for overridden select setting", + "displayName": "Test overridden select setting", + "isCustom": undefined, + "isOverridden": true, + "name": "test:isOverridden:select", + "options": Array [ + "apple", + "orange", + "banana", + ], + "readonly": false, + "type": "select", + "value": undefined, + }, + Object { + "ariaName": "test is overridden string", + "category": Array [ + "general", + ], + "defVal": "foo", + "description": "Description for overridden string", + "displayName": "An overridden string", + "isCustom": undefined, + "isOverridden": true, + "name": "test:isOverridden:string", + "options": undefined, + "readonly": false, + "type": "string", + "value": undefined, + }, Object { "ariaName": "test json setting", "category": Array [ @@ -158,6 +232,7 @@ exports[`AdvancedSettings should render normally 1`] = ` "description": "Description for Test json setting", "displayName": "Test json setting", "isCustom": undefined, + "isOverridden": false, "name": "test:json:setting", "options": undefined, "readonly": false, @@ -173,6 +248,7 @@ exports[`AdvancedSettings should render normally 1`] = ` "description": "Description for Test markdown setting", "displayName": "Test markdown setting", "isCustom": undefined, + "isOverridden": false, "name": "test:markdown:setting", "options": undefined, "readonly": false, @@ -188,6 +264,7 @@ exports[`AdvancedSettings should render normally 1`] = ` "description": "Description for Test number setting", "displayName": "Test number setting", "isCustom": undefined, + "isOverridden": false, "name": "test:number:setting", "options": undefined, "readonly": false, @@ -203,6 +280,7 @@ exports[`AdvancedSettings should render normally 1`] = ` "description": "Description for Test select setting", "displayName": "Test select setting", "isCustom": undefined, + "isOverridden": false, "name": "test:select:setting", "options": Array [ "apple", @@ -222,6 +300,7 @@ exports[`AdvancedSettings should render normally 1`] = ` "description": "Description for Test string setting", "displayName": "Test string setting", "isCustom": undefined, + "isOverridden": false, "name": "test:string:setting", "options": undefined, "readonly": false, @@ -329,7 +408,7 @@ exports[`AdvancedSettings should render specific setting if given setting key 1` categoryCounts={ Object { "elasticsearch": 2, - "general": 7, + "general": 11, } } clear={[Function]} @@ -347,6 +426,7 @@ exports[`AdvancedSettings should render specific setting if given setting key 1` "description": "Description for Test string setting", "displayName": "Test string setting", "isCustom": undefined, + "isOverridden": false, "name": "test:string:setting", "options": undefined, "readonly": false, diff --git a/src/core_plugins/kibana/public/management/sections/settings/advanced_settings.js b/src/core_plugins/kibana/public/management/sections/settings/advanced_settings.js index 7899bbd2b0681..9f607a0e0577f 100644 --- a/src/core_plugins/kibana/public/management/sections/settings/advanced_settings.js +++ b/src/core_plugins/kibana/public/management/sections/settings/advanced_settings.js @@ -90,6 +90,7 @@ export class AdvancedSettings extends Component { name: setting[0], value: setting[1].userValue, isCustom: config.isCustom(setting[0]), + isOverridden: config.isOverridden(setting[0]), }); }) .filter((c) => !c.readonly) diff --git a/src/core_plugins/kibana/public/management/sections/settings/advanced_settings.test.js b/src/core_plugins/kibana/public/management/sections/settings/advanced_settings.test.js index 113f23b6b6f4a..23b873d2e08de 100644 --- a/src/core_plugins/kibana/public/management/sections/settings/advanced_settings.test.js +++ b/src/core_plugins/kibana/public/management/sections/settings/advanced_settings.test.js @@ -19,6 +19,7 @@ import React from 'react'; import { shallow } from 'enzyme'; +import dedent from 'dedent'; import { AdvancedSettings } from './advanced_settings'; @@ -44,6 +45,7 @@ const config = { set: () => {}, remove: () => {}, isCustom: (setting) => setting.isCustom, + isOverridden: (key) => Boolean(config.getAll()[key].isOverridden), getAll: () => { return { 'test:array:setting': { @@ -109,6 +111,39 @@ const config = { type: 'string', isCustom: true, }, + 'test:isOverridden:string': { + isOverridden: true, + value: 'foo', + name: 'An overridden string', + description: 'Description for overridden string', + type: 'string', + }, + 'test:isOverridden:number': { + isOverridden: true, + value: 1234, + name: 'An overridden number', + description: 'Description for overridden number', + type: 'number', + }, + 'test:isOverridden:json': { + isOverridden: true, + value: dedent` + { + "foo": "bar" + } + `, + name: 'An overridden json', + description: 'Description for overridden json', + type: 'json', + }, + 'test:isOverridden:select': { + isOverridden: true, + value: 'orange', + name: 'Test overridden select setting', + description: 'Description for overridden select setting', + type: 'select', + options: ['apple', 'orange', 'banana'], + }, }; } }; diff --git a/src/core_plugins/kibana/public/management/sections/settings/components/field/__snapshots__/field.test.js.snap b/src/core_plugins/kibana/public/management/sections/settings/components/field/__snapshots__/field.test.js.snap index 7f3a8dd7f8242..881e228173b31 100644 --- a/src/core_plugins/kibana/public/management/sections/settings/components/field/__snapshots__/field.test.js.snap +++ b/src/core_plugins/kibana/public/management/sections/settings/components/field/__snapshots__/field.test.js.snap @@ -1,5 +1,106 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`Field for array setting should render as read only with help text if overridden 1`] = ` + + + +
+ + + + + Default: + + default_value + + + + + + } + fullWidth={false} + gutterSize="l" + idAria="array:test:setting-aria" + title={ +

+ Array test setting + +

+ } + titleSize="xs" + > + + This setting is overriden by the Kibana server and can not be changed. + + } + isInvalid={false} + label={ + + array:test:setting + + } + > + + + + + + +`; + exports[`Field for array setting should render custom setting icon if it is custom 1`] = ` `; +exports[`Field for boolean setting should render as read only with help text if overridden 1`] = ` + + + +
+ + + + + Default: + + true + + + + + + } + fullWidth={false} + gutterSize="l" + idAria="boolean:test:setting-aria" + title={ +

+ Boolean test setting + +

+ } + titleSize="xs" + > + + This setting is overriden by the Kibana server and can not be changed. + + } + isInvalid={false} + label={ + + boolean:test:setting + + } + > + + + + + + +`; + exports[`Field for boolean setting should render custom setting icon if it is custom 1`] = ` `; -exports[`Field for image setting should render custom setting icon if it is custom 1`] = ` +exports[`Field for image setting should render as read only with help text if overridden 1`] = ` + + + + + Default: + + null + + + + } fullWidth={false} @@ -568,12 +784,7 @@ exports[`Field for image setting should render custom setting icon if it is cust title={

Image test setting - +

} titleSize="xs" @@ -587,7 +798,14 @@ exports[`Field for image setting should render custom setting icon if it is cust error={null} fullWidth={false} hasEmptyLabelSpace={false} - helpText={null} + helpText={ + + This setting is overriden by the Kibana server and can not be changed. + + } isInvalid={false} label={ } > - @@ -616,7 +832,7 @@ exports[`Field for image setting should render custom setting icon if it is cust `; -exports[`Field for image setting should render default value if there is no user value set 1`] = ` +exports[`Field for image setting should render custom setting icon if it is custom 1`] = ` Image test setting - + } titleSize="xs" @@ -693,7 +914,7 @@ exports[`Field for image setting should render default value if there is no user `; -exports[`Field for image setting should render user value if there is user value is set 1`] = ` +exports[`Field for image setting should render default value if there is no user value set 1`] = ` - - - - - Default: - - null - - - - } fullWidth={false} @@ -757,7 +962,100 @@ exports[`Field for image setting should render user value if there is user value error={null} fullWidth={false} hasEmptyLabelSpace={false} - helpText={ + helpText={null} + isInvalid={false} + label={ + + image:test:setting + + } + > + + + + + + +`; + +exports[`Field for image setting should render user value if there is user value is set 1`] = ` + + + +
+ + + + + Default: + + null + + + + + + } + fullWidth={false} + gutterSize="l" + idAria="image:test:setting-aria" + title={ +

+ Image test setting + +

+ } + titleSize="xs" + > + `; +exports[`Field for json setting should render as read only with help text if overridden 1`] = ` + + + +
+ + + + + Default: + + {} + + + + + + } + fullWidth={false} + gutterSize="l" + idAria="json:test:setting-aria" + title={ +

+ Json test setting + +

+ } + titleSize="xs" + > + + This setting is overriden by the Kibana server and can not be changed. + + } + isInvalid={false} + label={ + + json:test:setting + + } + > +
+ +
+ + + + + +`; + exports[`Field for json setting should render custom setting icon if it is custom 1`] = ` `; -exports[`Field for markdown setting should render custom setting icon if it is custom 1`] = ` +exports[`Field for markdown setting should render as read only with help text if overridden 1`] = ` + + + + + Default: + + null + + + + } fullWidth={false} @@ -1198,12 +1636,7 @@ exports[`Field for markdown setting should render custom setting icon if it is c title={

Markdown test setting - +

} titleSize="xs" @@ -1217,7 +1650,14 @@ exports[`Field for markdown setting should render custom setting icon if it is c error={null} fullWidth={false} hasEmptyLabelSpace={false} - helpText={null} + helpText={ + + This setting is overriden by the Kibana server and can not be changed. + + } isInvalid={false} label={
@@ -1262,7 +1703,7 @@ exports[`Field for markdown setting should render custom setting icon if it is c `; -exports[`Field for markdown setting should render default value if there is no user value set 1`] = ` +exports[`Field for markdown setting should render custom setting icon if it is custom 1`] = ` Markdown test setting - + } titleSize="xs" @@ -1330,6 +1776,7 @@ exports[`Field for markdown setting should render default value if there is no u } } height="auto" + isReadOnly={false} maxLines={30} minLines={6} mode="markdown" @@ -1355,7 +1802,7 @@ exports[`Field for markdown setting should render default value if there is no u `; -exports[`Field for markdown setting should render user value if there is user value is set 1`] = ` +exports[`Field for markdown setting should render default value if there is no user value set 1`] = ` - - - + } + fullWidth={false} + gutterSize="l" + idAria="markdown:test:setting-aria" + title={ +

+ Markdown test setting + +

+ } + titleSize="xs" + > + + markdown:test:setting + + } + > +
+ +
+ + + + + +`; + +exports[`Field for markdown setting should render user value if there is user value is set 1`] = ` + + + +
+ + + Default: @@ -1454,6 +1995,7 @@ exports[`Field for markdown setting should render user value if there is user va } } height="auto" + isReadOnly={false} maxLines={30} minLines={6} mode="markdown" @@ -1479,6 +2021,107 @@ exports[`Field for markdown setting should render user value if there is user va `; +exports[`Field for number setting should render as read only with help text if overridden 1`] = ` + + + +
+ + + + + Default: + + 5 + + + + + + } + fullWidth={false} + gutterSize="l" + idAria="number:test:setting-aria" + title={ +

+ Number test setting + +

+ } + titleSize="xs" + > + + This setting is overriden by the Kibana server and can not be changed. + + } + isInvalid={false} + label={ + + number:test:setting + + } + > + + + + + + +`; + exports[`Field for number setting should render custom setting icon if it is custom 1`] = ` `; +exports[`Field for select setting should render as read only with help text if overridden 1`] = ` + + + +
+ + + + + Default: + + orange + + + + + + } + fullWidth={false} + gutterSize="l" + idAria="select:test:setting-aria" + title={ +

+ Select test setting + +

+ } + titleSize="xs" + > + + This setting is overriden by the Kibana server and can not be changed. + + } + isInvalid={false} + label={ + + select:test:setting + + } + > + + + + + + +`; + exports[`Field for select setting should render custom setting icon if it is custom 1`] = ` `; +exports[`Field for string setting should render as read only with help text if overridden 1`] = ` + + + +
+ + + + + Default: + + null + + + + + + } + fullWidth={false} + gutterSize="l" + idAria="string:test:setting-aria" + title={ +

+ String test setting + +

+ } + titleSize="xs" + > + + This setting is overriden by the Kibana server and can not be changed. + + } + isInvalid={false} + label={ + + string:test:setting + + } + > + + + + + + +`; + exports[`Field for string setting should render custom setting icon if it is custom 1`] = ` @@ -346,6 +346,7 @@ export class Field extends PureComponent { height="auto" minLines={6} maxLines={30} + isReadOnly={isOverridden} setOptions={{ showLineNumbers: false, tabSize: 2, @@ -369,7 +370,7 @@ export class Field extends PureComponent { } else { return ( { this.changeImageForm = input; }} @@ -387,7 +388,7 @@ export class Field extends PureComponent { })} onChange={this.onFieldChange} isLoading={loading} - disabled={loading} + disabled={loading || isOverridden} onKeyDown={this.onFieldKeyDown} data-test-subj={`advancedSetting-editField-${name}`} /> @@ -398,7 +399,7 @@ export class Field extends PureComponent { value={unsavedValue} onChange={this.onFieldChange} isLoading={loading} - disabled={loading} + disabled={loading || isOverridden} onKeyDown={this.onFieldKeyDown} data-test-subj={`advancedSetting-editField-${name}`} /> @@ -409,7 +410,7 @@ export class Field extends PureComponent { value={unsavedValue} onChange={this.onFieldChange} isLoading={loading} - disabled={loading} + disabled={loading || isOverridden} onKeyDown={this.onFieldKeyDown} data-test-subj={`advancedSetting-editField-${name}`} /> @@ -426,6 +427,14 @@ export class Field extends PureComponent { } renderHelpText(setting) { + if (setting.isOverridden) { + return ( + + This setting is overriden by the Kibana server and can not be changed. + + ); + } + const defaultLink = this.renderResetToDefaultLink(setting); const imageLink = this.renderChangeImageLink(setting); @@ -538,9 +547,12 @@ export class Field extends PureComponent { renderActions(setting) { const { ariaName, name } = setting; const { loading, isInvalid, changeImage, savedValue, unsavedValue } = this.state; - if(savedValue === unsavedValue && !changeImage) { + const isDisabled = loading || setting.isOverridden; + + if (savedValue === unsavedValue && !changeImage) { return; } + return ( @@ -549,7 +561,7 @@ export class Field extends PureComponent { fill aria-label={`Save ${ariaName}`} onClick={this.saveEdit} - disabled={loading || isInvalid} + disabled={isDisabled || isInvalid} data-test-subj={`advancedSetting-saveEditField-${name}`} > Save @@ -559,7 +571,7 @@ export class Field extends PureComponent { changeImage ? this.cancelChangeImage() : this.cancelEdit()} - disabled={loading} + disabled={isDisabled} data-test-subj={`advancedSetting-cancelEditField-${name}`} > Cancel diff --git a/src/core_plugins/kibana/public/management/sections/settings/components/field/field.test.js b/src/core_plugins/kibana/public/management/sections/settings/components/field/field.test.js index f49fc88b0e5cb..901d63af2f3e9 100644 --- a/src/core_plugins/kibana/public/management/sections/settings/components/field/field.test.js +++ b/src/core_plugins/kibana/public/management/sections/settings/components/field/field.test.js @@ -42,6 +42,7 @@ const settings = { value: undefined, defVal: ['default_value'], isCustom: false, + isOverridden: false, options: null, }, boolean: { @@ -53,6 +54,7 @@ const settings = { value: undefined, defVal: true, isCustom: false, + isOverridden: false, options: null, }, image: { @@ -64,6 +66,7 @@ const settings = { value: undefined, defVal: null, isCustom: false, + isOverridden: false, options: { maxSize: { length: 1000, @@ -81,6 +84,7 @@ const settings = { value: '{"foo": "bar"}', defVal: '{}', isCustom: false, + isOverridden: false, options: null, }, markdown: { @@ -92,6 +96,7 @@ const settings = { value: undefined, defVal: '', isCustom: false, + isOverridden: false, options: null, }, number: { @@ -103,6 +108,7 @@ const settings = { value: undefined, defVal: 5, isCustom: false, + isOverridden: false, options: null, }, select: { @@ -114,6 +120,7 @@ const settings = { value: undefined, defVal: 'orange', isCustom: false, + isOverridden: false, options: ['apple', 'orange', 'banana'], }, string: { @@ -125,6 +132,7 @@ const settings = { value: undefined, defVal: null, isCustom: false, + isOverridden: false, options: null, }, }; @@ -158,6 +166,22 @@ describe('Field', () => { expect(component).toMatchSnapshot(); }); + it('should render as read only with help text if overridden', async () => { + const component = shallow( + + ); + + expect(component).toMatchSnapshot(); + }); + it('should render user value if there is user value is set', async () => { const component = shallow( Joi.object({ }).default(), }).default(), + uiSettings: Joi.object().keys({ + overrides: Joi.object().unknown(true).default() + }).default(), + logging: Joi.object().keys({ silent: Joi.boolean().default(false), diff --git a/src/ui/public/config/config.js b/src/ui/public/config/config.js index ee46adf5de3c1..a4f668529d475 100644 --- a/src/ui/public/config/config.js +++ b/src/ui/public/config/config.js @@ -38,6 +38,7 @@ module.service(`config`, function ($rootScope, Promise) { this.isDeclared = (...args) => uiSettings.isDeclared(...args); this.isDefault = (...args) => uiSettings.isDefault(...args); this.isCustom = (...args) => uiSettings.isCustom(...args); + this.isOverridden = (...args) => uiSettings.isOverridden(...args); // modify remove() to use angular Promises this.remove = (key) => ( diff --git a/src/ui/ui_settings/__tests__/ui_settings_mixin_integration.js b/src/ui/ui_settings/__tests__/ui_settings_mixin_integration.js index 44d633b069395..aa4e20d2c0041 100644 --- a/src/ui/ui_settings/__tests__/ui_settings_mixin_integration.js +++ b/src/ui/ui_settings/__tests__/ui_settings_mixin_integration.js @@ -34,13 +34,13 @@ import { uiSettingsMixin } from '../ui_settings_mixin'; describe('uiSettingsMixin()', () => { const sandbox = sinon.createSandbox(); - async function setup(options = {}) { - const { - enabled = true - } = options; - + async function setup() { const config = await Config.withDefaultSchema({ - uiSettings: { enabled } + uiSettings: { + overrides: { + foo: 'bar' + } + } }); // maps of decorations passed to `server.decorate()` @@ -111,6 +111,9 @@ describe('uiSettingsMixin()', () => { sinon.assert.calledOnce(uiSettingsServiceFactory); sinon.assert.calledWithExactly(uiSettingsServiceFactory, server, { foo: 'bar', + overrides: { + foo: 'bar' + }, getDefaults: sinon.match.func, }); }); diff --git a/src/ui/ui_settings/__tests__/ui_settings_service.js b/src/ui/ui_settings/__tests__/ui_settings_service.js index 46757dc690baf..2f8e82880dd02 100644 --- a/src/ui/ui_settings/__tests__/ui_settings_service.js +++ b/src/ui/ui_settings/__tests__/ui_settings_service.js @@ -17,7 +17,6 @@ * under the License. */ -import { isEqual } from 'lodash'; import expect from 'expect.js'; import { errors as esErrors } from 'elasticsearch'; import Chance from 'chance'; @@ -42,6 +41,7 @@ describe('ui settings', () => { const { getDefaults, defaults = {}, + overrides, esDocSource = {}, savedObjectsClient = createObjectsClientStub(TYPE, ID, esDocSource) } = options; @@ -52,6 +52,7 @@ describe('ui settings', () => { buildNum: BUILD_NUM, getDefaults: getDefaults || (() => defaults), savedObjectsClient, + overrides, }); const createOrUpgradeSavedConfig = sandbox.stub(createOrUpgradeSavedConfigNS, 'createOrUpgradeSavedConfig'); @@ -67,21 +68,6 @@ describe('ui settings', () => { afterEach(() => sandbox.restore()); - describe('overview', () => { - it('has expected api surface', () => { - const { uiSettings } = setup(); - expect(uiSettings).to.have.property('get').a('function'); - expect(uiSettings).to.have.property('getAll').a('function'); - expect(uiSettings).to.have.property('getDefaults').a('function'); - expect(uiSettings).to.have.property('getRaw').a('function'); - expect(uiSettings).to.have.property('getUserProvided').a('function'); - expect(uiSettings).to.have.property('remove').a('function'); - expect(uiSettings).to.have.property('removeMany').a('function'); - expect(uiSettings).to.have.property('set').a('function'); - expect(uiSettings).to.have.property('setMany').a('function'); - }); - }); - describe('#setMany()', () => { it('returns a promise', () => { const { uiSettings } = setup(); @@ -127,6 +113,23 @@ describe('ui settings', () => { sinon.assert.calledTwice(savedObjectsClient.update); sinon.assert.calledOnce(createOrUpgradeSavedConfig); }); + + it('throws an error if any key is overridden', async () => { + const { uiSettings } = setup({ + overrides: { + foo: 'bar' + } + }); + + try { + await uiSettings.setMany({ + bar: 'box', + foo: 'baz' + }); + } catch (error) { + expect(error.message).to.be('Unable to update "foo" because it is overridden'); + } + }); }); describe('#set()', () => { @@ -140,6 +143,20 @@ describe('ui settings', () => { await uiSettings.set('one', 'value'); savedObjectsClient.assertUpdateQuery({ one: 'value' }); }); + + it('throws an error if the key is overridden', async () => { + const { uiSettings } = setup({ + overrides: { + foo: 'bar' + } + }); + + try { + await uiSettings.set('foo', 'baz'); + } catch (error) { + expect(error.message).to.be('Unable to update "foo" because it is overridden'); + } + }); }); describe('#remove()', () => { @@ -153,6 +170,20 @@ describe('ui settings', () => { await uiSettings.remove('one'); savedObjectsClient.assertUpdateQuery({ one: null }); }); + + it('throws an error if the key is overridden', async () => { + const { uiSettings } = setup({ + overrides: { + foo: 'bar' + } + }); + + try { + await uiSettings.remove('foo'); + } catch (error) { + expect(error.message).to.be('Unable to update "foo" because it is overridden'); + } + }); }); describe('#removeMany()', () => { @@ -172,6 +203,20 @@ describe('ui settings', () => { await uiSettings.removeMany(['one', 'two', 'three']); savedObjectsClient.assertUpdateQuery({ one: null, two: null, three: null }); }); + + it('throws an error if any key is overridden', async () => { + const { uiSettings } = setup({ + overrides: { + foo: 'bar' + } + }); + + try { + await uiSettings.setMany(['bar', 'foo']); + } catch (error) { + expect(error.message).to.be('Unable to update "foo" because it is overridden'); + } + }); }); describe('#getDefaults()', () => { @@ -210,18 +255,25 @@ describe('ui settings', () => { const esDocSource = { user: 'customized' }; const { uiSettings } = setup({ esDocSource }); const result = await uiSettings.getUserProvided(); - expect(isEqual(result, { - user: { userValue: 'customized' } - })).to.equal(true); + expect(result).to.eql({ + user: { + userValue: 'customized', + } + }); }); it('ignores null user configuration (because default values)', async () => { const esDocSource = { user: 'customized', usingDefault: null, something: 'else' }; const { uiSettings } = setup({ esDocSource }); const result = await uiSettings.getUserProvided(); - expect(isEqual(result, { - user: { userValue: 'customized' }, something: { userValue: 'else' } - })).to.equal(true); + expect(result).to.eql({ + user: { + userValue: 'customized' + }, + something: { + userValue: 'else' + } + }); }); it('returns an empty object on 404 responses', async () => { @@ -291,6 +343,27 @@ describe('ui settings', () => { expect(err).to.be(expectedUnexpectedError); } }); + + it('includes overridden values for overridden keys', async () => { + const esDocSource = { + user: 'customized' + }; + + const overrides = { + foo: 'bar' + }; + + const { uiSettings } = setup({ esDocSource, overrides }); + expect(await uiSettings.getUserProvided()).to.eql({ + user: { + userValue: 'customized', + }, + foo: { + userValue: 'bar', + isOverridden: true, + }, + }); + }); }); describe('#getRaw()', () => { @@ -324,6 +397,24 @@ describe('ui settings', () => { }, }); }); + + it('includes the values for overridden keys', async () => { + const esDocSource = { foo: 'bar' }; + const defaults = { key: { value: chance.word() } }; + const overrides = { foo: true }; + const { uiSettings } = setup({ esDocSource, defaults, overrides }); + const result = await uiSettings.getRaw(); + + expect(result).to.eql({ + foo: { + userValue: true, + isOverridden: true, + }, + key: { + value: defaults.key.value, + }, + }); + }); }); describe('#getAll()', () => { @@ -361,6 +452,29 @@ describe('ui settings', () => { bar: 'user-provided', }); }); + + it('includes the values for overridden keys', async () => { + const esDocSource = { + foo: 'user-override', + bar: 'user-provided', + }; + + const defaults = { + foo: { + value: 'default' + }, + }; + + const overrides = { + foo: 'bax' + }; + + const { uiSettings } = setup({ esDocSource, defaults, overrides }); + expect(await uiSettings.getAll()).to.eql({ + foo: 'bax', + bar: 'user-provided', + }); + }); }); describe('#get()', () => { @@ -392,5 +506,61 @@ describe('ui settings', () => { const result = await uiSettings.get('dateFormat'); expect(result).to.equal('YYYY-MM-DD'); }); + + it('returns the overridden value for an overrided key', async () => { + const esDocSource = { dateFormat: 'YYYY-MM-DD' }; + const overrides = { dateFormat: 'foo' }; + const { uiSettings } = setup({ esDocSource, overrides }); + expect(await uiSettings.get('dateFormat')).to.be('foo'); + }); + + it('returns the default value for an override with value null', async () => { + const esDocSource = { dateFormat: 'YYYY-MM-DD' }; + const overrides = { dateFormat: null }; + const defaults = { dateFormat: { value: 'foo' } }; + const { uiSettings } = setup({ esDocSource, overrides, defaults }); + expect(await uiSettings.get('dateFormat')).to.be('foo'); + }); + + it('returns the overridden value if the document does not exist', async () => { + const overrides = { dateFormat: 'foo' }; + const { uiSettings, savedObjectsClient } = setup({ overrides }); + savedObjectsClient.get.throws(savedObjectsClientErrors.createGenericNotFoundError()); + expect(await uiSettings.get('dateFormat')).to.be('foo'); + }); + }); + + describe('#isOverridden()', () => { + it('returns false if no overrides defined', () => { + const { uiSettings } = setup(); + expect(uiSettings.isOverridden('foo')).to.be(false); + }); + it('returns false if overrides defined but key is not included', () => { + const { uiSettings } = setup({ overrides: { foo: true, bar: true } }); + expect(uiSettings.isOverridden('baz')).to.be(false); + }); + it('returns false for object prototype properties', () => { + const { uiSettings } = setup({ overrides: { foo: true, bar: true } }); + expect(uiSettings.isOverridden('hasOwnProperty')).to.be(false); + }); + it('returns true if overrides defined and key is overridden', () => { + const { uiSettings } = setup({ overrides: { foo: true, bar: true } }); + expect(uiSettings.isOverridden('bar')).to.be(true); + }); + }); + + describe('#assertUpdateAllowed()', () => { + it('returns false if no overrides defined', () => { + const { uiSettings } = setup(); + expect(uiSettings.assertUpdateAllowed('foo')).to.be(undefined); + }); + it('throws 400 Boom error when keys is overridden', () => { + const { uiSettings } = setup({ overrides: { foo: true } }); + expect(() => uiSettings.assertUpdateAllowed('foo')).to.throwError(error => { + expect(error).to.have.property('message', 'Unable to update "foo" because it is overridden'); + expect(error).to.have.property('isBoom', true); + expect(error.output).to.have.property('statusCode', 400); + }); + }); }); }); diff --git a/src/ui/ui_settings/public/__snapshots__/ui_settings_client.test.js.snap b/src/ui/ui_settings/public/__snapshots__/ui_settings_client.test.js.snap index 84000c0913ed5..8915553b36bf1 100644 --- a/src/ui/ui_settings/public/__snapshots__/ui_settings_client.test.js.snap +++ b/src/ui/ui_settings/public/__snapshots__/ui_settings_client.test.js.snap @@ -20,6 +20,8 @@ You can use \`config.get(\\"throwableProperty\\", defaultValue)\`, which will ju \`defaultValue\` when the key is unrecognized." `; +exports[`#overrideLocalDefault #assertUpdateAllowed() throws error when keys is overridden 1`] = `"Unable to update \\"foo\\" because its value is overridden by the Kibana server"`; + exports[`#overrideLocalDefault key has no user value calls subscriber with new and previous value: single subscriber call 1`] = ` Array [ Array [ @@ -95,6 +97,10 @@ Object { } `; +exports[`#remove throws an error if key is overridden 1`] = `"Unable to update \\"bar\\" because its value is overridden by the Kibana server"`; + +exports[`#set throws an error if key is overridden 1`] = `"Unable to update \\"foo\\" because its value is overridden by the Kibana server"`; + exports[`#subscribe calls handler with { key, newValue, oldValue } when config changes 1`] = ` Array [ Array [ diff --git a/src/ui/ui_settings/public/ui_settings_client.js b/src/ui/ui_settings/public/ui_settings_client.js index f479ab37262cc..60505065143d7 100644 --- a/src/ui/ui_settings/public/ui_settings_client.js +++ b/src/ui/ui_settings/public/ui_settings_client.js @@ -102,6 +102,16 @@ You can use \`config.get("${key}", defaultValue)\`, which will just return return this.isDeclared(key) && !('value' in this._cache[key]); } + isOverridden(key) { + return this.isDeclared(key) && Boolean(this._cache[key].isOverridden); + } + + assertUpdateAllowed(key) { + if (this.isOverridden(key)) { + throw new Error(`Unable to update "${key}" because its value is overridden by the Kibana server`); + } + } + overrideLocalDefault(key, newDefault) { // capture the previous value const prevDefault = this._defaults[key] @@ -137,6 +147,8 @@ You can use \`config.get("${key}", defaultValue)\`, which will just return } async _update(key, value) { + this.assertUpdateAllowed(key); + const declared = this.isDeclared(key); const defaults = this._defaults; @@ -151,7 +163,7 @@ You can use \`config.get("${key}", defaultValue)\`, which will just return } const initialVal = declared ? this.get(key) : undefined; - this._setLocally(key, newVal, initialVal); + this._setLocally(key, newVal); try { const { settings } = await this._api.batchSet(key, newVal); @@ -165,6 +177,8 @@ You can use \`config.get("${key}", defaultValue)\`, which will just return } _setLocally(key, newValue) { + this.assertUpdateAllowed(key); + if (!this.isDeclared(key)) { this._cache[key] = {}; } diff --git a/src/ui/ui_settings/public/ui_settings_client.test.js b/src/ui/ui_settings/public/ui_settings_client.test.js index 6fda16da1096f..f41c9bc0018ca 100644 --- a/src/ui/ui_settings/public/ui_settings_client.test.js +++ b/src/ui/ui_settings/public/ui_settings_client.test.js @@ -123,6 +123,18 @@ describe('#set', () => { await expect(config.set('foo', 'bar')).resolves.toBe(false); }); + + it('throws an error if key is overridden', async () => { + const { config } = setup({ + initialSettings: { + foo: { + isOverridden: true, + value: 'bar' + } + } + }); + await expect(config.set('foo', true)).rejects.toThrowErrorMatchingSnapshot(); + }); }); describe('#remove', () => { @@ -140,6 +152,18 @@ describe('#remove', () => { await expect(config.remove('dateFormat')).resolves.toBe(false); }); + + it('throws an error if key is overridden', async () => { + const { config } = setup({ + initialSettings: { + bar: { + isOverridden: true, + userValue: true + } + } + }); + await expect(config.remove('bar')).rejects.toThrowErrorMatchingSnapshot(); + }); }); describe('#isDeclared', () => { @@ -293,4 +317,61 @@ describe('#overrideLocalDefault', () => { expect(config.getAll()).toMatchSnapshot('getAll after override'); }); }); + + describe('#isOverridden()', () => { + it('returns false if key is unknown', () => { + const { config } = setup(); + expect(config.isOverridden('foo')).toBe(false); + }); + it('returns false if key is no overridden', () => { + const { config } = setup({ + initialSettings: { + foo: { + userValue: 1 + }, + bar: { + isOverridden: true, + userValue: 2 + } + } + }); + expect(config.isOverridden('foo')).toBe(false); + }); + it('returns true when key is overridden', () => { + const { config } = setup({ + initialSettings: { + foo: { + userValue: 1 + }, + bar: { + isOverridden: true, + userValue: 2 + }, + } + }); + expect(config.isOverridden('bar')).toBe(true); + }); + it('returns false for object prototype properties', () => { + const { config } = setup(); + expect(config.isOverridden('hasOwnProperty')).toBe(false); + }); + }); + + describe('#assertUpdateAllowed()', () => { + it('returns false if no settings defined', () => { + const { config } = setup(); + expect(config.assertUpdateAllowed('foo')).toBe(undefined); + }); + it('throws error when keys is overridden', () => { + const { config } = setup({ + initialSettings: { + foo: { + isOverridden: true, + userValue: 'bar' + } + } + }); + expect(() => config.assertUpdateAllowed('foo')).toThrowErrorMatchingSnapshot(); + }); + }); }); diff --git a/src/ui/ui_settings/routes/__tests__/doc_exists.js b/src/ui/ui_settings/routes/__tests__/doc_exists.js index 7cc1f8b542354..2c03ab66e4983 100644 --- a/src/ui/ui_settings/routes/__tests__/doc_exists.js +++ b/src/ui/ui_settings/routes/__tests__/doc_exists.js @@ -82,7 +82,11 @@ export function docExistsSuite() { }, defaultIndex: { userValue: defaultIndex - } + }, + foo: { + userValue: 'bar', + isOverridden: true + }, } }); }); @@ -109,10 +113,33 @@ export function docExistsSuite() { }, defaultIndex: { userValue: defaultIndex - } + }, + foo: { + userValue: 'bar', + isOverridden: true + }, } }); }); + + it('returns a 400 if trying to set overridden value', async () => { + const { kbnServer } = await setup(); + + const { statusCode, result } = await kbnServer.inject({ + method: 'POST', + url: '/api/kibana/settings/foo', + payload: { + value: 'baz' + } + }); + + expect(statusCode).to.be(400); + assertSinonMatch(result, { + error: 'Bad Request', + message: 'Unable to update "foo" because it is overridden', + statusCode: 400 + }); + }); }); describe('setMany route', () => { @@ -138,9 +165,34 @@ export function docExistsSuite() { }, defaultIndex: { userValue: defaultIndex + }, + foo: { + userValue: 'bar', + isOverridden: true + }, + } + }); + }); + + it('returns a 400 if trying to set overridden value', async () => { + const { kbnServer } = await setup(); + + const { statusCode, result } = await kbnServer.inject({ + method: 'POST', + url: '/api/kibana/settings', + payload: { + changes: { + foo: 'baz' } } }); + + expect(statusCode).to.be(400); + assertSinonMatch(result, { + error: 'Bad Request', + message: 'Unable to update "foo" because it is overridden', + statusCode: 400 + }); }); }); @@ -164,9 +216,28 @@ export function docExistsSuite() { settings: { buildNum: { userValue: sinon.match.number - } + }, + foo: { + userValue: 'bar', + isOverridden: true + }, } }); }); + it('returns a 400 if deleting overridden value', async () => { + const { kbnServer } = await setup(); + + const { statusCode, result } = await kbnServer.inject({ + method: 'DELETE', + url: '/api/kibana/settings/foo' + }); + + expect(statusCode).to.be(400); + assertSinonMatch(result, { + error: 'Bad Request', + message: 'Unable to update "foo" because it is overridden', + statusCode: 400 + }); + }); }); } diff --git a/src/ui/ui_settings/routes/__tests__/doc_missing.js b/src/ui/ui_settings/routes/__tests__/doc_missing.js index 4fd39649a5004..d3afefcc0932a 100644 --- a/src/ui/ui_settings/routes/__tests__/doc_missing.js +++ b/src/ui/ui_settings/routes/__tests__/doc_missing.js @@ -48,7 +48,7 @@ export function docMissingSuite() { }); describe('get route', () => { - it('creates doc, returns a 200 with no settings', async () => { + it('creates doc, returns a 200 with only overridden settings', async () => { const { kbnServer } = getServices(); const { statusCode, result } = await kbnServer.inject({ @@ -58,7 +58,12 @@ export function docMissingSuite() { expect(statusCode).to.be(200); assertSinonMatch(result, { - settings: {} + settings: { + foo: { + userValue: 'bar', + isOverridden: true + } + } }); }); }); @@ -82,6 +87,10 @@ export function docMissingSuite() { }, defaultIndex: { userValue: defaultIndex + }, + foo: { + userValue: 'bar', + isOverridden: true } } }); @@ -109,6 +118,10 @@ export function docMissingSuite() { }, defaultIndex: { userValue: defaultIndex + }, + foo: { + userValue: 'bar', + isOverridden: true } } }); @@ -129,6 +142,10 @@ export function docMissingSuite() { settings: { buildNum: { userValue: sinon.match.number + }, + foo: { + userValue: 'bar', + isOverridden: true } } }); diff --git a/src/ui/ui_settings/routes/__tests__/index.js b/src/ui/ui_settings/routes/__tests__/index.js index ca4a9e589de9b..7479996576c6a 100644 --- a/src/ui/ui_settings/routes/__tests__/index.js +++ b/src/ui/ui_settings/routes/__tests__/index.js @@ -27,7 +27,6 @@ import { docMissingSuite } from './doc_missing'; import { indexMissingSuite } from './index_missing'; describe('uiSettings/routes', function () { - /** * The "doc missing" and "index missing" tests verify how the uiSettings * API behaves in between healthChecks, so they interact with the healthCheck diff --git a/src/ui/ui_settings/routes/__tests__/index_missing.js b/src/ui/ui_settings/routes/__tests__/index_missing.js index 838cc9b48b524..c8487d03b363b 100644 --- a/src/ui/ui_settings/routes/__tests__/index_missing.js +++ b/src/ui/ui_settings/routes/__tests__/index_missing.js @@ -59,7 +59,7 @@ export function indexMissingSuite() { } describe('get route', () => { - it('returns a 200 and with empty values', async () => { + it('returns a 200 and with just overridden values', async () => { const { kbnServer } = await setup(); const { statusCode, result } = await kbnServer.inject({ @@ -68,7 +68,14 @@ export function indexMissingSuite() { }); expect(statusCode).to.be(200); - expect(result).to.eql({ settings: {} }); + expect(result).to.eql({ + settings: { + foo: { + userValue: 'bar', + isOverridden: true + } + } + }); }); }); @@ -93,6 +100,10 @@ export function indexMissingSuite() { }, defaultIndex: { userValue: defaultIndex + }, + foo: { + userValue: 'bar', + isOverridden: true } } }); @@ -122,6 +133,10 @@ export function indexMissingSuite() { }, defaultIndex: { userValue: defaultIndex + }, + foo: { + userValue: 'bar', + isOverridden: true } } }); @@ -144,6 +159,10 @@ export function indexMissingSuite() { settings: { buildNum: { userValue: sinon.match.number + }, + foo: { + userValue: 'bar', + isOverridden: true } } }); diff --git a/src/ui/ui_settings/routes/__tests__/lib/servers.js b/src/ui/ui_settings/routes/__tests__/lib/servers.js index 38ccd172cf5a2..825c5c85ca7d5 100644 --- a/src/ui/ui_settings/routes/__tests__/lib/servers.js +++ b/src/ui/ui_settings/routes/__tests__/lib/servers.js @@ -39,7 +39,13 @@ export async function startServers() { log.indent(-4); await es.start(); - kbnServer = kbnTestServer.createServerWithCorePlugins(); + kbnServer = kbnTestServer.createServerWithCorePlugins({ + uiSettings: { + overrides: { + foo: 'bar', + } + } + }); await kbnServer.ready(); await kbnServer.server.plugins.elasticsearch.waitUntilReady(); } diff --git a/src/ui/ui_settings/ui_settings_mixin.js b/src/ui/ui_settings/ui_settings_mixin.js index 6ea81df2c229b..9b958efc97848 100644 --- a/src/ui/ui_settings/ui_settings_mixin.js +++ b/src/ui/ui_settings/ui_settings_mixin.js @@ -30,10 +30,12 @@ export function uiSettingsMixin(kbnServer, server) { const getDefaults = () => ( kbnServer.uiExports.uiSettingDefaults ); + const overrides = kbnServer.config.get('uiSettings.overrides'); server.decorate('server', 'uiSettingsServiceFactory', (options = {}) => { return uiSettingsServiceFactory(server, { getDefaults, + overrides, ...options }); }); @@ -41,6 +43,7 @@ export function uiSettingsMixin(kbnServer, server) { server.addMemoizedFactoryToRequest('getUiSettingsService', request => { return getUiSettingsServiceForRequest(server, request, { getDefaults, + overrides, }); }); diff --git a/src/ui/ui_settings/ui_settings_service.js b/src/ui/ui_settings/ui_settings_service.js index bf9b12133999a..6234884fdb881 100644 --- a/src/ui/ui_settings/ui_settings_service.js +++ b/src/ui/ui_settings/ui_settings_service.js @@ -18,14 +18,9 @@ */ import { defaultsDeep } from 'lodash'; -import { createOrUpgradeSavedConfig } from './create_or_upgrade_saved_config'; +import Boom from 'boom'; -function hydrateUserSettings(userSettings) { - return Object.keys(userSettings) - .map(key => ({ key, userValue: userSettings[key] })) - .filter(({ userValue }) => userValue !== null) - .reduce((acc, { key, userValue }) => ({ ...acc, [key]: { userValue } }), {}); -} +import { createOrUpgradeSavedConfig } from './create_or_upgrade_saved_config'; /** * Service that provides access to the UiSettings stored in elasticsearch. @@ -53,6 +48,7 @@ export class UiSettingsService { getDefaults = () => ({}), // function that accepts log messages in the same format as server.log log = () => {}, + overrides = {}, } = options; this._type = type; @@ -60,6 +56,7 @@ export class UiSettingsService { this._buildNum = buildNum; this._savedObjectsClient = savedObjectsClient; this._getDefaults = getDefaults; + this._overrides = overrides; this._log = log; } @@ -91,7 +88,26 @@ export class UiSettingsService { } async getUserProvided(options) { - return hydrateUserSettings(await this._read(options)); + const userProvided = {}; + + // write the userValue for each key stored in the saved object that is not overridden + for (const [key, userValue] of Object.entries(await this._read(options))) { + if (userValue !== null && !this.isOverridden(key)) { + userProvided[key] = { + userValue + }; + } + } + + // write all overridden keys, dropping the userValue is override is null and + // adding keys for overrides that are not in saved object + for (const [key, userValue] of Object.entries(this._overrides)) { + userProvided[key] = userValue === null + ? { isOverridden: true } + : { isOverridden: true, userValue }; + } + + return userProvided; } async setMany(changes) { @@ -114,7 +130,21 @@ export class UiSettingsService { await this.setMany(changes); } + isOverridden(key) { + return this._overrides.hasOwnProperty(key); + } + + assertUpdateAllowed(key) { + if (this.isOverridden(key)) { + throw Boom.badRequest(`Unable to update "${key}" because it is overridden`); + } + } + async _write({ changes, autoCreateOrUpgradeIfMissing = true }) { + for (const key of Object.keys(changes)) { + this.assertUpdateAllowed(key); + } + try { await this._savedObjectsClient.update(this._type, this._id, changes); } catch (error) { diff --git a/src/ui/ui_settings/ui_settings_service_factory.js b/src/ui/ui_settings/ui_settings_service_factory.js index 4b9859abf6bdb..595c8a2d904d5 100644 --- a/src/ui/ui_settings/ui_settings_service_factory.js +++ b/src/ui/ui_settings/ui_settings_service_factory.js @@ -37,6 +37,7 @@ export function uiSettingsServiceFactory(server, options) { const { savedObjectsClient, getDefaults, + overrides, } = options; return new UiSettingsService({ @@ -45,6 +46,7 @@ export function uiSettingsServiceFactory(server, options) { buildNum: config.get('pkg.buildNum'), savedObjectsClient, getDefaults, + overrides, log: (...args) => server.log(...args), }); } diff --git a/src/ui/ui_settings/ui_settings_service_for_request.js b/src/ui/ui_settings/ui_settings_service_for_request.js index 58dcbe22b4b0c..422c9cc14f833 100644 --- a/src/ui/ui_settings/ui_settings_service_for_request.js +++ b/src/ui/ui_settings/ui_settings_service_for_request.js @@ -34,11 +34,13 @@ import { uiSettingsServiceFactory } from './ui_settings_service_factory'; */ export function getUiSettingsServiceForRequest(server, request, options = {}) { const { - getDefaults + getDefaults, + overrides, } = options; const uiSettingsService = uiSettingsServiceFactory(server, { getDefaults, + overrides, savedObjectsClient: request.getSavedObjectsClient() }); diff --git a/test/common/services/kibana_server/ui_settings.js b/test/common/services/kibana_server/ui_settings.js index 8971a012f64b9..ff318fa5eeb16 100644 --- a/test/common/services/kibana_server/ui_settings.js +++ b/test/common/services/kibana_server/ui_settings.js @@ -66,7 +66,9 @@ export class KibanaServerUiSettings { const { payload } = await this._wreck.get('/api/kibana/settings'); for (const key of Object.keys(payload.settings)) { - await this._wreck.delete(`/api/kibana/settings/${key}`); + if (!payload.settings[key].isOverridden) { + await this._wreck.delete(`/api/kibana/settings/${key}`); + } } this._log.debug('replacing kibana config doc: %j', doc);