Should remarshal LastAppliedConfigAnnotation before ThreeWayDiff

[ciiiii]

Description

Config and live object are all handled with remarshal(), should object from LastAppliedConfigAnnotation need to be handled with remarshal() too? This can avoid null values in LastAppliedConfigAnnotation make diff result different.

Related Code

func Diff(config, live *unstructured.Unstructured, opts ...Option) (*DiffResult, error) {
	o := applyOptions(opts)
	if config != nil {
		config = remarshal(config, o)
		Normalize(config, opts...)
	}
	if live != nil {
		live = remarshal(live, o)
		Normalize(live, opts...)
	}
	orig, err := GetLastAppliedConfigAnnotation(live)
	if err != nil {
		o.log.V(1).Info(fmt.Sprintf("Failed to get last applied configuration: %v", err))
	} else {
		if orig != nil && config != nil {
                        orig = remarshal(orig, o) // add core here
			Normalize(orig, opts...)
			dr, err := ThreeWayDiff(orig, config, live)
			if err == nil {
				return dr, nil
			}
			o.log.V(1).Info(fmt.Sprintf("three-way diff calculation failed: %v. Falling back to two-way diff", err))
		}
	}
	return TwoWayDiff(config, live)
}

Example

live object:

apiVersion: v1
kind: Service
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"v1","kind":"Service","metadata":{"annotations":{},"labels":{"argocd.argoproj.io/instance":"test"},"name":"test","namespace":"default"},"spec":{"ports":[{"name":"http","nodePort":null,"port":8080,"targetPort":8080}],"selector":{"app":"test"},"type":"NodePort"}}
  creationTimestamp: "2022-01-17T10:34:45Z"
  labels:
    argocd.argoproj.io/instance: test
  name: test
  namespace: default
  resourceVersion: "199634708"
  selfLink: /api/v1/namespaces/default/services/test
  uid: 384f0d3f-5146-4654-a11b-e7d7065cab88
spec:
  clusterIP: 10.45.102.60
  externalTrafficPolicy: Cluster
  ports:
  - name: http
    nodePort: 30395
    port: 8080
    protocol: TCP
    targetPort: 8080
  selector:
    app: test
  sessionAffinity: None
  type: NodePort
status:
  loadBalancer: {}

config object:

apiVersion: v1
kind: Service
metadata:
  labels:
    argocd.argoproj.io/instance: test
  name: test
  namespace: default
spec:
  ports:
  - name: http
    nodePort: null
    port: 8080
    protocol: TCP
    targetPort: 8080
  selector:
    app: test
  type: NodePort

When someone set nodePort of Service to null, k8s will allocate a port to the Service, so the live object will have nodePort: 30395 and config object will have nodePort: null, remarshal() will clear nodePort: null, so there will be no diff between live object and config object, but object from LastAppliedConfigAnnotation without remarshal() will broke the result, mark application OutOfSync, Maybe we can prevent this meaningless sync.