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.

Sign up for GitHub

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

Jump to bottom

Unable to use nginx gateway fabric in front of docker registry service (need to set client_max_body_size) #1245

Closed
larsks opened this issue Nov 14, 2023 · 7 comments
Closed

Unable to use nginx gateway fabric in front of docker registry service (need to set client_max_body_size) #1245

larsks opened this issue Nov 14, 2023 · 7 comments
Labels
area/nginx-configuration Relates to nginx configuration enhancement New feature or request
Milestone
v1.3.0

Comments

@larsks
Copy link

larsks commented Nov 14, 2023

Describe the bug

We are trying to use the nginx gateway fabric to expose a container registry running the docker registry image. While the basic configuration works fine, pushing images into the registry generally fails with:

$ podman push exampleimage registry.apps.example.com/exampleimage:latest
Error: writing blob: uploading layer chunked: StatusCode: 413, <html>
<head><title>413 Request Entity Too Large<...

The solution is to increase client_max_body_size; but I've been unable to find any documentation that describes how to set custom nginx configuration options via the gateway api.

Expected behavior

I expected to be able to use nginx gateway fabric to expose an internal registry service.

Your environment

  • Version of the NGINX Gateway Fabric: v1.0.0
  • Version of Kubernetes: v1.27
  • Kubernetes platform (e.g. Mini-kube or GCP): kind
  • Details on how you expose the NGINX Gateway Fabric Pod: NodePort
  • Logs of NGINX container: nginxlog.txt
  • NGINX Configuration:
    nginxconf.txt
@mpstefan mpstefan added this to the v1.1.0 milestone Nov 15, 2023
@mpstefan mpstefan added enhancement New feature or request area/nginx-configuration Relates to nginx configuration size/small Estimated to be completed within ~2 days refined Requirements are refined and the issue is ready to be implemented. labels Nov 15, 2023
@sjberman
Copy link
Contributor

Hi @larsks, thanks for opening this issue. We are currently adding support for configuring some nginx options via a CRD, and will add this field to that CRD to make it easy to configure.

@kate-osborn kate-osborn added the blocked Blocked by other issue label Nov 16, 2023
@mpstefan
Copy link
Collaborator

Hey @larsks, after discussing this among the maintainers today, we realized we need a much better way to expose native NGINX configuration, as the more we expose the messier it starts to get. Looking at the Gateway API spec, the best way we think is by implementing some new policies to attach to the various Gateway API objects so we apply NGINX directive to any scope (GatewayClass, Gateway, Route) wherever possible.

We believe this will be a MUCH better experience, but since it will be significantly more effort, we'll likely have a first pass at this in 1.2 instead of 1.1. You can see the epic here: #1258

@mpstefan mpstefan mentioned this issue Nov 17, 2023
Closed
@larsks
Copy link
Author

larsks commented Nov 17, 2023

Thanks for the update!

It looks like the policy attachment model is (still) experimental right now. What about an additional option that takes advantage of the parametersRef attribute on the GatewayClass resource? This could be as simple as a reference to a ConfigMap that creates files in /etc/nginx/conf.d corresponding to the keys in the ConfigMap.

@larsks
Copy link
Author

larsks commented Nov 17, 2023

I was hoping I could work around this by mounting a custom config file into /etc/nginx/conf.d, but you're killin' me 😄

@sjberman
Copy link
Contributor

@larsks The parametersRef approach is what we originally considered (though using a CRD), but we had to realize that anything mounted at the GatewayClass level is propagated to every route and backend that lives under that class. We want to be deliberate about where we allow certain nginx configuration directives to be specified and design it in such a way that it makes the most sense for the level and use cases for which those directives apply (Gateway vs HTTPRoute vs Service, etc.).

We also want to investigate a more generalized (but secure) way for users to update more advanced nginx directives that may not make sense in a Policy, or haven't yet been added to a Policy. That is still in early stages though.

In the meantime, one workaround is to alter the base nginx config to fit your needs, and then build a custom nginx image that you can deploy instead of our release image.

@larsks
Copy link
Author

larsks commented Nov 18, 2023

It's relatively easy to patch nginx.conf without building a custom image; I'm leaving this here in case other folks need to do the same thing.

Assuming that our modified nginx.conf lives locally in configs/nginx.conf, the following kustomization.yaml will patch the NGINX Gateway Fabric deployment to use the custom configuration:

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
commonLabels:
  nginxGatewayVersion: v1.0.0

resources:
- https://github.com/nginxinc/nginx-gateway-fabric/releases/download/v1.0.0/crds.yaml
- https://github.com/nginxinc/nginx-gateway-fabric/releases/download/v1.0.0/nginx-gateway.yaml
- https://raw.githubusercontent.com/nginxinc/nginx-gateway-fabric/v1.0.0/deploy/manifests/service/nodeport.yaml

patches:
- patch: |
    apiVersion: apps/v1
    kind: Deployment
    metadata:
      name: nginx-gateway
      namespace: nginx-gateway
    spec:
      template:
        spec:
          containers:
            - name: nginx
              volumeMounts:
                - mountPath: /etc/nginx/nginx.conf
                  name: nginx-conf-override
                  subPath: nginx.conf
          volumes:
            - name: nginx-conf-override
              configMap:
                name: nginx-conf-override

configMapGenerator:
- name: nginx-conf-override
  namespace: nginx-gateway
  options:
    disableNameSuffixHash: true
  files:
  - configs/nginx.conf

@ja20222 ja20222 added the backlog Currently unprioritized work. May change with user feedback or as the product progresses. label Nov 20, 2023
@ja20222 ja20222 removed this from the v1.1.0 milestone Nov 20, 2023
@mpstefan mpstefan added this to the v1.2.0 milestone Dec 19, 2023
@mpstefan mpstefan removed the backlog Currently unprioritized work. May change with user feedback or as the product progresses. label Feb 7, 2024
@mpstefan mpstefan removed refined Requirements are refined and the issue is ready to be implemented. size/small Estimated to be completed within ~2 days labels Feb 21, 2024
@mpstefan mpstefan mentioned this issue Feb 21, 2024
Closed
@mpstefan mpstefan modified the milestones: v1.2.0, v1.3.0 Mar 5, 2024
@mpstefan mpstefan mentioned this issue Mar 29, 2024
Closed
@mpstefan
Copy link
Collaborator

This functionality will now be delivered as part of our ClientSettings Policy!

Closing this issue to avoid duplication.

@github-project-automation github-project-automation bot moved this from 🆕 New to ✅ Done in NGINX Gateway Fabric Mar 29, 2024
@mpstefan mpstefan closed this as not planned Won't fix, can't repro, duplicate, stale Mar 29, 2024
@kate-osborn kate-osborn removed the blocked Blocked by other issue label Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/nginx-configuration Relates to nginx configuration enhancement New feature or request
Projects
NGINX Gateway Fabric
Archived in project
Milestone
v1.3.0
Development

No branches or pull requests
5 participants
@larsks @mpstefan @sjberman @kate-osborn @ja20222