diff --git a/bootcfg/storage/storagepb/group.go b/bootcfg/storage/storagepb/group.go index 071533c0..1ddca816 100644 --- a/bootcfg/storage/storagepb/group.go +++ b/bootcfg/storage/storagepb/group.go @@ -23,6 +23,9 @@ func ParseGroup(data []byte) (*Group, error) { if err != nil { return nil, err } + if err := group.Normalize(); err != nil { + return nil, err + } return group, err } @@ -49,6 +52,23 @@ func (g *Group) Matches(labels map[string]string) bool { return true } +// Normalize normalizes Group selectors according to reserved selector rules +// which require "mac" addresses to be valid, normalized MAC addresses. +func (g *Group) Normalize() error { + for key, val := range g.Requirements { + switch strings.ToLower(key) { + case "mac": + macAddr, err := net.ParseMAC(val) + if err != nil { + return err + } + // range iteration copy with mutable map + g.Requirements[key] = macAddr.String() + } + } + return nil +} + // requirementString returns Group requirements as a string of sorted key // value pairs for comparisons. func (g *Group) requirementString() string { @@ -130,20 +150,7 @@ func (rg *RichGroup) ToGroup() (*Group, error) { Id: rg.Id, Name: rg.Name, Profile: rg.Profile, - Requirements: normalizeSelectors(rg.Requirements), + Requirements: rg.Requirements, Metadata: metadata, }, nil } - -func normalizeSelectors(selectors map[string]string) map[string]string { - for key, val := range selectors { - switch strings.ToLower(key) { - case "mac": - if macAddr, err := net.ParseMAC(val); err == nil { - // range iteration copy with mutable map - selectors[key] = macAddr.String() - } - } - } - return selectors -} diff --git a/bootcfg/storage/storagepb/group_test.go b/bootcfg/storage/storagepb/group_test.go index 27a8ef65..29b33dc4 100644 --- a/bootcfg/storage/storagepb/group_test.go +++ b/bootcfg/storage/storagepb/group_test.go @@ -1,6 +1,7 @@ package storagepb import ( + "net" "sort" "testing" @@ -54,11 +55,38 @@ func TestGroupValidate(t *testing.T) { } } +func TestNormalize(t *testing.T) { + expectedInvalidMAC := &net.AddrError{Err: "invalid MAC address", Addr: "not-a-mac"} + cases := []struct { + selectors map[string]string + normalized map[string]string + err error + }{ + {map[string]string{"platform": "metal"}, map[string]string{"platform": "metal"}, nil}, + {map[string]string{"mac": "52-da-00-89-d8-10"}, map[string]string{"mac": "52:da:00:89:d8:10"}, nil}, + {map[string]string{"MAC": "52-da-00-89-d8-10"}, map[string]string{"MAC": "52:da:00:89:d8:10"}, nil}, + // un-normalized MAC address should be normalized + {map[string]string{"mac": "52-DA-00-89-D8-10"}, map[string]string{"mac": "52:da:00:89:d8:10"}, nil}, + {map[string]string{"MAC": "52-DA-00-89-D8-10"}, map[string]string{"MAC": "52:da:00:89:d8:10"}, nil}, + // invalid MAC address should be rejected + {map[string]string{"mac": "not-a-mac"}, map[string]string{"mac": "not-a-mac"}, expectedInvalidMAC}, + } + for _, c := range cases { + group := &Group{Id: "id", Requirements: c.selectors} + err := group.Normalize() + // assert that: + // - Group selectors (MAC addresses) are normalized + // - Invalid MAC addresses cause a normalization error + assert.Equal(t, c.err, err) + assert.Equal(t, c.normalized, group.Requirements) + } +} + func TestGroupMatches(t *testing.T) { cases := []struct { - labels map[string]string - selectors map[string]string - expected bool + labels map[string]string + selectors map[string]string + expected bool }{ {map[string]string{"a": "b"}, map[string]string{"a": "b"}, true}, {map[string]string{"a": "b"}, map[string]string{"a": "c"}, false}, @@ -66,7 +94,7 @@ func TestGroupMatches(t *testing.T) { {map[string]string{"uuid": "a"}, map[string]string{"uuid": "a", "mac": "b"}, false}, } // assert that: - // - Group requirements are satisfied in order to be a match + // - Group selectors must be satisfied for a match // - labels may provide additional key/value pairs for _, c := range cases { group := &Group{Requirements: c.selectors}