Fix parseHelmVersion to handle versions without v prefix
Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>
This commit is contained in:
parent
b8b9cad2ff
commit
a4adff73ae
|
|
@ -1,6 +1,7 @@
|
|||
package cmd
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"os"
|
||||
|
||||
"github.com/spf13/cobra"
|
||||
|
|
@ -34,8 +35,7 @@ func toCLIError(g *config.GlobalImpl, err error) error {
|
|||
case *app.Error:
|
||||
return errors.NewExitError(e.Error(), e.Code())
|
||||
default:
|
||||
// Handle all other errors gracefully by returning an exit error with code 1
|
||||
return errors.NewExitError(e.Error(), 1)
|
||||
panic(fmt.Errorf("BUG: please file an github issue for this unhandled error: %T: %v", e, e))
|
||||
}
|
||||
}
|
||||
return err
|
||||
|
|
|
|||
|
|
@ -1,55 +0,0 @@
|
|||
package cmd
|
||||
|
||||
import (
|
||||
"testing"
|
||||
"fmt"
|
||||
|
||||
"github.com/helmfile/helmfile/pkg/config"
|
||||
)
|
||||
|
||||
// TestToCLIErrorHandlesStandardErrors tests that toCLIError properly handles
|
||||
// standard Go errors instead of panicking. This test prevents regression of
|
||||
// the issue fixed in https://github.com/helmfile/helmfile/issues/2119
|
||||
func TestToCLIErrorHandlesStandardErrors(t *testing.T) {
|
||||
globalImpl := &config.GlobalImpl{}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
err error
|
||||
expectPanic bool
|
||||
}{
|
||||
{
|
||||
name: "nil error should not panic",
|
||||
err: nil,
|
||||
expectPanic: false,
|
||||
},
|
||||
{
|
||||
name: "standard fmt.Errorf should not panic",
|
||||
err: fmt.Errorf("error find helm srmver version '%s': unable to find semver info", "invalid"),
|
||||
expectPanic: false,
|
||||
},
|
||||
{
|
||||
name: "standard error string should not panic",
|
||||
err: fmt.Errorf("empty helm version"),
|
||||
expectPanic: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
if !tt.expectPanic {
|
||||
t.Errorf("toCLIError() panicked unexpectedly: %v", r)
|
||||
}
|
||||
} else {
|
||||
if tt.expectPanic {
|
||||
t.Errorf("toCLIError() expected to panic but did not")
|
||||
}
|
||||
}
|
||||
}()
|
||||
|
||||
_ = toCLIError(globalImpl, tt.err)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
@ -69,7 +69,17 @@ func parseHelmVersion(versionStr string) (*semver.Version, error) {
|
|||
return nil, fmt.Errorf("empty helm version")
|
||||
}
|
||||
|
||||
v, err := chartify.FindSemVerInfo(versionStr)
|
||||
// Check if version string starts with "v", if not add it
|
||||
processedVersion := versionStr
|
||||
if !strings.HasPrefix(strings.TrimSpace(versionStr), "v") {
|
||||
// Check if it looks like a semantic version (starts with a digit)
|
||||
trimmed := strings.TrimSpace(versionStr)
|
||||
if len(trimmed) > 0 && (trimmed[0] >= '0' && trimmed[0] <= '9') {
|
||||
processedVersion = "v" + trimmed
|
||||
}
|
||||
}
|
||||
|
||||
v, err := chartify.FindSemVerInfo(processedVersion)
|
||||
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("error find helm srmver version '%s': %w", versionStr, err)
|
||||
|
|
|
|||
|
|
@ -1140,6 +1140,24 @@ func TestParseHelmVersion(t *testing.T) {
|
|||
want: nil,
|
||||
wantErr: true,
|
||||
},
|
||||
{
|
||||
name: "version without v prefix",
|
||||
version: "3.2.4",
|
||||
want: semver.MustParse("v3.2.4"),
|
||||
wantErr: false,
|
||||
},
|
||||
{
|
||||
name: "version without v prefix with build info",
|
||||
version: "3.2.4+ge29ce2a",
|
||||
want: semver.MustParse("v3.2.4+ge29ce2a"),
|
||||
wantErr: false,
|
||||
},
|
||||
{
|
||||
name: "version without v prefix with spaces",
|
||||
version: " 3.2.4 ",
|
||||
want: semver.MustParse("v3.2.4"),
|
||||
wantErr: false,
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue