Boeing / config-file-validator

Cross Platform tool to validate configuration files
https://boeing.github.io/config-file-validator/
Apache License 2.0
342 stars 68 forks source link

Add support for `nginx.conf` files #100

Open kehoecj opened 11 months ago

kehoecj commented 11 months ago

Description

Add support for nginx.conf validation. There is a go nginx config parser we should be able to use to perform the syntax validation. We'd have to lock it in to only detecting files named nginx.conf or some kind of wildcard like *.nginx.conf until we can implement some parts of https://github.com/Boeing/config-file-validator/issues/5

onlineque commented 11 months ago

Guys, let me try this one, please ;) Thanks !

kehoecj commented 11 months ago

Guys, let me try this one, please ;) Thanks !

Absolutely! Assigned it to you

onlineque commented 11 months ago

@kehoecj do you have an idea how to check for the nginx.conf file validity with gonginx ?

I've tried to use it this way:

func (nv NginxValidator) Validate(b []byte) (result bool, errMsg error) {
    result = true

    defer func() {
        if r := recover(); r != nil {
            result = false
            errMsg = fmt.Errorf("%v", r)
        }
    }()

    p := parser.NewStringParser(string(b))
    _ = p.Parse()

    return result, errMsg
}

but the results are not 100% correct. For example I used a config which was valid and I removed the closing bracket on its "server" block definition but it parsed happily without any issue. Thanks beforehand for any ideas.

kehoecj commented 11 months ago

@kehoecj do you have an idea how to check for the nginx.conf file validity with gonginx ?

I've tried to use it this way:

func (nv NginxValidator) Validate(b []byte) (result bool, errMsg error) {
  result = true

  defer func() {
      if r := recover(); r != nil {
          result = false
          errMsg = fmt.Errorf("%v", r)
      }
  }()

  p := parser.NewStringParser(string(b))
  _ = p.Parse()

  return result, errMsg
}

but the results are not 100% correct. For example I used a config which was valid and I removed the closing bracket on its "server" block definition but it parsed happily without any issue. Thanks beforehand for any ideas.

Haven't used the library before - just saw it as a potential option. Can you post the nginx.conf example you were using as input? Removing the closing bracket seems like it'd be a syntax error but maybe nginx isn't very picky. Does running nginx -t against the nginx.conf file validate or throw an error?

onlineque commented 11 months ago

The mentioned server block begins at line 84 and is intentionally not closed. This is how it looks like:

#user http;
worker_processes  1;

#error_log  logs/error.log;
#error_log  logs/error.log  notice;
#error_log  logs/error.log  info;

#pid        logs/nginx.pid;

events {
    worker_connections  1024;
}

http {
    include       mime.types;
    default_type  application/octet-stream;

    #log_format  main  '$remote_addr - $remote_user [$time_local] "$request" '
    #                  '$status $body_bytes_sent "$http_referer" '
    #                  '"$http_user_agent" "$http_x_forwarded_for"';

    #access_log  logs/access.log  main;

    sendfile        on;
    #tcp_nopush     on;

    #keepalive_timeout  0;
    keepalive_timeout  65;

    #gzip  on;

    server {
        listen       80;
        server_name  localhost;

        #charset koi8-r;

        #access_log  logs/host.access.log  main;

        location / {
            root   /usr/share/nginx/html;
            index  index.html index.htm;
        }

        #error_page  404              /404.html;

        # redirect server error pages to the static page /50x.html
        #
        error_page   500 502 503 504  /50x.html;
        location = /50x.html {
            root   /usr/share/nginx/html;
        }

        # proxy the PHP scripts to Apache listening on 127.0.0.1:80
        #
        #location ~ \.php$ {
        #    proxy_pass   http://127.0.0.1;
        #}

        # pass the PHP scripts to FastCGI server listening on 127.0.0.1:9000
        #
        #location ~ \.php$ {
        #    root           html;
        #    fastcgi_pass   127.0.0.1:9000;
        #    fastcgi_index  index.php;
        #    fastcgi_param  SCRIPT_FILENAME  /scripts$fastcgi_script_name;
        #    include        fastcgi_params;
        #}

        # deny access to .htaccess files, if Apache's document root
        # concurs with nginx's one
        #
        #location ~ /\.ht {
        #    deny  all;
        #}
    }

    # another virtual host using mix of IP-, name-, and port-based configuration
    #
    server {
        listen       8000;
        # listen       somename:8080;
        # server_name  somename  alias  another.alias;

        location / {
            root   html;
            index  index.html index.htm;
        }

    # HTTPS server
    #
    #server {
    #    listen       443 ssl;
    #    server_name  localhost;

    #    ssl_certificate      cert.pem;
    #    ssl_certificate_key  cert.key;

    #    ssl_session_cache    shared:SSL:1m;
    #    ssl_session_timeout  5m;

    #    ssl_ciphers  HIGH:!aNULL:!MD5;
    #    ssl_prefer_server_ciphers  on;

    #    location / {
    #        root   html;
    #        index  index.html index.htm;
    #    }
    #}

}

This is how it ends if you check it with nginx -t:

# nginx -c nginx.conf -t
2023/12/11 20:08:24 [emerg] 52636#52636: unexpected end of file, expecting "}" in /etc/nginx/nginx.conf:118
nginx: configuration file /etc/nginx/nginx.conf test failed
kehoecj commented 11 months ago

@onlineque Created an issue in the gonginx project with your findings and if this is a bug, expected behavior, or if there is a different way to do what we're trying to do.

kehoecj commented 10 months ago

@onlineque The library maintainer released an update: https://github.com/tufanbarisyildirim/gonginx/issues/31#issuecomment-1858066807. Can you try it out and see if it addresses the issue you found?

onlineque commented 10 months ago

Thanks, sure !

tufanbarisyildirim commented 10 months ago

hi @onlineque just to clarify, latest release (https://github.com/tufanbarisyildirim/gonginx/releases/tag/0.0.1) does not contain those changes, it was just to freeze the current version for people who already use it. you will either need to clone and use a replace directive in your go.mod or I can release a version for you, just let me know if you need a release.

onlineque commented 10 months ago

@kehoecj, please take a look at the WIP PR, I know the issue #5 is not yet implemented, so please let me know if this implementation is good enough for now. If not, just let me know the way you'd like to implement it. Thanks a lot !