[BUG] auto-gen should not update metadata paths

[JimBugwadia]

Software version numbers

1.3.4-rc4

Describe the bug

The auto-gen feature translates pod level paths under spec to spec.template.spec for pod controllers.

However, seems like it also attempts to translates metadata paths.

See slack discussion: https://kubernetes.slack.com/archives/CLGR9BJU9/p1618520060345800

To Reproduce

Use policy:

apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
  name: block-old-flux
spec:
  validationFailureAction: enforce
  background: false
  rules:
  - name: block-old-flux
    match:
      resources:
        kinds:
        - Pod
    validate:
      message: Cannot use old Flux v1 annotation.
      pattern:
        metadata:
          =(annotations):
            X(fluxcd.io/*): "*?"

Use this resource:

apiVersion: batch/v1beta1
kind: CronJob
metadata:
  name: flux-block-test
  annotations:
    fluxcd.io/automated: "true"
spec:
  jobTemplate:
    spec:
      template:
        spec:
          restartPolicy: Never
          containers:
          - command:
            - /bin/bash
            - -ce
            - |-
              sleep 3000
            image: bitnami/kubectl
            imagePullPolicy: IfNotPresent
            name: test
  schedule: 0 0 1 * *

This updates the CronJob to:

      pattern:
        spec:
          template:
          - metadata:
              =(annotations):
                X(fluxcd.io/*): '*?'

Expected behavior

The metadata path is not updated.

[realshuting]

There are use cases that we want to update metadata paths. If you look at a Deployment manifest, there are two metadata in its definition.

metadata:
...
spec:
  template:
    metadata:
    ...
    spec:

From the Slack conversation, the match resources have both Deployment and Pod, in this case, we can assume it's the top-level metadata. So Kyverno should set annotation to pod-policies.kyverno.io/autogen-controllers=none by default.

[JimBugwadia]

Yes, this is referring to the the top-level metadata only

[JimBugwadia]

Rethinking based on @realshuting's insights above....perhaps the only thing we need to make sure of is that if multiple controllers are specified in the policy, Kyverno does not apply any auto-gen behaviors:

For example, this policy should not trigger any auto-gen rules:

apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
  name: block-old-flux
spec:
  validationFailureAction: enforce
  background: false
  rules:
  - name: block-old-flux
    match:
      resources:
        kinds:
        - Deployment
        - CronJob
        - Job
        - StatefulSet
        - DaemonSet
        - Pod
    validate:
      message: Cannot use old Flux v1 annotation.
      pattern:
        metadata:
          =(annotations):
            X(fluxcd.io/*): "*?"

But this policy will (as it only is on the pod) and the controller level rules will update spec.template.spec.metadata i.e. the pod level metadata.

apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
  name: block-old-flux
spec:
  validationFailureAction: enforce
  background: false
  rules:
  - name: block-old-flux
    match:
      resources:
        kinds:
        - Pod
    validate:
      message: Cannot use old Flux v1 annotation.
      pattern:
        metadata:
          =(annotations):
            X(fluxcd.io/*): "*?"

Does that make sense? Any other constraints?

[chipzoller]

perhaps the only thing we need to make sure of is that if multiple controllers are specified in the policy, Kyverno does not apply any auto-gen behaviors:

I agree with this. And further, this use case is an example for the need for allowing a wildcard (*) in the kinds array. Because all Kubernetes resources share the same metadata object at the same level, it's conceivable that someone may want to, for example, block use of this annotation for any and every kind throughout the cluster. Naming them one-by-one doesn't scale well.

But this policy will (as it only is on the pod) and the controller level rules will update spec.template.spec.metadata i.e. the pod level metadata.

Yes, and the only reason the policy with the multiple kinds was specified was to workaround the (at the time unknown) auto-gen controller behavior.

[JimBugwadia]

Because all Kubernetes resources share the same metadata object at the same level, it's conceivable that someone may want to, for example, block use of this annotation for any and every kind throughout the cluster. Naming them one-by-one doesn't scale well.

Yes, its certainly conceivable but what we need to determine is whether this a common real world use case which justifies the performance penalty of evaluating the rule for each admission review request including ones like SubjectAccessReview?

[chipzoller]

That's exactly right, yes.