Ingress/Route per plugin endpoint #15432

metlos · 2019-12-09T12:47:26Z

What does this PR do?

Changes the way we create ingresses/routes, so that there is 1:1 mapping between an ingress/route and a plugin endpoint.

More than anything else, this enables 1 service to be reached from multiple URLs which hopefully helps enabling webview.

What issues does this PR fix or reference?

#15410 #15283

Additional Notes

This PR is a draft, because we've not gone through the design phase with this and I've not added any tests and most possibly broke others. Comments both on the possible better approach to this as well as the code itself are more than welcome.

…te per public endpoint instead of merging the endpoints by port and only creating ingresses/routes per exposed port. Signed-off-by: Lukas Krejci <lkrejci@redhat.com>

Signed-off-by: Lukas Krejci <lkrejci@redhat.com>

che-bot · 2019-12-09T12:47:45Z

❌ E2E Happy path tests failed ❗

See Details

Jenkins job
test report
logs and configs
Happy path tests DevFile
image: maxura/che-server:15432

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

⚠️ https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

ℹ️ Use comment "crw-ci-test" to rerun happy path E2E test.

che-bot · 2019-12-09T12:56:13Z

E2E tests of Eclipse Che Multiuser on OCP has failed:

Build details
Test report
docker image: eclipseche/che-server:15432
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.
Use command "ci-test" to rerun the test.

…erver be exposed on a unique location, separate from the other endpoints sharing the same port. Signed-off-by: Lukas Krejci <lkrejci@redhat.com>

…e-plugin-endpoints-on-the-same-port Signed-off-by: Lukas Krejci <lkrejci@redhat.com>

che-bot · 2019-12-18T16:51:25Z

✅ E2E Happy path tests succeed 🎉

See Details

Jenkins job
logs and configs
Happy path tests DevFile
image: maxura/che-server:15432

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

che-bot · 2019-12-18T17:26:08Z

E2E tests of Eclipse Che Multiuser on OCP has been successful:

Build details
Test report
docker image: eclipseche/che-server:15432

…e-plugin-endpoints-on-the-same-port Signed-off-by: Lukas Krejci <lkrejci@redhat.com>

metlos · 2019-12-20T11:41:23Z

This PR adds a new attribute called unique on the plugin endpoints. Endpoints with this attribute set to true will always be exposed separately, even if they share the port with some other endpoints.

Example configuration:

apiVersion: v2
publisher: eclipse
name: che-theia
version: next
type: Che Editor
displayName: theia-ide
title: Eclipse Theia development version.
description: Eclipse Theia, get the latest release each day.
icon: https://raw.githubusercontent.com/theia-ide/theia/master/logo/theia-logo-no-text-black.svg?sanitize=true
category: Editor
repository: https://github.com/eclipse/che-theia
firstPublicationDate: "2019-03-07"
spec:
  endpoints:
   -  name: "theia"
      public: true
      targetPort: 3100
      attributes:
        protocol: http
        type: ide
        secure: true
        cookiesAuthEnabled: true
        discoverable: false
        unique: true
...

In the current state, there seems to be some problem with single-host server strategy and this implementation - at least the welcome plugin is not able to correctly show the welcome page, but that hints possibly at some larger problem. The multi-host mode doesn't exhibit that problem though.

metlos · 2019-12-20T11:54:16Z

Note that I've uploaded a new image that contains the code from this PR on top the master in revision 4744a70: quay.io/lkrejci/che-server:7.7.0-issue-15283

azatsarynnyy · 2019-12-29T14:49:16Z

Hi @metlos,
thanks for the Che Server image. I tested it with Che Theia applying the patch eclipse-che/che-theia#591.
It works as expected. Webviews are accessible through the separate endpoint eclipse-che/che-plugin-registry#335.

Are you planning to merge this PR in master soon?

AndrienkoAleksandr · 2020-01-03T15:56:49Z

Hello, @metlos I a bit tested your pr, and found that it works only if the server attribute type is the same for server endpoints. Is it expected behavior? For me it looks badly, because actually we are using type attribute to find server from among others... @azatsarynnyy Do you agree?

…e-plugin-endpoints-on-the-same-port Signed-off-by: Lukas Krejci <lkrejci@redhat.com>

metlos · 2020-01-06T13:38:00Z

it works only if the server attribute type is the same for server endpoints.

Could you please provide some sample plugin and a description of what does not work?

azatsarynnyy · 2020-01-08T09:10:31Z

@AndrienkoAleksandr I've used this meta.yaml for testing.
There're two endpoints (theia and webviews) with the same targetPort but different type.
I've got different links for such endpoints. So, everything works as expected for me.
Maybe your case is a bit different @AndrienkoAleksandr

metlos · 2020-01-10T08:20:52Z

So given @azatsarynnyy 's comment, I think this PR is feature complete.

I am locally making some small refactorings to improve the code structure, so once that's done, I think we're ready to review and test this to get it merged.

So the outstanding tasks are the following:

Finish the refactoring
In the PR review, please focus on handling of the PreviewUrl command provisioning - the changes there are a little bit "speculative" and I would like to hear the opinions on whether it actually makes sense
Test this PR under all deployment scenarios, namely:
- Single-user, single-host,
- Multi-user, single-host,
- Single-user, multi-host,
- Multi-user, multi-host,,

While webviews are meant to only work with TLS, I think che server still needs to work in non-tls deployments because users might not be using Che Theia as their editor.

…e-plugin-endpoints-on-the-same-port Signed-off-by: Lukas Krejci <lkrejci@redhat.com>

che-bot · 2020-01-21T09:57:23Z

✅ E2E Happy path tests succeed 🎉

See Details

Jenkins job
logs and configs
Happy path tests DevFile
image: maxura/che-server:15432

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

sleshchenko · 2020-01-21T16:10:02Z

.../workspace/infrastructure/kubernetes/server/secure/jwtproxy/JwtProxySecureServerExposer.java

+
+          try {
+            ServicePort exposedServicePort =
+                proxyProvisioner.expose(


Why do we need a dedicated port for each exposableServerSet here?
It's kind of surprise me that here we are in a loop of onEachExposableServerSet invoke exposer.expose that will do iterate on onEachExposableServerSet more more time https://github.com/eclipse/che/blob/d46eb20398472a8e2a751002b1ae0ac85b5f56ef/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/server/external/ExternalServerExposer.java#L97
Did I miss something, if not - can we avoid it?

We need a dedicated port, because a server set needs to be exposed uniquely. So we need a unique route so that the user "sees" it as unique, but we also need a unique jwt service (and therefore a port), so that the backing service sees it as a request coming from a unique domain.

Good catch of calling onEachExposableServerSet twice unnecessarily. Because of the changes to correctly handle the preview URL, the exposeAsSingle method is now public on the server exposer as well, so we can use that instead in the jwt server exposer.

but we also need a unique jwt service (and therefore a port), so that the backing service sees it as a request coming from a unique domain.

Sorry, but I did not get it. Why backing service cares about domain requests come? Could you elaborate more?
In my understanding purpose why we introduce unique servers: for browser not to send all shared cookies...
And I don't understand how using one JwtProxy can break it...

skabashnyuk · 2020-01-22T09:29:26Z

@metlos can you please provide some updates to https://github.com/eclipse/che-plugin-registry#plugin-meta-yaml-structure documentation explaining the usage of unique attribute.

AndrienkoAleksandr · 2020-01-22T10:27:53Z

@metlos can you please provide some updates to https://github.com/eclipse/che-plugin-registry#plugin-meta-yaml-structure documentation explaining the usage of unique attribute.

It will be good to have this information also in the che-doc https://github.com/eclipse/che-docs/blob/master/src/main/pages/che-7/end-user-guide/ref_che-theia-plug-in-metadata.adoc
But up to you.

exposure. Signed-off-by: Lukas Krejci <lkrejci@redhat.com>

che-bot · 2020-01-22T14:24:37Z

E2E tests of Eclipse Che Multiuser on OCP has failed:

Build details
Test report
docker image: eclipseche/che-server:15432
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.
Use command "ci-test" to rerun the test.

che-bot · 2020-01-22T14:24:55Z

❌ E2E Happy path tests failed ❗

See Details

Jenkins job
test report
logs and configs
Happy path tests DevFile
image: maxura/che-server:15432

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

⚠️ https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

ℹ️ Use comment "crw-ci-test" to rerun happy path E2E test.

…3-multiple-plugin-endpoints-on-the-same-port Signed-off-by: Lukas Krejci <lkrejci@redhat.com>

sleshchenko

I suppose code can be a bit simplified by moving onExposableSet on KubernetesServerExposer level but it requires some work and I'm not sure it will work in all corner cases. It needs to try to understand it better.

So, the current state is good enough to proceed with it 👍

location. Signed-off-by: Lukas Krejci <lkrejci@redhat.com>

che-bot · 2020-01-24T16:44:12Z

❌ E2E Happy path tests failed ❗

See Details

Jenkins job
test report
logs and configs
Happy path tests DevFile
image: maxura/che-server:15432

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

⚠️ https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

ℹ️ Use comment "crw-ci-test" to rerun happy path E2E test.

che-bot · 2020-01-24T18:12:01Z

E2E tests of Eclipse Che Multiuser on OCP has failed:

Build details
Test report
docker image: eclipseche/che-server:15432
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.
Use command "ci-test" to rerun the test.

sleshchenko

Now code is even simpler 🎉

metlos · 2020-01-27T09:38:46Z

ci-test

metlos · 2020-01-27T10:42:48Z

crw-ci-test

che-bot · 2020-01-27T11:10:24Z

E2E tests of Eclipse Che Multiuser on OCP has failed:

Build details
Test report
docker image: eclipseche/che-server:15432
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.
Use command "ci-test" to rerun the test.

metlos · 2020-01-27T13:57:46Z

ci-test

metlos · 2020-01-27T14:24:49Z

crw-ci-test

che-bot · 2020-01-27T14:50:18Z

❌ E2E Happy path tests failed ❗

See Details

Jenkins job
test report
logs and configs
Happy path tests DevFile
image: maxura/che-server:15432

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

⚠️ https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

ℹ️ Use comment "crw-ci-test" to rerun happy path E2E test.

che-bot · 2020-01-27T15:23:38Z

E2E tests of Eclipse Che Multiuser on OCP has failed:

Build details
Test report
docker image: eclipseche/che-server:15432
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.
Use command "ci-test" to rerun the test.

Signed-off-by: Lukas Krejci <lkrejci@redhat.com>

…e-plugin-endpoints-on-the-same-port Signed-off-by: Lukas Krejci <lkrejci@redhat.com>

che-bot · 2020-01-27T23:33:51Z

✅ E2E Happy path tests succeed 🎉

See Details

Jenkins job
logs and configs
Happy path tests DevFile
image: maxura/che-server:15432

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

che-bot · 2020-01-27T23:54:21Z

E2E tests of Eclipse Che Multiuser on OCP has been successful:

Build details
Test report
docker image: eclipseche/che-server:15432

metlos added 2 commits December 4, 2019 15:28

Experimental fix for eclipse-che#15283. The fix creates a ingress/rou…

485efc5

…te per public endpoint instead of merging the endpoints by port and only creating ingresses/routes per exposed port. Signed-off-by: Lukas Krejci <lkrejci@redhat.com>

Fix the openshift routes to point to the correct service.

13c04b2

Signed-off-by: Lukas Krejci <lkrejci@redhat.com>

che-bot added kind/task Internal things, technical debt, and to-do tasks to be performed. status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. labels Dec 9, 2019

metlos added kind/enhancement A feature request - must adhere to the feature request template. team/platform labels Dec 9, 2019

metlos mentioned this pull request Dec 9, 2019

Bug/ multiple plugin endpoints on the same port #15431

Closed

Introduce a new server config attribute - "unique" - to require the s…

2c52500

…erver be exposed on a unique location, separate from the other endpoints sharing the same port. Signed-off-by: Lukas Krejci <lkrejci@redhat.com>

This comment has been minimized.

Sign in to view

Merge remote-tracking branch 'upstream/master' into bug/15283-multipl…

0f0a0e9

…e-plugin-endpoints-on-the-same-port Signed-off-by: Lukas Krejci <lkrejci@redhat.com>

Merge remote-tracking branch 'upstream/master' into bug/15283-multipl…

e6e2c22

…e-plugin-endpoints-on-the-same-port Signed-off-by: Lukas Krejci <lkrejci@redhat.com>

This was referenced Dec 29, 2019

Use a dedicated server as webview domain eclipse-che/che-theia#591

Merged

Add Che Theia editor endpoint to expose the plugins' webviews on a different origin to make them isolated from Che Theia origin eclipse-che/che-plugin-registry#335

Merged

Merge remote-tracking branch 'upstream/master' into bug/15283-multipl…

47ed31f

…e-plugin-endpoints-on-the-same-port Signed-off-by: Lukas Krejci <lkrejci@redhat.com>

azatsarynnyy mentioned this pull request Jan 8, 2020

Make Theia WebViews more secure #15616

Closed

metlos added 2 commits January 15, 2020 09:12

Merge remote-tracking branch 'upstream/master' into bug/15283-multipl…

c05b0b5

…e-plugin-endpoints-on-the-same-port Signed-off-by: Lukas Krejci <lkrejci@redhat.com>

Merge remote-tracking branch 'upstream/master' into bug/15283-multipl…

7c80efa

…e-plugin-endpoints-on-the-same-port Signed-off-by: Lukas Krejci <lkrejci@redhat.com>

metlos marked this pull request as ready for review January 17, 2020 11:40

metlos requested a review from amisevsk as a code owner January 17, 2020 11:40

sparkoo approved these changes Jan 21, 2020

View reviewed changes

sleshchenko reviewed Jan 21, 2020

View reviewed changes

Don't call forEachExposableServerSet unnecessarily during JWT proxy

d2e7bc6

exposure. Signed-off-by: Lukas Krejci <lkrejci@redhat.com>

Merge branch 'master' of https://github.com/eclipse/che into bug/1528…

76cf66a

…3-multiple-plugin-endpoints-on-the-same-port Signed-off-by: Lukas Krejci <lkrejci@redhat.com>

sleshchenko approved these changes Jan 24, 2020

View reviewed changes

Rework the flow such that the servers are split to sets only on a single

a64818a

location. Signed-off-by: Lukas Krejci <lkrejci@redhat.com>

sleshchenko approved these changes Jan 27, 2020

View reviewed changes

metlos added 2 commits January 27, 2020 23:22

The serverId and serverName were passed in swapped. d'oh!

33610ea

Signed-off-by: Lukas Krejci <lkrejci@redhat.com>

Merge remote-tracking branch 'upstream/master' into bug/15283-multipl…

42040b8

…e-plugin-endpoints-on-the-same-port Signed-off-by: Lukas Krejci <lkrejci@redhat.com>

metlos merged commit d1c45ef into eclipse-che:master Jan 27, 2020

che-bot added this to the 7.8.0 milestone Jan 27, 2020

che-bot removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Jan 27, 2020

amisevsk mentioned this pull request Feb 12, 2020

[workspace-operator] Adapt controller to support unique ingresses #15990

Closed

azatsarynnyy mentioned this pull request Feb 13, 2020

Plugins do not work in the latest Che-Theia build (che-theia:next) #16021

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ingress/Route per plugin endpoint #15432

Ingress/Route per plugin endpoint #15432

metlos commented Dec 9, 2019 •

edited by sleshchenko

Loading

che-bot commented Dec 9, 2019

che-bot commented Dec 9, 2019

This comment has been minimized.

che-bot commented Dec 18, 2019

che-bot commented Dec 18, 2019

metlos commented Dec 20, 2019

metlos commented Dec 20, 2019

azatsarynnyy commented Dec 29, 2019

AndrienkoAleksandr commented Jan 3, 2020

metlos commented Jan 6, 2020

azatsarynnyy commented Jan 8, 2020

metlos commented Jan 10, 2020 •

edited

Loading

che-bot commented Jan 21, 2020

sleshchenko Jan 21, 2020

metlos Jan 22, 2020

sleshchenko Jan 22, 2020

skabashnyuk commented Jan 22, 2020

AndrienkoAleksandr commented Jan 22, 2020

che-bot commented Jan 22, 2020

che-bot commented Jan 22, 2020

sleshchenko left a comment

che-bot commented Jan 24, 2020

che-bot commented Jan 24, 2020

sleshchenko left a comment

metlos commented Jan 27, 2020

metlos commented Jan 27, 2020

che-bot commented Jan 27, 2020

metlos commented Jan 27, 2020

metlos commented Jan 27, 2020

che-bot commented Jan 27, 2020

che-bot commented Jan 27, 2020

che-bot commented Jan 27, 2020

che-bot commented Jan 27, 2020

Ingress/Route per plugin endpoint #15432

Ingress/Route per plugin endpoint #15432

Conversation

metlos commented Dec 9, 2019 • edited by sleshchenko Loading

What does this PR do?

What issues does this PR fix or reference?

Additional Notes

che-bot commented Dec 9, 2019

che-bot commented Dec 9, 2019

This comment has been minimized.

che-bot commented Dec 18, 2019

che-bot commented Dec 18, 2019

metlos commented Dec 20, 2019

metlos commented Dec 20, 2019

azatsarynnyy commented Dec 29, 2019

AndrienkoAleksandr commented Jan 3, 2020

metlos commented Jan 6, 2020

azatsarynnyy commented Jan 8, 2020

metlos commented Jan 10, 2020 • edited Loading

che-bot commented Jan 21, 2020

sleshchenko Jan 21, 2020

Choose a reason for hiding this comment

metlos Jan 22, 2020

Choose a reason for hiding this comment

sleshchenko Jan 22, 2020

Choose a reason for hiding this comment

skabashnyuk commented Jan 22, 2020

AndrienkoAleksandr commented Jan 22, 2020

che-bot commented Jan 22, 2020

che-bot commented Jan 22, 2020

sleshchenko left a comment

Choose a reason for hiding this comment

che-bot commented Jan 24, 2020

che-bot commented Jan 24, 2020

sleshchenko left a comment

Choose a reason for hiding this comment

metlos commented Jan 27, 2020

metlos commented Jan 27, 2020

che-bot commented Jan 27, 2020

metlos commented Jan 27, 2020

metlos commented Jan 27, 2020

che-bot commented Jan 27, 2020

che-bot commented Jan 27, 2020

che-bot commented Jan 27, 2020

che-bot commented Jan 27, 2020

metlos commented Dec 9, 2019 •

edited by sleshchenko

Loading

metlos commented Jan 10, 2020 •

edited

Loading