network policy rules are incorrectly updated with kustomization labels

[sdickhoven]

What happened?

kustomize incorrectly updates network policy rules (rather than just the podSelector) when using labels with includeSelectors: true or commonLabels in the kustomization.

What did you expect to happen?

i expect the network policy rules to remain unaltered.

How can we reproduce it (as minimally and precisely as possible)?

kustomization.yaml:

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
namespace: foo
labels:
- includeSelectors: true
  pairs:
    key1: val1
    key2: val2
- includeTemplates: true
  pairs:
    key3: val3
resources:
- netpol.yaml

netpol.yaml:

apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
  name: my-app
spec:
  podSelector:
    matchLabels: {}
  ingress:
  - from:
    - namespaceSelector:
        matchLabels:
          kubernetes.io/metadata.name: ingress-nginx
      podSelector:
        matchLabels:
          app.kubernetes.io/name: ingress-nginx
          app.kubernetes.io/component: controller
    - namespaceSelector:
        matchLabels:
          kubernetes.io/metadata.name: kube-system
      podSelector:
        matchLabels:
          app.kubernetes.io/name: prometheus
    ports:
    - protocol: TCP
      port: 8080

Expected output

apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
  labels:
    key1: val1
    key2: val2
    key3: val3
  name: my-app
  namespace: foo
spec:
  ingress:
  - from:
    - namespaceSelector:
        matchLabels:
          kubernetes.io/metadata.name: ingress-nginx
      podSelector:
        matchLabels:
          app.kubernetes.io/component: controller
          app.kubernetes.io/name: ingress-nginx
    - namespaceSelector:
        matchLabels:
          kubernetes.io/metadata.name: kube-system
      podSelector:
        matchLabels:
          app.kubernetes.io/name: prometheus
    ports:
    - port: 8080
      protocol: TCP
  podSelector:
    matchLabels:
      key1: val1
      key2: val2

Actual output

apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
  labels:
    key1: val1
    key2: val2
    key3: val3
  name: my-app
  namespace: foo
spec:
  ingress:
  - from:
    - namespaceSelector:
        matchLabels:
          kubernetes.io/metadata.name: ingress-nginx
      podSelector:
        matchLabels:
          app.kubernetes.io/component: controller
          app.kubernetes.io/name: ingress-nginx
          key1: val1    # <<<<<<<< incorrect!!!
          key2: val2    # <<<<<<<< incorrect!!!
    - namespaceSelector:
        matchLabels:
          kubernetes.io/metadata.name: kube-system
      podSelector:
        matchLabels:
          app.kubernetes.io/name: prometheus
          key1: val1    # <<<<<<<< incorrect!!!
          key2: val2    # <<<<<<<< incorrect!!!
    ports:
    - port: 8080
      protocol: TCP
  podSelector:
    matchLabels:
      key1: val1
      key2: val2

Kustomize version

v5.6.0

Operating system

None

[k8s-ci-robot]

This issue is currently awaiting triage.

SIG CLI takes a lead on issue triage for this repo, but any Kubernetes member can accept issues by applying the triage/accepted label.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[sda399]

the default list includes those paths to podselectors. I guess you can overcome that with some reorganization of your netpol files.

edit:
labelTransformer might be a solution for you:

kustomize/plugin/builtin/labeltransformer/LabelTransformer_test.go

Lines 19 to 37 in 616c084

	kind: LabelTransformer
	metadata:
	name: notImportantHere
	labels:
	app: myApp
	quotedBoolean: "true"
	quotedFruit: "peach"
	unquotedBoolean: true
	env: production
	fieldSpecs:
	- path: spec/selector
	create: true
	version: v1
	kind: Service
	- path: metadata/labels
	create: true
	- path: spec/selector/matchLabels
	create: true
	kind: Deployment

[sdickhoven]

sure... i'm aware that this is an option.

but if i have a kustomization with 50 different resources (Deployments, ServiceAccounts, NetworkPolicys, PodDisruptionBudgets, Ingresses, Services, ServiceMonitors, ...) it becomes a bit tedious to spec out all label fields that i want modified.

plus, every time i add a new resource or remove a resource i have to remember to update the label transformer... not a great ux.

kustomize modifying a NetworkPolicy's ingress and egress rules is simply nonsensical since those typically have nothing to do with the resources being kustomize'd at all... except in rare edge cases when i need a network policy that allows traffic from all pods of a StatefulSet to themselves for some kind of cluster sync traffic.

it would make much more sense if i had to do something out of the ordinary for such a rare setup vs for something that is the norm: allow traffic from/to different workloads to/from the workload i'm kustomize'ing.

...just my opinion. 🤷

but i realize that there's probably no way to do this in a way that won't break existing kustomizations that rely on this odd behavior. 😞

so feel free to close this issue. i just wanted to point out that this behavior is a bit... suboptimal. 🤷

[sdickhoven]

but i realize that there's probably no way to do this in a way that won't break existing kustomizations that rely on this odd behavior. 😞

actually, come to think of it... i guess the following could be done to make this change backward compatible:

just like includeSelectors and includeTemplates, a new behavior modifier could be introduced. e.g.

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
namespace: foo
labels:
- includeSelectors: true
  includeNetworkPolicyIngressRules: false # default to `true`
  includeNetworkPolicyEgressRules: false # default to `true
  pairs:
    key1: val1
    key2: val2
- includeTemplates: true
  pairs:
    key3: val3
resources:
- netpol.yaml

it's kinda kludgy but it's the only thing i can think of to disable the less-than-perfect existing behavior in a backward-compatible way. 🤷

[ChristianCiach]

I don't know how often this extremely questionable behaviour of kustomize has broken our network policies by accident. In our team we've come to the conclusion to never use matchLabels in our network policies at all, for this very reason. Instead we only ever use matchExpressions, because Kustomize thankfully doesn't touch these. Of course, this doesn't help you if you source your network policies from remote bases or helm charts.

The fact that Kustomize destroys patches matchLabels but keeps matchExpressions alone makes me believe that the current behaviour is a mistake, possible implemented by accident. I would love to have an option to disable this.

Yes, these proposed options seem a bit clunky at first, but not any clunkier than the existing options of the labels generator.

[ChristianCiach]

By the way, this issue is very related to #2034.

According to the last comment there, we should be able to already use includeSelectors: false to avoid patching the selectors of Network Policies. This is news to me and I did never try it.

Edit: Oops, this is already mentioned in the issue description. This issue is explicitly about includeSelectors: true. I agree that it could make sense to control the behaviour of patching Network Policies separately.

[sda399]

just like includeSelectors and includeTemplates, a new behavior modifier could be introduced. e.g.

why not reorganize your network policies, mind you share your layout?

you can set labels with selectors in app/kustomization.yaml and labels with no selectors in netpol/kustomization.yaml
I think it is clear that Kustomize Labels are just a handy way to propagate labels to sub directories, so the directory organization should come first in mind. Or you might as well go write it manually in your manifests.

comonent01/
  app/
    - kusotmization.yaml
    - deployment.yaml
    - service.yaml
  netpols/
    - kustomization.yaml
    - netpol-component01.yaml

[sdickhoven]

why not reorganize your network policies, mind you share your layout?

i am aware that there are several workarounds to get what i want. but the point of this issue is that it is nonsensical for kustomize to modify label selectors in netpol rules (which 99 times out of 100 have nothing whatsoever to do with the workload being kustomized).

but to answer your question, we typically have layouts like this:

/ config
  / base
    / kustomization.yaml
    / deployment.yaml
    / service.yaml
    / servce-account.yaml
    / network-policy.yaml
    / ...
  / staging
    / kustomization.yaml
    / env-overlay.yaml
  / prod
    / kustomization.yaml
    / env-overlay.yaml

since our prod and staging clusters are very consistent (so that deployments can be properly tested in staging), all the selectors (for both the netpol's pod selector as well as the pod selectors of its rules) are 100% identical between the two environments.

so, yes, i could move the network-policy.yaml from /config/base to /config/staging & /config/prod... but then i literally have the same thing twice for absolutely no good reason.

[sda399]

but if i have a kustomization with 50 different resources (Deployments, ServiceAccounts, NetworkPolicys, PodDisruptionBudgets, Ingresses, Services, ServiceMonitors, ...) it becomes a bit tedious to spec out all label fields that i want modified.

so, yes, i could move the network-policy.yaml from /config/base to /config/staging & /config/prod... but then i literally have the same thing twice for absolutely no good reason.

or like so:

root
├── labelTransformers
│   ├── base
│   │   ├── kustomization.yaml
│   │   └── nnpl.yaml
│   └── prod
│       └── kustomization.yaml
├── kustomization.yaml
└── netpol.yaml

#! /bin/sh

R=root
[ ! -d $R ] || rm -r $R
mk() { mkdir -p ${1%/*}  && echo "$2" >$1 ; }

mk $R/netpol.yaml '
apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
  name: my-app
spec:
  podSelector:
    matchLabels: {}
  ingress:
  - from:
    - namespaceSelector:
        matchLabels:
          kubernetes.io/metadata.name: ingress-nginx
      podSelector:
        matchLabels:
          app.kubernetes.io/name: ingress-nginx
          app.kubernetes.io/component: controller
    - namespaceSelector:
        matchLabels:
          kubernetes.io/metadata.name: kube-system
      podSelector:
        matchLabels:
          app.kubernetes.io/name: prometheus
    ports:
    - protocol: TCP
      port: 8080
'

mk $R/labelTransformers/base/kustomization.yaml '
resources:
- nnpl.yaml
'

mk $R/labelTransformers/base/nnpl.yaml '
apiVersion: builtin
kind: LabelTransformer
metadata:
  name: no-netpol

labels: {}

fieldSpecs:

### METADATA LABELS / INCLUDE_TEMPLATES -------------------------------------
- path: metadata/labels
  create: true

- path: spec/template/metadata/labels
  create: true
  version: v1
  kind: ReplicationController

- path: spec/template/metadata/labels
  create: true
  kind: Deployment

- path: spec/template/metadata/labels
  create: true
  kind: ReplicaSet

- path: spec/template/metadata/labels
  create: true
  kind: DaemonSet

- path: spec/template/metadata/labels
  create: true
  group: apps
  kind: StatefulSet

- path: spec/volumeClaimTemplates[]/metadata/labels
  create: true
  group: apps
  kind: StatefulSet

- path: spec/template/metadata/labels
  create: true
  group: batch
  kind: Job

- path: spec/jobTemplate/metadata/labels
  create: true
  group: batch
  kind: CronJob

- path: spec/jobTemplate/spec/template/metadata/labels
  create: true
  group: batch
  kind: CronJob



### COMMON LABELS / INCLUDE_SELECTORS -------------------------------------------
- path: spec/selector
  create: true
  version: v1
  kind: Service

- path: spec/selector
  create: true
  version: v1
  kind: ReplicationController
- path: spec/selector/matchLabels
  create: true
  kind: Deployment

- path: spec/template/spec/affinity/podAffinity/preferredDuringSchedulingIgnoredDuringExecution/podAffinityTerm/labelSelector/matchLabels
  create: false
  group: apps
  kind: Deployment

- path: spec/template/spec/affinity/podAffinity/requiredDuringSchedulingIgnoredDuringExecution/labelSelector/matchLabels
  create: false
  group: apps
  kind: Deployment

- path: spec/template/spec/affinity/podAntiAffinity/preferredDuringSchedulingIgnoredDuringExecution/podAffinityTerm/labelSelector/matchLabels
  create: false
  group: apps
  kind: Deployment

- path: spec/template/spec/affinity/podAntiAffinity/requiredDuringSchedulingIgnoredDuringExecution/labelSelector/matchLabels
  create: false
  group: apps
  kind: Deployment

- path: spec/template/spec/topologySpreadConstraints/labelSelector/matchLabels
  create: false
  group: apps
  kind: Deployment

- path: spec/selector/matchLabels
  create: true
  kind: ReplicaSet

- path: spec/selector/matchLabels
  create: true
  kind: DaemonSet

- path: spec/selector/matchLabels
  create: true
  group: apps
  kind: StatefulSet

- path: spec/template/spec/affinity/podAffinity/preferredDuringSchedulingIgnoredDuringExecution/podAffinityTerm/labelSelector/matchLabels
  create: false
  group: apps
  kind: StatefulSet

- path: spec/template/spec/affinity/podAffinity/requiredDuringSchedulingIgnoredDuringExecution/labelSelector/matchLabels
  create: false
  group: apps
  kind: StatefulSet

- path: spec/template/spec/affinity/podAntiAffinity/preferredDuringSchedulingIgnoredDuringExecution/podAffinityTerm/labelSelector/matchLabels
  create: false
  group: apps
  kind: StatefulSet

- path: spec/template/spec/affinity/podAntiAffinity/requiredDuringSchedulingIgnoredDuringExecution/labelSelector/matchLabels
  create: false
  group: apps
  kind: StatefulSet

- path: spec/template/spec/topologySpreadConstraints/labelSelector/matchLabels
  create: false
  group: apps
  kind: StatefulSet

- path: spec/selector/matchLabels
  create: false
  group: batch
  kind: Job

- path: spec/jobTemplate/spec/selector/matchLabels
  create: false
  group: batch
  kind: CronJob

- path: spec/selector/matchLabels
  create: false
  group: policy
  kind: PodDisruptionBudget

- path: spec/podSelector/matchLabels
  create: false
  group: networking.k8s.io
  kind: NetworkPolicy

#- path: spec/ingress/from/podSelector/matchLabels
#  create: false
#  group: networking.k8s.io
#  kind: NetworkPolicy
#
#- path: spec/egress/to/podSelector/matchLabels
#  create: false
#  group: networking.k8s.io
#  kind: NetworkPolicy
'
mk $R/labelTransformers/prod/kustomization.yaml '
resources:
- ../base/

patches:
- patch: |-
    apiVersion: builtin
    kind: LabelTransformer
    metadata:
      name: no-netpol
    labels:
      $patch: replace
      cluster: prod
      debug: info
'

mk $R/kustomization.yaml '
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
namespace: foo
resources:
- netpol.yaml

transformers:
- labelTransformers/prod
'



cat <<EOF > expected.yaml
apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
  labels:
    cluster: prod
    debug: info
  name: my-app
  namespace: foo
spec:
  ingress:
  - from:
    - namespaceSelector:
        matchLabels:
          kubernetes.io/metadata.name: ingress-nginx
      podSelector:
        matchLabels:
          app.kubernetes.io/component: controller
          app.kubernetes.io/name: ingress-nginx
    - namespaceSelector:
        matchLabels:
          kubernetes.io/metadata.name: kube-system
      podSelector:
        matchLabels:
          app.kubernetes.io/name: prometheus
    ports:
    - port: 8080
      protocol: TCP
  podSelector:
    matchLabels:
      cluster: prod
      debug: info
EOF
#---
kustomize build --stack-trace $R > result.yaml
printf "%-64s%s\n" expected.yaml result.yaml
printf "%130s\n" " " | tr ' ' '-'
diff -W150 -y expected.yaml result.yaml