-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Load OpenAPI files from ConfigMap #5633
Comments
Hi @anderius thanks for reporting! Be sure to check out the docs and the Contributing Guidelines while you wait for a human to take a look at this 🙂 Cheers! |
I think there might be a couple different issues here... All configuration artifacts being present or able to be processed has always been an NGINX requirement. The page you link mentions the NGINX App Protect WAF Open API spec and I am guessing using an external reference to provide it to NIC + WAF as a proposal to avoid a timing / workflow race condition when using external references. The external references pattern that is linked to, comes from NGINX App Protect WAF and not the NIC project. We are working on building additional documentation that better describes this workflow. |
(Please direct me to another place if that is more suitable for this feature request) WAF bundles are nice, but to my understanding they only work in one of these conditions:
The need to create a custom operator, only for this, would be less than ideal. :-) My goal is to support a multi-tenancy scenario, in which the error of one tenant should not stop reloads (or even startup) for all tenants using the same ingress. Right now I consider loading OpenAPI files to be a big threat to that goal. Other file references has the option to specify file content directly. optionally base64 encoded. For instance the "json-validation-files": [
{
"fileName": "person_schema.json",
"contents": "{\r\n \"$schema\": \"http://json-schema.org/draft-07/schema#\",\r\n \"title\": \"Person\",\r\n \"type\": \"object\",\r\n \"properties\": {\r\n \"firstName\": {\r\n \"type\": \"string\",\r\n \"description\": \"The person's first name.\"\r\n },\r\n \"lastName\": {\r\n \"type\": \"string\",\r\n \"description\": \"The person's last name.\"\r\n },\r\n \"age\": {\r\n \"description\": \"Age in years which must be equal to or greater than zero.\",\r\n \"type\": \"integer\",\r\n \"minimum\": 0\r\n }\r\n }\r\n}"
}
], |
I think there is a conflation of products here.
Part of this is separate from the OpenAPI spec and relates to how the system is configured or deployed. Policy Bundles were an NGINX App Protect WAF response to the external reference within App Protect WAF itself. Today, we generally expect customers to be using JSON based Policy with the ingress controller. As that encapsulates the Policy as an object. It has the negative impact on the system in that a Policy compilation happens within the ingress controller container at reload and a corresponding CPU spike which can be prolonged if CPU limits are set. |
Perhaps file references are exactly what you need? "open-api-files": [
{
"link": "file:///myapi2.json"
}
] |
Thank you all for the input! Using a file reference is of course an option, but that would, as far as I know, require writing tooling for that. I work in an organization with multiple, dedicated platform teams, so that is doable. But, as always, writing nothing at all is usually better. :-) To be more precise, would it be possible with something like this openapi files as well? kubernetes-ingress/config/crd/bases/appprotect.f5.com_appolicies.yaml Lines 774 to 779 in c6f0637
Example usage that would be the same for openapi files: kubernetes-ingress/tests/data/ap-waf-grpc/grpc-block-sayhello.yaml Lines 26 to 29 in c6f0637
If that functionality is not provided by the ingress controller, please direct me to the correct project. @brianehlert We already use VirtualServer resources, but I am not aware of how that affects the impact of configuration error. Any doc links for that would be very much appreciated! |
I was just coming back to this and the file reference is essentially what the origional request is. It solves the remote reference problem. |
Is your feature request related to a problem? Please describe.
When reloading the configuration, or starting a Nginx IC pod, all external references must be present and responsive. This puts undesired dependencies on external systems.
There is a warning about that here:
Describe the solution you'd like
It would be a better option to put the OpenAPI file in a ConfigMap (or a custom resource), and load it from there. Or (if it is not too big), include it directly in the APPolicy (ie base64 encoded).
Additional context
Right now OpenAPI spec is configured like this (source):
The text was updated successfully, but these errors were encountered: