custom-headers annotation does not apply the configuration automatically

[PavelGloba]

What happened:

I tried to use a new feature to set custom headers per ingress. After setting the annotation the configuration does not apply automatically. While there are sync events in the logs, the config reload is not triggered and nginx.conf is not changed. The feature itself is working if I restart the nginx controller. Also it's working if I set incorrect path to the configmap and then change it back to the correct path. The update of the applied headers also does not work. If I change the configmap and then delete and add the annotation again, the headers are not changing. I also tried to create a new configmap and change the annotation to point to it, but it does not trigger config reload. Basically the only way to apply the configuration of the custom headers is to either reload the controller or to trigger an errror and then apply working configuration again.

What you expected to happen:

custom-headers annotation changes (add, edit, delete) should trigger the config reload. Ideally custom headers configuartion should be also updated when the configmap with headers changes, but it's probably not possible

NGINX Ingress controller version:

NGINX Ingress controller Release: v1.11.1 Build: 7c44f992012555ff7f4e47c08d7c542ca9b4b1f7 Repository: https://github.com/kubernetes/ingress-nginx nginx version: nginx/1.25.5

Kubernetes version:

v1.27.15-eks-db838b0

Environment:

AWS EKS

Configuration:

    allow-snippet-annotations: "false"
    global-allowed-response-headers: Referrer-Policy,Strict-Transport-Security,X-Content-Type-Options,X-Frame-Options,Content-Security-Policy
    http2-max-field-size: 512k
    http2-max-header-size: 512k
    large-client-header-buffers: 8 512k
    use-proxy-protocol: "true"

[longwuyuan]

I did not test this yet, but this behavior is consistent with a lot of other features. That means config reload is not triggered on the events that you listed.

I don't think a behavior change can be implemented in the near future so you are better off doing a manual reload or deploying the changes during initial controller installation.

/remove-kind bug /kind feature /triage accepted

[PavelGloba]

@longwuyuan are you sure this is a consistent behavior, because for example configuration snippets work out of the box. Basically every feature I used which is set per ingress works when it's applied to ingress. I understand that features itself should be enabled and configured before you can use them and the controller reload is needed. With the current implementation updates of ingresses are not possible, only the recreation or some workarounds like setting wrong configmap. If setting wrong configmap and then fixing it triggers the reload, the reload should also be triggered on any changes

[longwuyuan]

@PavelGloba your expectation is correct and fair. Your expectation is the desired way such config changes should work.

The "proxy_set_header" directive is the configuration that is the result of adding a custom header. While there seems to be a watch/loop type of design for server_blocks & location blocks, the proxy_set_header is in the global config. And the global config seems to get its nginx.conf directives only at the time of creating a new pod or some other specific event like recovering from a broken controller config.

More importantly, there is a acute shortage of resources and there is a required direction that the development activities of the ingress-nginx controller must and absolutely must take. Because of these reasons it will not be possible to work on changing the controller's behavior to reload and live update nginx.conf for adding the proxy_set_header directive in the global context of nginx.conf .

I understand this is not your desired result or the desired result of any other user who has to make changes to environments that are not offering downtime. But I thought that you should be well informed to make required decisions. If its possible for someone to work on this change, they will comment here, but I think it is not likely.

Please wait for comments from others.

[jgoelen]

@longwuyuan the fix for this problem doesn't seem to be a complex one. The function func (l1 *Location) Equal(l2 *Location) bool in the file pkg/apis/ingress/types_equals.go is missing an equals check for the customheaders.Config type. Maybe with some support I can start a pull request for this issue?

[longwuyuan]

@jgoelen I am not a developer so I can not comment effectively.

What I do not know is if that check you mention is unique to just one annotation or if that code-path is shared by other annotations. I also don't know if the e2e tests will cause a false positive. What I do know is that in the past this may seem trivial for one annotation but there is a need for developers to comment on this. So I will wait for a developer to comment.

On the other hand its a great help to get contributions as its the priceless value to K8S projects. Hence waiting for comments from others.

[longwuyuan]

You can message in the Kubernetes slack in the channel ingress-nginx-dev

[github-actions[bot]]

This is stale, but we won't close it automatically, just bare in mind the maintainers may be busy with other tasks and will reach your issue ASAP. If you have any question or request to prioritize this, please reach #ingress-nginx-dev on Kubernetes Slack.