google / knative-gcp

GCP event implementations to use with Knative Eventing.
https://github.com/knative/eventing
Apache License 2.0
159 stars 75 forks source link

Broker ignores some Binary Mode Cloud Events reponses #2097

Closed Harwayne closed 3 years ago

Harwayne commented 3 years ago

Not a real issue, I skipped the ! in the condition. Original issue below:


https://github.com/google/knative-gcp/blob/9e31468bc734d17641bcb0cff55ed8ac7f47d783/pkg/broker/handler/processors/deliver/processor.go#L183-L185

I don't think this is accurate. I believe in binary mode the Content-Type can be just about anything, see https://github.com/cloudevents/spec/blob/v1.0.1/http-protocol-binding.md#311-http-content-type.

In fact the first example in https://github.com/cloudevents/spec/blob/v1.0.1/http-protocol-binding.md#314-examples is:

POST /someresource HTTP/1.1
Host: webhook.example.com
ce-specversion: 1.0
ce-type: com.example.someevent
ce-time: 2018-04-05T03:56:24Z
ce-id: 1234-1234-1234
ce-source: /mycontext/subcontext
    .... further attributes ...
Content-Type: application/json; charset=utf-8
Content-Length: nnnn

{
    ... application data ...
}

Note that Content-Type is application/json; charset=utf-8, which is not prefixed with application/cloudevents.

_Originally posted by @Harwayne in https://github.com/google/knative-gcp/pull/2068#discussion_r564770148_

Harwayne commented 3 years ago

/close

I foolishly overlooked the !, this looks correct.

knative-prow-robot commented 3 years ago

@Harwayne: Closing this issue.

In response to [this](https://github.com/google/knative-gcp/issues/2097#issuecomment-767773045): >/close > >I foolishly overlooked the `!`, this looks correct. Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.