Support merging with an array item in composition patches #3335
Description
What problem are you facing?
We have recently encountered an issue with a Crossplane composition that patches the selector field of an array item. Crossplane does a merge patch, which overrides the whole array element together with the target field and the reference field. This causes the already resolved target fields to get lost, leaving only the selector field.
In such cases, there is a race condition between the Crossplane's composite reconciler (who composes and patches the composed resource) and the associated Crossplane provider's managed resource reconciler.
Here's how the situation can be observed:
---
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.9.2
creationTimestamp: null
name: myresources.xp.test.com
spec:
group: xp.test.com
names:
kind: MyResource
listKind: MyResourceList
plural: myresources
singular: myresource
scope: Cluster
versions:
- name: v1alpha1
schema:
openAPIV3Schema:
description: MyResource is the Schema for the myresources API
properties:
apiVersion:
description: 'APIVersion defines the versioned schema of this representation
of an object. Servers should convert recognized schemas to the latest
internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
type: string
kind:
description: 'Kind is a string value representing the REST resource this
object represents. Servers may infer this from the endpoint the client
submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
type: string
metadata:
type: object
spec:
description: MyResourceSpec defines the desired state of MyResource
properties:
forProvider:
description: A is an example field of MyResource. Edit myresource_types.go
to remove/update
properties:
b:
properties:
targetRefs:
items:
description: A Reference to a named object.
properties:
name:
description: Name of the referenced object.
type: string
policy:
description: Policies for referencing.
properties:
resolution:
default: Required
description: Resolution specifies whether resolution
of this reference is required. The default is
'Required', which means the reconcile will fail
if the reference cannot be resolved. 'Optional'
means this reference will be a no-op if it cannot
be resolved.
enum:
- Required
- Optional
type: string
resolve:
description: Resolve specifies when this reference
should be resolved. The default is 'IfNotPresent',
which will attempt to resolve the reference only
when the corresponding field is not present. Use
'Always' to resolve the reference on every reconcile.
enum:
- Always
- IfNotPresent
type: string
type: object
required:
- name
type: object
type: array
targetSelector:
description: A Selector selects an object.
properties:
matchControllerRef:
description: MatchControllerRef ensures an object with
the same controller reference as the selecting object
is selected.
type: boolean
matchLabels:
additionalProperties:
type: string
description: MatchLabels ensures an object with matching
labels is selected.
type: object
policy:
description: Policies for selection.
properties:
resolution:
default: Required
description: Resolution specifies whether resolution
of this reference is required. The default is 'Required',
which means the reconcile will fail if the reference
cannot be resolved. 'Optional' means this reference
will be a no-op if it cannot be resolved.
enum:
- Required
- Optional
type: string
resolve:
description: Resolve specifies when this reference
should be resolved. The default is 'IfNotPresent',
which will attempt to resolve the reference only
when the corresponding field is not present. Use
'Always' to resolve the reference on every reconcile.
enum:
- Always
- IfNotPresent
type: string
type: object
type: object
targets:
items:
type: string
type: array
type: object
bArr:
items:
properties:
targetRefs:
items:
description: A Reference to a named object.
properties:
name:
description: Name of the referenced object.
type: string
policy:
description: Policies for referencing.
properties:
resolution:
default: Required
description: Resolution specifies whether resolution
of this reference is required. The default is
'Required', which means the reconcile will fail
if the reference cannot be resolved. 'Optional'
means this reference will be a no-op if it cannot
be resolved.
enum:
- Required
- Optional
type: string
resolve:
description: Resolve specifies when this reference
should be resolved. The default is 'IfNotPresent',
which will attempt to resolve the reference
only when the corresponding field is not present.
Use 'Always' to resolve the reference on every
reconcile.
enum:
- Always
- IfNotPresent
type: string
type: object
required:
- name
type: object
type: array
targetSelector:
description: A Selector selects an object.
properties:
matchControllerRef:
description: MatchControllerRef ensures an object with
the same controller reference as the selecting object
is selected.
type: boolean
matchLabels:
additionalProperties:
type: string
description: MatchLabels ensures an object with matching
labels is selected.
type: object
policy:
description: Policies for selection.
properties:
resolution:
default: Required
description: Resolution specifies whether resolution
of this reference is required. The default is
'Required', which means the reconcile will fail
if the reference cannot be resolved. 'Optional'
means this reference will be a no-op if it cannot
be resolved.
enum:
- Required
- Optional
type: string
resolve:
description: Resolve specifies when this reference
should be resolved. The default is 'IfNotPresent',
which will attempt to resolve the reference only
when the corresponding field is not present. Use
'Always' to resolve the reference on every reconcile.
enum:
- Always
- IfNotPresent
type: string
type: object
type: object
targets:
items:
type: string
type: array
type: object
type: array
type: object
type: object
status:
description: MyResourceStatus defines the observed state of MyResource
type: object
type: object
served: true
storage: true
subresources:
status: {}
---
apiVersion: apiextensions.crossplane.io/v1
kind: CompositeResourceDefinition
metadata:
name: xmyresources.test.com
spec:
group: test.com
names:
kind: XMyResource
plural: xmyresources
versions:
- name: v1alpha1
served: true
referenceable: true
schema:
openAPIV3Schema:
type: object
properties:
spec:
type: object
properties:
name:
type: string
required:
- name
---
apiVersion: apiextensions.crossplane.io/v1
kind: Composition
metadata:
name: example
labels:
purpose: example
spec:
compositeTypeRef:
apiVersion: test.com/v1alpha1
kind: XMyResource
resources:
- name: myresource
base:
apiVersion: xp.test.com/v1alpha1
kind: MyResource
metadata:
spec:
forProvider: {}
patches:
- fromFieldPath: "spec.name"
toFieldPath: spec.forProvider.bArr[0].targetSelector.matchLabels[key2]
---
apiVersion: test.com/v1alpha1
kind: XMyResource
metadata:
name: test
spec:
name: testName
compositionRef:
name: exampleIf, after applying these manifests, the following patch is applied on the composed resource (simulating the provider's action):
spec:
forProvider:
b:
targets:
- testTarget1.a
- testTarget1.b
targetRefs:
- name: targetName1.a
- name: targetName1.b
targetSelector:
matchLabels:
key1: val1
bArr:
- targets:
- testTarget2.a
- testTarget2.b
targetRefs:
- name: targetName2.a
- name: targetName2.b
targetSelector:
matchLabels:
key2: val2, one can observe that the spec.forProvider.bArr[0].targets and spec.forProvider.bArr[0].targetRefs fields are emptied.
How could Crossplane help solve your problem?
We can introduce a new patch policy that will merge, instead of replace, array items. The current behavior of JSON merge patch works as expected but the resulting behavior is not intuitive and can easily cause issues.