Skip to content

Enforce required presence for value/key validation of recordOf and mapOf.#60406

Merged
azasypkin merged 1 commit intoelastic:masterfrom
azasypkin:issue-xxx-kbn-config
Mar 18, 2020
Merged

Enforce required presence for value/key validation of recordOf and mapOf.#60406
azasypkin merged 1 commit intoelastic:masterfrom
azasypkin:issue-xxx-kbn-config

Conversation

@azasypkin
Copy link
Copy Markdown
Contributor

@azasypkin azasypkin commented Mar 17, 2020 •
edited
Loading

While working on a new feature that heavily relies on recordOf I've noticed that we have two bugs:

  • Joi's required presence isn't set when we validate recordOf/mapOf values/keys like we do for the "main" validation
  • Error messages for schema violations deep within values of recordOf/mapOf don't include full path

@azasypkin azasypkin added bug Fixes for quality problems that affect the customer experience blocker Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// v7.7.0 labels Mar 17, 2020
@azasypkin azasypkin requested review from a team as code owners March 17, 2020 16:31
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Mar 17, 2020

Pinging @elastic/kibana-platform (Team:Platform)

@azasypkin azasypkin added the release_note:skip Skip the PR/issue when compiling release notes label Mar 17, 2020
@azasypkin azasypkin mentioned this pull request Mar 17, 2020
Merged
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Mar 17, 2020

💛 Build succeeded, but was flaky

Test Failures

Kibana Pipeline / kibana-xpack-agent / renders the data provider of a host dragged from the All Hosts widget on the hosts page.timeline data providers renders the data provider of a host dragged from the All Hosts widget on the hosts page

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has not failed recently on tracked branches

Stack Trace

