search

ngrok / kubernetes-ingress-controller

The official ngrok Ingress Controller for Kubernetes
https://ngrok.com
MIT License
183 stars 20 forks source link

Load cluster domain as a config option. Fall back to default if not set #339

Open fr6nco opened 4 months ago

fr6nco commented 4 months ago

…vc.cluster.local if not set

What

Cluster domain is hardcoded. Load the cluster domain from an environment variable. If the environment variable is not set, fall back to the default svc.cluster.local

Currently very simple, just please take a look if this is something you would accept. Test were not written, I could not find anything covering the cluster domain, so it should be added. Helm chart to be updated, where we could set the cluster domain to a configmap or simply passing it as an extraEnvVar: {}

How

Cluster domain is read from env variable.

Breaking Changes

no

salilsub commented 4 months ago

Hi @fr6nco . For cases like this we'd want to use a Helm chart override instead of using an environment variable. If you modified this PR/submitted a new PR using this approach, we'd accept that.

github-actions[bot] commented 4 months ago

:wave: Looks like there are changes in the Helm Chart's Chart.yaml file. Upon merge, a new release of the helm chart will be created if the Chart's version was changed.

github-actions[bot] commented 4 months ago

:wave: Looks like there are changes in the VERSION file. Upon merge, a new release of the docker image will be created. Please make sure the version is updated appropriately.

github-actions[bot] commented 4 months ago

:wave: Looks like there are changes in the Helm Chart's Chart.yaml file. Upon merge, a new release of the helm chart will be created if the Chart's version was changed.

github-actions[bot] commented 4 months ago

:wave: Looks like there are changes in the VERSION file. Upon merge, a new release of the docker image will be created. Please make sure the version is updated appropriately.

github-actions[bot] commented 4 months ago

:wave: Looks like there are changes in the Helm Chart's Chart.yaml file. Upon merge, a new release of the helm chart will be created if the Chart's version was changed.

github-actions[bot] commented 4 months ago

:wave: Looks like there are changes in the VERSION file. Upon merge, a new release of the docker image will be created. Please make sure the version is updated appropriately.

github-actions[bot] commented 4 months ago

:wave: Looks like there are changes in the Helm Chart's Chart.yaml file. Upon merge, a new release of the helm chart will be created if the Chart's version was changed.

github-actions[bot] commented 4 months ago

:wave: Looks like there are changes in the VERSION file. Upon merge, a new release of the docker image will be created. Please make sure the version is updated appropriately.

fr6nco commented 4 months ago

I bit more commints than expected. It was reworked, where the option is passed down as a config option. No longer via an env variable. The option is exposed via the helm chart.

github-actions[bot] commented 4 months ago

:wave: Looks like there are changes in the Helm Chart's Chart.yaml file. Upon merge, a new release of the helm chart will be created if the Chart's version was changed.

github-actions[bot] commented 4 months ago

:wave: Looks like there are changes in the VERSION file. Upon merge, a new release of the docker image will be created. Please make sure the version is updated appropriately.

github-actions[bot] commented 4 months ago

:wave: Looks like there are changes in the Helm Chart's Chart.yaml file. Upon merge, a new release of the helm chart will be created if the Chart's version was changed.

github-actions[bot] commented 4 months ago

:wave: Looks like there are changes in the VERSION file. Upon merge, a new release of the docker image will be created. Please make sure the version is updated appropriately.