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

Provide a v2 API to publish events that conforms to cloud events specification #4687

Conversation

montaro
Copy link
Contributor

@montaro montaro commented Jun 28, 2019

See also #4389

@montaro montaro added the area/eventing Issues or PRs related to eventing label Jun 28, 2019
@montaro montaro added this to the Sprint_Evengers_26 milestone Jun 28, 2019
@montaro montaro self-assigned this Jun 28, 2019
@montaro montaro marked this pull request as ready for review June 29, 2019 12:33
@montaro montaro requested a review from a user June 29, 2019 12:33
@montaro montaro requested a review from sayanh as a code owner June 29, 2019 12:33
@montaro montaro added the WIP label Jul 1, 2019
Copy link
Contributor

@sayanh sayanh left a comment

Choose a reason for hiding this comment

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

Unit tests for v2 publish API should be added

@sayanh
Copy link
Contributor

sayanh commented Jul 3, 2019

Request:

# curl -i \
> -H "Content-Type: application/json" \
> -X POST http://event-bus-publish.kyma-system:8080/v2/events \
> -d '{"source": "sample-app", "specversion": "0.3", "event-type":
"order.received", "event-type-version": "v4", "event-time": "2018-1
1-02T22:08:41+00:00", "data": { "comeon": "hello" }}' 

HTTP/1.1 400 Bad Request
Content-Type: application/json
Date: Wed, 03 Jul 2019 12:45:26 GMT
Content-Length: 263

Response:

{"status":400,"type":"validation_violation","message":"We need all required fields complete to keep you moving.","moreInfo":"","details":[{"field":"type","type":"missing_field","message":"We need all required fields complete to keep you moving.","moreInfo":""}]}

The response doesn't say what is missing.

@@ -0,0 +1,77 @@
package v2
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this file? I find it similar to components/event-bus/api/publish/error.go.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm trying to consolidate the APIs files, so we can easily remove the v1 in the future

@marcobebway
Copy link
Contributor

marcobebway commented Jul 3, 2019

For the kyma/components/event-service, I get vendor is out of sync when running dep check.

@sayanh
Copy link
Contributor

sayanh commented Jul 3, 2019

Publishing from an application gives 404:

curl -i \
-H "Content-Type: application/json" --cert generated.pem -kL \
-X POST http://gateway.35.242.214.68.xip.io/v2/events \
-d '{"source": "sample-app", "specversion": "0.3", "type": "order.received", "eventtypeversion": "v4", "time": "2018-11-02T22:08:41+00:00", "data": { "comeon": "hello" }}'
HTTP/1.1 301 Moved Permanently
location: https://gateway.35.242.214.68.xip.io/v2/events
date: Wed, 03 Jul 2019 14:54:10 GMT
server: istio-envoy
content-length: 0

HTTP/2 404
date: Wed, 03 Jul 2019 14:54:11 GMT
server: istio-envoy

@montaro montaro added area/application-connector Issues or PRs related to application connectivity and removed WIP labels Jul 4, 2019
@montaro
Copy link
Contributor Author

montaro commented Jul 4, 2019

@sayanh This is working now after changing the application-connector

❯ echo '{"specversion": "0.3", "type" : "order.created", "eventtypeversion" : "v1", "time" : "2018-05-02T22:08:41+00:00", "data" : {"orderCode" : "1234"}}'|http POST https://gateway.kyma.local/sample-external-solution/v2/events --cert=generated.pem --verify=no
HTTP/1.1 200 OK
content-length: 80
content-type: application/json
date: Thu, 04 Jul 2019 09:55:15 GMT
server: istio-envoy
x-envoy-upstream-service-time: 1281

{
    "reason": "Message successfully published to the channel",
    "status": "published"
}

@montaro
Copy link
Contributor Author

montaro commented Jul 4, 2019

/test pre-master-kyma-components-application-connectivity-validator

Copy link
Contributor

@Szymongib Szymongib left a comment

Choose a reason for hiding this comment

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

Please fix the failing build.

@montaro montaro changed the title Provide a v2 api to publish events that conforms to cloud events specification Provide a v2 API to publish events that conforms to cloud events specification Jul 5, 2019
@sayanh
Copy link
Contributor

sayanh commented Jul 5, 2019

Following need to be done:

  • event-service tests are not working(more tests need to be there just like blackb_test.go)
  • clear segregation of v1 and v2 components should be done. At this moment directory structures are not consistent

@Szymongib
Copy link
Contributor

You might want to add some acceptance tests for the Event Service V2 as I do not think the existing tests cover this.

@montaro montaro force-pushed the provide-a-v2-api-to-publish-events-that-conforms-to-cloud-events-specification branch from 5ae8f44 to c8f9ba8 Compare July 10, 2019 10:45
@montaro montaro merged commit 3605918 into kyma-project:master Jul 10, 2019
@montaro montaro deleted the provide-a-v2-api-to-publish-events-that-conforms-to-cloud-events-specification branch December 20, 2019 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/application-connector Issues or PRs related to application connectivity area/eventing Issues or PRs related to eventing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants