eclipse-theia / eclipse-theia/theia-cloud
[operator] Cannot customize TheiaCloudClient due to overriding restrictions in Guice
- 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