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

Support union types #77

Closed
ari-becker opened this issue Aug 11, 2019 · 7 comments
Closed

Support union types #77

ari-becker opened this issue Aug 11, 2019 · 7 comments

Comments

@ari-becker
Copy link
Collaborator

The new API presumes that one runs dhall-to-yaml --omitEmpty. However, there are areas in the API which expect an empty block to be defined, for example,

let Kubernetes = https://raw.githubusercontent.com/dhall-lang/dhall-kubernetes/4ad58156b7fdbbb6da0543d8b314df899feca077/defaults.dhall
in
Kubernetes.Volume // { name = "data" , emptyDir = Kubernetes.EmptyDirVolumeSource }

which unfortunately renders as

name: data

instead of:

name: data
emptyDir: {}

The current workaround (in this specific case) is to hard-code the default medium (which is the empty string) so that emptyDir renders and Kubernetes knows that the volume is an emptyDir volume. But it makes me uneasy... I know that there are various areas of the dhall-kops API which work the same way.

@arianvp
Copy link
Member

arianvp commented Aug 15, 2019

I think the problem is here that Kubernetes.Volume is actually modelling a sum-type . And does so by having a record of mutually exclusive optionals. This is a Go-ism probably; due to its lack of sum-typing. This is not supported by the OpenAPI v2 spec

https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.10/#volume-v1-core

Is is valid for a Volume to have more than 1 VolumeSource defined? I don't think so right?

