Skip to content

Conversation

lance
Copy link
Contributor

@lance lance commented Jul 3, 2019

Description of the change:

The generated YAML CRD created when running operator-sdk add api ...
includes an OpenAPI v3.0 validation section. E.g.

  validation:
    openAPIV3Schema:
      properties:
        apiVersion:
          description: 'APIVersion defines the versioned schema of this representation
            of an object. Servers should convert recognized schemas to the latest
            internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#resources'
          type: string
        kind:
          description: 'Kind is a string value representing the REST resource this
            object represents. Servers may infer this from the endpoint the client
            submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#types-kinds'
          type: string
        metadata:
          type: object
        spec:
          type: object
        status:
          type: object

The properties for a CRD should be specified here for validation.
In the case of the Memcached example, a size property should be
added to the generated CRD. This change documents that process in
the user guide.

Motivation for the change:

I got hung up on this when going through the tutorial. I thought I was missing a step or something wasn't right when examining the generated CRD files and following the user guide. Perhaps this will make it a little clearer for people just starting out.

The generated YAML CRD created when running `operator-sdk add api ...`
includes an OpenAPI v3.0 validation section. E.g.

```YAML
  validation:
    openAPIV3Schema:
      properties:
        apiVersion:
          description: 'APIVersion defines the versioned schema of this representation
            of an object. Servers should convert recognized schemas to the latest
            internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#resources'
          type: string
        kind:
          description: 'Kind is a string value representing the REST resource this
            object represents. Servers may infer this from the endpoint the client
            submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#types-kinds'
          type: string
        metadata:
          type: object
        spec:
          type: object
        status:
          type: object
```

The properties for a CRD should be specified here for validation.
In the case of the Memcached example, a `size` property should be
added to the generated CRD. This change documents that process in
the user guide.
@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jul 3, 2019
$ operator-sdk generate k8s
```

Update the Open API validation section in the CRD at `deploy/crds/cache_v1alpha1_memcached_crd.yaml` so that Kubernetes can validate the properties in a Memcached Custom Resource when it is created or updated.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should read something like:

To update the OpenAPI validation section in the CRD `deploy/crds/cache_v1alpha1_memcached_crd.yaml`, run the following command:

\```console
$ operator-sdk generate openapi
\```

This validation section allows Kubernetes to validate the properties in a Memcached Custom Resource when it is created or updated. An example of the generated YAML is as follows:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh! I didn't realize that generate openapi was available. Thanks for this. The change has been made.

@lilic
Copy link
Member

lilic commented Jul 5, 2019

/test e2e-aws-ansible

Copy link
Member

@lilic lilic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, once the suggestion that @estroz left is addressed this can be merged.

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 5, 2019
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jul 5, 2019
Copy link
Member

@estroz estroz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 5, 2019
@hasbro17
Copy link
Contributor

hasbro17 commented Jul 9, 2019

@estroz We don't have any of the upstream validation directive tags documented anywhere yet right?
Once we have that, then in the future I think this might be better as a separate doc that we point to from the user-guide rather than a section in here.

Co-Authored-By: Haseeb Tariq <hasbro17@gmail.com>
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jul 16, 2019
@estroz estroz requested a review from lilic July 16, 2019 20:00
Copy link
Member

@lilic lilic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 17, 2019
@lilic
Copy link
Member

lilic commented Jul 17, 2019

Thanks for this!

@lilic lilic merged commit f4dd3b1 into operator-framework:master Jul 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants