Support looping over sub-elements using foreach

[JimBugwadia]

Is your feature request related to a problem? Please describe.

Several Kubernetes resources contain lists of sub-elements. For example pods contain lists of containers, init-containers, and volumes. Kyverno policy rules that require looping over these lists and performing conditional checks, or lookups, or updates, are not easy to write.

Describe the solution you'd like

Add support for a new construct called foreach that makes it easier to loop over

See: https://docs.google.com/document/d/1oZpbhjp6fJMO8KOdtNcaGWTt3DJMioHapQeVRxf8vms/edit#heading=h.x76w8o1ahkvf

Please add review comments to the document for now. I will update the issue when we finalize the design.

[stuh84]

We have an issue that would be resolved by this (as referenced in issue #1831), so would be more than happy to test/provide feedback where we can?

[MarcelMue]

I think that the implementation of this feature would be important currently. Dealing with lists (especially nested ones) seems very cumbersome currently.

I read in the design doc, that there are issues with patchStrategicMerge - would it make sense to create a first implementation of foreach only for patchesJson6902?

[realshuting]

would it make sense to create a first implementation of foreach only for patchesJson6902?

I consider this as an alternative to patch the list using mutate policies, and it shouldn't be too complex to support. We don't have any detailed design for patchStrategicMerge yet. Supporting it in JSON patch should at least solve some use cases.

[realshuting]

Adding a mutate use case from Kyverno Slack https://kubernetes.slack.com/archives/CLGR9BJU9/p1632827247033200:

spec:
  background: false
  rules:
    - name: replace-release-name-deployment-env-secret
      match:
        resources:
          kinds:
            - Deployment
      mutate:
        patchStrategicMerge:
          spec:
            template:
              spec:
                containers:
                  - (name): "*"
                    env:
                      - (name): "*"
                        valueFrom:
                          secretKeyRef:
                            (name): "release-name---*"
                            name: "{{ replace_all('{{@}}', 'release-name---', request.object.metadata.annotations.\"meta.helm.sh/release-name\") }}"
    - name: replace-release-name-deployment-env-configmap
      match:
        resources:
          kinds:
            - Deployment
      mutate:
        patchStrategicMerge:
          spec:
            template:
              spec:
                containers:
                  - (name): "*"
                    env:
                      - (name): "*"
                        valueFrom:
                          configMapKeyRef:
                            (name): "release-name---*"
                            name: "{{ replace_all('{{@}}', 'release-name---', request.object.metadata.annotations.\"meta.helm.sh/release-name\") }}"

Looks like we cannot patch names for configMapKeyRef and SecretkeyRef at the same time.

[Monska85]

Adding a mutate use case from Kyverno Slack https://kubernetes.slack.com/archives/CLGR9BJU9/p1632827247033200:

spec:
  background: false
  rules:
    - name: replace-release-name-deployment-env-secret
      match:
        resources:
          kinds:
            - Deployment
      mutate:
        patchStrategicMerge:
          spec:
            template:
              spec:
                containers:
                  - (name): "*"
                    env:
                      - (name): "*"
                        valueFrom:
                          secretKeyRef:
                            (name): "release-name---*"
                            name: "{{ replace_all('{{@}}', 'release-name---', request.object.metadata.annotations.\"meta.helm.sh/release-name\") }}"
    - name: replace-release-name-deployment-env-configmap
      match:
        resources:
          kinds:
            - Deployment
      mutate:
        patchStrategicMerge:
          spec:
            template:
              spec:
                containers:
                  - (name): "*"
                    env:
                      - (name): "*"
                        valueFrom:
                          configMapKeyRef:
                            (name): "release-name---*"
                            name: "{{ replace_all('{{@}}', 'release-name---', request.object.metadata.annotations.\"meta.helm.sh/release-name\") }}"

Looks like we cannot patch names for configMapKeyRef and SecretkeyRef at the same time.

To be more precise the secretKeyRef is correctly replaced and the configMapKeyRef is replaced only if the replacement does not contain the special value {{@}}. At the same time, if I add another rule to change simply a value, I have the same behaviour of the configMapKeyRef: the value is replaced only if does not contain the special value {{@}}.

The example below does not work

spec:
  background: false
  rules:
    - name: replace-release-name-deployment-env-secret
      [...]
                      - (name): "*"
                        valueFrom:
                          secretKeyRef:
                            (name): "release-name---*"
                            name: "{{ replace_all('{{@}}', 'release-name---', request.object.metadata.labels.\"meta.helm.sh/release-name\") }}"
    - name: replace-release-name-deployment-env-configmap
      [...]
                      - (name): "*"
                        valueFrom:
                          configMapKeyRef:
                            (name): "release-name---*"
                            name: "{{ replace_all('{{@}}', 'release-name---', request.object.metadata.labels.\"meta.helm.sh/release-name\") }}"
    - name: replace-release-name-deployment-env-value
      [...]
                      - (name): "*"
                        (value): "?*"
                        value: "{{ replace_all('{{@}}', 'release-name---', request.object.metadata.labels.\"meta.helm.sh/release-name\") }}"

Instead this example works perfectly:

spec:
  background: false
  rules:
    - name: replace-release-name-deployment-env-secret
      [...]
                      - (name): "*"
                        valueFrom:
                          secretKeyRef:
                            (name): "release-name---*"
                            name: "{{ replace_all('{{@}}', 'release-name---', request.object.metadata.labels.\"meta.helm.sh/release-name\") }}"
    - name: replace-release-name-deployment-env-configmap
      [...]
                      - (name): "*"
                        valueFrom:
                          configMapKeyRef:
                            (name): "release-name---*"
                            name: "{{ replace_all('release-name---configmap', 'release-name---', request.object.metadata.labels.\"meta.helm.sh/release-name\") }}"
    - name: replace-release-name-deployment-env-value
      [...]
                      - (name): "*"
                        (value): "?*"
                        value: "{{ replace_all('release-name---value', 'release-name---', request.object.metadata.labels.\"meta.helm.sh/release-name\") }}"

[realshuting]

Ok, so it has nothing to do with list operations, logged #2457 to track.

[realshuting]

Can we close this @JimBugwadia ?

[JimBugwadia]

We can! Not sure if a JSONPatch foreach is something we want to investigate, or if the validate and strategic merge patch are sufficient. Any thoughts?

[realshuting]

A JSONPatch iterator #2307 should be enough to address the list issue. We can evaluate once it's supported and open new issues if needed.

[realshuting]

foreach mutate is supported via #2493.

[bluebrown]

how can this handle nested lists? For example, I want to do something like below.

  - list: request.object.spec.tls[].hosts
    patchesJson6902: |-
      - path: /spec/tls/{{elementIndex}}/hosts/{{elementIndex}}
        op: replace
        value: {{replace('{{element}}', '.old.com', '.new.com', `1`)}}