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

using oneOf prevents doing any validation of the given parameter #97

Closed
doy-materialize opened this issue Aug 11, 2022 · 2 comments · Fixed by #143
Closed

using oneOf prevents doing any validation of the given parameter #97

doy-materialize opened this issue Aug 11, 2022 · 2 comments · Fixed by #143
Assignees
Labels
kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed

Comments

@doy-materialize
Copy link

What happened?

i'm trying to use crd2pulumi to generate types for a crd which uses oneOf to express that two of its properties are mutually exclusive, but one of them is required. when i try to generate the pulumi code for this crd, instead of generating property validation, this object is just given a type of Any.

Steps to reproduce

you can see this with a small extension to the crontab example:

apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
  # name must match the spec fields below, and be in the form: <plural>.<group>
  name: crontabs.stable.example.com
spec:
  # group name to use for REST API: /apis/<group>/<version>
  group: stable.example.com
  # list of versions supported by this CustomResourceDefinition
  versions:
    - name: v1
      # Each version can be enabled/disabled by Served flag.
      served: true
      # One and only one version must be marked as the storage version.
      storage: true
      schema:
        openAPIV3Schema:
          type: object
          properties:
            spec:
              type: object
              oneOf:
              - properties:
                  image: {}
                required:
                - image
              - properties:
                  replicas: {}
                required:
                - replicas
              properties:
                cronSpec:
                  type: string
                image:
                  type: string
                replicas:
                  type: integer
  # either Namespaced or Cluster
  scope: Namespaced
  names:
    # plural name to be used in the URL: /apis/<group>/<version>/<plural>
    plural: crontabs
    # singular name to be used as an alias on the CLI and for display
    singular: crontab
    # kind is normally the CamelCased singular type. Your resource manifests use this.
    kind: CronTab
    # shortNames allow shorter string to match your resource on the CLI
    shortNames:
    - ct

Expected Behavior

while i understand that translating things like oneOf constraints to language type systems isn't necessarily possible, i would still expect the normal property validation to be happening (like, probably at baseline i would expect the same results either with or without the oneOf constraint).

Actual Behavior

this generates a python type signature that looks like this:

    def __init__(__self__, *,
                 api_version: Optional[pulumi.Input[str]] = None,
                 kind: Optional[pulumi.Input[str]] = None,
                 metadata: Optional[pulumi.Input['_meta.v1.ObjectMetaArgs']] = None,
                 spec: Optional[Any] = None):

Versions used

crd2pulumi version 1.2.2

Additional context

this appears to happen because crd2pulumi expects that if oneOf is present, just generating types for the spec given in the oneOf configuration is sufficient (

oneOf, foundOneOf, _ := unstruct.NestedMapSlice(schema, "oneOf")
if foundOneOf {
oneOfTypeSpecs := make([]pschema.TypeSpec, 0, len(oneOf))
for i, oneOfSchema := range oneOf {
oneOfTypeSpec := GetTypeSpec(oneOfSchema, name+"OneOf"+strconv.Itoa(i), types)
if isAnyType(oneOfTypeSpec) {
return anyTypeSpec
}
oneOfTypeSpecs = append(oneOfTypeSpecs, oneOfTypeSpec)
}
return pschema.TypeSpec{
OneOf: oneOfTypeSpecs,
}
}
), but this isn't correct based on the kubernetes documentation (https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#specifying-a-structural-schema) - allOf/oneOf/anyOf are required to contain a subset of the fields given by properties, and the definition of those fields is not allowed to specify type (since type will be specified by the definition under properties), so crd2pulumi's current schema parser will always just map them to Any. i have only tested this with oneOf and using the python generator, but looking at the code i assume this is a problem with any language, and with any of those three constraints.

this seems related to #46 but a bit more general.

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

@doy-materialize doy-materialize added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Aug 11, 2022
@lblackstone lblackstone added kind/bug Some behavior is incorrect or out of spec and removed kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Aug 16, 2022
@mattolenik mattolenik added this to the 0.78 milestone Sep 6, 2022
@lukehoban lukehoban removed this from the 0.78 milestone Oct 6, 2022
@cleverguy25
Copy link

@pulumi-bot
Copy link

This issue has been addressed in PR #143 and shipped in release v1.5.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants