Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SecondaryToPrimaryMapper does not take into account primary resource kind and api version #2475

Open
vladimirionescu opened this issue Jul 30, 2024 · 1 comment

Comments

@vladimirionescu
Copy link

vladimirionescu commented Jul 30, 2024

Bug Report

What did you do?

I am writing an operator managing a primary resource which has dependent resources. I am extending the CRUDKubernetesDependentResource and using the default SecondaryToPrimary mapper based on owner references.

In the same namespace, there may be other resources with the same name as my primary (but with different types!), and they may have secondary resources of the same type as the secondary resource my operator is managing (a ConfigMap for example).

What did you expect to see?

I expected the SecondaryToPrimaryMapper to take into account primary resource kind and attribute secondary resources correctly.

What did you see instead? Under which circumstances?

All secondary resources with an owner reference matching the name of the primary (but not necessarily the kind) get mapped to the primary, leading to an IllegalStateException being thrown because there is more than 1 secondary resource found.

Environment

Kubernetes cluster type:

GKE

$ Mention java-operator-sdk version from pom.xml file

4.9.1

$ java -version

openjdk version "21.0.3" 2024-04-16 LTS
OpenJDK Runtime Environment Corretto-21.0.3.9.1 (build 21.0.3+9-LTS)
OpenJDK 64-Bit Server VM Corretto-21.0.3.9.1 (build 21.0.3+9-LTS, mixed mode, sharing)

$ kubectl version

Client Version: v1.30.3
Kustomize Version: v5.0.4-0.20230601165947-6ce0bf390ce3
Server Version: v1.29.6-gke.1038001

Possible Solution

As a workaround, I provided my own implementation of SecondaryToPrimaryMapper using owner references:

fun <T : HasMetadata> fromOwnerReferences(
    kind: String, apiVersion: String
): SecondaryToPrimaryMapper<T> {
    return SecondaryToPrimaryMapper { resource: T ->
        resource.metadata.ownerReferences.stream().filter { it.kind == kind && it.apiVersion == apiVersion }.map { or: OwnerReference? ->
            ResourceID.fromOwnerReference(
                resource, or, false
            )
        }.collect(Collectors.toSet())
    }
}

Additional context

@csviri
Copy link
Collaborator

csviri commented Jul 30, 2024

Hi @vladimirionescu , thx for reporting, note that this is already fixed for upcoming v5.

public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromOwnerReferences(
HasMetadata primaryResource,
boolean clusterScoped) {
return fromOwnerReferences(primaryResource.getApiVersion(), primaryResource.getKind(),
clusterScoped);
}
public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromOwnerReferences(
String apiVersion, String kind,
boolean clusterScope) {
return resource -> resource.getMetadata().getOwnerReferences()
.stream()
.filter(r -> r.getKind().equals(kind)
&& r.getApiVersion().equals(apiVersion))
.map(or -> ResourceID.fromOwnerReference(resource, or, clusterScope))
.collect(Collectors.toSet());
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants