[Cloud Security] Metering integration tests#187219
Conversation
|
Pinging @elastic/kibana-cloud-security-posture (Team:Cloud Security) |
maxcold
left a comment
There was a problem hiding this comment.
Need a bit more time for the full review (though don't be blocked if someone else from the team gets to it first)
One question - have you checked the discussion about the MSW usage in #184555 ? Even though these are different test suite, it would be good to have a consistent approach to the MSW usage in our suits across the board
...n/test_suites/security/cloud_security_posture/serverless_metering/cloud_security_metering.ts
Outdated
Show resolved
Hide resolved
...n/test_suites/security/cloud_security_posture/serverless_metering/cloud_security_metering.ts
Outdated
Show resolved
Hide resolved
...n/test_suites/security/cloud_security_posture/serverless_metering/cloud_security_metering.ts
Show resolved
Hide resolved
Yes, those are different use cases, in #184555 it mocks responses and does not create a real server, here, since the task manager is running on another node worker (there is one worker for the test and another for the server) it cannot intercept the requests. That's the reason I used MSW middleware. |
💔 Build FailedFailed CI StepsHistoryTo update your PR or re-run it, just comment with: |
...plugins/security_solution_serverless/server/cloud_security/defend_for_containers_metering.ts
Show resolved
Hide resolved
...n/test_suites/security/cloud_security_posture/serverless_metering/cloud_security_metering.ts
Show resolved
Hide resolved
| body: JSON.stringify(records), | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| agent, | ||
| agent: isHttps ? agent : undefined, // Conditionally add agent if URL is HTTPS for supporting integration tests. |
There was a problem hiding this comment.
I am curious why we used fetch instead of the core HTTP service?
There was a problem hiding this comment.
I guess the reason is that the agent attribute is not supported in the core fetch. @joeypoon, please correct me if I'm wrong.
seanrathier
left a comment
There was a problem hiding this comment.
I have not had the opportunity to develop any large FTRs but this looks clean to me. I'll approve knowing that @maxcold 's concerns about MKI test will be resolved.
| body: JSON.stringify(records), | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| agent, | ||
| agent: isHttps ? agent : undefined, // Conditionally add agent if URL is HTTPS for supporting integration tests. |
There was a problem hiding this comment.
I am curious why we used fetch instead of the core HTTP service?
| ) { | ||
| const version = '1.2.5'; | ||
|
|
||
| const { body: postPackageResponse } = |
There was a problem hiding this comment.
can we move parts which are same to own variables ? eg. the object inside send looks pretty much the same in both code branches, probably can be extracted
| range: { | ||
| 'event.ingested': { | ||
| gt: searchFrom.toISOString(), | ||
| // gt: searchFrom.toISOString(), |
There was a problem hiding this comment.
I'd rather add the tech debt ticket to the comment, than leaving the old code commented out
There was a problem hiding this comment.
I solved it in #186777, so I remove the comment, and will merge the other PR first.
⏳ Build in-progress
History
|
## Summary Assigns ownership of new dev dep `@mswjs/http-middleware` to cloud security team (`kibana-cloud-security-posture`). Dependency was addded in #187219
## Summary Assigns ownership of new dev dep `@mswjs/http-middleware` to cloud security team (`kibana-cloud-security-posture`). Dependency was addded in elastic#187219 (cherry picked from commit c426b06)
# Backport This will backport the following commits from `main` to `8.x`: - [Adds @mswjs/http-middleware to renovate.json (#193257)](#193257) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Jeramy Soucy","email":"jeramy.soucy@elastic.co"},"sourceCommit":{"committedDate":"2024-09-18T13:44:53Z","message":"Adds @mswjs/http-middleware to renovate.json (#193257)\n\n## Summary\r\n\r\nAssigns ownership of new dev dep `@mswjs/http-middleware` to cloud\r\nsecurity team (`kibana-cloud-security-posture`).\r\n\r\nDependency was addded in https://github.com/elastic/kibana/pull/187219","sha":"c426b06aa442935d02cf171da66212ddce5e5ce9","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["chore","Team:Security","release_note:skip","v9.0.0","v8.16.0","backport:version"],"title":"Adds @mswjs/http-middleware to renovate.json","number":193257,"url":"https://github.com/elastic/kibana/pull/193257","mergeCommit":{"message":"Adds @mswjs/http-middleware to renovate.json (#193257)\n\n## Summary\r\n\r\nAssigns ownership of new dev dep `@mswjs/http-middleware` to cloud\r\nsecurity team (`kibana-cloud-security-posture`).\r\n\r\nDependency was addded in https://github.com/elastic/kibana/pull/187219","sha":"c426b06aa442935d02cf171da66212ddce5e5ce9"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/193257","number":193257,"mergeCommit":{"message":"Adds @mswjs/http-middleware to renovate.json (#193257)\n\n## Summary\r\n\r\nAssigns ownership of new dev dep `@mswjs/http-middleware` to cloud\r\nsecurity team (`kibana-cloud-security-posture`).\r\n\r\nDependency was addded in https://github.com/elastic/kibana/pull/187219","sha":"c426b06aa442935d02cf171da66212ddce5e5ce9"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Jeramy Soucy <jeramy.soucy@elastic.co>
solves:
Using MSW middleware to mock the usage API service and verify that metering records are collected and sent as expected to the usage service from the background metering task.