Clusterclass variables #16

mantis-toboggan-md · 2023-12-13T21:13:58Z

This PR adds components to generate form elements from clusterclass variable definitions. I used this modified docker quickstart clusterclass to test all input and validation options:

apiVersion: cluster.x-k8s.io/v1beta1
kind: ClusterClass
metadata:
  name: 'quickstart-more-variables'
  namespace: default
spec:
  controlPlane:
    machineInfrastructure:
      ref:
        apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
        kind: DockerMachineTemplate
        name: quick-start-control-plane
        namespace: default
    metadata: {}
    ref:
      apiVersion: controlplane.cluster.x-k8s.io/v1beta1
      kind: KubeadmControlPlaneTemplate
      name: quick-start-control-plane
      namespace: default
  infrastructure:
    ref:
      apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
      kind: DockerClusterTemplate
      name: quick-start-cluster
      namespace: default
  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
    - name: string-required
      required: true
      schema:
        openAPIV3Schema:
          default: us-east-1
          description: This string has only required validation
          type: string
    - name: string-min-length
      required: true
      schema:
        openAPIV3Schema:
          default: default
          description: >-
            this string has a rule that its length must be greater than or equal
            to 3
          example: sshkeyexample
          minLength: 3
          type: string
    - name: string-max-length
      required: true
      schema:
        openAPIV3Schema:
          default: t3.large
          description: >-
            this string has a rule that its length must be less than or equal to
            10
          maxLength: 10
          type: string
    - name: string-min-max-length
      required: true
      schema:
        openAPIV3Schema:
          default: t3.large
          description: this string has a rules that its length must be between 3 and 10
          maxLength: 10
          minLength: 3
          type: string
    - name: string-pattern
      required: true
      schema:
        openAPIV3Schema:
          default: abc-123
          description: this string must match the pattern /^[a-z]+-[0-9]+$/
          example: zz-00
          pattern: ^[a-z]+-[0-9]+$
          type: string
    - name: string-no-validate
      required: false
      schema:
        openAPIV3Schema:
          default: t3.large
          description: this string has no validation
          type: string
    - name: number-exclusive-min-max
      required: false
      schema:
        openAPIV3Schema:
          default: 3
          description: this string has exclusive min max of 2 and 5
          exclusiveMaximum: true
          exclusiveMinimum: true
          maximum: 5
          minimum: 2
          type: number
    - name: number-inclusive-min-max
      required: false
      schema:
        openAPIV3Schema:
          default: 3
          description: this string has inclusive min max of 2 and 5
          maximum: 5
          minimum: 2
          type: number
    - name: testBoolean
      required: true
      schema:
        openAPIV3Schema:
          description: This is a test value
          type: boolean
    - name: testArrayStrings-min
      required: false
      schema:
        openAPIV3Schema:
          description: This array must have at least 3 items
          items:
            type: string
          minItems: 3
          type: array
    - name: testArrayStrings-max
      required: false
      schema:
        openAPIV3Schema:
          description: This array must have at most 5 items
          items:
            type: string
          maxItems: 5
          type: array
    - name: enums
      required: false
      schema:
        openAPIV3Schema:
          description: This is a string type with enum defined
          enum:
            - option_1
            - option_2
            - option_3
          type: string
    - name: enums-multi
      required: false
      schema:
        openAPIV3Schema:
          description: >-
            This is an array type with enum defined. It may have at most two
            elements
          items:
            type: string
          maxItems: 2
          type: array
    - name: enums-object
      required: false
      schema:
        openAPIV3Schema:
          description: This is an object type with enum defined
          enum:
            - option_1: '1'
            - option_2: '2'
            - option_3: '3'
          type: object
  workers:
    machineDeployments:
      - class: default-worker
        template:
          bootstrap:
            ref:
              apiVersion: bootstrap.cluster.x-k8s.io/v1beta1
              kind: KubeadmConfigTemplate
              name: quick-start-default-worker-bootstraptemplate
              namespace: default
          infrastructure:
            ref:
              apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
              kind: DockerMachineTemplate
              name: quick-start-default-worker-machinetemplate
              namespace: default
          metadata: {}

