Repository navigation
CRUDKubernetesDependentResource#delete() Deletes Non-Children Resources #2349
Description
Bug Report
I have a KafkaUserDR which is a CRUDKubernetesDependentResource<KafkaUser, LHCluster>. In my LHClusterReconciler#reconcile method, I call kafkaUserDR.delete(...).
KafkaUser is the class of the CRD that is created by the Dependent Resource, and LHCluster is the primary.
The name of the Primary resource (the LHCluster) is my-cluster. I had another CRD of a different type (LHKafkaUser), and that an instance of that CRD also named my-cluster.
The LHKafkaUser CRD creates as a dependent a KafkaUser and therefore put an ownerRef on the created KafkaUser. The ownerRef on that KafkaUser correctly reported the kind: LHKafkaUser.
However, in the LHCluster Reconciler, calling delete() on the kafkaUserDR incorrectly deleted that resource: it was owned by the LHKafkaUser and NOT by the LHCluster.
Potential Fix
I think the fix is that the CRUDKubernetesDependentResource#delete() method needs to check both the name and kind on the ownerRef of resources before deleting it. Right now, the behavior suggests that it is not checking the kind.
Environment
Using a KIND cluster. JOSDK version 4.4.x
Activity
coltmcnealy-lh commentedon Apr 15, 2024
One workaround for now is to just make sure to avoid naming clashes. But that's not exactly ideal, and when we ship our Operator as a product to our customers (which we are doing soon), we can't guarantee that they will follow such practices.
csviri commentedon Apr 15, 2024
Hi, the issue is here, when mapping from owner references:
java-operator-sdk/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java
Line 102 in a08d725
To solve this cross namespace, we need to add
apiVersion+kindinto the referencing with annotation (additional annotation probably). (AFAIK this is not handled in controller-runtime either)Will schedule this just for v5 (if no objections), since it is already handling the primary type sort of, which could be enhanced and the reference type easily checked.
coltmcnealy-lh commentedon Apr 15, 2024
Thanks for the update! Is there a ticket I can use to track the V5 progress?
Also, for now it looks like I can solve the issue by implementing
SecondaryToPrimaryMapperin my Dependent Resources. Is that correct?csviri commentedon Apr 15, 2024
There is a milestone:
https://github.com/operator-framework/java-operator-sdk/milestone/5
yes, definitely, you can do that, that will fix it.