eclipse-theia / theia-cloud

Eclipse Public License 2.0
56 stars 31 forks source link

Make `AppDefinition`s more powerful in landing pages #349

Open iyannsch opened 2 weeks ago

iyannsch commented 2 weeks ago

Is your feature request related to a problem? Please describe.

Currently, a landing page only knows about those appDefinitons that are configured for use in the helm chart under landingPage.additionalApps. This introduces two problematic aspects and leaves a lot of potential aside.

Administrative effort

When a new AppDefinition is introduced to the cluster, it's not available whatsoever. An administrator has to make manually add it to the helm chart using the resource's exact name and decide on a label. This process is cumbersome and prone to typos.

Loss of information

While the landing page and the respective section in the helm chart only accept a name and label per additional app, the AppDefinition in K8s offers way more information. Some of it might not be useful for the landing page, other might provide value (resource limits, image name). Due to the limitation of having to manually link landingPage.additionalApps with the existing resources, information is lost.

Describe the solution you'd like

We are currently building a landing page suited for multiple use cases introducing a lot of complexity here. For that it would be very helpful to make the AppDefinitions more powerful and push more decision making power to the landing page. I think of the following features:

Auto-Discovering suitable AppDefinitions

Instead of having to manually configure additionalApps in the helm chart, I propose the landing page to automatically discover suitable AppDefinitions in the cluster/namespace and load them alongside their complete properties. This would solve both problems described above. In the current Theia Cloud setup, the operator already has API access to K8s and could provide the landing page with internal information about available definitions. This introduces the problem that all existing definitions would be visible to a landing page.

Add key-value configuration of AppDefinitions

The AppDefinition resource could be enlarged even further by add a key-value configuration mechanism utilized during manifest creation. Following the argumentation above, I would propose to add a showAtLandingpage: false entry that could decide whether it's auto-discoverable. Apart from this advantage, the mechanism also introduces powerful logic into landing pages, e.g., by multiplexing in respect to path parameters. We could automatically start a blueprint whenever it's provided via the ?appDef path parameter but show a well-designed overview of all available apps when started without. This overview could be parametrized by (exemplary) key-value information like

"showAtLandingpage": true,
"values": {
   "public": true,
   "language": "Java",
   "learnability": "Easy",
   "built_by": "Jon Doe",
}

Describe alternatives you've considered

We could fork both the theia-cloud-helm and theia-cloud repos and enlarge the set of configurable information passed between them (landingPage.additionalApps). This would unlock all user-facing advantages but still require considerable manual efforts and potential for errors.

Cluster provider

No response

Additional information

No response

lucas-koehler commented 2 weeks ago

Hi @iyannsch , thank you for the elaborate suggestions. What and how to implement this needs discussion with the team but here are my initial thoughts.

Administrative effort When a new AppDefinition is introduced to the cluster, it's not available whatsoever. An administrator has to make manually add it to the helm chart using the resource's exact name and decide on a label. This process is cumbersome and prone to typos.

This is true. However, the current approach and landing page are more of a starting point rather than a production-ready page. IMO for this use case the current approach with the Helm chart is sufficient. I agree that this is not ideal for production level landing pages and it would be helpful if landing pages can automatically discover all available app definitions. More thoughts on this below.

Loss of information While the landing page and the respective section in the helm chart only accept a name and label per additional app, the AppDefinition in K8s offers way more information. Some of it might not be useful for the landing page, other might provide value (resource limits, image name). Due to the limitation of having to manually link landingPage.additionalApps with the existing resources, information is lost.

Similar to above, the current approach is IMO sufficient for the provided landing page but not optimal for more elaborate landing pages.

Auto-Discovering suitable AppDefinitions Instead of having to manually configure additionalApps in the helm chart, I propose the landing page to automatically discover suitable AppDefinitions in the cluster/namespace and load them alongside their complete properties. This would solve both problems described above. In the current Theia Cloud setup, the operator already has API access to K8s and could provide the landing page with internal information about available definitions. This introduces the problem that all existing definitions would be visible to a landing page.

I'm not sure if this is required for the example landing page but it certainly would be helpful to enable custom landing pages to query available app definitions.

The operator should not be involved in this, though. The operator does not and will not offer any directly accessible API. It only handles custom resources (i.e. app definitions, sessions, workspaces). If you are interested, you can find more information on the operator pattern in the Kubernetes documentation.

Instead, an endpoint could be added to the REST service to expose available app definitions: The REST service is the external entry point to interact with Theia Cloud. For instance, the landing page calls it to launch a new session. When exposing app definitions here, we need to consider whether app definitions should be exposed publicly. If not all information should be exposed, a stripped down version could be returned.

Add key-value configuration of AppDefinitions The AppDefinition resource could be enlarged even further by add a key-value configuration mechanism utilized during manifest creation. Following the argumentation above, I would propose to add a showAtLandingpage: false entry that could decide whether it's auto-discoverable.

Adding an explicit property showAtLandingpage seems a bit strange to me: The app definitions (as well as the other custom resources) are meant to describe the cluster state to be processed by the operator. Adding a UI property there seems like a mixture of concerns. However, this is certainly open for discussion.

Regarding the key-value pairs. This should already be possible via the options property of app definitions. It allows to add arbitrary key value pairs. I think this should solve this point? I noticed that the options property is not documented on the website and, thus, added issue https://github.com/eclipsesource/theia-cloud-website/issues/37 to rectify this.

iyannsch commented 2 weeks ago

Thanks for your thoughts, I agree with all of your aspects! The current state of Theia (Cloud) is already awesome and one cannot implement everything in the first run - I just wanted to hint at possible options to make it even better from an outside perspective. Especially the aspect of limiting the informations passed from the manifest to the landing page is bothering me as this also limits the usability of the options..

Looking forward to hearing decisions or takeaways from you internal discussion about it :)

lucas-koehler commented 1 week ago

We'll discuss this next week as some project members are currently on vacation :)

lucas-koehler commented 5 days ago

Hi @iyannsch, we decided that we are going to expose the app definitions via an endpoint in the REST service. We won't add the explicit property showAtLandingpage due to the managed conflict of concerns. However, this could then be done custom by adding such a property to the app definition's options. When exactly we implement this will be prioritized with Jonas.

iyannsch commented 5 days ago

Awesome! 🔥 Please let me know once you know a timeline for the feature or I can support you with anything :)