che-server icon indicating copy to clipboard operation
che-server copied to clipboard
verified publisher icon eclipse-che

Delete personal access tokens without provider name annotation

Open vinokurig opened this issue 1 year ago • 22 comments

What does this PR do?

Check a personal access token to have the che.eclipse.org/scm-provider-name annotation instead of the deprecated che.eclipse.org/scm-personal-access-token-name. All token secrets that were created before the 7.84 version, will be removed. A new secret with the new annotation will be created on workspace create/start.

Screenshot/screencast of this PR

What issues does this PR fix or reference?

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

How to test this PR?

  1. Deploy che from the PR image: quay.io/eclipse/che-server:pr-685.
  2. Setup GitHub oauth.
  3. Start a workspace from the GitHub repository with a devfile.
  4. Go to the Che Dashboard -> User Preference -> Personal Access Tokens tab, see: a token is added to the list.
  5. Go to Openshift console Workloads -> Secrets and find the personal-access-token-... secret.
  6. Open the secret's YAML and remove the che.eclipse.org/scm-provider-name annotation.
  7. Go to the Che Dashboard -> User Preference -> Personal Access Tokens tab (refresh the page if needed), see: the token item is removed.
  8. Delete the previously created workspace and start a new one from the same repository url.
  9. Go to the Che Dashboard -> User Preference -> Personal Access Tokens tab, see: a new token is added to the list.

PR Checklist

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

Reviewers

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

vinokurig avatar May 15 '24 07:05 vinokurig

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 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

openshift-ci[bot] avatar May 15 '24 07:05 openshift-ci[bot]

/retest

vinokurig avatar May 15 '24 09:05 vinokurig

/retest

vinokurig avatar May 15 '24 12:05 vinokurig

/retest

vinokurig avatar May 20 '24 08:05 vinokurig

@vinokurig, the test gitlab-with-oauth-setup-flow is fail due to the known issue -> https://github.com/eclipse-che/che/issues/22953 the test gitlab-with-pat-setup-flow is fail due to expiration of Personal Access Token on the test GitLab account. (see the issue above)

So, I think the PR can be merged.

artaleks9 avatar May 23 '24 11:05 artaleks9

/retest

artaleks9 avatar May 23 '24 12:05 artaleks9

/retest

artaleks9 avatar May 23 '24 22:05 artaleks9

/retest

artaleks9 avatar May 24 '24 08:05 artaleks9

/retest

artaleks9 avatar May 27 '24 06:05 artaleks9

/retest

artaleks9 avatar May 30 '24 07:05 artaleks9

For some reason that is still unclear to me, the gitlab-with-pat-setup-flow test constantly fails In all other PRs the test passes... I, moreover, checked the functionality - it works as expected.. Perhaps I didn’t take something into account or I need to carefully repeat all the steps of the test... In general, if there is such a possibility, I want to ask you don't merge this PR I'll try to find time and investigate this.

artaleks9 avatar May 30 '24 10:05 artaleks9

@vinokurig

  1. So...The test gitlab-with-pat-setup-flow is failed due to deleting Secret personal-access-token from <user>-che namespace after adding it. If to add the che.eclipse.org/scm-provider-name annotation to PAT yaml - it works properly.

  2. The PAT for gitlab.com is added successfully, but it was observed the bug with deleting it from User Preferences on Dashboard.

See the screenshot:

pr685-User-Preferences

artaleks9 avatar Jun 05 '24 09:06 artaleks9

@artaleks9

So...The test gitlab-with-pat-setup-flow is failed due to deleting Secret personal-access-token from -che namespace after adding it. If to add the che.eclipse.org/scm-provider-name annotation to PAT yaml - it works properly.

The pull request already changes the token secret content. I add the provider name label to the yaml template. However the test fails because it fetches the template from main, not from the pr branch.

vinokurig avatar Jun 06 '24 07:06 vinokurig

@vinokurig I checked new sample of .ci/openshift-ci/pat-secret.yaml and discovered the Secret is deleting from the <user>-che namespace after adding anyway.

If to change the values in the annotations of pat-secret.yaml, then it works:

    che.eclipse.org/scm-personal-access-token-name: git-provider-name
    che.eclipse.org/scm-provider-name: <some name>

I mean, seems the che.eclipse.org/scm-personal-access-token-name is still need to have specific provider name, like gitlab. Please take a look on it.

