Add test coverage and use Problem Details in ratsd core API #29
Conversation
Signed-off-by: Ian Chin Wang <[email protected]>
Use RFC 7807 for HTTP problem details until more fields defined in RFC 9457 become necessary in ratsd. Signed-off-by: Ian Chin Wang <[email protected]>
Signed-off-by: Ian Chin Wang <[email protected]>
cowbon
requested review from
thomas-fossati,
deeglaze,
iolivergithub and
jraman567
as code owners
cowbon
force-pushed
the
cowbon-patch-1
branch
from
b2cce1e to
e09e26e
Compare
| Detail: errMsg, | ||
| Status: http.StatusBadRequest, | ||
| } | ||
| s.reportProblem(w, p) | ||
| return | ||
| } | ||
|
|
||
| respCt := fmt.Sprintf(`application/eat+jwt; eat_profile=%q`, TagGithubCom2024Veraisonratsd) | ||
| if *(param.Accept) != respCt { |
There was a problem hiding this comment.
Accept is not a mandatory header. If the client didn't supply it in the request, I think this will blow up :-)
There was a problem hiding this comment.
On a related subject: why is Accept treated specially compared to Content-Type?
There was a problem hiding this comment.
It seems OpenAPI generator handles empty Accept as */*, thus the header always receives a value. But I agree it's safer to enforce a check first. Regarding why Accept is treated specially compared to Content-Type, I'm not sure if we should also return 406 Not Acceptable when the Content-Type in the requests does not match what's expected by ratsd. And if so, what's the usage of 400 Bad Request especially when you define it explicitly in OpenAPI spec docs/api/ratsd.yaml?
Fix #22 with RFC 7807 to align with how Veraison/Services handles HTTP client errors.