fix: focus only on plugin name but give more info about what we get

And improve test

Follow Scott comment:
> here I would have a list of valid and invalid names based on pattern, with a check and intended outcome for each one so that we comprehensively cover the rules.

https://github.com/helm/helm/pull/31491/files#r2524820312

Signed-off-by: Benoit Tigeot <benoit.tigeot@lifen.fr>
pull/31491/head
Benoit Tigeot 10 months ago
parent f4b139a82c
commit cf077ceb27
No known key found for this signature in database
GPG Key ID: 8E6D4FC8AEBDA62C

@ -68,9 +68,6 @@ type MetadataLegacy struct {
} }
func (m *MetadataLegacy) Validate() error { func (m *MetadataLegacy) Validate() error {
if m.Name == "" {
return fmt.Errorf("missing plugin name")
}
if !validPluginName.MatchString(m.Name) { if !validPluginName.MatchString(m.Name) {
return fmt.Errorf("invalid plugin name %q: must contain only a-z, A-Z, 0-9, _ and -", m.Name) return fmt.Errorf("invalid plugin name %q: must contain only a-z, A-Z, 0-9, _ and -", m.Name)
} }

@ -16,40 +16,49 @@ limitations under the License.
package plugin package plugin
import ( import (
"strings"
"testing" "testing"
) )
func TestMetadataLegacyValidate_EmptyName(t *testing.T) { func TestMetadataLegacyValidate_PluginName(t *testing.T) {
metadata := MetadataLegacy{ tests := []struct {
Name: "", name string
Version: "1.0.0", pluginName string
} shouldPass bool
}{
err := metadata.Validate() // Valid names
if err == nil { {"lowercase", "myplugin", true},
t.Fatal("expected error for empty plugin name") {"uppercase", "MYPLUGIN", true},
} {"mixed case", "MyPlugin", true},
{"with digits", "plugin123", true},
{"with hyphen", "my-plugin", true},
{"with underscore", "my_plugin", true},
{"mixed chars", "my-awesome_plugin_123", true},
expectedMsg := "plugin name is empty or missing" // Invalid names
if !strings.Contains(err.Error(), expectedMsg) { {"empty", "", false},
t.Errorf("expected error to contain %q, got %q", expectedMsg, err.Error()) {"space", "my plugin", false},
} {"colon", "Name:", false},
{"period", "my.plugin", false},
{"slash", "my/plugin", false},
{"dollar", "$plugin", false},
{"unicode", "plügîn", false},
} }
func TestMetadataLegacyValidate_InvalidName(t *testing.T) { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
metadata := MetadataLegacy{ metadata := MetadataLegacy{
Name: "invalid name", Name: tt.pluginName,
Version: "1.0.0", Version: "1.0.0",
} }
err := metadata.Validate() err := metadata.Validate()
if err == nil {
t.Fatal("expected error for invalid plugin name")
}
expectedMsg := "plugin names can only contain ASCII characters" if tt.shouldPass && err != nil {
if !strings.Contains(err.Error(), expectedMsg) { t.Errorf("expected %q to be valid, got error: %v", tt.pluginName, err)
t.Errorf("expected error to contain %q, got %q", expectedMsg, err.Error()) }
if !tt.shouldPass && err == nil {
t.Errorf("expected %q to be invalid, but passed", tt.pluginName)
}
})
} }
} }

@ -47,11 +47,8 @@ type MetadataV1 struct {
} }
func (m *MetadataV1) Validate() error { func (m *MetadataV1) Validate() error {
if m.Name == "" {
return fmt.Errorf("missing plugin `name`")
}
if !validPluginName.MatchString(m.Name) { if !validPluginName.MatchString(m.Name) {
return fmt.Errorf("invalid plugin `name` %q: must contain only a-z, A-Z, 0-9, _ and -", m.Name) return fmt.Errorf("invalid plugin `name`")
} }
if m.APIVersion != "v1" { if m.APIVersion != "v1" {

Loading…
Cancel
Save