-
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
[Security Solution] Support Serverless Cypress tests with different roles #169017
[Security Solution] Support Serverless Cypress tests with different roles #169017
Conversation
373613e
to
51d6847
Compare
116a374
to
223754f
Compare
71a71fe
to
4b8a43c
Compare
@@ -16,6 +16,7 @@ export default defineCypressConfig({ | |||
}, | |||
defaultCommandTimeout: 150000, | |||
env: { | |||
IS_SERVERLESS: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This value is already set in our test code, more specifically in the parallel script. Having it here as well may lead to misunderstandings or wrong values, we must have it in just one place. Personally, I would prefer having it outside this configuration file.
@@ -16,6 +16,7 @@ export default defineCypressConfig({ | |||
}, | |||
defaultCommandTimeout: 150000, | |||
env: { | |||
CLOUD_SERVERLESS: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This value is already set in our test code, more specifically in the scripts we call in our package.json
. Having it here as well may lead to misunderstandings or wrong values, we must have it in just one place. Personally, I would prefer to have it outside this configuration file.
@@ -22,10 +22,12 @@ export default defineCypressConfig({ | |||
viewportWidth: 1680, | |||
numTestsKeptInMemory: 10, | |||
env: { | |||
IS_SERVERLESS: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This value is already set in our test code, more specifically in the parallel script. Having it here as well may lead to misunderstandings or wrong values, we must have it in just one place. Personally, I would prefer to have it outside this configuration file.
ad6a6e2
to
7b75270
Compare
71fc147
to
416bfc8
Compare
💚 Build Succeeded
Metrics [docs]
History
To update your PR or re-run it, just comment with: cc @maximpn |
…oles (elastic#169017) **Addresses:** elastic#164451 ## Summary This PR allows to run role based reused between ESS and Serverless Cypress tests. ## Details The main idea behind is to make environmental differences for tests unnoticeable. As Serverless env already has roles and users but ESS env allows to create any possible role and user we just need to create Serverless roles and corresponding users + specific ESS roles and corresponding users in ESS env before running any ESS tests. This way tests will run in a similar env and don't have to bother by roles/users creation in test suites. This is achieved by using separate Cypress support files (Cypress includes `support/e2e.js` by default) `ess_e2e.ts` and `serverless_e2e.ts` executed for corresponding environments. `ess_e2e.ts` contains logic to create mentioned above roles and users while `serverless_e2e.ts` doesn't contain such logic. _Only one user created per role and user has the same name as its corresponding role with `changeme` password._ To have an ability to create roles we need to store their definitions somewhere. It's also convenient to have JSON definitions instead of YAML. Plus Serverless roles should be pulled from `project-controller` repo but it's not addressed in this PR. I've chosen the following locations - Serverless Security roles in `packages/kbn-es/src/serverless_resources/security_roles.json`. While `@kbn/es` is a common package it has `serverless_resources` folder containing `roles.yml` with a mix of `https://github.com/elastic/project-controller/blob/main/internal/project/observability/config/roles.yml`, `https://github.com/elastic/project-controller/blob/main/internal/project/esproject/config/roles.yml` and `https://github.com/elastic/project-controller/blob/main/internal/project/security/config/roles.yml` copied from `project-controller` and used for ES data restore. As there is no automation yet it looks logical to keep Security roles subset next to ES Serverless resources. - ESS Security specific roles in `x-pack/plugins/security_solution/common/test/ess_roles.json` On top of that the following has been done - `reader` role replaced with `t1_analyst` where possible in tests (besides `e2e/explore/cases/attach_alert_to_case.cy.ts` but it's purely ESS test so it's fine) as `reader` is ESS specific and make harder to run the same tests in ESS and Serverless environments but both roles are almost equivalent - `login()` helper function accepts all known roles (Serverless + ESS) but throws an exception if a custom ESS role is used under Serverless env - `x-pack/plugins/security_solution/server/lib/detection_engine/scripts/roles_users` isn't necessary anymore as `security_roles.json` + `ess_roles.json` contain all the necessary data to create roles and users ### Does it enable role support for MKI environments? No. This PR only enabling role support for Non-MKI Serverless environments. MKI env has predefined roles but not users. This will be addressed in a follow up PR. ## Flaky test runner Two unskiped in this PR Serverless Cypress tests using non default role `detection_response/detection_alerts/missing_privileges_callout.cy.ts` and `detection_response/prebuilt_rules/prebuilt_rules_install_update_authorization.cy.ts` [150 runs](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/3723) 🟢 (there is one env related failure but it doesn't look related to the changes in this PR)
Addresses: #164451
Summary
This PR allows to run role based reused between ESS and Serverless Cypress tests.
Details
The main idea behind is to make environmental differences for tests unnoticeable. As Serverless env already has roles and users but ESS env allows to create any possible role and user we just need to create Serverless roles and corresponding users + specific ESS roles and corresponding users in ESS env before running any ESS tests. This way tests will run in a similar env and don't have to bother by roles/users creation in test suites. This is achieved by using Cypress support file (Cypress includes
support/e2e.js
by default)e2e.ts
executed for corresponding environments.Only one user created per role and user has the same name as its corresponding role with
changeme
password.To have an ability to create roles we need to store their definitions somewhere. It's also convenient to have JSON definitions instead of YAML. Plus Serverless roles should be pulled from
project-controller
repo but it's not addressed in this PR. I've chosen the following locationspackages/kbn-es/src/serverless_resources/security_roles.json
. While@kbn/es
is a common package it hasserverless_resources
folder containingroles.yml
with a mix ofhttps://github.com/elastic/project-controller/blob/main/internal/project/observability/config/roles.yml
,https://github.com/elastic/project-controller/blob/main/internal/project/esproject/config/roles.yml
andhttps://github.com/elastic/project-controller/blob/main/internal/project/security/config/roles.yml
copied fromproject-controller
and used for ES data restore. As there is no automation yet it looks logical to keep Security roles subset next to ES Serverless resources.x-pack/plugins/security_solution/common/test/ess_roles.json
On top of that the following has been done
reader
role replaced witht1_analyst
where possible in tests (besidese2e/explore/cases/attach_alert_to_case.cy.ts
but it's purely ESS test so it's fine) asreader
is ESS specific and make harder to run the same tests in ESS and Serverless environments but both roles are almost equivalentlogin()
helper function accepts all known roles (Serverless + ESS) but throws an exception if a custom ESS role is used under Serverless envx-pack/plugins/security_solution/server/lib/detection_engine/scripts/roles_users
isn't necessary anymore assecurity_roles.json
+ess_roles.json
contain all the necessary data to create roles and usersDoes it enable role support for MKI environments?
No. This PR only enabling role support for Non-MKI Serverless environments. MKI env has predefined roles but not users. This will be addressed in a follow up PR.
Flaky test runner
Two unskiped in this PR Serverless Cypress tests using non default role
detection_response/detection_alerts/missing_privileges_callout.cy.ts
anddetection_response/prebuilt_rules/prebuilt_rules_install_update_authorization.cy.ts
150 runs 🟢 (there is one env related failure but it doesn't look related to the changes in this PR)