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.

Sign up for GitHub

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

Jump to bottom

Helm chart - cannot unset values.router.configuration safely #4781

Open
shaikatzz opened this issue Mar 11, 2024 · 2 comments · May be fixed by #4935
Open

Helm chart - cannot unset values.router.configuration safely #4781

shaikatzz opened this issue Mar 11, 2024 · 2 comments · May be fixed by #4935
Labels
raised by user

Comments

@shaikatzz
Copy link

Describe the bug
I would like to provide my own config.yaml file into the container, and in order to do so i have to make router.configuration empty so it will skip setting --config /app/configuration.yaml.

The problem is that if I set this value to empty, it will fail on service creation because of this line:
index .Values.router.configuration "health_check").listen

With the following error:
Error: template: router/charts/router/templates/service.yaml:21:31: executing "router/charts/router/templates/service.yaml" at <index .Values.router.configuration "health_check">: error calling index: cannot index slice/array with type string

To Reproduce
Steps to reproduce the behavior:
set this chart as a depedency of a new chart you create an run helm template . --set router.router.configuration={}

Expected behavior
should unset configuration safely and let us load our own config file from file system.
@dan-catalano-vc
Copy link

dan-catalano-vc commented Apr 9, 2024 •
edited

I am having similar experience where we are unable to successfully use APOLLO_ROUTER_CONFIG_PATH b/c the following cli option is always set regardless if router.configuration is undefined in the Values

This reference lists and describes the options supported by the router binary. Where indicated, some of these options can also be provided via an environment variable. If an option is provided both ways, the command-line value takes precedence.

sure seems like --config should not be populated if the values file doesn't define router.configuration given the Chart

        - args:
            - '--config'
            - /app/configuration.yaml

having the same experience with helm template locally, as well as using ArgoCD's version of helm

version.BuildInfo{Version:"v3.14.3", GitCommit:"f03cc04caaa8f6d7c3e67cf918929150cf6f3f12", GitTreeState:"clean", GoVersion:"go1.22.1"}

@dan-catalano-vc
Copy link

created a discussion on the topic #4934

dan-catalano-vc added a commit to validationcloud/router that referenced this issue Apr 10, 2024
@dan-catalano-vc
helm chart - make .Values.router optional
791cdb9 
- discussion apollographql#4934 explains issue
- get liveness & readiness port from containerPorts
- check if .Values.router exists before checking if
  .Values.router.configuration exist
- remove router.configuration & router.args from default values.yaml
Issue apollographql#4781
@dan-catalano-vc dan-catalano-vc linked a pull request Apr 10, 2024 that will close this issue
Open
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
raised by user
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

helm chart - make .Values.router optional validationcloud/router
2 participants
@shaikatzz @dan-catalano-vc