match in GenericKubernetesResourceMatcher.java doesn't seem to calculate the DiffJsonPatch correctly.

[gnfpt]

Bug Report

I believe there's an issue with the way JsonDiff.asJson() is being used in GenericKubernetesResourceMatcher.java

What did you do?

I have a CR being updated with an EnvVar deleted from the list. That's the only change. The dependent resource isn't reconciled.

In GenericKubernetesResourceMatcher.java match() (L:163) it's being called JsonDiff.asJson(desiredNode, actualNode). The outcome is to add the missing element instead of remove.

What did you expect to see?

I expected to see the missing element in desired EnvVars to be removed from actual EnvVars.

What did you see instead? Under which circumstances?

The outcome is to add the element missing in desired EnvVars.

Environment

AWS EKS v1.32.7-eks-ace6451
JOSDK operator-framework-spring-boot-starter:6.1.1
amazoncorretto:17.0.5
openjdk 17.0.5 2022-10-18 LTS
OpenJDK Runtime Environment Corretto-17.0.5.8.1 (build 17.0.5+8-LTS)
OpenJDK 64-Bit Server VM Corretto-17.0.5.8.1 (build 17.0.5+8-LTS, mixed mode, sharing)

Client Version: v1.32.1
Kustomize Version: v5.5.0
Server Version: v1.33.1

Possible Solution

swap the parameters in the call to JsonDiff.asJson() but this is going to catch "metadata" and "status" missing in "desired". Might have some side effects.

Additional context

Example of using JsonDiff.asJson() to calculate the patch and applying it to "actual" in order to get "desired" result.

public static void main(String[] args) {
    int[] desired = {0, 2, 3};
    int[] actual  = {0, 1, 2, 3};

    var desiredNode = new ObjectMapper().convertValue(desired, JsonNode.class);
    var actualNode = new ObjectMapper().convertValue(actual, JsonNode.class);

    var patch = JsonDiff.asJson(actualNode, desiredNode);  
    var result = JsonPatch.apply(patch, actualNode);

    System.out.println("desired -> " + Arrays.toString(desired));
    System.out.println("actual -> " + Arrays.toString(actual));
    System.out.println("patch -> " + patch);
    System.out.println("result -> " + result);
}

[csviri]

Hi @gnfpt ,

there is essentially a limitation for such generic differs, basically you cannot tell if from the available information that if a change was removed by you or added by a thord party actor. So basically should be used when works for you in best effort bases.
You can do 2 things override the match method and try to handle this situation explicitly.
But would advice to use the SSA / SSA matcher, what handles such situations.

just sidenote: JsonDiff is part of fabric8 client not josdk.

[gnfpt]

Hi @csviri

The issue isn't in JsonDiff. I think the parameters are swapped when the method is called in JOSDK.

If we calculate the diffPatch and apply it to "actual", the way it's being used in JOSDK doesn't seem to produce the expected result.

The way josdk is calling

JsonDiff.asJson(desiredNode, actualNode))
desired -> [0, 2, 3]
actual -> [0, 1, 2, 3]
patch -> [{"op":"add","path":"/1","value":1}]
result -> [0,1,1,2,3]

And this is the result of applying it the way I'm suggesting it should be:

JsonDiff.asJson(actualNode, desiredNode)
desired -> [0, 2, 3]
actual -> [0, 1, 2, 3]
patch -> [{"op":"remove","path":"/1"}]
result -> [0,2,3]

basically you cannot tell if from the available information that if a change was removed by you or added by a thord party actor

I'm not sure I understood what you mean. My understanding is that "actual" represents the state in k8s and "desired" is how it should look at the end of reconciliation process.

[csviri]

@gnfpt I see could you pls create a PR for this?