From cb36f0f8c636028ab69146493eda733e5dad2b10 Mon Sep 17 00:00:00 2001 From: Shoma Kokuryo Date: Thu, 23 Aug 2018 02:39:26 +0000 Subject: [PATCH 1/7] implement label feature (without indexing) --- cmd/sabactl/machines.go | 17 ++-- e2e/sabactl_test.go | 20 +++-- machines.go | 17 ++-- models/etcd/index.go | 44 +++------- models/etcd/index_test.go | 48 ++++++----- models/etcd/machine_test.go | 2 +- models/etcd/main_test.go | 6 +- query.go | 26 ++++-- query_test.go | 13 +-- web/coreos_test.go | 14 +-- web/ignitions_test.go | 16 ++-- web/machines.go | 22 +++-- web/machines_test.go | 164 +++++++++++++++++++++++++----------- 13 files changed, 248 insertions(+), 161 deletions(-) diff --git a/cmd/sabactl/machines.go b/cmd/sabactl/machines.go index 6f63a510..f6d1f210 100644 --- a/cmd/sabactl/machines.go +++ b/cmd/sabactl/machines.go @@ -40,15 +40,14 @@ type machinesGetCmd struct { } var machinesGetQuery = map[string]string{ - "serial": "Serial name", - "datacenter": "Datacenter name", - "rack": "Rack name", - "role": "Role name", - "product": "Product name (e.g. 'R630')", - "ipv4": "IPv4 address", - "ipv6": "IPv6 address", - "bmc-type": "BMC type", - "state": "State", + "serial": "Serial name", + "rack": "Rack name", + "role": "Role name", + "labels": "Label name and value (-labels key=val,...)", + "ipv4": "IPv4 address", + "ipv6": "IPv6 address", + "bmc-type": "BMC type", + "state": "State", } func (r *machinesGetCmd) SetFlags(f *flag.FlagSet) { diff --git a/e2e/sabactl_test.go b/e2e/sabactl_test.go index fb31017d..9ea03508 100644 --- a/e2e/sabactl_test.go +++ b/e2e/sabactl_test.go @@ -179,19 +179,23 @@ func testSabactlMachines(t *testing.T) { specs := []*sabakan.MachineSpec{ { - Serial: "12345678", - Product: "R730xd", - Datacenter: "ty3", - Role: "worker", + Serial: "12345678", + Labels: map[string]string{ + "product": "R730xd", + "datacenter": "ty3", + }, + Role: "worker", BMC: sabakan.MachineBMC{ Type: sabakan.BmcIdrac9, }, }, { - Serial: "abcdefg", - Product: "R730xd", - Datacenter: "ty3", - Role: "boot", + Serial: "abcdefg", + Labels: map[string]string{ + "product": "R730xd", + "datacenter": "ty3", + }, + Role: "boot", BMC: sabakan.MachineBMC{ Type: sabakan.BmcIpmi2, }, diff --git a/machines.go b/machines.go index 901c9c65..f018cea0 100644 --- a/machines.go +++ b/machines.go @@ -48,15 +48,14 @@ type MachineBMC struct { // MachineSpec is a set of attributes to define a machine. type MachineSpec struct { - Serial string `json:"serial"` - Product string `json:"product"` - Datacenter string `json:"datacenter"` - Rack uint `json:"rack"` - IndexInRack uint `json:"index-in-rack"` - Role string `json:"role"` - IPv4 []string `json:"ipv4"` - IPv6 []string `json:"ipv6"` - BMC MachineBMC `json:"bmc"` + Serial string `json:"serial"` + Labels map[string]string `json:"labels"` + Rack uint `json:"rack"` + IndexInRack uint `json:"index-in-rack"` + Role string `json:"role"` + IPv4 []string `json:"ipv4"` + IPv6 []string `json:"ipv6"` + BMC MachineBMC `json:"bmc"` } // Machine represents a server hardware. diff --git a/models/etcd/index.go b/models/etcd/index.go index 1e76a381..e30f36ac 100644 --- a/models/etcd/index.go +++ b/models/etcd/index.go @@ -12,27 +12,23 @@ import ( // machinesIndex is on-memory index of the etcd values type machinesIndex struct { - mux sync.RWMutex - Product map[string][]string - Datacenter map[string][]string - Rack map[string][]string - Role map[string][]string - IPv4 map[string]string - IPv6 map[string]string - BMCType map[string][]string - State map[sabakan.MachineState][]string + mux sync.RWMutex + Rack map[string][]string + Role map[string][]string + IPv4 map[string]string + IPv6 map[string]string + BMCType map[string][]string + State map[sabakan.MachineState][]string } func newMachinesIndex() *machinesIndex { return &machinesIndex{ - Product: make(map[string][]string), - Datacenter: make(map[string][]string), - Rack: make(map[string][]string), - Role: make(map[string][]string), - IPv4: make(map[string]string), - IPv6: make(map[string]string), - BMCType: make(map[string][]string), - State: make(map[sabakan.MachineState][]string), + Rack: make(map[string][]string), + Role: make(map[string][]string), + IPv4: make(map[string]string), + IPv6: make(map[string]string), + BMCType: make(map[string][]string), + State: make(map[sabakan.MachineState][]string), } } @@ -71,8 +67,6 @@ func (mi *machinesIndex) AddIndex(m *sabakan.Machine) { func (mi *machinesIndex) addNoLock(m *sabakan.Machine) { spec := &m.Spec - mi.Product[spec.Product] = append(mi.Product[spec.Product], spec.Serial) - mi.Datacenter[spec.Datacenter] = append(mi.Datacenter[spec.Datacenter], spec.Serial) mcrack := fmt.Sprint(spec.Rack) mi.Rack[mcrack] = append(mi.Rack[mcrack], spec.Serial) mi.Role[spec.Role] = append(mi.Role[spec.Role], spec.Serial) @@ -109,12 +103,8 @@ func (mi *machinesIndex) DeleteIndex(m *sabakan.Machine) { func (mi *machinesIndex) deleteNoLock(m *sabakan.Machine) { spec := &m.Spec - i := indexOf(mi.Product[spec.Product], spec.Serial) - mi.Product[spec.Product] = append(mi.Product[spec.Product][:i], mi.Product[spec.Product][i+1:]...) - i = indexOf(mi.Datacenter[spec.Datacenter], spec.Serial) - mi.Datacenter[spec.Datacenter] = append(mi.Datacenter[spec.Datacenter][:i], mi.Datacenter[spec.Datacenter][i+1:]...) mcrack := fmt.Sprint(spec.Rack) - i = indexOf(mi.Rack[mcrack], spec.Serial) + i := indexOf(mi.Rack[mcrack], spec.Serial) mi.Rack[mcrack] = append(mi.Rack[mcrack][:i], mi.Rack[mcrack][i+1:]...) i = indexOf(mi.Role[spec.Role], spec.Serial) mi.Role[spec.Role] = append(mi.Role[spec.Role][:i], mi.Role[spec.Role][i+1:]...) @@ -147,12 +137,6 @@ func (mi *machinesIndex) query(q sabakan.Query) []string { res := make(map[string]struct{}) - for _, serial := range mi.Product[q.Product()] { - res[serial] = struct{}{} - } - for _, serial := range mi.Datacenter[q.Datacenter()] { - res[serial] = struct{}{} - } for _, serial := range mi.Rack[q.Rack()] { res[serial] = struct{}{} } diff --git a/models/etcd/index_test.go b/models/etcd/index_test.go index 94cd3add..0474d7fa 100644 --- a/models/etcd/index_test.go +++ b/models/etcd/index_test.go @@ -13,16 +13,16 @@ func TestMachinesIndex(t *testing.T) { mi := newMachinesIndex() machines := []*sabakan.Machine{ - sabakan.NewMachine(sabakan.MachineSpec{Serial: "1", Product: "R630", Datacenter: "ty3", Role: "boot", BMC: sabakan.MachineBMC{Type: sabakan.BmcIpmi2}}), - sabakan.NewMachine(sabakan.MachineSpec{Serial: "2", Product: "R630", Datacenter: "ty3", Role: "worker", BMC: sabakan.MachineBMC{Type: sabakan.BmcIpmi2}}), - sabakan.NewMachine(sabakan.MachineSpec{Serial: "3", Product: "R730xd", Datacenter: "ty3", Role: "worker", BMC: sabakan.MachineBMC{Type: sabakan.BmcIpmi2}}), + sabakan.NewMachine(sabakan.MachineSpec{Serial: "1", Labels: map[string]string{"product": "R630", "datacenter": "ty3"}, Role: "boot", BMC: sabakan.MachineBMC{Type: sabakan.BmcIpmi2}}), + sabakan.NewMachine(sabakan.MachineSpec{Serial: "2", Labels: map[string]string{"product": "R630", "datacenter": "ty3"}, Role: "worker", BMC: sabakan.MachineBMC{Type: sabakan.BmcIpmi2}}), + sabakan.NewMachine(sabakan.MachineSpec{Serial: "3", Labels: map[string]string{"product": "R730xd", "datacenter": "ty3"}, Role: "worker", BMC: sabakan.MachineBMC{Type: sabakan.BmcIpmi2}}), } for _, m := range machines { mi.AddIndex(m) } - serials := mi.query(sabakan.Query{"product": "R730xd"}) + serials := mi.query(sabakan.Query{"labels": "product=R730xd"}) if len(serials) != 1 { t.Fatal("wrong query count:", len(serials)) } @@ -32,25 +32,29 @@ func TestMachinesIndex(t *testing.T) { prev := sabakan.NewMachine( sabakan.MachineSpec{ - Serial: "2", - Product: "R630", - Datacenter: "ty3", - Role: "worker", - BMC: sabakan.MachineBMC{Type: sabakan.BmcIpmi2}, + Serial: "2", + Labels: map[string]string{ + "product": "R630", + "datacenter": "ty3", + }, + Role: "worker", + BMC: sabakan.MachineBMC{Type: sabakan.BmcIpmi2}, }) current := sabakan.NewMachine( sabakan.MachineSpec{ - Serial: "2", - Product: "R730xd", - Datacenter: "ty3", - Role: "worker", - BMC: sabakan.MachineBMC{Type: sabakan.BmcIpmi2}, + Serial: "2", + Labels: map[string]string{ + "product": "R730xd", + "datacenter": "ty3", + }, + Role: "worker", + BMC: sabakan.MachineBMC{Type: sabakan.BmcIpmi2}, }) current.Status.State = sabakan.StateRetiring mi.UpdateIndex(prev, current) - serials = mi.query(sabakan.Query{"product": "R730xd"}) + serials = mi.query(sabakan.Query{"labels": "product=R730xd"}) if len(serials) != 2 { t.Fatal("wrong query count:", len(serials)) } @@ -66,14 +70,16 @@ func TestMachinesIndex(t *testing.T) { mi.DeleteIndex(sabakan.NewMachine( sabakan.MachineSpec{ - Serial: "3", - Product: "R730xd", - Datacenter: "ty3", - Role: "worker", - BMC: sabakan.MachineBMC{Type: sabakan.BmcIpmi2}, + Serial: "3", + Labels: map[string]string{ + "product": "R730xd", + "datacenter": "ty3", + }, + Role: "worker", + BMC: sabakan.MachineBMC{Type: sabakan.BmcIpmi2}, })) - serials = mi.query(sabakan.Query{"product": "R730xd"}) + serials = mi.query(sabakan.Query{"labels": "product=R730xd"}) if len(serials) != 1 { t.Fatal("wrong query count:", len(serials)) } diff --git a/models/etcd/machine_test.go b/models/etcd/machine_test.go index d4759aae..8af90788 100644 --- a/models/etcd/machine_test.go +++ b/models/etcd/machine_test.go @@ -130,7 +130,7 @@ func testQuery(t *testing.T) { t.Errorf("unexpected responsed machine: %#v", resp[0]) } - q = sabakan.Query{"product": "R630"} + q = sabakan.Query{"labels": "product=R630"} resp, err = d.machineQuery(context.Background(), q) if err != nil { t.Fatal(err) diff --git a/models/etcd/main_test.go b/models/etcd/main_test.go index f2f657f5..749a0ac9 100644 --- a/models/etcd/main_test.go +++ b/models/etcd/main_test.go @@ -106,9 +106,9 @@ func initializeTestData(d *driver, ch <-chan struct{}) ([]*sabakan.Machine, erro <-ch machines := []*sabakan.Machine{ - sabakan.NewMachine(sabakan.MachineSpec{Serial: "12345678", Product: "R630", Role: "worker"}), - sabakan.NewMachine(sabakan.MachineSpec{Serial: "12345679", Product: "R630", Role: "worker"}), - sabakan.NewMachine(sabakan.MachineSpec{Serial: "123456789", Product: "R730", Role: "worker"}), + sabakan.NewMachine(sabakan.MachineSpec{Serial: "12345678", Labels: map[string]string{"product": "R630"}, Role: "worker"}), + sabakan.NewMachine(sabakan.MachineSpec{Serial: "12345679", Labels: map[string]string{"product": "R630"}, Role: "worker"}), + sabakan.NewMachine(sabakan.MachineSpec{Serial: "123456789", Labels: map[string]string{"product": "R730"}, Role: "worker"}), } err = d.machineRegister(ctx, machines) if err != nil { diff --git a/query.go b/query.go index eeb06a4d..d924dd92 100644 --- a/query.go +++ b/query.go @@ -1,6 +1,8 @@ package sabakan import "fmt" +import "net/url" +import "strings" // Query is an URL query type Query map[string]string @@ -34,11 +36,25 @@ func (q Query) Match(m *Machine) bool { return false } } - if product := q["product"]; len(product) > 0 && product != m.Spec.Product { - return false - } - if datacenter := q["datacenter"]; len(datacenter) > 0 && datacenter != m.Spec.Datacenter { - return false + if labels := q["labels"]; len(labels) > 0 { + // Split into each query + rawQueries := strings.Split(labels, ",") + for _, rawQuery := range rawQueries { + rawQuery = strings.TrimSpace(rawQuery) + query, err := url.ParseQuery(rawQuery) + if err != nil { + return false + } + for k, v := range query { + if label, exists := m.Spec.Labels[k]; exists { + if v[0] != label { + return false + } + } else { + return false + } + } + } } if rack := q["rack"]; len(rack) > 0 && rack != fmt.Sprint(m.Spec.Rack) { return false diff --git a/query_test.go b/query_test.go index 425ef3b8..53a0e2e2 100644 --- a/query_test.go +++ b/query_test.go @@ -11,17 +11,18 @@ func TestMatch(t *testing.T) { b bool }{ {Query{"serial": "1234"}, NewMachine(MachineSpec{Serial: "1234"}), true}, - {Query{"product": "R630"}, NewMachine(MachineSpec{Product: "R630"}), true}, - {Query{"datacenter": "us"}, NewMachine(MachineSpec{Datacenter: "us"}), true}, + {Query{"labels": "product=R630"}, NewMachine(MachineSpec{Labels: map[string]string{"product": "R630"}}), true}, + {Query{"labels": "datacenter=us"}, NewMachine(MachineSpec{Labels: map[string]string{"datacenter": "us"}}), true}, {Query{"rack": "1"}, NewMachine(MachineSpec{Rack: 1}), true}, {Query{"role": "boot"}, NewMachine(MachineSpec{Role: "boot"}), true}, {Query{"ipv4": "10.20.30.40"}, NewMachine(MachineSpec{IPv4: []string{"10.20.30.40", "10.21.30.40"}}), true}, {Query{"ipv6": "aa::ff"}, NewMachine(MachineSpec{IPv6: []string{"aa::ff", "bb::ff"}}), true}, - {Query{"product": "R630", "datacenter": "us"}, NewMachine(MachineSpec{Product: "R630", Datacenter: "us"}), true}, + {Query{"labels": "product=R630,datacenter=us"}, NewMachine(MachineSpec{Labels: map[string]string{"product": "R630", "datacenter": "us"}}), true}, {Query{"state": "healthy"}, NewMachine(MachineSpec{}), true}, - - {Query{"product": "R630", "datacenter": "jp"}, NewMachine(MachineSpec{Product: "R630", Datacenter: "us"}), false}, - {Query{"product": "R730", "datacenter": "us"}, NewMachine(MachineSpec{Product: "R630", Datacenter: "us"}), false}, + {Query{"labels": "product=R630"}, NewMachine(MachineSpec{Labels: map[string]string{"product": "R630", "datacenter": "jp"}}), true}, + {Query{"labels": "product=R630,datacenter=jp"}, NewMachine(MachineSpec{Labels: map[string]string{"product": "R630"}}), false}, + {Query{"labels": "product=R630,datacenter=jp"}, NewMachine(MachineSpec{Labels: map[string]string{"product": "R630", "datacenter": "us"}}), false}, + {Query{"labels": "product=R730,datacenter=us"}, NewMachine(MachineSpec{Labels: map[string]string{"product": "R630", "datacenter": "us"}}), false}, {Query{"ipv4": "10.20.30.40"}, NewMachine(MachineSpec{IPv4: []string{"10.21.30.40", "10.22.30.40"}}), false}, {Query{"ipv6": "aa::ff"}, NewMachine(MachineSpec{IPv6: []string{"bb::ff", "cc::ff"}}), false}, {Query{"ipv4": "10.20.30.40"}, NewMachine(MachineSpec{}), false}, diff --git a/web/coreos_test.go b/web/coreos_test.go index b6f2074c..1e689b6d 100644 --- a/web/coreos_test.go +++ b/web/coreos_test.go @@ -42,12 +42,14 @@ func testHandleiPXEWithSerial(t *testing.T) { machines := []*sabakan.Machine{ sabakan.NewMachine(sabakan.MachineSpec{ - Serial: "2222abcd", - Product: "R630", - Datacenter: "ty3", - Rack: 1, - Role: "cs", - BMC: sabakan.MachineBMC{Type: "iDRAC-9"}, + Serial: "2222abcd", + Labels: map[string]string{ + "product": "R630", + "datacenter": "ty3", + }, + Rack: 1, + Role: "cs", + BMC: sabakan.MachineBMC{Type: "iDRAC-9"}, }), } diff --git a/web/ignitions_test.go b/web/ignitions_test.go index 72faa280..f9b873e4 100644 --- a/web/ignitions_test.go +++ b/web/ignitions_test.go @@ -88,13 +88,15 @@ networkd: machines := []*sabakan.Machine{ sabakan.NewMachine(sabakan.MachineSpec{ - Serial: "2222abcd", - Product: "R630", - Datacenter: "ty3", - Rack: 1, - Role: "cs", - IPv4: []string{"10.69.0.4"}, - BMC: sabakan.MachineBMC{Type: "iDRAC-9"}, + Serial: "2222abcd", + Labels: map[string]string{ + "product": "R630", + "datacenter": "ty3", + }, + Rack: 1, + Role: "cs", + IPv4: []string{"10.69.0.4"}, + BMC: sabakan.MachineBMC{Type: "iDRAC-9"}, }), } diff --git a/web/machines.go b/web/machines.go index 7ddc547f..f4edb7ac 100644 --- a/web/machines.go +++ b/web/machines.go @@ -3,11 +3,17 @@ package web import ( "encoding/json" "net/http" + "regexp" "strings" "github.com/cybozu-go/sabakan" ) +var ( + labelValueRegex = regexp.MustCompile(`[[:print:]]+`) + labelKeyRegex = regexp.MustCompile(`[a-z0-9A-Z-_/.]+`) +) + func (s Server) handleMachines(w http.ResponseWriter, r *http.Request) { switch r.Method { case "GET": @@ -40,18 +46,18 @@ func (s Server) handleMachinesPost(w http.ResponseWriter, r *http.Request) { renderError(r.Context(), w, BadRequest("serial is empty")) return } - if m.Product == "" { - renderError(r.Context(), w, BadRequest("product is empty")) - return - } - if m.Datacenter == "" { - renderError(r.Context(), w, BadRequest("datacenter is empty")) - return - } if !sabakan.IsValidRole(m.Role) { renderError(r.Context(), w, BadRequest("invalid role")) return } + if len(m.Labels) > 0 { + for k, v := range m.Labels { + if !labelKeyRegex.MatchString(k) || !labelValueRegex.MatchString(v) { + renderError(r.Context(), w, BadRequest("labels contain invalid character")) + return + } + } + } if m.BMC.Type == "" { renderError(r.Context(), w, BadRequest("BMC type is empty")) return diff --git a/web/machines_test.go b/web/machines_test.go index 5e6ffd60..4123863d 100644 --- a/web/machines_test.go +++ b/web/machines_test.go @@ -26,71 +26,129 @@ func testMachinesPost(t *testing.T) { }{ {`[{ "serial": "1234abcd", - "product": "R630", - "datacenter": "ty3", + "labels": { + "product": "R630", + "datacenter": "ty3" + }, "rack": 1, "role": "boot", "bmc": {"type": "iDRAC-9"} }]`, http.StatusCreated}, {`[{ "serial": "1234abcd", - "product": "R630", - "datacenter": "ty3", + "labels": { + "product": "R630", + "datacenter": "ty3" + }, "rack": 1, "role": "boot", "bmc": {"type": "iDRAC-9"} }]`, http.StatusConflict}, {`[{ - "product": "R630", - "datacenter": "ty3", + "serial": "1234abcd", + "labels": { + "product": "R630" + }, + "rack": 1, + "role": "boot", + "bmc": {"type": "iDRAC-9"} +}]`, http.StatusConflict}, + {`[{ + "labels": { + "product": "R630", + "datacenter": "ty3" + }, "rack": 1, "role": "boot", "bmc": {"type": "iDRAC-9"} }]`, http.StatusBadRequest}, {`[{ "serial": "5678abcd", - "datacenter": "ty3", + "labels": { + "datacenter": "ty3" + }, "rack": 1, "role": "boot", "bmc": {"type": "iDRAC-9"} -}]`, http.StatusBadRequest}, +}]`, http.StatusCreated}, {`[{ "serial": "0000abcd", - "product": "R630", + "labels": { + "product": "R630" + }, "rack": 1, "role": "boot", "bmc": {"type": "iDRAC-9"} -}]`, http.StatusBadRequest}, +}]`, http.StatusCreated}, {`[{ "serial": "2222abcd", - "product": "R630", - "datacenter": "ty3", + "labels": { + "product": "R630", + "datacenter": "ty3" + }, "rack": 1, "bmc": {"type": "iDRAC-9"} }]`, http.StatusBadRequest}, {`[{ "serial": "2222abcd", - "product": "R630", - "datacenter": "ty3", + "labels": { + "product": "R630", + "datacenter": "ty3" + }, "rack": 1, "role": "boot" }]`, http.StatusBadRequest}, {`[{ "serial": "2222abcd", - "product": "R630", - "datacenter": "ty3", + "labels": { + "product": "R630", + "datacenter": "ty3" + }, "rack": 1, "role": "boot", "bmc": {"type": "unknown-BMC"} }]`, http.StatusBadRequest}, {`[{ "serial": "2222abcd", - "product": "R630", - "datacenter": "ty3", + "labels": { + "product": "R630", + "datacenter": "ty3" + }, + "rack": 1, + "role": "invalid/Role", + "bmc": {"type": "iDRAC-9"} +}]`, http.StatusBadRequest}, + {`[{ + "serial": "2222abcd", + "labels": { + "~+ = ';": "invalidkey" + }, + "rack": 1, + "role": "invalid/Role", + "bmc": {"type": "iDRAC-9"} +}]`, http.StatusBadRequest}, + {`[{ + "serial": "2222abcd", + "labels": { + "invalid_value": "\n\t" + }, "rack": 1, "role": "invalid/Role", "bmc": {"type": "iDRAC-9"} }]`, http.StatusBadRequest}, + {`[{ + "serial": "3333abcd", + "labels": {}, + "rack": 1, + "role": "boot", + "bmc": {"type": "iDRAC-9"} +}]`, http.StatusCreated}, + {`[{ + "serial": "3333efgh", + "rack": 1, + "role": "boot", + "bmc": {"type": "iDRAC-9"} +}]`, http.StatusCreated}, } for _, c := range cases { @@ -120,28 +178,34 @@ func testMachinesGet(t *testing.T) { m.Machine.Register(context.Background(), []*sabakan.Machine{ sabakan.NewMachine(sabakan.MachineSpec{ - Serial: "1234abcd", - Product: "R630", - Datacenter: "ty3", - Rack: 1, - Role: "boot", - BMC: sabakan.MachineBMC{Type: sabakan.BmcIdrac9}, + Serial: "1234abcd", + Labels: map[string]string{ + "product": "R630", + "datacenter": "ty3", + }, + Rack: 1, + Role: "boot", + BMC: sabakan.MachineBMC{Type: sabakan.BmcIdrac9}, }), sabakan.NewMachine(sabakan.MachineSpec{ - Serial: "5678abcd", - Product: "R740", - Datacenter: "ty3", - Rack: 1, - Role: "worker", - BMC: sabakan.MachineBMC{Type: sabakan.BmcIdrac9}, + Serial: "5678abcd", + Labels: map[string]string{ + "product": "R740", + "datacenter": "ty3", + }, + Rack: 1, + Role: "worker", + BMC: sabakan.MachineBMC{Type: sabakan.BmcIdrac9}, }), sabakan.NewMachine(sabakan.MachineSpec{ - Serial: "1234efgh", - Product: "R630", - Datacenter: "ty3", - Rack: 2, - Role: "boot", - BMC: sabakan.MachineBMC{Type: sabakan.BmcIpmi2}, + Serial: "1234efgh", + Labels: map[string]string{ + "product": "R630", + "datacenter": "ty3", + }, + Rack: 2, + Role: "boot", + BMC: sabakan.MachineBMC{Type: sabakan.BmcIpmi2}, }), }) @@ -156,12 +220,12 @@ func testMachinesGet(t *testing.T) { expected: map[string]bool{"1234abcd": true}, }, { - query: map[string][]string{"product": {"R630"}}, + query: map[string][]string{"labels": {"product=R630"}}, status: http.StatusOK, expected: map[string]bool{"1234abcd": true, "1234efgh": true}, }, { - query: map[string][]string{"datacenter": {"ty3"}}, + query: map[string][]string{"labels": {"datacenter=ty3"}}, status: http.StatusOK, expected: map[string]bool{"1234abcd": true, "5678abcd": true, "1234efgh": true}, }, @@ -233,18 +297,22 @@ func testMachinesDelete(t *testing.T) { handler := newTestServer(m) m1 := sabakan.NewMachine(sabakan.MachineSpec{ - Serial: "1234abcd", - Product: "R630", - Datacenter: "ty3", - Rack: 1, - Role: "boot", + Serial: "1234abcd", + Labels: map[string]string{ + "product": "R630", + "datacenter": "ty3", + }, + Rack: 1, + Role: "boot", }) m2 := sabakan.NewMachine(sabakan.MachineSpec{ - Serial: "qqq", - Product: "R630", - Datacenter: "ty3", - Rack: 2, - Role: "cs", + Serial: "qqq", + Labels: map[string]string{ + "product": "R630", + "datacenter": "ty3", + }, + Rack: 2, + Role: "cs", }) m2.Status.State = sabakan.StateRetired From e2270818ec040f755f4d477805b460182d6c1832 Mon Sep 17 00:00:00 2001 From: Shoma Kokuryo Date: Thu, 23 Aug 2018 04:15:16 +0000 Subject: [PATCH 2/7] implement indexing by labals --- models/etcd/index.go | 20 ++++++++++++++++++++ query.go | 15 +++++++++------ 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/models/etcd/index.go b/models/etcd/index.go index e30f36ac..c9f025b3 100644 --- a/models/etcd/index.go +++ b/models/etcd/index.go @@ -10,11 +10,16 @@ import ( "github.com/cybozu-go/sabakan" ) +const ( + labelSep = "=" +) + // machinesIndex is on-memory index of the etcd values type machinesIndex struct { mux sync.RWMutex Rack map[string][]string Role map[string][]string + Labels map[string][]string IPv4 map[string]string IPv6 map[string]string BMCType map[string][]string @@ -25,6 +30,7 @@ func newMachinesIndex() *machinesIndex { return &machinesIndex{ Rack: make(map[string][]string), Role: make(map[string][]string), + Labels: make(map[string][]string), IPv4: make(map[string]string), IPv6: make(map[string]string), BMCType: make(map[string][]string), @@ -84,6 +90,10 @@ func (mi *machinesIndex) addNoLock(m *sabakan.Machine) { mi.IPv6[spec.BMC.IPv6] = spec.Serial } mi.State[m.Status.State] = append(mi.State[m.Status.State], spec.Serial) + for k, v := range spec.Labels { + labelKey := k + labelSep + v + mi.Labels[labelKey] = append(mi.Labels[labelKey], spec.Serial) + } } func indexOf(data []string, element string) int { @@ -121,6 +131,11 @@ func (mi *machinesIndex) deleteNoLock(m *sabakan.Machine) { i = indexOf(mi.State[m.Status.State], spec.Serial) mi.State[m.Status.State] = append(mi.State[m.Status.State][:i], mi.State[m.Status.State][i+1:]...) + for k, v := range spec.Labels { + labelKey := k + labelSep + v + i = indexOf(mi.Labels[labelKey], spec.Serial) + mi.Labels[labelKey] = append(mi.Labels[labelKey][:i], mi.Labels[labelKey][i+1:]...) + } } // UpdateIndex updates target machine on the index @@ -159,6 +174,11 @@ func (mi *machinesIndex) query(q sabakan.Query) []string { for _, serial := range mi.State[sabakan.MachineState(q.State())] { res[serial] = struct{}{} } + for _, labelKey := range q.Labels() { + for _, serial := range mi.Labels[labelKey] { + res[serial] = struct{}{} + } + } serials := make([]string, 0, len(res)) for serial := range res { diff --git a/query.go b/query.go index d924dd92..ca43240e 100644 --- a/query.go +++ b/query.go @@ -75,12 +75,6 @@ func (q Query) Match(m *Machine) bool { // Serial returns value of serial in the query func (q Query) Serial() string { return q["serial"] } -// Product returns value of product in the query -func (q Query) Product() string { return q["product"] } - -// Datacenter returns value of datacenter in the query -func (q Query) Datacenter() string { return q["datacenter"] } - // Rack returns value of rack in the query func (q Query) Rack() string { return q["rack"] } @@ -99,6 +93,15 @@ func (q Query) BMCType() string { return q["bmc-type"] } // State returns value of state the query func (q Query) State() string { return q["state"] } +// Labels return label's key and value combined with '=' +func (q Query) Labels() []string { + queries := strings.Split(q["labels"], ",") + for idx, rawQuery := range queries { + queries[idx] = strings.TrimSpace(rawQuery) + } + return queries +} + // IsEmpty returns true if query is empty or no values are presented func (q Query) IsEmpty() bool { for _, v := range q { From d5206ff1c62bc3ae1cbf143e21134d42e24845b0 Mon Sep 17 00:00:00 2001 From: Shoma Kokuryo Date: Thu, 23 Aug 2018 04:37:26 +0000 Subject: [PATCH 3/7] fix regex --- web/machines.go | 4 ++-- web/machines_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/web/machines.go b/web/machines.go index f4edb7ac..08c98234 100644 --- a/web/machines.go +++ b/web/machines.go @@ -10,8 +10,8 @@ import ( ) var ( - labelValueRegex = regexp.MustCompile(`[[:print:]]+`) - labelKeyRegex = regexp.MustCompile(`[a-z0-9A-Z-_/.]+`) + labelValueRegex = regexp.MustCompile(`^[[:print:]]+$`) + labelKeyRegex = regexp.MustCompile(`^[a-z0-9A-Z-_/.]+$`) ) func (s Server) handleMachines(w http.ResponseWriter, r *http.Request) { diff --git a/web/machines_test.go b/web/machines_test.go index 4123863d..4f2aeec0 100644 --- a/web/machines_test.go +++ b/web/machines_test.go @@ -121,7 +121,7 @@ func testMachinesPost(t *testing.T) { {`[{ "serial": "2222abcd", "labels": { - "~+ = ';": "invalidkey" + "invalidKey~+ = ';": "invalidkey" }, "rack": 1, "role": "invalid/Role", @@ -130,7 +130,7 @@ func testMachinesPost(t *testing.T) { {`[{ "serial": "2222abcd", "labels": { - "invalid_value": "\n\t" + "invalid_value": "invalid\n\tvalue" }, "rack": 1, "role": "invalid/Role", From 5a968602d49bbef180771ddde08fbb67f7372888 Mon Sep 17 00:00:00 2001 From: Shoma Kokuryo Date: Thu, 23 Aug 2018 04:53:56 +0000 Subject: [PATCH 4/7] update docs --- docs/api.md | 10 ++++------ docs/getting_started.md | 6 ++++-- docs/machine.md | 25 +++++++++++++------------ docs/sabactl.md | 7 +++---- 4 files changed, 24 insertions(+), 24 deletions(-) diff --git a/docs/api.md b/docs/api.md index e2af23d2..d7ad5cd6 100644 --- a/docs/api.md +++ b/docs/api.md @@ -179,10 +179,9 @@ In the HTTP request body, specify the following list of the machine information Field | Description ----- | ----------- `serial=` | The serial number of the machine -`datacenter=` | The data center name where the machine is in +`labels={, ...}` | The labels of the machine `rack=` | The rack number where the machine is in. If it is omitted, value set to `0` `role=` | The role of the machine (e.g. `boot` or `worker`) -`product=` | The product name of the machine (e.g. `R630`) `bmc=` | The BMC spec **Successful response** @@ -208,8 +207,8 @@ Field | Description ```console $ curl -s -X POST 'localhost:10080/api/v1/machines' -d ' [ - { "serial": "1234abcd", "product": "R630", "datacenter": "ty3", "rack": 1, "role": "boot", "bmc": {"type": "iDRAC-9"} }, - { "serial": "2345bcde", "product": "R630", "datacenter": "ty3", "rack": 1, "role": "worker", "bmc": {"type": "iDRAC-9"} } + { "serial": "1234abcd", "labels": {"product": "R630", "datacenter": "ty3"}, "rack": 1, "role": "boot", "bmc": {"type": "iDRAC-9"} }, + { "serial": "2345bcde", "labels": {"product": "R630", "datacenter": "ty3"}, "rack": 1, "role": "worker", "bmc": {"type": "iDRAC-9"} } ]' ``` @@ -220,10 +219,9 @@ Search registered machines. A user can specify the following URL queries. Query | Description ----- | ----------- `serial=` | The serial number of the machine -`datacenter=` | The data center name where the machine is in +`labels=,...` | The labels of the machine. `rack=` | The rack number where the machine is in `role=` | The role of the machine -`product=` | The product name of the machine(e.g. `R630`) `ipv4=` | IPv4 address `ipv6=` | IPv6 address `bmc-type=` | BMC type diff --git a/docs/getting_started.md b/docs/getting_started.md index b6703b65..d4d28240 100644 --- a/docs/getting_started.md +++ b/docs/getting_started.md @@ -130,8 +130,10 @@ Prepare `machines.json` as follows: [ { "serial": "1234abcd", - "product": "Dell R640", - "datacenter": "tokyo1", + "labels": { + "product": "Dell R640", + "datacenter": "tokyo1", + } "rack": 0, "role": "boot", "bmc": { diff --git a/docs/machine.md b/docs/machine.md index 87b47282..5f8be342 100644 --- a/docs/machine.md +++ b/docs/machine.md @@ -10,16 +10,15 @@ MachineSpec struct `MachineSpec` can be represented as a JSON object having these fields: -Field | Type | Description ---------------- | -------- | ----------- -`serial` | string | SMBIOS serial number of the machine. -`product` | string | Product name of the machine -`datacenter` | string | Data center name where the machine exists. -`rack` | int | Logical rack number (LRN) where the machine exists. -`index-in-rack` | int | Logical position in a rack. -`ipv4` | []string | IPv4 addresses for OS. -`ipv6` | []string | IPv6 addresses for OS. -`bmc` | object | BMC parameters; See below. +Field | Type | Description +--------------- | -------- | ----------- +`serial` | string | SMBIOS serial number of the machine. +`labels` | map[string]string | Labels of the machine such as `product` or `datacenter` +`rack` | int | Logical rack number (LRN) where the machine exists. +`index-in-rack` | int | Logical position in a rack. +`ipv4` | []string | IPv4 addresses for OS. +`ipv6` | []string | IPv6 addresses for OS. +`bmc` | object | BMC parameters; See below. Key in `bmc` | Type | Description --------------- | ------ | ----------- @@ -41,8 +40,10 @@ A JSON representation of `Machine` looks like: { "spec": { "serial": "1234abcd", - "product": "Dell R630", - "datacenter": "tokyo1", + "labels": { + "product": "Dell R630", + "datacenter": "tokyo1", + } "rack": 1, "index-in-rack": 1, "role": "boot", diff --git a/docs/sabactl.md b/docs/sabactl.md index f2a99529..047c2ae1 100644 --- a/docs/sabactl.md +++ b/docs/sabactl.md @@ -62,8 +62,8 @@ Detailed specification of the input JSON file is same as that of the [`POST /api ```json [ - { "serial": "", "datacenter": "", "rack": "", "product": "", "role": "", "bmc": { "type": "iDRAC-9" }}, - { "serial": "", "datacenter": "", "rack": "", "product": "", "role": "", "bmc": { "type": "iDRAC-9" }} + { "serial": "", "labels": {"product": "", "datacenter": ""}, "rack": "", "role": "", "bmc": { "type": "iDRAC-9" }}, + { "serial": "", "labels": {"product": "", "datacenter": ""}, "rack": "", "role": "", "bmc": { "type": "iDRAC-9" }} ] ``` @@ -75,10 +75,9 @@ Show machines filtered by query parameters. ```console $ sabactl machines get \ [--serial ] \ - [--datacenter ] \ [--rack ] \ [--role ] \ - [--product ] \ + [--labels ,...] [--ipv4 ] \ [--ipv6 ] \ [--bmc-type ] \ From 58bbcc11279e0bf6ddf4b9c8eccee8f88c3d8181 Mon Sep 17 00:00:00 2001 From: Shoma Kokuryo Date: Thu, 23 Aug 2018 06:30:01 +0000 Subject: [PATCH 5/7] fix redundant trailing comma --- docs/getting_started.md | 2 +- docs/machine.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/getting_started.md b/docs/getting_started.md index d4d28240..544b6301 100644 --- a/docs/getting_started.md +++ b/docs/getting_started.md @@ -132,7 +132,7 @@ Prepare `machines.json` as follows: "serial": "1234abcd", "labels": { "product": "Dell R640", - "datacenter": "tokyo1", + "datacenter": "tokyo1" } "rack": 0, "role": "boot", diff --git a/docs/machine.md b/docs/machine.md index 5f8be342..a7dced56 100644 --- a/docs/machine.md +++ b/docs/machine.md @@ -42,7 +42,7 @@ A JSON representation of `Machine` looks like: "serial": "1234abcd", "labels": { "product": "Dell R630", - "datacenter": "tokyo1", + "datacenter": "tokyo1" } "rack": 1, "index-in-rack": 1, From fce0d6b934a6ff81ba6c8d0a586b9af5c10c3d86 Mon Sep 17 00:00:00 2001 From: Shoma Kokuryo Date: Thu, 23 Aug 2018 07:08:41 +0000 Subject: [PATCH 6/7] implement validation process as method --- machines.go | 14 +++++++++++++- web/machines.go | 8 +------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/machines.go b/machines.go index f018cea0..910f0860 100644 --- a/machines.go +++ b/machines.go @@ -31,7 +31,9 @@ const ( ) var ( - reValidRole = regexp.MustCompile(`^[a-zA-Z][0-9a-zA-Z._-]*$`) + reValidRole = regexp.MustCompile(`^[a-zA-Z][0-9a-zA-Z._-]*$`) + reValidLabelVal = regexp.MustCompile(`^[[:print:]]+$`) + reValidLabelName = regexp.MustCompile(`^[a-z0-9A-Z-_/.]+$`) ) // IsValidRole returns true if role is valid as machine role @@ -39,6 +41,16 @@ func IsValidRole(role string) bool { return reValidRole.MatchString(role) } +// IsValidLabelName returns true if label name is valid +func IsValidLabelName(name string) bool { + return reValidLabelName.MatchString(name) +} + +// IsValidLabelValue returns true if label value is valid +func IsValidLabelValue(value string) bool { + return reValidLabelVal.MatchString(value) +} + // MachineBMC is a bmc interface struct for Machine type MachineBMC struct { IPv4 string `json:"ipv4"` diff --git a/web/machines.go b/web/machines.go index 08c98234..4752bb54 100644 --- a/web/machines.go +++ b/web/machines.go @@ -3,17 +3,11 @@ package web import ( "encoding/json" "net/http" - "regexp" "strings" "github.com/cybozu-go/sabakan" ) -var ( - labelValueRegex = regexp.MustCompile(`^[[:print:]]+$`) - labelKeyRegex = regexp.MustCompile(`^[a-z0-9A-Z-_/.]+$`) -) - func (s Server) handleMachines(w http.ResponseWriter, r *http.Request) { switch r.Method { case "GET": @@ -52,7 +46,7 @@ func (s Server) handleMachinesPost(w http.ResponseWriter, r *http.Request) { } if len(m.Labels) > 0 { for k, v := range m.Labels { - if !labelKeyRegex.MatchString(k) || !labelValueRegex.MatchString(v) { + if !sabakan.IsValidLabelName(k) || !sabakan.IsValidLabelValue(v) { renderError(r.Context(), w, BadRequest("labels contain invalid character")) return } From d4025c5cd71929735816d9d2489c242eedb4c7de Mon Sep 17 00:00:00 2001 From: Shoma Kokuryo Date: Thu, 23 Aug 2018 07:39:58 +0000 Subject: [PATCH 7/7] add test for label validation --- machines_test.go | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/machines_test.go b/machines_test.go index 3ff3db91..ca3e2ac0 100644 --- a/machines_test.go +++ b/machines_test.go @@ -22,6 +22,40 @@ func TestIsValidRole(t *testing.T) { } } +func TestIsValidLabelName(t *testing.T) { + t.Parallel() + + validNames := []string{"valid_name1", "valid-name2", "valid/name3"} + for _, vn := range validNames { + if !IsValidLabelName(vn) { + t.Error("validator should return true:", vn) + } + } + invalidNames := []string{"^in;valid name\\1", "in$valid#name&2", "invalid@name=3"} + for _, ivn := range invalidNames { + if IsValidLabelName(ivn) { + t.Error("validator should return false:", ivn) + } + } +} + +func TestIsValidLabelValue(t *testing.T) { + t.Parallel() + + validVals := []string{"^valid value@1", "valid$value-=2", "%valid':value;3"} + for _, vv := range validVals { + if !IsValidLabelValue(vv) { + t.Error("validator should return true:", vv) + } + } + invalidVals := []string{`inválidvaluõ1`, `iñvalidvålue`} + for _, ivv := range invalidVals { + if IsValidLabelValue(ivv) { + t.Error("validator should return false:", ivv) + } + } +} + func TestMachine(t *testing.T) { t.Parallel()