CypressError: Timed out retrying: expected 'siem' to equal 'host.name: "siem"'
    at cypressErr (http://elastic:changeme@localhost:61121/__cypress/runner/cypress_runner.js:149934:9)
    at throwErr (http://elastic:changeme@localhost:61121/__cypress/runner/cypress_runner.js:149867:11)
    at Object.throwErrByPath (http://elastic:changeme@localhost:61121/__cypress/runner/cypress_runner.js:149915:3)
    at retry (http://elastic:changeme@localhost:61121/__cypress/runner/cypress_runner.js:144197:19)
    at onFailFn (http://elastic:changeme@localhost:61121/__cypress/runner/cypress_runner.js:132448:16)
    at tryCatcher (http://elastic:changeme@localhost:61121/__cypress/runner/cypress_runner.js:167415:23)
    at Promise._settlePromiseFromHandler (http://elastic:changeme@localhost:61121/__cypress/runner/cypress_runner.js:165351:31)
    at Promise._settlePromise (http://elastic:changeme@localhost:61121/__cypress/runner/cypress_runner.js:165408:18)
    at Promise._settlePromise0 (http://elastic:changeme@localhost:61121/__cypress/runner/cypress_runner.js:165453:10)
    at Promise._settlePromises (http://elastic:changeme@localhost:61121/__cypress/runner/cypress_runner.js:165528:18)
    at Async../node_modules/bluebird/js/release/async.js.Async._drainQueue (http://elastic:changeme@localhost:61121/__cypress/runner/cypress_runner.js:162140:16)
    at Async../node_modules/bluebird/js/release/async.js.Async._drainQueues (http://elastic:changeme@localhost:61121/__cypress/runner/cypress_runner.js:162150:10)
    at Async.drainQueues (http://elastic:changeme@localhost:61121/__cypress/runner/cypress_runner.js:162024:14)

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@azasypkin azasypkin requested a review from mshustov March 18, 2020 07:21
pgayvallet
pgayvallet approved these changes Mar 18, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@pgayvallet pgayvallet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for kbn-config-schema changes

@azasypkin azasypkin requested a review from legrego March 18, 2020 10:25
legrego
legrego approved these changes Mar 18, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@legrego legrego left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@azasypkin azasypkin merged commit 45f59f7 into elastic:master Mar 18, 2020
@azasypkin azasypkin deleted the issue-xxx-kbn-config branch March 18, 2020 11:19
@azasypkin azasypkin mentioned this pull request Mar 18, 2020
Merged
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 18, 2020
@gmmorris
Merge branch 'master' into alerting/view-in-app
4584654 
* master:
  [License Management] NP migration (elastic#60250)
  Fix create alert button from not showing in alerts list (elastic#60444)
  [SIEM][Case] Update connector through flyout (elastic#60307)
  add data-test-subj where possible on SO management table (elastic#60226)
  Enforce `required` presence for value/key validation of `recordOf` and `mapOf`. (elastic#60406)
@azasypkin
Copy link
Copy Markdown
Contributor Author

azasypkin commented Mar 18, 2020

7.x/7.7.0: 90ce69a

jloleysens added a commit that referenced this pull request Mar 18, 2020
@jloleysens
Merge branch 'app/painless' of github.com:elastic/kibana into app/pai…
8733480 
…nless

* 'app/painless' of github.com:elastic/kibana: (64 commits)
  Fix filter scope in bool query (#60488)
  change index pattern id to be the same as index pattern title (#60436)
  [Endpoint] resolver v1 events (#59233)
  Branding fixes for dashboard, loader and space selector (#60073)
  skip flaky suite (#60535)
  [SIEM][Detection Engine] Fixes bug with timeline templates not working
  Fixed errors which are happening if switch between alert types (#60453)
  [EPM] Add mapping field types to index template generation v2 (#60266)
  [NP] Cutover ensureDefaultIndexPattern to kibana_utils (#59895)
  Closes #60265. Adds Beta badge to service map (#60482)
  [Visualize] Duplicated query filters in es request (#60106)
  [ML] Disable functional transform tests
  Fixes to service map single node banner (#60072)
  [Uptime] replace fetch with kibana http (#59881)
  Upgrade @types/node to match Node.js runtime (#60368)
  [License Management] NP migration (#60250)
  Fix create alert button from not showing in alerts list (#60444)
  [SIEM][Case] Update connector through flyout (#60307)
  add data-test-subj where possible on SO management table (#60226)
  Enforce `required` presence for value/key validation of `recordOf` and `mapOf`. (#60406)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 19, 2020
@gmmorris
rge branch 'alerting/tls-warning' of github.com:gmmorris/kibana into …
38c5e1e 
…alerting/tls-warning

* 'alerting/tls-warning' of github.com:gmmorris/kibana: (33 commits)
  [ML] Disable functional transform tests
  Fixes to service map single node banner (elastic#60072)
  [Uptime] replace fetch with kibana http (elastic#59881)
  Upgrade @types/node to match Node.js runtime (elastic#60368)
  [License Management] NP migration (elastic#60250)
  Fix create alert button from not showing in alerts list (elastic#60444)
  [SIEM][Case] Update connector through flyout (elastic#60307)
  add data-test-subj where possible on SO management table (elastic#60226)
  Enforce `required` presence for value/key validation of `recordOf` and `mapOf`. (elastic#60406)
  [ML] Re-enabling file upload telemetry (elastic#60418)
  [NP] Use local helper shortenDottedString for discover (elastic#60271)
  [Console] Fix for `_settings` and x-pack autocomplete (elastic#60246)
  Task/host enhancements (elastic#59671)
  [Search service] Asynchronous ES search strategy (elastic#53538)
  Index Action - Moved index params fields to connector config (elastic#60349)
  Edits UI text for ML nodes and job button (elastic#60184)
  Publish getIsNavDrawerLocked$ method on core chrome service. (elastic#60191)
  Disabled edit alert button on management ui for non registered UI alert types (elastic#60439)
  Revert "[Console] Fix bool filter autocompletions and refactor (elastic#60361)"
  [Console] Fix bool filter autocompletions and refactor (elastic#60361)
  ...
@jen-huang jen-huang mentioned this pull request Mar 19, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pgayvallet pgayvallet pgayvallet approved these changes

@legrego legrego legrego approved these changes

@mshustov mshustov Awaiting requested review from mshustov

Assignees

No one assigned

Labels

backported blocker bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes Team:Core Platform Core services: plugins, logging, config, saved objects, http, ES client, i18n, etc t// v7.7.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@azasypkin @elasticmachine @kibanamachine @pgayvallet @legrego