From d252b20eccae38d8434e24d9f6e777bbd39a6658 Mon Sep 17 00:00:00 2001 From: Johnny Steenbergen Date: Wed, 6 Nov 2019 14:41:06 -0800 Subject: [PATCH] feat(pkger): add support for variable resource kind to pkger --- pkger/models.go | 122 ++++- pkger/models_test.go | 4 +- pkger/parser.go | 155 +++++- pkger/parser_test.go | 887 ++++++++++++++++++---------------- pkger/testdata/variables.json | 45 ++ pkger/testdata/variables.yml | 33 ++ variable.go | 2 +- 7 files changed, 805 insertions(+), 443 deletions(-) create mode 100644 pkger/testdata/variables.json create mode 100644 pkger/testdata/variables.yml diff --git a/pkger/models.go b/pkger/models.go index 973af57817..6644e4ae90 100644 --- a/pkger/models.go +++ b/pkger/models.go @@ -13,17 +13,27 @@ const ( kindDashboard kind = "dashboard" kindLabel kind = "label" kindPackage kind = "package" + kindVariable kind = "variable" ) +var kinds = map[kind]bool{ + kindBucket: true, + kindDashboard: true, + kindLabel: true, + kindPackage: true, + kindVariable: true, +} + type kind string func (k kind) String() string { - switch k { - case kindBucket, kindLabel, kindDashboard, kindPackage: + if kinds[k] { return string(k) - default: + } + if k == kindUnknown { return "unknown" } + return string(k) } // SafeID is an equivalent influxdb.ID that encodes safely with @@ -160,6 +170,7 @@ type Summary struct { Dashboards []SummaryDashboard `json:"dashboards"` Labels []SummaryLabel `json:"labels"` LabelMappings []SummaryLabelMapping `json:"labelMappings"` + Variables []SummaryVariable `json:"variables"` } // SummaryBucket provides a summary of a pkg bucket. @@ -226,6 +237,11 @@ type SummaryLabelMapping struct { influxdb.LabelMapping } +// SummaryVariable provides a summary of a pkg variable. +type SummaryVariable struct { + influxdb.Variable +} + type bucket struct { id influxdb.ID OrgID influxdb.ID @@ -275,17 +291,17 @@ func (b *bucket) shouldApply() bool { b.RetentionPeriod != b.existing.RetentionPeriod } -type labelMapKey struct { +type assocMapKey struct { resType influxdb.ResourceType name string } -type labelMapVal struct { +type assocMapVal struct { exists bool v interface{} } -func (l labelMapVal) bucket() (*bucket, bool) { +func (l assocMapVal) bucket() (*bucket, bool) { if l.v == nil { return nil, false } @@ -293,7 +309,7 @@ func (l labelMapVal) bucket() (*bucket, bool) { return b, ok } -func (l labelMapVal) dashboard() (*dashboard, bool) { +func (l assocMapVal) dashboard() (*dashboard, bool) { if l.v == nil { return nil, false } @@ -308,7 +324,7 @@ type label struct { Color string Description string - mappings map[labelMapKey]labelMapVal + mappings map[assocMapKey]assocMapVal // exists provides context for a resource that already // exists in the platform. If a resource already exists(exists=true) @@ -359,7 +375,7 @@ func (l *label) mappingSummary() []SummaryLabelMapping { return mappings } -func (l *label) getMappedResourceID(k labelMapKey) influxdb.ID { +func (l *label) getMappedResourceID(k assocMapKey) influxdb.ID { switch k.resType { case influxdb.BucketsResourceType: b, ok := l.mappings[k].bucket() @@ -380,14 +396,14 @@ func (l *label) setBucketMapping(b *bucket, exists bool) { return } if l.mappings == nil { - l.mappings = make(map[labelMapKey]labelMapVal) + l.mappings = make(map[assocMapKey]assocMapVal) } - key := labelMapKey{ + key := assocMapKey{ resType: influxdb.BucketsResourceType, name: b.Name, } - l.mappings[key] = labelMapVal{ + l.mappings[key] = assocMapVal{ exists: exists, v: b, } @@ -398,14 +414,14 @@ func (l *label) setDashboardMapping(d *dashboard) { return } if l.mappings == nil { - l.mappings = make(map[labelMapKey]labelMapVal) + l.mappings = make(map[assocMapKey]assocMapVal) } - key := labelMapKey{ + key := assocMapKey{ resType: d.ResourceType(), name: d.Name, } - l.mappings[key] = labelMapVal{v: d} + l.mappings[key] = assocMapVal{v: d} } func (l *label) properties() map[string]string { @@ -428,6 +444,82 @@ func toInfluxLabels(labels ...*label) []influxdb.Label { return iLabels } +type variable struct { + id influxdb.ID + OrgID influxdb.ID + Name string + Description string + Type string + Query string + Language string + ConstValues []string + MapValues map[string]string + + mappings map[assocMapKey]assocMapVal +} + +func (v *variable) summarize() SummaryVariable { + args := &influxdb.VariableArguments{ + Type: v.Type, + } + switch args.Type { + case "query": + args.Values = influxdb.VariableQueryValues{ + Query: v.Query, + Language: v.Language, + } + case "constant": + args.Values = influxdb.VariableConstantValues(v.ConstValues) + case "map": + args.Values = influxdb.VariableMapValues(v.MapValues) + } + + return SummaryVariable{ + Variable: influxdb.Variable{ + ID: v.id, + OrganizationID: v.OrgID, + Name: v.Name, + Description: v.Description, + Arguments: args, + }, + } +} + +func (v *variable) valid() []failure { + var failures []failure + switch v.Type { + case "map": + if len(v.MapValues) == 0 { + failures = append(failures, failure{ + Field: "values", + Msg: "map variable must have at least 1 key/val pair", + }) + } + case "constant": + if len(v.ConstValues) == 0 { + failures = append(failures, failure{ + Field: "values", + Msg: "constant variable must have a least 1 value provided", + }) + } + case "query": + if v.Query == "" { + failures = append(failures, failure{ + Field: "query", + Msg: "query variable must provide a query string", + }) + } + if v.Language != "influxql" && v.Language != "flux" { + const msgFmt = "query variable language must be either %q or %q; got %q" + failures = append(failures, failure{ + Field: "language", + Msg: fmt.Sprintf(msgFmt, "influxql", "flux", v.Language), + }) + } + } + return failures +} + type dashboard struct { id influxdb.ID OrgID influxdb.ID diff --git a/pkger/models_test.go b/pkger/models_test.go index 2492fd0906..45e88e92b7 100644 --- a/pkger/models_test.go +++ b/pkger/models_test.go @@ -94,8 +94,8 @@ func TestPkg(t *testing.T) { Name: "name2", Description: "desc2", Color: "blurple", - mappings: map[labelMapKey]labelMapVal{ - labelMapKey{ + mappings: map[assocMapKey]assocMapVal{ + assocMapKey{ resType: influxdb.BucketsResourceType, name: bucket1.Name, }: { diff --git a/pkger/parser.go b/pkger/parser.go index 18f21d1299..8388beb467 100644 --- a/pkger/parser.go +++ b/pkger/parser.go @@ -135,6 +135,7 @@ type Pkg struct { mLabels map[string]*label mBuckets map[string]*bucket mDashboards map[string]*dashboard + mVariables map[string]*variable isVerified bool // dry run has verified pkg resources with existing resources isParsed bool // indicates the pkg has been parsed and all resources graphed accordingly @@ -146,10 +147,6 @@ type Pkg struct { func (p *Pkg) Summary() Summary { var sum Summary - for _, l := range p.labels() { - sum.Labels = append(sum.Labels, l.summarize()) - } - for _, b := range p.buckets() { sum.Buckets = append(sum.Buckets, b.summarize()) } @@ -158,6 +155,10 @@ func (p *Pkg) Summary() Summary { sum.Dashboards = append(sum.Dashboards, d.summarize()) } + for _, l := range p.labels() { + sum.Labels = append(sum.Labels, l.summarize()) + } + for _, m := range p.labelMappings() { sum.LabelMappings = append(sum.LabelMappings, SummaryLabelMapping{ ResourceName: m.ResourceName, @@ -166,6 +167,10 @@ func (p *Pkg) Summary() Summary { }) } + for _, v := range p.variables() { + sum.Variables = append(sum.Variables, v.summarize()) + } + return sum } @@ -226,6 +231,19 @@ func (p *Pkg) dashboards() []*dashboard { return dashes } +func (p *Pkg) variables() []*variable { + vars := make([]*variable, 0, len(p.mVariables)) + for _, v := range p.mVariables { + vars = append(vars, v) + } + + sort.Slice(vars, func(i, j int) bool { + return vars[i].Name < vars[j].Name + }) + + return vars +} + // labelMappings returns the mappings that will be created for // valid pairs of labels and resources of which all have IDs. // If a resource does not exist yet, a label mapping will not @@ -293,7 +311,7 @@ func (p *Pkg) validMetadata() error { } res := errResource{ - Kind: "Package", + Kind: kindPackage.String(), Idx: -1, } for _, f := range failures { @@ -332,6 +350,7 @@ func (p *Pkg) graphResources() error { graphFns := []func() error{ // labels are first to validate associations with other resources p.graphLabels, + p.graphVariables, p.graphBuckets, p.graphDashboards, } @@ -467,6 +486,47 @@ func (p *Pkg) graphDashboards() error { }) } +func (p *Pkg) graphVariables() error { + p.mVariables = make(map[string]*variable) + return p.eachResource(kindVariable, func(r Resource) []failure { + if r.Name() == "" { + return []failure{{ + Field: "name", + Msg: "must be provided", + }} + } + + if _, ok := p.mVariables[r.Name()]; ok { + return []failure{{ + Field: "name", + Msg: "duplicate name: " + r.Name(), + }} + } + + newVar := &variable{ + Name: r.Name(), + Description: r.stringShort("description"), + Type: strings.ToLower(r.stringShort("type")), + Query: strings.TrimSpace(r.stringShort("query")), + Language: strings.ToLower(strings.TrimSpace(r.stringShort("language"))), + ConstValues: r.slcStr("values"), + MapValues: r.mapStrStr("values"), + } + + p.mVariables[r.Name()] = newVar + + // here we set the var on the var map and return fails + // reaons for this is we could end up providing bad + // errors to the user if we dont' set it b/c of a bad + // query or something, and its being referenced by a + // dashboard or something. The var exists, its just + // invalid. So the mapping is correct. So we keep this + // to validate that mapping is correct, and return fails + // to indicate fails from the var. + return newVar.valid() + }) +} + func (p *Pkg) eachResource(resourceKind kind, fn func(r Resource) []failure) error { var parseErr ParseErr for i, r := range p.Spec.Resources { @@ -602,7 +662,7 @@ func parseChart(r Resource) (chart, []failure) { Geom: r.stringShort("geom"), } - if leg, ok := ifaceMapToResource(r["legend"]); ok { + if leg, ok := ifaceToResource(r["legend"]); ok { c.Legend.Type = leg.stringShort("type") c.Legend.Orientation = leg.stringShort("orientation") } @@ -669,6 +729,9 @@ func (r Resource) kind() (kind, error) { if newKind == kindUnknown { return kindUnknown, errors.New("invalid kind") } + if !kinds[newKind] { + return newKind, errors.New("unsupported kind provided") + } return newKind, nil } @@ -695,7 +758,7 @@ func (r Resource) nestedAssociations() []Resource { var resources []Resource for _, iface := range ifaces { - newRes, ok := ifaceMapToResource(iface) + newRes, ok := ifaceToResource(iface) if !ok { continue } @@ -757,15 +820,7 @@ func (r Resource) intShort(key string) int { } func (r Resource) string(key string) (string, bool) { - if s, ok := r[key].(string); ok { - return s, true - } - - if i, ok := r[key].(int); ok { - return strconv.Itoa(i), true - } - - return "", false + return ifaceToStr(r[key]) } func (r Resource) stringShort(key string) string { @@ -786,7 +841,7 @@ func (r Resource) slcResource(key string) []Resource { var newResources []Resource for _, iFace := range iFaceSlc { - r, ok := ifaceMapToResource(iFace) + r, ok := ifaceToResource(iFace) if !ok { continue } @@ -796,7 +851,51 @@ func (r Resource) slcResource(key string) []Resource { return newResources } -func ifaceMapToResource(i interface{}) (Resource, bool) { +func (r Resource) slcStr(key string) []string { + v, ok := r[key] + if !ok { + return nil + } + + iFaceSlc, ok := v.([]interface{}) + if !ok { + return nil + } + + var out []string + for _, iface := range iFaceSlc { + s, ok := ifaceToStr(iface) + if !ok { + continue + } + out = append(out, s) + } + + return out +} + +func (r Resource) mapStrStr(key string) map[string]string { + res, ok := ifaceToResource(r[key]) + if !ok { + return nil + } + + m := make(map[string]string) + for k, v := range res { + s, ok := ifaceToStr(v) + if !ok { + continue + } + m[k] = s + } + return m +} + +func ifaceToResource(i interface{}) (Resource, bool) { + if i == nil { + return nil, false + } + res, ok := i.(Resource) if ok { return res, true @@ -822,6 +921,26 @@ func ifaceMapToResource(i interface{}) (Resource, bool) { return newRes, true } +func ifaceToStr(v interface{}) (string, bool) { + if v == nil { + return "", false + } + + if s, ok := v.(string); ok { + return s, true + } + + if i, ok := v.(int); ok { + return strconv.Itoa(i), true + } + + if f, ok := v.(float64); ok { + return strconv.FormatFloat(f, 'f', -1, 64), true + } + + return "", false +} + // ParseErr is a error from parsing the given package. The ParseErr // provides a list of resources that failed and all validations // that failed for that resource. A resource can multiple errors, diff --git a/pkger/parser_test.go b/pkger/parser_test.go index d901213e5c..9b6d5fa5ee 100644 --- a/pkger/parser_test.go +++ b/pkger/parser_test.go @@ -16,25 +16,10 @@ func TestParse(t *testing.T) { }) t.Run("malformed required metadata", func(t *testing.T) { - containsField := func(t *testing.T, expected []string, actual string) { - t.Helper() - - for _, e := range expected { - if e == actual { - return - } - } - assert.Fail(t, "did not find field: "+actual) - } - - tests := []struct { - name string - in string - expectedFields []string - }{ + tests := []testPkgResourceError{ { name: "missing apiVersion", - in: `kind: Package + pkgStr: `kind: Package meta: pkgName: first_bucket_package pkgVersion: 1 @@ -44,11 +29,11 @@ spec: name: buck_1 retention_period: 1h `, - expectedFields: []string{"apiVersion"}, + valFields: []string{"apiVersion"}, }, { name: "apiVersion is invalid version", - in: `apiVersion: 222.2 #invalid apiVersion + pkgStr: `apiVersion: 222.2 #invalid apiVersion kind: Package meta: pkgName: first_bucket_package @@ -59,11 +44,11 @@ spec: name: buck_1 retention_period: 1h `, - expectedFields: []string{"apiVersion"}, + valFields: []string{"apiVersion"}, }, { name: "missing kind", - in: `apiVersion: 0.1.0 + pkgStr: `apiVersion: 0.1.0 meta: pkgName: first_bucket_package pkgVersion: 1 @@ -73,11 +58,11 @@ spec: name: buck_1 retention_period: 1h `, - expectedFields: []string{"kind"}, + valFields: []string{"kind"}, }, { name: "missing pkgName", - in: `apiVersion: 0.1.0 + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgVersion: 1 @@ -87,11 +72,11 @@ spec: name: buck_1 retention_period: 1h `, - expectedFields: []string{"pkgName"}, + valFields: []string{"pkgName"}, }, { name: "missing pkgVersion", - in: `apiVersion: 0.1.0 + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: foo_name @@ -101,46 +86,27 @@ spec: name: buck_1 retention_period: 1h `, - expectedFields: []string{"pkgVersion"}, + valFields: []string{"pkgVersion"}, }, { name: "missing multiple", - in: `spec: + pkgStr: `spec: resources: - kind: Bucket name: buck_1 retention_period: 1h `, - expectedFields: []string{"pkgVersion", "pkgName", "kind", "apiVersion"}, + valFields: []string{"apiVersion", "kind", "pkgVersion", "pkgName"}, }, } for _, tt := range tests { - fn := func(t *testing.T) { - _, err := Parse(EncodingYAML, FromString(tt.in)) - require.Error(t, err) - - pErr, ok := IsParseErr(err) - require.True(t, ok) - - require.Len(t, pErr.Resources, 1) - - failedResource := pErr.Resources[0] - assert.Equal(t, "Package", failedResource.Kind) - - require.Len(t, failedResource.ValidationFails, len(tt.expectedFields)) - - for _, f := range failedResource.ValidationFails { - containsField(t, tt.expectedFields, f.Field) - } - } - - t.Run(tt.name, fn) + testPkgErrors(t, kindPackage, tt) } }) }) - t.Run("pkg with just a bucket", func(t *testing.T) { + t.Run("pkg with a bucket", func(t *testing.T) { t.Run("with valid bucket pkg should be valid", func(t *testing.T) { testfileRunner(t, "testdata/bucket", func(t *testing.T, pkg *Pkg) { buckets := pkg.buckets() @@ -156,16 +122,13 @@ spec: }) }) - t.Run("with missing bucket name should error", func(t *testing.T) { - tests := []struct { - name string - numErrs int - in string - }{ + t.Run("handles bad config", func(t *testing.T) { + tests := []testPkgResourceError{ { - name: "missing name", - numErrs: 1, - in: `apiVersion: 1 + name: "missing name", + validationErrs: 1, + valFields: []string{"name"}, + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: first_bucket_package @@ -175,10 +138,12 @@ spec: - kind: Bucket retention_period: 1h `, - }, { - name: "mixed valid and missing name", - numErrs: 1, - in: `apiVersion: 1 + }, + { + name: "mixed valid and missing name", + validationErrs: 1, + valFields: []string{"name"}, + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: first_bucket_package @@ -191,10 +156,13 @@ spec: - kind: Bucket retention_period: 1h `, - }, { - name: "mixed valid and multiple bad names", - numErrs: 2, - in: `apiVersion: 0.1.0 + }, + { + name: "mixed valid and multiple bad names", + resourceErrs: 2, + validationErrs: 1, + valFields: []string{"name"}, + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: first_bucket_package @@ -210,22 +178,12 @@ spec: retention_period: 1h `, }, - } - - for _, tt := range tests { - fn := func(t *testing.T) { - _, err := Parse(EncodingYAML, FromString(tt.in)) - pErr, ok := IsParseErr(err) - require.True(t, ok) - assert.Len(t, pErr.Resources, tt.numErrs) - } - - t.Run(tt.name, fn) - } - }) - - t.Run("with duplicate buckets should error", func(t *testing.T) { - yamlFile := `apiVersion: 0.1.0 + { + name: "duplicate bucket names", + resourceErrs: 1, + validationErrs: 1, + valFields: []string{"name"}, + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: first_bucket_package @@ -238,21 +196,17 @@ spec: - kind: Bucket retention_period: 1h name: valid name -` - _, err := Parse(EncodingYAML, FromString(yamlFile)) - require.Error(t, err) +`, + }, + } - pErr, ok := IsParseErr(err) - require.True(t, ok) - assert.Len(t, pErr.Resources, 1) - - fields := pErr.Resources[0].ValidationFails - require.Len(t, fields, 1) - assert.Equal(t, "name", fields[0].Field) + for _, tt := range tests { + testPkgErrors(t, kindBucket, tt) + } }) }) - t.Run("pkg with just a label", func(t *testing.T) { + t.Run("pkg with a label", func(t *testing.T) { t.Run("with valid label pkg should be valid", func(t *testing.T) { testfileRunner(t, "testdata/label", func(t *testing.T, pkg *Pkg) { labels := pkg.labels() @@ -275,15 +229,12 @@ spec: }) t.Run("with missing label name should error", func(t *testing.T) { - tests := []struct { - name string - numErrs int - in string - }{ + tests := []testPkgResourceError{ { - name: "missing name", - numErrs: 1, - in: `apiVersion: 0.1.0 + name: "missing name", + validationErrs: 1, + valFields: []string{"name"}, + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: first_label_pkg @@ -294,9 +245,10 @@ spec: `, }, { - name: "mixed valid and missing name", - numErrs: 1, - in: `apiVersion: 0.1.0 + name: "mixed valid and missing name", + validationErrs: 1, + valFields: []string{"name"}, + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: label_pkg @@ -309,9 +261,11 @@ spec: `, }, { - name: "multiple labels with missing name", - numErrs: 2, - in: `apiVersion: 0.1.0 + name: "multiple labels with missing name", + resourceErrs: 2, + validationErrs: 1, + valFields: []string{"name"}, + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: label_pkg @@ -325,14 +279,7 @@ spec: } for _, tt := range tests { - fn := func(t *testing.T) { - _, err := Parse(EncodingYAML, FromString(tt.in)) - pErr, ok := IsParseErr(err) - require.True(t, ok) - assert.Len(t, pErr.Resources, tt.numErrs) - } - - t.Run(tt.name, fn) + testPkgErrors(t, kindLabel, tt) } }) }) @@ -398,17 +345,12 @@ spec: }) t.Run("association doesn't exist then provides an error", func(t *testing.T) { - tests := []struct { - name string - numErrs int - in string - errIdxs []int - }{ + tests := []testPkgResourceError{ { name: "no labels provided", - numErrs: 1, - errIdxs: []int{0}, - in: `apiVersion: 0.1.0 + assErrs: 1, + assIdxs: []int{0}, + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: label_pkg @@ -424,9 +366,9 @@ spec: }, { name: "mixed found and not found", - numErrs: 1, - errIdxs: []int{1}, - in: `apiVersion: 0.1.0 + assErrs: 1, + assIdxs: []int{1}, + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: label_pkg @@ -446,9 +388,9 @@ spec: }, { name: "multiple not found", - numErrs: 1, - errIdxs: []int{0, 1}, - in: `apiVersion: 0.1.0 + assErrs: 1, + assIdxs: []int{0, 1}, + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: label_pkg @@ -468,9 +410,9 @@ spec: }, { name: "duplicate valid nested labels", - numErrs: 1, - errIdxs: []int{1}, - in: `apiVersion: 0.1.0 + assErrs: 1, + assIdxs: []int{1}, + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: label_pkg @@ -491,23 +433,7 @@ spec: } for _, tt := range tests { - fn := func(t *testing.T) { - _, err := Parse(EncodingYAML, FromString(tt.in)) - - pErr, ok := IsParseErr(err) - require.True(t, ok) - require.Len(t, pErr.Resources, tt.numErrs) - - assFails := pErr.Resources[0].AssociationFails - require.Len(t, assFails, len(tt.errIdxs)) - assert.Equal(t, "associations", assFails[0].Field) - - for i, f := range assFails { - assert.Equal(t, tt.errIdxs[i], f.Index) - } - } - - t.Run(tt.name, fn) + testPkgErrors(t, kindBucket, tt) } }) }) @@ -555,17 +481,12 @@ spec: }) t.Run("handles invalid config", func(t *testing.T) { - tests := []struct { - name string - ymlStr string - numErrs int - errFields []string - }{ + tests := []testPkgResourceError{ { - name: "color missing hex value", - numErrs: 1, - errFields: []string{"charts[0].colors[0].hex"}, - ymlStr: `apiVersion: 0.1.0 + name: "color missing hex value", + validationErrs: 1, + valFields: []string{"charts[0].colors[0].hex"}, + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: pkg_name @@ -592,10 +513,10 @@ spec: `, }, { - name: "no colors provided", - numErrs: 2, - errFields: []string{"charts[0].colors", "charts[0].colors"}, - ymlStr: `apiVersion: 0.1.0 + name: "no colors provided", + validationErrs: 2, + valFields: []string{"charts[0].colors", "charts[0].colors"}, + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: pkg_name @@ -619,10 +540,10 @@ spec: `, }, { - name: "query missing text value", - numErrs: 1, - errFields: []string{"charts[0].queries[0].query"}, - ymlStr: `apiVersion: 0.1.0 + name: "query missing text value", + validationErrs: 1, + valFields: []string{"charts[0].queries[0].query"}, + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: pkg_name @@ -649,10 +570,10 @@ spec: `, }, { - name: "no queries provided", - numErrs: 1, - errFields: []string{"charts[0].queries"}, - ymlStr: `apiVersion: 0.1.0 + name: "no queries provided", + validationErrs: 1, + valFields: []string{"charts[0].queries"}, + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: pkg_name @@ -677,10 +598,10 @@ spec: `, }, { - name: "no width provided", - numErrs: 1, - errFields: []string{"charts[0].width"}, - ymlStr: `apiVersion: 0.1.0 + name: "no width provided", + validationErrs: 1, + valFields: []string{"charts[0].width"}, + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: pkg_name @@ -707,10 +628,10 @@ spec: `, }, { - name: "no height provided", - numErrs: 1, - errFields: []string{"charts[0].height"}, - ymlStr: `apiVersion: 0.1.0 + name: "no height provided", + validationErrs: 1, + valFields: []string{"charts[0].height"}, + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: pkg_name @@ -739,24 +660,7 @@ spec: } for _, tt := range tests { - fn := func(t *testing.T) { - _, err := Parse(EncodingYAML, FromString(tt.ymlStr)) - require.Error(t, err) - - pErr, ok := IsParseErr(err) - require.True(t, ok, err) - - require.Len(t, pErr.Resources, 1) - - resErr := pErr.Resources[0] - assert.Equal(t, "dashboard", resErr.Kind) - - require.Len(t, resErr.ValidationFails, tt.numErrs) - for i, vFail := range resErr.ValidationFails { - assert.Equal(t, tt.errFields[i], vFail.Field) - } - } - t.Run(tt.name, fn) + testPkgErrors(t, kindDashboard, tt) } }) }) @@ -821,17 +725,12 @@ spec: }) t.Run("handles invalid config", func(t *testing.T) { - tests := []struct { - name string - ymlStr string - numErrs int - errFields []string - }{ + tests := []testPkgResourceError{ { - name: "color missing hex value", - numErrs: 1, - errFields: []string{"charts[0].colors[0].hex"}, - ymlStr: `apiVersion: 0.1.0 + name: "color missing hex value", + validationErrs: 1, + valFields: []string{"charts[0].colors[0].hex"}, + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: pkg_name @@ -868,10 +767,10 @@ spec: `, }, { - name: "no colors provided", - numErrs: 3, - errFields: []string{"charts[0].colors", "charts[0].colors", "charts[0].colors"}, - ymlStr: `apiVersion: 0.1.0 + name: "no colors provided", + validationErrs: 3, + valFields: []string{"charts[0].colors", "charts[0].colors", "charts[0].colors"}, + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: pkg_name @@ -902,10 +801,10 @@ spec: `, }, { - name: "missing query value", - numErrs: 1, - errFields: []string{"charts[0].queries[0].query"}, - ymlStr: `apiVersion: 0.1.0 + name: "missing query value", + validationErrs: 1, + valFields: []string{"charts[0].queries[0].query"}, + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: pkg_name @@ -942,10 +841,10 @@ spec: `, }, { - name: "no queries provided", - numErrs: 1, - errFields: []string{"charts[0].queries"}, - ymlStr: `apiVersion: 0.1.0 + name: "no queries provided", + validationErrs: 1, + valFields: []string{"charts[0].queries"}, + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: pkg_name @@ -979,10 +878,10 @@ spec: scale: linear`, }, { - name: "no width provided", - numErrs: 1, - errFields: []string{"charts[0].width"}, - ymlStr: `apiVersion: 0.1.0 + name: "no width provided", + validationErrs: 1, + valFields: []string{"charts[0].width"}, + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: pkg_name @@ -1018,10 +917,10 @@ spec: scale: linear`, }, { - name: "no height provided", - numErrs: 1, - errFields: []string{"charts[0].height"}, - ymlStr: `apiVersion: 0.1.0 + name: "no height provided", + validationErrs: 1, + valFields: []string{"charts[0].height"}, + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: pkg_name @@ -1057,10 +956,10 @@ spec: scale: linear`, }, { - name: "missing text color but has scale color", - numErrs: 1, - errFields: []string{"charts[0].colors"}, - ymlStr: `apiVersion: 0.1.0 + name: "missing text color but has scale color", + validationErrs: 1, + valFields: []string{"charts[0].colors"}, + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: pkg_name @@ -1094,10 +993,10 @@ spec: scale: linear`, }, { - name: "missing x axis", - numErrs: 1, - errFields: []string{"charts[0].axes"}, - ymlStr: `apiVersion: 0.1.0 + name: "missing x axis", + validationErrs: 1, + valFields: []string{"charts[0].axes"}, + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: pkg_name @@ -1130,10 +1029,10 @@ spec: scale: linear`, }, { - name: "missing y axis", - numErrs: 1, - errFields: []string{"charts[0].axes"}, - ymlStr: `apiVersion: 0.1.0 + name: "missing y axis", + validationErrs: 1, + valFields: []string{"charts[0].axes"}, + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: pkg_name @@ -1167,81 +1066,59 @@ spec: } for _, tt := range tests { - fn := func(t *testing.T) { - _, err := Parse(EncodingYAML, FromString(tt.ymlStr)) - require.Error(t, err) - - pErr, ok := IsParseErr(err) - require.True(t, ok, err) - - require.Len(t, pErr.Resources, 1) - - resErr := pErr.Resources[0] - assert.Equal(t, "dashboard", resErr.Kind) - - require.Len(t, resErr.ValidationFails, tt.numErrs) - for i, vFail := range resErr.ValidationFails { - assert.Equal(t, tt.errFields[i], vFail.Field) - } - } - t.Run(tt.name, fn) + testPkgErrors(t, kindDashboard, tt) } }) }) - }) - t.Run("pkg with single dashboard xy chart", func(t *testing.T) { - t.Run("xy chart", func(t *testing.T) { - testfileRunner(t, "testdata/dashboard_xy", func(t *testing.T, pkg *Pkg) { - sum := pkg.Summary() - require.Len(t, sum.Dashboards, 1) + t.Run("pkg with single dashboard xy chart", func(t *testing.T) { + t.Run("xy chart", func(t *testing.T) { + testfileRunner(t, "testdata/dashboard_xy", func(t *testing.T, pkg *Pkg) { + sum := pkg.Summary() + require.Len(t, sum.Dashboards, 1) - actual := sum.Dashboards[0] - assert.Equal(t, "dash_1", actual.Name) - assert.Equal(t, "desc1", actual.Description) + actual := sum.Dashboards[0] + assert.Equal(t, "dash_1", actual.Name) + assert.Equal(t, "desc1", actual.Description) - require.Len(t, actual.Charts, 1) - actualChart := actual.Charts[0] - assert.Equal(t, 3, actualChart.Height) - assert.Equal(t, 6, actualChart.Width) - assert.Equal(t, 1, actualChart.XPosition) - assert.Equal(t, 2, actualChart.YPosition) + require.Len(t, actual.Charts, 1) + actualChart := actual.Charts[0] + assert.Equal(t, 3, actualChart.Height) + assert.Equal(t, 6, actualChart.Width) + assert.Equal(t, 1, actualChart.XPosition) + assert.Equal(t, 2, actualChart.YPosition) - props, ok := actualChart.Properties.(influxdb.XYViewProperties) - require.True(t, ok) - assert.Equal(t, "xy", props.GetType()) - assert.Equal(t, true, props.ShadeBelow) - assert.Equal(t, "xy chart note", props.Note) - assert.True(t, props.ShowNoteWhenEmpty) + props, ok := actualChart.Properties.(influxdb.XYViewProperties) + require.True(t, ok) + assert.Equal(t, "xy", props.GetType()) + assert.Equal(t, true, props.ShadeBelow) + assert.Equal(t, "xy chart note", props.Note) + assert.True(t, props.ShowNoteWhenEmpty) - require.Len(t, props.Queries, 1) - q := props.Queries[0] - queryText := `from(bucket: v.bucket) |> range(start: v.timeRangeStart, stop: v.timeRangeStop) |> filter(fn: (r) => r._measurement == "boltdb_writes_total") |> filter(fn: (r) => r._field == "counter")` - assert.Equal(t, queryText, q.Text) - assert.Equal(t, "advanced", q.EditMode) + require.Len(t, props.Queries, 1) + q := props.Queries[0] + queryText := `from(bucket: v.bucket) |> range(start: v.timeRangeStart, stop: v.timeRangeStop) |> filter(fn: (r) => r._measurement == "boltdb_writes_total") |> filter(fn: (r) => r._field == "counter")` + assert.Equal(t, queryText, q.Text) + assert.Equal(t, "advanced", q.EditMode) - require.Len(t, props.ViewColors, 1) - c := props.ViewColors[0] - assert.NotZero(t, c.ID) - assert.Equal(t, "laser", c.Name) - assert.Equal(t, "scale", c.Type) - assert.Equal(t, "#8F8AF4", c.Hex) - assert.Equal(t, 3.0, c.Value) + require.Len(t, props.ViewColors, 1) + c := props.ViewColors[0] + assert.NotZero(t, c.ID) + assert.Equal(t, "laser", c.Name) + assert.Equal(t, "scale", c.Type) + assert.Equal(t, "#8F8AF4", c.Hex) + assert.Equal(t, 3.0, c.Value) + }) }) - }) - t.Run("handles invalid config", func(t *testing.T) { - tests := []struct { - name string - jsonStr string - numErrs int - errFields []string - }{ - { - name: "color missing hex value", - numErrs: 1, - errFields: []string{"charts[0].colors[0].hex"}, - jsonStr: `{ + t.Run("handles invalid config", func(t *testing.T) { + tests := []testPkgResourceError{ + { + name: "color missing hex value", + encoding: EncodingJSON, + validationErrs: 1, + valFields: []string{"charts[0].colors[0].hex"}, + pkgStr: `{ "apiVersion": "0.1.0", "kind": "Package", "meta": { @@ -1309,12 +1186,13 @@ spec: } } `, - }, - { - name: "invalid geom flag", - numErrs: 1, - errFields: []string{"charts[0].geom"}, - jsonStr: ` + }, + { + name: "invalid geom flag", + encoding: EncodingJSON, + validationErrs: 1, + valFields: []string{"charts[0].geom"}, + pkgStr: ` { "apiVersion": "0.1.0", "kind": "Package", @@ -1384,34 +1262,16 @@ spec: } } `, - }, - } - - for _, tt := range tests { - fn := func(t *testing.T) { - _, err := Parse(EncodingJSON, FromString(tt.jsonStr)) - require.Error(t, err) - - pErr, ok := IsParseErr(err) - require.True(t, ok, err) - - require.Len(t, pErr.Resources, 1) - - resErr := pErr.Resources[0] - assert.Equal(t, "dashboard", resErr.Kind) - - require.Len(t, resErr.ValidationFails, tt.numErrs) - for i, vFail := range resErr.ValidationFails { - assert.Equal(t, tt.errFields[i], vFail.Field) - } + }, } - t.Run(tt.name, fn) - } - }) - }) - t.Run("pkg with single dashboard gauge chart", func(t *testing.T) { - t.Run("gauge chart", func(t *testing.T) { + for _, tt := range tests { + testPkgErrors(t, kindDashboard, tt) + } + }) + }) + + t.Run("pkg with single dashboard gauge chart", func(t *testing.T) { testfileRunner(t, "testdata/dashboard_gauge", func(t *testing.T, pkg *Pkg) { sum := pkg.Summary() require.Len(t, sum.Dashboards, 1) @@ -1449,17 +1309,13 @@ spec: }) t.Run("handles invalid config", func(t *testing.T) { - tests := []struct { - name string - jsonStr string - numErrs int - errFields []string - }{ + tests := []testPkgResourceError{ { - name: "color a gauge type", - numErrs: 1, - errFields: []string{"charts[0].colors"}, - jsonStr: ` + name: "color a gauge type", + encoding: EncodingJSON, + validationErrs: 1, + valFields: []string{"charts[0].colors"}, + pkgStr: ` { "apiVersion": "0.1.0", "kind": "Package", @@ -1517,10 +1373,11 @@ spec: `, }, { - name: "color mixing a hex value", - numErrs: 1, - errFields: []string{"charts[0].colors[0].hex"}, - jsonStr: ` + name: "color mixing a hex value", + encoding: EncodingJSON, + validationErrs: 1, + valFields: []string{"charts[0].colors[0].hex"}, + pkgStr: ` { "apiVersion": "0.1.0", "kind": "Package", @@ -1583,10 +1440,11 @@ spec: `, }, { - name: "missing a query value", - numErrs: 1, - errFields: []string{"charts[0].queries[0].query"}, - jsonStr: ` + name: "missing a query value", + encoding: EncodingJSON, + validationErrs: 1, + valFields: []string{"charts[0].queries[0].query"}, + pkgStr: ` { "apiVersion": "0.1.0", "kind": "Package", @@ -1652,24 +1510,7 @@ spec: } for _, tt := range tests { - fn := func(t *testing.T) { - _, err := Parse(EncodingJSON, FromString(tt.jsonStr)) - require.Error(t, err) - - pErr, ok := IsParseErr(err) - require.True(t, ok, err) - - require.Len(t, pErr.Resources, 1) - - resErr := pErr.Resources[0] - assert.Equal(t, "dashboard", resErr.Kind) - - require.Len(t, resErr.ValidationFails, tt.numErrs) - for i, vFail := range resErr.ValidationFails { - assert.Equal(t, tt.errFields[i], vFail.Field) - } - } - t.Run(tt.name, fn) + testPkgErrors(t, kindDashboard, tt) } }) }) @@ -1703,17 +1544,12 @@ spec: }) t.Run("association doesn't exist then provides an error", func(t *testing.T) { - tests := []struct { - name string - numErrs int - in string - errIdxs []int - }{ + tests := []testPkgResourceError{ { name: "no labels provided", - numErrs: 1, - errIdxs: []int{0}, - in: `apiVersion: 0.1.0 + assErrs: 1, + assIdxs: []int{0}, + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: label_pkg @@ -1729,9 +1565,9 @@ spec: }, { name: "mixed found and not found", - numErrs: 1, - errIdxs: []int{1}, - in: `apiVersion: 0.1.0 + assErrs: 1, + assIdxs: []int{1}, + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: label_pkg @@ -1751,9 +1587,9 @@ spec: }, { name: "multiple not found", - numErrs: 1, - errIdxs: []int{0, 1}, - in: `apiVersion: 0.1.0 + assErrs: 1, + assIdxs: []int{0, 1}, + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: label_pkg @@ -1773,9 +1609,9 @@ spec: }, { name: "duplicate valid nested labels", - numErrs: 1, - errIdxs: []int{1}, - in: `apiVersion: 0.1.0 + assErrs: 1, + assIdxs: []int{1}, + pkgStr: `apiVersion: 0.1.0 kind: Package meta: pkgName: label_pkg @@ -1796,26 +1632,263 @@ spec: } for _, tt := range tests { - fn := func(t *testing.T) { - _, err := Parse(EncodingYAML, FromString(tt.in)) - - pErr, ok := IsParseErr(err) - require.True(t, ok) - require.Len(t, pErr.Resources, tt.numErrs) - - assFails := pErr.Resources[0].AssociationFails - require.Len(t, assFails, len(tt.errIdxs)) - assert.Equal(t, "associations", assFails[0].Field) - - for i, f := range assFails { - assert.Equal(t, tt.errIdxs[i], f.Index) - } - } - - t.Run(tt.name, fn) + testPkgErrors(t, kindDashboard, tt) } }) }) + + t.Run("pkg with a variable", func(t *testing.T) { + t.Run("with valid fields should produce summary", func(t *testing.T) { + testfileRunner(t, "testdata/variables", func(t *testing.T, pkg *Pkg) { + sum := pkg.Summary() + + require.Len(t, sum.Variables, 4) + + varEquals := func(t *testing.T, name, vType string, vals interface{}, v SummaryVariable) { + t.Helper() + + assert.Equal(t, name, v.Name) + assert.Equal(t, name+" desc", v.Description) + require.NotNil(t, v.Arguments) + assert.Equal(t, vType, v.Arguments.Type) + assert.Equal(t, vals, v.Arguments.Values) + } + + // validates we support all known variable types + varEquals(t, + "var_const", + "constant", + influxdb.VariableConstantValues([]string{"first val"}), + sum.Variables[0], + ) + + varEquals(t, + "var_map", + "map", + influxdb.VariableMapValues{"k1": "v1"}, + sum.Variables[1], + ) + + varEquals(t, + "var_query_1", + "query", + influxdb.VariableQueryValues{ + Query: `buckets() |> filter(fn: (r) => r.name !~ /^_/) |> rename(columns: {name: "_value"}) |> keep(columns: ["_value"])`, + Language: "flux", + }, + sum.Variables[2], + ) + + varEquals(t, + "var_query_2", + "query", + influxdb.VariableQueryValues{ + Query: "an influxql query of sorts", + Language: "influxql", + }, + sum.Variables[3], + ) + }) + }) + + t.Run("handles bad config", func(t *testing.T) { + tests := []testPkgResourceError{ + { + name: "name missing", + validationErrs: 1, + valFields: []string{"name"}, + pkgStr: `apiVersion: 0.1.0 +kind: Package +meta: + pkgName: pkg_name + pkgVersion: 1 + description: pack description +spec: + resources: + - kind: Variable + type: map + values: + k1: v1 +`, + }, + { + name: "map var missing values", + validationErrs: 1, + valFields: []string{"values"}, + pkgStr: `apiVersion: 0.1.0 +kind: Package +meta: + pkgName: pkg_name + pkgVersion: 1 + description: pack description +spec: + resources: + - kind: Variable + name: var + type: map +`, + }, + { + name: "const var missing values", + validationErrs: 1, + valFields: []string{"values"}, + pkgStr: `apiVersion: 0.1.0 +kind: Package +meta: + pkgName: pkg_name + pkgVersion: 1 + description: pack description +spec: + resources: + - kind: Variable + name: var + type: constant +`, + }, + { + name: "query var missing query", + validationErrs: 1, + valFields: []string{"query"}, + pkgStr: `apiVersion: 0.1.0 +kind: Package +meta: + pkgName: pkg_name + pkgVersion: 1 + description: pack description +spec: + resources: + - kind: Variable + name: var + type: query + language: influxql +`, + }, + { + name: "query var missing query language", + validationErrs: 1, + valFields: []string{"language"}, + pkgStr: `apiVersion: 0.1.0 +kind: Package +meta: + pkgName: pkg_name + pkgVersion: 1 + description: pack description +spec: + resources: + - kind: Variable + name: var + type: query + query: from(v.bucket) |> count() +`, + }, + { + name: "query var provides incorrect query language", + validationErrs: 1, + valFields: []string{"language"}, + pkgStr: `apiVersion: 0.1.0 +kind: Package +meta: + pkgName: pkg_name + pkgVersion: 1 + description: pack description +spec: + resources: + - kind: Variable + name: var + type: query + query: from(v.bucket) |> count() + language: wrongo language +`, + }, + { + name: "duplicate var names", + validationErrs: 1, + valFields: []string{"name"}, + pkgStr: `apiVersion: 0.1.0 +kind: Package +meta: + pkgName: pkg_name + pkgVersion: 1 + description: pack description +spec: + resources: + - kind: Variable + name: var + type: query + query: from(v.bucket) |> count() + language: flux + - kind: Variable + name: var + type: query + query: from(v.bucket) |> mean() + language: flux +`, + }, + } + + for _, tt := range tests { + testPkgErrors(t, kindVariable, tt) + } + }) + }) +} + +type testPkgResourceError struct { + name string + encoding Encoding + pkgStr string + resourceErrs int + validationErrs int + valFields []string + assErrs int + assIdxs []int +} + +// defaults to yaml encoding if encoding not provided +// defaults num resources to 1 if resource errs not provided. +func testPkgErrors(t *testing.T, k kind, tt testPkgResourceError) { + t.Helper() + encoding := EncodingYAML + if tt.encoding != EncodingUnknown { + encoding = tt.encoding + } + + resErrs := 1 + if tt.resourceErrs > 0 { + resErrs = tt.resourceErrs + } + + fn := func(t *testing.T) { + t.Helper() + + _, err := Parse(encoding, FromString(tt.pkgStr)) + require.Error(t, err) + + pErr, ok := IsParseErr(err) + require.True(t, ok, err) + + require.Len(t, pErr.Resources, resErrs) + + resErr := pErr.Resources[0] + assert.Equal(t, k.String(), resErr.Kind) + + require.Len(t, resErr.ValidationFails, len(tt.valFields)) + for i, vFail := range resErr.ValidationFails { + assert.Equal(t, tt.valFields[i], vFail.Field) + } + + assFails := pErr.Resources[0].AssociationFails + require.Len(t, assFails, len(tt.assIdxs)) + if tt.assErrs == 0 { + return + } + + for i, f := range assFails { + assert.Equal(t, "associations", assFails[i].Field) + assert.Equal(t, tt.assIdxs[i], f.Index) + } + } + t.Run(tt.name, fn) } type baseAsserts struct { diff --git a/pkger/testdata/variables.json b/pkger/testdata/variables.json new file mode 100644 index 0000000000..2c83959d58 --- /dev/null +++ b/pkger/testdata/variables.json @@ -0,0 +1,45 @@ +{ + "apiVersion": "0.1.0", + "kind": "Package", + "meta": { + "pkgName": "pkg_name", + "pkgVersion": "1", + "description": "pack description" + }, + "spec": { + "resources": [ + { + "kind": "Variable", + "name": "var_query_1", + "description": "var_query_1 desc", + "type": "query", + "query": "buckets() |> filter(fn: (r) => r.name !~ /^_/) |> rename(columns: {name: \"_value\"}) |> keep(columns: [\"_value\"])", + "language": "flux" + }, + { + "kind": "Variable", + "name": "var_query_2", + "description": "var_query_2 desc", + "type": "query", + "query": "an influxql query of sorts", + "language": "influxql" + }, + { + "kind": "Variable", + "name": "var_const", + "description": "var_const desc", + "type": "constant", + "values": ["first val"] + }, + { + "kind": "Variable", + "name": "var_map", + "description": "var_map desc", + "type": "map", + "values": { + "k1": "v1" + } + } + ] + } +} diff --git a/pkger/testdata/variables.yml b/pkger/testdata/variables.yml new file mode 100644 index 0000000000..8efe8f3063 --- /dev/null +++ b/pkger/testdata/variables.yml @@ -0,0 +1,33 @@ +apiVersion: 0.1.0 +kind: Package +meta: + pkgName: pkg_name + pkgVersion: 1 + description: pack description +spec: + resources: + - kind: Variable + name: var_query_1 + description: var_query_1 desc + type: query + language: flux + query: | + buckets() |> filter(fn: (r) => r.name !~ /^_/) |> rename(columns: {name: "_value"}) |> keep(columns: ["_value"]) + - kind: Variable + name: var_query_2 + description: var_query_2 desc + type: query + query: an influxql query of sorts + language: influxql + - kind: Variable + name: var_const + description: var_const desc + type: constant + values: + - first val + - kind: Variable + name: var_map + description: var_map desc + type: map + values: + k1: v1 diff --git a/variable.go b/variable.go index 51af93c6dd..f0ddf29b9b 100644 --- a/variable.go +++ b/variable.go @@ -123,7 +123,7 @@ func (m *Variable) Valid() error { "query": true, } - if _, prs := validTypes[m.Arguments.Type]; !prs { + if !validTypes[m.Arguments.Type] { return fmt.Errorf("invalid arguments type") }