Skip to content

Commit b3bfa4c

Browse files
ciarams87ciarams87
committed
Normalize EPP headers to lowercase
1 parent da75ec6 commit b3bfa4c

File tree

11 files changed

+106
-1026
lines changed

11 files changed

+106
-1026
lines changed

‎cmd/gateway/endpoint_picker.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"io"
88
"net"
99
"net/http"
10+
"strings"
1011
"time"
1112

1213
corev3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
@@ -37,7 +38,7 @@ func realExtProcClientFactory() extProcClientFactory {
3738
return func(target string) (extprocv3.ExternalProcessorClient, func() error, error) {
3839
creds := credentials.NewTLS(&tls.Config{
3940
// add RootCAs or, if you have a self-signed server cert:
40-
InsecureSkipVerify: true,
41+
InsecureSkipVerify: true, //nolint:gosec
4142
})
4243
conn, err := grpc.NewClient(target, grpc.WithTransportCredentials(creds))
4344
if err != nil {
@@ -153,8 +154,13 @@ func buildHeaderRequest(r *http.Request) *extprocv3.ProcessingRequest {
153154

154155
for key, values := range r.Header {
155156
for _, value := range values {
157+
// Normalize header keys to lowercase for case-insensitive matching
158+
// This fixes the issue where Go's HTTP header normalization (Title-Case)
159+
// doesn't match EPP's expected lowercase header keys
160+
normalizedKey := strings.ToLower(key)
161+
156162
headerMap.Headers = append(headerMap.Headers, &corev3.HeaderValue{
157-
Key: key,
163+
Key: normalizedKey,
158164
Value: value,
159165
})
160166
}

‎internal/controller/nginx/config/servers.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,7 @@ func createInternalLocationsForRule(
459459
}
460460
intInfLocation.EPPInternalPath = intLocation.Path
461461
if b.EndpointPickerNsName != "" {
462-
intInfLocation.EPPHost = string(b.EndpointPickerConfig.Name) + "." + b.EndpointPickerNsName
462+
intInfLocation.EPPHost = string(b.EndpointPickerConfig.Name) + "." + b.EndpointPickerNsName + ".svc.cluster.local"
463463
} else {
464464
intInfLocation.EPPHost = string(b.EndpointPickerConfig.Name)
465465
}
@@ -517,7 +517,11 @@ func createInferenceLocationsForRule(
517517
portNum = int(b.EndpointPickerConfig.Port.Number)
518518
}
519519
extLocations[i].EPPInternalPath = intLocation.Path
520-
extLocations[i].EPPHost = string(b.EndpointPickerConfig.Name)
520+
if b.EndpointPickerNsName != "" {
521+
extLocations[i].EPPHost = (string(b.EndpointPickerConfig.Name) + "." + b.EndpointPickerNsName + ".svc.cluster.local") //nolint:lll
522+
} else {
523+
extLocations[i].EPPHost = string(b.EndpointPickerConfig.Name)
524+
}
521525
extLocations[i].EPPPort = portNum
522526
}
523527
}

‎internal/controller/nginx/modules/src/epp.js‎

Lines changed: 32 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -10,48 +10,40 @@ const WORKLOAD_ENDPOINT_VAR = 'inference_workload_endpoint';
1010
const SHIM_URI = 'http://127.0.0.1:54800';
1111

1212
async function getEndpoint(r) {
13-
const headerEndpoint = r.headersIn['test-epp-endpoint-selection'];
14-
if (headerEndpoint) {
15-
// Header is provided: Use endpoints directly and bypass Shim server
16-
const endpoints = headerEndpoint.split(',').map(e => e.trim());
17-
r.variables[WORKLOAD_ENDPOINT_VAR] = endpoints.join(',');
18-
r.log(`Using header-specified endpoints: ${r.variables[WORKLOAD_ENDPOINT_VAR]}`);
19-
} else {
20-
if (!r.variables[EPP_HOST_HEADER_VAR] || !r.variables[EPP_PORT_HEADER_VAR]) {
21-
throw Error(
22-
`Missing required variables: ${EPP_HOST_HEADER_VAR} and/or ${EPP_PORT_HEADER_VAR}`,
23-
);
24-
}
25-
if (!r.variables[EPP_INTERNAL_PATH_VAR]) {
26-
throw Error(`Missing required variable: ${EPP_INTERNAL_PATH_VAR}`);
27-
}
13+
if (!r.variables[EPP_HOST_HEADER_VAR] || !r.variables[EPP_PORT_HEADER_VAR]) {
14+
throw Error(
15+
`Missing required variables: ${EPP_HOST_HEADER_VAR} and/or ${EPP_PORT_HEADER_VAR}`,
16+
);
17+
}
18+
if (!r.variables[EPP_INTERNAL_PATH_VAR]) {
19+
throw Error(`Missing required variable: ${EPP_INTERNAL_PATH_VAR}`);
20+
}
2821

29-
let headers = Object.assign({}, r.headersIn);
30-
headers[EPP_HOST_HEADER] = r.variables[EPP_HOST_HEADER_VAR];
31-
headers[EPP_PORT_HEADER] = r.variables[EPP_PORT_HEADER_VAR];
22+
let headers = Object.assign({}, r.headersIn);
23+
headers[EPP_HOST_HEADER] = r.variables[EPP_HOST_HEADER_VAR];
24+
headers[EPP_PORT_HEADER] = r.variables[EPP_PORT_HEADER_VAR];
3225

33-
try {
34-
const response = await ngx.fetch(SHIM_URI, {
35-
method: r.method,
36-
headers: headers,
37-
body: r.requestText,
38-
});
39-
const endpointHeader = response.headers.get(ENDPOINT_HEADER);
40-
if (response.status === 200 && endpointHeader) {
41-
r.variables[WORKLOAD_ENDPOINT_VAR] = endpointHeader;
42-
r.log(
43-
`found inference endpoint from EndpointPicker: ${r.variables[WORKLOAD_ENDPOINT_VAR]}`,
44-
);
45-
} else {
46-
const body = await response.text();
47-
r.error(
48-
`could not get specific inference endpoint from EndpointPicker; ` +
49-
`status: ${response.status}; body: ${body}`,
50-
);
51-
}
52-
} catch (err) {
53-
r.error(`Error in ngx.fetch: ${err}`);
26+
try {
27+
const response = await ngx.fetch(SHIM_URI, {
28+
method: r.method,
29+
headers: headers,
30+
body: r.requestText,
31+
});
32+
const endpointHeader = response.headers.get(ENDPOINT_HEADER);
33+
if (response.status === 200 && endpointHeader) {
34+
r.variables[WORKLOAD_ENDPOINT_VAR] = endpointHeader;
35+
r.log(
36+
`found inference endpoint from EndpointPicker: ${r.variables[WORKLOAD_ENDPOINT_VAR]}`,
37+
);
38+
} else {
39+
const body = await response.text();
40+
r.error(
41+
`could not get specific inference endpoint from EndpointPicker; ` +
42+
`status: ${response.status}; body: ${body}`,
43+
);
5444
}
45+
} catch (err) {
46+
r.error(`Error in ngx.fetch: ${err}`);
5547
}
5648

5749
// If performing a rewrite, $request_uri won't be used,
@@ -63,4 +55,5 @@ async function getEndpoint(r) {
6355

6456
r.internalRedirect(r.variables[EPP_INTERNAL_PATH_VAR] + args);
6557
}
58+
6659
export default { getEndpoint };

‎internal/controller/nginx/modules/test/epp.test.js‎

Lines changed: 55 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ describe('getEndpoint', () => {
4040
});
4141

4242
it('sets endpoint and logs on 200 with endpoint header', async () => {
43-
const endpoint = 'http://endpoint';
43+
const endpoint = '10.0.0.1:8080';
4444
globalThis.ngx = {
4545
fetch: vi.fn().mockResolvedValue({
4646
status: 200,
@@ -49,7 +49,11 @@ describe('getEndpoint', () => {
4949
}),
5050
};
5151
const r = makeRequest({
52-
variables: { epp_host: 'host', epp_port: '1234', epp_internal_path: '/foo' },
52+
variables: {
53+
epp_host: 'host',
54+
epp_port: '1234',
55+
epp_internal_path: '/foo',
56+
},
5357
});
5458
await epp.getEndpoint(r);
5559
expect(r.variables.inference_workload_endpoint).toBe(endpoint);
@@ -66,7 +70,11 @@ describe('getEndpoint', () => {
6670
}),
6771
};
6872
const r = makeRequest({
69-
variables: { epp_host: 'host', epp_port: '1234', epp_internal_path: '/foo' },
73+
variables: {
74+
epp_host: 'host',
75+
epp_port: '1234',
76+
epp_internal_path: '/foo',
77+
},
7078
});
7179
await epp.getEndpoint(r);
7280
expect(r.error).toHaveBeenCalledWith(
@@ -80,15 +88,19 @@ describe('getEndpoint', () => {
8088
fetch: vi.fn().mockRejectedValue(new Error('network fail')),
8189
};
8290
const r = makeRequest({
83-
variables: { epp_host: 'host', epp_port: '1234', epp_internal_path: '/foo' },
91+
variables: {
92+
epp_host: 'host',
93+
epp_port: '1234',
94+
epp_internal_path: '/foo',
95+
},
8496
});
8597
await epp.getEndpoint(r);
8698
expect(r.error).toHaveBeenCalledWith(expect.stringContaining('Error in ngx.fetch'));
8799
expect(r.internalRedirect).toHaveBeenCalledWith('/foo');
88100
});
89101

90102
it('preserves args in internal redirect when args are present', async () => {
91-
const endpoint = 'http://endpoint';
103+
const endpoint = '10.0.0.1:8080';
92104
globalThis.ngx = {
93105
fetch: vi.fn().mockResolvedValue({
94106
status: 200,
@@ -97,21 +109,51 @@ describe('getEndpoint', () => {
97109
}),
98110
};
99111
const r = makeRequest({
100-
variables: { epp_host: 'host', epp_port: '1234', epp_internal_path: '/foo' },
112+
variables: {
113+
epp_host: 'host',
114+
epp_port: '1234',
115+
epp_internal_path: '/foo',
116+
},
101117
args: { a: '1', b: '2' },
102118
});
103119
await epp.getEndpoint(r);
104120
expect(r.internalRedirect).toHaveBeenCalledWith('/foo?a=1&b=2');
105121
});
106-
it('returns the header-specified endpoints if provided', async () => {
122+
123+
it('forwards all headers including test headers to EPP', async () => {
124+
const endpoint = '10.0.0.1:8080';
125+
const fetchMock = vi.fn().mockResolvedValue({
126+
status: 200,
127+
headers: { get: () => endpoint },
128+
text: vi.fn(),
129+
});
130+
globalThis.ngx = {
131+
fetch: fetchMock,
132+
};
107133
const r = makeRequest({
108-
variables: {},
109-
headersIn: { 'X-Endpoint-Selector': '10.1.2.3, 10.1.2.4' },
134+
variables: {
135+
epp_host: 'host',
136+
epp_port: '1234',
137+
epp_internal_path: '/foo',
138+
},
139+
headersIn: {
140+
'test-epp-endpoint-selection': '10.0.0.1:8080,10.0.0.2:8080',
141+
'content-type': 'application/json',
142+
},
110143
});
111144
await epp.getEndpoint(r);
112-
expect(r.variables.inference_workload_endpoint).toBe('10.1.2.3,10.1.2.4');
113-
expect(r.log).toHaveBeenCalledWith(
114-
expect.stringContaining('Using header-specified endpoints'),
145+
146+
// Verify that all headers (including test header) were forwarded to EPP
147+
expect(fetchMock).toHaveBeenCalledWith(
148+
'http://127.0.0.1:54800',
149+
expect.objectContaining({
150+
headers: expect.objectContaining({
151+
'test-epp-endpoint-selection': '10.0.0.1:8080,10.0.0.2:8080',
152+
'content-type': 'application/json',
153+
'X-EPP-Host': 'host',
154+
'X-EPP-Port': '1234',
155+
}),
156+
}),
115157
);
116158
});
117-
});
159+
});

‎internal/controller/state/graph/backend_refs.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,8 @@ func addBackendRefsToRouteRules(
7777

7878
// addHTTPBackendRefsToRules iterates over the rules of a Route and adds a list of BackendRef to each rule.
7979
// If a reference in a rule is invalid, the function will add a condition to the rule.
80+
//
81+
//nolint:gocyclo
8082
func addBackendRefsToRules(
8183
route *L7Route,
8284
refGrantResolver *referenceGrantResolver,

‎tests/Makefile‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ EXPERIMENTAL_CONFORMANCE_PROFILES = GATEWAY-TLS
1818
CONFORMANCE_PROFILES = $(STANDARD_CONFORMANCE_PROFILES) # by default we use the standard conformance profiles. If experimental is enabled we override this and add the experimental profiles.
1919
SKIP_TESTS =
2020
CEL_TEST_TARGET =
21-
INFERENCE_SUPPORTED_FEATURES = GatewayFollowingEPPRouting
22-
INFERENCE_SKIP_TESTS = InferencePoolResolvedRefsCondition, EppUnAvailableFailOpen,HTTPRouteInvalidInferencePoolRef,InferencePoolAccepted,HTTPRouteMultipleGatewaysDifferentPools,HTTPRouteMultipleRulesDifferentPools,InferencePoolHTTPRoutePortValidation,InferencePoolInvalidEPPService
21+
INFERENCE_SUPPORTED_FEATURES = GatewayFollowingEPPRouting,EppUnAvailableFailOpen,HTTPRouteInvalidInferencePoolRef,InferencePoolAccepted,HTTPRouteMultipleGatewaysDifferentPools,HTTPRouteMultipleRulesDifferentPools,InferencePoolHTTPRoutePortValidation,InferencePoolInvalidEPPService
22+
INFERENCE_SKIP_TESTS = InferencePoolResolvedRefsCondition
2323

2424
# Check if ENABLE_EXPERIMENTAL is true
2525
ifeq ($(ENABLE_EXPERIMENTAL),true)
@@ -188,7 +188,7 @@ add-local-ip-to-cluster: ## Add local IP to the GKE cluster master-authorized-ne
188188
update-firewall-with-local-ip: ## Update the firewall rule with local IP address
189189
./scripts/update-firewall-with-local-ip.sh
190190

191-
HELM_PARAMETERS += --set nginxGateway.name=nginx-gateway --set nginx.service.type=ClusterIP --skip-schema-validation --set nginxGateway.gwAPIInferenceExtension.enable=$(ENABLE_INFERENCE_EXTENSION) --set nginxGateway.config.logging.level=debug
191+
HELM_PARAMETERS += --set nginxGateway.name=nginx-gateway --set nginx.service.type=ClusterIP --set nginxGateway.gwAPIInferenceExtension.enable=$(ENABLE_INFERENCE_EXTENSION) --set nginxGateway.config.logging.level=debug
192192

193193
# this target is used to install the gateway-api CRDs from the main branch (only used in the nightly CI job)
194194
# it overrides the target in the main Makefile when the GW_API_VERSION is set to main

‎tests/conformance/conformance_test.go‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ limitations under the License.
1818
package conformance
1919

2020
import (
21-
"fmt"
2221
"os"
2322
"testing"
2423

@@ -96,7 +95,6 @@ func TestConformance(t *testing.T) {
9695
}
9796

9897
func TestInferenceExtensionConformance(t *testing.T) {
99-
g := NewWithT(t)
10098

10199
t.Logf(`Running inference conformance tests with %s GatewayClass\n cleanup: %t\n`+
102100
`debug: %t\n enable all features: %t \n supported extended features: [%v]\n exempt features: [%v]\n`+
@@ -106,9 +104,6 @@ func TestInferenceExtensionConformance(t *testing.T) {
106104
)
107105

108106
opts := inference_conformance.DefaultOptions(t)
109-
ipaddressType := v1.IPAddressType
110-
opts.UnusableNetworkAddresses = []v1beta1.GatewaySpecAddress{{Type: &ipaddressType, Value: unusableGatewayIPAddress}}
111-
opts.UsableNetworkAddresses = []v1beta1.GatewaySpecAddress{{Type: &ipaddressType, Value: "192.0.2.1"}}
112107

113108
opts.Implementation = conf_v1.Implementation{
114109
Organization: "nginx",
@@ -120,12 +115,6 @@ func TestInferenceExtensionConformance(t *testing.T) {
120115
},
121116
}
122117

123-
_, err := os.Stat(inferenceBaseManifest)
124-
g.Expect(err).ToNot(HaveOccurred(), fmt.Sprintf("base manifest file %s not found", inferenceBaseManifest))
125-
126-
opts.ManifestFS = append(opts.ManifestFS, os.DirFS("."))
127-
opts.BaseManifests = inferenceBaseManifest
128-
129118
opts.ConformanceProfiles.Insert(inference_conformance.GatewayLayerProfileName)
130119
inference_conformance.RunConformanceWithOptions(t, opts)
131120
}

0 commit comments

Comments
 (0)