Stage the place for openapi sports for new versions of the IMC #2588

matzew · 2020-02-17T16:40:32Z

Proposed Changes

Use versioned OpenAPI for CRDs in eventing for resources that are being promoted.

like #2554 did for "core" APIs

Signed-off-by: Matthias Wessendorf <mwessend@redhat.com>

knative-prow-robot · 2020-02-17T16:40:56Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: matzew

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [matzew]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

matzew · 2020-02-17T16:40:57Z

config/channels/in-memory-channel/300-in-memory-channel.yaml

-                      replyURI:
-                        type: string
-                        minLength: 1
-                      deliveryURI:


so... deliveryURI seems not used ...

yes it's called delivery now

matzew · 2020-02-17T16:42:32Z

/cc @dprotaso

matzew · 2020-02-17T17:57:51Z

/test pull-knative-eventing-integration-tests

matzew · 2020-02-17T18:23:27Z

/retest

matzew · 2020-02-18T07:09:11Z

/retest

n3wscott · 2020-02-18T14:45:44Z

if you install this, does the CRD report back a happy status?

matzew · 2020-02-18T15:01:45Z

marking as WIP, since I noticed a glitch here ...

matzew · 2020-02-18T16:34:18Z

@lionelvillard I did a change on the delivery (was deliveryURI before)

I think... according to this we are structural

and getting the yaml is giving me:

apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"apiextensions.k8s.io/v1beta1","kind":"CustomResourceDefinition","metadata":{"annotations":{},"labels":{"duck.knative.dev/addressable":"true","eventing.knative.dev/release":"devel","knative.dev/crd-install":"true","messaging.knative.dev/subscribable":"true"},"name":"inmemorychannels.messaging.knative.dev"},"spec":{"additionalPrinterColumns":[{"JSONPath":".status.conditions[?(@.type==\"Ready\")].status","name":"Ready","type":"string"},{"JSONPath":".status.conditions[?(@.type==\"Ready\")].reason","name":"Reason","type":"string"},{"JSONPath":".status.address.url","name":"URL","type":"string"},{"JSONPath":".metadata.creationTimestamp","name":"Age","type":"date"}],"group":"messaging.knative.dev","names":{"categories":["all","knative","messaging","channel"],"kind":"InMemoryChannel","plural":"inmemorychannels","shortNames":["imc"],"singular":"inmemorychannel"},"scope":"Namespaced","subresources":{"status":{}},"versions":[{"name":"v1alpha1","schema":{"openAPIV3Schema":{"properties":{"spec":{"properties":{"subscribable":{"properties":{"subscribers":{"description":"Events received on the channel are forwarded to its subscribers.","items":{"properties":{"delivery":{"description":"Channel delivery options. More information: https://knative.dev/docs/eventing/event-delivery.","type":"object"},"ref":{"description":"a reference to a Kubernetes object from which to retrieve the target URI.","properties":{"apiVersion":{"type":"string"},"kind":{"type":"string"},"name":{"minLength":1,"type":"string"},"namespace":{"minLength":1,"type":"string"},"uid":{"minLength":1,"type":"string"}},"required":["namespace","name","uid"],"type":"object"},"replyURI":{"description":"Endpoint for the reply.","minLength":1,"type":"string"},"subscriberURI":{"description":"Endpoint for the subscriber.","minLength":1,"type":"string"},"uid":{"description":"Used to understand the origin of the subscriber.","minLength":1,"type":"string"}},"required":["uid"],"type":"object"},"type":"array"}},"type":"object"}},"type":"object"}},"type":"object"}},"served":true,"storage":true},{"name":"v1beta1","served":false,"storage":false}]}}
  creationTimestamp: "2020-02-18T16:29:20Z"
  generation: 1
  labels:
    duck.knative.dev/addressable: "true"
    eventing.knative.dev/release: devel
    knative.dev/crd-install: "true"
    messaging.knative.dev/subscribable: "true"
  name: inmemorychannels.messaging.knative.dev
  resourceVersion: "14499"
  selfLink: /apis/apiextensions.k8s.io/v1/customresourcedefinitions/inmemorychannels.messaging.knative.dev
  uid: 3f34cc43-d598-40e2-9ef7-9baaf2b16456
spec:
  conversion:
    strategy: None
  group: messaging.knative.dev
  names:
    categories:
    - all
    - knative
    - messaging
    - channel
    kind: InMemoryChannel
    listKind: InMemoryChannelList
    plural: inmemorychannels
    shortNames:
    - imc
    singular: inmemorychannel
  preserveUnknownFields: true
  scope: Namespaced
  versions:
  - additionalPrinterColumns:
    - jsonPath: .status.conditions[?(@.type=="Ready")].status
      name: Ready
      type: string
    - jsonPath: .status.conditions[?(@.type=="Ready")].reason
      name: Reason
      type: string
    - jsonPath: .status.address.url
      name: URL
      type: string
    - jsonPath: .metadata.creationTimestamp
      name: Age
      type: date
    name: v1alpha1
    schema:
      openAPIV3Schema:
        properties:
          spec:
            properties:
              subscribable:
                properties:
                  subscribers:
                    description: Events received on the channel are forwarded to its
                      subscribers.
                    items:
                      properties:
                        delivery:
                          description: 'Channel delivery options. More information:
                            https://knative.dev/docs/eventing/event-delivery.'
                          type: object
                        ref:
                          description: a reference to a Kubernetes object from which
                            to retrieve the target URI.
                          properties:
                            apiVersion:
                              type: string
                            kind:
                              type: string
                            name:
                              minLength: 1
                              type: string
                            namespace:
                              minLength: 1
                              type: string
                            uid:
                              minLength: 1
                              type: string
                          required:
                          - namespace
                          - name
                          - uid
                          type: object
                        replyURI:
                          description: Endpoint for the reply.
                          minLength: 1
                          type: string
                        subscriberURI:
                          description: Endpoint for the subscriber.
                          minLength: 1
                          type: string
                        uid:
                          description: Used to understand the origin of the subscriber.
                          minLength: 1
                          type: string
                      required:
                      - uid
                      type: object
                    type: array
                type: object
            type: object
        type: object
    served: true
    storage: true
    subresources:
      status: {}
  - additionalPrinterColumns:
    - jsonPath: .status.conditions[?(@.type=="Ready")].status
      name: Ready
      type: string
    - jsonPath: .status.conditions[?(@.type=="Ready")].reason
      name: Reason
      type: string
    - jsonPath: .status.address.url
      name: URL
      type: string
    - jsonPath: .metadata.creationTimestamp
      name: Age
      type: date
    name: v1beta1
    served: false
    storage: false
    subresources:
      status: {}
status:
  acceptedNames:
    categories:
    - all
    - knative
    - messaging
    - channel
    kind: InMemoryChannel
    listKind: InMemoryChannelList
    plural: inmemorychannels
    shortNames:
    - imc
    singular: inmemorychannel
  conditions:
  - lastTransitionTime: "2020-02-18T16:29:20Z"
    message: no conflicts found
    reason: NoConflicts
    status: "True"
    type: NamesAccepted
  - lastTransitionTime: "2020-02-18T16:29:20Z"
    message: the initial names have been accepted
    reason: InitialNamesAccepted
    status: "True"
    type: Established
  storedVersions:
  - v1alpha1

slinkydeveloper

Couple of nits for the validation

slinkydeveloper · 2020-02-18T17:00:51Z

config/channels/in-memory-channel/300-in-memory-channel.yaml

+                          type: string
+                          description: "Used to understand the origin of the subscriber."
+                          minLength: 1
+                        subscriberURI:


Both this and the one below should use the format uri or uri-ref

slinkydeveloper · 2020-02-18T17:02:25Z

config/channels/in-memory-channel/300-in-memory-channel.yaml

+                            - name
+                            - uid
+                          properties:
+                            apiVersion:


The apiVersion subschema should be a const schema, but since OpenAPI 3.0 doesn't contain it, maybe you should add the property enum: ["v1alpha1"]

slinkydeveloper · 2020-02-18T17:02:35Z

config/channels/in-memory-channel/300-in-memory-channel.yaml

+                          properties:
+                            apiVersion:
+                              type: string
+                            kind:


Same as above

vaikas · 2020-02-18T19:02:03Z

Given the issues that actually come up with the conversion and the validation once the conversion webhook comes into play, I'm not sure that staging these adds much value. I'm fine with this, but as witnessed by various issues in the (status needs to be filled in, etc.):
#2577
I'd much rather we move the schemas at the same time we add the conversion.

vaikas · 2020-02-18T19:03:31Z

