Predefined custom snippets

[tylermichael]

What do you want to happen?

A lot of NGINX functionality is locked behind using the custom snippet feature, but it is associated with a CVE. This means that in many environments, you cannot enable it.

To bridge this divide, I think a feature somewhat similar to what's discussed in #11259 should be added that allows the admin operator to predefine custom snippets, and then opt-in an ingress to use them. AFAICT, this is an improvement on the current custom snippets because it limits the surface area where arbitrary code could be introduced to the ingress controller config.

The admin operator could then add the config files that define these snippets to CODEOWNERS to prevent malicious changes.

This comment describes how I think it could work:

apiVersion: v1
kind: ConfigMap
metadata:
  name: ingress-nginx-controller
  namespace: ingress-nginx
data:
  server-snippets: # ...
  configuration-snippets: |
    - name: "add-custom-headers"
      snippet: |
          add_header Foo "Bar";
          add_header X-My-Custom-Header "1234";
    - name: "do-not-proxy-cookies"
      snippet: |
          proxy_set_header Cookie "";
---
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  annotations:
    nginx.ingress.kubernetes.io/enable-configuration-snippets: "add-custom-headers,do-not-proxy-cookies"
  name: foo
  namespace: default
spec:
  ingressClassName: ingress-nginx
  rules:
  - host: foo.example.com
    http:
      paths:
      - path: /
        pathType: Prefix
        backend:
          service:
            name: backend
            port: 
              number: 80

Related issue

#11667

[k8s-ci-robot]

This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[longwuyuan]

@tylermichael completely agree on the useful of your suggestion. It is actually a demand by many other users as well.

But the flip side is that contributions like your suggestions & other features that are also super useful need maintenance. Relevance being the resource crunch of developers dedicated to supporting & maintaining is so bad that we are actually deprecating useful functioning popular features. So I would not expect any traction on this feature request.

[tylermichael]

@longwuyuan Understood. I wouldn't mind creating a PR to add support for this as it's a bit of a blocker in my org.

[longwuyuan]

• @tylermichael I am not a developer so please wait for other comments.

• Example you showed is about add_headers and proxy_set_$$$$$ . AFAIK, these 2 config options are already present https://kubernetes.github.io/ingress-nginx/user-guide/nginx-configuration/configmap/#proxy-set-headers .Other discussion has to be specific to a directive you need

• A generic approach to create a custom-snippets is a good idea. It is also helpful to know you are interested in contributing the code for that.. It could help other users of snippets users.

• But I don't think you should expect much traction on this. There were CVEs reported and there was a lack of resources to support/maintain many such useful features. So the focus now is the core function of Ingress-Controller as specified in the upstream K8S KEP. Implementation of the Gateway-API is also the focus.

• So adding features, without resources to maintain & support them, is not an option, unfortunately. In fact we have deprecated several popular useful helpful features because they got in the way of shipping a secure & stable ingress-controller.

[tylermichael]

@longwuyuan That proxy-set-headers config sets the configured headers on all requests for all ingresses. There's currently no way to set proxy headers per ingress.

The way I view this feature is, it could actually reduce dev workload because as mentioned in many other issues, there are a lot of NGINX features that are not configurable via annotations. And now that custom snippets are informally deprecated (there are even talks to formally deprecate and remove them), this provides a more secure escape hatch, removing the need to add explicit support for functionality that can be accomplished via snippets. AFAICT, the CVE exists because arbitrary code can be added on any ingress, breaking isolation principles, but this proposal moves the definition of snippets to a trusted configuration file.

But I will wait for others to comment before starting a PR. I appreciate you and the other maintainers continued support of the project!

[strongjz]

At this time we not actively adding new features as we migrate our focus to ingate.

We have discussed at the gateway-api community meeting and our latest ingress-nginx Kubcon Maintainer talk.

The repo to following along is at: https://github.com/kubernetes-sigs/ingate

/close

[k8s-ci-robot]

@strongjz: Closing this issue.

In response to this:

At this time we not actively adding new features as we migrate our focus to ingate.

We have discussed at the gateway-api community meeting and our latest ingress-nginx Kubcon Maintainer talk.

The repo to following along is at: https://github.com/kubernetes-sigs/ingate

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.