Skip to content
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

Catch delete token error if namespace is not provisioned #1046

Closed
wants to merge 2 commits into from

Conversation

vinokurig
Copy link
Contributor

@vinokurig vinokurig commented Jan 23, 2024

What does this PR do?

Do not throw an error on Git Service revoke if the related oauth access token is not deleted.
Currently dashboard has a timer that controls che-server's namespace provision API request: If the namespace provision is executed another provision request will be ignored for 15 seconds:

if (timeElapsed < timeToStale) {
dispatch({
type: Type.ABORT_BACKEND_CHECK,
});
return;
}

This timer causes a side effect that activates the revoke a Git service button after an actual revoke is performed. With this PR we intercept the delete token error on such case and deactivating the revoke button.

What issues does this PR fix or reference?

Is it tested? How?

  1. Setup GitHub oauth: https://eclipse.dev/che/docs/stable/administration-guide/configuring-oauth-2-for-github
  2. Start a workspace from a GitHub repository, accept the GitHub token agreement.
  3. Go to the dashboard Git Services tab and revoke the GitHub authorisation.
  4. Quickly go to the Personal Access Tokens tab and then quickly switch back to the Git Services tab.
  5. See: the GitHub still has an authorised state as the provision request was ignored and the related oauth token was not deleted.
  6. Revoke the GitHub authorisation.
  7. See: a notification about successful revoke appears and the authorisation status is changed to unauthorised.

Release Notes

Docs PR

@che-bot
Copy link
Contributor

che-bot commented Jan 23, 2024

Click here to review and test in web IDE: Contribute

Copy link

Docker image build succeeded: quay.io/eclipse/che-dashboard:pr-1046

kubectl patch command
kubectl patch -n eclipse-che "checluster/eclipse-che" --type=json -p="[{"op": "replace", "path": "/spec/components/dashboard/deployment", "value": {containers: [{image: "quay.io/eclipse/che-dashboard:pr-1046", name: che-dashboard}]}}]"

Copy link
Member

@ibuziuk ibuziuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please, take a look at the screencast since I'm not sure PR is addressing the issue I'm facing:

Screen.Recording.2024-01-19.at.13.45.50.mov

Copy link

Docker image build succeeded: quay.io/eclipse/che-dashboard:pr-1046

kubectl patch command
kubectl patch -n eclipse-che "checluster/eclipse-che" --type=json -p="[{"op": "replace", "path": "/spec/components/dashboard/deployment", "value": {containers: [{image: "quay.io/eclipse/che-dashboard:pr-1046", name: che-dashboard}]}}]"

Signed-off-by: Oleksii Orel <oorel@redhat.com>
@olexii4
Copy link
Contributor

olexii4 commented Jan 24, 2024

@ibuziuk Could you recheck these changes? I fixed a bug and added tests.

2024-01-24.04.00.07.mov

Copy link

Docker image build succeeded: quay.io/eclipse/che-dashboard:pr-1046

kubectl patch command
kubectl patch -n eclipse-che "checluster/eclipse-che" --type=json -p="[{"op": "replace", "path": "/spec/components/dashboard/deployment", "value": {containers: [{image: "quay.io/eclipse/che-dashboard:pr-1046", name: che-dashboard}]}}]"

Copy link

codecov bot commented Jan 24, 2024

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (fb4e0c6) 88.24% compared to head (c00581e) 88.28%.
Report is 3 commits behind head on main.

Files Patch % Lines
...shboard-frontend/src/store/GitOauthConfig/index.ts 33.33% 8 Missing ⚠️
...ontend/src/store/Workspaces/devWorkspaces/index.ts 86.36% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1046      +/-   ##
==========================================
+ Coverage   88.24%   88.28%   +0.03%     
==========================================
  Files         389      390       +1     
  Lines       39845    39984     +139     
  Branches     2650     2666      +16     
==========================================
+ Hits        35163    35299     +136     
- Misses       4656     4659       +3     
  Partials       26       26              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

openshift-ci bot commented Jan 24, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ibuziuk, olexii4, vinokurig

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@vinokurig
Copy link
Contributor Author

Needs another approach.

@vinokurig vinokurig closed this Jan 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants