Skip to content

[Infra] Fix failing test by changing read role permission #205707

Merged
jennypavlova merged 6 commits intoelastic:mainfrom
jennypavlova:203740-fix-failing-test
Jan 7, 2025
Merged

[Infra] Fix failing test by changing read role permission #205707
jennypavlova merged 6 commits intoelastic:mainfrom
jennypavlova:203740-fix-failing-test

Conversation

@jennypavlova
Copy link
Copy Markdown
Member

@jennypavlova jennypavlova commented Jan 7, 2025

Closes #203740

Summary

This PR fixes failing test by changing read role permission to include streams and apm. As I mentioned in this comment I saw some 403 errors related to some streams and apm APIs requests so this should fix the test as it was meant to test if the dashboards tab behaves correctly based on the admin/read-only role.

However, we should think about a solution to those errors in case we have this scenario (read-only user role without apm/streams access) and have a better error message/explanation of what is missing instead of only showing the error toasts - I saw that we reverted (#202418) already a solution (#200151) for APM because of other issues but now that we also include the streams (#200060) (not sure if we need to do the request in infra but that's probably a different discussion) it's something we can revisit at one point to improve the user experience.

@jennypavlova jennypavlova self-assigned this Jan 7, 2025
@jennypavlova
Copy link
Copy Markdown
Member Author

/ci

@jennypavlova jennypavlova added release_note:skip Skip the PR/issue when compiling release notes backport:prev-minor Team:obs-ux-infra_services - DEPRECATED DEPRECATED - Use Team:obs-presentation. labels Jan 7, 2025
@kibanamachine
Copy link
Copy Markdown
Contributor

Flaky Test Runner Stats

🎉 All tests passed! - kibana-flaky-test-suite-runner#7650

[✅] x-pack/test/functional/apps/infra/config.ts: 25/25 tests passed.

see run history

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

cc @jennypavlova

@jennypavlova jennypavlova marked this pull request as ready for review January 7, 2025 13:55
@jennypavlova jennypavlova requested a review from a team January 7, 2025 13:56
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services)

@jennypavlova jennypavlova merged commit 6fc90d0 into elastic:main Jan 7, 2025
@kibanamachine
Copy link
Copy Markdown
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12653083796

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jan 7, 2025
…5707)

Closes elastic#203740
## Summary

This PR fixes failing test by changing read role permission to include
`streams` and `apm`. As I mentioned in this
[comment](elastic#203740 (comment))
I saw some 403 errors related to some `streams` and `apm` APIs requests
so this should fix the test as it was meant to test if the dashboards
tab behaves correctly based on the admin/read-only role.

However, we should think about a solution to those errors in case we
have this scenario (read-only user role without apm/streams access) and
have a better error message/explanation of what is missing instead of
only showing the error toasts - I saw that we reverted
(elastic#202418) already a solution
(elastic#200151) for APM because of
other issues but now that we also include the `streams`
(elastic#200060) (not sure if we need to
do the request in infra but that's probably a different discussion) it's
something we can revisit at one point to improve the user experience.

(cherry picked from commit 6fc90d0)
@kibanamachine
Copy link
Copy Markdown
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Jan 7, 2025
) (#205747)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Infra] Fix failing test by changing read role permission
(#205707)](#205707)

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

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

<!--BACKPORT
[{"author":{"name":"jennypavlova","email":"dzheni.pavlova@elastic.co"},"sourceCommit":{"committedDate":"2025-01-07T14:10:04Z","message":"[Infra]
Fix failing test by changing read role permission (#205707)\n\nCloses
#203740\r\n## Summary\r\n\r\nThis PR fixes failing test by changing read
role permission to include\r\n`streams` and `apm`. As I mentioned in
this\r\n[comment](https://github.com/elastic/kibana/issues/203740#issuecomment-2574907832)\r\nI
saw some 403 errors related to some `streams` and `apm` APIs
requests\r\nso this should fix the test as it was meant to test if the
dashboards\r\ntab behaves correctly based on the admin/read-only
role.\r\n\r\nHowever, we should think about a solution to those errors
in case we\r\nhave this scenario (read-only user role without
apm/streams access) and\r\nhave a better error message/explanation of
what is missing instead of\r\nonly showing the error toasts - I saw that
we reverted\r\n(#202418) already a
solution\r\n(#200151) for APM
because of\r\nother issues but now that we also include the
`streams`\r\n(#200060) (not sure
if we need to\r\ndo the request in infra but that's probably a different
discussion) it's\r\nsomething we can revisit at one point to improve the
user
experience.","sha":"6fc90d0410445cdb79419d0f6132a442d595a4e7","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:prev-minor","Team:obs-ux-infra_services"],"title":"[Infra]
Fix failing test by changing read role permission
","number":205707,"url":"https://github.com/elastic/kibana/pull/205707","mergeCommit":{"message":"[Infra]
Fix failing test by changing read role permission (#205707)\n\nCloses
#203740\r\n## Summary\r\n\r\nThis PR fixes failing test by changing read
role permission to include\r\n`streams` and `apm`. As I mentioned in
this\r\n[comment](https://github.com/elastic/kibana/issues/203740#issuecomment-2574907832)\r\nI
saw some 403 errors related to some `streams` and `apm` APIs
requests\r\nso this should fix the test as it was meant to test if the
dashboards\r\ntab behaves correctly based on the admin/read-only
role.\r\n\r\nHowever, we should think about a solution to those errors
in case we\r\nhave this scenario (read-only user role without
apm/streams access) and\r\nhave a better error message/explanation of
what is missing instead of\r\nonly showing the error toasts - I saw that
we reverted\r\n(#202418) already a
solution\r\n(#200151) for APM
because of\r\nother issues but now that we also include the
`streams`\r\n(#200060) (not sure
if we need to\r\ndo the request in infra but that's probably a different
discussion) it's\r\nsomething we can revisit at one point to improve the
user
experience.","sha":"6fc90d0410445cdb79419d0f6132a442d595a4e7"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/205707","number":205707,"mergeCommit":{"message":"[Infra]
Fix failing test by changing read role permission (#205707)\n\nCloses
#203740\r\n## Summary\r\n\r\nThis PR fixes failing test by changing read
role permission to include\r\n`streams` and `apm`. As I mentioned in
this\r\n[comment](https://github.com/elastic/kibana/issues/203740#issuecomment-2574907832)\r\nI
saw some 403 errors related to some `streams` and `apm` APIs
requests\r\nso this should fix the test as it was meant to test if the
dashboards\r\ntab behaves correctly based on the admin/read-only
role.\r\n\r\nHowever, we should think about a solution to those errors
in case we\r\nhave this scenario (read-only user role without
apm/streams access) and\r\nhave a better error message/explanation of
what is missing instead of\r\nonly showing the error toasts - I saw that
we reverted\r\n(#202418) already a
solution\r\n(#200151) for APM
because of\r\nother issues but now that we also include the
`streams`\r\n(#200060) (not sure
if we need to\r\ndo the request in infra but that's probably a different
discussion) it's\r\nsomething we can revisit at one point to improve the
user experience.","sha":"6fc90d0410445cdb79419d0f6132a442d595a4e7"}}]}]
BACKPORT-->

Co-authored-by: jennypavlova <dzheni.pavlova@elastic.co>
viduni94 pushed a commit to viduni94/kibana that referenced this pull request Jan 23, 2025
…5707)

Closes elastic#203740
## Summary

This PR fixes failing test by changing read role permission to include
`streams` and `apm`. As I mentioned in this
[comment](elastic#203740 (comment))
I saw some 403 errors related to some `streams` and `apm` APIs requests
so this should fix the test as it was meant to test if the dashboards
tab behaves correctly based on the admin/read-only role.

However, we should think about a solution to those errors in case we
have this scenario (read-only user role without apm/streams access) and
have a better error message/explanation of what is missing instead of
only showing the error toasts - I saw that we reverted
(elastic#202418) already a solution
(elastic#200151) for APM because of
other issues but now that we also include the `streams`
(elastic#200060) (not sure if we need to
do the request in infra but that's probably a different discussion) it's
something we can revisit at one point to improve the user experience.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-infra_services - DEPRECATED DEPRECATED - Use Team:obs-presentation. v8.18.0 v9.0.0

Projects

None yet

4 participants