Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

Support more than one authentication session #906

Merged
merged 5 commits into from
Nov 9, 2020
Merged

Support more than one authentication session #906

merged 5 commits into from
Nov 9, 2020

Conversation

vinokurig
Copy link
Contributor

@vinokurig vinokurig commented Nov 4, 2020

Signed-off-by: Igor Vinokur ivinokur@redhat.com

What does this PR do?

  • Handle multiple GitHub authentication sessions.
  • Add back the vscode built-in github plugin to be able to publish https repositories.

Screenshot/screencast of this PR

What issues does this PR fix or reference?

fixes eclipse-che/che#18227

How to test this PR?

  1. Setup GitHub Identity provider
  2. Start a workspace with GitHub Pull Requests plugin and a project cloned from GitHub by https url.
    You can use this devfile to start a workspace:
apiVersion: 1.0.0
metadata:
  name: wksp-custom-dwdiu
projects:
  - name: che-theia
    source:
      location: 'https://github.com/eclipse/che-theia.git'
      type: git
      branch: master
components:
  - type: chePlugin
    reference: 'https://raw.githubusercontent.com/eclipse/che-plugin-registry/master/v3/plugins/ms-vscode/vscode-github-pullrequest/0.20.0/meta.yaml'
  - type: cheEditor
    reference: 'https://raw.githubusercontent.com/chepullreq4/pr-check-files/master/che-theia/pr-906/simple/che-theia-editor.yaml'
  1. Sign-in to the GitHub Pull Requests plugin.
  2. Checkout to a new branch and commit some changes.
  3. Open a new Pull Request, see https://code.visualstudio.com/docs/editor/github#_creating-pull-requests

The GitHub Pull Requests plugin publishes the branch to origin, so an additional GitHub authentication session is requested.

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.

Happy Path Channel

HAPPY_PATH_CHANNEL=stable

Signed-off-by: Igor Vinokur <ivinokur@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Nov 4, 2020

✅ E2E Happy path tests succeed 🎉

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia docker.io/maxura/che-theia:906
che-theia-endpoint-runtime-binary docker.io/maxura/che-theia-endpoint-runtime-binary:906

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

  • Use comment "[crw-ci-test]" to rerun happy path E2E test.
  • Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

@che-bot
Copy link
Contributor

che-bot commented Nov 4, 2020

✅ E2E Happy path tests succeed 🎉

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia docker.io/maxura/che-theia:906
che-theia-endpoint-runtime-binary docker.io/maxura/che-theia-endpoint-runtime-binary:906

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

  • Use comment "[crw-ci-test]" to rerun happy path E2E test.
  • Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

@che-bot
Copy link
Contributor

che-bot commented Nov 5, 2020

✅ E2E Happy path tests succeed 🎉

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia docker.io/maxura/che-theia:906
che-theia-endpoint-runtime-binary docker.io/maxura/che-theia-endpoint-runtime-binary:906

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

  • Use comment "[crw-ci-test]" to rerun happy path E2E test.
  • Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

session = {
id: 'github-session',
const session = {
id: v4(),
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need uuid or just a counter ?

Copy link
Member

@azatsarynnyy azatsarynnyy left a comment

Choose a reason for hiding this comment

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

Maybe, I'm doing something wrong, but here's how it looks on my side:

Peek 2020-11-05 22-34

@vinokurig
Copy link
Contributor Author

@azatsarynnyy
Looks like you are using an old che-plugin-registry image, so the GitHub PR plugin is not updated there.
Please set the version of the che-plugin-registry image to nightly:

  1. Run kubectl edit checluster eclipse-che -n che
  2. Press a key and add pluginRegistryImage: quay.io/eclipse/che-plugin-registry:nightly
  3. Press Esc key and enter wq to apply changes

@vinokurig
Copy link
Contributor Author

@benoitf

do we need uuid or just a counter ?

uuid looks simpler for me.

@benoitf
Copy link
Contributor

benoitf commented Nov 6, 2020

@vinokurig I think the devfile you're suggesting, you should use reference: and link to the master version of plug-in registry meta.yaml so devfile, host it on a gist, and provide the link directly so it can be tested with che.openshift.io without any user action

@vinokurig
Copy link
Contributor Author

@benoitf

I think the devfile you're suggesting, you should use reference: and link to the master version of plug-in registry meta.yaml so devfile, host it on a gist, and provide the link directly so it can be tested with che.openshift.io without any user action

Yes you are right, I've edited the description.

@vinokurig
Copy link
Contributor Author

@azatsarynnyy Please use the edited devfile from the description.

@azatsarynnyy
Copy link
Member

@azatsarynnyy Please use the edited devfile from the description.

Thanks @vinokurig ! Checking...

@azatsarynnyy
Copy link
Member

@vinokurig should the test Devfile also include Che Theia Editor from this PR?

apiVersion: 1.0.0
metadata:
  name: wksp-custom-dwdiu
projects:
  - name: che-theia
    source:
      location: 'https://github.com/eclipse/che-theia.git'
      type: git
components:
  - type: chePlugin
    reference: 'https://raw.githubusercontent.com/eclipse/che-plugin-registry/master/v3/plugins/ms-vscode/vscode-github-pullrequest/0.20.0/meta.yaml'
  - type: cheEditor
    reference: https://raw.githubusercontent.com/chepullreq4/pr-check-files/master/che-theia/pr-906/simple/che-theia-editor.yaml

@che-bot
Copy link
Contributor

che-bot commented Nov 9, 2020

❌ E2E Happy path tests failed ❗

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia docker.io/maxura/che-theia:906
che-theia-endpoint-runtime-binary docker.io/maxura/che-theia-endpoint-runtime-binary:906

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

  • Use comment "[crw-ci-test]" to rerun happy path E2E test.
  • Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

@vinokurig vinokurig force-pushed the che-18227 branch 2 times, most recently from d1a5c9e to 23d8811 Compare November 9, 2020 08:59
@@ -9,8 +9,9 @@
# Red Hat, Inc. - initial API and implementation
###
# Builder Image
#
FROM #{INCLUDE:docker/${BUILD_IMAGE_TARGET}/builder-from.dockerfile}
#yarn
Copy link
Contributor

Choose a reason for hiding this comment

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

typo ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, removed the leftovers .

@che-bot
Copy link
Contributor

che-bot commented Nov 9, 2020

❌ E2E Happy path tests failed ❗

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia docker.io/maxura/che-theia:906
che-theia-endpoint-runtime-binary docker.io/maxura/che-theia-endpoint-runtime-binary:906

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

  • Use comment "[crw-ci-test]" to rerun happy path E2E test.
  • Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

Signed-off-by: Igor Vinokur <ivinokur@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Nov 9, 2020

✅ E2E Happy path tests succeed 🎉

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia docker.io/maxura/che-theia:906
che-theia-endpoint-runtime-binary docker.io/maxura/che-theia-endpoint-runtime-binary:906

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

  • Use comment "[crw-ci-test]" to rerun happy path E2E test.
  • Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

@benoitf
Copy link
Contributor

benoitf commented Nov 9, 2020

does it require eclipse-theia/theia#8725 ?

@vinokurig
Copy link
Contributor Author

No, they can be merged separately

@vinokurig
Copy link
Contributor Author

@azatsarynnyy

@vinokurig should the test Devfile also include Che Theia Editor from this PR?

apiVersion: 1.0.0
metadata:
 name: wksp-custom-dwdiu
projects:
 - name: che-theia
   source:
     location: 'https://github.com/eclipse/che-theia.git'
     type: git
components:
 - type: chePlugin
   reference: 'https://raw.githubusercontent.com/eclipse/che-plugin-registry/master/v3/plugins/ms-vscode/vscode-github->pullrequest/0.20.0/meta.yaml'
 - type: cheEditor
   reference: https://raw.githubusercontent.com/chepullreq4/pr-check-files/master/che-theia/pr-906/simple/che-theia-editor.yaml

Thank you for the suggestion, added it to the description.

Copy link
Contributor

@benoitf benoitf left a comment

Choose a reason for hiding this comment

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

It worked
(using the devfile of description, devfile of the latest comment has an invalid -> string)

image

But it is asked me again to authenticate with github
image

I used the same workspace but it is asking me several times to use github authentication while I didn't restart che, etc.

@vinokurig
Copy link
Contributor Author

The built in vscode github plugin is a separate plugin which requires another GitHub authentication session. It doesn't mean that if you signed in the GitHub Pull Requests plugin the built in GitHub plugin signed in. vscode behaves in the same way.

@benoitf
Copy link
Contributor

benoitf commented Nov 9, 2020

I understand that it's on different plug-ins but still, the user experience is not good.
I'm expecting single sign-on between git plug-in and github PR plug-in

Copy link
Contributor

@benoitf benoitf left a comment

Choose a reason for hiding this comment

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

forgot to approve this morning

@vinokurig vinokurig merged commit ca076c6 into master Nov 9, 2020
@vinokurig vinokurig deleted the che-18227 branch November 9, 2020 15:25
vinokurig pushed a commit that referenced this pull request Apr 6, 2021
* Update Che-Theia repository localtion

* Update Che-Theia repository location

Signed-off-by: Artem Zatsarynnyi <azatsary@redhat.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can not open a new Pull Request from the vscode GitHub PR plugin
4 participants