Skip to content
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.

Sign up for GitHub

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

[TIP] Enables TI plugin with kibana.yml feature flag #137838

Merged
merged 5 commits into from
Aug 9, 2022
Merged

[TIP] Enables TI plugin with kibana.yml feature flag #137838

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e6ca139
[TIP] Enables TI plugin with kibana.yml feature flag
PhilippeOberti Aug 2, 2022
5bfd9bc
[TIP] Add config entry for e2e Cypress tests
PhilippeOberti Aug 2, 2022
346e397
[TIP] Use experimentalKey feature instead of filtering out entry manu…
PhilippeOberti Aug 4, 2022
815f876
[TIP] Simplified TI plugin logic
PhilippeOberti Aug 4, 2022
0b2e554
Merge branch 'main' into tip-feature-flag
kibanamachine Aug 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion x-pack/plugins/security_solution/common/experimental_features.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,10 @@ export const allowedExperimentalValues = Object.freeze({
pendingActionResponsesWithAck: true,
policyListEnabled: true,
policyResponseInFleetEnabled: true,
threatIntelligenceEnabled: false,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure I was allowed to use this object but it seemed to make sense, as the Threat Intelligence plugin is loaded within the Security Solution plugin


/**
* This is used for enabling the end to end tests for the security_solution telemetry.
* This is used for enabling the end-to-end tests for the security_solution telemetry.
* We disable the telemetry since we don't have specific roles or permissions around it and
* we don't want people to be able to violate security by getting access to whole documents
* around telemetry they should not.
Expand Down
10 changes: 0 additions & 10 deletions .../components/navigation/use_security_solution_navigation/__snapshots__/index.test.tsx.snap
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions ...y_solution/public/common/components/navigation/use_security_solution_navigation/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ export const useSecuritySolutionNavigation = () => {

const disabledNavTabs = [
...(!useIsExperimentalFeatureEnabled('kubernetesEnabled') ? ['kubernetes'] : []),
...(!useIsExperimentalFeatureEnabled('threatIntelligenceEnabled')
? ['threat-intelligence']
: []),
];
const enabledNavTabs: GenericNavRecord = omit(disabledNavTabs, navTabs);

Expand Down
4 changes: 3 additions & 1 deletion ...ic/common/components/navigation/use_security_solution_navigation/use_navigation_items.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,9 @@ function usePrimaryNavigationItemsToDisplay(navTabs: Record<string, NavTab>) {
...(navTabs[SecurityPageName.users] != null
? [navTabs[SecurityPageName.users]]
: []),
navTabs[SecurityPageName.threatIntelligence],
...(navTabs[SecurityPageName.threatIntelligence] != null
? [navTabs[SecurityPageName.threatIntelligence]]
: []),
],
},
{
Expand Down
18 changes: 15 additions & 3 deletions x-pack/plugins/security_solution/public/common/links/app_links.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
* 2.0.
*/
import type { CoreStart } from '@kbn/core/public';
import type { AppLinkItems } from './types';
import { links as threatIntelligenceLinks } from '../../threat_intelligence/links';
import type { ExperimentalFeatures } from '../../../common/experimental_features';
import type { AppLinkItems, LinkItem } from './types';
import { links as detectionLinks } from '../../detections/links';
import { links as timelinesLinks } from '../../timelines/links';
import { getCasesLinkItems } from '../../cases/links';
Expand All @@ -30,17 +32,27 @@ export const links = Object.freeze([

export const getFilteredLinks = async (
core: CoreStart,
plugins: StartPlugins
plugins: StartPlugins,
experimentalFeatures: Readonly<ExperimentalFeatures>
): Promise<AppLinkItems> => {
const managementFilteredLinks = await getManagementFilteredLinks(core, plugins);

const threatHuntingFilteredLinks = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hesitated to go the opposite way (adding the threatIntelligence entry to the threatHuntingLandingLinks.link property) but going this current route meant modifying one less file.

Copy link
Contributor

Choose a reason for hiding this comment

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

no need to do this, links have support for experimentalFeatures, you can add the experimentalKey: 'threatIntelligenceEnabled', prop directly to the threat intelligence link definition at: https://github.com/elastic/kibana/blob/main/x-pack/plugins/security_solution/public/threat_intelligence/links.ts

Copy link
Contributor Author

@PhilippeOberti PhilippeOberti Aug 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

haha I had took a note of this experimentalKey while working on another feature, tried it and didn't get it to work... I'll try again thank you :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah that worked, I don't know what I did... must have forgotten to update my kibana.yml... 🤦‍♂️

...threatHuntingLandingLinks,
links: !experimentalFeatures.threatIntelligenceEnabled
? threatHuntingLandingLinks.links?.filter(
(p: LinkItem) => p.id !== threatIntelligenceLinks.id
)
: threatHuntingLandingLinks.links,
};

return Object.freeze([
dashboardsLandingLinks,
detectionLinks,
cloudSecurityPostureRootLinks,
timelinesLinks,
casesLinks,
threatHuntingLandingLinks,
threatHuntingFilteredLinks,
gettingStartedLinks,
managementFilteredLinks,
]);
Expand Down
20 changes: 16 additions & 4 deletions x-pack/plugins/security_solution/public/plugin.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import type {
import { DEFAULT_APP_CATEGORIES, AppNavLinkStatus } from '@kbn/core/public';
import { Storage } from '@kbn/kibana-utils-plugin/public';
import type { TimelineState } from '@kbn/timelines-plugin/public';
import type { ThreatIntelligence } from './threat_intelligence';
import type {
PluginSetup,
PluginStart,
Expand Down Expand Up @@ -331,8 +332,11 @@ export class Plugin implements IPlugin<PluginSetup, PluginStart, SetupPlugins, S
management: new subPluginClasses.Management(),
landingPages: new subPluginClasses.LandingPages(),
cloudSecurityPosture: new subPluginClasses.CloudSecurityPosture(),
threatIntelligence: new subPluginClasses.ThreatIntelligence(),
};

if (this.experimentalFeatures.threatIntelligenceEnabled) {
this._subPlugins.threatIntelligence = new subPluginClasses.ThreatIntelligence();
}
}
return this._subPlugins;
}
Expand All @@ -346,7 +350,8 @@ export class Plugin implements IPlugin<PluginSetup, PluginStart, SetupPlugins, S
plugins: StartPlugins
): Promise<StartedSubPlugins> {
const subPlugins = await this.subPlugins();
return {

const startPlugins: StartedSubPlugins = {
overview: subPlugins.overview.start(),
alerts: subPlugins.alerts.start(storage),
cases: subPlugins.cases.start(),
Expand All @@ -360,8 +365,15 @@ export class Plugin implements IPlugin<PluginSetup, PluginStart, SetupPlugins, S
management: subPlugins.management.start(core, plugins),
landingPages: subPlugins.landingPages.start(),
cloudSecurityPosture: subPlugins.cloudSecurityPosture.start(),
threatIntelligence: subPlugins.threatIntelligence.start(),
};

if (this.experimentalFeatures.threatIntelligenceEnabled) {
startPlugins.threatIntelligence = (
subPlugins.threatIntelligence as ThreatIntelligence
).start();
}

return startPlugins;
}
/**
* Lazily instantiate a `SecurityAppStore`. We lazily instantiate this because it requests large dynamic imports. We instantiate it once because each subPlugin needs to share the same reference.
Expand Down Expand Up @@ -513,7 +525,7 @@ export class Plugin implements IPlugin<PluginSetup, PluginStart, SetupPlugins, S
updateAppLinks(links, linksPermissions);

// set filtered links asynchronously
const filteredLinks = await getFilteredLinks(core, plugins);
const filteredLinks = await getFilteredLinks(core, plugins, this.experimentalFeatures);
updateAppLinks(filteredLinks, linksPermissions);
});
}
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/security_solution/public/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ export interface SubPlugins {
management: Management;
landingPages: LandingPages;
cloudSecurityPosture: CloudSecurityPosture;
threatIntelligence: ThreatIntelligence;
threatIntelligence?: ThreatIntelligence; // the Threat Intelligence plugin is temporarily hidden behind a feature flag
}

// TODO: find a better way to defined these types
Expand All @@ -148,5 +148,5 @@ export interface StartedSubPlugins {
management: ReturnType<Management['start']>;
landingPages: ReturnType<LandingPages['start']>;
cloudSecurityPosture: ReturnType<CloudSecurityPosture['start']>;
threatIntelligence: ReturnType<ThreatIntelligence['start']>;
PhilippeOberti marked this conversation as resolved.
Show resolved Hide resolved
threatIntelligence?: ReturnType<ThreatIntelligence['start']>;
}
1 change: 1 addition & 0 deletions x-pack/test/security_solution_cypress/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export default async function ({ readConfigFile }: FtrConfigProviderContext) {
'riskyHostsEnabled',
'riskyUsersEnabled',
'insightsRelatedAlertsByProcessAncestry',
'threatIntelligenceEnabled',
Copy link
Member

Choose a reason for hiding this comment

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

Is there any plan of adding specific TI plugin tests on Security Solution Cypress tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MadameSheema we have some e2e Cypress tests in our plugin directly and this PR updates some e2e Cypress tests within the Security Solution plugin.

I wasn't thinking about adding any other tests, especially because this feature flag is temporary and should be very short lived. We're planning on releasing everything in 8.5 (plans could change though).

Did you have anything specify in mind you wanted me/us to add?

])}`,
`--home.disableWelcomeScreen=true`,
],
Expand Down
1 change: 1 addition & 0 deletions x-pack/test/threat_intelligence_cypress/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export default async function ({ readConfigFile }: FtrConfigProviderContext) {
`--xpack.securitySolution.enableExperimental=${JSON.stringify([
'riskyHostsEnabled',
'riskyUsersEnabled',
'threatIntelligenceEnabled',
])}`,
`--home.disableWelcomeScreen=true`,
],
Expand Down