feat: add values template file #80
Conversation
omidasadpour
force-pushed
the
feat/auto-generate-values.yaml-from-toml
branch
2 times, most recently
from
eea9315 to
f473461
Compare
charts/rollups-node/script.sh
Outdated
| VALUES_FILE="values.yaml" | ||
|
|
||
| # Download the Config.toml file from the rollups-node repository | ||
| wget https://raw.githubusercontent.com/cartesi/rollups-node/main/internal/config/generate/Config.toml |
There was a problem hiding this comment.
We should use the tagged URL, so we can target a version of the configuration. I know we don't have a tagged release for this file yet, but we should be prepared.
There was a problem hiding this comment.
For this file, since we're gonna use the config. map exclusively, we should iterate over it and define the environment in case they are defined.
We don't want to create a new entry here when a new config is created by the script. Some exceptions are the ones the depends on volumes and secrets, like MNEMONIC and MNEMONIC_FILE.
There was a problem hiding this comment.
Something like this ?
env:
{{- range $key, $value := .Values.validator.config }}
{{- if ne $value "" }}
- name: {{ $key }}
value: {{ quote $value }}
{{- end }}
{{- end }}
3434449 to
0e6ae5a
Compare
| {{- range $key, $value := .Values.validator.config }} | ||
| {{- if and (ne $value "") (not (in $key (list "CARTESI_AUTH_MNEMONIC" "CARTESI_AUTH_MNEMONIC_FILE" "CARTESI_AUTH_MNEMONIC_ACCOUNT_INDEX" "CARTESI_AUTH_AWS_KMS_KEY_ID" "CARTESI_AUTH_AWS_KMS_REGION"))) }} | ||
| - name: {{ $key }} | ||
| value: {{ quote $value }} |
There was a problem hiding this comment.
Could we extract this to a helper method and handle all the cases for secrets and chainID there?
Keep an eye on these issues:
| secretName: {{ include "validator.fullname" . }}-mnemonic | ||
| {{- else }} | ||
| secretName: {{ .Values.dapp.mnemonic.secretRef }} | ||
| secretName: {{ .Values.validator.config.CARTESI_AUTH_MNEMONIC_FILE }} |
There was a problem hiding this comment.
This is misleading since the docs tell you to point to a file and not a secretName.
Maybe the user will need to use extraVolumes to mount a secret outside of helm or use extraDeploy.
There was a problem hiding this comment.
This part uses if else . so, if user doesn't define the CARTESI_AUTH_MNEMONIC , the helm will not create secret and use existing secret.
Them. the existing secrets or the created secret via helm will be mount as a config file.
There was a problem hiding this comment.
Them. the existing secrets or the created secret via helm will be mount as a config file.
I don't see this happening, could you point me where is this.
Also, my point was that CARTESI_AUTH_MNEMONIC_FILE asks where the file is and the the secretName - how it's being used. Got it?
There was a problem hiding this comment.
I don't see this happening, could you point me where is this.
There was a problem hiding this comment.
Also, my point was that
CARTESI_AUTH_MNEMONIC_FILEasks where the file is and the thesecretName- how it's being used. Got it?
I see. So, we should extract the name from the path and use it in deployment file then.
There was a problem hiding this comment.
No, we should not use the value of CARTESI_AUTH_MNEMONIC_FILE as the secretName at all.
If the user informs CARTESI_AUTH_MNEMONIC, we make the secret and mount it and define CARTESI_AUTH_MNEMONIC_FILE to point to it. This is already being done, maybe we should document that this is happening.
If the user informs CARTESI_AUTH_MNEMONIC_FILE they should take care of putting the value of the MNEMONIC on the file the the variable points to.
There was a problem hiding this comment.
Get U know. So, we should change the condition. I'll send a fixup !
omidasadpour
force-pushed
the
feat/auto-generate-values.yaml-from-toml
branch
from
0e6ae5a to
8aa226a
Compare
|
I'm gonna merge it and make some adjustments at the other PR. |
endersonmaia
merged commit 41e9dc9
into
feature/adapt-to-rollups-node-1.3.0
/close #79