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.

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

[SecuritySolution] Share getStartedPage between ESS and serverless #174867

Merged
merged 28 commits into from
Jan 24, 2024

Conversation

angorayc
Copy link
Contributor

@angorayc angorayc commented Jan 15, 2024

Summary

#174742

This PR move the get_started component from security_solution_serverless plugin to security_solution plugin, so we can share the same UI between ESS and serverless.

Parameters are set via x-pack/plugins/security_solution/public/contract_get_started_page.ts

  1. productTypes - set by serverless only
  2. projectsUrl - set by serverless only (when running serverless locally, this value is empty)
  3. projectFeaturesUrl - set by serverless only (when running serverless locally, this value is empty)
  4. availableSteps - set by both serverless and ESS (ESS doesn't contain create your first project step)

Known issue: #175296


Serverless: 6 steps in total + the first step is finished by default

serverless

ESS: 5 steps in total

Screenshot 2024-01-24 at 20 04 19

Checklist

@angorayc
Copy link
Contributor Author

buildkite test this

@angorayc angorayc changed the title init [SecuritySolution] Share getStartedPage between ESS and serverless Jan 16, 2024
@angorayc
Copy link
Contributor Author

buildkite test this

@angorayc angorayc added v8.13.0 release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Team:Threat Hunting:Explore labels Jan 16, 2024
@angorayc
Copy link
Contributor Author

buildkite test this

@angorayc
Copy link
Contributor Author

buildkite test this

@angorayc angorayc added the ci:cloud-deploy Create or update a Cloud deployment label Jan 17, 2024
@angorayc
Copy link
Contributor Author

buildkite test this

@angorayc angorayc marked this pull request as ready for review January 17, 2024 14:01
@angorayc angorayc requested review from a team as code owners January 17, 2024 14:01
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting-explore (Team:Threat Hunting:Explore)

@angorayc angorayc requested a review from semd January 17, 2024 14:02
@angorayc angorayc self-assigned this Jan 18, 2024
@pgayvallet
Copy link
Contributor

Screenshot 2024-01-22 at 08 22 10

FWIW, it's probably caused by the PR adding @kbn/config-schema to the browser-side bundle

@semd
Copy link
Contributor

semd commented Jan 24, 2024

I desk-tested the PR and I've noticed a problem in the ESS environment. The side navigation is not showing on the onboarding page:

navigation_gone

This is caused by the absolute positioning of the main onboarding component, these are styles are only work for serverless.

Probably this component should not be absolute, also, we are nesting a second KibanaPageTemplate inside the main KibanaPageTemplate (rendered SecuritySolutionTemplateWrapper), which does not seem correct either.
Consider changing the structure of onboarding.tsx to fit both serverless and ESS versions.

@angorayc
Copy link
Contributor Author

angorayc commented Jan 24, 2024

The 2nd KibanaPageTemplate in the Onboarding component is now removed:

ESS:

Screen.Recording.2024-01-24.at.21.04.46.mov

Serverless:

Screen.Recording.2024-01-24.at.20.56.50.mov

@kibana-ci
Copy link
Collaborator

kibana-ci commented Jan 24, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #48 / Observability Rules Rules Endpoints Custom Threshold rule - DOCUMENTS_COUNT - FIRED Rule creation should set correct information in the alert document

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 4876 4945 +69
securitySolutionEss 102 96 -6
securitySolutionServerless 322 262 -60
total +3

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
securitySolution 117 124 +7

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 11.1MB 11.2MB +82.1KB
securitySolutionEss 44.5KB 39.4KB -5.1KB
securitySolutionServerless 220.5KB 156.5KB -64.0KB
total +13.0KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
securitySolution 36 37 +1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
securitySolution 65.3KB 68.4KB +3.0KB
securitySolutionEss 6.4KB 6.4KB -60.0B
securitySolutionServerless 35.6KB 35.5KB -176.0B
total +2.8KB
Unknown metric groups

API count

id before after diff
securitySolution 186 193 +7

async chunk count

id before after diff
securitySolution 50 51 +1
securitySolutionEss 2 1 -1
securitySolutionServerless 40 39 -1
total -1

ESLint disabled line counts

id before after diff
securitySolution 470 472 +2
securitySolutionEss 5 4 -1
securitySolutionServerless 35 33 -2
total -1

miscellaneous assets size

id before after diff
securitySolution 2.1MB 2.5MB ⚠️ +366.1KB
securitySolutionEss 4.4MB 2.3MB -2.0MB
securitySolutionServerless 2.7MB 2.3MB -366.1KB
total -2.0MB

Total ESLint disabled count

id before after diff
securitySolution 542 544 +2
securitySolutionEss 5 4 -1
securitySolutionServerless 35 33 -2
total -1

History

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

cc @angorayc

Copy link
Contributor

@semd semd left a comment

Choose a reason for hiding this comment

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

LGTM! thanks for doing this. great work!

@angorayc angorayc enabled auto-merge (squash) January 24, 2024 14:20
@semd
Copy link
Contributor

semd commented Jan 24, 2024

Linking a follow-up task to make the onboarding page component agnostic, and move the specific serverless/ess code to their appropriate plugins: #174766

@angorayc
Copy link
Contributor Author

angorayc commented Jan 24, 2024

I've also put all the follow up tasks in this issue: #174742

@angorayc angorayc merged commit 7d04132 into elastic:main Jan 24, 2024
38 checks passed
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
…lastic#174867)

## Summary

elastic#174742

This PR move the get_started component from
`security_solution_serverless` plugin to `security_solution` plugin, so
we can share the same UI between ESS and serverless.

Parameters are set via
`x-pack/plugins/security_solution/public/contract_get_started_page.ts`
1. productTypes - set by serverless only
2. projectsUrl - set by serverless only (when running serverless
locally, this value is empty)
3. projectFeaturesUrl - set by serverless only (when running serverless
locally, this value is empty)
4. availableSteps - set by both serverless and ESS (ESS doesn't contain
`create your first project` step)

Known issue: elastic#175296

---

#### Serverless: 6 steps in total + the first step is finished by
default


![serverless](https://github.com/elastic/kibana/assets/6295984/8bbf6557-8c8e-42c6-843b-fc24ac1dd178)


#### ESS: 5 steps in total

![Screenshot 2024-01-24 at 20 04
19](https://github.com/elastic/kibana/assets/6295984/6486916a-9976-4fb5-bf07-721ba4d411aa)

### Checklist



- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: kibanamachine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting ci:cloud-deploy Create or update a Cloud deployment needs_docs release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting:Explore v8.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants