Explore CEL validation for ClusterClass variables

sbueringer commented 1 year ago

What would you like to be added (User Story)?

As a user it would be nice if I could use CEL to validate ClusterClass variables.

Detailed Description

Context:

ClusterClass variable validation today heavily reuses CRD OpenAPI schema validation
Recently CEL was introduced to validate CRs

Links:

Anything else you would like to add?

If there is interest in the community, it could be interesting to explore if we could also integrate CEL into variable validation. Today we already use the upstream code for OpenAPI schema validation. We could probably get CEL validation almost for free (as it's probably implemented in the code we already use)

Label(s) to be applied

/kind feature /area topology One or more /area label. See https://github.com/kubernetes-sigs/cluster-api/labels?q=area for the list of labels.

fabriziopandini commented 1 year ago

/triage accepted I think this is an interesting exploration to carry on

/help

k8s-ci-robot commented 1 year ago

@fabriziopandini: This request has been marked as needing help from a contributor.

Guidelines

Please ensure that the issue body includes answers to the following questions:

Why are we solving this issue?
To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
Does this issue have zero to low barrier of entry?
How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed by commenting with the /remove-help command.

In response to [this](https://github.com/kubernetes-sigs/cluster-api/issues/8565): >/triage accepted >I think this is an interesting exploration to carry on > >/help Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.

chaunceyjiang commented 1 year ago

apiVersion: cluster.x-k8s.io/v1beta1
kind: ClusterClass
metadata:
  name: quick-start
  namespace: default
spec:
....
....
  - name: podSecurityStandard
    required: false
    schema:
      openAPIV3Schema:
        properties:
          audit:
            default: restricted
            description: audit sets the level for the audit PodSecurityConfiguration
              mode. One of privileged, baseline, restricted.
            type: string
          enabled:
            default: true
            description: enabled enables the patches to enable Pod Security Standard
              via AdmissionConfiguration.
            type: boolean
          enforce:
            default: baseline
            description: enforce sets the level for the enforce PodSecurityConfiguration
              mode. One of privileged, baseline, restricted.
            type: string
          warn:
            default: restricted
            description: warn sets the level for the warn PodSecurityConfiguration
              mode. One of privileged, baseline, restricted.
            type: string
        type: object
        x-kubernetes-validations:
        - message: Value is immutable
          rule: self == oldSelf
....

I wanted to try writing CEL expressions in ClusterClass variables, but I failed. I received an error message strict decoding error: unknown field "spec.variables[3].schema.openAPIV3Schema.x-kubernetes-validations".

This is the complete content of clusterclass.

``` apiVersion: cluster.x-k8s.io/v1beta1 kind: ClusterClass metadata: name: quick-start namespace: default spec: controlPlane: machineInfrastructure: ref: apiVersion: infrastructure.cluster.x-k8s.io/v1beta1 kind: DockerMachineTemplate name: quick-start-control-plane ref: apiVersion: controlplane.cluster.x-k8s.io/v1beta1 kind: KubeadmControlPlaneTemplate name: quick-start-control-plane infrastructure: ref: apiVersion: infrastructure.cluster.x-k8s.io/v1beta1 kind: DockerClusterTemplate name: quick-start-cluster patches: - definitions: - jsonPatches: - op: add path: /spec/template/spec/kubeadmConfigSpec/clusterConfiguration/imageRepository valueFrom: variable: imageRepository selector: apiVersion: controlplane.cluster.x-k8s.io/v1beta1 kind: KubeadmControlPlaneTemplate matchResources: controlPlane: true description: Sets the imageRepository used for the KubeadmControlPlane. enabledIf: '{{ ne .imageRepository "" }}' name: imageRepository - definitions: - jsonPatches: - op: add path: /spec/template/spec/kubeadmConfigSpec/clusterConfiguration/etcd valueFrom: template: | local: imageTag: {{ .etcdImageTag }} selector: apiVersion: controlplane.cluster.x-k8s.io/v1beta1 kind: KubeadmControlPlaneTemplate matchResources: controlPlane: true description: Sets tag to use for the etcd image in the KubeadmControlPlane. name: etcdImageTag - definitions: - jsonPatches: - op: add path: /spec/template/spec/kubeadmConfigSpec/clusterConfiguration/dns valueFrom: template: | imageTag: {{ .coreDNSImageTag }} selector: apiVersion: controlplane.cluster.x-k8s.io/v1beta1 kind: KubeadmControlPlaneTemplate matchResources: controlPlane: true description: Sets tag to use for the etcd image in the KubeadmControlPlane. name: coreDNSImageTag - definitions: - jsonPatches: - op: add path: /spec/template/spec/customImage valueFrom: template: | kindest/node:{{ .builtin.machineDeployment.version | replace "+" "_" }} selector: apiVersion: infrastructure.cluster.x-k8s.io/v1beta1 kind: DockerMachineTemplate matchResources: machineDeploymentClass: names: - default-worker - jsonPatches: - op: add path: /spec/template/spec/customImage valueFrom: template: | kindest/node:{{ .builtin.controlPlane.version | replace "+" "_" }} selector: apiVersion: infrastructure.cluster.x-k8s.io/v1beta1 kind: DockerMachineTemplate matchResources: controlPlane: true description: Sets the container image that is used for running dockerMachines for the controlPlane and default-worker machineDeployments. name: customImage - definitions: - jsonPatches: - op: add path: /spec/template/spec/kubeadmConfigSpec/clusterConfiguration/apiServer/extraArgs value: admission-control-config-file: /etc/kubernetes/kube-apiserver-admission-pss.yaml - op: add path: /spec/template/spec/kubeadmConfigSpec/clusterConfiguration/apiServer/extraVolumes value: - hostPath: /etc/kubernetes/kube-apiserver-admission-pss.yaml mountPath: /etc/kubernetes/kube-apiserver-admission-pss.yaml name: admission-pss pathType: File readOnly: true - op: add path: /spec/template/spec/kubeadmConfigSpec/files valueFrom: template: | - content: | apiVersion: apiserver.config.k8s.io/v1 kind: AdmissionConfiguration plugins: - name: PodSecurity configuration: apiVersion: pod-security.admission.config.k8s.io/v1{{ if semverCompare "< v1.25" .builtin.controlPlane.version }}beta1{{ end }} kind: PodSecurityConfiguration defaults: enforce: "{{ .podSecurityStandard.enforce }}" enforce-version: "latest" audit: "{{ .podSecurityStandard.audit }}" audit-version: "latest" warn: "{{ .podSecurityStandard.warn }}" warn-version: "latest" exemptions: usernames: [] runtimeClasses: [] namespaces: [kube-system] path: /etc/kubernetes/kube-apiserver-admission-pss.yaml selector: apiVersion: controlplane.cluster.x-k8s.io/v1beta1 kind: KubeadmControlPlaneTemplate matchResources: controlPlane: true description: Adds an admission configuration for PodSecurity to the kube-apiserver. enabledIf: '{{ .podSecurityStandard.enabled }}' name: podSecurityStandard variables: - name: imageRepository required: true schema: openAPIV3Schema: default: "" description: imageRepository sets the container registry to pull images from. If empty, nothing will be set and the from of kubeadm will be used. example: registry.k8s.io type: string - name: etcdImageTag required: true schema: openAPIV3Schema: default: "" description: etcdImageTag sets the tag for the etcd image. example: 3.5.3-0 type: string - name: coreDNSImageTag required: true schema: openAPIV3Schema: default: "" description: coreDNSImageTag sets the tag for the coreDNS image. example: v1.8.5 type: string - name: podSecurityStandard required: false schema: openAPIV3Schema: properties: audit: default: restricted description: audit sets the level for the audit PodSecurityConfiguration mode. One of privileged, baseline, restricted. type: string enabled: default: true description: enabled enables the patches to enable Pod Security Standard via AdmissionConfiguration. type: boolean enforce: default: baseline description: enforce sets the level for the enforce PodSecurityConfiguration mode. One of privileged, baseline, restricted. type: string warn: default: restricted description: warn sets the level for the warn PodSecurityConfiguration mode. One of privileged, baseline, restricted. type: string type: object x-kubernetes-validations: - message: Value is immutable rule: self == oldSelf workers: machineDeployments: - class: default-worker template: bootstrap: ref: apiVersion: bootstrap.cluster.x-k8s.io/v1beta1 kind: KubeadmConfigTemplate name: quick-start-default-worker-bootstraptemplate infrastructure: ref: apiVersion: infrastructure.cluster.x-k8s.io/v1beta1 kind: DockerMachineTemplate name: quick-start-default-worker-machinetemplate ```

sbueringer commented 1 year ago

Has to be implemented first

chaunceyjiang commented 1 year ago

Can i pick up this issue?

chaunceyjiang commented 1 year ago

/assign

chrischdi commented 6 months ago

Question: did we consider using validating admission policies instead? https://kubernetes.io/docs/reference/access-authn-authz/validating-admission-policy/

sbueringer commented 6 months ago

Variable schema including cel that validates variable values is basically the same as schema validation in CRDs that validates CRs

I'm not sure how we would do this with validating admission policy. Especially when the variable schema is provided by a runtime extension. Would this mean the runtime extension has to generate validating admission policies who match on variable values with specific names in a Cluster? (and also match on the ClusterClass used in a Cluster)

"Inline" CEL in variable schemas like in CRDs seems more straightforward and easy to use to me. IIRC we also run the variable validation in the cluster topology controller today. So we either couldn't do this with the cel validation or we would have to find a way to run validating admission policies in our controller.

But I'm not super familiar with validating admission policies, I might miss something obvious.

fabriziopandini commented 6 months ago

/priority backlog

kubernetes-sigs / cluster-api