List of structs containing references are patched without references

[RedbackThomson]

Describe the bug
After the controllers resolve resource references contained within a list of structs (such as EC2's RouteTable.Routes), they incorrectly patch the spec, adding the concrete field. I have narrowed this down to the patchResourceMetadataAndSpec method. When you pass r.kc.Patch a resource with a list of structs containing resolved references, it will overwrite all of the elements within any slices in the target resource. This is a behaviour from controller-runtime's patching mechanism - https://github.com/kubernetes-sigs/controller-runtime/blob/2f77235e25b1e42d9e4957199f3cd0f2c3fb0d72/pkg/client/patch.go#L143-L149. Using StrategicMergePatch would be the solution, as that can update structs within lists, however this is not supported for custom resources.

Steps to reproduce

1. Apply the following manifest:

apiVersion: ec2.services.k8s.aws/v1alpha1
kind: VPC
metadata:
  name: gitops-cluster-vpc
spec:
  cidrBlock: 192.168.0.0/16
  enableDNSSupport: true
  enableDNSHostnames: true
  tagSpecifications:
  - resourceType: vpc
    tags:
    - key: Name
      value: gitops-cluster
---
apiVersion: ec2.services.k8s.aws/v1alpha1
kind: InternetGateway
metadata:
  name: gitops-cluster-igw
spec:
  vpcRef:
    from:
      name: gitops-cluster-vpc
---
apiVersion: ec2.services.k8s.aws/v1alpha1
kind: RouteTable
metadata:
  name: gitops-cluster-private-route-table-az2
spec:
  vpcRef:
    from:
      name: gitops-cluster-vpc
  routes: []
  tagSpecifications:
  - resourceType: "route-table"
    tags:
    - key: Name
      value: Private Subnets AZ2
    - key: Network
      value: Private02

2. Update the RouteTable.Routes, adding the following reference:

- destinationCIDRBlock: 0.0.0.0/0
  gatewayRef:
    from:
      name: gitops-cluster-igw

3. Describe the RouteTable and see Spec.Routes[0].GatewayRef does not exist anymore, but Spec.Routes[0].GatewayID has the concrete value

Expected outcome
I would expect that the reference would remain within the spec, so that changes to the gateway are reflected within the route table - as expected in a GitOps compliant system.

[a-hilaly]

/lifecycle frozen

[jaypipes]

@RedbackThomson @brycahta @vijtrip2 seems to me that the root of the problem is that the runtime reconciler is modifying the Spec when resolving references (this is regardless of whether the reference field is in a list of structs or not). I think a safer approach would be to resolve the references but not save the resolved identifiers in the Spec, instead saving the resolved references in a separate struct that we attach to the AWSResource so that the resource managers can grab those resolved identifiers when needed.