Late Init: include zero values behind pointers

[jwefers]

TL;DR

During LateInit, Upjets WithZeroValueJSONOmitEmptyFilter filters pointers that point to a zero value,. It does NOT, hoever, filter structs only including fields that are pointers to zero-values. This is inconsistent. This PR fixes it by not filtering pointers to zero-values anymore, but only if the pointer itself is nil.

Origin story

I am working on upjetting the auth0/auth0 provider. The Auth0 API is very picky on PATCH requests, which surfaced an (imho) issue in how upjet deals in LateInit when deciding which values to include.

So in auth0_tenant.flags, there are a lot of boolean flags, most of which are set to false by default. Except enable_sso, that is always true nowadays and cannot be changed.
It must also not ever be sent in a HTTP PATCH, even if the value is unchanged. This throws a 400.

With a default upjet resource config, after the first Observe, exactly this flag set to true ends up in spec (and none other):

...
forProvider:
  flags: # from late init
    - enableSso: true # from late init

An Update will now include this in the terraform file, it ends up in the Http Patch and i get an eternal 400. But suspiciously, all other flags are missing. Why is especially this problematic one included, out of all of them?

So i add the flag to LateInit.IgnoreFields. Now i end up with:

...
forProvider:
  flags: # from late init
    - {} # from late init

This triggers another annoying issue with AUth0: When sending an empty flags object, i get another 400 that i must at least include one flag.

At this point the question became: Why is only the one flag included in late init - and why, when ignored, i still get an empty flags object?

The issue

After some debugging, the issue lies in pkg/resource/lateinit.go -> zeroValueJSONOmitEmptyFilter, which is generated into all resources.
Here, we look at two cases:

1. When a field of type struct is tested, L133 case v.IsZero(): asks the go reflect package, if this is a zero value. Go reflect will test all fields inside the struct if they are zero. A non-nil pointer field is considered not zero
2. When a field of type ptr is tested, L137 case k == reflect.Ptr && v.Elem().IsZero(): upjet overrides Go's behavior, and treats both the nilp ptr and also a derefenced zero value as zero.

In my case, flags: [{}] ends up in spec, because a struct with only ptr fields, where at least one ptr is not nil, is not considered a zero value. With current upjet behavior, the fields inside are considered zero though.

This behavior should be changed by treating a *T pointing to the zero value of T as non-zero. Intuitively, boolean flags set to false are valid config options and part of the "source of truth", crossplane wants to hold. The same imho holds true for other primitive values such as "", 0 etc. If the upstream API returns these values, they are a setting that is set. Imho this would also be more in line with https://docs.crossplane.io/latest/concepts/managed-resources/#late-initialization.

Description of your changes

It removes the case that tests for a ptr. A ptr would only be filtered if it is nil, due to case v.IsZero(). If not, it goes to the default case, which is "not filtered".

I changed the corresponding test and added a second one for the new behavior.

Fixes #

I have:

• Read and followed Upjet's contribution process.
• Run make reviewable to ensure this PR is ready for review.
• Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

I changed https://github.com/jwefers/provider-auth0 to use my fork of upjet with this PR applied.
It behaves as desired: all values, including false booleans, empty strings, 0 ints, that are explicitly configured in the tenant, are now late-init'd into spec.