-
Notifications
You must be signed in to change notification settings - Fork 171
Validation for policy configurations #646
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
277976d to
7d4316d
Compare
7d4316d to
eb3b396
Compare
'ljsonschema' uses the draft-04 version of the JSON schema. 'const' was introduced later and so we can't use them. An enum of 1 element is mostly equivalent and a valid option for our use case.
eb3b396 to
b4d827c
Compare
|
There have been a few changes:
|
| --- request | ||
| GET /?user_key=value | ||
| --- error_code: 200 | ||
| --- must_die |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The must die signals nginx should quit, right? does it need the --- backend and upstream then? And the error_code ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. These are leftovers because at first, I was logging an error instead of exiting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
Now we have a way to check that we do not use invalid policy configs in tests.
We only want to use this option in tests.
2305484 to
8c2df39
Compare
This PR introduces a way to validate a policy's configuration against the JSON schema defined for its configuration.
This is what I used to detect the problems in our tests fixed in previous PRs: #641 , #644 , #645
We need to define and agree on a few things before this becomes mergeable. That's why I tagged it as WIP:
rapidjsonto perform the validation, and it needs cmake. This means that it should be installed on the base image.rapidjsonuses the draft04 version of the JSON schema. I needed to replace theconsts in our schemas so it works correctly asconstwas introduced in newer versions.