mantis-toboggan-md · 2023-12-22T16:55:19Z

I updated this PR to add (limited) support for machine deployment variable overrides. It's limited because I don't think we can reasonably parse variable names if a patch is using valueFrom.template https://cluster-api.sigs.k8s.io/tasks/experimental-features/cluster-class/write-clusterclass#using-variable-values-in-json-patches

eva-vashkevich · 2023-12-22T18:22:28Z

pkg/capi/components/CCVariables/Variable.vue

+      const required = this.variable?.required;
+
+      if (required) {
+        // out.push(formRulesGenerator(this.$store.getters['i18n/t'], { key: this.variable.name }).required as Validator);


commented out code

eva-vashkevich · 2023-12-22T18:24:23Z

pkg/capi/components/CCVariables/index.vue

+      default: () => {}
+    },
+
+    // <cluster.x-k8s.io>.spec.topology.variables


is this intentional?

Originally yeah, but I guess it's redundant now that the cluster variables are typed. I've removed it

eva-vashkevich · 2023-12-22T18:26:58Z

pkg/capi/components/CCVariables/index.vue

+   *  remove any variables not defined in the new cluster class
+   *  if a variable is defined in both cluster classes, clear out the old default
+   *  set default values from the new cluster class definitions
+   *  if a variable is defined in both old and new cluster classes, and the user has set its value to something other than the old default, preserve that


Are you sure we want to preserve it? I'd reset it to the default

I mean I guess it depends on why a user would be changing the cluster class after entering in variables. I would definitely find it tiresome to re-enter data if I was swapping between cluster classes to verify that I had the one I wanted.

eva-vashkevich · 2023-12-22T18:30:26Z

pkg/capi/util/validators.ts

+import { Validator, ValidationOptions } from '@shell/utils/validators/formRules';
+import { Translation } from '@shell/types/t';
+
+// const stringFormats = {


is this intentional?

No, sorry - I updated the PR to include the validation I had started writing out here & removed this.

mantis-toboggan-md · 2024-01-03T01:42:36Z

pkg/capi/components/CCVariables/index.vue

+      }
+    },
+
+    newComponentType(variableDef: ClusterClassVariable, i: number) {


I'm not crazy about this nor the re-render key hack, but I found it absurdly convoluted to create a new row for each new component type using purely scss; this is easy to follow ultimately.

mantis-toboggan-md · 2024-01-03T01:46:54Z

pkg/capi/edit/cluster.x-k8s.io.cluster.vue

@@ -0,0 +1,115 @@
+<script lang="ts">


This is just a simple way for me to test the CCVariables component - I will remove it before merging.

eva-vashkevich

Looks good with some non-blocking comments

eva-vashkevich · 2024-01-12T21:25:09Z

pkg/capi/components/CCVariables/index.vue

+
+      const patches = this.clusterClass?.spec?.patches || [];
+
+      patches.forEach((p) => {


type is missing

eva-vashkevich · 2024-01-12T21:45:21Z

pkg/capi/components/CCVariables/index.vue

+        const neuDef = (neu || []).find(n => n.name === existingVar.name);
+
+        if (!neuDef) {
+          return acc;


IMO, if there is no default and currently it is set to the old default, I think it should be an empty value and not the old default

I've updated inline comments to make this function clearer; from my own testing, it does what you're describing, but let me know if I missed something.

eva-vashkevich · 2024-01-12T21:48:50Z

pkg/capi/components/CCVariables/Variable.vue

+      const required = this.variable?.required;
+
+      if (required && this.validateRequired) {
+        out.push(val => !isDefined(val) ? t('validation.required', { key: this.variable.name }) : undefined);


type is missing

eva-vashkevich · 2024-01-12T21:54:13Z

pkg/capi/l10n/en-us.yaml

+  exclusiveMaxValue: '"{key}" must be less than {maximum}.'
+  exclusiveMinValue: '"{key}" must be greater than {minimum}.'
+  maxItems: '"{key}" may not contain more than {maxItems} items.'


Should we handle a case of 1 item separately? otherwise we are getting "Must not contain more than 1 items" which is not correct.

eva-vashkevich · 2024-01-12T21:54:24Z

pkg/capi/l10n/en-us.yaml

+  exclusiveMinValue: '"{key}" must be greater than {minimum}.'
+  maxItems: '"{key}" may not contain more than {maxItems} items.'
+  minItems: '"{key}" must contain at least {minItems} items.'


mantis-toboggan-md requested a review from eva-vashkevich December 13, 2023 21:13

mantis-toboggan-md force-pushed the clusterclass-variables branch from 76cccb3 to 2ff9339 Compare December 20, 2023 23:45

eva-vashkevich reviewed Dec 22, 2023

View reviewed changes

mantis-toboggan-md commented Jan 3, 2024

View reviewed changes

mantis-toboggan-md requested a review from eva-vashkevich January 3, 2024 01:45

mantis-toboggan-md commented Jan 3, 2024

View reviewed changes

mantis-toboggan-md force-pushed the clusterclass-variables branch from 5d575e5 to 73a1ac5 Compare January 11, 2024 22:48

mantis-toboggan-md added 22 commits January 12, 2024 12:05

enums

0c2ae5c

required validation

89782ce

style

850caa2

more validators

7a1106b

fix length validators

c19c624

remove test page

51566eb

jsdoc openapiv3schema validator function

f07b726

fix variable widths and new rows

eabe146

refactor error handling

da8dac3

remove test file

1423371

re-set variables when clusterclass changes

ae0d883

fix spacer element visibility when cluster class changes

c069fb2

update required validator to account for a required boolean

f74e868

add machine-scoped ccvariables

b30c156

exclude patches using builtin vars

dff4855

update list component validation, required validator

f6469e8

add string format validation

4ffe731

cleanup following review

8fba487

fix component imports

18d82ed

fix ts warnings

da0982a

typescript fixes 2 electric bugaloo

dceb67d

fix vscode settings after rebase

30c7bec

mantis-toboggan-md force-pushed the clusterclass-variables branch from 74e5d54 to 30c7bec Compare January 12, 2024 20:09

eva-vashkevich approved these changes Jan 12, 2024

View reviewed changes

updates following review

c8bfd1e

mantis-toboggan-md requested a review from eva-vashkevich January 16, 2024 22:23

mantis-toboggan-md merged commit d46382d into rancher:main Jan 23, 2024
1 check passed

mantis-toboggan-md mentioned this pull request Feb 13, 2024

[CAPI] add support for clusterclass variables #22

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clusterclass variables #16

Clusterclass variables #16

mantis-toboggan-md commented Dec 13, 2023 •

edited

Loading

mantis-toboggan-md commented Dec 22, 2023

eva-vashkevich Dec 22, 2023

eva-vashkevich Dec 22, 2023

mantis-toboggan-md Jan 3, 2024

eva-vashkevich Dec 22, 2023

mantis-toboggan-md Jan 3, 2024

eva-vashkevich Jan 3, 2024

eva-vashkevich Dec 22, 2023

mantis-toboggan-md Jan 3, 2024

mantis-toboggan-md Jan 3, 2024

mantis-toboggan-md Jan 3, 2024

eva-vashkevich left a comment

eva-vashkevich Jan 12, 2024

eva-vashkevich Jan 12, 2024

mantis-toboggan-md Jan 16, 2024

eva-vashkevich Jan 12, 2024

eva-vashkevich Jan 12, 2024

eva-vashkevich Jan 12, 2024


		const patches = this.clusterClass?.spec?.patches \|\| [];

		patches.forEach((p) => {

Clusterclass variables #16

Clusterclass variables #16

Conversation

mantis-toboggan-md commented Dec 13, 2023 • edited Loading

mantis-toboggan-md commented Dec 22, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eva-vashkevich left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mantis-toboggan-md commented Dec 13, 2023 •

edited

Loading