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

[Cloud Posture] add resource findings table #131334

Merged
merged 3 commits into from
May 3, 2022

Conversation

orouz
Copy link
Contributor

@orouz orouz commented May 2, 2022

Summary

this PR adds the table component for the /findings/resource/:resourceId route.

it includes:

  • table component with shared columns from group_by_none
  • api hook to query es with hidden filter for resource_id: resourceId

Demo

Screen.Recording.2022-05-02.at.18.08.18.mov

Checklist

Delete any items that are not applicable to this PR.

@tinnytintin10
Copy link

tinnytintin10 commented May 2, 2022

@orouz can you please update the System ID column to be Cluster ID in the resource view table cc @kfirpeled

@orouz
Copy link
Contributor Author

orouz commented May 2, 2022

can you please update the System ID column to be Cluster ID in the resource view table

@tinnytintin10 should this change affect both group-by-none and resource-view tables or just the latter ?

@tinnytintin10
Copy link

can you please update the System ID column to be Cluster ID in the resource view table

@tinnytintin10 should this change affect both group-by-none and resource-view tables or just the latter ?

yes, it should be reflected on both. ty!

@orouz orouz force-pushed the resource_findings_table branch from 455d1de to 645b8c5 Compare May 3, 2022 10:17
@orouz orouz force-pushed the resource_findings_table branch from 645b8c5 to 4ece26a Compare May 3, 2022 13:26
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
cloudSecurityPosture 243 245 +2

Async chunks

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

id before after diff
cloudSecurityPosture 392.2KB 393.4KB +1.2KB

History

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

Comment on lines +42 to +46
const columns: [
EuiTableActionsColumnType<CspFinding>,
...Array<EuiBasicTableColumn<CspFinding>>
] = useMemo(
() => [getExpandColumn<CspFinding>({ onClick: setSelectedFinding }), ...getFindingsColumns()],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed because columns are partially shared (for now) between group-by-none and resource-findings

Comment on lines +31 to +38
query: {
...query,
bool: {
...query?.bool,
filter: [...(query?.bool?.filter || []), { term: { 'resource_id.keyword': resourceId } }],
},
},
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is where we say this query is limited to a resourceId

might change this later to just be merge(a,b) to read nicer

@orouz orouz marked this pull request as ready for review May 3, 2022 14:53
@orouz orouz requested a review from a team as a code owner May 3, 2022 14:53
@orouz orouz added release_note:skip Skip the PR/issue when compiling release notes Team:Cloud Security Cloud Security team related v8.3.0 labels May 3, 2022
@elasticmachine
Copy link
Contributor

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

width: '40px',
actions: [
{
name: 'Expand',
Copy link
Contributor

@kfirpeled kfirpeled May 3, 2022

Choose a reason for hiding this comment

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

should be localized (both name and description)

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 think it's OK to merge as is and fix this in the next PR which is rebased on this one, and i'd like to rebase it on upstream

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@orouz orouz merged commit 687aad0 into elastic:main May 3, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label May 3, 2022
academo pushed a commit to academo/kibana that referenced this pull request May 4, 2022
kertal pushed a commit to kertal/kibana that referenced this pull request May 24, 2022
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 release_note:skip Skip the PR/issue when compiling release notes Team:Cloud Security Cloud Security team related v8.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants