diff --git a/cmd/influxd/launcher/pkger_test.go b/cmd/influxd/launcher/pkger_test.go index d439de3d4c..c06ad5fbc0 100644 --- a/cmd/influxd/launcher/pkger_test.go +++ b/cmd/influxd/launcher/pkger_test.go @@ -192,7 +192,7 @@ func TestLauncher_Pkger(t *testing.T) { vars := sum.Variables require.Len(t, vars, 1) - assert.Equal(t, "var_query_1", vars[0].Name) + assert.Equal(t, "query var", vars[0].Name) hasLabelAssociations(t, vars[0].LabelAssociations, 1, "label_1") varArgs := vars[0].Arguments require.NotNil(t, varArgs) @@ -344,7 +344,7 @@ spec: if !exportAllSum { assert.NotZero(t, vars[0].ID) } - assert.Equal(t, "var_query_1", vars[0].Name) + assert.Equal(t, "query var", vars[0].Name) hasLabelAssociations(t, vars[0].LabelAssociations, 1, "label_1") varArgs := vars[0].Arguments require.NotNil(t, varArgs) @@ -1040,6 +1040,7 @@ kind: Variable metadata: name: var_query_1 spec: + name: query var description: var_query_1 desc type: query language: flux @@ -1200,7 +1201,7 @@ kind: Variable metadata: name: var_query_1 spec: - descriptin: new desc + description: new desc type: query language: flux query: | diff --git a/pkger/models.go b/pkger/models.go index 68ddfdb104..d4d1ac9c10 100644 --- a/pkger/models.go +++ b/pkger/models.go @@ -2059,6 +2059,7 @@ type variable struct { id influxdb.ID OrgID influxdb.ID name *references + displayName *references Description string Type string Query string @@ -2087,6 +2088,13 @@ func (v *variable) Labels() []*label { } func (v *variable) Name() string { + if displayName := v.displayName.String(); displayName != "" { + return displayName + } + return v.name.String() +} + +func (v *variable) PkgName() string { return v.name.String() } @@ -2161,7 +2169,13 @@ func (v *variable) valid() []validationErr { }) } } - return failures + if len(failures) > 0 { + return []validationErr{ + objectValidationErr(fieldSpec, failures...), + } + } + + return nil } type mapperVariables []*variable diff --git a/pkger/parser.go b/pkger/parser.go index 295c8bea51..b8a13fff03 100644 --- a/pkger/parser.go +++ b/pkger/parser.go @@ -1060,17 +1060,37 @@ func (p *Pkg) graphTelegrafs() *parseErr { func (p *Pkg) graphVariables() *parseErr { p.mVariables = make(map[string]*variable) + uniqNames := make(map[string]bool) return p.eachResource(KindVariable, 1, func(o Object) []validationErr { nameRef := p.getRefWithKnownEnvs(o.Metadata, fieldName) if _, ok := p.mVariables[nameRef.String()]; ok { - return []validationErr{{ - Field: fieldName, - Msg: "duplicate name: " + nameRef.String(), - }} + return []validationErr{ + objectValidationErr(fieldMetadata, validationErr{ + Field: fieldName, + Msg: "duplicate name: " + nameRef.String(), + }), + } } + displayNameRef := p.getRefWithKnownEnvs(o.Spec, fieldName) + + name := nameRef.String() + if displayName := displayNameRef.String(); displayName != "" { + name = displayName + } + if uniqNames[name] { + return []validationErr{ + objectValidationErr(fieldSpec, validationErr{ + Field: fieldName, + Msg: "duplicate name: " + nameRef.String(), + }), + } + } + uniqNames[name] = true + newVar := &variable{ name: nameRef, + displayName: displayNameRef, Description: o.Spec.stringShort(fieldDescription), Type: normStr(o.Spec.stringShort(fieldType)), Query: strings.TrimSpace(o.Spec.stringShort(fieldQuery)), @@ -1086,8 +1106,8 @@ func (p *Pkg) graphVariables() *parseErr { }) sort.Sort(newVar.labels) - p.mVariables[o.Name()] = newVar - p.setRefs(newVar.name) + p.mVariables[newVar.PkgName()] = newVar + p.setRefs(newVar.name, newVar.displayName) return append(failures, newVar.valid()...) }) diff --git a/pkger/parser_test.go b/pkger/parser_test.go index 32eb9fe3ec..3ac00eb795 100644 --- a/pkger/parser_test.go +++ b/pkger/parser_test.go @@ -3483,28 +3483,28 @@ spec: assert.Equal(t, vals, v.Arguments.Values) } + varEquals(t, + "query var", + "query", + influxdb.VariableQueryValues{ + Query: `buckets() |> filter(fn: (r) => r.name !~ /^_/) |> rename(columns: {name: "_value"}) |> keep(columns: ["_value"])`, + Language: "flux", + }, + sum.Variables[0], + ) + // validates we support all known variable types varEquals(t, "var_const_3", "constant", influxdb.VariableConstantValues([]string{"first val"}), - sum.Variables[0], + sum.Variables[1], ) varEquals(t, "var_map_4", "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], ) @@ -3539,7 +3539,7 @@ spec: { name: "map var missing values", validationErrs: 1, - valFields: []string{"values"}, + valFields: []string{fieldSpec, fieldValues}, pkgStr: `apiVersion: influxdata.com/v2alpha1 kind: Variable metadata: @@ -3552,7 +3552,7 @@ spec: { name: "const var missing values", validationErrs: 1, - valFields: []string{"values"}, + valFields: []string{fieldSpec, fieldValues}, pkgStr: `apiVersion: influxdata.com/v2alpha1 kind: Variable metadata: @@ -3565,7 +3565,7 @@ spec: { name: "query var missing query", validationErrs: 1, - valFields: []string{"query"}, + valFields: []string{fieldSpec, fieldQuery}, pkgStr: `apiVersion: influxdata.com/v2alpha1 kind: Variable metadata: @@ -3579,7 +3579,7 @@ spec: { name: "query var missing query language", validationErrs: 1, - valFields: []string{"language"}, + valFields: []string{fieldSpec, fieldLanguage}, pkgStr: `apiVersion: influxdata.com/v2alpha1 kind: Variable metadata: @@ -3593,7 +3593,7 @@ spec: { name: "query var provides incorrect query language", validationErrs: 1, - valFields: []string{"language"}, + valFields: []string{fieldSpec, fieldLanguage}, pkgStr: `apiVersion: influxdata.com/v2alpha1 kind: Variable metadata: @@ -3608,7 +3608,7 @@ spec: { name: "duplicate var names", validationErrs: 1, - valFields: []string{"name"}, + valFields: []string{fieldMetadata, fieldName}, pkgStr: `apiVersion: influxdata.com/v2alpha1 kind: Variable metadata: @@ -3628,6 +3628,32 @@ spec: type: query query: an influxql query of sorts language: influxql +`, + }, + { + name: "duplicate meta name and spec name", + validationErrs: 1, + valFields: []string{fieldSpec, fieldName}, + pkgStr: `apiVersion: influxdata.com/v2alpha1 +kind: Variable +metadata: + name: var_query_2 +spec: + description: var_query_2 desc + type: query + query: an influxql query of sorts + language: influxql +--- +apiVersion: influxdata.com/v2alpha1 +kind: Variable +metadata: + name: valid_query +spec: + name: var_query_2 + description: var_query_2 desc + type: query + query: an influxql query of sorts + language: influxql `, }, } diff --git a/pkger/service_test.go b/pkger/service_test.go index afe3c324c4..4d926f3c3f 100644 --- a/pkger/service_test.go +++ b/pkger/service_test.go @@ -403,7 +403,7 @@ func TestService(t *testing.T) { }, }, } - assert.Equal(t, expected, diff.Variables[0]) + assert.Equal(t, expected, diff.Variables[1]) expected = DiffVariable{ // no ID here since this one would be new @@ -416,7 +416,7 @@ func TestService(t *testing.T) { }, }, } - assert.Equal(t, expected, diff.Variables[1]) + assert.Equal(t, expected, diff.Variables[2]) }) }) }) @@ -1330,11 +1330,7 @@ func TestService(t *testing.T) { testfileRunner(t, "testdata/variables.yml", func(t *testing.T, pkg *Pkg) { fakeVarSVC := mock.NewVariableService() fakeVarSVC.CreateVariableF = func(_ context.Context, v *influxdb.Variable) error { - id, err := strconv.Atoi(v.Name[len(v.Name)-1:]) - if err != nil { - return err - } - v.ID = influxdb.ID(id) + v.ID = influxdb.ID(fakeVarSVC.CreateVariableCalls.Count() + 1) return nil } @@ -1346,8 +1342,8 @@ func TestService(t *testing.T) { require.NoError(t, err) require.Len(t, sum.Variables, 4) - expected := sum.Variables[0] - assert.Equal(t, SafeID(3), expected.ID) + expected := sum.Variables[1] + assert.True(t, expected.ID > 0 && expected.ID < 5) assert.Equal(t, SafeID(orgID), expected.OrgID) assert.Equal(t, "var_const_3", expected.Name) assert.Equal(t, "var_const_3 desc", expected.Description) @@ -1355,7 +1351,7 @@ func TestService(t *testing.T) { assert.Equal(t, influxdb.VariableConstantValues{"first val"}, expected.Arguments.Values) for _, actual := range sum.Variables { - assert.Contains(t, []SafeID{1, 2, 3, 4}, actual.ID) + assert.Containsf(t, []SafeID{1, 2, 3, 4}, actual.ID, "actual var: %+v", actual) } }) }) @@ -1419,7 +1415,7 @@ func TestService(t *testing.T) { require.NoError(t, err) require.Len(t, sum.Variables, 4) - expected := sum.Variables[0] + expected := sum.Variables[1] assert.Equal(t, SafeID(1), expected.ID) assert.Equal(t, "var_const_3", expected.Name) diff --git a/pkger/testdata/variables.json b/pkger/testdata/variables.json index b771958572..8d736f73ad 100644 --- a/pkger/testdata/variables.json +++ b/pkger/testdata/variables.json @@ -6,7 +6,8 @@ "name": "var_query_1" }, "spec": { - "description": "var_query_1 desc", + "name": "query var", + "description": "query var desc", "type": "query", "query": "buckets() |> filter(fn: (r) => r.name !~ /^_/) |> rename(columns: {name: \"_value\"}) |> keep(columns: [\"_value\"])", "language": "flux" diff --git a/pkger/testdata/variables.yml b/pkger/testdata/variables.yml index 62f4ae8f87..ff60038826 100644 --- a/pkger/testdata/variables.yml +++ b/pkger/testdata/variables.yml @@ -3,7 +3,8 @@ kind: Variable metadata: name: var_query_1 spec: - description: var_query_1 desc + name: query var + description: query var desc type: query language: flux query: |