-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Security Solution][Endpoint] Add an additional hint message for Console commands pending more than 15s #135500
Merged
kevinlog
merged 9 commits into
elastic:main
from
paul-tavares:task/olm-4272-show-hint-on-long-running-actions
Jul 6, 2022
Merged
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
c81750d
Add `enteredAt` to CommandHistoryItem and some refactoring in `handle…
paul-tavares 16d4ff3
New components: ConsoleText + LongRunningCommandHint
paul-tavares fcf4ab1
Adds logic to display the Long running command hint to the execution …
paul-tavares 366b643
use ConsoleText in command execution result
paul-tavares 02f9fcd
Fix type issue
paul-tavares ad5271c
tests for logic around showing long running command hint
paul-tavares 9249259
Apply suggestions from code review
paul-tavares 209e3dd
Merge branch 'main' into task/olm-4272-show-hint-on-long-running-actions
paul-tavares 564a2ff
Merge branch 'main' of github.com:elastic/kibana into task/olm-4272-s…
kevinlog File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
73 changes: 73 additions & 0 deletions
73
...olution/public/management/components/console/components/command_execution_output.test.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
import React from 'react'; | ||
import { ConsoleProps } from '..'; | ||
import { AppContextTestRender } from '../../../../common/mock/endpoint'; | ||
import { getConsoleTestSetup } from '../mocks'; | ||
import { act } from '@testing-library/react'; | ||
import { CommandExecutionComponentProps } from '../types'; | ||
|
||
describe('When using CommandExecutionOutput component', () => { | ||
let render: (props?: Partial<ConsoleProps>) => ReturnType<AppContextTestRender['render']>; | ||
let renderResult: ReturnType<typeof render>; | ||
let setCmd1ToComplete: () => void; | ||
|
||
beforeEach(() => { | ||
const { renderConsole, commands, enterCommand } = getConsoleTestSetup(); | ||
|
||
const cmd1 = commands.find((command) => command.name === 'cmd1'); | ||
|
||
if (!cmd1) { | ||
throw new Error('cmd1 command not found in test mocks'); | ||
} | ||
|
||
(cmd1.RenderComponent as jest.Mock).mockImplementation( | ||
(props: CommandExecutionComponentProps) => { | ||
setCmd1ToComplete = () => props.setStatus('success'); | ||
|
||
return <div>{'output'}</div>; | ||
} | ||
); | ||
|
||
render = (props = {}) => { | ||
renderResult = renderConsole(props); | ||
enterCommand('cmd1'); | ||
return renderResult; | ||
}; | ||
}); | ||
|
||
it('should show long running hint message if pending and >15s have passed', () => { | ||
jest.useFakeTimers(); | ||
render(); | ||
|
||
expect(renderResult.queryByTestId('test-longRunningCommandHint')).toBeNull(); | ||
|
||
act(() => { | ||
jest.advanceTimersByTime(16 * 1000); | ||
}); | ||
|
||
expect(renderResult.getByTestId('test-longRunningCommandHint')).not.toBeNull(); | ||
}); | ||
|
||
it('should remove long running hint message if command completes', async () => { | ||
jest.useFakeTimers(); | ||
render(); | ||
|
||
act(() => { | ||
jest.advanceTimersByTime(16 * 1000); | ||
}); | ||
|
||
expect(renderResult.getByTestId('test-longRunningCommandHint')).not.toBeNull(); | ||
|
||
act(() => { | ||
setCmd1ToComplete(); | ||
}); | ||
|
||
expect(renderResult.queryByTestId('test-longRunningCommandHint')).toBeNull(); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So here the
elapsedSeconds
never increments or is always0
. Basically, the timeout is set to execute after 15 seconds and sets theisLongRunningCommand
totrue
. We're not actually using the time difference here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what you mean.
Let me explain this logic around
elapsedSeconds
. You can enter a command and then close the Responder window... When we re-open that same responder window, we need to determine how much time has elapsed since the command was entered so that we can correctly calculate thesetTimeout()
ms.Example:
:00 s
and immediately close the consolesetTimeout()
should now only wait10s
since 5s has already elapsed... so the(15 - elapsedSeconds)
does that.15s
has not yet elapsedThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see. When the command is executed and the console stays open beyond 15 seconds the
elapsedSeconds
is always 0 and thus theelapsedSeconds >= 15
block does not execute is what I meant. I didn't test it by closing and then re-opening the console, in which case the block does execute and it works as expected.