artaleks9 avatar Jun 06 '24 08:06 artaleks9

@artaleks9

The PAT for gitlab.com is added successfully, but it was observed the bug with deleting it from User Preferences on Dashboard.

This can be reproduced on dogfooding instance, so this is not related to the PR. Please create an issue for that. Note that the token must be added by the oc apply command in this case.

vinokurig avatar Jun 07 '24 09:06 vinokurig

@artaleks9

If to change the values in the annotations of pat-secret.yaml, then it works:

  che.eclipse.org/scm-personal-access-token-name: git-provider-name
  che.eclipse.org/scm-provider-name: <some name>

I mean, seems the che.eclipse.org/scm-personal-access-token-name is still need to have specific provider name, like gitlab. Please take a look on it.

fixed

vinokurig avatar Jun 07 '24 11:06 vinokurig

The bitbucket-no-pat-oauth-flow-raw-devfile-url is fail with an error:

error: timed out waiting for the condition on devworkspaces/public-repo-wksp-testname

I guess, it's related to an infrastructure problem. Doing to rerun.

artaleks9 avatar Jun 07 '24 15:06 artaleks9

/retest

artaleks9 avatar Jun 07 '24 15:06 artaleks9

@vinokurig

@artaleks9

The PAT for gitlab.com is added successfully, but it was observed the bug with deleting it from User Preferences on Dashboard.

This can be reproduced on dogfooding instance, so this is not related to the PR. Please create an issue for that. Note that the token must be added by the oc apply command in this case.

I deployed Che-next and can't reproduce it, PAT can be deleted from User Preferences after asdding by oc apply -f gitlab-com-stringdata-pat.yaml -n user-che command.

I tried two variants of gitlab-com-stringdata-pat.yaml - both worked properly:

apiVersion: v1
kind: Secret
metadata:
  name: personal-access-token-gitlab
  labels:
    app.kubernetes.io/part-of: che.eclipse.org
    app.kubernetes.io/component: scm-personal-access-token
  annotations:
    che.eclipse.org/che-userid: 'cc787d86-941b-4076-9afb-a37f67b9c3db'
    che.eclipse.org/scm-personal-access-token-name: gitlab
    che.eclipse.org/scm-url: 'https://gitlab.com'
stringData:
  token: <pat>
type: Opaque

apiVersion: v1
kind: Secret
metadata:
  name: personal-access-token-gitlab
  labels:
    app.kubernetes.io/part-of: che.eclipse.org
    app.kubernetes.io/component: scm-personal-access-token
  annotations:
    che.eclipse.org/che-userid: 'cc787d86-941b-4076-9afb-a37f67b9c3db'
    che.eclipse.org/scm-personal-access-token-name: gitlab
    che.eclipse.org/scm-provider-name: gitlab
    che.eclipse.org/scm-url: 'https://gitlab.com'
stringData:
  token: <pat>
type: Opaque

Che-next and dogfooding instance - are the same or not ?

artaleks9 avatar Jun 10 '24 13:06 artaleks9

@artaleks9

Che-next and dogfooding instance - are the same or not ?

Not sure, could you reproduce the case on dogfooding instance?

vinokurig avatar Jun 10 '24 13:06 vinokurig

@vinokurig

@artaleks9

Che-next and dogfooding instance - are the same or not ?

Not sure, could you reproduce the case on dogfooding instance?

I also can't reproduce it on che-dogfooding.apps.che-dev.x6e0.p1.openshiftapps.com...PAT is deleted on Dashboard after adding by oc apply.

artaleks9 avatar Jun 10 '24 14:06 artaleks9

@artaleks9

I also can't reproduce it on che-dogfooding.apps.che-dev.x6e0.p1.openshiftapps.com...PAT is deleted on Dashboard after adding by oc apply.

Screenshot 2024-07-11 at 11 57 08 This screenshot was taken from the dogfooding instance.

vinokurig avatar Jul 11 '24 08:07 vinokurig

@ibuziuk

would it affect PAT that were configured before 7.80.0 ? my understanding that they would be brutally deleted after new version and if a user leveraged PAT flow they would not be able to work until configuring a new PAT from scratch, right?

yes, looks like we need a decision here

vinokurig avatar Jul 30 '24 11:07 vinokurig

Not a priority issue.

vinokurig avatar Sep 13 '24 09:09 vinokurig