Skip to content

Commit 96b0a66

Browse files
mmmommmmmmommm
committed
feat: propagate gateway-name label to infra resources in default mode
Signed-off-by: Kise Ryota <kiseryota.contact@gmail.com>
1 parent 98c1be7 commit 96b0a66

63 files changed

Lines changed: 231 additions & 7 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎internal/infrastructure/kubernetes/proxy/resource_provider.go‎

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ func (r *ResourceRender) ownerReferences() []metav1.OwnerReference {
157157
// ServiceAccount returns the expected proxy serviceAccount.
158158
func (r *ResourceRender) ServiceAccount() (*corev1.ServiceAccount, error) {
159159
// Set the labels based on the owning gateway name.
160-
saLabels := r.envoyLabels(r.infra.GetProxyMetadata().Labels)
160+
saLabels := r.objectLabels(r.infra.GetProxyMetadata().Labels)
161161
if OwningGatewayLabelsAbsent(saLabels) {
162162
return nil, fmt.Errorf("missing owning gateway labels")
163163
}
@@ -179,6 +179,8 @@ func (r *ResourceRender) ServiceAccount() (*corev1.ServiceAccount, error) {
179179
}
180180

181181
// envoyLabels returns the labels, including extraLabels, used for Envoy resources.
182+
// These labels also form the immutable pod selector, so keys must not be added
183+
// here for existing installations.
182184
func (r *ResourceRender) envoyLabels(extraLabels map[string]string) map[string]string {
183185
appLabels := EnvoyAppLabel()
184186
if r.GatewayNamespaceMode {
@@ -189,6 +191,22 @@ func (r *ResourceRender) envoyLabels(extraLabels map[string]string) map[string]s
189191
return appLabels
190192
}
191193

194+
// objectLabels returns the labels for the metadata of generated resources. In
195+
// default mode it also sets the standard gateway-name Gateway API label, which
196+
// is kept out of envoyLabels/stableSelector because the selector is immutable.
197+
// Merged gateways are owned by the GatewayClass, so the per-Gateway label is
198+
// omitted there.
199+
func (r *ResourceRender) objectLabels(extraLabels map[string]string) map[string]string {
200+
labels := r.envoyLabels(extraLabels)
201+
if !r.GatewayNamespaceMode {
202+
if gwName := labels[gatewayapi.OwningGatewayNameLabel]; gwName != "" {
203+
labels[gatewayapi.GatewayNameLabel] = gwName
204+
}
205+
}
206+
207+
return labels
208+
}
209+
192210
// Service returns the expected Service based on the provided infra.
193211
func (r *ResourceRender) Service() (*corev1.Service, error) {
194212
var ports []corev1.ServicePort
@@ -223,7 +241,7 @@ func (r *ResourceRender) Service() (*corev1.Service, error) {
223241
}
224242

225243
// Set the infraLabels based on the owning gatewayclass name.
226-
infraLabels := r.envoyLabels(r.infra.GetProxyMetadata().Labels)
244+
infraLabels := r.objectLabels(r.infra.GetProxyMetadata().Labels)
227245
if OwningGatewayLabelsAbsent(infraLabels) {
228246
return nil, fmt.Errorf("missing owning gateway infraLabels")
229247
}
@@ -254,7 +272,9 @@ func (r *ResourceRender) Service() (*corev1.Service, error) {
254272
// Set the spec of gateway service
255273
serviceSpec := resource.ExpectedServiceSpec(envoyServiceConfig)
256274
serviceSpec.Ports = ports
257-
serviceSpec.Selector = resource.GetSelector(infraLabels).MatchLabels
275+
// The selector must stay stable and must not include gateway-name, so build
276+
// it from envoyLabels rather than the metadata labels (objectLabels).
277+
serviceSpec.Selector = resource.GetSelector(r.envoyLabels(r.infra.GetProxyMetadata().Labels)).MatchLabels
258278

259279
if (*envoyServiceConfig.Type) == egv1a1.ServiceTypeClusterIP {
260280
if len(r.infra.Addresses) > 0 {
@@ -315,7 +335,7 @@ func (r *ResourceRender) Service() (*corev1.Service, error) {
315335
// ConfigMap returns the expected ConfigMap based on the provided infra.
316336
func (r *ResourceRender) ConfigMap(cert string) (*corev1.ConfigMap, error) {
317337
// Set the labels based on the owning gateway name.
318-
cmLabels := r.envoyLabels(r.infra.GetProxyMetadata().Labels)
338+
cmLabels := r.objectLabels(r.infra.GetProxyMetadata().Labels)
319339
if OwningGatewayLabelsAbsent(cmLabels) {
320340
return nil, fmt.Errorf("missing owning gateway labels")
321341
}
@@ -667,7 +687,7 @@ func (r *ResourceRender) getPodAnnotations(resourceAnnotation map[string]string,
667687

668688
func (r *ResourceRender) getLabels() (map[string]string, error) {
669689
// Set the labels based on the owning gateway name.
670-
resourceLabels := r.envoyLabels(r.infra.GetProxyMetadata().Labels)
690+
resourceLabels := r.objectLabels(r.infra.GetProxyMetadata().Labels)
671691
if OwningGatewayLabelsAbsent(resourceLabels) {
672692
return nil, fmt.Errorf("missing owning gateway labels")
673693
}
@@ -676,10 +696,13 @@ func (r *ResourceRender) getLabels() (map[string]string, error) {
676696
}
677697

678698
func (r *ResourceRender) getPodLabels(pod *egv1a1.KubernetesPodSpec) map[string]string {
679-
podLabels := r.infra.GetProxyMetadata().Labels
699+
// Copy into a fresh map so the pod-spec labels are not written back into the
700+
// shared infra metadata map (which also feeds getLabels/stableSelector).
701+
podLabels := map[string]string{}
702+
maps.Copy(podLabels, r.infra.GetProxyMetadata().Labels)
680703
maps.Copy(podLabels, pod.Labels)
681704

682-
return r.envoyLabels(podLabels)
705+
return r.objectLabels(podLabels)
683706
}
684707

685708
// OwningGatewayLabelsAbsent Check if labels are missing some OwningGatewayLabels

‎internal/infrastructure/kubernetes/proxy/resource_provider_test.go‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2126,3 +2126,97 @@ func writeTestDataToFile(filename string, resources []any) error {
21262126

21272127
return os.WriteFile(filename, combinedYAML, 0o600)
21282128
}
2129+
2130+
// TestGatewayNameLabelPropagation verifies the standard Gateway API
2131+
// "gateway.networking.k8s.io/gateway-name" label is propagated to generated
2132+
// resources in default mode, is omitted when gateways are merged, and is never
2133+
// added to the (immutable) stable selector in default mode.
2134+
func TestGatewayNameLabelPropagation(t *testing.T) {
2135+
newRender := func(gatewayNamespaceMode bool, metaLabels map[string]string) *ResourceRender {
2136+
i := ir.NewInfra()
2137+
i.Proxy.Name = "default/eg"
2138+
i.Proxy.GetProxyMetadata().Labels = metaLabels
2139+
return &ResourceRender{
2140+
infra: i.GetProxyInfra(),
2141+
GatewayNamespaceMode: gatewayNamespaceMode,
2142+
}
2143+
}
2144+
2145+
t.Run("default mode, single gateway: label on resources, not on selector", func(t *testing.T) {
2146+
r := newRender(false, map[string]string{
2147+
gatewayapi.OwningGatewayNameLabel: "eg",
2148+
gatewayapi.OwningGatewayNamespaceLabel: "default",
2149+
})
2150+
2151+
objLabels := r.objectLabels(r.infra.GetProxyMetadata().Labels)
2152+
require.Equal(t, "eg", objLabels[gatewayapi.GatewayNameLabel],
2153+
"generated resources must carry the standard gateway-name label")
2154+
2155+
selector := r.stableSelector().MatchLabels
2156+
require.NotContains(t, selector, gatewayapi.GatewayNameLabel,
2157+
"gateway-name must NOT be part of the immutable selector in default mode")
2158+
})
2159+
2160+
t.Run("merged gateways: label omitted", func(t *testing.T) {
2161+
r := newRender(false, map[string]string{
2162+
gatewayapi.OwningGatewayClassLabel: "eg-class",
2163+
})
2164+
2165+
objLabels := r.objectLabels(r.infra.GetProxyMetadata().Labels)
2166+
require.NotContains(t, objLabels, gatewayapi.GatewayNameLabel,
2167+
"merged fleets belong to the GatewayClass; the per-Gateway label must be omitted")
2168+
})
2169+
2170+
t.Run("gateway namespace mode: unchanged (label on resources and selector)", func(t *testing.T) {
2171+
r := newRender(true, map[string]string{
2172+
gatewayapi.OwningGatewayNameLabel: "eg",
2173+
gatewayapi.OwningGatewayNamespaceLabel: "default",
2174+
})
2175+
2176+
objLabels := r.objectLabels(r.infra.GetProxyMetadata().Labels)
2177+
require.Contains(t, objLabels, gatewayapi.GatewayNameLabel)
2178+
2179+
selector := r.stableSelector().MatchLabels
2180+
require.Contains(t, selector, gatewayapi.GatewayNameLabel,
2181+
"gateway namespace mode keeps its existing selector behavior")
2182+
})
2183+
}
2184+
2185+
// TestServiceSelectorGatewayName renders the actual Service, whose selector is
2186+
// built independently of stableSelector, to guard against the gateway-name
2187+
// label leaking into the (default-mode) Service selector.
2188+
func TestServiceSelectorGatewayName(t *testing.T) {
2189+
cfg, err := config.New(os.Stdout, os.Stderr)
2190+
require.NoError(t, err)
2191+
2192+
t.Run("default mode: gateway-name on metadata, not on selector", func(t *testing.T) {
2193+
r, err := NewResourceRender(context.Background(), newFakeKubernetesInfraProvider(cfg), newTestInfra())
2194+
require.NoError(t, err)
2195+
svc, err := r.Service()
2196+
require.NoError(t, err)
2197+
2198+
require.Contains(t, svc.Labels, gatewayapi.GatewayNameLabel,
2199+
"Service metadata must carry the standard gateway-name label")
2200+
require.NotContains(t, svc.Spec.Selector, gatewayapi.GatewayNameLabel,
2201+
"gateway-name must NOT leak into the Service selector in default mode")
2202+
})
2203+
2204+
t.Run("gateway namespace mode: gateway-name on selector", func(t *testing.T) {
2205+
cfg.EnvoyGateway.Provider = &egv1a1.EnvoyGatewayProvider{
2206+
Type: egv1a1.ProviderTypeKubernetes,
2207+
Kubernetes: &egv1a1.EnvoyGatewayKubernetesProvider{
2208+
Deploy: &egv1a1.KubernetesDeployMode{
2209+
Type: new(egv1a1.KubernetesDeployModeTypeGatewayNamespace),
2210+
},
2211+
},
2212+
}
2213+
infra := newTestInfraWithNamespacedName(types.NamespacedName{Namespace: "ns1", Name: "gateway-1"})
2214+
r, err := NewResourceRender(context.Background(), newFakeKubernetesInfraProvider(cfg), infra)
2215+
require.NoError(t, err)
2216+
svc, err := r.Service()
2217+
require.NoError(t, err)
2218+
2219+
require.Contains(t, svc.Spec.Selector, gatewayapi.GatewayNameLabel,
2220+
"gateway namespace mode keeps gateway-name in the Service selector")
2221+
})
2222+
}

‎internal/infrastructure/kubernetes/proxy/testdata/configmap/default.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ metadata:
1010
app.kubernetes.io/name: envoy
1111
gateway.envoyproxy.io/owning-gateway-name: default
1212
gateway.envoyproxy.io/owning-gateway-namespace: default
13+
gateway.networking.k8s.io/gateway-name: default
1314
name: envoy-default-37a8eec1
1415
namespace: envoy-gateway-system
1516
ownerReferences:

‎internal/infrastructure/kubernetes/proxy/testdata/configmap/with-annotations.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ metadata:
1313
app.kubernetes.io/name: envoy
1414
gateway.envoyproxy.io/owning-gateway-name: default
1515
gateway.envoyproxy.io/owning-gateway-namespace: default
16+
gateway.networking.k8s.io/gateway-name: default
1617
name: envoy-default-37a8eec1
1718
namespace: envoy-gateway-system
1819
ownerReferences:

‎internal/infrastructure/kubernetes/proxy/testdata/daemonsets/component-level.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ metadata:
77
app.kubernetes.io/name: envoy
88
gateway.envoyproxy.io/owning-gateway-name: default
99
gateway.envoyproxy.io/owning-gateway-namespace: default
10+
gateway.networking.k8s.io/gateway-name: default
1011
name: envoy-default-37a8eec1
1112
namespace: envoy-gateway-system
1213
ownerReferences:
@@ -34,6 +35,7 @@ spec:
3435
app.kubernetes.io/name: envoy
3536
gateway.envoyproxy.io/owning-gateway-name: default
3637
gateway.envoyproxy.io/owning-gateway-namespace: default
38+
gateway.networking.k8s.io/gateway-name: default
3739
spec:
3840
automountServiceAccountToken: false
3941
containers:

‎internal/infrastructure/kubernetes/proxy/testdata/daemonsets/custom.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ metadata:
77
app.kubernetes.io/name: envoy
88
gateway.envoyproxy.io/owning-gateway-name: default
99
gateway.envoyproxy.io/owning-gateway-namespace: default
10+
gateway.networking.k8s.io/gateway-name: default
1011
name: envoy-default-37a8eec1
1112
namespace: envoy-gateway-system
1213
ownerReferences:
@@ -35,6 +36,7 @@ spec:
3536
foo.bar: custom-label
3637
gateway.envoyproxy.io/owning-gateway-name: default
3738
gateway.envoyproxy.io/owning-gateway-namespace: default
39+
gateway.networking.k8s.io/gateway-name: default
3840
spec:
3941
automountServiceAccountToken: false
4042
containers:

‎internal/infrastructure/kubernetes/proxy/testdata/daemonsets/default-env.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ metadata:
77
app.kubernetes.io/name: envoy
88
gateway.envoyproxy.io/owning-gateway-name: default
99
gateway.envoyproxy.io/owning-gateway-namespace: default
10+
gateway.networking.k8s.io/gateway-name: default
1011
name: envoy-default-37a8eec1
1112
namespace: envoy-gateway-system
1213
ownerReferences:
@@ -34,6 +35,7 @@ spec:
3435
app.kubernetes.io/name: envoy
3536
gateway.envoyproxy.io/owning-gateway-name: default
3637
gateway.envoyproxy.io/owning-gateway-namespace: default
38+
gateway.networking.k8s.io/gateway-name: default
3739
spec:
3840
automountServiceAccountToken: false
3941
containers:

‎internal/infrastructure/kubernetes/proxy/testdata/daemonsets/default.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ metadata:
77
app.kubernetes.io/name: envoy
88
gateway.envoyproxy.io/owning-gateway-name: default
99
gateway.envoyproxy.io/owning-gateway-namespace: default
10+
gateway.networking.k8s.io/gateway-name: default
1011
name: envoy-default-37a8eec1
1112
namespace: envoy-gateway-system
1213
ownerReferences:
@@ -34,6 +35,7 @@ spec:
3435
app.kubernetes.io/name: envoy
3536
gateway.envoyproxy.io/owning-gateway-name: default
3637
gateway.envoyproxy.io/owning-gateway-namespace: default
38+
gateway.networking.k8s.io/gateway-name: default
3739
spec:
3840
automountServiceAccountToken: false
3941
containers:

‎internal/infrastructure/kubernetes/proxy/testdata/daemonsets/disable-prometheus.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ metadata:
77
app.kubernetes.io/name: envoy
88
gateway.envoyproxy.io/owning-gateway-name: default
99
gateway.envoyproxy.io/owning-gateway-namespace: default
10+
gateway.networking.k8s.io/gateway-name: default
1011
name: envoy-default-37a8eec1
1112
namespace: envoy-gateway-system
1213
ownerReferences:
@@ -30,6 +31,7 @@ spec:
3031
app.kubernetes.io/name: envoy
3132
gateway.envoyproxy.io/owning-gateway-name: default
3233
gateway.envoyproxy.io/owning-gateway-namespace: default
34+
gateway.networking.k8s.io/gateway-name: default
3335
spec:
3436
automountServiceAccountToken: false
3537
containers:

‎internal/infrastructure/kubernetes/proxy/testdata/daemonsets/extension-env.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ metadata:
77
app.kubernetes.io/name: envoy
88
gateway.envoyproxy.io/owning-gateway-name: default
99
gateway.envoyproxy.io/owning-gateway-namespace: default
10+
gateway.networking.k8s.io/gateway-name: default
1011
name: envoy-default-37a8eec1
1112
namespace: envoy-gateway-system
1213
ownerReferences:
@@ -34,6 +35,7 @@ spec:
3435
app.kubernetes.io/name: envoy
3536
gateway.envoyproxy.io/owning-gateway-name: default
3637
gateway.envoyproxy.io/owning-gateway-namespace: default
38+
gateway.networking.k8s.io/gateway-name: default
3739
spec:
3840
automountServiceAccountToken: false
3941
containers:

0 commit comments

Comments
 (0)