fix(helm): don't panic if reference parsing fails
Description
Something in my setup broke and skaffold started panicking in the envVarForImage function:
WARN[0014] unable to extract values for IMAGE_REPO, IMAGE_TAG and IMAGE_DIGEST from image k3d-localhost:5111/***:v0.0.1-1004-g4cd4ad1a8@1ff316f0e620a4907dd028061654680b7c4b00f48d43dbdb906fec77bc3d96e5 due to error:
invalid reference format subtask=-1 task=DevLoop
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x10 pc=0x105e7d664]
goroutine 1 [running]:
github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/helm.envVarForImage({0x14000b20bb0, 0xa}, {0x14000d9ae00, 0x75})
/tmpfs/src/github/skaffold/pkg/skaffold/helm/args.go:149 +0x494
github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/helm.ConstructOverrideArgs(0x14000fbdb48, {0x14000d68300, 0x6, 0x14001270f40?}, {0x1400016cb00, 0x9, 0x10}, 0x14000d9af00?)
/tmpfs/src/github/skaffold/pkg/skaffold/helm/args.go:62 +0xa88
// omitted for brevity
I am not sure how to reproduce this reliably, but I figured panicking is never desirable here, and the fix seemed easy. LMK what you think.
Not sure how to test this properly either, if you have any pointers for this, LMK.
Codecov Report
Merging #8796 (c6378e5) into main (290280e) will decrease coverage by
6.54%. The diff coverage is50.00%.
:exclamation: Current head c6378e5 differs from pull request most recent head 7e490d3. Consider uploading reports for the commit 7e490d3 to get more accurate results
@@ Coverage Diff @@
## main #8796 +/- ##
==========================================
- Coverage 70.48% 63.94% -6.54%
==========================================
Files 515 620 +105
Lines 23150 31457 +8307
==========================================
+ Hits 16317 20116 +3799
- Misses 5776 9835 +4059
- Partials 1057 1506 +449
| Impacted Files | Coverage Δ | |
|---|---|---|
| cmd/skaffold/app/cmd/completion.go | 13.04% <0.00%> (-1.25%) |
:arrow_down: |
| cmd/skaffold/app/cmd/config/list.go | 65.21% <ø> (ø) |
|
| cmd/skaffold/app/cmd/config/set.go | 88.72% <ø> (ø) |
|
| cmd/skaffold/app/cmd/config/util.go | 54.28% <ø> (ø) |
|
| cmd/skaffold/app/cmd/credits.go | 100.00% <ø> (ø) |
|
| cmd/skaffold/app/cmd/credits/export.go | 0.00% <0.00%> (ø) |
|
| cmd/skaffold/app/cmd/deploy.go | 40.90% <0.00%> (-12.94%) |
:arrow_down: |
| cmd/skaffold/app/cmd/generate_pipeline.go | 60.00% <ø> (ø) |
|
| cmd/skaffold/app/cmd/inspect_modules.go | 65.00% <ø> (ø) |
|
| cmd/skaffold/app/cmd/inspect_profiles.go | 66.66% <ø> (ø) |
|
| ... and 40 more |
... and 409 files with indirect coverage changes
:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more
@alphanota @plumpy @mattsanta Would you review this pull request?