Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: batleforc The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @batleforc. Thanks for your PR. I'm waiting for a eclipse-che member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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 kubernetes-sigs/prow repository. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1492 +/- ##
==========================================
- Coverage 93.26% 92.25% -1.02%
==========================================
Files 564 563 -1
Lines 54795 55049 +254
Branches 4159 4109 -50
==========================================
- Hits 51107 50785 -322
- Misses 3642 4217 +575
- Partials 46 47 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@batleforc hello, thank you for the contribution. Could you please clarify the UX e.g. when exactly user needs to do this procedure manually? I guess it is related to the short-lived tokens, but afaik for OpenShift it is 24h by default |
|
Hi @ibuziuk In my case, the token should at most live 6H (set in the different OIDC providers) and sometimes the end of the token's life comes in the middle of a debug process or a long-lived process so restarting the full app ends up being a headache more than anything. And extending the token's life is not possible. |
|
@batleforc thank you for clarification, but I'm a bit concerned about exposing this capability on the UI since it is pretty niche. Could you clarify how you are currently updating the kubeconfig? is it |
|
@ibuziuk in the PR yes i call this function https://github.com/batleforc/che-dashboard/blob/main/packages/dashboard-frontend/src/services/backend-client/devWorkspaceApi.ts#L126 that is directly exposed in the fronted. At the moment my user or i are annoyed by it and redo the kube oidc login full process or wait for some more time to restart the container. |
| export const refreshKubeconfigWorkspace = | ||
| (workspace: devfileApi.DevWorkspace): AppThunk => | ||
| async () => { | ||
| const defer: IDeferred<void> = getDefer(); |
There was a problem hiding this comment.
@batleforc Hello, thank you for the contribution.
I see that the defer promise never resolves(defer.resolve() is never called). The getDefer() pattern is unnecessary here.
Example of replacing with a plain async/await:
export const refreshKubeconfigWorkspace =
(workspace: devfileApi.DevWorkspace): AppThunk =>
async () => {
if (workspace.status?.phase !== DevWorkspaceStatus.RUNNING) {
return;
}
const devworkspaceId = workspace.status?.devworkspaceId;
const namespace = workspace.metadata.namespace;
if (!devworkspaceId || !namespace) {
throw new Error(
`Failed to refresh kubeconfig for "${workspace.metadata.name}": missing devworkspaceId or namespace.`,
);
}
await injectKubeConfig(namespace, devworkspaceId);
await podmanLogin(namespace, devworkspaceId);
};There was a problem hiding this comment.
Thank's. I've directly applied your example.
@batleforc Hello, need to increase test coverage for your changes from 55% to 92% (add some tests). |
|
@batleforc thank you for the details and follow-up. I believe we need some option to re-inject the token without the direct user interaction and exposing internals on UI. |
|
Shoud i finish adding more test or not ? @ibuziuk |
@tolusha @olexii4 @batleforc tbh, I would only add this action to VS Code, without adding new menu item to the user dashboard @TheChosenMok please, review, wdyt ^ |
|
@ibuziuk one of the reasons has to why I chose to put it in the dashboard was to not have to trigger a plugin for the different kinds of IDE (VsCode, VsCode Desktop, JetBrains, WebShell, and existing and future ones) |
|
@batleforc, that is a fair take. For me, the UX is questionable, though, when the user is expected to go to the dashboard from the workspace and manually trigger the injection. |
|
@ibuziuk in any case he has to, to re-login to the dashboard |


What does this PR do?
Add an item to the WS dropdown that allow re-injecting the user's kube token inside the WS
Screenshot/screencast of this PR
What issues does this PR fix or reference?
eclipse-che/che#23545
Is it tested? How?
Release Notes
Docs PR
Do i need to do a docs PR ?