nginxinc / crossplane

Quick and reliable way to convert NGINX configurations into JSON and back.
Apache License 2.0
719 stars 87 forks source link

Does not detect a spelling mistake in proxy_pass directive. #30

Closed j450nb closed 6 years ago

j450nb commented 6 years ago

Crossplane 0.2.0 Python 3.5.2 Ubuntu 16.04

Using this location configuration with an incorrectly spelled proxy_pass directive.

location /proxy-passs {
  proxy_passs http://any.thing;
}

Crossplane does not detect this error but nginx -t does.

aluttik commented 6 years ago

Hi, thanks for taking the time to submit a ticket.

This is actually expected behavior. We want crossplane to work this way so that it won't raise errors if users are using directives from third party modules.

On a related note: crossplane can't find all kinds of errors in an nginx config. As you mentioned, even nginx -t works better for that use case. The only errors it actually looks for are when (recognized) directives A) are in the wrong context or B) have the wrong number/type of arguments.

j450nb commented 6 years ago

Hi @aluttik Thanks for responding so quick. That seems to make sense however it means that we can't use Crossplane for our use case at the moment which is testing if a developer has made a spelling mistake. Would it be possible to enable error checking for at least the proxy_pass directive as this is not a third party directive?

aluttik commented 6 years ago

@j450nb: No problem :)

And ah, that's unfortunate. I am a little confused as to how we could enable error checking for the proxy_pass directive if the error you're looking for is a spelling error. Do you mean like we should check for common spelling mistakes like proxy_passs or proxypass?

I think a better solution might be to add an option for parsing that make it so all unrecognized directives throw errors. This would currently cause a bunch of problems because stuff inside of certain blocks (like map blocks and type blocks) are read in as directives with arguments... But anyway, if we could sort that out, would that solve your issue?

j450nb commented 6 years ago

Yes, I think that if could get an option to throw an error for all unknown directives that would probably solve our issue. Thanks for the response.

aluttik commented 6 years ago

@j450nb: Sorry for taking so long on pushing this feature out. The NGINX Amplify team has been keeping me pretty busy recently, but I finally managed to find some time!

Anyway, a --strict flag has been added to the crossplane parse command and a strict parameter has been added to crossplane.parse(). If you specify strict parsing, unrecognized directives should raise errors like:

{
    "file": "/etc/nginx/nginx.conf",
    "error": "unknown directive "proxy_passs" in /etc/nginx/nginx.conf:12",
    "line": 12
}

The feature is currently merged to the master branch of this repo, and will be included in what will probably be the crossplane 0.3.2 release.

Hope this works for you! If you have any questions or concerns, feel free to hit me up.