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

Allow forwarding headers from auth_request without flattening #12880

Open
aslafy-z opened this issue Feb 21, 2025 · 1 comment
Open

Allow forwarding headers from auth_request without flattening #12880

aslafy-z opened this issue Feb 21, 2025 · 1 comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@aslafy-z
Copy link
Contributor

aslafy-z commented Feb 21, 2025

Currently, ingress-nginx flattens multiple occurrences of the same header returned by auth_request into a single header with comma-separated values. This behavior prevents proper forwarding of multiple headers with the same name, which is required for certain use cases such as Kubernetes impersonation headers.

For example, when the auth_request response includes:

Impersonate-Group: A
Impersonate-Group: B
Impersonate-Group: C

ingress-nginx transforms this into:

Impersonate-Group: A, B, C

This is problematic because upstream applications expecting separate headers receive a single combined header instead.

Proposal:
Introduce a mechanism in Ingress NGINX that allows forwarding multiple headers as separate entries instead of flattening them.

Suggested Implementation

The following Lua script demonstrates how to properly forward multiple headers from auth_request using ngx_req.add_header, which ensures that repeated headers are preserved:

local auth_path = ngx.var.auth_path
local header_list = ngx.var.forward_headers

-- Split the comma-separated list into a table
local headers = {}
for header in string.gmatch(header_list, "[^,]+") do
  table.insert(headers, header:match("^%s*(.-)%s*$")) -- Trim spaces
end

local res = ngx.location.capture(auth_path)
if res.status ~= 200 then
    ngx.exit(res.status)
end

local ngx_req = require "ngx.req"

for _, header_name in ipairs(headers) do
  local values = res.header[header_name]
  if values then
    if type(values) ~= "table" then
      values = { values } -- Convert single value to table
    end

    for _, value in ipairs(values) do
      ngx_req.add_header(header_name, value)
    end
  end
end

Usage

This Lua script can be integrated into an Ingress NGINX configuration like this:

    set $auth_path "/_auth";
    set $forward_headers "Impersonate-Group,Impersonate-Extra";
    access_by_lua_file "/etc/nginx/lua/forward_header.lua";

Expected Behavior

With this change, multiple headers of the same name are forwarded separately to the upstream application instead of being merged into a single comma-separated value.

Current implementation

The current implementation responsible for handling headers in auth_request is located in:
/~https://github.com/kubernetes/ingress-nginx/blob/main/internal/ingress/controller/template/template.go#L578-L611 and /~https://github.com/kubernetes/ingress-nginx/blob/main/rootfs/etc/nginx/template/nginx.tmpl#L1182-L1209.

Note that it already uses lua to evaluate these headers most of the time.

A modification to this logic should allow preserving multiple headers instead of flattening them.

Benefits

  • Enables correct handling of headers required for Kubernetes impersonation (eg, kubernetes-dashboard)
  • Improves compatibility with applications that expect multiple headers instead of a single comma-separated value.
  • Provides greater flexibility in handling auth_request headers.

Would the Ingress NGINX maintainers be open to reviewing a PR for this feature?

@aslafy-z aslafy-z added the kind/feature Categorizes issue or PR as related to a new feature. label Feb 21, 2025
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Feb 21, 2025
@k8s-ci-robot
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
Development

No branches or pull requests

2 participants