The current spec is:

    "io.k8s.api.core.v1.Volume": {
      "description": "Volume represents a named volume in a pod that may be accessed by any container in the pod.",
      "properties": {
        "awsElasticBlockStore": {
          "$ref": "#/definitions/io.k8s.api.core.v1.AWSElasticBlockStoreVolumeSource",
          "description": "AWSElasticBlockStore represents an AWS Disk resource that is attached to a kubelet's host machine and then exposed to the pod. More info: https://kubernetes.io/docs/concepts/storage/volumes#awselasticblockstore"
        },
        "azureDisk": {
          "$ref": "#/definitions/io.k8s.api.core.v1.AzureDiskVolumeSource",
          "description": "AzureDisk represents an Azure Data Disk mount on the host and bind mount to the pod."
        },
        "azureFile": {
          "$ref": "#/definitions/io.k8s.api.core.v1.AzureFileVolumeSource",
          "description": "AzureFile represents an Azure File Service mount on the host and bind mount to the pod."
        },
        "cephfs": {
          "$ref": "#/definitions/io.k8s.api.core.v1.CephFSVolumeSource",
          "description": "CephFS represents a Ceph FS mount on the host that shares a pod's lifetime"
        },
        "cinder": {
          "$ref": "#/definitions/io.k8s.api.core.v1.CinderVolumeSource",
          "description": "Cinder represents a cinder volume attached and mounted on kubelets host machine More info: https://releases.k8s.io/HEAD/examples/mysql-cinder-pd/README.md"
        },
        "configMap": {
          "$ref": "#/definitions/io.k8s.api.core.v1.ConfigMapVolumeSource",
          "description": "ConfigMap represents a configMap that should populate this volume"
        },
        "csi": {
          "$ref": "#/definitions/io.k8s.api.core.v1.CSIVolumeSource",
          "description": "CSI (Container Storage Interface) represents storage that is handled by an external CSI driver (Alpha feature)."
        },
        "downwardAPI": {
          "$ref": "#/definitions/io.k8s.api.core.v1.DownwardAPIVolumeSource",
          "description": "DownwardAPI represents downward API about the pod that should populate this volume"
        },
        "emptyDir": {
          "$ref": "#/definitions/io.k8s.api.core.v1.EmptyDirVolumeSource",
          "description": "EmptyDir represents a temporary directory that shares a pod's lifetime. More info: https://kubernetes.io/docs/concepts/storage/volumes#emptydir"
        },
        "fc": {
          "$ref": "#/definitions/io.k8s.api.core.v1.FCVolumeSource",
          "description": "FC represents a Fibre Channel resource that is attached to a kubelet's host machine and then exposed to the pod."
        },
        "flexVolume": {
          "$ref": "#/definitions/io.k8s.api.core.v1.FlexVolumeSource",
          "description": "FlexVolume represents a generic volume resource that is provisioned/attached using an exec based plugin."
        },
        "flocker": {
          "$ref": "#/definitions/io.k8s.api.core.v1.FlockerVolumeSource",
          "description": "Flocker represents a Flocker volume attached to a kubelet's host machine. This depends on the Flocker control service being running"
        },
        "gcePersistentDisk": {
          "$ref": "#/definitions/io.k8s.api.core.v1.GCEPersistentDiskVolumeSource",
          "description": "GCEPersistentDisk represents a GCE Disk resource that is attached to a kubelet's host machine and then exposed to the pod. More info: https://kubernetes.io/docs/concepts/storage/volumes#gcepersistentdisk"
        },
        "gitRepo": {
          "$ref": "#/definitions/io.k8s.api.core.v1.GitRepoVolumeSource",
          "description": "GitRepo represents a git repository at a particular revision. DEPRECATED: GitRepo is deprecated. To provision a container with a git repo, mount an EmptyDir into an InitContainer that clones the repo using git, then mount the EmptyDir into the Pod's container."
        },
        "glusterfs": {
          "$ref": "#/definitions/io.k8s.api.core.v1.GlusterfsVolumeSource",
          "description": "Glusterfs represents a Glusterfs mount on the host that shares a pod's lifetime. More info: https://releases.k8s.io/HEAD/examples/volumes/glusterfs/README.md"
        },
        "hostPath": {
          "$ref": "#/definitions/io.k8s.api.core.v1.HostPathVolumeSource",
          "description": "HostPath represents a pre-existing file or directory on the host machine that is directly exposed to the container. This is generally used for system agents or other privileged things that are allowed to see the host machine. Most containers will NOT need this. More info: https://kubernetes.io/docs/concepts/storage/volumes#hostpath"
        },
        "iscsi": {
          "$ref": "#/definitions/io.k8s.api.core.v1.ISCSIVolumeSource",
          "description": "ISCSI represents an ISCSI Disk resource that is attached to a kubelet's host machine and then exposed to the pod. More info: https://releases.k8s.io/HEAD/examples/volumes/iscsi/README.md"
        },
        "name": {
          "description": "Volume's name. Must be a DNS_LABEL and unique within the pod. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names",
          "type": "string"
        },
        "nfs": {
          "$ref": "#/definitions/io.k8s.api.core.v1.NFSVolumeSource",
          "description": "NFS represents an NFS mount on the host that shares a pod's lifetime More info: https://kubernetes.io/docs/concepts/storage/volumes#nfs"
        },
        "persistentVolumeClaim": {
          "$ref": "#/definitions/io.k8s.api.core.v1.PersistentVolumeClaimVolumeSource",
          "description": "PersistentVolumeClaimVolumeSource represents a reference to a PersistentVolumeClaim in the same namespace. More info: https://kubernetes.io/docs/concepts/storage/persistent-volumes#persistentvolumeclaims"
        },
        "photonPersistentDisk": {
          "$ref": "#/definitions/io.k8s.api.core.v1.PhotonPersistentDiskVolumeSource",
          "description": "PhotonPersistentDisk represents a PhotonController persistent disk attached and mounted on kubelets host machine"
        },
        "portworxVolume": {
          "$ref": "#/definitions/io.k8s.api.core.v1.PortworxVolumeSource",
          "description": "PortworxVolume represents a portworx volume attached and mounted on kubelets host machine"
        },
        "projected": {
          "$ref": "#/definitions/io.k8s.api.core.v1.ProjectedVolumeSource",
          "description": "Items for all in one resources secrets, configmaps, and downward API"
        },
        "quobyte": {
          "$ref": "#/definitions/io.k8s.api.core.v1.QuobyteVolumeSource",
          "description": "Quobyte represents a Quobyte mount on the host that shares a pod's lifetime"
        },
        "rbd": {
          "$ref": "#/definitions/io.k8s.api.core.v1.RBDVolumeSource",
          "description": "RBD represents a Rados Block Device mount on the host that shares a pod's lifetime. More info: https://releases.k8s.io/HEAD/examples/volumes/rbd/README.md"
        },
        "scaleIO": {
          "$ref": "#/definitions/io.k8s.api.core.v1.ScaleIOVolumeSource",
          "description": "ScaleIO represents a ScaleIO persistent volume attached and mounted on Kubernetes nodes."
        },
        "secret": {
          "$ref": "#/definitions/io.k8s.api.core.v1.SecretVolumeSource",
          "description": "Secret represents a secret that should populate this volume. More info: https://kubernetes.io/docs/concepts/storage/volumes#secret"
        },
        "storageos": {
          "$ref": "#/definitions/io.k8s.api.core.v1.StorageOSVolumeSource",
          "description": "StorageOS represents a StorageOS volume attached and mounted on Kubernetes nodes."
        },
        "vsphereVolume": {
          "$ref": "#/definitions/io.k8s.api.core.v1.VsphereVirtualDiskVolumeSource",
          "description": "VsphereVolume represents a vSphere volume attached and mounted on kubelets host machine"
        }
      },
      "required": [
        "name"
      ],
      "type": "object"
    },

But only one of those keys can be set at the same time.

I think OpenAPI 3.0 actually allows encoding this correctly using https://swagger.io/docs/specification/data-models/inheritance-and-polymorphism/

but we will have to ask upstream to change the openAPI spec to properly denote that this is a sum type and not a product type. Then we could change our generator accordingly.

@arianvp
Copy link
Member

arianvp commented Aug 15, 2019

If Volume was defined like this, I think we could handle it:

definitions:
  io.k8s.api.core.v1.Volume:
    oneOf:
      - type: object
        properties:
          awsElasticBlockStore:
            $ref: #/definitions/io.k8s.api.core.v1.AWSElasticBlockStoreVolumeSource
      - type: object
        properties:
          azureDisk: 
            $ref: #/definitions/io.k8s.api.core.v1.AzureDiskVolumeSource
      - type: object
        properties:
          azureFile:
            $ref: #/definitions/io.k8s.api.core.v1.AzureFileVolumeSource
      - type: object
        properties:
          cephfs:
            $ref: #/definitions/io.k8s.api.core.v1.CephFSVolumeSource
      - type: object
        properties:
          cinder:
            $ref: #/definitions/io.k8s.api.core.v1.CinderVolumeSource
      - type: object
        properties:
          configMap:
            $ref: #/definitions/io.k8s.api.core.v1.ConfigMapVolumeSource
      - type: object
        properties:
          csi:
            $ref: #/definitions/io.k8s.api.core.v1.CSIVolumeSource
      - type: object
        properties:
          downwardAPI:
            $ref: #/definitions/io.k8s.api.core.v1.DownwardAPIVolumeSource
      - type: object
        properties:
          emptyDir:
            $ref: #/definitions/io.k8s.api.core.v1.EmptyDirVolumeSourcg

@arianvp
Copy link
Member

arianvp commented Aug 15, 2019

Upstream issue: kubernetes/kubernetes#51163 kubernetes/kubernetes#79472

(oneOf is only supported in OpenAPI 3, not in swagger2)

@arianvp
Copy link
Member

arianvp commented Aug 15, 2019

@ari-becker for now we could add a special case in the generator for Volumes as they're quite commonly used. There are probably other places in the Kubernetes API where we need to add special cases but lets do those on a case by case basis. What do you think?

@ari-becker
Copy link
Collaborator Author

@arianvp Sure, sounds workable to me

@arianvp
Copy link
Member

arianvp commented Aug 17, 2019

there's a PR that adds union types to k8s. doesnt seem like it will be merged soon but it's useful for us to cross-reference to see what types in the API are actually unions https://github.com/kubernetes/kubernetes/pull/77370/files

@arianvp arianvp changed the title Intentionally empty objects with 2.0 API? Support union types Sep 12, 2019
@ari-becker
Copy link
Collaborator Author

Closing this because:

a) the original issue is stale - support went back to omitting null rather than --omit-empty
b) the issue which @arianvp linked to, r.e. union support in Kubernetes, was closed as stale, and I don't see much reason to keep an issue on this open if upstream isn't building support?

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

No branches or pull requests

2 participants