I'm not sure when this crept in, but the {provider} path parameter isn't properly defined in most ERM endpoints and thus the swagger spec isn't valid as it stands. You can test with using he 'swagger-cli' not module as such: ``` cd api/v1/swagger swagger-cli validate swagger.yaml ```
Created attachment 147777 [details] [review] Bug 33145: Fix ERM swagger specs This patch adds the missing 'Provider name' parameter definition to the various ERM paths that were missing it. Test plan 1) Install swagger-cli using npm 2) cd api/v1/swagger 3) Run `swagger-cli validate swagger.yaml` 4) Confirm validation fails 5) Apply the patch here 6) Re-run the swagger-cli test above and confirm it now passes.
I must admit.. I thought we had a unit test to catch invalid specs generally.. so surprised this crept in.
Created attachment 147780 [details] [review] Bug 33145: Fix ERM swagger specs This patch adds the missing 'Provider name' parameter definition to the various ERM paths that were missing it. Test plan 1) Install swagger-cli using npm 2) cd api/v1/swagger 3) Run `swagger-cli validate swagger.yaml` 4) Confirm validation fails 5) Apply the patch here 6) Re-run the swagger-cli test above and confirm it now passes. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tomas: Please have a look here
I run this for validating, installing swagger-cli is failing for me in ktd: docker run --rm -v $PWD/api/v1/swagger:/swagger jeanberu/swagger-cli swagger-cli validate swagger/swagger.yaml
Created attachment 147782 [details] [review] Bug 33145: Fix ERM swagger specs This patch adds the missing 'Provider name' parameter definition to the various ERM paths that were missing it. Test plan 1) Run: docker run --rm -v $PWD/api/v1/swagger:/swagger \ jeanberu/swagger-cli \ swagger-cli validate swagger/swagger.yaml => FAIL: The spec is not valid! 2) Apply this patch 3) Repeat 1 => SUCCESS: Tests pass! Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Edit: tcohen edited the test plan because of the failure to install swagger-cli cleanly
Pushed to master for 23.05. Nice work everyone, thanks!
Don't we need a regression test?
Nice work, thanks everyone! Pushed to 22.11.x for the next release.
Not needed in 22.05.x