Skip to content
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

Add support for nginx.conf files #100

Open
kehoecj opened this issue Dec 1, 2023 · 10 comments · May be fixed by #107
Open

Add support for nginx.conf files #100

kehoecj opened this issue Dec 1, 2023 · 10 comments · May be fixed by #107
Assignees
Labels
help wanted Extra attention is needed new validation type New config file type to validate

Comments

@kehoecj
Copy link
Contributor

kehoecj commented Dec 1, 2023

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 #5

@kehoecj kehoecj added help wanted Extra attention is needed new validation type New config file type to validate labels Dec 1, 2023
@onlineque
Copy link
Contributor

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

@kehoecj
Copy link
Contributor Author

kehoecj commented Dec 4, 2023

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

Absolutely! Assigned it to you

@onlineque
Copy link
Contributor

@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
Copy link
Contributor Author

kehoecj commented Dec 11, 2023

@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
Copy link
Contributor

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
Copy link
Contributor Author

kehoecj commented Dec 12, 2023

@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
Copy link
Contributor Author

kehoecj commented Dec 15, 2023

@onlineque The library maintainer released an update: tufanbarisyildirim/gonginx#31 (comment). Can you try it out and see if it addresses the issue you found?

@onlineque
Copy link
Contributor

Thanks, sure !

@tufanbarisyildirim
Copy link

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
Copy link
Contributor

@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 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed new validation type New config file type to validate
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants