Skip to content

Conversation

@rartych
Copy link
Contributor

@rartych rartych commented Jan 13, 2025

What type of PR is this?

  • correction

What this PR does / why we need it:

String pattern added to x-correlator scheme to improve security and inline with current recommendation for string parameters.
The proposed pattern covers recommended UUID strings and the length of 55 allows to use strings similar to traceparent from Trace Context.

CAMARA_common.yaml was updated accordingly.

Which issue(s) this PR fixes:

Fixes #352

Does this PR introduce a breaking change?

  • Yes
  • No

Special notes for reviewers:

The change should not be breaking for current implementations using UUID in x-correlator.

Changelog input

String pattern added to x-correlator scheme

Additional documentation

Copy link
Contributor

@PedroDiez PedroDiez left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@patrice-conil patrice-conil left a comment

Choose a reason for hiding this comment

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

LGTM

@rartych rartych merged commit b467cbf into camaraproject:main Jan 16, 2025
1 check passed
@hdamker
Copy link
Collaborator

hdamker commented Jan 16, 2025

Does this PR introduce a breaking change?

x Yes
No

I'm not sure that we should introduce this as a "breaking change" ... if we do so then all 1.0.0 API versions from Fall24 will go to automatically to 2.x.x in Spring25.

I formulated my proposal along the vendor opaque value in Trace Context, to avoid that we exclude any valid vendor specific correlator by this change and therefore don't need to declare it as breaking change. But maybe too late here ...

@rartych
Copy link
Contributor Author

rartych commented Jan 16, 2025

Formally it is a breaking change (modifying mandatory parameter).
API Design Guidelines recommend using UUID (if API Client implements "UUID-like" correlator then there is no breaking change) , however this recommendation is not included in API definitions - Possibly it can be added in x-correlator description field in CAMARA_common.yaml

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve definition of x-correlator header

5 participants