crossplane-contrib / crossplane-contrib/function-patch-and-transform

Patching from a non-existent 'from' field deletes the 'to' field

Open
#85 1 comment 0 reactions 0 assignees View on GitHub
known-issue
Dominant language
Go
Stars
46
Forks
41
Avg merge
1d 10h
Merged PRs (30d)
3

Description

### Background

I plan to include https://github.com/crossplane-contrib/function-patch-and-transform/pull/81 in the v0.3.0 release of this function.

#81 changes how this function handles patch errors. This includes how the function handles patching from a field path that doesn't exist, which is a kind of patch error.

Most patch errors now cause the function to return a fatal result. A fatal result stops the entire function pipeline. The fatal result will appear as an event associated with the XR. For example a type mismatch like patching from an integer field to an array field would cause a fatal error.

We treat errors that occur due to missing 'from' field paths specially. Each patch can configure whether the 'from' field path is optional (the default) or required. This is because more than any other kind of error, a missing field path is likely to fix itself eventually.

Consider for example a composition that patches from MR A's status to the XR's spec, then patches that XR spec field to MR B's spec. MR B's spec is derived from MR A's status. It's normal for the patches to fail due to a missing field path until the status field is populated from the external system.

Sometimes it's fine for MR B to be created, then have its spec field derived from MR A's status once that status becomes available. You use an optional field path patch for this.

Other times MR B should not be created at all until the data from MR A's status is available. You use a required field path patch for this.

After #81 this function behaves per this table:

|Patch Policy|Patch From|Patch To|Result|
|-|-|-|-|
|Required|XR|New composed resource|Emit an event. Don't create the composed resource.|
|Required|XR|Existing composed resource|Emit an event. Skip the patch.|
|Required|Env|New composed resource|Emit an event. Don't create the composed resource.|
|Required|Env|Existing composed resource|Emit an event. Skip the patch.|
|Required|*|XR|Emit an event. Skip the patch.|
|Required|*|Env|Emit an event. Skip the patch.|
|Optional|XR|New composed resource|Skip the patch. Create the composed resource.|
|Optional|XR|Existing composed resource|Skip the patch.|
|Optional|Env|New composed resource|Skip the patch. Create the composed resource.|
|Optional|Env|Existing composed resource|Skip the patch.|
|Optional|*|XR|Skip the patch.|
|Optional|*|Env|Skip the patch.|

Another way to look at this is:

* The only purpose of a required 'from' field path is to block creation of the new 'to' resource.
* If the 'to' resource already exists, we treat a required 'from' field path like optional, but emit a warning.

### The Issue

This function computes desired state similarly to native P&T, but applies it differently. Functions use server-side apply (SSA), while native P&T uses client-side apply. One of the benefits of server-side apply is that it supports deleting fields. Our client-side apply implementation does not. This leads to issues like https://github.com/crossplane/crossplane/issues/4162.

This means that when native P&T skips a patch, the field it's patching to just remains how it was.

Because SSA interprets omitting a field from desired state as a desired to delete that field, when this functions skips a patch the field will be deleted.

This means that:

* If you're patching to a resource that exists (e.g. the XR or an existing composed resource), and...
* The patch has previously succeeded in patching from the 'from' field path to the 'to' field path, and...
* The patch begins to fail because the 'to' field path no longer exists

This function will omit the 'to' field path from its desired state, causing it to be deleted.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.