-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[ENDPOINT][INGEST]Task/endpoint ingest update #67234
Changes from 22 commits
e300f90
11c3837
0cf3ae9
82b20af
2b0fa3e
4ccc118
3398ed6
f232a7c
b1777d9
8444aec
057f440
f9852af
ad71c88
3efce1c
8586e3b
412f5e0
7583c48
04dee90
2208beb
b43801e
9ee8750
4f4e14c
635d035
ac2441d
165c0fb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,15 +4,13 @@ | |
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
import React, { useContext } from 'react'; | ||
import { CoreStart } from 'src/core/public'; | ||
import { CoreStart } from 'kibana/public'; | ||
import { useKibana } from '../../../../../../../src/plugins/kibana_react/public'; | ||
|
||
export const CoreContext = React.createContext<CoreStart | null>(null); | ||
|
||
export function useCore() { | ||
const core = useContext(CoreContext); | ||
if (core === null) { | ||
throw new Error('CoreContext not initialized'); | ||
export function useCore(): CoreStart { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. updated useCore() to use the new useKibana hook There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is pretty cool, I didn't know that |
||
const { services } = useKibana(); | ||
if (services === null) { | ||
throw new Error('KibanaContextProvider not initialized'); | ||
} | ||
return core; | ||
return services; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will return the same thing that core returned. could be renamed to say core if that clears up any confusion. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't mind keeping it as |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,11 +22,12 @@ import { PAGE_ROUTING_PATHS } from './constants'; | |
import { DefaultLayout, WithoutHeaderLayout } from './layouts'; | ||
import { Loading, Error } from './components'; | ||
import { IngestManagerOverview, EPMApp, AgentConfigApp, FleetApp, DataStreamApp } from './sections'; | ||
import { CoreContext, DepsContext, ConfigContext, setHttpClient, useConfig } from './hooks'; | ||
import { DepsContext, ConfigContext, setHttpClient, useConfig } from './hooks'; | ||
import { PackageInstallProvider } from './sections/epm/hooks'; | ||
import { useCore, sendSetup, sendGetPermissionsCheck } from './hooks'; | ||
import { FleetStatusProvider } from './hooks/use_fleet_status'; | ||
import './index.scss'; | ||
import { KibanaContextProvider } from '../../../../../../src/plugins/kibana_react/public'; | ||
|
||
export interface ProtectedRouteProps extends RouteProps { | ||
isAllowed?: boolean; | ||
|
@@ -229,15 +230,15 @@ const IngestManagerApp = ({ | |
const isDarkMode = useObservable<boolean>(coreStart.uiSettings.get$('theme:darkMode')); | ||
return ( | ||
<coreStart.i18n.Context> | ||
<CoreContext.Provider value={coreStart}> | ||
<KibanaContextProvider services={{ ...coreStart }}> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. uses the newer KibanaContextProvider in place of CoreContext provider, which also exposes coreStart and allows use to use the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ingest Team: FYI: I actually learned about this functionality from Kibana from one of you - maybe @jfsiii 😄 |
||
<DepsContext.Provider value={{ setup: setupDeps, start: startDeps }}> | ||
<ConfigContext.Provider value={config}> | ||
<EuiThemeProvider darkMode={isDarkMode}> | ||
<IngestManagerRoutes basepath={basepath} /> | ||
</EuiThemeProvider> | ||
</ConfigContext.Provider> | ||
</DepsContext.Provider> | ||
</CoreContext.Provider> | ||
</KibanaContextProvider> | ||
</coreStart.i18n.Context> | ||
); | ||
}; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
import React from 'react'; | ||
import { FormattedMessage } from '@kbn/i18n/react'; | ||
import { EuiEmptyPrompt, EuiText } from '@elastic/eui'; | ||
import { ConfigureEndpointDatasource } from '../../../../../../../../siem/public'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't this create a circular dependency? Ingest now depends on SIEM which already depends on Ingest. I'm away from my computer for a bit so I can't dig in, but can Ingest definite/export this instead? |
||
import { NewDatasource } from '../../../../types'; | ||
import { CreateDatasourceFrom } from '../types'; | ||
|
||
export interface CustomConfigureDatasourceProps { | ||
packageName: string; | ||
from: CreateDatasourceFrom; | ||
datasource: NewDatasource | (NewDatasource & { id: string }); | ||
} | ||
|
||
/** | ||
* Allows external plugins to create custom content for the Ingest | ||
* Datasource configuration. | ||
*/ | ||
export type CustomConfigureDatasourceContent = React.FC<CustomConfigureDatasourceProps>; | ||
|
||
const ConfigureDatasourceMapping: { [key: string]: CustomConfigureDatasourceContent } = { | ||
endpoint: ConfigureEndpointDatasource, | ||
}; | ||
|
||
const EmptyConfigureDatasource: CustomConfigureDatasourceContent = () => ( | ||
<EuiEmptyPrompt | ||
iconType="checkInCircleFilled" | ||
iconColor="secondary" | ||
body={ | ||
<EuiText> | ||
<p> | ||
<FormattedMessage | ||
id="xpack.ingestManager.createDatasource.stepConfigure.noConfigOptionsMessage" | ||
defaultMessage="Nothing to configure" | ||
/> | ||
</p> | ||
</EuiText> | ||
} | ||
/> | ||
); | ||
|
||
export const CustomConfigureDatasource = (props: CustomConfigureDatasourceProps) => { | ||
const ConfigureDatasourceContent = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jen-huang The package definition has a flag called I don't see that property defined in Datasources type, so I assume that is future functionality? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, we need to implement that on the ingest management side. Could you make an issue for it? I wasn't aware that flag was already implemented on registry side :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @jen-huang . Created #67939 to track |
||
ConfigureDatasourceMapping[props.packageName] || EmptyConfigureDatasource; | ||
return <ConfigureDatasourceContent {...props} />; | ||
}; |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -5,28 +5,29 @@ | |||||
*/ | ||||||
import React from 'react'; | ||||||
import { FormattedMessage } from '@kbn/i18n/react'; | ||||||
import { | ||||||
EuiPanel, | ||||||
EuiFlexGroup, | ||||||
EuiFlexItem, | ||||||
EuiSpacer, | ||||||
EuiEmptyPrompt, | ||||||
EuiText, | ||||||
EuiCallOut, | ||||||
} from '@elastic/eui'; | ||||||
import { EuiPanel, EuiFlexGroup, EuiFlexItem, EuiSpacer, EuiCallOut } from '@elastic/eui'; | ||||||
import { i18n } from '@kbn/i18n'; | ||||||
import { PackageInfo, NewDatasource, DatasourceInput } from '../../../types'; | ||||||
import { Loading } from '../../../components'; | ||||||
import { DatasourceValidationResults, validationHasErrors } from './services'; | ||||||
import { DatasourceInputPanel } from './components'; | ||||||
import { DatasourceInputPanel, CustomConfigureDatasource } from './components'; | ||||||
import { CreateDatasourceFrom } from './types'; | ||||||
|
||||||
export const StepConfigureDatasource: React.FunctionComponent<{ | ||||||
from?: CreateDatasourceFrom; | ||||||
packageInfo: PackageInfo; | ||||||
datasource: NewDatasource; | ||||||
datasource: NewDatasource | (NewDatasource & { id: string }); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe? 😬 If also export the
Suggested change
|
||||||
updateDatasource: (fields: Partial<NewDatasource>) => void; | ||||||
validationResults: DatasourceValidationResults; | ||||||
submitAttempted: boolean; | ||||||
}> = ({ packageInfo, datasource, updateDatasource, validationResults, submitAttempted }) => { | ||||||
}> = ({ | ||||||
from = 'config', | ||||||
packageInfo, | ||||||
datasource, | ||||||
updateDatasource, | ||||||
validationResults, | ||||||
submitAttempted, | ||||||
}) => { | ||||||
const hasErrors = validationResults ? validationHasErrors(validationResults) : false; | ||||||
|
||||||
// Configure inputs (and their streams) | ||||||
|
@@ -68,19 +69,10 @@ export const StepConfigureDatasource: React.FunctionComponent<{ | |||||
</EuiFlexGroup> | ||||||
) : ( | ||||||
<EuiPanel> | ||||||
<EuiEmptyPrompt | ||||||
iconType="checkInCircleFilled" | ||||||
iconColor="secondary" | ||||||
body={ | ||||||
<EuiText> | ||||||
<p> | ||||||
<FormattedMessage | ||||||
id="xpack.ingestManager.createDatasource.stepConfigure.noConfigOptionsMessage" | ||||||
defaultMessage="Nothing to configure" | ||||||
/> | ||||||
</p> | ||||||
</EuiText> | ||||||
} | ||||||
<CustomConfigureDatasource | ||||||
packageName={packageInfo.name} | ||||||
datasource={datasource} | ||||||
from={from} | ||||||
/> | ||||||
</EuiPanel> | ||||||
); | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -69,7 +69,8 @@ export const EditDatasourcePage: React.FunctionComponent = () => { | |
const [loadingError, setLoadingError] = useState<Error>(); | ||
const [agentConfig, setAgentConfig] = useState<AgentConfig>(); | ||
const [packageInfo, setPackageInfo] = useState<PackageInfo>(); | ||
const [datasource, setDatasource] = useState<NewDatasource>({ | ||
const [datasource, setDatasource] = useState<NewDatasource & { id: string }>({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here for the type, if you like 😃 |
||
id: '', | ||
name: '', | ||
description: '', | ||
config_id: '', | ||
|
@@ -93,7 +94,6 @@ export const EditDatasourcePage: React.FunctionComponent = () => { | |
} | ||
if (datasourceData?.item) { | ||
const { | ||
id, | ||
revision, | ||
inputs, | ||
created_by, | ||
|
@@ -299,6 +299,7 @@ export const EditDatasourcePage: React.FunctionComponent = () => { | |
), | ||
children: ( | ||
<StepConfigureDatasource | ||
from={'edit'} | ||
packageInfo={packageInfo} | ||
datasource={datasource} | ||
updateDatasource={updateDatasource} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,3 +11,7 @@ export { IngestManagerStart } from './plugin'; | |
export const plugin = (initializerContext: PluginInitializerContext) => { | ||
return new IngestManagerPlugin(initializerContext); | ||
}; | ||
|
||
export { CustomConfigureDatasourceContent } from './applications/ingest_manager/sections/agent_config/create_datasource_page/components/custom_configure_datasource'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this exported out of Ingest? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ahh. Got it. Thank @jen-huang |
||
|
||
export { NewDatasource } from './applications/ingest_manager/types'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,3 +11,9 @@ import { PluginSetup, PluginStart } from './types'; | |
export const plugin = (context: PluginInitializerContext): Plugin => new Plugin(context); | ||
|
||
export { Plugin, PluginSetup, PluginStart }; | ||
|
||
/** | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. did you forget to move these comments to the actual component? |
||
* Exports Endpoint-specific datasource configuration instructions | ||
* for use in the Ingest app create / edit datasource config | ||
*/ | ||
export { ConfigureEndpointDatasource } from './management/pages/policy/view/ingest_manager_integration/configure_datasource'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should add JSDocs to this, since its a plugin level export. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
import React, { memo } from 'react'; | ||
import { FormattedMessage } from '@kbn/i18n/react'; | ||
import { EuiEmptyPrompt, EuiText } from '@elastic/eui'; | ||
import { useKibana } from '../../../../../../../../../src/plugins/kibana_react/public'; | ||
import { LinkToApp } from '../../../../../common/components/endpoint/link_to_app'; | ||
import { | ||
CustomConfigureDatasourceContent, | ||
NewDatasource, | ||
} from '../../../../../../../ingest_manager/public'; | ||
import { getManagementUrl } from '../../../..'; | ||
|
||
type DatasourceWithId = NewDatasource & { id: string }; | ||
|
||
/** | ||
* Exports Endpoint-specific datasource configuration instructions | ||
* for use in the Ingest app create / edit datasource config | ||
*/ | ||
export const ConfigureEndpointDatasource = memo<CustomConfigureDatasourceContent>( | ||
({ from, datasource }: { from: string; datasource: NewDatasource | DatasourceWithId }) => { | ||
const { services } = useKibana(); | ||
let policyUrl = ''; | ||
if (from === 'edit') { | ||
policyUrl = getManagementUrl({ | ||
name: 'policyDetails', | ||
policyId: (datasource as DatasourceWithId).id, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this cast safe? or should you be checking if datasource has an id? (just asking, not familiar w/ the code.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @oatkiller The cast is safe and necessary since tsserver doesn't realize that datasource does have an id if it's coming from the |
||
}); | ||
} | ||
|
||
return ( | ||
<EuiEmptyPrompt | ||
body={ | ||
<EuiText> | ||
<p> | ||
{from === 'edit' ? ( | ||
<LinkToApp | ||
appId="siem" | ||
appPath={policyUrl} | ||
href={`${services.application.getUrlForApp('siem')}${policyUrl}`} | ||
> | ||
<FormattedMessage | ||
id="xpack.siem.endpoint.ingestManager.editDatasource.stepConfigure" | ||
defaultMessage="View and configure Security Policy" | ||
/> | ||
</LinkToApp> | ||
) : ( | ||
<FormattedMessage | ||
id="xpack.siem.endpoint.ingestManager.createDatasource.stepConfigure" | ||
defaultMessage="The recommended Security Policy has been associated with this data source. The Security Policy can be edited in the Security application once your data source has been saved." | ||
/> | ||
)} | ||
</p> | ||
</EuiText> | ||
} | ||
/> | ||
); | ||
} | ||
); | ||
|
||
ConfigureEndpointDatasource.displayName = 'ConfigureEndpointDatasource'; |
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 about this comment.
NewDatasource
is the interface needed to supplement the create API for datasource and I think is reused on the server as well - so its not only a client side UI type.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.
ok i might just take that comment out