-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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 Security] Remove DataView labels update logic and override table columns instead #172615
Comments
Pinging @elastic/kibana-cloud-security-posture (Team:Cloud Security) |
…tions Findings Table (#173870) ## Summary This PR is part of the [Quick Wins](elastic/security-team#8254) improvements for 8.13.0. It enables the Edit DataView option on the Misconfigurations Findings Page. It also adds the following: - Unit tests to test the basic functionality of the `CloudSecurityDataTable` component. - Updated unit tests on the configurations page to reflect the current hook - Added deprecation notice on the `useCloudSecurityTable` hook. - FTR test to check if the option to edit DataView is enabled. - A Check on the `useLatestFindingsDataView` hook to only update when needed (This update logic is [planned](#172615) to be replaced by a server-side callback when installing the Cloud Security integration. Still, for now, it is an improvement over the current logic). ### Screenshots ![image](https://github.com/elastic/kibana/assets/19270322/b7b738a6-4b90-4c9d-af15-fc91ca4a92d4) ![image](https://github.com/elastic/kibana/assets/19270322/44649e30-7bb3-4eec-83ca-d3ec7924c527) ![image](https://github.com/elastic/kibana/assets/19270322/accf66b1-882f-4c04-837c-24c8953aba2f)
Hi @opauloh can you remind me what blocks this issue? |
Yes, the implemented solution relies on information stored in the DataViews at the moment of installing the CSP integration, but we have some FTR tests, such as this one, that erases the information on the DataViews, so a better solution is still to be found to address this issue, so I moved to blocked as we had other tasks with higher priority. |
we discussed it and we think it worth moving the FTR tests to the |
Updated the ticket to fix the sdh we have today and assigning this ticket to @animehart |
Hi @kfirpeled , it seems like the fix this issue has already been included in @opauloh PR (#176266) the only issue with that PR is that it's breaking some of our FTR as such in my opinion wouldn't it be better to just go with that PR and then create a follow up ticket or PR to address the affected FTRs ? Unless the solution on that PR is not the solution that we wanted or I'm misunderstood something |
@opauloh @kfirpeled After reviewing related PRs I realized that this is a breaking change for our users as we don't allow set custom names to the column headers anymore (before it was possible). Maybe with our current user base, it's not a big deal, but I think we should be more transparent and clear about it, the ticket description is quite technical and doesn't capture this. ![]() |
The option to "Edit data view field" was removed in a separate PR. @animehart Can you link the PR that removes the edit data view option from the Findings page data tables? |
Follow these steps to verify this ticket:
|
Verified BC 4 PermissionsMinimum Permissions Elasticsearch Minimum Permissions Kibana
User accessScreen.Recording.2024-07-25.at.9.34.19.PM.mov
|
@opauloh @kfirpeled how are the verification steps related to the issue itself? The issue talks about removing the Data View update logic, I'm not sure how it is related to the problem with permissions. I also had some confusion around the required permissions, described them in the thread https://elastic.slack.com/archives/C03E5KGNWT1/p1722002692188249 , I think we need to document the additional required privileges otherwise it might be very confusing for the users |
I was mainly focused on following the steps described here. Maybe those instructions belonged to another ticket? cc @kfirpeled But I can also confirm that this ticket itself is verified: ![]() |
The issue with access roles can be reproduced on serverless, I opened a separate ticket for it as the access control logic on Serverless is very different from ESS Marking this issue as Verified on serverless |
Motivation
Currently, the Findings data table uses the useLatestFindingsDataView hook to update the DataView fields Label when the labels are empty. This can lead to unreliability when testing, and issues with side effects due to updating data in a method that should only retrieve data.
This ticket aims, to refactor that logic and split it into two separate methods, keep the hook to only fetch the data view, and move the update logic to the Kibana server-side in the plugin initialization
Update: we wish to stop updating the data view. When doing it from the client side it requires permissions to data view management, and when the user lack these permissions the findings fails to load. see https://github.com/elastic/sdh-security-team/issues/949.
And in a second thought, the
Alerts
page doesn't update the data view, it just overrides the table's column label. We should do the same.Definition of done
Vulnerabilities and Misconfigurations DataViews labels are updated independentlyThe labels are still updatedOut of scope
Related tasks/epics
The text was updated successfully, but these errors were encountered: