Skip to content

wiz: fix result.evaluation to be lowercased in cloud_configuration_finding - #10914

Merged
maxcold merged 3 commits into
elastic:mainfrom
maxcold:csp-fix-result-evaluation
Aug 30, 2024
Merged

wiz: fix result.evaluation to be lowercased in cloud_configuration_finding#10914
maxcold merged 3 commits into
elastic:mainfrom
maxcold:csp-fix-result-evaluation

Conversation

@maxcold

@maxcold maxcold commented Aug 28, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Proposed commit message

Cloud Security features in Kibana rely on the lowercase value of result.evaluation. Fixing cloud_configuration_finding ingest pipeline to provide the lowercased values

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

Author's Checklist

  • [ ]

How to test this PR locally

see #10790

Related issues

Screenshots
@maxcold maxcold added Team:Cloud Security Cloud Security team [elastic/cloud-security-posture] bugfix Pull request that fixes a bug issue labels Aug 28, 2024
@maxcold
maxcold requested a review from a team August 28, 2024 11:24
@JordanSh

JordanSh commented Aug 28, 2024

Copy link
Copy Markdown
Contributor

what do you think about changing our code to turn upper case to lower case where needed? just so 3Ps can have easier time integrating
@maxcold

maxcold commented Aug 28, 2024

Copy link
Copy Markdown
Contributor Author

@JordanSh I considered it, we can maybe add it as a safety net, but my concern is that this duality will cause more headaches to the users. While we can update our code everywhere where we rely on the result.evaluation to lowercase the value, the Wiz data stream can be used not only from our flows. Eg. users can create dashboards, detection rules manually, etc. And they will need to remember that the value can be both lowercase and uppercase and fix it in each artifact they create. Eg. all detection rules will need to check for both values. This is not a good UX in my opinion. So I prefer to be more strict with the data schema.
@elasticmachine

elasticmachine commented Aug 28, 2024 •
edited
Loading

Copy link
Copy Markdown

🚀 Benchmarks report

Package wiz 👍(2) 💚(0) 💔(2)

Expand to view
Data stream Previous EPS New EPS Diff (%) Result
cloud_configuration_finding 3636.36 2976.19 -660.17 (-18.15%) 💔
issue 3484.32 2053.39 -1430.93 (-41.07%) 💔

To see the full report comment with /test benchmark fullreport
@maxcold
maxcold marked this pull request as ready for review August 28, 2024 11:47
@maxcold
maxcold requested a review from a team as a code owner August 28, 2024 11:47
@andrewkroh andrewkroh added the Team:Security-Service Integrations Security Service Integrations team [elastic/security-service-integrations] label Aug 28, 2024
@elasticmachine

elasticmachine commented Aug 28, 2024

Copy link
Copy Markdown

Pinging @elastic/security-service-integrations (Team:Security-Service Integrations)
opauloh
opauloh approved these changes Aug 28, 2024
View reviewed changes

@opauloh opauloh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm, cc @animehart
@efd6 efd6 changed the title [Cloud Security] fix result.evaluation to be lowercased Aug 28, 2024
efd6
efd6 reviewed Aug 28, 2024
View reviewed changes

@efd6 efd6 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see that the relevant field here existed prior to this change, but is not an ECS field. I do not see any discussion about introduction on a non-ECS field in the original change. Is there background for that?
Comment thread packages/wiz/changelog.yml Outdated
@maxcold @efd6
Update packages/wiz/changelog.yml
3425945 
Co-authored-by: Dan Kortschak <dan.kortschak@elastic.co>
@maxcold

maxcold commented Aug 29, 2024

Copy link
Copy Markdown
Contributor Author

@efd6 the field result.evaluation is used by cloud_security_posture integration (the integration Cloud Security team owns) and Cloud Security UX in Kibana rely on this field heavily. You can find more context in this document https://docs.google.com/document/d/1govaQj_8LwUOLtk3zrcjCEvRgyKWRuC-TUihCrk04ao/edit
@elasticmachine

elasticmachine commented Aug 29, 2024

Copy link
Copy Markdown

💚 Build Succeeded

History
@elastic-sonarqube

elastic-sonarqube Bot commented Aug 29, 2024

Copy link
Copy Markdown

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube
@maxcold
maxcold merged commit 21d32e9 into elastic:main Aug 30, 2024
@maxcold
maxcold deleted the csp-fix-result-evaluation branch August 30, 2024 07:36
@elasticmachine

elasticmachine commented Aug 30, 2024

Copy link
Copy Markdown

Package wiz - 1.7.1 containing this change is available at https://epr.elastic.co/search?package=wiz
harnish-crest-data pushed a commit to chavdaharnish/integrations that referenced this pull request Feb 4, 2025
@maxcold @efd6
wiz: fix result.evaluation to be lowercased in cloud_configuration_fi…
ba6891e 
…nding (elastic#10914)

* fix result.evaluation to be lowercased

* add PR link to the changelog

* Update packages/wiz/changelog.yml

Co-authored-by: Dan Kortschak <dan.kortschak@elastic.co>

---------

Co-authored-by: Dan Kortschak <dan.kortschak@elastic.co>
harnish-crest-data pushed a commit to chavdaharnish/integrations that referenced this pull request Feb 5, 2025
@maxcold @efd6
wiz: fix result.evaluation to be lowercased in cloud_configuration_fi…
4c8b105 
…nding (elastic#10914)

* fix result.evaluation to be lowercased

* add PR link to the changelog

* Update packages/wiz/changelog.yml

Co-authored-by: Dan Kortschak <dan.kortschak@elastic.co>

---------

Co-authored-by: Dan Kortschak <dan.kortschak@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Pull request that fixes a bug issue Integration:wiz Wiz Team:Cloud Security Cloud Security team [elastic/cloud-security-posture] Team:Security-Service Integrations Security Service Integrations team [elastic/security-service-integrations]

6 participants

@maxcold @JordanSh @elasticmachine @opauloh @efd6 @andrewkroh