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

Rwork the token cleanup mechanism #719

Merged
merged 1 commit into from
Sep 10, 2024
Merged

Rwork the token cleanup mechanism #719

merged 1 commit into from
Sep 10, 2024

Conversation

vinokurig
Copy link
Contributor

@vinokurig vinokurig commented Sep 9, 2024

What does this PR do?

Rework the token secrets cleanup mechanism by fetching kubernetes secrets and deleting the redundant secrets directly, instead of fetching personal access tokens and then deleting the secrets according to the PAT objects.

Screenshot/screencast of this PR

What issues does this PR fix or reference?

https://issues.redhat.com/browse/CRW-7185

How to test this PR?

  1. Deploy che with the PR image: quay.io/eclipse/che-server:pr-719
  2. Configure GitHub oauth
  3. Enable the experimental feature that forces a refresh of the personal access token on workspace startup
  4. Start a workspace from a GitHub repository with a devfile.
  5. Go to Dashboard user preferences -> personal access tokens

See: only one personal access token item is present in the list

PR Checklist

As the author of this Pull Request I made sure that:

Release Notes

Fix a bug when the list of PATs in the dashboard has redundant tokens if the refresh mode is enabled.

Reviewers

Reviewers, please comment how you tested the PR when approving it.

try {
for (KubernetesNamespaceMeta namespaceMeta : namespaceFactory.list()) {
List<Secret> secrets = doGetPersonalAccessTokenSecrets(namespaceMeta);
for (int i = 1; i < secrets.size(); i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we remove all secrets or keep the last one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to keep the last one

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems we keep the very first one, should it be the last one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vinokurig
Copy link
Contributor Author

/retest

Copy link

openshift-ci bot commented Sep 10, 2024

@vinokurig: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/v14-gitlab-with-oauth-setup-flow 2d464ea link true /test v14-gitlab-with-oauth-setup-flow

Full PR test history. Your PR dashboard.

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 kubernetes-sigs/prow repository. I understand the commands that are listed here.

Copy link
Contributor

@SkorikSergey SkorikSergey left a comment

Choose a reason for hiding this comment

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

Checked on Che 7.91.0 with che-server PR image. There is only one token as expected.

Copy link

openshift-ci bot commented Sep 10, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: SkorikSergey, tolusha, 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 vinokurig merged commit 3445db7 into main Sep 10, 2024
27 of 28 checks passed
@vinokurig vinokurig deleted the CRW-7185 branch September 10, 2024 14:10
@devstudio-release
Copy link

Build 3.17 :: server_3.x/357: Console, Changes, Git Data

@devstudio-release
Copy link

@devstudio-release
Copy link

@devstudio-release
Copy link

Build 3.17 :: get-sources-rhpkg-container-build_3.x/7686: FAILURE

server : 3.x :: Failed in 64104174 : BREW:BUILD/STATUS:UNKNOWN
FAILURE:; copied to quay

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