config/channels/in-memory-channel/300-in-memory-channel.yaml

+                          minLength: 1
+                        delivery:
+                          description: "Channel delivery options. More information: https://knative.dev/docs/eventing/event-delivery."
+                          type: object


you need status here, see #2577

@vaikas I will follow up - just seeing this

matzew · 2020-02-19T16:09:03Z

@vaikas I will give the conversion a shot

vaikas · 2020-02-21T02:27:50Z

As discussed offline:
#2621

matzew · 2020-02-21T12:28:33Z

/close

knative-prow-robot · 2020-02-21T12:28:36Z

@matzew: Closed this PR.

In response to this:

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

stage the place for openapi sports for new versions of the IMC

dee7d1a

Signed-off-by: Matthias Wessendorf <mwessend@redhat.com>

matzew requested review from lionelvillard and n3wscott February 17, 2020 16:40

knative-prow-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 17, 2020

googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Feb 17, 2020

knative-prow-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 17, 2020

knative-prow-robot requested a review from evankanderson February 17, 2020 16:40

knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 17, 2020

matzew commented Feb 17, 2020

View reviewed changes

knative-prow-robot requested a review from dprotaso February 17, 2020 16:42

matzew changed the title ~~WIP: stage the place for openapi sports for new versions of the IMC~~ Stage the place for openapi sports for new versions of the IMC Feb 18, 2020

knative-prow-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 18, 2020

matzew changed the title ~~Stage the place for openapi sports for new versions of the IMC~~ WIP: Stage the place for openapi sports for new versions of the IMC Feb 18, 2020

knative-prow-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 18, 2020

updating on delivery field

01b6cf5

matzew changed the title ~~WIP: Stage the place for openapi sports for new versions of the IMC~~ Stage the place for openapi sports for new versions of the IMC Feb 18, 2020

knative-prow-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 18, 2020

slinkydeveloper suggested changes Feb 18, 2020

View reviewed changes

vaikas reviewed Feb 18, 2020

View reviewed changes

knative-prow-robot closed this Feb 21, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Stage the place for openapi sports for new versions of the IMC #2588

Stage the place for openapi sports for new versions of the IMC #2588

matzew commented Feb 17, 2020

knative-prow-robot commented Feb 17, 2020

matzew Feb 17, 2020

lionelvillard Feb 18, 2020

matzew commented Feb 17, 2020

matzew commented Feb 17, 2020

matzew commented Feb 17, 2020

matzew commented Feb 18, 2020

n3wscott commented Feb 18, 2020

matzew commented Feb 18, 2020

matzew commented Feb 18, 2020

slinkydeveloper left a comment

slinkydeveloper Feb 18, 2020

slinkydeveloper Feb 18, 2020

slinkydeveloper Feb 18, 2020

vaikas commented Feb 18, 2020

vaikas Feb 18, 2020

matzew Feb 19, 2020

matzew commented Feb 19, 2020

vaikas commented Feb 21, 2020

matzew commented Feb 21, 2020

knative-prow-robot commented Feb 21, 2020

Stage the place for openapi sports for new versions of the IMC #2588

Stage the place for openapi sports for new versions of the IMC #2588

Conversation

matzew commented Feb 17, 2020

Proposed Changes

knative-prow-robot commented Feb 17, 2020

matzew Feb 17, 2020

Choose a reason for hiding this comment

lionelvillard Feb 18, 2020

Choose a reason for hiding this comment

matzew commented Feb 17, 2020

matzew commented Feb 17, 2020

matzew commented Feb 17, 2020

matzew commented Feb 18, 2020

n3wscott commented Feb 18, 2020

matzew commented Feb 18, 2020

matzew commented Feb 18, 2020

slinkydeveloper left a comment

Choose a reason for hiding this comment

slinkydeveloper Feb 18, 2020

Choose a reason for hiding this comment

slinkydeveloper Feb 18, 2020

Choose a reason for hiding this comment

slinkydeveloper Feb 18, 2020

Choose a reason for hiding this comment

vaikas commented Feb 18, 2020

vaikas Feb 18, 2020

Choose a reason for hiding this comment

matzew Feb 19, 2020

Choose a reason for hiding this comment

matzew commented Feb 19, 2020

vaikas commented Feb 21, 2020

matzew commented Feb 21, 2020

knative-prow-robot commented Feb 21, 2020