Skip to content
This repository has been archived by the owner on Mar 4, 2024. It is now read-only.

Stricter validation in pod_set_spec #17

Open
knkski opened this issue Mar 20, 2019 · 3 comments
Open

Stricter validation in pod_set_spec #17

knkski opened this issue Mar 20, 2019 · 3 comments

Comments

@knkski
Copy link
Contributor

knkski commented Mar 20, 2019

Not sure if this is the right repo vs https://github.com/juju/juju, can refile there if necessary.

It would be nice if pod_set_spec could either reject or warn about invalid properties the spec that it's passed. For example, I'm able to call this without warning:

layer.caas_base.pod_spec_set({
    'containers': {...}, # Some valid YAML
    'customResourceDefinition': [{
        ... # Some valid YAML
        'names': {...} # The `spec.names` key that appears in normal K8S CRD YAML
    }],
})

Looking at https://github.com/juju/juju/blob/d1e735e6bfe3641bc0c25c348904f59e7be91d3c/caas/containers.go#L81, I think that property is not supported, but is never complained about, which is very confusing when trying to adapt some pre-existing CRD spec to Juju.

@johnsca
Copy link
Contributor

johnsca commented Mar 20, 2019

+1. We could do it here, but it would have to be kept in sync with core. So it might be better to have the hook tool return an error message that is then surfaced by this layer.

Edit: It looks like there is validation for required fields, but nothing to warn or reject unknown fields. Are they ignored?

@knkski
Copy link
Contributor Author

knkski commented Mar 20, 2019

@johnsca: Yeah, they're just ignored

@wallyworld
Copy link
Contributor

The yaml required to be produced by the charm is now aligned with native k8s yaml for crds. The charm specifies a map of crd spec yaml, keyed on name, eg

customResourceDefinitions:
  pytorchjobs.kubeflow.org:
    group: kubeflow.org
    version: v1beta1
    names:
      kind: PyTorchJob
      plural: pytorchjobs
      singular: pytorchjob
    scope: Namespaced
    validation:
      openAPIV3Schema:
        properties:
          spec:
            properties:
              pytorchReplicaSpecs:
                properties:
                  Master:
                    properties:
                      replicas:
                        maximum: 1
                        minimum: 1
                        type: integer
                  Worker:
                    properties:
                      replicas:
                        minimum: 1
                        type: integer

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants