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

[Synthetics] Remove fleet permission requirement for private location monitor cruds #159378

Merged
merged 42 commits into from Jun 14, 2023

Conversation

shahzad31
Copy link
Contributor

@shahzad31 shahzad31 commented Jun 9, 2023

Summary

Remove fleet permission requirement for private location monitor cruds !!

Only synthetics write permission will be required to add/edit/delete monitors in private locations !!

image

Release notes

Creating, Editing, deleting monitors in private locations, no longer requires fleet all permission. You will only need synthetics write permissions.

  • Creating private locations, managing monitors in private locations will only need synthetics permissions
  • Fleet permissions are still required if user have to create a new agent policy which needs to be attached with private location
  • If policies already exists for example or have been setup by an admin, Synthetics users with synthetics permissions can read those policies and will be able to use them as part of private location management.

@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.
  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@shahzad31 shahzad31 marked this pull request as ready for review June 9, 2023 14:49
@shahzad31 shahzad31 requested a review from a team as a code owner June 9, 2023 14:49
@shahzad31 shahzad31 added release_note:skip Skip the PR/issue when compiling release notes v8.9.0 release_note:enhancement and removed release_note:skip Skip the PR/issue when compiling release notes labels Jun 9, 2023
@botelastic botelastic bot added the Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability label Jun 9, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Jun 9, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@shahzad31 shahzad31 requested a review from a team as a code owner June 12, 2023 08:34
paulb-elastic added a commit to paulb-elastic/observability-docs that referenced this pull request Jun 12, 2023
Update required permissions as a result of elastic/kibana#159378
Copy link
Contributor

@dominiqueclarke dominiqueclarke left a comment

Choose a reason for hiding this comment

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

This does not work for proejct monitors.

There are tests in the api integrations that still show that errors populate when a user uses an api key/user without fleet permissions.

One test you updated actually wasn't a faithful test, because it was using supertest instead of supertestWithoutAuth. I've updated it and you can now see the test fails. Plus there are "passing" test that test that errors occur when users do not have fleet permissions.

@dominiqueclarke dominiqueclarke self-requested a review June 13, 2023 18:04
Copy link
Contributor

@florent-leborgne florent-leborgne left a comment

Choose a reason for hiding this comment

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

I've reviewed the current text as is, but wouldn't it be possible to just add a small mention besides/below the disabled button within the empty state that states "You need the XX privilege to create an agent policy", instead of the big callout?

shahzad31 and others added 3 commits June 14, 2023 12:45
…mmon/components/permissions.tsx

Co-authored-by: florent-leborgne <florent.leborgne@elastic.co>
…ttings/private_locations/location_form.tsx

Co-authored-by: florent-leborgne <florent.leborgne@elastic.co>
Copy link
Contributor

@dominiqueclarke dominiqueclarke left a comment

Choose a reason for hiding this comment

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

Please disable the button as well when user does not have permissions to create an agent policy

Screenshot 2023-06-14 at 10 20 00 AM

@dominiqueclarke
Copy link
Contributor

LGTM.

@florent-leborgne, I'm just wondering about this content for users who do not have Fleet privileges

image

Copy link
Contributor

@dominiqueclarke dominiqueclarke left a comment

Choose a reason for hiding this comment

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

LGTM

…mmon/components/permissions.tsx

Co-authored-by: florent-leborgne <florent.leborgne@elastic.co>
@florent-leborgne
Copy link
Contributor

@dominiqueclarke the callout is not ideal as we're trying to clarify different permissions needed for different entities but I think it's needed for them to understand that managing policies is different from managing locations, even though you need one to do the other.
The sentence itself is incorrect though, I'll suggest an edit.

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Fleet Cypress Tests / View agents list Bulk actions should allow to bulk upgrade agents and cancel that upgrade

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
synthetics 1.2MB 1.2MB -2.0KB
Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 13 15 +2
securitySolution 410 414 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 14 16 +2
securitySolution 493 497 +4
total +6

History

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

@shahzad31 shahzad31 merged commit cf9f5bb into elastic:main Jun 14, 2023
24 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jun 14, 2023
@shahzad31 shahzad31 deleted the internal-system-user branch June 14, 2023 20:07
@shahzad31
Copy link
Contributor Author

POST FF Testing looks good, created a small follow up PR

#161351

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:enhancement Team:Fleet Team label for Observability Data Collection Fleet team Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v8.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants