Allow DI for condition and discriminator classes

[Javatar81]

There are several attributes that refer to a singleton object represented by its Class, e.g. conditions and discriminators. Examples are:

@KubernetesDependent(resourceDiscriminator = MyDiscriminator.class)

@Dependent(reconcilePrecondition = MyReconcilepreondition.class)

@Dependent(readyPostcondition = MyReadyPostcondition.class)

@Dependent(deletePostcondition = MyDeletePostcondition.class)

There is no way to inject additional dependencies into these classes since they are always instantiated via their default constructor. It would be useful to support attribute and constructor injection via CDI.

One use case is to inject the OpenShiftClient into a reconcile precondition class to implement checks agains the OCP API.

[AleksanderBrzozowski]

It would be also useful to support constructor based injection 🙂 Another use case is to use config properties, that are exposed as a bean.

[metacosm]

It would be also useful to support constructor based injection 🙂 Another use case is to use config properties, that are exposed as a bean.

Could you elaborate, please?

[AleksanderBrzozowski]

Yes, of course.

So, if we have a configuration like below:

Dependent(type = SomeResource::class)

Then, constructor based injection won't work (tested with quarkus-operator-sdk 6.3.0):

java.lang.RuntimeException: java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
    [error]: Build step io.quarkus.arc.deployment.ArcProcessor#generateResources threw an exception: jakarta.enterprise.inject.spi.DeploymentException: It's not possible to automatically add a synthetic no-args constructor to an unproxyable bean class. You need to manually add a non-private no-args constructor to com.mypackage.SomeResource in order to fulfill the requirements for normal scoped/intercepted/decorated beans.
    at io.quarkus.arc.processor.Beans.cannotAddSyntheticNoArgsConstructor(Beans.java:957)
    at io.quarkus.arc.processor.Beans.validateBean(Beans.java:762)
    at io.quarkus.arc.processor.BeanInfo.validate(BeanInfo.java:605)
    at io.quarkus.arc.processor.BeanDeployment.validateBeans(BeanDeployment.java:1547)
    at io.quarkus.arc.processor.BeanDeployment.validate(BeanDeployment.java:481)
    at io.quarkus.arc.processor.BeanProcessor.validate(BeanProcessor.java:164)
    at io.quarkus.arc.deployment.ArcProcessor.validate(ArcProcessor.java:471)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:568)
    at io.quarkus.deployment.ExtensionLoader$3.execute(ExtensionLoader.java:864)
    at io.quarkus.builder.BuildContext.run(BuildContext.java:282)
    at org.jboss.threads.ContextHandler$1.runWith(ContextHandler.java:18)
    at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2513)
    at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1538)
    at java.base/java.lang.Thread.run(Thread.java:833)
    at org.jboss.threads.JBossThread.run(JBossThread.java:501)

It seems to be failing even in build phase 🤔

And for the second part of the question - it is not possible to inject when using readyPostcondition. Even if there is an inject annotation, the property is still null.

So from my point of view, it would be nice if any property of @Dependent annotation that requires class would support injection (constructor based injection). Not sure if it is possible with the actual architecture of the Operator SDK.

Any thoughts?

[Javatar81]

JOSDK should allow conditions and discriminators to be CDI beans, i.e. if the referenced class, let's say MyDiscriminator.class is annotated with e.g. @ApplicationScoped the @Dependent registration process should pick up the singleton instance provided by CDI and must not instantiate it itself (though this could still be the case if the class is not an CDI bean). This would enable field and constructor injection.

[csviri]

Just a small remark, discriminators are probably going away in v5: https://github.com/operator-framework/java-operator-sdk/pull/2252 https://github.com/operator-framework/java-operator-sdk/issues/2253

[metacosm]

@Javatar81 would the approach described in https://github.com/operator-framework/java-operator-sdk/pull/2252 work in your use case? I'm personally still on the fence on whether or not we should remove discriminators altogether in future releases because I think they might make sense in some optimization scenarios. However, an alternative could be to override the getSecondaryResource method on your dependent implementation so I'd be interested in hearing more about your use case and how you use discriminators.

[Javatar81]

I am currently migrating all my discriminator to the ResourceIDMatcherDiscriminator approach. If that works, this will be a good sign that https://github.com/operator-framework/java-operator-sdk/pull/2252 would also work for me.

[Javatar81]

This makes me think that we could remove discriminators from the list for CDI managed beans. However, I think we still need it for the classes referenced in the following attributes of @Dependent:

• reconcilePrecondition
• readyPostcondition
• deletePostcondition
• type

[metacosm]

Note that the dependent resources (the type attribute in your list) are already exposed as CDI beans.

[metacosm]

One issue with making the conditions beans, is that we wouldn't be able to completely resolve the workflows at build time as is currently the case, if I'm not mistaken… This is something I need to think about more.

[metacosm]

Another issue is that all these classes have the same type so I'm not sure how resolution would work since we would probably need to add a qualifier to distinguish them, which starts to make things rather complicated. Maybe I'm not just seeing well how things would work in that scenario…

[metacosm]

@Javatar81 @AleksanderBrzozowski is this still something you are interested in? If yes, could you provide more details as to what you'd like to be able to do more specifically, in particular with respect to how CDI would handle the fact that conditions all share the same base type?

[AleksanderBrzozowski]

So, what we do is we have a class which extends from BulkDependentResource (and some other types as well). The class needs access to the application properties - which are currently injected.

For the condition to pass, the properties are not needed.