From 717a240b1d5d1f282afcdde40bf1262aece03487 Mon Sep 17 00:00:00 2001 From: Dalton Hubble Date: Wed, 9 Mar 2016 02:23:56 -0800 Subject: [PATCH] config: Add bytes JSON to storagepb.Group, parse YAML config * Allow bootcfg groups metadata to be defined via YAML * Convert YAML metadata to JSON bytes field in a protobuf message * Strip metadata except strings, string slices, and string-keyed maps with nesting --- Documentation/config.md | 2 +- bootcfg/config/config.go | 147 +++++++++++++++++------- bootcfg/config/config_test.go | 103 ++++++++++++++--- bootcfg/storage/storagepb/storage.pb.go | 56 ++++----- bootcfg/storage/storagepb/storage.proto | 4 +- cmd/bootcfg-rpc/main.go | 2 +- cmd/bootcfg/main.go | 2 +- 7 files changed, 218 insertions(+), 98 deletions(-) diff --git a/Documentation/config.md b/Documentation/config.md index 01037e69..e2597a17 100644 --- a/Documentation/config.md +++ b/Documentation/config.md @@ -26,6 +26,6 @@ Run Run with a fake signing key. export BOOTCFG_PASSPHRASE=test - ./bin/bootcfg -address=0.0.0.0:8080 -key-ring-path sign/fixtures/secring.gpg -data-path examples/ -config examples/etcd-rkt.yaml + ./bin/bootcfg -address=0.0.0.0:8080 -key-ring-path bootcfg/sign/fixtures/secring.gpg -data-path examples/ -config examples/etcd-rkt.yaml diff --git a/bootcfg/config/config.go b/bootcfg/config/config.go index af082af5..70452ace 100644 --- a/bootcfg/config/config.go +++ b/bootcfg/config/config.go @@ -1,6 +1,7 @@ package config import ( + "encoding/json" "errors" "fmt" "io/ioutil" @@ -10,6 +11,7 @@ import ( "github.com/coreos/coreos-baremetal/bootcfg/api" "github.com/coreos/coreos-baremetal/bootcfg/storage/storagepb" + "github.com/coreos/pkg/capnslog" "github.com/satori/go.uuid" "gopkg.in/yaml.v2" ) @@ -19,15 +21,20 @@ const ( APIVersion = "v1alpha1" ) -// Config parse errors +// Config parse errors. var ( - ErrIncorrectVersion = errors.New("api: incorrect API version") + ErrIncorrectVersion = errors.New("config: incorrect API version") ) +var log = capnslog.NewPackageLogger("github.com/coreos/coreos-baremetal/bootcfg", "config") + // Config is a user defined matching of machines to specifications. type Config struct { - APIVersion string `yaml:"api_version"` - Groups []api.Group `yaml:"groups"` + APIVersion string `yaml:"api_version"` + // allow YAML source for Groups + YAMLGroups []api.Group `yaml:"groups"` + // populate protobuf Groups at parse + Groups []*storagepb.Group `yaml:"-"` } // LoadConfig opens a file and parses YAML data to returns a Config. @@ -46,30 +53,112 @@ func ParseConfig(data []byte) (*Config, error) { if err != nil { return nil, err } - // normalize MAC addresses - for _, group := range config.Groups { - for key, val := range group.Matcher { - switch strings.ToLower(key) { - case "mac": - if macAddr, err := net.ParseMAC(val); err == nil { - // range iteration copy with mutable map - group.Matcher[key] = macAddr.String() - } - } + // convert YAML Groups into protobuf Groups + config.Groups = make([]*storagepb.Group, 0) + for _, ygroup := range config.YAMLGroups { + group := &storagepb.Group{ + Name: ygroup.Name, + Profile: ygroup.Spec, + Requirements: normalizeMatchers(ygroup.Matcher), } + // Id: Generate a random UUID or use the name + if ygroup.Name == "" { + group.Id = uuid.NewV4().String() + } else { + group.Id = group.Name + } + // Metadata: go-yaml unmarshal provides Config.Metadata as a + // map[string]interface{}, which unmarshals nested maps as + // map[interface{}]interface{}. Walk the metadata, filtering non-string + // keys and nested elements. + if b, err := json.Marshal(filterValues(ygroup.Metadata)); err == nil { + group.Metadata = b + } else { + return nil, fmt.Errorf("config: cannot marshal metadata %v", err) + } + config.Groups = append(config.Groups, group) } + // validate Config and Groups if err := config.validate(); err != nil { return nil, err } return config, nil } +func normalizeMatchers(reqs map[string]string) map[string]string { + for key, val := range reqs { + switch strings.ToLower(key) { + case "mac": + if macAddr, err := net.ParseMAC(val); err == nil { + // range iteration copy with mutable map + reqs[key] = macAddr.String() + log.Errorf("normalizing MAC address %s to %s", val, macAddr.String()) + } + } + } + return reqs +} + +// filterValues returns a new map, filtering out key/value pairs whose value +// is not a string, []string, or string key'd map. Recurses on interface +// values. +func filterValues(unknown map[string]interface{}) map[string]interface{} { + // copy the map, skipping all disallowed value types + m := make(map[string]interface{}) + for key, v := range unknown { + switch val := v.(type) { + case string: + m[key] = val + case []string: + m[key] = val + case []interface{}: + m[key] = filterSlice(val) + case map[interface{}]interface{}: + m[key] = filterValues(filterNonStringKeys(val)) + default: + log.Errorf("ignoring metadata value %v", val) + } + } + return m +} + +// filterSlice returns a new slice, filtering out elements whose keys are not +// strings. +func filterSlice(unknown []interface{}) []string { + s := make([]string, 0, len(unknown)) + for _, e := range unknown { + switch elem := e.(type) { + case string: + s = append(s, elem) + default: + log.Errorf("ignoring metadata elem %v", elem) + } + } + return s +} + +// filterNonStringKeys returns a new map, filtering out key/value pairs whose +// keys are not strings. +func filterNonStringKeys(unknown map[interface{}]interface{}) map[string]interface{} { + // copy the map, skipping all non-string keys + m := make(map[string]interface{}) + for k, val := range unknown { + switch key := k.(type) { + case string: + m[key] = val + default: + log.Errorf("ignoring metadata key %v", key) + } + } + return m +} + // validate the group config's API version and reserved tag matchers. func (c *Config) validate() error { if c.APIVersion != APIVersion { return ErrIncorrectVersion } - for _, group := range c.Groups { + for _, group := range c.YAMLGroups { for key, val := range group.Matcher { switch strings.ToLower(key) { case "mac": @@ -85,31 +174,3 @@ func (c *Config) validate() error { } return nil } - -// PBGroups returns the parsed storagepb.Group slice. -func (c *Config) PBGroups() []*storagepb.Group { - groups := make([]*storagepb.Group, len(c.Groups)) - i := 0 - for _, g := range c.Groups { - group := &storagepb.Group{ - Id: uuid.NewV4().String(), - Name: g.Name, - Profile: g.Spec, - Metadata: make(map[string]string), - Requirements: g.Matcher, - } - // gRPC message fields must have concrete types. - // Limit YAML metadata nesting to a depth of 1 for now. - for key, unknown := range g.Metadata { - switch val := unknown.(type) { - case string: - group.Metadata[key] = val - default: - // skip subtree - } - } - groups[i] = group - i++ - } - return groups -} diff --git a/bootcfg/config/config_test.go b/bootcfg/config/config_test.go index ccbb6d27..06af6637 100644 --- a/bootcfg/config/config_test.go +++ b/bootcfg/config/config_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/coreos/coreos-baremetal/bootcfg/api" + "github.com/coreos/coreos-baremetal/bootcfg/storage/storagepb" "github.com/stretchr/testify/assert" ) @@ -19,20 +20,27 @@ groups: role: worker region: us-central1-a mac: aB:Ab:3d:45:cD:10 + metadata: + a: b + c: + - d + - e + f: + g: + h ` -var validConfig = &Config{ - APIVersion: "v1alpha1", - Groups: []api.Group{ - api.Group{ - Name: "node1", - Spec: "worker", - Matcher: api.RequirementSet(map[string]string{ - "role": "worker", - "region": "us-central1-a", - "mac": "ab:ab:3d:45:cd:10", - }), +var validGroups = []*storagepb.Group{ + &storagepb.Group{ + Id: "node1", + Name: "node1", + Profile: "worker", + Requirements: map[string]string{ + "role": "worker", + "region": "us-central1-a", + "mac": "ab:ab:3d:45:cd:10", }, + Metadata: []byte(`{"a":"b","c":["d","e"],"f":{"g":"h"}}`), }, } @@ -43,8 +51,9 @@ func TestLoadConfig(t *testing.T) { f.Write([]byte(validData)) config, err := LoadConfig(f.Name()) - assert.Equal(t, validConfig, config) assert.Nil(t, err) + assert.Equal(t, "v1alpha1", config.APIVersion) + assert.Equal(t, validGroups, config.Groups) // read from file that does not exist config, err = LoadConfig("") assert.Nil(t, config) @@ -57,18 +66,19 @@ func TestParseConfig(t *testing.T) { cases := []struct { data string - expectedConfig *Config + expectedGroups []*storagepb.Group expectedErr error }{ - {validData, validConfig, nil}, - + {validData, validGroups, nil}, {invalidData, nil, ErrIncorrectVersion}, {invalidYAML, nil, fmt.Errorf("yaml: found character that cannot start any token")}, } for _, c := range cases { config, err := ParseConfig([]byte(c.data)) - assert.Equal(t, c.expectedConfig, config) assert.Equal(t, c.expectedErr, err) + if c.expectedErr == nil { + assert.Equal(t, c.expectedGroups, config.Groups) + } } } @@ -78,7 +88,7 @@ func TestValidate(t *testing.T) { } invalidMAC := &Config{ APIVersion: "v1alpha1", - Groups: []api.Group{ + YAMLGroups: []api.Group{ api.Group{ Matcher: api.RequirementSet(map[string]string{ "mac": "?:?:?:?", @@ -88,7 +98,7 @@ func TestValidate(t *testing.T) { } nonNormalizedMAC := &Config{ APIVersion: "v1alpha1", - Groups: []api.Group{ + YAMLGroups: []api.Group{ api.Group{ Matcher: api.RequirementSet(map[string]string{ "mac": "aB:Ab:3d:45:cD:10", @@ -96,6 +106,20 @@ func TestValidate(t *testing.T) { }, }, } + validConfig := &Config{ + APIVersion: "v1alpha1", + YAMLGroups: []api.Group{ + api.Group{ + Name: "node1", + Spec: "worker", + Matcher: api.RequirementSet(map[string]string{ + "role": "worker", + "region": "us-central1-a", + "mac": "ab:ab:3d:45:cd:10", + }), + }, + }, + } cases := []struct { config *Config @@ -110,3 +134,46 @@ func TestValidate(t *testing.T) { assert.Equal(t, c.expectedErr, c.config.validate()) } } + +func TestFilterSlice(t *testing.T) { + s := []interface{}{"a", 3.14, "b", "c"} + expected := []string{"a", "b", "c"} + filtered := filterSlice(s) + assert.Equal(t, expected, filtered) +} + +func TestFilterKeys(t *testing.T) { + m := map[interface{}]interface{}{ + "a": "b", + 3.14: "c", + } + expected := map[string]interface{}{ + "a": "b", + } + filtered := filterNonStringKeys(m) + assert.Equal(t, expected, filtered) +} + +func TestFilterValues(t *testing.T) { + m := map[string]interface{}{ + "a": "b", + "c": 3.14, + "d": map[interface{}]interface{}{ + "e": "f", + 3.14: "g", + "h": []interface{}{"i", "j", "k", 3.14}, + }, + "l": true, + "m": "true", + } + expected := map[string]interface{}{ + "a": "b", + "d": map[string]interface{}{ + "e": "f", + "h": []string{"i", "j", "k"}, + }, + "m": "true", + } + filtered := filterValues(m) + assert.Equal(t, expected, filtered) +} diff --git a/bootcfg/storage/storagepb/storage.pb.go b/bootcfg/storage/storagepb/storage.pb.go index b22c0eb6..7216dda1 100644 --- a/bootcfg/storage/storagepb/storage.pb.go +++ b/bootcfg/storage/storagepb/storage.pb.go @@ -37,8 +37,8 @@ type Group struct { Profile string `protobuf:"bytes,3,opt,name=profile" json:"profile,omitempty"` // tags required to match the group Requirements map[string]string `protobuf:"bytes,4,rep,name=requirements" json:"requirements,omitempty" protobuf_key:"bytes,1,opt,name=key" protobuf_val:"bytes,2,opt,name=value"` - // custom metadata - Metadata map[string]string `protobuf:"bytes,5,rep,name=metadata" json:"metadata,omitempty" protobuf_key:"bytes,1,opt,name=key" protobuf_val:"bytes,2,opt,name=value"` + // JSON encoded metadata (with restrictions) + Metadata []byte `protobuf:"bytes,5,opt,name=metadata,proto3" json:"metadata,omitempty"` } func (m *Group) Reset() { *m = Group{} } @@ -53,13 +53,6 @@ func (m *Group) GetRequirements() map[string]string { return nil } -func (m *Group) GetMetadata() map[string]string { - if m != nil { - return m.Metadata - } - return nil -} - type Profile struct { // profile id Id string `protobuf:"bytes,1,opt,name=id" json:"id,omitempty"` @@ -113,27 +106,26 @@ func init() { } var fileDescriptor0 = []byte{ - // 347 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x09, 0x6e, 0x88, 0x02, 0xff, 0x94, 0x52, 0x4b, 0x4e, 0xc3, 0x30, - 0x10, 0x55, 0x3e, 0xfd, 0x4d, 0x5b, 0x04, 0x23, 0x84, 0x42, 0x17, 0xb4, 0xca, 0x02, 0x75, 0x95, - 0x05, 0x6c, 0xa0, 0x2c, 0x90, 0x40, 0x80, 0x58, 0x80, 0x50, 0x2e, 0x80, 0xd2, 0xc6, 0x54, 0x56, - 0x13, 0xbb, 0xb8, 0x0e, 0x52, 0x8f, 0xc1, 0x4d, 0xb8, 0x05, 0xd7, 0xc2, 0xb1, 0x9d, 0xaa, 0x55, - 0x36, 0x74, 0x37, 0xef, 0xcd, 0x9b, 0x8f, 0xdf, 0x18, 0xfa, 0x2b, 0xc9, 0x45, 0x32, 0x27, 0xd1, - 0x52, 0x70, 0xc9, 0xb1, 0x63, 0xe1, 0x72, 0x1a, 0xfe, 0xba, 0xd0, 0x78, 0x12, 0xbc, 0x58, 0xe2, - 0x01, 0xb8, 0x34, 0x0d, 0x9c, 0x91, 0x33, 0xee, 0xc4, 0x2a, 0x42, 0x04, 0x9f, 0x25, 0x39, 0x09, - 0x5c, 0xcd, 0xe8, 0x18, 0x03, 0x68, 0xa9, 0x0e, 0x1f, 0x34, 0x23, 0x81, 0xa7, 0xe9, 0x0a, 0xe2, - 0x23, 0xf4, 0x04, 0xf9, 0x2c, 0xa8, 0x20, 0x39, 0x61, 0x72, 0x15, 0xf8, 0x23, 0x6f, 0xdc, 0xbd, - 0x08, 0xa3, 0xcd, 0xa4, 0x48, 0x4f, 0x89, 0xe2, 0x2d, 0xd1, 0x03, 0x93, 0x62, 0x1d, 0xef, 0xd4, - 0xe1, 0x04, 0xda, 0x39, 0x91, 0x49, 0x9a, 0xc8, 0x24, 0x68, 0xe8, 0x1e, 0x67, 0xb5, 0x1e, 0x2f, - 0x56, 0x60, 0xea, 0x37, 0xfa, 0xc1, 0x2d, 0x1c, 0xd5, 0xda, 0xe3, 0x21, 0x78, 0x0b, 0xb2, 0xb6, - 0xef, 0x2a, 0x43, 0x3c, 0x86, 0xc6, 0x57, 0x92, 0x15, 0xd5, 0xcb, 0x0c, 0x98, 0xb8, 0x57, 0xce, - 0xe0, 0x06, 0xfa, 0x3b, 0xbd, 0xf7, 0x29, 0x0e, 0xbf, 0x1d, 0x68, 0xbd, 0x59, 0x37, 0xfe, 0xe3, - 0xe5, 0x10, 0xba, 0x74, 0xce, 0xa8, 0xa4, 0x9c, 0xbd, 0x2b, 0xb1, 0xf1, 0x13, 0x2a, 0xea, 0x39, - 0xc5, 0x53, 0x68, 0xcf, 0x32, 0x5e, 0xa4, 0x65, 0xd6, 0x37, 0x6e, 0x6b, 0xac, 0x52, 0xe7, 0xe0, - 0x4f, 0x39, 0x97, 0xca, 0x21, 0x47, 0x39, 0x84, 0x5b, 0x0e, 0xbd, 0x12, 0x79, 0xa7, 0x32, 0xb1, - 0xce, 0x87, 0x3f, 0x6a, 0x27, 0xcb, 0xe0, 0x09, 0x34, 0x17, 0x44, 0x30, 0x92, 0xd9, 0xbd, 0x2c, - 0x2a, 0x79, 0xaa, 0x66, 0x8a, 0x54, 0x6d, 0xe7, 0x95, 0xbc, 0x41, 0x78, 0x0d, 0xad, 0x59, 0x9e, - 0x66, 0x94, 0x95, 0xb7, 0x2e, 0x0f, 0x31, 0xac, 0x8f, 0x89, 0xee, 0x8d, 0xc2, 0x5c, 0xa2, 0xd2, - 0x0f, 0x26, 0xd0, 0xdb, 0x4e, 0xec, 0x63, 0xe3, 0xb4, 0xa9, 0xbf, 0xe8, 0xe5, 0x5f, 0x00, 0x00, - 0x00, 0xff, 0xff, 0x97, 0x23, 0xfd, 0x6e, 0xb3, 0x02, 0x00, 0x00, + // 331 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x09, 0x6e, 0x88, 0x02, 0xff, 0x94, 0x52, 0xcd, 0x4e, 0xf2, 0x40, + 0x14, 0x4d, 0x69, 0xf9, 0xbb, 0xf0, 0x7d, 0xd1, 0x1b, 0x63, 0x2a, 0x1b, 0x48, 0x17, 0x86, 0x55, + 0x17, 0xb8, 0x51, 0x36, 0x26, 0x1a, 0x35, 0x6e, 0x8c, 0xe9, 0x0b, 0x98, 0xc2, 0x8c, 0x64, 0x42, + 0x3b, 0x83, 0xc3, 0xd4, 0x84, 0xc7, 0xf0, 0x4d, 0x7c, 0x2e, 0x9f, 0xc2, 0xf9, 0x2b, 0xa9, 0x61, + 0xa3, 0xbb, 0x7b, 0xce, 0xb9, 0xc3, 0x39, 0xf7, 0x50, 0xf8, 0xb7, 0x55, 0x42, 0xe6, 0x2b, 0x9a, + 0x6e, 0xa4, 0x50, 0x02, 0xfb, 0x1e, 0x6e, 0x16, 0xc9, 0x57, 0x00, 0xed, 0x07, 0x29, 0xaa, 0x0d, + 0xfe, 0x87, 0x16, 0x23, 0x71, 0x30, 0x09, 0xa6, 0xfd, 0x4c, 0x4f, 0x88, 0x10, 0xf1, 0xbc, 0xa4, + 0x71, 0xcb, 0x32, 0x76, 0xc6, 0x18, 0xba, 0xfa, 0x17, 0x5e, 0x59, 0x41, 0xe3, 0xd0, 0xd2, 0x35, + 0xc4, 0x7b, 0x18, 0x4a, 0xfa, 0x56, 0x31, 0x49, 0x4b, 0xca, 0xd5, 0x36, 0x8e, 0x26, 0xe1, 0x74, + 0x30, 0x4b, 0xd2, 0xbd, 0x53, 0x6a, 0x5d, 0xd2, 0xac, 0xb1, 0x74, 0xc7, 0x95, 0xdc, 0x65, 0x3f, + 0xde, 0xe1, 0x08, 0x7a, 0x25, 0x55, 0x39, 0xc9, 0x55, 0x1e, 0xb7, 0xb5, 0xc5, 0x30, 0xdb, 0xe3, + 0xd1, 0x35, 0x1c, 0x1f, 0x3c, 0xc7, 0x23, 0x08, 0xd7, 0x74, 0xe7, 0x73, 0x9b, 0x11, 0x4f, 0xa0, + 0xfd, 0x9e, 0x17, 0x55, 0x9d, 0xdc, 0x81, 0x79, 0xeb, 0x32, 0x48, 0x3e, 0x02, 0xe8, 0x3e, 0xfb, + 0xc0, 0xbf, 0x39, 0x77, 0x0c, 0x03, 0xb6, 0xe2, 0x4c, 0x31, 0xc1, 0x5f, 0xf4, 0xb2, 0x3b, 0x19, + 0x6a, 0xea, 0x91, 0xe0, 0x19, 0xf4, 0x96, 0x85, 0xa8, 0x88, 0x51, 0x23, 0x57, 0x88, 0xc5, 0x5a, + 0x3a, 0x87, 0x68, 0x21, 0x84, 0xb2, 0x47, 0x0c, 0x66, 0xd8, 0x28, 0xe2, 0x89, 0xaa, 0x1b, 0xad, + 0x64, 0x56, 0x4f, 0x3e, 0x75, 0x26, 0xcf, 0xe0, 0x29, 0x74, 0xd6, 0x54, 0x72, 0x5a, 0xf8, 0x5c, + 0x1e, 0x19, 0x9e, 0x69, 0x4f, 0x49, 0x74, 0xba, 0xd0, 0xf0, 0x0e, 0xe1, 0x15, 0x74, 0x97, 0x25, + 0x29, 0x18, 0x37, 0x7f, 0x87, 0xe9, 0x7b, 0x7c, 0x68, 0x93, 0xde, 0xba, 0x0d, 0x57, 0x76, 0xbd, + 0x3f, 0x9a, 0xc3, 0xb0, 0x29, 0xfc, 0xa5, 0xc6, 0x45, 0xc7, 0x7e, 0x45, 0x17, 0xdf, 0x01, 0x00, + 0x00, 0xff, 0xff, 0xd7, 0x24, 0xba, 0x3a, 0x56, 0x02, 0x00, 0x00, } diff --git a/bootcfg/storage/storagepb/storage.proto b/bootcfg/storage/storagepb/storage.proto index 995abbcd..d45419f3 100644 --- a/bootcfg/storage/storagepb/storage.proto +++ b/bootcfg/storage/storagepb/storage.proto @@ -10,8 +10,8 @@ message Group { string profile = 3; // tags required to match the group map requirements = 4; - // custom metadata - map metadata = 5; + // JSON encoded metadata (with restrictions) + bytes metadata = 5; } message Profile { diff --git a/cmd/bootcfg-rpc/main.go b/cmd/bootcfg-rpc/main.go index ef37a048..8e3a3573 100644 --- a/cmd/bootcfg-rpc/main.go +++ b/cmd/bootcfg-rpc/main.go @@ -74,7 +74,7 @@ func main() { // storage store := storage.NewFileStore(&storage.Config{ Dir: flags.dataPath, - Groups: cfg.PBGroups(), + Groups: cfg.Groups, }) // gRPC Server diff --git a/cmd/bootcfg/main.go b/cmd/bootcfg/main.go index cf35ff4a..b41873e4 100644 --- a/cmd/bootcfg/main.go +++ b/cmd/bootcfg/main.go @@ -103,7 +103,7 @@ func main() { if err != nil { log.Fatal(err) } - store.BootstrapGroups(cfg.Groups) + store.BootstrapGroups(cfg.YAMLGroups) // HTTP server config := &api.Config{