CRUDKubernetesDependentResource#delete() Deletes Non-Children Resources

[coltmcnealy-lh]

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

[coltmcnealy-lh]

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]

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

return Mappers.fromOwnerReferences(clustered);

To solve this cross namespace, we need to add apiVersion+kind into 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]

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 SecondaryToPrimaryMapper in my Dependent Resources. Is that correct?

[csviri]

Is there a ticket I can use to track the V5 progress?

There is a milestone:
https://github.com/operator-framework/java-operator-sdk/milestone/5

Also, for now it looks like I can solve the issue by implementing SecondaryToPrimaryMapper in my Dependent Resources. Is that correct?

yes, definitely, you can do that, that will fix it.