You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "pottekkat (via GitHub)" <gi...@apache.org> on 2023/10/16 06:20:38 UTC
[PR] fix: add missing properties to schema [apisix]
pottekkat opened a new pull request, #10339:
URL: https://github.com/apache/apisix/pull/10339
### Description
<!-- Please include a summary of the change and which issue is fixed. -->
<!-- Please also include relevant motivation and context. -->
**This is a draft PR.**
Adds missing properties to the schema so that it can be used for validating configuration files more accurately.
If this is okay, I will add other properties to the schema in this same PR before merging it.
### Checklist
- [ ] I have explained the need for this PR and the problem it solves
- [ ] I have explained the changes or the new features added to this PR
- [ ] I have added tests corresponding to this change
- [ ] I have updated the documentation to reflect this change
- [ ] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)
<!--
Note
1. Mark the PR as draft until it's ready to be reviewed.
2. Always add/update tests for any changes unless you have a good reason.
3. Always update the documentation to reflect the changes made in the PR.
4. Make a new commit to resolve conversations instead of `push -f`.
5. To resolve merge conflicts, merge master instead of rebasing.
6. Use "request review" to notify the reviewer after making changes.
7. Only a reviewer can mark a conversation as resolved.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: add missing properties to schema [apisix]
Posted by "pottekkat (via GitHub)" <gi...@apache.org>.
pottekkat commented on PR #10339:
URL: https://github.com/apache/apisix/pull/10339#issuecomment-1812435391
All checks pass with these changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: add missing properties to schema [apisix]
Posted by "pottekkat (via GitHub)" <gi...@apache.org>.
pottekkat commented on PR #10339:
URL: https://github.com/apache/apisix/pull/10339#issuecomment-1766719502
@Revolyssup Should we add a case where it can be either a string or a number?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: add missing properties to schema [apisix]
Posted by "pottekkat (via GitHub)" <gi...@apache.org>.
pottekkat commented on PR #10339:
URL: https://github.com/apache/apisix/pull/10339#issuecomment-1766234765
I'm unsure why the tests are failing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: add missing properties to schema [apisix]
Posted by "pottekkat (via GitHub)" <gi...@apache.org>.
pottekkat commented on PR #10339:
URL: https://github.com/apache/apisix/pull/10339#issuecomment-1812566371
It seems like the issue might have been in the etcd schema and with merging the config and deployment schemas. Checking what the issue is incrementally.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: add missing properties to schema [apisix]
Posted by "Revolyssup (via GitHub)" <gi...@apache.org>.
Revolyssup commented on code in PR #10339:
URL: https://github.com/apache/apisix/pull/10339#discussion_r1369021552
##########
apisix/cli/schema.lua:
##########
@@ -302,110 +683,87 @@ local config_schema = {
type = "integer"
},
http_request_phase = {
- enum = {"access", "rewrite"},
+ enum = { "access", "rewrite" },
default = "access",
},
},
- required = {"name", "file", "priority"}
+ required = { "name", "file", "priority" }
}
}
}
},
+ plugin_attr = {
+ type = "object"
+ },
deployment = {
type = "object",
properties = {
role = {
- enum = {"traditional", "control_plane", "data_plane", "standalone"},
+ enum = { "traditional", "control_plane", "data_plane", "standalone" },
default = "traditional"
- }
+ },
},
- },
- },
- required = {"apisix", "deployment"},
-}
-
-local admin_schema = {
- type = "object",
- properties = {
- admin_key = {
- type = "array",
- properties = {
- items = {
- properties = {
- name = {type = "string"},
- key = {type = "string"},
- role = {type = "string"},
+ dependencies = {
+ role = {
+ oneOf = {
+ {
Review Comment:
@pottekkat Will you make sure that this dependency schema fits right with this[1] yaml snippet here?
1. https://github.com/apache/apisix/blob/88406dceab26ed7f25b663088b0b5e760bd45a38/t/cli/test_validate_config.sh#L210C1-L210C1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: add missing properties to schema [apisix]
Posted by "Revolyssup (via GitHub)" <gi...@apache.org>.
Revolyssup commented on code in PR #10339:
URL: https://github.com/apache/apisix/pull/10339#discussion_r1362364318
##########
apisix/cli/schema.lua:
##########
@@ -246,18 +405,225 @@ local config_schema = {
},
}
},
+ enable_control = {
+ type = "boolean",
+ },
+ control = {
+ type = "object",
+ properties = {
+ ip = {
+ type = "string",
+ },
+ port = {
+ type = "integer",
+ minimum = 1,
+ maximum = 65535
+ },
+ }
+ },
+ data_encryption = {
+ type = "object",
+ properties = {
+ enable = {
+ type = "boolean",
+ },
+ keyring = {
+ type = "array",
+ items = {
+ type = "string",
+ }
+ },
+ }
+ },
nginx_config = {
type = "object",
properties = {
+ user = {
+ type = "string",
+ },
+ error_log = {
+ type = "string",
+ },
+ error_log_level = {
+ type = "string",
+ enum = { "info", "debug", "notice", "warn", "error", "crit", "alert", "emerg" }
+ },
+ worker_processes = {
+ anyOf = {
+ {
+ type = "integer",
+ minimum = 1,
+ },
+ {
+ type = "string"
+ },
+ {
+ type = "string",
+ enum = { "auto" }
+ }
+ }
+ },
+ enable_cpu_affinity = {
+ type = "boolean",
+ },
+ worker_rlimit_nofile = {
+ type = "integer",
+ },
+ worker_shutdown_timeout = {
+ type = "string",
+ },
+ max_pending_timers = {
+ type = "integer",
+ },
+ max_running_timers = {
+ type = "integer",
+ },
+ event = {
+ type = "object",
+ properties = {
+ worker_connections = {
+ type = "integer"
+ },
+ }
+ },
envs = {
type = "array",
minItems = 1,
items = {
type = "string",
}
+ },
+ meta = {
+ type = "object",
+ properties = {
+ lua_shared_dict = {
+ type = "object",
+ },
+ }
+ },
+ stream = {
+ type = "object",
+ properties = {
+ enable_access_log = {
+ type = "boolean",
+ },
+ access_log = {
+ type = "string",
+ },
+ access_log_format = {
+ type = "string",
+ },
+ access_log_format_escape = {
+ type = "string",
+ },
+ lua_shared_dict = {
+ type = "object",
+ },
+ }
+ },
+ main_configuration_snippet = {
+ type = "string"
+ },
+ http_configuration_snippet = {
+ type = "string"
+ },
+ http_server_configuration_snippet = {
+ type = "string"
+ },
+ http_server_location_configuration_snippet = {
+ type = "string"
+ },
+ http_admin_configuration_snippet = {
+ type = "string"
+ },
+ http_end_configuration_snippet = {
+ type = "string"
+ },
+ stream_configuration_snippet = {
+ type = "string"
+ },
+ http = {
+ type = "object",
+ properties = {
+ enable_access_log = {
+ type = "boolean"
+ },
+ access_log = {
+ type = "string"
+ },
+ access_log_buffer = {
+ type = "integer"
+ },
+ access_log_format = {
+ type = "string"
+ },
+ access_log_format_escape = {
+ type = "string"
+ },
+ keepalive_timeout = {
+ type = "string"
+ },
+ client_header_timeout = {
+ type = "string"
+ },
+ client_body_timeout = {
+ type = "string"
+ },
+ client_max_body_size = {
+ type = "number"
Review Comment:
```suggestion
type = "string"
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: add missing properties to schema [apisix]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on PR #10339:
URL: https://github.com/apache/apisix/pull/10339#issuecomment-1818355434
@Revolyssup please help to check
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: add missing properties to schema [apisix]
Posted by "pottekkat (via GitHub)" <gi...@apache.org>.
pottekkat commented on PR #10339:
URL: https://github.com/apache/apisix/pull/10339#issuecomment-1812080615
I reverted to the same changes as in the last commit where the tests were passing in this commit: https://github.com/apache/apisix/pull/10339/commits/bf8bdb87649dc18cabd85624ce301d83991e65a2
But now, the tests fail.
I'm now trying by removing all changes and checking if the tests pass now and build from there.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: add missing properties to schema [apisix]
Posted by "pottekkat (via GitHub)" <gi...@apache.org>.
pottekkat commented on code in PR #10339:
URL: https://github.com/apache/apisix/pull/10339#discussion_r1362365510
##########
apisix/cli/schema.lua:
##########
@@ -246,18 +405,225 @@ local config_schema = {
},
}
},
+ enable_control = {
+ type = "boolean",
+ },
+ control = {
+ type = "object",
+ properties = {
+ ip = {
+ type = "string",
+ },
+ port = {
+ type = "integer",
+ minimum = 1,
+ maximum = 65535
+ },
+ }
+ },
+ data_encryption = {
+ type = "object",
+ properties = {
+ enable = {
+ type = "boolean",
+ },
+ keyring = {
+ type = "array",
+ items = {
+ type = "string",
+ }
+ },
+ }
+ },
nginx_config = {
type = "object",
properties = {
+ user = {
+ type = "string",
+ },
+ error_log = {
+ type = "string",
+ },
+ error_log_level = {
+ type = "string",
+ enum = { "info", "debug", "notice", "warn", "error", "crit", "alert", "emerg" }
+ },
+ worker_processes = {
+ anyOf = {
+ {
+ type = "integer",
+ minimum = 1,
+ },
+ {
+ type = "string"
+ },
+ {
+ type = "string",
+ enum = { "auto" }
+ }
+ }
+ },
+ enable_cpu_affinity = {
+ type = "boolean",
+ },
+ worker_rlimit_nofile = {
+ type = "integer",
+ },
+ worker_shutdown_timeout = {
+ type = "string",
+ },
+ max_pending_timers = {
+ type = "integer",
+ },
+ max_running_timers = {
+ type = "integer",
+ },
+ event = {
+ type = "object",
+ properties = {
+ worker_connections = {
+ type = "integer"
+ },
+ }
+ },
envs = {
type = "array",
minItems = 1,
items = {
type = "string",
}
+ },
+ meta = {
+ type = "object",
+ properties = {
+ lua_shared_dict = {
+ type = "object",
+ },
+ }
+ },
+ stream = {
+ type = "object",
+ properties = {
+ enable_access_log = {
+ type = "boolean",
+ },
+ access_log = {
+ type = "string",
+ },
+ access_log_format = {
+ type = "string",
+ },
+ access_log_format_escape = {
+ type = "string",
+ },
+ lua_shared_dict = {
+ type = "object",
+ },
+ }
+ },
+ main_configuration_snippet = {
+ type = "string"
+ },
+ http_configuration_snippet = {
+ type = "string"
+ },
+ http_server_configuration_snippet = {
+ type = "string"
+ },
+ http_server_location_configuration_snippet = {
+ type = "string"
+ },
+ http_admin_configuration_snippet = {
+ type = "string"
+ },
+ http_end_configuration_snippet = {
+ type = "string"
+ },
+ stream_configuration_snippet = {
+ type = "string"
+ },
+ http = {
+ type = "object",
+ properties = {
+ enable_access_log = {
+ type = "boolean"
+ },
+ access_log = {
+ type = "string"
+ },
+ access_log_buffer = {
+ type = "integer"
+ },
+ access_log_format = {
+ type = "string"
+ },
+ access_log_format_escape = {
+ type = "string"
+ },
+ keepalive_timeout = {
+ type = "string"
+ },
+ client_header_timeout = {
+ type = "string"
+ },
+ client_body_timeout = {
+ type = "string"
+ },
+ client_max_body_size = {
+ type = "number"
Review Comment:
Let me try. I originally had it as string but it threw an error.
[`f3ecf05` (#10339)](https://github.com/apache/apisix/pull/10339/commits/f3ecf055e025c1b92abc4a0804a9403e31ea8a68)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: add missing properties to schema [apisix]
Posted by "pottekkat (via GitHub)" <gi...@apache.org>.
pottekkat commented on PR #10339:
URL: https://github.com/apache/apisix/pull/10339#issuecomment-1766711880
@Revolyssup I got this error before so I tried changing it:
https://github.com/apache/apisix/actions/runs/6549717930/job/17787244082?pr=10339#step:5:287
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: add missing properties to schema [apisix]
Posted by "Revolyssup (via GitHub)" <gi...@apache.org>.
Revolyssup commented on PR #10339:
URL: https://github.com/apache/apisix/pull/10339#issuecomment-1766698318
@pottekkat Can you change this to string? The test will pass
https://github.com/apache/apisix/pull/10339/files#diff-33a40d0c58eba6fec065b7df0f75fd0637387555e3ad0170d8e6674948575093R573
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: add missing properties to schema [apisix]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 merged PR #10339:
URL: https://github.com/apache/apisix/pull/10339
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix: add missing properties to schema [apisix]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on PR #10339:
URL: https://github.com/apache/apisix/pull/10339#issuecomment-1772028979
LGTM, That's a pretty big change. So let's make the ci pass
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org