nginxinc/nginx-go-crossplane

"include" directive is not allowed within an "if" block

dareste opened this issue · 2 comments

The following block of code is a valid nginx configuration:

 19    server
 20    {
 21       listen 12345;
 22       location /
 23       {
 24          if ($request_method = 'OPTIONS')
 25          {
 26             include conf.d/cors_header;
 27
 28             return 204;
 29          }
 30
 31          root /usr/share/nginx/html/;
 32          try_files $uri $uri/ /en/index.html =404;
 33
 34       }
 35    }

Despite that, the tool detects the "include" as a non-allowed directive under the "if" context:

  "errors": [
    {
      "file": "test.conf",
      "line": 26,
      "error": "\"include\" directive is not allowed here in test.conf:26"
    }
  ],

To reproduce

  • Take the mentioned configuration, load it in an nginx environment. Verify that it succeeds and there are no deferred errors.
  • Use the same config as an input for nginx-go-crossplane latest version, and see the above error.

Expected behavior

The mentioned config shouldn't be throwing an error.

Additional context

This is likely caused by the directives map config here:

"include": {

I think we should modify the mask to include the "http > location > if"

	"include": {
		ngxAnyConf | ngxConfTake1 | ngxHTTPLifConf,
	},
ornj commented

Agreed that snippet should work, although I think the mask should just be nginxAnyConf | nginxConfTake1 based on the NGINX source.

Will you open a PR?

I agree that the mask should be consistent with what we see in the NGINX source definition. The problem is that here we define ngxAnyConf as "any context except server > if, location > if and location > limit_except":

// helpful directive location alias describing "any" context
// doesn't include ngxHTTPSifConf, ngxHTTPLifConf, or ngxHTTPLmtConf.
const ngxAnyConf = ngxMainConf | ngxEventConf | ngxMailMainConf | ngxMailSrvConf |
ngxStreamMainConf | ngxStreamSrvConf | ngxStreamUpsConf |
ngxHTTPMainConf | ngxHTTPSrvConf | ngxHTTPLocConf | ngxHTTPUpsConf

The only place where we use ngxAnyConf is when we define the include directive, so I think we should be ok by changing the ngxAnyConf definition to include these three exceptions, which are valid contexts for the include directive.

I'll open a PR.