Skip to content
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

[Fleet] Add fleet subfeatures #178006

Merged
merged 4 commits into from
Mar 11, 2024
Merged

Conversation

nchaulet
Copy link
Member

@nchaulet nchaulet commented Mar 5, 2024

Summary

Resolve https://github.com/elastic/ingest-dev/issues/2892

Introduce this under a fleet feature flag subfeaturePrivileges, so this will have no impact for users.

The privileges UI is currently broken but will be fixed by #178239

Screenshot 2024-03-05 at 8 58 06 AM

@elastic/kibana-security it seems there is a bug nothing happened when I click on customize sub-features.
If I remove requireAllSpaces: true from the top privilieges it seems to work, but currently fleet require all spaces.

How to test

Add the following to your kibana.dev.yml

xpack.fleet.enableExperimental: ['subfeaturePrivileges']

@nchaulet nchaulet added release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team labels Mar 5, 2024
@nchaulet nchaulet self-assigned this Mar 5, 2024
@apmmachine
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • /oblt-deploy-serverless : Deploy a serverless Kibana instance using the Observability test environments.
  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@azasypkin
Copy link
Member

@elastic/kibana-security it seems there is a bug nothing happened when I click on customize sub-features.
If I remove requireAllSpaces: true from the top privilieges it seems to work, but currently fleet require all spaces.

Yeah, looks like a bug indeed, I'll dig into this tomorrow.

@azasypkin
Copy link
Member

Presumably something like this should fix the bug, but I'd like to take a closer look tomorrow before proposing/working-on the solution:

