Closed rhamilto closed 1 week ago
@rhamilto: This pull request references CONSOLE-4269 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 story to target the "4.18.0" version, but no target version was set.
@rhamilto: This pull request references CONSOLE-4269 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 story to target the "4.18.0" version, but no target version was set.
@rhamilto: This pull request references CONSOLE-4269 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 story to target the "4.18.0" version, but no target version was set.
@vojtechszocs, I removed the changes to useTelemetry.ts
from this PR since they are unrelated the scope of the story and opened a bug to address.
@rhamilto: This pull request references CONSOLE-4269 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 story to target the "4.18.0" version, but no target version was set.
@rhamilto: This pull request references CONSOLE-4269 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 story to target the "4.18.0" version, but no target version was set.
@rhamilto: This pull request references CONSOLE-4269 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 story to target the "4.18.0" version, but no target version was set.
@rhamilto: This pull request references CONSOLE-4269 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 story to target the "4.18.0" version, but no target version was set.
@rhamilto: This pull request references CONSOLE-4269 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 story to target the "4.18.0" version, but no target version was set.
@rhamilto: This pull request references CONSOLE-4269 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 story to target the "4.18.0" version, but no target version was set.
QE Approver: /assign @yapei Docs Approver: /assign @opayne1 PX Approver: /assign @reestr
@rhamilto when edit "Enabled" field on plugin details page, the plugin could be enabled repeatedly, we could see multiple plugins with same name in console operator, is it an issue?
@rhamilto when edit "Enabled" field on plugin details page, the plugin could be enabled repeatedly, we could see multiple plugins with same name in console operator, is it an issue?
@yanpzhan, I believe that is an existing issue. It's a bug, but not one that is hugely problematic. Mind opening a bug?
@yanpzhan, I believe that is an existing issue. It's a bug, but not one that is hugely problematic. Mind opening a bug?
This appears to be the result of the patch to the console config taking a while to occur (which happens sporadically). A partial fix is to disable the Save
button if the enabled value hasn't been changed in the modal, but that only works reliably if the previous patch has already occurred and the console config is updated.
/retest
QE Approver: /assign @yapei Docs Approver: /assign @opayne1 PX Approver: /assign @reestr
Also worth to notice that this PR is cherry-picking CONSOLE-4264: Notify users of Console plugin related Content Security Policy violations and also contributing a fix for issue found by @yapei.
@opayne1 please review only commits contributed by @rhamilto, since @vojtechszocs was already reviewed by you. @yapei please test only functionality related to the ConsolePlugin details page. Thank you 🥇
/test e2e-gcp-console
/label px-approved
Checked on cluster launched against the pr, tested consoleplugin 'Details' page and 'Plugin manifest' tab, features work as expected. /label qe-approved
@rhamilto: This pull request references CONSOLE-4269 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 story to target the "4.18.0" version, but no target version was set.
/label docs-approved
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: jhadvig, rhamilto
The full list of commands accepted by this bot can be found here.
The pull request process is described here
Hey, @rhamilto. This is great. I'm happy to see we have a nice details page.
Out of scope for this PR and story, but I had a few thoughts for future improvements:
@jhadvig @vojtechszocs FYI
Hey, @rhamilto. This is great. I'm happy to see we have a nice details page.
Thanks for the feedback, @spadgett.
- It would be good to show the plugin's supported OCP versions and make it clear when a plugin is disabled since its version range doesn't match the current OCP version. (I believe the supported versions are in the manifest, but would be good to have this on the overview tab. See plugin manifest properties.)
Easy enough to add.
- Should we add a top-level nav items for Console Plugins to the Administration nav section? It would make it more discoverable.
Agreed.
- Syntax highlighting for the manifest JSON would be nice. Maybe we can use Monaco.
@vojtechszocs and I started down the path of using Monaco, but decided against it for the initial implementation because the read only mode of the existing YAML editor posed some usability questions that we didn't have time to contend with.
- I think a table of the extensions used by the plugin would be nice to have in addition to the manifest JSON (but keep the manifest tab for the complete JSON).
Easy enough to add.
- It would be good to show the plugin's supported OCP versions and make it clear when a plugin is disabled since its version range doesn't match the current OCP version. (I believe the supported versions are in the manifest, but would be good to have this on the overview tab. See plugin manifest properties.)
@spadgett, maybe something like this (note all the details in the video are not accurate such as Enabled
)?
https://github.com/user-attachments/assets/115f4b54-c895-4cd6-8b63-1875e8ad4d92
3. Syntax highlighting for the manifest JSON would be nice. Maybe we can use Monaco.
@spadgett, we must've done something wonky with our initial attempt as this appears to be working correctly now.
I do wonder, is it weird to have two YAML editors?
https://github.com/user-attachments/assets/787c3d4b-2959-4d1c-92db-c4ed10849ca8
4. I think a table of the extensions used by the plugin would be nice to have in addition to the manifest JSON (but keep the manifest tab for the complete JSON).
Hmm. Given the variance amongst the extension properties, this one is a bit trickier to tabularize. 🤔
- It would be good to show the plugin's supported OCP versions and make it clear when a plugin is disabled since its version range doesn't match the current OCP version. (I believe the supported versions are in the manifest, but would be good to have this on the overview tab. See plugin manifest properties.)
@spadgett, maybe something like this (note all the details in the video are not accurate such as
Enabled
)?Screen.Recording.2024-11-19.at.9.08.21.AM.mov
@rhamilto Yeah, that looks good. We should probably show the current console version in the warning popover and add something to the plugin list table if we don't already have it. We can make it a separate story.
- I think a table of the extensions used by the plugin would be nice to have in addition to the manifest JSON (but keep the manifest tab for the complete JSON).
Hmm. Given the variance amongst the extension properties, this one is a bit trickier to tabularize. 🤔
Yeah, I was hesitant about that, too. But even just the list of extension IDs would be helpful. We could show the resource kind for any extension that targets a specific resource and leave the column blank for other extension points. Or maybe not a table, but some other presentation.
- Syntax highlighting for the manifest JSON would be nice. Maybe we can use Monaco.
@spadgett, we must've done something wonky with our initial attempt as this appears to be working correctly now.
I do wonder, is it weird to have two YAML editors?
Yeah, I'm undecided. Maybe we can style it differently or simplify the editor to make it more clear that it's read-only?
All of these suggestions can be follow-up PRs, particularly since this has merged. I'd say the compatible versions tweak is the most important to help admins troubleshoot plugins that aren't enabled.
2. Should we add a top-level nav items for Console Plugins to the Administration nav section? It would make it more discoverable.
Made this a bug so we can backport: https://github.com/openshift/console/pull/14521 @spadgett, mind reviewing?
Includes changes from https://github.com/openshift/console/pull/14374, which should merge first. Note https://github.com/openshift/console/pull/14374 includes a bug that needs to be addressed where not Loaded plugins do not appear in the
Console plugins
list.In collaboration with @vojtechszocs 🥇
TODOs:
Description
,Version
,Status
,Enabled
, andCSP Violations
popover text | @opayne1, can you help here?Demos
Plugin is
Loaded
https://github.com/user-attachments/assets/08e9b736-12bb-4336-a0bb-72e7cc408657
Plugin is not
Loaded
(information from pluginStore is not available)https://github.com/user-attachments/assets/2d8e1efb-880d-480b-aa61-dc8531ed06cc
Development setup
console-demo-plugin
locally.