Improved support for Pod related Validating Admission Policies.

[vinayakankugoyal]

What would you like to be added?

Validating Admission Policies is one of my favorite features in kubernetes. I have been using it to write several policies. The most common type of of policies I write are related to PodSpec (most security folks also likely end up doing this). However most of the time I end up writing a policy like the following.

apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingAdmissionPolicy
metadata:
  name: "no-gitrepo-volumes"
spec:
  failurePolicy: Fail
  matchConstraints:
    resourceRules:
    - apiGroups:   [""]
      apiVersions: ["v1"]
      operations:  ["CREATE", "UPDATE"]
      resources:   ["pods", "podtemplates", "replicationcontrollers"]
    - apiGroups:   ["apps"]
      apiVersions: ["v1"]
      operations:  ["CREATE", "UPDATE"]
      resources:   ["deployments", "replicasets", "statefulsets", "daemonsets"]
    - apiGroups:   ["batch"]
      apiVersions: ["v1"]
      operations:  ["CREATE", "UPDATE"]
      resources:   ["jobs", "cronjobs"]
  validations:
    - expression: |-
        object.kind != 'Pod' ||
        !object.spec.?volumes.orValue([]).exists(v, has(v.gitRepo))
      message: "gitRepo volumes are not allowed."
    - expression: |-
        object.kind != 'PodTemplate' ||
        !object.template.spec.?volumes.orValue([]).exists(v, has(v.gitRepo))
      message: "gitRepo volumes are not allowed."
    - expression: |-
        !['Deployment','ReplicaSet','DaemonSet','StatefulSet','Job', 'ReplicationController'].exists(kind, object.kind == kind) ||
        !object.spec.template.spec.?volumes.orValue([]).exists(v, has(v.gitRepo))
      message: "gitRepo volumes are not allowed."
    - expression: |-
        object.kind != 'CronJob' ||
        !object.spec.jobTemplate.spec.template.spec.?volumes.orValue([]).exists(v, has(v.gitRepo))
      message: "gitRepo volumes are not allowed."

Notice how I have to extract the PodSpec from different object types and so have to write multiple validating expressions. This gets annoying and messy. Could we provide a function or something that extracts the PodSpec from the templates? Then we could write policies like:

apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingAdmissionPolicy
metadata:
  name: "no-gitrepo-volumes"
spec:
  failurePolicy: Fail
  matchConstraints:
    resourceRules:
    - apiGroups:   [""]
      apiVersions: ["v1"]
      operations:  ["CREATE", "UPDATE"]
      resources:   ["pods", "podtemplates", "replicationcontrollers"]
    - apiGroups:   ["apps"]
      apiVersions: ["v1"]
      operations:  ["CREATE", "UPDATE"]
      resources:   ["deployments", "replicasets", "statefulsets", "daemonsets"]
    - apiGroups:   ["batch"]
      apiVersions: ["v1"]
      operations:  ["CREATE", "UPDATE"]
      resources:   ["jobs", "cronjobs"]
  validations:
    - expression: |-
        !extractPodSpec(object).?volumes.orValue([]).exists(v, has(v.gitRepo))
      message: "gitRepo volumes are not allowed."

Why is this needed?

Adding something similar to what I proposed above will greatly help in simplifying policy authoring and therefore reduce the chances of bugs in policies.

[k8s-ci-robot]

This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

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.

[cici37]

/sig api-machinery

[cici37]

I guess this could go to the umbrella CEL wishlist function issue: #124490

[cici37]

For extracting podSpec, there should be workaround for this. Have you tried using variables?
You could have a variable defined for the podSpec and use it directly in the validation expression.

[vinayakankugoyal]

The variable approach doesn't seem much better because the expression for the variable itself would have to be something pretty "gnarly" like

object.kind == Pod ? object.spec : object.kind == PodTemplate ? object.template.spec : ['Deployment','ReplicaSet','DaemonSet','StatefulSet','Job', 'ReplicationController'].exists(kind, object.kind == kind) ? object.spec.template.spec : object.kind != 'CronJob' ? object.spec.jobTemplate.spec.template.spec. : nil

its a workaround but its very hard to read and someone will make a mistake, these are meant to be security policies so we want simplicity of expression so that the chances of getting something wrong are low

[cici37]

For the cel extension library added, I personally would like it to be more generic rather than being used for simplifying the expression only. If there is great reason for adding this kind of support I am not strongly against it as well. For the concern of having mistake on the expression, it could be prevented by properly testing? Love to hear more thoughts on this.

[BenTheElder]

/assign @cici37