[Security Solution][Endpoint] Enable Kibana feature controls (RBAC)configuration by space for Endpoint management #193003
Conversation
…for-multiple-spaces
…for-multiple-spaces
paul-tavares
added
release_note:skip
…for-multiple-spaces
|
/ci |
|
/ci |
|
Flaky test runner 1: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6966 |
…spaces' into task/olm-8537-rbac-for-multiple-spaces
|
/ci |
|
/ci |
…for-multiple-spaces
…gement and spaces
|
/ci |
paul-tavares
added
the
backport:prev-minor
elastic-vault-github-plugin-prod
bot
requested a review
from a team
as a code owner
|
|
||
| return subFeature; | ||
| }; | ||
|
|
||
| const securitySubFeaturesList: Array<[SecuritySubFeatureId, SubFeatureConfig]> = [ |
There was a problem hiding this comment.
I wonder if it wouldn't be more readable if you iterate over securitySubFeaturesList instead of wrapping each subFeature in enableSpaceAwarenessIfNeeded. What do you think?
There was a problem hiding this comment.
I guess. It accomplishes the same thing, but I'm open to changing it.
what do you find about this implementation that makes it less readable?
I did try to make the function names clear as to what they do and also opted to do it this way only because it gives us some flexibility if we ever have a case where a specific feature privilege does require it to be space agonistic.
There was a problem hiding this comment.
I was thinking more about doing it in one place insterad of passing to all separately:
const securitySubFeaturesListWithSpaceAwarnessIfNeeded = securitySubFeaturesList.map(
([id, config]) => [id, enableSpaceAwarenessIfNeeded(config)];
);
There was a problem hiding this comment.
yeah... Not sure if its better - I don't like that additional iteration by using .map() and recreating the entire structure again.
I'm going to leave it as is for now.
| (suffix: string = '') => { | ||
| return `${props.categoryId ? `${props.categoryId}_` : ''}${ | ||
| props.featureId | ||
| }_${subFeatureNameTestId}${suffix ? `_${suffix}` : ''}`; |
There was a problem hiding this comment.
I always opt against having generators for test-ids because it's almost impossible to find the element from the web console in the codebase. What do you say if we hardcode the test-ids?
There was a problem hiding this comment.
I'm actually of the opposite opinion. Having static test ids in some cases makes it harder to find the elements you are after, especially when a component is reused multiple times on the page - as is the case here. Static only make sense (to me) when a component is not reused on a page (is only ever rendered once)
I rather keep it as is, unless you have a better way to select elements on the page in a more efficient way
There was a problem hiding this comment.
Whenever I want to debug functionality, I get the data-test-id from devconsole. If it was hardcoded I could just use Ctrl+F and search for it, with the prefixes generators etc it's almost impossible - and have to look for chunks and later confirm if I found a correct prefix.
However, I don't insist, just sharing personal opinion :P It can stay this way 👍 Thanks!
There was a problem hiding this comment.
So yeah, I do think we differ in the way we approach test development. If I'm working on e2e tests, I do in fact use the browser's developer tools to grab the data-test-subj's for elements I need to access. And that is what these changes are doing to the role forms. And in this case, because those forms are used multiple times on the page, using a static data-test-subj would negate the purpose as to why I made them - to facilitate and more predictably select elements in DOM.
I will leave it as is since you are ok with it.
There was a problem hiding this comment.
Of course, all good 👍 thanks for the consideration :)
…spaces' into task/olm-8537-rbac-for-multiple-spaces
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
…nfiguration by space for Endpoint management (elastic#193003) ## Summary ### Kibana Core Security plugin - Updated several Role forms to include `data-test-subj` in order to better select items from tests ### Security Solution Plugin (and associated packages) Changes in support of space awareness: - New feature flag control: `endpointManagementSpaceAwarenessEnabled` - Elastic Defend related kibana feature controls are changed to `requireAllSpaces: false` when feature flag is enabled In addition: - The Cypress serverless FTR configuration used for Defend Workflows tests was updated to enable the creation of spaces and also role management UI (cherry picked from commit 9a9c0f1)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…BAC)configuration by space for Endpoint management (#193003) (#193525) # Backport This will backport the following commits from `main` to `8.x`: - [[Security Solution][Endpoint] Enable Kibana feature controls (RBAC)configuration by space for Endpoint management (#193003)](#193003) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Paul Tavares","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-09-20T09:47:37Z","message":"[Security Solution][Endpoint] Enable Kibana feature controls (RBAC)configuration by space for Endpoint management (#193003)\n\n## Summary\r\n\r\n### Kibana Core Security plugin\r\n\r\n- Updated several Role forms to include `data-test-subj` in order to\r\nbetter select items from tests\r\n\r\n\r\n\r\n### Security Solution Plugin (and associated packages)\r\n\r\nChanges in support of space awareness:\r\n\r\n- New feature flag control: `endpointManagementSpaceAwarenessEnabled`\r\n- Elastic Defend related kibana feature controls are changed to\r\n`requireAllSpaces: false` when feature flag is enabled\r\n\r\nIn addition:\r\n\r\n- The Cypress serverless FTR configuration used for Defend Workflows\r\ntests was updated to enable the creation of spaces and also role\r\nmanagement UI","sha":"9a9c0f1afe0369d5cfaae1b9468c60dbe800d73b","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:Defend Workflows","backport:prev-minor","v8.16.0"],"title":"[Security Solution][Endpoint] Enable Kibana feature controls (RBAC)configuration by space for Endpoint management","number":193003,"url":"https://github.com/elastic/kibana/pull/193003","mergeCommit":{"message":"[Security Solution][Endpoint] Enable Kibana feature controls (RBAC)configuration by space for Endpoint management (#193003)\n\n## Summary\r\n\r\n### Kibana Core Security plugin\r\n\r\n- Updated several Role forms to include `data-test-subj` in order to\r\nbetter select items from tests\r\n\r\n\r\n\r\n### Security Solution Plugin (and associated packages)\r\n\r\nChanges in support of space awareness:\r\n\r\n- New feature flag control: `endpointManagementSpaceAwarenessEnabled`\r\n- Elastic Defend related kibana feature controls are changed to\r\n`requireAllSpaces: false` when feature flag is enabled\r\n\r\nIn addition:\r\n\r\n- The Cypress serverless FTR configuration used for Defend Workflows\r\ntests was updated to enable the creation of spaces and also role\r\nmanagement UI","sha":"9a9c0f1afe0369d5cfaae1b9468c60dbe800d73b"}},"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/193003","number":193003,"mergeCommit":{"message":"[Security Solution][Endpoint] Enable Kibana feature controls (RBAC)configuration by space for Endpoint management (#193003)\n\n## Summary\r\n\r\n### Kibana Core Security plugin\r\n\r\n- Updated several Role forms to include `data-test-subj` in order to\r\nbetter select items from tests\r\n\r\n\r\n\r\n### Security Solution Plugin (and associated packages)\r\n\r\nChanges in support of space awareness:\r\n\r\n- New feature flag control: `endpointManagementSpaceAwarenessEnabled`\r\n- Elastic Defend related kibana feature controls are changed to\r\n`requireAllSpaces: false` when feature flag is enabled\r\n\r\nIn addition:\r\n\r\n- The Cypress serverless FTR configuration used for Defend Workflows\r\ntests was updated to enable the creation of spaces and also role\r\nmanagement UI","sha":"9a9c0f1afe0369d5cfaae1b9468c60dbe800d73b"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Paul Tavares <[email protected]>
Summary
Kibana Core Security plugin
data-test-subjin order to better select items from testsSecurity Solution Plugin (and associated packages)
Changes in support of space awareness:
endpointManagementSpaceAwarenessEnabledrequireAllSpaces: falsewhen feature flag is enabledIn addition:
Checklist