[Feature request] Strongly-typed fields for CRD

[ItalyPaleAle]

Currently, the CRD used by this operator has a single property "value" which accepts anything (any JSON-serializable option).

We would like to propose a new version of the CRD (possibly v1alpha2 or v2alpha1?) which lists the allowed fields explicitly, including most (though not all) options that are allowed by the Dapr Helm chart today.

The proposed schema (which is based on Dapr 1.11 and is missing a few fields for 1.12):

spec:
  # Dapr version
  version: 'string; required'

  images:
    registry: 'string; defaults to "ghcr.io/dapr"'
    tag: 'string; defaults to the .version property'
    # For control-plane services and sidecars
    imagePullPolicy: 'string (ImagePullPolicy); defaults to "IfNotPresent"'
    imagePullSecrets: 'string'
  
  # Options for injected Dapr sidecars
  sidecars:
    # Full name of the Docker image for the Dapr sidecar.
    # This is used to override the default image when deploying sidecars, and it's useful for debugging.
    # Specifying a value here changes the ImagePullPolicy for sidecars to "Always". Otherwise, the sidecars' ImagePullPolicy is set to the same as `global.imagePullPolicy` (`IfNotPresent` by default).
    sidecarImage: 'string'
    runAsNonRoot: 'boolean; defaults to "true"'
    readOnlyRootFilesystem: 'boolean; defaults to "true"'
    # When enabled, the sidecar container has `securityContext.capabilities.drop: ["ALL"]`.
    dropAllCapabilities: 'boolean; defaults to "false"'
  
  # Options for the Dapr control plane
  controlPlane:
    logLevel: 'string; defaults to "info"'
    logAsJSON: 'boolean; defaults to "false"'
    os: 'string; defaults to "linux"'
    arch: 'string; defaults to "amd64"'
    enableMetrics: 'boolean; defaults to "true"'
    metricsPort: 'boolean; defaults to "9090"'
    tolerations: 'array of corev1.Toleration'
    seccompProfile: 'string'
    runAsNonRoot: 'boolean; defaults to "true"'
    operator:
      replicaCount: 'int; defaults to "1"'
      resources: 'kuberentes resources object'
      enableServiceReconciler: 'boolean; defaults to "false"'
    sentry:
      replicaCount: 'int; defaults to "1"'
      resources: 'kuberentes resources object'
      tokenAudience: 'string'
      trustDomain: 'string'
    placement:
      # If true, enables high-availability mode with 3 replicas.
      # Otherwise, a single replica is used.
      ha: 'boolean; defaults to "false"'
      resources: 'kuberentes resources object'
      # Only for HA mode
      cluster:
        inMemoryLog: 'boolean; defaults to "true"'
        logStorePath: 'string'
        logStorePathWindows: 'string'
      # Unused when HA mode is used and cluster.inMemoryLog is true
      volumeClaims: 
        storageSize: 'string'
        storageClassName: 'string'

  # Needs to be updated for Dapr 1.12
  mTLS:
    enabled: 'boolean; defaults to "true"'
    workloadCertTTL: 'string'
    allowedClockSkew: 'string'

  dnsSuffix: 'string; defaults to ".cluster.local"'

  # Additional options passed to Helm as-is.
  # Each option is passed as "--set" to Helm.
  # These are passed after any option built from the spec.
  additionalHelmOptions: 'array of strings'

The reconciler would then generate the Helm --set flags based on the values defined above in the CRD.

A benefit of using a CRD with strictly-defined fields is that it allows catching errors when the resource is deployed on K8s, before the reconciler is invoked. It also allows auto-completion in code editors, among other things.

[lburgazzoli]

@ItalyPaleAle do you know if the helm charts provides a schema for the values ? if that's exist it would be simple to include such schema in the CRD definition.

[lburgazzoli]

An additional consideration: if we go this path, then the values expected by the helm chart must be backward compatible or we would end up having to create a number of CRDs, one each time a breaking change is introduced in the helm chart values.

[ItalyPaleAle]

@ItalyPaleAle do you know if the helm charts provides a schema for the values ? if that's exist it would be simple to include such schema in the CRD definition.

There’s the values.yaml but it’s not really a schema

An additional consideration: if we go this path, then the values expected by the helm chart must be backward compatible or we would end up having to create a number of CRDs, one each time a breaking change is introduced in the helm chart values.

They mostly are. Fields are added but generally not removed. Also we wouldn’t be removing values from the CRD here to be able to support older versions of dapr. Passing extra values to Helm is not a problem as they are ignored.

[lburgazzoli]

@ItalyPaleAle do you know if the helm charts provides a schema for the values ? if that's exist it would be simple to include such schema in the CRD definition.

There’s the values.yaml but it’s not really a schema

I think that since helm support schema files for values.yaml, the best would be to have it added to the helm charts first, then we can leverage it later in the CRD and/or validation.

In the meantime we can add a subset of commonly used/stable options to the CRD to make it more type safe, @ItalyPaleAle what do you think ?

[ItalyPaleAle]

I think that since helm support schema files for values.yaml, the best would be to have it added to the helm charts first, then we can leverage it later in the CRD and/or validation.

I think this could be a good idea regardless, although I don't know how quickly it could be implemented.

Also want to point out that the schema above has some differences with the values in "values.yaml" in the OSS project. For example, setting a custom sidecar image also sets "ImagePullPolicy" to always for that pod (since custom sidecar images are generally used for development/testing). Or the grouping of some options is different.

The idea is that this could be a bit more opinionated (but not by much).

In the meantime we can add a subset of commonly used/stable options to the CRD to make it more type safe

How large would this subset be?