eclipse-theia / eclipse-theia/theia-cloud

[operator] Cannot customize TheiaCloudClient due to overriding restrictions in Guice

Open
#489 0 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Java
Stars
99
Forks
49
Avg merge
2h 33m
Merged PRs (30d)
1

Description

### Describe the bug

Follow-up of https://github.com/eclipse-theia/theia-cloud/issues/433 that has been closed without any resolution.

From https://github.com/eclipse-theia/theia-cloud/issues/433#issue-3188840367:

> ### Describe the bug
> The current design of `AbstractTheiaCloudOperatorModule` in Theia Cloud does not support customization of certain dependencies, particularly when attempting to override `@Provides` methods using `Modules.override`.
>
> Our use case:
>
> When we enable the `activity-tracker`, additional logic in `TheiaCloudClient` (such as calls to `getClusterIPFromSessionName`) is triggered. To inspect (errors like `No ClusterIP found for session`) and enhance this behavior, we subclass `TheiaCloudClient` and want to inject our custom version.
>
> However, the framework defines a `@Provides` method like the following:
>
> @Provides
> @Singleton
> protected TheiaCloudClient provideTheiaCloudClient(NamespacedKubernetesClient client) {
> return new DefaultTheiaCloudClient(client);
> }
> #### Problem 1: Overriding the provider method directly causes a runtime error
> @Override
> protected TheiaCloudClient provideTheiaCloudClient(NamespacedKubernetesClient client) {
> return new MyTheiaCloudClient(client);
> }
> Results in:
>
> ```
> Overriding @Provides methods is not allowed.
> @Provides method: AbstractTheiaCloudOperatorModule.provideTheiaCloudClient()
> overridden by: MyOperatorModule.provideTheiaCloudClient()
> ```
>
> #### Problem 2: Defining a second provider method causes a binding conflict
> @Provides
> @Singleton
> protected TheiaCloudClient provideCustomTheiaCloudClient(NamespacedKubernetesClient client) {
> return new MyTheiaCloudClient(client);
> }
> Produces:
>
> ```
> [Guice/BindingAlreadySet]: TheiaCloudClient was bound multiple times.
> Bound at:
> 1 : MyMonitorMessagingServiceTest$1.configure(MyMonitorMessagingServiceTest.java:91)
> \_ installed by: Modules$OverrideModule -> MyMonitorMessagingServiceTest$1
> 2 : AbstractTheiaCloudOperatorModule.provideTheiaCloudClient(AbstractTheiaCloudOperatorModule.java:165)
> \_ installed by: Modules$OverrideModule -> MyOperatorModule
> ```
>
> #### Problem 3: `Modules.override(...)` cannot return an `AbstractTheiaCloudOperatorModule` type
> Attempting to override with:
>
> @Override
> public AbstractTheiaCloudOperatorModule createModule(TheiaCloudOperatorArguments arguments) {
> return Modules.override(new MyOperatorModule(arguments)).with(new AbstractModule() {
> @Override
> protected void configure() {
> // attempt to customize the provider
> }
> });
> }
> ...fails at compile time because `Modules.override(...)` returns only a `Module`, not an `AbstractTheiaCloudOperatorModule`.
>
> This restriction exists even though [the launcher does not use the specific API of `AbstractTheiaCloudOperatorModule`](https://github.com/eclipse-theia/theia-cloud/blob/55813e2c212b0b787fa4181affe466f987f0337a/java/operator/org.eclipse.theia.cloud.operator/src/main/java/org/eclipse/theia/cloud/operator/LeaderElectionTheiaCloudOperatorLauncher.java#L108-L117). The type is only passed to Guice as a module:
>
> protected void startOperatorAsLeader(TheiaCloudOperatorArguments arguments) {
> AbstractTheiaCloudOperatorModule module = this.createModule(arguments);
> Injector injector = Guice.createInjector(module);
> TheiaCloudOperator theiaCloud = injector.getInstance(TheiaCloudOperator.class);
> theiaCloud.start();
> }
> Maybe I am doing something wrong. What is the correct way of customizing the default Theia Cloud client? Thank you!
>
> ### Expected behavior
> The framework provides a way to customize the operator.
>
> ### Cluster provider
> _No response_
>
> ### Version
> _No response_
>
> ### Additional information
> Related [#426 (comment)](https://github.com/eclipse-theia/theia-cloud/issues/426#issue-3060187598)
>
> I am new to this API, but I do not understand why getting the IP requires a new client creation when it is already set at the constructor of the default implementation:
>
> [theia-cloud/java/common/org.eclipse.theia.cloud.common/src/main/java/org/eclipse/theia/cloud/common/k8s/client/TheiaCloudClient.java](https://github.com/eclipse-theia/theia-cloud/blob/55813e2c212b0b787fa4181affe466f987f0337a/java/common/org.eclipse.theia.cloud.common/src/main/java/org/eclipse/theia/cloud/common/k8s/client/TheiaCloudClient.java#L81)
>
> Line 81 in [55813e2](/eclipse-theia/theia-cloud/commit/55813e2c212b0b787fa4181affe466f987f0337a)
>
> try (final KubernetesClient client = new KubernetesClientBuilder().build()) {
> [theia-cloud/java/common/org.eclipse.theia.cloud.common/src/main/java/org/eclipse/theia/cloud/common/k8s/client/DefaultTheiaCloudClient.java](https://github.com/eclipse-theia/theia-cloud/blob/55813e2c212b0b787fa4181affe466f987f0337a/java/common/org.eclipse.theia.cloud.common/src/main/java/org/eclipse/theia/cloud/common/k8s/client/DefaultTheiaCloudClient.java#L28-L31)
>
> Lines 28 to 31 in [55813e2](/eclipse-theia/theia-cloud/commit/55813e2c212b0b787fa4181affe466f987f0337a)
>
> public DefaultTheiaCloudClient(Client client) {
> super(client.adapt(KubernetesClientImpl.class));
> this.client = client.adapt(NamespacedKubernetesClient.class);
> }

### Expected behavior

The framework provides a way to customize the operator.

### Cluster provider

_No response_

### Version

_No response_

### Additional information

_No response_

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with AbstractTheiaCloudOperatorModule and LeaderElectionTheiaCloudOperatorLauncher, then inspect TheiaCloudClient and DefaultTheiaCloudClient for the existing provider and construction path. Determine how a custom TheiaCloudClient can be bound without Guice override conflicts or the module type restriction. Done means the framework documents or supports a working customization path for the operator.

Written by the indexing model from the issue text.

Assessment

Tech stack
java, kubernetes
Domain
backend, infrastructure
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.