Further optimize check privileges response validation #90631
Conversation
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
legrego
added
chore
release_note:skip
|
Pinging @elastic/kibana-security (Team:Security) |
legrego
added
the
auto-backport
| const actualActions = Object.keys(value).sort(); | ||
| if ( | ||
| actions.length !== actualActions.length || | ||
| ![...actions].sort().every((x, i) => x === actualActions[i]) |
There was a problem hiding this comment.
note: actions is provided by consumers, so we shouldn't be mutating their copy of the data.
According to this benchmark, using the spread operator is the fastest way to copy an array.
| {}, | ||
| { | ||
| unknowns: 'allow', | ||
| validate: (value) => { | ||
| const actualResources = Object.keys(value).sort(); | ||
| if ( | ||
| resources.length !== actualResources.length || | ||
| !resources.sort().every((x, i) => x === actualResources[i]) | ||
| ![...resources].sort().every((x, i) => x === actualResources[i]) |
There was a problem hiding this comment.
note: resources is provided by consumers, so we shouldn't be mutating their copy of the data.
According to this benchmark, using the spread operator is the fastest way to copy an array.
|
@elasticmachine merge upstream |
| function buildValidationSchema(application: string, actions: string[], resources: string[]) { | ||
| const actionValidationSchema = buildActionsValidationSchema(actions); | ||
| const actionValidationSchema = schema.boolean(); | ||
| const actionsValidationSchema = schema.object( | ||
| {}, | ||
| { | ||
| unknowns: 'allow', | ||
| validate: (value) => { | ||
| const actualActions = Object.keys(value).sort(); | ||
| if ( | ||
| actions.length !== actualActions.length || | ||
| ![...actions].sort().every((x, i) => x === actualActions[i]) |
There was a problem hiding this comment.
Nit: FWIW, these changes will cause this validation schema to behave differently (fail) if the consumer passes in any duplicate actions. We only have one consumer, which is already ensuring that actions are unique:
May be worth noting this in a TSdoc on the validateEsPrivilegeResponse function, though.
Edit: added in 3026eb8.
💚 Build SucceededMetrics [docs]
History
To update your PR or re-run it, just comment with: |
Summary
Followup to #90074
Performs a similar optimization to #90074, but this time for
actionsinstead ofresources.@dgieselaar would you be able to profile this to see if it shows a reasonable improvement based on your testing? Or I'd be happy to do so if you help me get your tooling setup 🙂