[Cloud Security] Misconfiguration preview & Refactor CSP Plugin to include new package PHASE 4 by animehart · Pull Request #191677 · elastic/kibana

animehart · 2024-08-28T20:33:44Z

The previous #190105 was way too big and made it hard to review without missing any bugs or potential bugs, Thus we decided we are going to make series of smaller PR to make things more manageable

We will be splitting it into 4 PR
Phase 1: Creating empty packages for csp and csp-common
Phase 2: Move Types from CSP plugin to the Package + Deleting duplicates in the CSP plugin where possible
Phase 3: Move Functions, Utils or Helpers, Hooks to Package
Phase 4: Misconfiguration Preview feature (with Cypress test and other required test)

353329193-5ad22c4e-81c2-4a8b-89f7-fdbc2a686c2d

This is Phase 4 of the Process,

elasticmachine · 2024-08-28T20:38:29Z

Pinging @elastic/kibana-cloud-security-posture (Team:Cloud Security)

…fix'

…s on text

tiansivive

LGTM from Entity Analytics

…fix'

…plementation

maxcold · 2024-08-30T11:14:27Z

/ci

…n' of github.com:animehart/kibana into misconfiguration-preview-refactor-phase-4-implementation

MadameSheema · 2024-08-30T16:14:09Z

From Cypress perspective there is work pending to be done to merge this PR. As it is, this test is not going to be executed in any of our existing pipelines:

Is missing the integration with buildkite for both PRs and periodic pipeline.
Is missing the integration with the flaky test suite runner.
We need to make sure that the team understands the best practices and how to react in case of failure in any of the different stages.

Please feel free to setup a meeting with @elastic/security-engineering-productivity to let you know which are the correct steps to make this happen.

MadameSheema · 2024-08-30T16:18:30Z

+    createMockFinding();
+    expandFirstAlertHostFlyout();
+    cy.log('check if Misconfiguration preview section rendered');
+    cy.get(HOST_INSIGHT_MISCONFIGURATION).should('exist');


For robustness, is better to check that the element is visible, since the element can exist on the dom but can be not visible for the end user.

MadameSheema · 2024-08-30T16:18:41Z

+    cy.get(HOST_INSIGHT_MISCONFIGURATION).should('exist');
+
+    cy.log('check if Misconfiguration preview title shown');
+    cy.get(HOST_INSIGHT_MISCONFIGURATION_TITLE).should('exist');


https://github.com/elastic/kibana/pull/191677/files#r1739057369

animehart · 2024-08-30T16:31:02Z

I will move the cypress test in separate ticket/PR then as it seems that there's quite a lot of stuff to do (too much for this 1 PR)

…eeds to be done before we are able to run cypress on ci, will do this on separate PR later on

…plementation

…eld names

…n' of github.com:animehart/kibana into misconfiguration-preview-refactor-phase-4-implementation

…plementation

opauloh

just a few more comments

opauloh · 2024-09-04T00:56:11Z

