Ability to pass custom Nginx configs to the data plane

[vazkarvishal]

Describe the bug
In version 1.5.0 we were able to pass custom NGINX configuration blocks as volume mounts to the helm chart using ExtraVolumes and ExtraVolumeMounts variables. But they don't seem to be working in version 2.x although the variables exist in the helm chart. (Value not expected error while syncing the helm chart).

Question 1: Is this a bug which can be fixed?

Question 2: Is there a better way of passing global Nginx Data Plane configurations such as the below:

apiVersion: v1
kind: ConfigMap
metadata:
  name: nginx-conf-override
  namespace: nginx-gateway
data:
  nginx.conf: |2
        load_module /usr/lib/nginx/modules/ngx_http_js_module.so;
        include /etc/nginx/main-includes/*.conf;

        worker_processes auto;

        pid /var/run/nginx/nginx.pid;

        events {
          worker_connections 1024;
        }

        http {
          include /etc/nginx/conf.d/*.conf;
          include /etc/nginx/mime.types;
          js_import /usr/lib/nginx/modules/njs/httpmatches.js;

          default_type application/octet-stream;

          underscores_in_headers on;
          large_client_header_buffers 4 16k;

          proxy_headers_hash_bucket_size 512;
          proxy_headers_hash_max_size 1024;
          server_names_hash_bucket_size 256;
          server_names_hash_max_size 1024;
          variables_hash_bucket_size 512;
          variables_hash_max_size 1024;
          client_max_body_size 2M;

          sendfile on;
          tcp_nopush on;

          server_tokens off;

          server {
            listen unix:/var/run/nginx/nginx-status.sock;
            access_log off;

            location /stub_status {
                stub_status;
            }
          }
        }

        stream {
          variables_hash_bucket_size 512;
          variables_hash_max_size 1024;

          map_hash_max_size 2048;
          map_hash_bucket_size 256;

          log_format stream-main '$remote_addr [$time_local] '
                                 '$protocol $status $bytes_sent $bytes_received '
                                 '$session_time "$ssl_preread_server_name"';
          access_log /dev/stdout stream-main;
          include /etc/nginx/stream-conf.d/*.conf;
        }
 

Below is the screenshot of the sync error:

Seems to be a miss on the values schema from when the data plane and gateway schemas have been split but not reflected correctly. Can we fix this?

[nginx-bot]

Hi @vazkarvishal! Welcome to the project! 🎉

Thanks for opening this issue!
Be sure to check out our Contributing Guidelines and the Issue Lifecycle while you wait for someone on the team to take a look at this.

[sjberman]

Hey @vazkarvishal, looks like this is a bug with the names of the helm values. It should just be called volumes and volumeMounts, without the extra prefix (for the nginx data plane config). Can you try specifying using those value names instead? If that works, then we'll need to update our own values.yaml file to fix those names.

[vazkarvishal]

Okay @sjberman that makes sense. Looks like the nginx_proxy CRD schema has the volumes and volumeMounts values but the helm values schema does not have the values in there. I am assuming that the below container block is optional hence wont check those variables at all correct?

"container": {
          "description": "The container configuration for the NGINX container. This is applied globally to all Gateways managed by this\ninstance of NGINX Gateway Fabric.",
          "required": [],
          "title": "container",
          "type": "object"
        },

[sjberman]

The container block is optional and its fields should map directly to the NginxProxy API, per your observation.

[vazkarvishal]

Okay I will try that now and revert back to you @sjberman. Is this the best way to add NGINX configs to the Data Plane? or would you recommend using a different approach? (separate to this bug). Also would it be worth adding a bug label to this issue?

[sjberman]

We have the SnippetsFilter API which allows you to inject custom nginx configuration for cases where you need something that isn't supported through a first class Policy or Gateway API config.

[vazkarvishal]

Yeah although, I cannot enforce it globally and we have 100s of HTTPRoutes which will need updating if we use that. Thus having to fallback to something we can manage globally. While we fix this, is it also possible to fix the deployment which gets created for the data plane to have the metadata > name value to be dynamic/have something self-documenting like data-plane? right now it gets a random name like nginx-nginx

[sjberman]

The name of the deployment is standardized by the Gateway API spec, and is the <gateway-name>-<gatewayclass-name>, so that will need to stay as-is right now.

[vazkarvishal]

Okay I have tested it now and it looks like this has made it work, so happy for you to go ahead and change the helm values. Alternatively I am happy to make the change for you (if it is merely removing the helm values from the values file). Thanks!

[sjberman]

We'd be happy to accept your change :) it would just be updating the name of the values to remove the extra prefix.