[Infra] Fix failing test by changing read role permission #205707
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…tion endpoint using sources (elastic#203424)" This reverts commit 5bacf1f.
|
/ci |
jennypavlova
added
release_note:skip
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. |
💚 Build Succeeded
Metrics [docs]
|
|
Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services) |
rmyz
approved these changes
Jan 7, 2025
|
Starting backport for target branches: 8.x |
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)
💚 All backports created successfully
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":"[email protected]"},"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 <[email protected]>
kowalczyk-krzysztof
pushed a commit
to kowalczyk-krzysztof/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.
crespocarlos
pushed a commit
to crespocarlos/kibana
that referenced
this pull request
Jan 8, 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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #203740
Summary
This PR fixes failing test by changing read role permission to include
streamsandapm. As I mentioned in this comment I saw some 403 errors related to somestreamsandapmAPIs 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.