Skip to content

Commit c7e008f

Browse files
committed
Refactors Code for Service AppProtocol
Signed-off-by: Daneyon Hansen <daneyon.hansen@solo.io>
1 parent 9766902 commit c7e008f

10 files changed

Lines changed: 220 additions & 121 deletions

File tree

pkg/i2gw/implementations/kgateway/README.md

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -87,12 +87,13 @@ The command should generate Gateway API and Kgateway resources.
8787
- `nginx.ingress.kubernetes.io/service-upstream`: When set to `"true"`, configures Kgateway to route to the Service’s cluster IP (or equivalent static host) instead of individual Pod IPs. For each covered Service, the emitter creates a `Backend` resource with `spec.type: Static` and rewrites the corresponding `HTTPRoute.spec.rules[].backendRefs[]` to reference that `Backend` (group `gateway.kgateway.dev`, kind `Backend`).
8888
- `nginx.ingress.kubernetes.io/backend-protocol`: Indicates the L7 protocol that is used to communicate with the proxied backend.
8989
- **Supported values (mapped):** `GRPC`, `GRPCS`
90-
- The emitter creates (or reuses) a `Backend` with `spec.type: Static` and sets `spec.static.appProtocol: grpc`.
91-
- Matching `HTTPRoute.spec.rules[].backendRefs[]` are rewritten to reference this `Backend` (group `gateway.kgateway.dev`, kind `Backend`).
90+
- If `service-upstream: "true"` is also set for the same Service backend, the emitter sets `spec.static.appProtocol: grpc` on the generated `Backend`.
91+
- Otherwise, the emitter does **not** create or modify Kubernetes `Service` resources. Instead, it emits an **INFO** notification with a `kubectl patch`
92+
command to update the existing Service port with `appProtocol: grpc`.
9293
- **Values treated as default HTTP/1.x (no-op):** `HTTP`, `HTTPS`, `AUTO_HTTP`
9394
- **Unsupported values (rejected by provider):** `FCGI` (and others)
94-
- **Independent from features that create a `Backend`:** For example, `backend-protocol` does **not** require `service-upstream: "true"`. If `backend-protocol` is set to `GRPC/GRPCS`,
95-
the provider will still produce the needed static backend metadata so the emitter can create `Backend` resources and rewrite `backendRefs`.
95+
- **Safety note:** Because emitting Service manifests could overwrite user-managed Service configuration, ingress2gateway intentionally avoids generating
96+
Service resources for this annotation.
9697

9798
### External Auth
9899

@@ -194,21 +195,19 @@ Currently supported:
194195
- `spec.type: Static`
195196
- `spec.static.hosts` containing a single `{host, port}` entry derived from the Service (e.g. `myservice.default.svc.cluster.local:80`).
196197
- Matching `HTTPRoute.spec.rules[].backendRefs[]` are rewritten to reference this `Backend` instead of the core Service.
197-
- `nginx.ingress.kubernetes.io/backend-protocol` (partial support):
198-
- When set to `GRPC` or `GRPCS`, the provider emits backend protocol metadata and the emitter ensures there is a `Backend` with:
199-
- `spec.type: Static`
200-
- `spec.static.hosts` containing a single `{host, port}` entry derived from the Service
201-
- `spec.static.appProtocol: grpc`
202-
- Matching `HTTPRoute.spec.rules[].backendRefs[]` are rewritten to reference this `Backend` instead of the core Service.
203-
- **Note:** `HTTP`, `HTTPS`, and `AUTO_HTTP` are treated as default HTTP/1.x behavior and do not emit additional config.
198+
- `nginx.ingress.kubernetes.io/backend-protocol`:
199+
- When set to `GRPC` or `GRPCS` **and** `service-upstream: "true"` is set for the same backend, the emitter stamps `spec.static.appProtocol: grpc` on the generated `Backend`.
200+
- When set to `GRPC` or `GRPCS` **without** `service-upstream: "true"`, the emitter emits an **INFO** notification that includes a `kubectl patch service ...`
201+
command to set `spec.ports[].appProtocol` on the existing Service.
202+
- `HTTP`, `HTTPS`, and `AUTO_HTTP` are treated as default HTTP/1.x behavior and do not emit additional config.
204203

205204
### Summary of Policy Types
206205

207206
| Annotation Type | Kgateway Resource |
208207
|------------------------------------|-----------------------|
209208
| Request/response behavior | `TrafficPolicy` |
210209
| Upstream connection behavior | `BackendConfigPolicy` |
211-
| Upstream representation/protocol | `Backend` |
210+
| Upstream representation. | `Backend` |
212211
| TLS passthrough | `TLSRoute` |
213212

214213
## Limitations
Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
/*
2+
Copyright 2023 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package kgateway
18+
19+
import (
20+
"fmt"
21+
22+
"github.com/kgateway-dev/ingress2gateway/pkg/i2gw/intermediate"
23+
"github.com/kgateway-dev/ingress2gateway/pkg/i2gw/notifications"
24+
"k8s.io/apimachinery/pkg/types"
25+
)
26+
27+
// backendProtoPatchKey de-dupes notifications across rules/routes/policies.
28+
type backendProtoPatchKey struct {
29+
Namespace string
30+
Service string
31+
Port int32
32+
AppProtocol string
33+
}
34+
35+
// emitBackendProtocolPatchNotifications emits an INFO notification with the correct kubectl patch
36+
// command to set ServicePort.appProtocol on an existing Service.
37+
//
38+
// IMPORTANT:
39+
// - We intentionally do NOT emit a Service object to avoid overwriting user-managed Service config.
40+
// - We also skip backends that have been rewritten to a kgateway Backend (service-upstream case),
41+
// because the generated Backend will carry appProtocol instead.
42+
func emitBackendProtocolPatchNotifications(
43+
pol intermediate.Policy,
44+
sourceIngressName string,
45+
httpRouteKey types.NamespacedName,
46+
httpCtx intermediate.HTTPRouteContext,
47+
seen map[backendProtoPatchKey]struct{},
48+
) {
49+
if pol.BackendProtocol == nil {
50+
return
51+
}
52+
53+
// Map ingress-nginx backend-protocol → ServicePort.appProtocol
54+
var appProto string
55+
switch *pol.BackendProtocol {
56+
case intermediate.BackendProtocolGRPC:
57+
appProto = "grpc"
58+
default:
59+
// Nothing to do for unsupported/unknown mappings.
60+
return
61+
}
62+
63+
for _, idx := range pol.RuleBackendSources {
64+
if idx.Rule >= len(httpCtx.Spec.Rules) {
65+
continue
66+
}
67+
rule := httpCtx.Spec.Rules[idx.Rule]
68+
if idx.Backend >= len(rule.BackendRefs) {
69+
continue
70+
}
71+
72+
br := rule.BackendRefs[idx.Backend]
73+
74+
// If already rewritten to a kgateway Backend, skip; appProtocol will be applied there.
75+
if br.BackendRef.Group != nil && *br.BackendRef.Group != "" {
76+
continue
77+
}
78+
if br.BackendRef.Kind != nil && *br.BackendRef.Kind != "" && *br.BackendRef.Kind != "Service" {
79+
continue
80+
}
81+
if br.BackendRef.Name == "" || br.BackendRef.Port == nil {
82+
continue
83+
}
84+
85+
svcName := string(br.BackendRef.Name)
86+
// If service-upstream is enabled for this backend, the emitter will generate a
87+
// kgateway Backend with appProtocol, so do not suggest patching the Service.
88+
if len(pol.Backends) > 0 {
89+
if _, ok := pol.Backends[backendKeyForService(httpRouteKey.Namespace, svcName)]; ok {
90+
continue
91+
}
92+
}
93+
94+
port := int32(*br.BackendRef.Port)
95+
96+
key := backendProtoPatchKey{
97+
Namespace: httpRouteKey.Namespace,
98+
Service: svcName,
99+
Port: port,
100+
AppProtocol: appProto,
101+
}
102+
if _, ok := seen[key]; ok {
103+
continue
104+
}
105+
seen[key] = struct{}{}
106+
107+
// Use strategic merge patch (safe for core types) so only the matching port entry is updated.
108+
// This is far safer than emitting a Service manifest that users might blindly apply.
109+
patch := fmt.Sprintf(`{"spec":{"ports":[{"port":%d,"appProtocol":"%s"}]}}`, port, appProto)
110+
cmd := fmt.Sprintf(
111+
"kubectl patch service %s -n %s --type=strategic -p '%s'",
112+
svcName,
113+
httpRouteKey.Namespace,
114+
patch,
115+
)
116+
117+
msg := fmt.Sprintf(
118+
`Ingress %q uses nginx.ingress.kubernetes.io/backend-protocol=%q for Service %s/%s port %d.
119+
120+
To avoid overwriting existing Service configuration, ingress2gateway does not emit a Service for this annotation.
121+
Apply the equivalent behavior by patching your existing Service port's appProtocol:
122+
123+
%s`,
124+
sourceIngressName,
125+
*pol.BackendProtocol,
126+
httpRouteKey.Namespace,
127+
svcName,
128+
port,
129+
cmd,
130+
)
131+
132+
notifications.NotificationAggr.DispatchNotification(
133+
notifications.NewNotification(notifications.InfoNotification, msg),
134+
"ingress-nginx",
135+
)
136+
}
137+
}

pkg/i2gw/implementations/kgateway/emitter.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,9 @@ func (e *Emitter) Emit(ir *intermediate.IR) ([]client.Object, error) {
8484
// Track Backends per (namespace, svcName) for backend-dependent features, i.e. service-upstream.
8585
backends := map[types.NamespacedName]*kgateway.Backend{}
8686

87+
// De-dupe backend-protocol patch notifications by (ns, svc, port, appProtocol).
88+
backendProtoPatchSeen := map[backendProtoPatchKey]struct{}{}
89+
8790
for httpRouteKey, httpRouteContext := range ir.HTTPRoutes {
8891
ingx := httpRouteContext.ProviderSpecificIR.IngressNginx
8992
if ingx == nil {
@@ -169,6 +172,17 @@ func (e *Emitter) Emit(ir *intermediate.IR) ([]client.Object, error) {
169172
backendCfg,
170173
)
171174

175+
// backend-protocol: do NOT emit/patch Services.
176+
// Instead, emit an INFO notification with a safe kubectl patch command for the user
177+
// (and skip when service-upstream rewrote the backendRef to a kgateway Backend).
178+
emitBackendProtocolPatchNotifications(
179+
pol,
180+
polSourceIngressName,
181+
httpRouteKey,
182+
httpRouteContext,
183+
backendProtoPatchSeen,
184+
)
185+
172186
// Apply service-upstream via Backend and HTTPRoute backendRef rewrites.
173187
applyServiceUpstream(
174188
pol,

pkg/i2gw/implementations/kgateway/service_upstream.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,14 +85,22 @@ func applyServiceUpstream(
8585
continue
8686
}
8787

88+
// service-upstream IR Backends don't currently carry protocol; backend-protocol
89+
// is stored on the Policy. Prefer the per-backend protocol if present, otherwise
90+
// fall back to the Policy-level protocol.
91+
proto := be.Protocol
92+
if proto == nil {
93+
proto = pol.BackendProtocol
94+
}
95+
8896
// Ensure a Kgateway Backend exists with the correct host/port.
8997
kb := ensureStaticBackendForService(
9098
ingressName,
9199
httpRouteKey,
92100
svcName,
93101
be.Host,
94102
be.Port,
95-
be.Protocol,
103+
proto,
96104
backends,
97105
)
98106

pkg/i2gw/implementations/kgateway/testing/testdata/input/backend_protocol.yaml

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ kind: Ingress
33
metadata:
44
annotations:
55
ingress2gateway.kubernetes.io/implementation: kgateway
6-
nginx.ingress.kubernetes.io/backend-protocol: "GRPC"
6+
nginx.ingress.kubernetes.io/backend-protocol: "GRPC" # A notification should be emitted for this
77
name: ingress-myservicea1
88
namespace: default
99
spec:
@@ -20,6 +20,7 @@ spec:
2020
path: /
2121
pathType: Prefix
2222
---
23+
# No notification should be emitted since backend-protocol is not set here
2324
apiVersion: networking.k8s.io/v1
2425
kind: Ingress
2526
metadata:
@@ -46,7 +47,8 @@ kind: Ingress
4647
metadata:
4748
annotations:
4849
ingress2gateway.kubernetes.io/implementation: kgateway
49-
nginx.ingress.kubernetes.io/service-upstream: "true" # Adding service-upstream here should not interfere with backend-protocol
50+
# Adding service-upstream here should generate a Backend CR and rewrite the HTTPRoute backendRef
51+
nginx.ingress.kubernetes.io/service-upstream: "true"
5052
nginx.ingress.kubernetes.io/backend-protocol: "GRPC"
5153
name: ingress-myserviceb
5254
namespace: default

pkg/i2gw/implementations/kgateway/testing/testdata/output/backend_protocol.yaml

Lines changed: 2 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -31,18 +31,13 @@ spec:
3131
parentRefs:
3232
- name: nginx
3333
rules:
34-
# From ingress-myservicea1 (service-upstream=true, backend-protocol=GRPC)
35-
# so rewritten to Backend, which has appProtocol=grpc.
3634
- backendRefs:
37-
- group: gateway.kgateway.dev
38-
kind: Backend
39-
name: myservicea-service-upstream
35+
- name: myservicea
36+
port: 80
4037
matches:
4138
- path:
4239
type: PathPrefix
4340
value: /
44-
# From ingress-myservicea2 (no service-upstream/backend-protocol)
45-
# so still core Service.
4641
- backendRefs:
4742
- name: myservicea
4843
port: 80
@@ -66,8 +61,6 @@ spec:
6661
parentRefs:
6762
- name: nginx
6863
rules:
69-
# From ingress-myserviceb (service-upstream=true, backend-protocol=GRPC)
70-
# so rewritten to Backend, which has appProtocol=grpc.
7164
- backendRefs:
7265
- group: gateway.kgateway.dev
7366
kind: Backend
@@ -81,22 +74,6 @@ status:
8174
---
8275
apiVersion: gateway.kgateway.dev/v1alpha1
8376
kind: Backend
84-
metadata:
85-
labels:
86-
ingress2gateway.kubernetes.io/source-ingress: ingress-myservicea1
87-
name: myservicea-service-upstream
88-
namespace: default
89-
spec:
90-
type: Static
91-
static:
92-
hosts:
93-
- host: myservicea.default.svc.cluster.local
94-
port: 80
95-
appProtocol: grpc
96-
status: {}
97-
---
98-
apiVersion: gateway.kgateway.dev/v1alpha1
99-
kind: Backend
10077
metadata:
10178
labels:
10279
ingress2gateway.kubernetes.io/source-ingress: ingress-myserviceb

pkg/i2gw/intermediate/provider_ingressnginx.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,10 @@ type Policy struct {
182182
// BackendTLS defines the backend TLS policy.
183183
BackendTLS *BackendTLSPolicy
184184

185+
// BackendProtocol defines the upstream application protocol to use when communicating with
186+
// backend Services covered by this policy.
187+
BackendProtocol *BackendProtocol
188+
185189
// SSLRedirect indicates whether SSL redirect is enabled, corresponding to
186190
// nginx.ingress.kubernetes.io/ssl-redirect. When true, requests should be
187191
// redirected to HTTPS.

pkg/i2gw/providers/ingressnginx/README.md

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# Ingress Nginx Provider
22

3-
The project supports translating ingress-nginx specific annotations.
3+
The project supports translating ingress-nginx specific annotations. Some annotations may be translated into
4+
implementation-specific resources or user-facing notifications depending on the selected implementation.
45

56
## Ingress Class Name
67

@@ -85,6 +86,22 @@ The ingress-nginx provider currently supports translating the following annotati
8586

8687
---
8788

89+
### Backend Protocol
90+
91+
- `nginx.ingress.kubernetes.io/backend-protocol`: Indicates the L7 protocol that is used to communicate with the proxied backend.
92+
- **Supported values (recorded):** `GRPC`, `GRPCS`
93+
- The provider records protocol intent as policy metadata (used by implementation emitters).
94+
- For the Kgateway implementation:
95+
- If `service-upstream: "true"` is also enabled for the same Service backend, the Kgateway emitter stamps `spec.static.appProtocol: grpc`
96+
on the generated `Backend`.
97+
- Otherwise, the Kgateway emitter does **not** generate Kubernetes `Service` resources. Instead, it emits an **INFO** notification with a `kubectl patch`
98+
command to set `spec.ports[].appProtocol` on the existing Service.
99+
- **Values treated as default HTTP/1.x (no-op):** `HTTP`, `HTTPS`, `AUTO_HTTP`
100+
- **Unsupported values (rejected):** `FCGI` (and others)
101+
- **Safety note:** The provider does not attempt to create or mutate Kubernetes Services; implementation emitters decide how to safely project this intent.
102+
103+
---
104+
88105
### Backend (Upstream) Configuration
89106

90107
- `nginx.ingress.kubernetes.io/proxy-connect-timeout`: Controls the upstream connection timeout. For the Kgateway implementation,

0 commit comments

Comments
 (0)