+) => {
+  const passed = buckets.find((bucket) => bucket?.key === 'passed');
+  const failed = buckets.find((bucket) => bucket?.key === 'failed');
+  const noStatus = buckets.find((bucket) => bucket?.key === 'unknown');


it seems inconsistent to use noStatus pointing to unknown, I think it could be unknown instead.

opauloh · 2024-09-04T01:00:34Z

+        passed: { match: { 'result.evaluation': 'passed' } },
+        failed: { match: { 'result.evaluation': 'failed' } },
+      },
+    },
+  },
+});
+
+export const getMisconfigurationAggregationCount = (
+  buckets: Array<estypes.AggregationsStringRareTermsBucketKeys | undefined>
+) => {
+  const passed = buckets.find((bucket) => bucket?.key === 'passed');
+  const failed = buckets.find((bucket) => bucket?.key === 'failed');
+  const noStatus = buckets.find((bucket) => bucket?.key === 'unknown');


better to avoid those magic strings to avoid potential typos and improve maintainability, what about defining those status in a constant:

const RESULT_EVALUATION = { PASSED: 'passed', FAILED: 'failed', UNKNOWN: 'unknown', }; export const getFindingsCountAggQueryMisconfigurationPreview = () => ({ count: { filters: { other_bucket_key: RESULT_EVALUATION.UNKNOWN, filters: { [RESULT_EVALUATION.PASSED]: { match: { 'result.evaluation': RESULT_EVALUATION.PASSED } }, [RESULT_EVALUATION.FAILED]: { match: { 'result.evaluation': RESULT_EVALUATION.FAILED } }, }, }, }, });

nit: maybe this can be improved to use a single .reducer function instead of 3 .find functions.

opauloh · 2024-09-04T01:24:17Z

+          <EuiTitle
+            size="s"
+            css={css`
+              font-weight: ${euiTheme.font.weight.bold};


isn't EuiTitle bold by default?

opauloh · 2024-09-04T01:29:42Z

+        count: getMisconfigurationAggregationCount(
+          Object.entries(aggregations.count.buckets).map(([key, value]) => ({
+            key,
+            doc_count: value.doc_count || 0,
+          }))
+        ),


since we already have a getMisconfigurationAggregationCount function, it's better to have it handle the object parsing so the function is easier to consume:

return { count: getMisconfigurationAggregationCount(aggregations.count.buckets) };

opauloh · 2024-09-04T01:33:41Z

+          id="observedEntity-accordion"
+          data-test-subj="entityInsightTestSubj"
+          buttonProps={{
+            'data-test-subj': 'observedEntity-accordion-button',


better avoid using existing identifiers:

Suggested change

id="observedEntity-accordion"

data-test-subj="entityInsightTestSubj"

buttonProps={{

'data-test-subj': 'observedEntity-accordion-button',

id="entityInsight-accordion"

data-test-subj="entityInsightTestSubj"

buttonProps={{

'data-test-subj': 'entityInsight-accordion-button',

opauloh · 2024-09-04T01:34:20Z

+            css={css`
+              font-weight: ${euiTheme.font.weight.bold};
+            `}


same question about the EuiTitle

…n' of github.com:animehart/kibana into misconfiguration-preview-refactor-phase-4-implementation

…plementation

kibana-ci · 2024-09-04T09:41:12Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 842c829
Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-191677-842c8296fafc
Security Deployment

Failed CI Steps

Test Failures

[job] [logs] Jest Tests #8 / CasesParamsFields renders all params fields are rendered

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	5660	5673	+13

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
`@kbn/cloud-security-posture-common`	49	52	+3

Async chunks

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

id	before	after	diff
`securitySolution`	20.7MB	20.7MB	+33.5KB

Unknown metric groups

API count

id	before	after	diff
`@kbn/cloud-security-posture-common`	49	52	+3

History

💔 Build #231848 failed 1f95ac2
💛 Build #231778 was flaky 6609c40
💛 Build #231275 was flaky def06a5
💔 Build #231247 failed 3ddc7b7

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

maxcold

Small visual bug, otherwise looks good. Please don't forget to add a work item of adding e2e cypress tests in case you don't plan to do it right away in the follow up PR

…lugin PHASE 1 (#192114) ## Summary In an attempt to make Reviewing easier and more accurate, the implementation of Misconfiguration Data grid on Host.name flyout in Alerts Page will be split into 2 Phases Phase 1: Move Functions, Utils or Helpers, Hooks, constants to Package Phase 2: Implementing the feature This is Phase 1 of the process This PR also include a small bug fix mentioned here #191677 (review) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

re-Added Misconfiguration Preview Component as well the test

61cc974

animehart marked this pull request as ready for review August 28, 2024 20:35

animehart requested review from a team as code owners August 28, 2024 20:35

animehart requested a review from tiansivive August 28, 2024 20:35

animehart added v8.16.0 Team:Cloud Security Cloud Security team related labels Aug 28, 2024

animehart added the release_note:skip Skip the PR/issue when compiling release notes label Aug 28, 2024

kibanamachine added 2 commits August 28, 2024 20:47

[CI] Auto-commit changed files from 'node scripts/notice'

89eb0e7

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

199b8d0

…fix'

elastic-vault-github-plugin-prod Bot requested a review from a team as a code owner August 28, 2024 20:59

animehart added 3 commits August 28, 2024 14:56

added jest config file

0aa00d8

will include test later

a683d21

addressing comments from original PR about convention on setting size…

750fa9b

…s on text

tiansivive approved these changes Aug 29, 2024

View reviewed changes

opauloh reviewed Aug 30, 2024

View reviewed changes

animehart added 2 commits August 29, 2024 22:06

addressing pr comments

0423a29

add more cypress stuff

a31584c

animehart requested a review from opauloh August 30, 2024 07:01

animehart and others added 4 commits August 30, 2024 00:12

fix cypress log message

1d6b486

update type

f5656f6

update cypress test to be more stable

574f2c4

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

292a222

…fix'

elastic-vault-github-plugin-prod Bot requested a review from a team as a code owner August 30, 2024 08:55

Merge branch 'main' into misconfiguration-preview-refactor-phase-4-im…

62379d0

…plementation

maxcold added the ci:project-deploy-security Create a Security Serverless Project label Aug 30, 2024

maxcold reviewed Aug 30, 2024

View reviewed changes

more PR Comments, renaming file name and fixes

a303ddf

Merge branch 'misconfiguration-preview-refactor-phase-4-implementatio…

733111e

…n' of github.com:animehart/kibana into misconfiguration-preview-refactor-phase-4-implementation

MadameSheema reviewed Aug 30, 2024

View reviewed changes

animehart added 6 commits August 30, 2024 10:13

removed Cypress test for now, because there are multiple steps that n…

cae20ab

…eeds to be done before we are able to run cypress on ci, will do this on separate PR later on

fix tsconfig

3ddc7b7

Merge branch 'main' into misconfiguration-preview-refactor-phase-4-im…

def06a5

…plementation

addressing Max comments: removing useless field and renaming weird fi…

fb41b2f

…eld names

Merge branch 'misconfiguration-preview-refactor-phase-4-implementatio…

60bc1bf

…n' of github.com:animehart/kibana into misconfiguration-preview-refactor-phase-4-implementation

Merge branch 'main' into misconfiguration-preview-refactor-phase-4-im…

6609c40

…plementation

opauloh reviewed Sep 4, 2024

View reviewed changes

animehart added 2 commits September 4, 2024 00:04

addressing pr comments

3d5b126

Merge branch 'misconfiguration-preview-refactor-phase-4-implementatio…

1f95ac2

…n' of github.com:animehart/kibana into misconfiguration-preview-refactor-phase-4-implementation

animehart requested a review from opauloh September 4, 2024 07:05

Merge branch 'main' into misconfiguration-preview-refactor-phase-4-im…

842c829

…plementation

maxcold approved these changes Sep 4, 2024

View reviewed changes

animehart merged commit 8b7d965 into elastic:main Sep 4, 2024

kibanamachine added the backport:skip This PR does not require backporting label Sep 4, 2024

animehart mentioned this pull request Sep 4, 2024

[Cloud Security] Host Name Misconfiguration Datagrid & Refactor CSP Plugin PHASE 1 #192114

Merged

Conversation

animehart commented Aug 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Aug 28, 2024

Uh oh!

tiansivive left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maxcold commented Aug 30, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MadameSheema commented Aug 30, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

animehart commented Aug 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

opauloh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kibana-ci commented Sep 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

API count

History

Uh oh!

maxcold left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

animehart commented Aug 28, 2024 •

edited

Loading

animehart commented Aug 30, 2024 •

edited

Loading

kibana-ci commented Sep 4, 2024 •

edited

Loading