Concurrent updates on annotations and labels in Kubernetes Dashboard cause unintended data loss

[fengyinqiao]

What happened?

Description:

We encountered an issue when multiple users make concurrent updates to the same resource in Kubernetes Dashboard, specifically when one user modifies the annotations and another modifies the labels without refreshing the dashboard or re-fetching the latest resource version.

Actual behavior:

After User B submits the changes, only the label (hello: world) is reflected in the final YAML. The annotation (foo: bar) added by User A is lost, leading to unintended data loss.

Root Cause:

This appears to be a classic case of a "last writer wins" issue. When User B submits their changes, the YAML they are editing does not contain the latest version of the resource, which leads to the annotation added by User A being overwritten.

Impact:

This issue can cause critical changes (such as annotations used for configurations, selectors, or application metadata) to be unintentionally lost, leading to potential operational problems.

What did you expect to happen?

Both users' changes should be merged, with the final YAML containing both the updated annotations and labels. In this case:

• Annotations: foo: bar
• Labels: hello: world

How can we reproduce it (as minimally and precisely as possible)?

1. User A modifies the resource's YAML in the dashboard, adding an annotation like foo: bar, and submits the change.
2. User B, without refreshing the dashboard, modifies the same resource's YAML, adding or changing a label (e.g., hello: world) based on the outdated YAML (which doesn't include A's changes).
3. User B submits the modification.

Anything else we need to know?

No response

What browsers are you seeing the problem on?

Chrome

Kubernetes Dashboard version

dashboard-api:1.7.0,dashboard-auth:1.1.3,dashboard-metrics-scraper:1.1.1,dashboard-web:1.4.0,kong:3.6

Kubernetes version

v1.22.15

Dev environment

No response

[floreks]

You are right. We are preparing data for the three-way merge patch incorrectly. We also need to fetch the original resource from the API that matches the resource version of the object coming in the request. This way we will have the original, latest, and changed resource so that the three-way merge patch will know if the data added in between should be removed or not.

[fengyinqiao]

You are right. We are preparing data for the three-way merge patch incorrectly. We also need to fetch the original resource from the API that matches the resource version of the object coming in the request. This way we will have the original, latest, and changed resource so that the three-way merge patch will know if the data added in between should be removed or not.

@floreks I think only the two-way mege patch is needed. Original: front-end page data, Edited: the user modified data, the two make a Diff, and then send the patches to apiserver.

kubectl edit does this: https: //github.com/kubernetes/kubernetes/blob/9FFC095F88C74C0F5B64A64583F0B7C83FC/k8s.io/kk Ubectl/PKG/CMD/Util/Editor/Editoptions.go#L683

[fengyinqiao]

You are right. We are preparing data for the three-way merge patch incorrectly. We also need to fetch the original resource from the API that matches the resource version of the object coming in the request. This way we will have the original, latest, and changed resource so that the three-way merge patch will know if the data added in between should be removed or not.

@floreks If in your way, assuming that the user A deletes the field foo and submits it, the user B adds the field hello on the basis of the original data (the front page is not refreshed). In the end, the three-way merge patch will get the following results: field-added:foo(actually, the field foo has been deleted), field-added:hello.

Results derived according to the following code:

// Create a 3-way merge patch based-on JSON merge patch.
// Calculate addition-and-change patch between current and modified.
// Calculate deletion patch between original and modified.
func CreateThreeWayJSONMergePatch(original, modified, current []byte, fns ...mergepatch.PreconditionFunc) ([]byte, error) {...}

original: foo+, hello-
modified: foo+, hello+
current: foo-, hello-

+: field included, -: field excluded

addition-and-change patch between current and modified: foo, hello
deletion patch between original and modified: null
result: foo, hello

[floreks]

original: foo+, hello-
modified: foo+, hello+
current: foo-, hello-

+: field included, -: field excluded

addition-and-change patch between current and modified: foo, hello
deletion patch between original and modified: null

I have also considered this scenario and I think it has 1 small issue.
Since original and modified already contain foo, the addition patch between original and modified will only contain hello.
Then the deletion patch will be empty and the result patch will be to apply hello on the latest resource version resulting in correct update.

EDIT: oh wait, scratch that. They are doing addition between current and modified. This can cause issues then.

[fengyinqiao]

They are doing addition between current and modified. This can cause issues then.

@floreks Yes, look at the following code: