Skip to content

Conversation

@blacksd
Copy link

@blacksd blacksd commented Dec 4, 2025

What type of PR is this?
Bug fix

What this PR does / why we need it:
This PR fixes an issue where the global.imagePullSecrets configuration in the gateway-helm chart was not being applied to dynamically created Envoy Proxy deployments. While the global imagePullSecrets setting was correctly applied to the Envoy Gateway controller deployment and rate limit deployment, it was missing from the Envoy Proxy deployments that are created when Gateway resources are instantiated.

Background:
The gateway-helm chart follows a consistent pattern where global configuration parameters (like global.imagePullSecrets and global.imageRegistry) are propagated to all component deployments:

  • Envoy Gateway controller deployment: Uses global.imagePullSecrets or deployment.envoyGateway.imagePullSecrets or global.images.envoyGateway.pullSecrets
  • Rate limit deployment: Uses global.imagePullSecrets or global.images.ratelimit.pullSecrets
  • Envoy Proxy deployments (fixed by this PR): Now uses global.imagePullSecrets

Changes:

  • Updated eg.default-envoy-gateway-config template to include envoyDeployment.pod.imagePullSecrets configuration
  • This configuration is applied through the EnvoyGateway provider settings, which control how the Envoy Gateway controller creates Envoy Proxy deployments
  • Added release note documenting the fix

Impact:
Users deploying Envoy Gateway in environments with private container registries can now set global.imagePullSecrets once, and it will be correctly applied to all components including the dynamically created Envoy Proxy deployments. This ensures consistent behavior across all chart-managed resources.

Which issue(s) this PR fixes:
Fixes #

Release Notes: Yes

@blacksd blacksd requested a review from a team as a code owner December 4, 2025 15:58
Comment on lines +159 to +164
{{- if .Values.global.imagePullSecrets }}
envoyDeployment:
pod:
imagePullSecrets:
{{- toYaml .Values.global.imagePullSecrets | nindent 10 }}
{{- end }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure these get mapped to the EnvoyGateway CRD and configmap which doesn't support these fields - https://gateway.envoyproxy.io/docs/api/extension_types/#envoygatewaykubernetesprovider

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The image pull secrets defined in the helm chart get applied to the envoy-gateway controller deployment but this helm chart doesn't manage EnvoyProxy resources which is what you appear to be trying to inject into.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some options:

  • Add the concept of a global EnvoyProxy as a field within kubernetesprovider and then the controller would default to this instead of empty. It would then follow an inheritance chain like other resources and values from separate EnvoyProxy resources would override
  • If your only concern is imagePullSecrets, perhaps a new global field could be added under kubernetesprovider which then gets checked by the relevant resources during provisioning (rate limit, envoyproxy, etc)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this won't be worked before #4764.

Copy link
Author

@blacksd blacksd Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My immediately following concern would have been to address the distroless image that's not overridden with the global image selection.
Thanks for referencing the other issue; perhaps I can help moving that forward.

@codecov
Copy link

codecov bot commented Dec 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.43%. Comparing base (0fa26d7) to head (8171812).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7657   +/-   ##
=======================================
  Coverage   72.43%   72.43%           
=======================================
  Files         232      232           
  Lines       34307    34307           
=======================================
+ Hits        24849    24851    +2     
  Misses       7682     7682           
+ Partials     1776     1774    -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants