-
Notifications
You must be signed in to change notification settings - Fork 170
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
Add JSON schemas for the policy configurations #522
Conversation
Apart from the code itself, each policy will, at least, have a schema for its configuration. The idea is to have a separate directory for each policy that will contain the policy itself and the schema.
This allows to require 'apicast.policy.somepolicy' instead of 'apicast.policy.somepolicy.policy'.
f723d2c
to
dde322b
Compare
@@ -42,7 +42,8 @@ http { | |||
log_format time '[$time_local] $host:$server_port $remote_addr:$remote_port "$request" $status $body_bytes_sent ($request_time) $post_action_impact'; | |||
access_log off; | |||
|
|||
lua_package_path ";;{{prefix}}/?.lua;{{prefix}}/src/?.lua"; | |||
# src/?/policy.lua allows us to require apicast.policy.apolicy |
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.
I think it is fine now. 👍 Later we can solve it with custom loader.
"type": "array", | ||
"items": { | ||
"type": "string", | ||
"enum": ["GET", "HEAD", "POST", "PUT", "DELETE", "PATCH"] |
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.
To be future-proff we definitely should include OPTIONS
and when we are at it I think we can include TRACE
and CONNECT
too. https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods
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.
Done.
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.
👍 good start
We will have to add more descriptions to those configuration options, but that we can do as we will have some UI prototype.
@ddcesare @thomasmaas please check the JSON schema so you know how they'll look like.
I think we should address #522 (comment) before merging and add at least OPTIONS HTTP method.
👍
dde322b
to
ffec937
Compare
This PR adds JSON schemas for the configurations of the policies that have one. This schemas will be used by the UI to know which parameters it needs to ask the user when configuring a policy.
This PR also changes the organization a bit. Now every policy has its own directory with that contains the policy itself and its schema.
Also, in order to be able to require
apicast.policy.a_policy
instead ofapicast.policy.a_policy.policy
, I needed to change lua paths in several places. Ideas on how to make this simpler welcome :)