Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(kong): allow using templates when specifying controller.proxy.nameOverride #914

Merged
merged 1 commit into from
Oct 26, 2023

Conversation

pmalek
Copy link
Member

@pmalek pmalek commented Oct 26, 2023

What this PR does / why we need it:

When using the ingress chart
we have to specify the proxy name override

nameOverride: kong-gateway-proxy

in order to get controller's CONTROLLER_PUBLISH_SERVICE set properly.

Currently the override is set to kong-gateway-proxy which works only when the release name (set by the user) is kong. When a different release name is chosen users have to know what to change and how to change it, e.g. when using a release name kong123 controller.proxy.nameOverride has to be set to kong123-gateway-proxy.

Without this change installing the ingress chart using kong123 release name yields the following controller env variables:

- name: CONTROLLER_KONG_ADMIN_SVC
  value: kong/kong123-gateway-admin
- name: CONTROLLER_PUBLISH_SERVICE
  value: kong/kong-gateway-proxy

With this change:

- name: CONTROLLER_KONG_ADMIN_SVC
  value: kong/kong123-gateway-admin
- name: CONTROLLER_PUBLISH_SERVICE
  value: kong/kong123-gateway-proxy

without the user specifying the override themselves.

Release kong/kong chart 2.30.0.

@pmalek pmalek added the enhancement New feature or request label Oct 26, 2023
@pmalek pmalek self-assigned this Oct 26, 2023
@pmalek pmalek requested a review from a team as a code owner October 26, 2023 10:52
@pmalek pmalek force-pushed the allow-templating-controller-publish-service branch from aeca61e to b19465a Compare October 26, 2023 10:53
Copy link
Contributor

@czeslavo czeslavo left a comment

Choose a reason for hiding this comment

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

👍

@pmalek
Copy link
Member Author

pmalek commented Oct 26, 2023

Holding this for @mheap's review

@mheap
Copy link
Member

mheap commented Oct 26, 2023

LGTM. Should we deprecate gatewayDiscovery.generateAdminApiService to use tpl for admin service discovery too?

@pmalek
Copy link
Member Author

pmalek commented Oct 26, 2023

LGTM. Should we deprecate gatewayDiscovery.generateAdminApiService to use tpl for admin service discovery too?

I'm not sure but even if that should be a separate PR.

@pmalek pmalek merged commit 1308d1d into main Oct 26, 2023
23 checks passed
@pmalek pmalek deleted the allow-templating-controller-publish-service branch October 26, 2023 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants