Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explore CEL validation for ClusterClass variables #8565

Open
sbueringer opened this issue Apr 25, 2023 · 9 comments
Open

Explore CEL validation for ClusterClass variables #8565

sbueringer opened this issue Apr 25, 2023 · 9 comments
Assignees
Labels
area/clusterclass Issues or PRs related to clusterclass help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. priority/backlog Higher priority than priority/awaiting-more-evidence. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@sbueringer
Copy link
Member

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.

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. area/topology needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Apr 25, 2023
@fabriziopandini
Copy link
Member

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

/help

@k8s-ci-robot
Copy link
Contributor

@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:

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

/help

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/test-infra repository.

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels May 1, 2023
@killianmuldoon killianmuldoon added the area/clusterclass Issues or PRs related to clusterclass label May 4, 2023
@chaunceyjiang
Copy link
Contributor

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
Copy link
Member Author

Has to be implemented first

@chaunceyjiang
Copy link
Contributor

Can i pick up this issue?

@chaunceyjiang
Copy link
Contributor

/assign

@chrischdi
Copy link
Member

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

@sbueringer
Copy link
Member Author

sbueringer commented Apr 2, 2024

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
Copy link
Member

/priority backlog

@k8s-ci-robot k8s-ci-robot added the priority/backlog Higher priority than priority/awaiting-more-evidence. label Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/clusterclass Issues or PRs related to clusterclass help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. priority/backlog Higher priority than priority/awaiting-more-evidence. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

No branches or pull requests

6 participants