camaraproject / Commonalities

Repository to describe, develop, document and test the common guidelines and assets for CAMARA APIs
Apache License 2.0
9 stars 24 forks source link

x-correlator_guideline_format_convention #194

Closed PedroDiez closed 1 month ago

PedroDiez commented 2 months ago

What type of PR is this?

What this PR does / why we need it:

This PR covers the x-correlator naming guideline adoption. Also given format to CAMARA_common.yaml (2-space indentation)

Documentation impacted (some doubts commented within PR):

Which issue(s) this PR fixes:

Fixes #191

Special notes for reviewers:

Some notes: 1) To be merged after #193 2) Should we take advantage of this PR and align error exceptions for POST /notifications? To align with #189. Commented within PR 3) Removed Exception 415 4) Refactor bearer format to bearerFormat: "{$request.body#/sinkCredential.credentialType}"

Changelog input

 release-note

Additional documentation

N/A

PedroDiez commented 2 months ago

cc @jlurien

PedroDiez commented 1 month ago

ready for review cc @shilpa-padgaonkar @rartych @patrice-conil @bigludo7

I have updated bearer format to: "{$request.body#/sinkCredential.credentialType}"

As https://swagger.io/docs/specification/authentication/bearer-authentication/ indicates, is an optional property and a hint for the client. ( # optional, arbitrary value for documentation purposes), so semantically speaking accordingly to CloudEvents model is the concept of credentialType and we are restricting to be only ACCESSTOKEN (to me makes more sense but to have aligment/consensus on that)

If you think it is better to indicate: "{$request.body#/sinkCredential.accessToken}" just because we only allow ACCESSTOKEN so far and the format is "accessToken" (i.e. a string) it is also fine to me.

Just to be transparent about this.

shilpa-padgaonkar commented 1 month ago

@patrice-conil or @bigludo7 : Could you kindly review and approve the PR if you find no issues so that we can proceed to merge?

PedroDiez commented 1 month ago

If no other comments I will be merging on Monday next week