Patch does not work when targetObjectRef points to Kubernetes Custom Resource

[morningspace]

I'm using resource-locker-operator w/ Argo CD to patch resources in GitOps. However, when targetObjectRef points to Kubernetes Custom Resource, the patch doesn't work. Here is an example:

apiVersion: redhatcop.redhat.io/v1alpha1
kind: ResourceLocker
metadata:
  name: test-resource-locker
  namespace: resource-locker-operator
spec:
  patches:
  - id: test-patch
    patchTemplate: |
      metadata:
        annotations:
          foo: bar
    patchType: application/strategic-merge-patch+json
    targetObjectRef:
      apiVersion: somedomain.com/v1alpha1
      kind: MyType
      name: my-instance
      namespace: default
  serviceAccountRef:
    name: resource-locker-operator-controller-manager

This is supposed to add the annotation to MyType/my-instance, but I don't see it happens. And there is an error in logs:

2021-12-18T12:20:15.997Z    ERROR   patch-reconciler.resource-locker-operator/test-resource-locker.test-patch   unable to apply     {"patch": {}, "on target": ... }, "error": "the body of the request was in an unknown format - accepted media types include: application/json-patch+json, application/merge-patch+json, application/apply-patch+yaml"}
github.com/go-logr/zapr.(*zapLogger).Error
    /go/pkg/mod/github.com/go-logr/zapr@v0.2.0/zapr.go:132
github.com/redhat-cop/operator-utils/pkg/util/lockedresourcecontroller.(*LockedPatchReconciler).Reconcile
    /go/pkg/mod/github.com/redhat-cop/operator-utils@v1.1.4/pkg/util/lockedresourcecontroller/patch-reconciler.go:246

[morningspace]

Did quick look at the code, and issue happens at this line. It looks the patch variable should have been non-empty, but that's not true, and this points to the line above that, which means b bytes.Buffer is empty.

I know that I didn't specify sourceObjectRefs, so sourceMaps is empty, but this should not be a problem, since when I try that w/ a builtin Kubernetes resource, such as ServiceAccount, it works. For example, below snippet works.

apiVersion: redhatcop.redhat.io/v1alpha1
kind: ResourceLocker
metadata:
  name: test-resource-locker
  namespace: resource-locker-operator
spec:
  patches:
  - id: test-patch
    patchTemplate: |
      metadata:
        annotations:
          foo: bar
    patchType: application/strategic-merge-patch+json
    targetObjectRef:
      apiVersion: v1
      kind: ServiceAccount
      name: my-instance
      namespace: default
  serviceAccountRef:
    name: resource-locker-operator-controller-manager

@raffaelespazzoli

[raffaelespazzoli]

I took a look at the code. The fact that patch prints empty does not mean that the patch is empty. client.Patch is an type that does not print itself. So that log statement is wrong and misleading. Now, I don't know why you get that error. It sounds like the patch is badly formatted. Anyway we can see what is being sent on the wire? Do you possibly have non-printable characters in that pacthTemplate field?

btw, I'm rewriting this operator here: https://github.com/raffaelespazzoli/patch-operator I want to focus just on patches.

[morningspace]

@raffaelespazzoli Thanks for your prompt reply. After search around, it looks this is related to https://github.com/kubernetes/kubernetes/issues/97423#issuecomment-749312425. The custom resources cannot use strategic merge. After I change it to merge, it works.

So, patchType needs to be application/merge-patch+json instead of application/strategic-merge-patch+json. Maybe this can be noted in README :-)

btw, I'm rewriting this operator here: https://github.com/raffaelespazzoli/patch-operator

Great to know this, have starred it!

[morningspace]

I'm going to close this issue, since it's resolved.

[raffaelespazzoli]

@morningspace I'm glad you found a workaround, but i'm not sure that your explanation of the issue is correct. The code does a server-side patch and that should always work even with custom resources.

[morningspace]

@raffaelespazzoli I guess this answers the question:

// Because custom resources are decoded as Unstructured and because we're missing the metadata about how to handle
// each field in a strategic merge patch, we can't find the go struct tags. Hence, we can't easily  do a strategic merge
// for custom resources. So we should fail fast and return an error.

Same thing seems to happen inside lpr.GetClient().Patch, which is the dynamic client running in cluster.