fix(envoy-gateway): use EnvoyProxy for the LB-adoption annotation, not global config - #24
Merged
Merged
Conversation
…t global config Confirmed by direct testing on both platform and stage: the global EnvoyGateway config's provider.kubernetes.envoyService.annotations (what this chart relied on) is NOT applied to the per-Gateway Service at all. Restarting the controller with correct config in that field still produced a Service with no adoption annotation, which DigitalOcean's cloud-controller-manager then bound to a brand new, untracked load balancer instead - it happened on every attempt, on both clusters. Per Envoy Gateway's own docs (https://gateway.envoyproxy.io/docs/tasks/operations/customize-envoyproxy/), Service-level customization requires an EnvoyProxy resource referenced through the GatewayClass's parametersRef. Added templates/envoyproxy.yaml and wired it in; moved the per-cluster override value from gateway-helm.config.envoyGateway.provider.kubernetes.envoyService.annotations to gatewayClass.envoyServiceAnnotations, which now actually reaches the Service that matters. This bug existed from the chart's very first version - it just never got exercised until a Gateway actually became Accepted for the first time today.
Helm template diffdiff -u --recursive --label base --label head base head
--- base
+++ head
@@ -37,8 +37,6 @@
default: info
provider:
kubernetes:
- envoyService:
- annotations: {}
rateLimitDeployment:
container:
image: docker.io/envoyproxy/ratelimit:ff287602
@@ -512,11 +510,31 @@
secretName: envoy-gateway
---
+# Source: envoy-gateway/templates/envoyproxy.yaml
+# Per-Gateway Service customization (like the LB-adoption annotation) is
+# only honored via an EnvoyProxy resource referenced through the
+# GatewayClass's parametersRef (see templates/gatewayclass.yaml) - the
+# global EnvoyGateway config does not apply here, confirmed by testing.
+apiVersion: gateway.envoyproxy.io/v1alpha1
+kind: EnvoyProxy
+metadata:
+ name: envoy-gateway
+ namespace: default
+spec:
+ provider:
+ type: Kubernetes
+ kubernetes:
+ envoyService:
+ annotations:
+ {}
+
+---
# Source: envoy-gateway/templates/gateway.yaml
# The Service (and adopted DO load balancer) Envoy Gateway provisions for
-# this Gateway gets its annotations from gateway-helm's own
-# config.envoyGateway.provider.kubernetes.envoyService.annotations - set
-# per-cluster in clusters/<name>/overrides/envoy-gateway.yaml.
+# this Gateway gets its annotations from the EnvoyProxy resource
+# (templates/envoyproxy.yaml) referenced via this GatewayClass's
+# parametersRef - set per-cluster in
+# clusters/<name>/overrides/envoy-gateway.yaml.
apiVersion: gateway.networking.k8s.io/v1
kind: Gateway
metadata:
@@ -557,6 +575,11 @@
name: envoy-gateway
spec:
controllerName: gateway.envoyproxy.io/gatewayclass-controller
+ parametersRef:
+ group: gateway.envoyproxy.io
+ kind: EnvoyProxy
+ name: envoy-gateway
+ namespace: default
---
# Source: envoy-gateway/charts/gateway-helm/charts/crds/templates/gatewayapi-safe-upgrade-policy.yaml
diff -u --recursive --label base --label head base head
--- base
+++ head
@@ -218,7 +218,7 @@
app.kubernetes.io/managed-by: Helm
type: Opaque
data:
- SECRET_KEY_BASE: OTFPaHRIWWFNaWIyZGtzUFNkdU9EZ1BDSE50elNTTTFhRVlmVU10bmVRNVpMRVFPM0RzYkZqdUx3MjhYdkJNejJ2RHl5Qk8yd1JvUFJRd0JxTzBISTczSEth
+ SECRET_KEY_BASE: SEFlNkJiZTNZdWx2d214TkRHMzRrMVR0S09tRXVGNVBkV2huZXhCV3VuUE90NXd4UTBqVWpnMEdiMkF0Wklva1gyQmFndkc1eFRqWXRuYjNtNUR6SExNMVF5
TOTP_VAULT_KEY: ZHN4dmJuM2p4RGQxNmF6MlFwc1g1QjhPK2xseGpRMlNKRTJpNUJ6eDM4ST0=
DATABASE_URL: cG9zdGdyZXM6Ly9wb3N0Z3Jlczpwb3N0Z3Jlc0BwbGF1c2libGUtYW5hbHl0aWNzLXBvc3RncmVzcWw6NTQzMi9wbGF1c2libGVfZGI=
CLICKHOUSE_DATABASE_URL: aHR0cDovL2NsaWNraG91c2U6cGFzc3dvcmRAcGxhdXNpYmxlLWFuYWx5dGljcy1jbGlja2hvdXNlOjgxMjMvcGxhdXNpYmxlX2V2ZW50c19kYg== |
…ange
An empty map through {{- range }} produces zero output lines, leaving
`annotations:` with nothing after it - YAML parses that as null, and
the EnvoyProxy CRD's schema requires an object. Caught by CI's
kubeconform check on the default (no cluster override) case. toYaml
correctly renders {} for an empty map.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes DigitalOcean load balancer “adoption” for Envoy Gateway by moving the per-Service annotation configuration from the (non-applicable) global EnvoyGateway config into an EnvoyProxy resource that is referenced by GatewayClass.spec.parametersRef, ensuring the annotation actually reaches the per-Gateway Service Envoy Gateway creates.
Changes:
- Add a chart-managed
EnvoyProxyresource that carries per-Gateway Service annotations. - Wire
GatewayClass.spec.parametersRefto reference thatEnvoyProxy. - Update per-cluster overrides to set the LB-adoption annotation via
gatewayClass.envoyServiceAnnotations.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| clusters/stage-do-atl1/overrides/envoy-gateway.yaml | Moves DO LB adoption annotation override to gatewayClass.envoyServiceAnnotations. |
| clusters/platform-do-atl1/overrides/envoy-gateway.yaml | Moves DO LB adoption annotation override to gatewayClass.envoyServiceAnnotations. |
| charts/infrastructure/envoy-gateway/values.yaml | Removes ineffective global config annotations; introduces gatewayClass.envoyServiceAnnotations with updated rationale. |
| charts/infrastructure/envoy-gateway/templates/gatewayclass.yaml | Adds parametersRef to point to the chart-managed EnvoyProxy. |
| charts/infrastructure/envoy-gateway/templates/gateway.yaml | Updates comments to reflect the new annotation source (EnvoyProxy via parametersRef). |
| charts/infrastructure/envoy-gateway/templates/envoyproxy.yaml | Adds EnvoyProxy manifest that applies envoyService.annotations from values. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
envoy-gateway's Gateway finally reached Accepted after #23, and its controller created a per-Gateway Service with no LB-adoption annotation. DigitalOcean's cloud-controller-manager provisioned a new, untracked LB instead of adopting the real one, on both clusters.
Assumed at first this was a stale controller pod (stage's had been running 37h) and restarted both. Same result - a second stray LB each time. Ruled out staleness.
Root cause: the LB-adoption annotation was set under the global EnvoyGateway config (
provider.kubernetes.envoyService.annotations), which renders fine but isn't actually applied to the Service. Per Envoy Gateway's docs, Service customization requires anEnvoyProxyresource referenced via the GatewayClass'sparametersRef. This bug's been in the chart since day one, it just had no way to surface until a Gateway actually got Accepted today.Fix:
templates/envoyproxy.yaml.GatewayClass.parametersRefto it.gatewayClass.envoyServiceAnnotations.Two stray LBs are still live from the failed attempts. Once this merges, their Services need deleting one more time so they get recreated correctly.