diff --git a/x-pack/plugins/security/public/management/roles/edit_role/privileges/kibana/feature_table/feature_table_expanded_row.tsx b/x-pack/plugins/security/public/management/roles/edit_role/privileges/kibana/feature_table/feature_table_expanded_row.tsx
index abc33abcb86..42090f8c6c0 100644
--- a/x-pack/plugins/security/public/management/roles/edit_role/privileges/kibana/feature_table/feature_table_expanded_row.tsx
+++ b/x-pack/plugins/security/public/management/roles/edit_role/privileges/kibana/feature_table/feature_table_expanded_row.tsx
@@ -63,7 +63,8 @@ export const FeatureTableExpandedRow = ({
       privilegeCalculator.updateSelectedFeaturePrivilegesForCustomization(
         feature.id,
         privilegeIndex,
-        e.target.checked
+        e.target.checked,
+        allSpacesSelected
       )
     );
     setIsCustomizing(e.target.checked);
diff --git a/x-pack/plugins/security/public/management/roles/edit_role/privileges/kibana/privilege_form_calculator/privilege_form_calculator.ts b/x-pack/plugins/security/public/management/roles/edit_role/privileges/kibana/privilege_form_calculator/privilege_form_calculator.ts
index d6afdaf6efa..14493fcd561 100644
--- a/x-pack/plugins/security/public/management/roles/edit_role/privileges/kibana/privilege_form_calculator/privilege_form_calculator.ts
+++ b/x-pack/plugins/security/public/management/roles/edit_role/privileges/kibana/privilege_form_calculator/privilege_form_calculator.ts
@@ -185,13 +185,19 @@ export class PrivilegeFormCalculator {
    * @param featureId the feature id
    * @param privilegeIndex  the index of the kibana privileges role component
    * @param willBeCustomizing flag indicating if this feature is about to have its sub-feature privileges customized or not
+   * @param allSpacesSelected indicates if the privilege form is configured to grant access to all spaces.
    */
   public updateSelectedFeaturePrivilegesForCustomization(
     featureId: string,
     privilegeIndex: number,
-    willBeCustomizing: boolean
+    willBeCustomizing: boolean,
+    allSpacesSelected: boolean
   ) {
-    const primary = this.getDisplayedPrimaryFeaturePrivilege(featureId, privilegeIndex);
+    const primary = this.getDisplayedPrimaryFeaturePrivilege(
+      featureId,
+      privilegeIndex,
+      allSpacesSelected
+    );
     const selectedFeaturePrivileges = this.getSelectedFeaturePrivileges(featureId, privilegeIndex);
 
     if (!primary) {

@nchaulet
Copy link
Member Author

nchaulet commented Mar 8, 2024

/ci

@nchaulet nchaulet marked this pull request as ready for review March 8, 2024 12:56
@nchaulet nchaulet requested a review from a team as a code owner March 8, 2024 12:56
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@nchaulet
Copy link
Member Author

nchaulet commented Mar 8, 2024

@elasticmachine merge upstream

@juliaElastic
Copy link
Contributor

In the issue it seems we want to exclude None from Settings subfeature, is it possible?

@nchaulet
Copy link
Member Author

nchaulet commented Mar 8, 2024

In the issue it seems we want to exclude None from Settings subfeature, is it possible?

Looks it does not seems possible to not have None

Copy link
Contributor

@juliaElastic juliaElastic left a comment

Choose a reason for hiding this comment

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

LGTM, probably we should confirm with Nima, what should be the behaviour if Settings:None is selected.

@nchaulet
Copy link
Member Author

@elasticmachine merge upstream

Copy link
Member

@kpollich kpollich left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for setting up the feature flag and getting started here 👍

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
fleet 153.8KB 153.8KB +24.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @nchaulet

@nchaulet nchaulet merged commit f034f86 into elastic:main Mar 11, 2024
19 checks passed
@nchaulet nchaulet deleted the fleet-sub-privileges branch March 11, 2024 14:10
@kibanamachine kibanamachine added v8.14.0 backport:skip This commit does not require backporting labels Mar 11, 2024
azasypkin added a commit that referenced this pull request Mar 13, 2024
… `all spaces` requirement (#178239)

## Summary

Allow customizing sub-feature privileges with `all spaces` requirement.
See #178006 (comment) for
more details about what was broken.

## To do

- [x] Fix bug and update existing tests
- [x] Add more tests with `all spaces` requirement

__Reported in:__
#178006 (comment)

## How to test

Add the following to your `kibana.dev.yml` and try to customize
sub-feature privileges of the `Fleet` feature

```yaml
xpack.fleet.enableExperimental: ['subfeaturePrivileges']
```

Before this PR, the `Customize sub-feature privileges` switch didn't
work.

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Mar 13, 2024
… `all spaces` requirement (elastic#178239)

## Summary

Allow customizing sub-feature privileges with `all spaces` requirement.
See elastic#178006 (comment) for
more details about what was broken.

## To do

- [x] Fix bug and update existing tests
- [x] Add more tests with `all spaces` requirement

__Reported in:__
elastic#178006 (comment)

## How to test

Add the following to your `kibana.dev.yml` and try to customize
sub-feature privileges of the `Fleet` feature

```yaml
xpack.fleet.enableExperimental: ['subfeaturePrivileges']
```

Before this PR, the `Customize sub-feature privileges` switch didn't
work.

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit acc94b3)
kibanamachine added a commit that referenced this pull request Mar 13, 2024
…es with &#x60;all spaces&#x60; requirement (#178239) (#178644)

# Backport

This will backport the following commits from `main` to `8.13`:
- [fix(platform-security): allow customizing sub-feature privileges with
&#x60;all spaces&#x60; requirement
(#178239)](#178239)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Aleh
Zasypkin","email":"aleh.zasypkin@elastic.co"},"sourceCommit":{"committedDate":"2024-03-13T15:29:54Z","message":"fix(platform-security):
allow customizing sub-feature privileges with `all spaces` requirement
(#178239)\n\n## Summary\r\n\r\nAllow customizing sub-feature privileges
with `all spaces` requirement.\r\nSee
#178006 (comment)
for\r\nmore details about what was broken.\r\n\r\n## To do\r\n\r\n- [x]
Fix bug and update existing tests\r\n- [x] Add more tests with `all
spaces` requirement\r\n\r\n__Reported
in:__\r\nhttps://github.com//pull/178006#issue-2169314164\r\n\r\n##
How to test\r\n\r\nAdd the following to your `kibana.dev.yml` and try to
customize\r\nsub-feature privileges of the `Fleet`
feature\r\n\r\n```yaml\r\nxpack.fleet.enableExperimental:
['subfeaturePrivileges']\r\n```\r\n\r\nBefore this PR, the `Customize
sub-feature privileges` switch didn't\r\nwork.\r\n\r\nCo-authored-by:
Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>","sha":"acc94b3b512c7ba9d9c3223c941154ba416cc5a2","branchLabelMapping":{"^v8.14.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","Team:Security","Feature:Users/Roles/API
Keys","release_note:skip","backport:all-open","v8.14.0"],"title":"fix(platform-security):
allow customizing sub-feature privileges with `all spaces`
requirement","number":178239,"url":"https://github.com/elastic/kibana/pull/178239","mergeCommit":{"message":"fix(platform-security):
allow customizing sub-feature privileges with `all spaces` requirement
(#178239)\n\n## Summary\r\n\r\nAllow customizing sub-feature privileges
with `all spaces` requirement.\r\nSee
#178006 (comment)
for\r\nmore details about what was broken.\r\n\r\n## To do\r\n\r\n- [x]
Fix bug and update existing tests\r\n- [x] Add more tests with `all
spaces` requirement\r\n\r\n__Reported
in:__\r\nhttps://github.com//pull/178006#issue-2169314164\r\n\r\n##
How to test\r\n\r\nAdd the following to your `kibana.dev.yml` and try to
customize\r\nsub-feature privileges of the `Fleet`
feature\r\n\r\n```yaml\r\nxpack.fleet.enableExperimental:
['subfeaturePrivileges']\r\n```\r\n\r\nBefore this PR, the `Customize
sub-feature privileges` switch didn't\r\nwork.\r\n\r\nCo-authored-by:
Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>","sha":"acc94b3b512c7ba9d9c3223c941154ba416cc5a2"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.14.0","branchLabelMappingKey":"^v8.14.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/178239","number":178239,"mergeCommit":{"message":"fix(platform-security):
allow customizing sub-feature privileges with `all spaces` requirement
(#178239)\n\n## Summary\r\n\r\nAllow customizing sub-feature privileges
with `all spaces` requirement.\r\nSee
#178006 (comment)
for\r\nmore details about what was broken.\r\n\r\n## To do\r\n\r\n- [x]
Fix bug and update existing tests\r\n- [x] Add more tests with `all
spaces` requirement\r\n\r\n__Reported
in:__\r\nhttps://github.com//pull/178006#issue-2169314164\r\n\r\n##
How to test\r\n\r\nAdd the following to your `kibana.dev.yml` and try to
customize\r\nsub-feature privileges of the `Fleet`
feature\r\n\r\n```yaml\r\nxpack.fleet.enableExperimental:
['subfeaturePrivileges']\r\n```\r\n\r\nBefore this PR, the `Customize
sub-feature privileges` switch didn't\r\nwork.\r\n\r\nCo-authored-by:
Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>","sha":"acc94b3b512c7ba9d9c3223c941154ba416cc5a2"}}]}]
BACKPORT-->

Co-authored-by: Aleh Zasypkin <aleh.zasypkin@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v8.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants