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

Make AppDefinitions more powerful in landing pages #349

Closed
iyannsch opened this issue Sep 5, 2024 · 5 comments · Fixed by #361
Closed

Make AppDefinitions more powerful in landing pages #349

iyannsch opened this issue Sep 5, 2024 · 5 comments · Fixed by #361
Labels
enhancement New feature or request

Comments

@iyannsch
Copy link

iyannsch commented Sep 5, 2024

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

@iyannsch iyannsch added the enhancement New feature or request label Sep 5, 2024
@lucas-koehler
Copy link
Contributor

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 eclipsesource/theia-cloud-website#37 to rectify this.

@iyannsch
Copy link
Author

iyannsch commented Sep 5, 2024

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
Copy link
Contributor

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

@lucas-koehler
Copy link
Contributor

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
Copy link
Author

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

lucas-koehler added a commit that referenced this issue Oct 22, 2024
### App definition endpoint
- Add AppDefinitionResource with endpoint to list all app definitions.
  The endpoint is restricted to authenticated users but is still available in anonymous mode

### Sensitive data redaction
- Add `SensitiveData` annotation to mark properties that should not be serialized publicly by Jackson.
- Add a corresponding serializer and serializer modifiert for Jackson and register the modifier in the service
- Add unit tests for the serializer

### Javascript API
- Update openapi.json from service
- Regenerate api code
- Add AppDefinitions namespace with function to list app definitions

### Testing Page
- Add button to get app definitions
- Apply formatting rules to App.tsx by saving it
- Minor fix in example keycloak URL to remove obsolete `/auth`

### Misc
- Add mockito dependency to the common maven module
lucas-koehler added a commit that referenced this issue Oct 22, 2024
### App definition endpoint
- Add AppDefinitionResource with endpoint to list all app definitions.
  The endpoint is restricted to authenticated users but is still available in anonymous mode

### Sensitive data redaction
- Add `SensitiveData` annotation to mark properties that should not be serialized publicly by Jackson.
- Add a corresponding serializer and serializer modifiert for Jackson and register the modifier in the service
- Add unit tests for the serializer

### Javascript API
- Update openapi.json from service
- Regenerate api code
- Add AppDefinitions namespace with function to list app definitions

### Testing Page
- Add button to get app definitions
- Apply formatting rules to App.tsx by saving it
- Minor fix in example keycloak URL to remove obsolete `/auth`

### Misc
- Add mockito dependency to the common maven module
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants