-
Notifications
You must be signed in to change notification settings - Fork 14
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
NETOBSERV-1621: fix error messages #523
Conversation
@jpinsonneau: This pull request references NETOBSERV-1621 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.16.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
New image: It will expire after two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=c905b62 make set-plugin-image |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #523 +/- ##
==========================================
- Coverage 56.65% 56.55% -0.11%
==========================================
Files 174 173 -1
Lines 9003 9042 +39
Branches 1180 1183 +3
==========================================
+ Hits 5101 5114 +13
- Misses 3536 3560 +24
- Partials 366 368 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
on this particular point:
Isn't it on purpose? Or perhaps just accidental, but it seems logical that non-authorized users should not be able to get insights in the config |
Approving although I'd like to have a discussion on the comment I left above ; but in the meantime I guess it's ok to be verified by QE |
We will need to backport that |
The frontend was not showing any error here which means it uses default config and run flow / metrics queries before getting the not authorized error. |
Rebased but started to have issues with testing 🤔 I have added a fix to bypass the error when |
@memodi @jpinsonneau do you understand the cypress errors? Is that something requiring a change on the cypress test side? |
/retest-required |
It seems to be related to prometheus enabled in the FlowCollector but not provided in the test cluster:
|
/test plugin-cypress |
tests pass \o/ |
/ok-to-test |
New image: It will expire after two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=8a1f905 make set-plugin-image |
- on 401/403, mention netobserv-reader role - add link to downstream doc too - make some routes admin-only
Rebased without changes |
@memodi this tab is enabled only when Loki datasource is available. By default, none are available until the config is loaded from the configmap through the backend API. If your user doesn't have the proper rights to load the content, the tab will be greyed out as we can't load the config either. Is that an issue for you ? |
/lgtm |
@jpinsonneau in that case the information message is misleading, because in my case loki was enabled and it asks for loki to be enabled for Traffic flows tab. I was hoping to Traffic flows will be consistently enabled like Topology and Overview tabs and this hasn't been issue until 1.5 release. |
I can enable all datasources by default in that case so the view will be enabled until we check for config when the user has the proper rights |
New changes are detected. LGTM label has been removed. |
/ok-to-test |
New image: It will expire after two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=7b00437 make set-plugin-image |
/label qe-approved |
@jpinsonneau: This pull request references NETOBSERV-1621 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.17.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Description
This PR fixes multiple cases:
getConfig
was ignoredLokistack
/Manual
auth configurations/ready
endpoint error is now displayed in console to avoid confusionAlso refactored the code to prepare for Prometheus datasource and added a
denyAll
checker for debuggingDependencies
n/a
Checklist
If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.