Implicit empty "target" in patch breaks Kustomize strategic merge logic

[artem-nefedov]

Describe the bug

When using "patches" directive in Kustomization manifest, if "target" is not specified for a specific patch, it's added implicitly as target: {}. The problem is that it breaks strategic merge logic, because Kustomize allows for strategic merge patches to have no target, but any existing target (even an empty one) changes behavior.

Suppose you have the following "ns.yaml" manifest:

---
apiVersion: v1
kind: Namespace
metadata:
  name: foo
---
apiVersion: v1
kind: Namespace
metadata:
  name: bar

And the following "kustomization.yaml":

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

resources:
- ns.yaml

patches:
- patch: |
    $patch: delete
    apiVersion: v1
    kind: Namespace
    metadata:
      name: bar

This patch will delete namespace only "bar". But if we add empty target: {} to this patch, it will delete everything! This is reproduced with local kustomize, and the same thing happens with Kustomization applied by controller.

Steps to reproduce

Make Kustomization manifest with "patches" section, and specify a strategic merge patch with no "target" section.

Expected behavior

Patch is passed as-is to kustomize, with no target added.

Screenshots and recordings

No response

OS / Distro

N/A

Flux version

0.18.3

Flux check

✔ kubectl 1.21.3 >=1.18.0-0 ✔ Kubernetes 1.21.2-eks-0389ca3 >=1.16.0-0 ► private-repo/fluxcd/helm-controller:v0.12.0 ► private-repo/fluxcd/kustomize-controller:v0.15.5 ► private-repo/fluxcd/notification-controller:v0.17.1 ► private-repo/fluxcd/source-controller:v0.16.0

Git provider

No response

Container Registry provider

No response

Additional context

No response

Code of Conduct

• [X] I agree to follow this project's Code of Conduct

[somtochiama]

Hey @artem-nefedov, I have been unable to reproduce this bug on my end. Using the example given above creates the foo namespace(here is the directory in my git repo https://github.com/SomtochiAma/fleet-infra/blob/main/clusters/target-patch/kustomization.yaml).

Is there something I might be missing? What does your flux kustomization look like?

[artem-nefedov]

@SomtochiAma Sorry, maybe I was not clear. I used the local kustomize just to show how patch will work.

To reproduce the issue with flux, you need:

1. Contents of target directory:

• kustomization.yaml:

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

resources:
- ns.yaml

• ns.yaml:

---
apiVersion: v1
kind: Namespace
metadata:
  name: foo
---
apiVersion: v1
kind: Namespace
metadata:
  name: bar

2. Kustomization manifest:

apiVersion: kustomize.toolkit.fluxcd.io/v1beta2
kind: Kustomization
metadata:
  name: test
spec:
  path: ./your-path-to-target-dir
  interval: 1m0s
  prune: true
  sourceRef:
    kind: Bucket
    name: my-bucket
    namespace: flux-system
  patches:
  - patch: |
      $patch: delete
      apiVersion: v1
      kind: Namespace
      metadata:
        name: bar

After applying this kustomization, you can check it in-cluster and see that it has target: {} implicitly added, and that both namespaces are deleted (not applied) as a result.

[artem-nefedov]

Any progress on this issue? Based on last status, I believe it was successfully reproduced?

[hiddeco]

I have no insights into where this got stuck, but have you tried setting an explicit nil?

[artem-nefedov]

Just tested it: explicit null still gets replaced with {}, and the issue still occurs. nil is a type error because it's just a string.

[somtochiama]

Hey @artem-nefedov, I was able to replicate this. The issue now is that not setting an empty target breaks some tests. So we are trying to figure out when setting an empty target works and when it doesn't.

[stefanprodan]

We require a target to be specified, docs here: https://fluxcd.io/docs/components/kustomize/kustomization/#patches If target is not found we should error out and stop reconciling.

[artem-nefedov]

Have you considered changing this requirement? It leads to unnecessary duplication - same thing has to be specified in target and in patch. Sometimes it is useful, but more often it's not. Furthermore, documentation at https://fluxcd.io/docs/components/kustomize/kustomization/#patches claims that patches work like kustomize patches, which do in fact support strategic merge without "target".

[jamiezieziula]

I have encountered this as well - I think the request here is for better error messaging. For example - I was missing a target specified for a patch on a deployment, but Flux threw the following error:

ServiceAccount/cloud2/auth dry-run failed, error: failed to create typed patch object (cloud2/auth; /v1, Kind=ServiceAccount): .spec: field not declared in schema...

which is not helpful for debugging