helm/peerpods: fixing constant names on default values templates#2811
Conversation
| # (default: "defaultURI") | ||
| # LIBVIRT_URI: "defaultURI" | ||
| # (default: "qemu:///system") | ||
| # LIBVIRT_URI: "qemu:///system" |
There was a problem hiding this comment.
I guess something we have to reconcile now is that in kustomisation.yaml we had configuration default e.g. qemu+ssh://root@192.168.122.1/system?no_verify=1 for LIBVIRT_URI and LIBVIRT_EFI_FIRMWARE="/usr/share/OVMF/OVMF_CODE_4M.fd" which I think would be used over the code defaults, so the charts values might not work out of the box for users?
There was a problem hiding this comment.
vide my comment on your pr.
beraldoleal
force-pushed
the
helm-fix-parser
branch
from
7bc9ea3 to
932f6d2
Compare
wainersm
left a comment
wainersm
left a comment
There was a problem hiding this comment.
Hi @beraldoleal ,
I didn't review the AST parsing because I don't know that code. The outcome yamls look fine though.
Lets assume Claude owns it :) We should probably start adding comments like "Wrote by Claude - be careful". |
|
@wainersm sorry, but I pushed two new commits here. Do I still have your bless? |
My ack stands. But while we are here, what if we align the libvirt URI too? |
I'm ok with, lets see what @stevenhorsman thinks... |
beraldoleal
force-pushed
the
helm-fix-parser
branch
from
801587a to
a544473
Compare
stevenhorsman
left a comment
stevenhorsman
left a comment
There was a problem hiding this comment.
Looking good. Thanks for the improvements!
|
I can't merge this, because is the is mandatory... but its related to #2812. |
Let's see if Wainer, or anyone else can approve that, otherwise we might need to stop it being required due to lack of maintainers |
I just approved it. |
The config-extractor parser was extracting constant names instead of their values when generating values.yaml files. For example, it would output `LIBVIRT_NET: "defaultNetworkName"` instead of `LIBVIRT_NET: "default"`. This bug went unnoticed because the values were commented out by default, so the Go code used the correct hardcoded defaults at runtime. Thanks @wainersm for catching this. Signed-off-by: Beraldo Leal <bleal@redhat.com>
Align Helm with Kustomize by defaulting LIBVIRT_EFI_FIRMWARE to /usr/share/OVMF/OVMF_CODE_4M.fd instead of empty string. Signed-off-by: Beraldo Leal <bleal@redhat.com>
Align Helm with Kustomize by defaulting LIBVIRT_URI to align with kustomize. Signed-off-by: Beraldo Leal <bleal@redhat.com>
Align Helm with Kustomize defaults. Signed-off-by: Beraldo Leal <bleal@redhat.com>
beraldoleal
force-pushed
the
helm-fix-parser
branch
from
a544473 to
d21cdfc
Compare
|
I'm trying to understand the order of those vuln checks.. so I just rebased on main and pushed.. to see if the error is gone... |
3 participants
The config-extractor parser was extracting constant names instead of their values when generating values.yaml files.
For example, it would output
LIBVIRT_NET: "defaultNetworkName"instead ofLIBVIRT_NET: "default".This bug went unnoticed because the values were commented out by default, so the Go code used the correct hardcoded defaults at runtime.
Thanks @wainersm for catching this.