feat(helm): Add global imageRegistry and imagePullSecrets overrides

[pasha-gurkov]

What this PR does / why we need it:
The PR enables global container registry and pullSecrets overrides which makes it easier to use Envoy Gateway with private container registries.

Which issue(s) this PR fixes:
Fixes #5718

Release Notes: Yes

Detailed description:

Add global.imageRegistry and global.imagePullSecrets to gateway-helm chart values.

Add override logic on top: global.imageRegistry and global.imagePullSecrets take precedence if defined, then deployment-specific values, then global.images values. This ensures changes are backwards-compatible with the logic before.

Add new ENV vars: REPOSITORY, RATELIMIT_REPOSITORY, RATELIMIT_TAG -- REPOSITORY is added to not overcomplicate Makefiles with parsing logic that would be required to stay fully compatible with the current ways (like issuing IMAGE=xyz make image); RATELIMIT_REPOSITORY and RATELIMIT_TAG just felt logical to add to make things consistent and configurable in the same way.

Delete ENV vars: IMAGE_NAME, IMAGE -- these are replaced with REGISTRY and REPOSITORY, if default registry is used, just REPOSITORY suffices. This changes developer-side behaviour though, but I'm not sure what's better -- Makefile madness or this. Also, this maps better to values.yaml.

[pasha-gurkov]

@arkodg I pushed the backwards-compatible version. It now has some additional parsing inside _helpers.tpl, and the overriding logic became somewhat hairy -- if global.imageRegistry is defined, it overrides both global.images and deployment. Regarding my comments for ENV variables, if you'd rather return those to how they were, I'm not opposed to it - they no longer map to much.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.51%. Comparing base (096cb8d) to head (eccc41b).
Report is 10 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5719      +/-   ##
==========================================
+ Coverage   65.19%   65.51%   +0.31%     
==========================================
  Files         214      214              
  Lines       34321    34394      +73     
==========================================
+ Hits        22377    22533     +156     
+ Misses      10591    10499      -92     
- Partials     1353     1362       +9     

☔ 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.

[arkodg]

thanks @pasha-gurkov,

• can we scope the changes in this PR to only helm charts, else it will take much longer for a review, because those Makefile changes, affect CI and releases and multiple other callers
• should it be global.images.registry and global.images.pullSecrets instead, wdyt @envoyproxy/gateway-maintainers @envoyproxy/gateway-reviewers
• and global.images.envoyGateway.image should have higher precedence over global.images.registry since its most specific

[pasha-gurkov]

Thanks for the comment @arkodg ,

can we scope the changes in this PR to only helm charts, else it will take much longer for a review

Sure thing, I'll push it later today.

should it be global.images.registry and global.images.pullSecrets

My 2 cents is it should be global.imageRegistry as that's what bitnami charts use. This should give the most win for the end-users (single override working everywhere), as bitnami charts are very popular.

and global.images.envoyGateway.image should have higher precedence over global.images.registry since its most specific

This would render the PR useless. The point of this is to be able to override registry somewhere at the top level and let it propagate downstream, with minimal possible changes to the downstream charts. gateway-specific values taking precedence would mean one would still have to override global.images.envoyGateway.image or deployment.envoyGateway.image, since they're both more specific than global.

Globals overriding specific values is a common practice for helm globals, please take a look at https://github.com/bitnami/charts/blob/0f3c1ccdeefe6e244c9ff9577e06b6c7e9b4bb15/bitnami/common/templates/_images.tpl#L13 or https://github.com/grafana/agent/blob/18b357cd9cd0bbc5946b73e2ca82ad237c2d52d3/operations/helm/charts/grafana-agent/templates/containers/_agent.yaml#L3 .

If end-user wants to opt-out, and e.g. use their private registry for every image but envoy-gateway, they can override a chart-specific global:

gateway-helm:
  global:
    imageRegistry: ""

[pasha-gurkov]

can we scope the changes in this PR to only helm charts, else it will take much longer for a review

Sure thing, I'll push it later today.

Reverted all Makefile and other relevant changes.

[arkodg]

thanks @pasha-gurkov, the reasoning for top level overriding the specific images makes sense (by default the specifics are set today in the helm chart)

[pasha-gurkov]

@arkodg thanks for the feedback! Please check the latest commit, should have addressed all of it.

[arkodg]

LGTM thanks !

[pasha-gurkov]

Thank you very much @arkodg!