nginxinc / nginx-go-crossplane

A library for working with NGINX configs in Go
Apache License 2.0
46 stars 12 forks source link

Add lua analyzer to stream and upstream #103

Open rikatz opened 1 month ago

rikatz commented 1 month ago

Proposed changes

Some directives are missing on NGINX stream, and some other inside main HTTP server.

This PR fixes the missing directives

Checklist

Before creating a PR, run through this checklist and mark each as complete.

ornj commented 1 month ago

Thank you for submitting a PR @rikatz.

The current set of Lua directives supported by crossplane focus on supporting ngx_http_lua_module. I think the changes you're making are releated to ngx_stream_lua_module.

@xynicole @dengsh12 Should we support this module by merging the flags into moduleLuaDirectives or do we want to model it as a separate optional module?

xynicole commented 1 month ago

@ornj I am leaning towards the second option as it provides better modularity and clarity. Additionally, since it involves two separate repositories, it would be easier to use a generate approach rather than manually adding the directives. Furthermore, since the ngx_stream_lua_module is not distributed with NGINX by default, using it as a separate optional module allows us to avoid affecting the existingngx_http_lua_module directives.

dengsh12 commented 1 month ago

Thank you for submitting a PR @rikatz.

The current set of Lua directives supported by crossplane focus on supporting ngx_http_lua_module. I think the changes you're making are releated to ngx_stream_lua_module.

@xynicole @dengsh12 Should we support this module by merging the flags into moduleLuaDirectives or do we want to model it as a separate optional module?

I thought in a separate file is easier for management and make more sense. It seems they are registered as two modules in fact(different ngx_module_t). If we merge into one file it comes to a new special case(and seems a bit weird) for the code generator. The relationships among ParseOptions.DirectiveSources is or so put them in two files won't hurt any valid config files.

After the go generate tool available it should be easy to add & keep update for it. Since we have specialize bitmasks for block directives in http_lua_module, I wonder if we also need some special operations for this module?