From 7ffa2000119e70422f4eff342fbb2320ca9742fc Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Date: Fri, 12 Aug 2022 12:12:20 +0200 Subject: [PATCH 01/17] Add warning/error for dashboards with visualization by value --- .../validate_kibana_dashboard_by_value.go | 86 +++++++++++++++++++ code/go/internal/validator/spec.go | 1 + code/go/pkg/validator/validator_test.go | 1 + .../visualizations_by_reference/changelog.yml | 5 ++ .../docs/README.md | 1 + ...-82273ffe-6acc-4f2f-bbee-c1004abba63d.json | 54 ++++++++++++ ...-5e1a01ff-6f9a-41c1-b7ad-326472db42b6.json | 5 ++ ...-8287a5d5-1576-4f3a-83c4-444e9058439b.json | 5 ++ .../visualizations_by_reference/manifest.yml | 28 ++++++ 9 files changed, 186 insertions(+) create mode 100644 code/go/internal/validator/semantic/validate_kibana_dashboard_by_value.go create mode 100644 test/packages/visualizations_by_reference/changelog.yml create mode 100644 test/packages/visualizations_by_reference/docs/README.md create mode 100644 test/packages/visualizations_by_reference/kibana/dashboard/visualizations_by_reference-82273ffe-6acc-4f2f-bbee-c1004abba63d.json create mode 100644 test/packages/visualizations_by_reference/kibana/visualization/visualizations_by_reference-5e1a01ff-6f9a-41c1-b7ad-326472db42b6.json create mode 100644 test/packages/visualizations_by_reference/kibana/visualization/visualizations_by_reference-8287a5d5-1576-4f3a-83c4-444e9058439b.json create mode 100644 test/packages/visualizations_by_reference/manifest.yml diff --git a/code/go/internal/validator/semantic/validate_kibana_dashboard_by_value.go b/code/go/internal/validator/semantic/validate_kibana_dashboard_by_value.go new file mode 100644 index 000000000..46e2f2e2a --- /dev/null +++ b/code/go/internal/validator/semantic/validate_kibana_dashboard_by_value.go @@ -0,0 +1,86 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package semantic + +import ( + "encoding/json" + "log" + "path" + + "github.com/pkg/errors" + + ve "github.com/elastic/package-spec/code/go/internal/errors" + "github.com/elastic/package-spec/code/go/internal/fspath" + "github.com/elastic/package-spec/code/go/internal/pkgpath" +) + +type reference struct { + Id string + Name string + Type string +} + +// ValidateDashboardsByValue returns validation errors if there are any Kibana +// Dashboard that defines visualizations by reference instead of value. +// That is, it returns validation errors if a Kibana dashbaord file, foo.json, +// defines some visualization using reference (key panelRef). +func ValidateDashboardsByValue(fsys fspath.FS) ve.ValidationErrors { + var errs ve.ValidationErrors + + filePaths := path.Join("kibana", "dashboard", "*.json") + objectFiles, err := pkgpath.Files(fsys, filePaths) + if err != nil { + errs = append(errs, errors.Wrap(err, "error finding Kibana Dashboard files")) + return errs + } + + for _, objectFile := range objectFiles { + filePath := objectFile.Path() + + objectReferences, err := objectFile.Values("$.references") + if err != nil { + // log.Printf("Warning: unable to get kibana dashboard references in file [%s]", fsys.Path(filePath)) + // errs = append(errs, errors.Wrapf(err, "unable to get Kibana Dashboard references in file [%s]", fsys.Path(filePath))) + continue + } + + references, err := toReferenceSlice(objectReferences) + if err != nil { + errs = append(errs, errors.Wrapf(err, "unable to convert references in file [%s]", fsys.Path(filePath))) + continue + } + + for _, reference := range references { + if reference.Type == "visualization" { + // errs = append(errs, fmt.Errorf("Kibana Dashboard %s contains a visualization by reference \"%s\"", fsys.Path(filePath), reference.Id)) + log.Printf("Warning: Kibana Dashboard %s contains a visualization by reference \"%s\"", fsys.Path(filePath), reference.Id) + } + } + } + + return errs +} + +func toReferenceSlice(val interface{}) ([]reference, error) { + vals, ok := val.([]interface{}) + if !ok { + return nil, errors.New("conversion error") + } + + var refs []reference + jsonbody, err := json.Marshal(vals) + if err != nil { + log.Printf("error encoding reference list: %s", err) + return refs, nil + } + + err = json.Unmarshal(jsonbody, &refs) + if err != nil { + log.Printf("error unmarshaling references: %s", err) + return refs, nil + } + + return refs, nil +} diff --git a/code/go/internal/validator/spec.go b/code/go/internal/validator/spec.go index e60d33810..660da77be 100644 --- a/code/go/internal/validator/spec.go +++ b/code/go/internal/validator/spec.go @@ -76,6 +76,7 @@ func (s Spec) ValidatePackage(pkg Package) ve.ValidationErrors { //semantic.ValidateUniqueFields, semantic.ValidateDimensionFields, semantic.ValidateRequiredFields, + semantic.ValidateDashboardsByValue, } return rules.validate(&pkg) diff --git a/code/go/pkg/validator/validator_test.go b/code/go/pkg/validator/validator_test.go index ca47c2ed4..8f43c590a 100644 --- a/code/go/pkg/validator/validator_test.go +++ b/code/go/pkg/validator/validator_test.go @@ -90,6 +90,7 @@ func TestValidateFile(t *testing.T) { "field services.docker-custom-agent: Must not validate the schema (not)", }, }, + "visualizations_by_reference": {}, } for pkgName, test := range tests { diff --git a/test/packages/visualizations_by_reference/changelog.yml b/test/packages/visualizations_by_reference/changelog.yml new file mode 100644 index 000000000..ba105f727 --- /dev/null +++ b/test/packages/visualizations_by_reference/changelog.yml @@ -0,0 +1,5 @@ +- version: 0.1.2 + changes: + - description: initial release + type: enhancement + link: https://github.com/elastic/package-spec/pull/131 \ No newline at end of file diff --git a/test/packages/visualizations_by_reference/docs/README.md b/test/packages/visualizations_by_reference/docs/README.md new file mode 100644 index 000000000..1c9bf4968 --- /dev/null +++ b/test/packages/visualizations_by_reference/docs/README.md @@ -0,0 +1 @@ +Main \ No newline at end of file diff --git a/test/packages/visualizations_by_reference/kibana/dashboard/visualizations_by_reference-82273ffe-6acc-4f2f-bbee-c1004abba63d.json b/test/packages/visualizations_by_reference/kibana/dashboard/visualizations_by_reference-82273ffe-6acc-4f2f-bbee-c1004abba63d.json new file mode 100644 index 000000000..0ed721d69 --- /dev/null +++ b/test/packages/visualizations_by_reference/kibana/dashboard/visualizations_by_reference-82273ffe-6acc-4f2f-bbee-c1004abba63d.json @@ -0,0 +1,54 @@ +{ + "attributes": { + "description": "Dashboard", + "panelsJSON": [ + { + "embeddableConfig": { + "enhancements": {} + }, + "gridData": { + "h": 15, + "i": "2df03d80-37c4-4dea-9b5e-f3f9eefb319e", + "w": 24, + "x": 24, + "y": 0 + }, + "panelIndex": "2df03d80-37c4-4dea-9b5e-f3f9eefb319e", + "panelRefName": "panel_0", + "version": "7.5.2" + }, + { + "embeddableConfig": { + "enhancements": {} + }, + "gridData": { + "h": 15, + "i": "b09a478f-8a72-44d3-bbcc-1f2a546c74f0", + "w": 24, + "x": 0, + "y": 0 + }, + "panelIndex": "b09a478f-8a72-44d3-bbcc-1f2a546c74f0", + "panelRefName": "panel_1", + "version": "7.5.2" + } + ], + "timeRestore": false, + "title": "Dashboard by References", + "version": 1 + }, + "id": "visualizations_by_reference-82273ffe-6acc-4f2f-bbee-c1004abba63d", + "references": [ + { + "id": "visualizations_by_reference-5e1a01ff-6f9a-41c1-b7ad-326472db42b6", + "name": "panel_0", + "type": "visualization" + }, + { + "id": "visualizations_by_reference-8287a5d5-1576-4f3a-83c4-444e9058439b", + "name": "panel_1", + "type": "visualization" + } + ], + "type": "dashboard" +} diff --git a/test/packages/visualizations_by_reference/kibana/visualization/visualizations_by_reference-5e1a01ff-6f9a-41c1-b7ad-326472db42b6.json b/test/packages/visualizations_by_reference/kibana/visualization/visualizations_by_reference-5e1a01ff-6f9a-41c1-b7ad-326472db42b6.json new file mode 100644 index 000000000..61fe9366d --- /dev/null +++ b/test/packages/visualizations_by_reference/kibana/visualization/visualizations_by_reference-5e1a01ff-6f9a-41c1-b7ad-326472db42b6.json @@ -0,0 +1,5 @@ +{ + "id": "visualizations_by_reference-5e1a01ff-6f9a-41c1-b7ad-326472db42b6", + "references": [], + "type": "visualization" +} diff --git a/test/packages/visualizations_by_reference/kibana/visualization/visualizations_by_reference-8287a5d5-1576-4f3a-83c4-444e9058439b.json b/test/packages/visualizations_by_reference/kibana/visualization/visualizations_by_reference-8287a5d5-1576-4f3a-83c4-444e9058439b.json new file mode 100644 index 000000000..2cb91ce09 --- /dev/null +++ b/test/packages/visualizations_by_reference/kibana/visualization/visualizations_by_reference-8287a5d5-1576-4f3a-83c4-444e9058439b.json @@ -0,0 +1,5 @@ +{ + "id": "visualizations_by_reference-8287a5d5-1576-4f3a-83c4-444e9058439b", + "references": [], + "type": "visualization" +} diff --git a/test/packages/visualizations_by_reference/manifest.yml b/test/packages/visualizations_by_reference/manifest.yml new file mode 100644 index 000000000..909840ab7 --- /dev/null +++ b/test/packages/visualizations_by_reference/manifest.yml @@ -0,0 +1,28 @@ +format_version: 1.0.4 +name: visualizations_by_reference +title: BAD +description: This package is bad. +version: 0.1.2 +type: integration +release: beta +conditions: + kibana.version: '^7.9.0' +policy_templates: + - name: apache + title: Apache logs and metrics + description: Collect logs and metrics from Apache instances + inputs: + - type: apache/metrics + title: Collect metrics from Apache instances + description: Collecting Apache status metrics + vars: + - name: hosts + type: text + title: Hosts + multi: true + required: true + show_user: true + default: + - http://127.0.0.1 +owner: + github: elastic/foobar From d4665f0feee59623fa0509e13376c4e15abc9e21 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Date: Fri, 12 Aug 2022 15:49:10 +0200 Subject: [PATCH 02/17] fix lint --- .../semantic/validate_kibana_dashboard_by_value.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/code/go/internal/validator/semantic/validate_kibana_dashboard_by_value.go b/code/go/internal/validator/semantic/validate_kibana_dashboard_by_value.go index 46e2f2e2a..bafa5df95 100644 --- a/code/go/internal/validator/semantic/validate_kibana_dashboard_by_value.go +++ b/code/go/internal/validator/semantic/validate_kibana_dashboard_by_value.go @@ -17,7 +17,7 @@ import ( ) type reference struct { - Id string + ID string Name string Type string } @@ -54,8 +54,8 @@ func ValidateDashboardsByValue(fsys fspath.FS) ve.ValidationErrors { for _, reference := range references { if reference.Type == "visualization" { - // errs = append(errs, fmt.Errorf("Kibana Dashboard %s contains a visualization by reference \"%s\"", fsys.Path(filePath), reference.Id)) - log.Printf("Warning: Kibana Dashboard %s contains a visualization by reference \"%s\"", fsys.Path(filePath), reference.Id) + // errs = append(errs, fmt.Errorf("Kibana Dashboard %s contains a visualization by reference \"%s\"", fsys.Path(filePath), reference.ID)) + log.Printf("Warning: Kibana Dashboard %s contains a visualization by reference \"%s\"", fsys.Path(filePath), reference.ID) } } } From 04b0262842db80a5a81858abfa28b8f460d17068 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Date: Fri, 12 Aug 2022 15:49:16 +0200 Subject: [PATCH 03/17] add changelog --- versions/1/changelog.yml | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/versions/1/changelog.yml b/versions/1/changelog.yml index 82bad56b5..d4179cd02 100644 --- a/versions/1/changelog.yml +++ b/versions/1/changelog.yml @@ -4,15 +4,18 @@ ## - version: 1.14.1-next changes: - - description: Add support for icons in dark mode - type: enhancement - link: https://github.com/elastic/package-spec/pull/378 - description: Add definition for the license file in a package type: enhancement link: https://github.com/elastic/package-spec/pull/367 - description: Prepare for next patch release type: enhancement link: https://github.com/elastic/package-spec/pull/375 + - description: Add support for icons in dark mode + type: enhancement + link: https://github.com/elastic/package-spec/pull/378 + - description: Add warning when dashboards include visualizations by reference + type: enhancement + link: https://github.com/elastic/package-spec/pull/389 - version: 1.14.0 changes: - description: Add 'textarea' var type From cdf5076d993ec590701770e0eabbd54381260a25 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Date: Fri, 12 Aug 2022 15:56:23 +0200 Subject: [PATCH 04/17] Rename validate method --- .../semantic/validate_kibana_dashboard_by_value.go | 9 +++++---- code/go/internal/validator/spec.go | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/code/go/internal/validator/semantic/validate_kibana_dashboard_by_value.go b/code/go/internal/validator/semantic/validate_kibana_dashboard_by_value.go index bafa5df95..fb9a30db5 100644 --- a/code/go/internal/validator/semantic/validate_kibana_dashboard_by_value.go +++ b/code/go/internal/validator/semantic/validate_kibana_dashboard_by_value.go @@ -22,11 +22,12 @@ type reference struct { Type string } -// ValidateDashboardsByValue returns validation errors if there are any Kibana +// ValidateVisualizationsUsedByValue warns if there are any Kibana // Dashboard that defines visualizations by reference instead of value. -// That is, it returns validation errors if a Kibana dashbaord file, foo.json, -// defines some visualization using reference (key panelRef). -func ValidateDashboardsByValue(fsys fspath.FS) ve.ValidationErrors { +// That is, it warns if a Kibana dashbaord file, foo.json, +// defines some visualization using reference (containing an element of +// "visualization" type inside references key). +func ValidateVisualizationsUsedByValue(fsys fspath.FS) ve.ValidationErrors { var errs ve.ValidationErrors filePaths := path.Join("kibana", "dashboard", "*.json") diff --git a/code/go/internal/validator/spec.go b/code/go/internal/validator/spec.go index 660da77be..ce2989d11 100644 --- a/code/go/internal/validator/spec.go +++ b/code/go/internal/validator/spec.go @@ -76,7 +76,7 @@ func (s Spec) ValidatePackage(pkg Package) ve.ValidationErrors { //semantic.ValidateUniqueFields, semantic.ValidateDimensionFields, semantic.ValidateRequiredFields, - semantic.ValidateDashboardsByValue, + semantic.ValidateVisualizationsUsedByValue, } return rules.validate(&pkg) From 5710b4414c5b473a1f3745e9648e42261bdf7b67 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Date: Fri, 12 Aug 2022 15:57:36 +0200 Subject: [PATCH 05/17] rename file --- ...dashboard_by_value.go => validate_dashboards_used_by_value.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename code/go/internal/validator/semantic/{validate_kibana_dashboard_by_value.go => validate_dashboards_used_by_value.go} (100%) diff --git a/code/go/internal/validator/semantic/validate_kibana_dashboard_by_value.go b/code/go/internal/validator/semantic/validate_dashboards_used_by_value.go similarity index 100% rename from code/go/internal/validator/semantic/validate_kibana_dashboard_by_value.go rename to code/go/internal/validator/semantic/validate_dashboards_used_by_value.go From 42917c0b129f4cef7d10456cb81c28b585d6897a Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Date: Fri, 12 Aug 2022 17:31:07 +0200 Subject: [PATCH 06/17] Add tests --- .../validate_dashboards_used_by_value.go | 31 +++--- .../validate_dashboards_used_by_value_test.go | 98 +++++++++++++++++++ 2 files changed, 117 insertions(+), 12 deletions(-) create mode 100644 code/go/internal/validator/semantic/validate_dashboards_used_by_value_test.go diff --git a/code/go/internal/validator/semantic/validate_dashboards_used_by_value.go b/code/go/internal/validator/semantic/validate_dashboards_used_by_value.go index fb9a30db5..47e094675 100644 --- a/code/go/internal/validator/semantic/validate_dashboards_used_by_value.go +++ b/code/go/internal/validator/semantic/validate_dashboards_used_by_value.go @@ -40,38 +40,45 @@ func ValidateVisualizationsUsedByValue(fsys fspath.FS) ve.ValidationErrors { for _, objectFile := range objectFiles { filePath := objectFile.Path() + // objectReferences, err := objectFile.Values("$.references[?(@.type=='visualization')]") objectReferences, err := objectFile.Values("$.references") if err != nil { // log.Printf("Warning: unable to get kibana dashboard references in file [%s]", fsys.Path(filePath)) // errs = append(errs, errors.Wrapf(err, "unable to get Kibana Dashboard references in file [%s]", fsys.Path(filePath))) continue } - references, err := toReferenceSlice(objectReferences) if err != nil { errs = append(errs, errors.Wrapf(err, "unable to convert references in file [%s]", fsys.Path(filePath))) continue } - - for _, reference := range references { - if reference.Type == "visualization" { - // errs = append(errs, fmt.Errorf("Kibana Dashboard %s contains a visualization by reference \"%s\"", fsys.Path(filePath), reference.ID)) - log.Printf("Warning: Kibana Dashboard %s contains a visualization by reference \"%s\"", fsys.Path(filePath), reference.ID) - } + if checkAnyVisualizationByReference(references) { + // errs = append(errs, fmt.Errorf("Kibana Dashboard %s contains a visualization by reference \"%s\"", fsys.Path(filePath), reference.ID)) + log.Printf("Warning: Kibana Dashboard %s contains a visualization by reference", fsys.Path(filePath)) } } return errs } -func toReferenceSlice(val interface{}) ([]reference, error) { - vals, ok := val.([]interface{}) - if !ok { - return nil, errors.New("conversion error") +func checkAnyVisualizationByReference(references []reference) bool { + byReference := false + if len(references) > 0 { + log.Printf("WARNING Dashboard with visualization by reference") + } + + for _, reference := range references { + if reference.Type == "visualization" { + log.Printf("Warning: Visualization by reference: %s", reference.ID) + byReference = true + } } + return byReference +} +func toReferenceSlice(val interface{}) ([]reference, error) { var refs []reference - jsonbody, err := json.Marshal(vals) + jsonbody, err := json.Marshal(val) if err != nil { log.Printf("error encoding reference list: %s", err) return refs, nil diff --git a/code/go/internal/validator/semantic/validate_dashboards_used_by_value_test.go b/code/go/internal/validator/semantic/validate_dashboards_used_by_value_test.go new file mode 100644 index 000000000..8c6729529 --- /dev/null +++ b/code/go/internal/validator/semantic/validate_dashboards_used_by_value_test.go @@ -0,0 +1,98 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package semantic + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestToReferencesToSlice(t *testing.T) { + + var tests = []struct { + name string + references []map[string]string + expected []reference + }{ + { + "References", + []map[string]string{ + { + "id": "12345", + "name": "panel_0", + "type": "visualization", + }, + { + "id": "9000", + "name": "panel_1", + "type": "other", + }, + }, + []reference{ + reference{ + ID: "12345", + Name: "panel_0", + Type: "visualization", + }, + reference{ + ID: "9000", + Name: "panel_1", + Type: "other", + }, + }, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + references, err := toReferenceSlice(test.references) + require.NoError(t, err) + assert.Equal(t, test.expected, references) + }) + } +} + +func TestAnyVisualizationByReference(t *testing.T) { + + var tests = []struct { + name string + references []reference + expected bool + }{ + { + "AllReferences", + []reference{ + {"12345", "panel_0", "visualization"}, + {"9000", "panel_1", "visualization"}, + }, + true, + }, + { + "NoneReferences", + []reference{{"42", "panel_42", "other"}}, + false, + }, + { + "Empty", + []reference{}, + false, + }, + { + "SomeReferences", + []reference{ + {"12345", "panel_0", "visualization"}, + {"42", "panel_42", "other"}, + }, + true, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + anyReference := checkAnyVisualizationByReference(test.references) + assert.Equal(t, test.expected, anyReference) + }) + } +} From 84211e0e35b4ff6b293af79640b9405b1adc4113 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Date: Tue, 16 Aug 2022 17:25:42 +0200 Subject: [PATCH 07/17] Use json path to get visualization references --- .../validate_dashboards_used_by_value.go | 39 ++++++++----------- .../validate_dashboards_used_by_value_test.go | 39 +++++++++---------- 2 files changed, 36 insertions(+), 42 deletions(-) diff --git a/code/go/internal/validator/semantic/validate_dashboards_used_by_value.go b/code/go/internal/validator/semantic/validate_dashboards_used_by_value.go index 47e094675..3603915bd 100644 --- a/code/go/internal/validator/semantic/validate_dashboards_used_by_value.go +++ b/code/go/internal/validator/semantic/validate_dashboards_used_by_value.go @@ -6,6 +6,7 @@ package semantic import ( "encoding/json" + "fmt" "log" "path" @@ -40,40 +41,34 @@ func ValidateVisualizationsUsedByValue(fsys fspath.FS) ve.ValidationErrors { for _, objectFile := range objectFiles { filePath := objectFile.Path() - // objectReferences, err := objectFile.Values("$.references[?(@.type=='visualization')]") - objectReferences, err := objectFile.Values("$.references") + objectReferences, err := objectFile.Values(`$.references[?(@.type=="visualization")]`) if err != nil { - // log.Printf("Warning: unable to get kibana dashboard references in file [%s]", fsys.Path(filePath)) - // errs = append(errs, errors.Wrapf(err, "unable to get Kibana Dashboard references in file [%s]", fsys.Path(filePath))) + errs = append(errs, errors.Wrapf(err, "unable to get Kibana Dashboard references in file [%s]", fsys.Path(filePath))) continue } - references, err := toReferenceSlice(objectReferences) - if err != nil { - errs = append(errs, errors.Wrapf(err, "unable to convert references in file [%s]", fsys.Path(filePath))) - continue - } - if checkAnyVisualizationByReference(references) { - // errs = append(errs, fmt.Errorf("Kibana Dashboard %s contains a visualization by reference \"%s\"", fsys.Path(filePath), reference.ID)) - log.Printf("Warning: Kibana Dashboard %s contains a visualization by reference", fsys.Path(filePath)) - } + + anyReference(objectReferences, fsys.Path(filePath)) } return errs } -func checkAnyVisualizationByReference(references []reference) bool { - byReference := false - if len(references) > 0 { - log.Printf("WARNING Dashboard with visualization by reference") +func anyReference(val interface{}, path string) (bool, error) { + references, err := toReferenceSlice(val) + if err != nil { + return false, fmt.Errorf("unable to convert references in file [%s]", path) + } + + if len(references) == 0 { + return false, nil } + log.Printf("Warning: Kibana Dashboard %s contains visualizations by reference:", path) for _, reference := range references { - if reference.Type == "visualization" { - log.Printf("Warning: Visualization by reference: %s", reference.ID) - byReference = true - } + log.Printf("Warning: visualization by reference found: %s", reference.ID) } - return byReference + return true, nil + } func toReferenceSlice(val interface{}) ([]reference, error) { diff --git a/code/go/internal/validator/semantic/validate_dashboards_used_by_value_test.go b/code/go/internal/validator/semantic/validate_dashboards_used_by_value_test.go index 8c6729529..a806eebc5 100644 --- a/code/go/internal/validator/semantic/validate_dashboards_used_by_value_test.go +++ b/code/go/internal/validator/semantic/validate_dashboards_used_by_value_test.go @@ -55,43 +55,42 @@ func TestToReferencesToSlice(t *testing.T) { } } -func TestAnyVisualizationByReference(t *testing.T) { +func TestAnyReference(t *testing.T) { var tests = []struct { name string - references []reference + path string + references []map[string]string expected bool }{ { "AllReferences", - []reference{ - {"12345", "panel_0", "visualization"}, - {"9000", "panel_1", "visualization"}, + "path", + []map[string]string{ + { + "id": "12345", + "name": "panel_0", + "type": "visualization", + }, + { + "id": "9000", + "name": "panel_1", + "type": "visualization", + }, }, true, }, - { - "NoneReferences", - []reference{{"42", "panel_42", "other"}}, - false, - }, { "Empty", - []reference{}, + "path", + []map[string]string{}, false, }, - { - "SomeReferences", - []reference{ - {"12345", "panel_0", "visualization"}, - {"42", "panel_42", "other"}, - }, - true, - }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - anyReference := checkAnyVisualizationByReference(test.references) + anyReference, err := anyReference(test.references, test.path) + require.NoError(t, err) assert.Equal(t, test.expected, anyReference) }) } From b51efdd86caa9a0bfa6f3488078f1a74811eb4bf Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Date: Wed, 17 Aug 2022 11:33:59 +0200 Subject: [PATCH 08/17] Test lens and visualization --- .../validate_dashboards_used_by_value.go | 36 ++++++++++++------- .../validate_dashboards_used_by_value_test.go | 24 +++++++++---- 2 files changed, 41 insertions(+), 19 deletions(-) diff --git a/code/go/internal/validator/semantic/validate_dashboards_used_by_value.go b/code/go/internal/validator/semantic/validate_dashboards_used_by_value.go index 3603915bd..c9d830ae5 100644 --- a/code/go/internal/validator/semantic/validate_dashboards_used_by_value.go +++ b/code/go/internal/validator/semantic/validate_dashboards_used_by_value.go @@ -9,6 +9,7 @@ import ( "fmt" "log" "path" + "strings" "github.com/pkg/errors" @@ -41,33 +42,44 @@ func ValidateVisualizationsUsedByValue(fsys fspath.FS) ve.ValidationErrors { for _, objectFile := range objectFiles { filePath := objectFile.Path() - objectReferences, err := objectFile.Values(`$.references[?(@.type=="visualization")]`) + objectReferences, err := objectFile.Values(`$.references`) if err != nil { - errs = append(errs, errors.Wrapf(err, "unable to get Kibana Dashboard references in file [%s]", fsys.Path(filePath))) + // no references key in dashboard json + // errs = append(errs, errors.Wrapf(err, "unable to get Kibana Dashboard references in file [%s]", fsys.Path(filePath))) continue } - anyReference(objectReferences, fsys.Path(filePath)) + ids, err := anyReference(objectReferences, fsys.Path(filePath)) + if err != nil { + errs = append(errs, errors.Wrap(err, "error getting references")) + } + if len(ids) > 0 { + log.Printf("Warning: visualization by reference found in %s: %s", filePath, strings.Join(ids, ", ")) + } } return errs } -func anyReference(val interface{}, path string) (bool, error) { - references, err := toReferenceSlice(val) +func anyReference(val interface{}, path string) ([]string, error) { + allReferences, err := toReferenceSlice(val) if err != nil { - return false, fmt.Errorf("unable to convert references in file [%s]", path) + return []string{}, fmt.Errorf("unable to convert references in file [%s]: %w", path, err) } - if len(references) == 0 { - return false, nil + if len(allReferences) == 0 { + return []string{}, nil } - log.Printf("Warning: Kibana Dashboard %s contains visualizations by reference:", path) - for _, reference := range references { - log.Printf("Warning: visualization by reference found: %s", reference.ID) + var idReferences []string + for _, reference := range allReferences { + switch reference.Type { + case "lens", "visualization": + log.Printf("Warning: %s by reference found: %s (dashboard %s)", reference.Type, reference.ID, path) + idReferences = append(idReferences, reference.ID) + } } - return true, nil + return idReferences, nil } diff --git a/code/go/internal/validator/semantic/validate_dashboards_used_by_value_test.go b/code/go/internal/validator/semantic/validate_dashboards_used_by_value_test.go index a806eebc5..6f28b3922 100644 --- a/code/go/internal/validator/semantic/validate_dashboards_used_by_value_test.go +++ b/code/go/internal/validator/semantic/validate_dashboards_used_by_value_test.go @@ -61,10 +61,10 @@ func TestAnyReference(t *testing.T) { name string path string references []map[string]string - expected bool + expected []string }{ { - "AllReferences", + "SomeReferences", "path", []map[string]string{ { @@ -75,23 +75,33 @@ func TestAnyReference(t *testing.T) { { "id": "9000", "name": "panel_1", - "type": "visualization", + "type": "lens", + }, + { + "id": "4", + "name": "panel_1", + "type": "map", + }, + { + "id": "42", + "name": "panel_1", + "type": "index-pattern", }, }, - true, + []string{"12345", "9000"}, }, { "Empty", "path", []map[string]string{}, - false, + []string{}, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - anyReference, err := anyReference(test.references, test.path) + ids, err := anyReference(test.references, test.path) require.NoError(t, err) - assert.Equal(t, test.expected, anyReference) + assert.Equal(t, test.expected, ids) }) } } From 67886df35494afe69e397cbc29383829ffe0ee11 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Date: Wed, 17 Aug 2022 12:26:11 +0200 Subject: [PATCH 09/17] Update toReferenceSlice Update also messages shown to indicate both the id and type of the reference found. --- .../validate_dashboards_used_by_value.go | 53 ++++++++++--------- .../validate_dashboards_used_by_value_test.go | 31 ++++++----- ...-82273ffe-6acc-4f2f-bbee-c1004abba63d.json | 2 +- 3 files changed, 47 insertions(+), 39 deletions(-) diff --git a/code/go/internal/validator/semantic/validate_dashboards_used_by_value.go b/code/go/internal/validator/semantic/validate_dashboards_used_by_value.go index c9d830ae5..ca5e9cdfe 100644 --- a/code/go/internal/validator/semantic/validate_dashboards_used_by_value.go +++ b/code/go/internal/validator/semantic/validate_dashboards_used_by_value.go @@ -5,11 +5,9 @@ package semantic import ( - "encoding/json" "fmt" "log" "path" - "strings" "github.com/pkg/errors" @@ -45,57 +43,64 @@ func ValidateVisualizationsUsedByValue(fsys fspath.FS) ve.ValidationErrors { objectReferences, err := objectFile.Values(`$.references`) if err != nil { // no references key in dashboard json - // errs = append(errs, errors.Wrapf(err, "unable to get Kibana Dashboard references in file [%s]", fsys.Path(filePath))) continue } - ids, err := anyReference(objectReferences, fsys.Path(filePath)) + references, err := anyReference(objectReferences) if err != nil { - errs = append(errs, errors.Wrap(err, "error getting references")) + errs = append(errs, errors.Wrap(err, "error getting references in file: %s", fsys.Path(filePath))) } - if len(ids) > 0 { - log.Printf("Warning: visualization by reference found in %s: %s", filePath, strings.Join(ids, ", ")) + if len(references) > 0 { + s := fmt.Sprintf("%s (%s)", references[0].ID, references[0].Type) + for _, ref := range references[1:] { + s = fmt.Sprintf("%s, %s (%s)", s, ref.ID, ref.Type) + } + log.Printf("Warning: references found in dashboard %s: %s", filePath, s) } } return errs } -func anyReference(val interface{}, path string) ([]string, error) { +func anyReference(val interface{}) ([]reference, error) { allReferences, err := toReferenceSlice(val) if err != nil { - return []string{}, fmt.Errorf("unable to convert references in file [%s]: %w", path, err) + return []reference{}, fmt.Errorf("unable to convert references: %w", err) } if len(allReferences) == 0 { - return []string{}, nil + return []reference{}, nil } - var idReferences []string + var references []reference for _, reference := range allReferences { switch reference.Type { case "lens", "visualization": - log.Printf("Warning: %s by reference found: %s (dashboard %s)", reference.Type, reference.ID, path) - idReferences = append(idReferences, reference.ID) + references = append(references, reference) } } - return idReferences, nil + return references, nil } func toReferenceSlice(val interface{}) ([]reference, error) { - var refs []reference - jsonbody, err := json.Marshal(val) - if err != nil { - log.Printf("error encoding reference list: %s", err) - return refs, nil + vals, ok := val.([]interface{}) + if !ok { + return nil, errors.New("conversion error to array") } + var refs []reference + for _, v := range vals { + r, ok := v.(map[string]interface{}) + if !ok { + return nil, errors.New("conversion error to reference element") + } + ref := reference{ + ID: r["id"].(string), + Type: r["type"].(string), + Name: r["name"].(string), + } - err = json.Unmarshal(jsonbody, &refs) - if err != nil { - log.Printf("error unmarshaling references: %s", err) - return refs, nil + refs = append(refs, ref) } - return refs, nil } diff --git a/code/go/internal/validator/semantic/validate_dashboards_used_by_value_test.go b/code/go/internal/validator/semantic/validate_dashboards_used_by_value_test.go index 6f28b3922..7ded94067 100644 --- a/code/go/internal/validator/semantic/validate_dashboards_used_by_value_test.go +++ b/code/go/internal/validator/semantic/validate_dashboards_used_by_value_test.go @@ -15,18 +15,18 @@ func TestToReferencesToSlice(t *testing.T) { var tests = []struct { name string - references []map[string]string + references interface{} expected []reference }{ { "References", - []map[string]string{ - { + []interface{}{ + map[string]interface{}{ "id": "12345", "name": "panel_0", "type": "visualization", }, - { + map[string]interface{}{ "id": "9000", "name": "panel_1", "type": "other", @@ -60,41 +60,44 @@ func TestAnyReference(t *testing.T) { var tests = []struct { name string path string - references []map[string]string - expected []string + references interface{} + expected []reference }{ { "SomeReferences", "path", - []map[string]string{ - { + []interface{}{ + map[string]interface{}{ "id": "12345", "name": "panel_0", "type": "visualization", }, - { + map[string]interface{}{ "id": "9000", "name": "panel_1", "type": "lens", }, - { + map[string]interface{}{ "id": "4", "name": "panel_1", "type": "map", }, - { + map[string]interface{}{ "id": "42", "name": "panel_1", "type": "index-pattern", }, }, - []string{"12345", "9000"}, + []reference{ + {"12345", "panel_0", "visualization"}, + {"9000", "panel_1", "lens"}, + }, }, { "Empty", "path", - []map[string]string{}, - []string{}, + []interface{}{}, + []reference{}, }, } for _, test := range tests { diff --git a/test/packages/visualizations_by_reference/kibana/dashboard/visualizations_by_reference-82273ffe-6acc-4f2f-bbee-c1004abba63d.json b/test/packages/visualizations_by_reference/kibana/dashboard/visualizations_by_reference-82273ffe-6acc-4f2f-bbee-c1004abba63d.json index 0ed721d69..a44c7d494 100644 --- a/test/packages/visualizations_by_reference/kibana/dashboard/visualizations_by_reference-82273ffe-6acc-4f2f-bbee-c1004abba63d.json +++ b/test/packages/visualizations_by_reference/kibana/dashboard/visualizations_by_reference-82273ffe-6acc-4f2f-bbee-c1004abba63d.json @@ -47,7 +47,7 @@ { "id": "visualizations_by_reference-8287a5d5-1576-4f3a-83c4-444e9058439b", "name": "panel_1", - "type": "visualization" + "type": "lens" } ], "type": "dashboard" From bf278520f3e933cefe3ee3891682f18ebab7d7ed Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Date: Wed, 17 Aug 2022 12:42:25 +0200 Subject: [PATCH 10/17] Rename files --- ..._used_by_value.go => validate_visualizations_used_by_value.go} | 0 ...alue_test.go => validate_visualizations_used_by_value_test.go} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename code/go/internal/validator/semantic/{validate_dashboards_used_by_value.go => validate_visualizations_used_by_value.go} (100%) rename code/go/internal/validator/semantic/{validate_dashboards_used_by_value_test.go => validate_visualizations_used_by_value_test.go} (100%) diff --git a/code/go/internal/validator/semantic/validate_dashboards_used_by_value.go b/code/go/internal/validator/semantic/validate_visualizations_used_by_value.go similarity index 100% rename from code/go/internal/validator/semantic/validate_dashboards_used_by_value.go rename to code/go/internal/validator/semantic/validate_visualizations_used_by_value.go diff --git a/code/go/internal/validator/semantic/validate_dashboards_used_by_value_test.go b/code/go/internal/validator/semantic/validate_visualizations_used_by_value_test.go similarity index 100% rename from code/go/internal/validator/semantic/validate_dashboards_used_by_value_test.go rename to code/go/internal/validator/semantic/validate_visualizations_used_by_value_test.go From 008ec0dacba362a4671965d917370a9f8666d369 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Date: Wed, 17 Aug 2022 13:01:23 +0200 Subject: [PATCH 11/17] fix arguments --- .../semantic/validate_visualizations_used_by_value.go | 2 +- .../semantic/validate_visualizations_used_by_value_test.go | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/code/go/internal/validator/semantic/validate_visualizations_used_by_value.go b/code/go/internal/validator/semantic/validate_visualizations_used_by_value.go index ca5e9cdfe..787e74337 100644 --- a/code/go/internal/validator/semantic/validate_visualizations_used_by_value.go +++ b/code/go/internal/validator/semantic/validate_visualizations_used_by_value.go @@ -48,7 +48,7 @@ func ValidateVisualizationsUsedByValue(fsys fspath.FS) ve.ValidationErrors { references, err := anyReference(objectReferences) if err != nil { - errs = append(errs, errors.Wrap(err, "error getting references in file: %s", fsys.Path(filePath))) + errs = append(errs, errors.Wrapf(err, "error getting references in file: %s", fsys.Path(filePath))) } if len(references) > 0 { s := fmt.Sprintf("%s (%s)", references[0].ID, references[0].Type) diff --git a/code/go/internal/validator/semantic/validate_visualizations_used_by_value_test.go b/code/go/internal/validator/semantic/validate_visualizations_used_by_value_test.go index 7ded94067..ea65c0255 100644 --- a/code/go/internal/validator/semantic/validate_visualizations_used_by_value_test.go +++ b/code/go/internal/validator/semantic/validate_visualizations_used_by_value_test.go @@ -59,13 +59,11 @@ func TestAnyReference(t *testing.T) { var tests = []struct { name string - path string references interface{} expected []reference }{ { "SomeReferences", - "path", []interface{}{ map[string]interface{}{ "id": "12345", @@ -95,14 +93,13 @@ func TestAnyReference(t *testing.T) { }, { "Empty", - "path", []interface{}{}, []reference{}, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - ids, err := anyReference(test.references, test.path) + ids, err := anyReference(test.references) require.NoError(t, err) assert.Equal(t, test.expected, ids) }) From 06f27a9a8f7bc212ad4017cbbae45b8d43f6a4e8 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Date: Wed, 17 Aug 2022 19:08:34 +0200 Subject: [PATCH 12/17] Add a new env.var to enable transform warnings into errors for testing --- code/go/internal/validator/common/helpers.go | 35 ++++++++++++ .../internal/validator/common/helpers_test.go | 44 ++++++++++++++ code/go/internal/validator/folder_spec.go | 7 ++- .../semantic/validate_required_fields.go | 3 +- .../semantic/validate_unique_fields.go | 3 +- .../validate_visualizations_used_by_value.go | 9 ++- code/go/internal/validator/spec_test.go | 7 ++- code/go/pkg/validator/validator_test.go | 57 +++++++++++++++++-- 8 files changed, 154 insertions(+), 11 deletions(-) create mode 100644 code/go/internal/validator/common/helpers.go create mode 100644 code/go/internal/validator/common/helpers_test.go diff --git a/code/go/internal/validator/common/helpers.go b/code/go/internal/validator/common/helpers.go new file mode 100644 index 000000000..bb9b0dbf2 --- /dev/null +++ b/code/go/internal/validator/common/helpers.go @@ -0,0 +1,35 @@ +package common + +import ( + "os" + "strconv" +) + +const EnvVarWarningsAsErrors = "PACKAGE_SPEC_WARNINGS_AS_ERRORS" + +func IsDefinedWarningsAsErrors() bool { + var err error + warningsAsErrors := false + warningsAsErrorsStr, found := os.LookupEnv(EnvVarWarningsAsErrors) + if found { + warningsAsErrors, err = strconv.ParseBool(warningsAsErrorsStr) + if err != nil { + return false + } + } + return warningsAsErrors +} + +func EnableWarningsAsErrors() error { + if err := os.Setenv(EnvVarWarningsAsErrors, "true"); err != nil { + return err + } + return nil +} + +func DisableWarningsAsErrors() error { + if err := os.Unsetenv(EnvVarWarningsAsErrors); err != nil { + return err + } + return nil +} diff --git a/code/go/internal/validator/common/helpers_test.go b/code/go/internal/validator/common/helpers_test.go new file mode 100644 index 000000000..3c5afab86 --- /dev/null +++ b/code/go/internal/validator/common/helpers_test.go @@ -0,0 +1,44 @@ +package common + +import ( + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestIsDefinedWarningsAsErrors(t *testing.T) { + cases := []struct { + name string + envVarValue string + expected bool + }{ + {"true", "true", true}, + {"false", "false", false}, + {"other", "other", false}, + {"empty", "", false}, + } + + for _, test := range cases { + t.Run(test.name, func(t *testing.T) { + if err := os.Setenv(EnvVarWarningsAsErrors, test.envVarValue); err != nil { + require.NoError(t, err) + } + value := IsDefinedWarningsAsErrors() + assert.Equal(t, test.expected, value) + + if err := DisableWarningsAsErrors(); err != nil { + require.NoError(t, err) + } + }) + } + + t.Run("undefined", func(t *testing.T) { + if err := DisableWarningsAsErrors(); err != nil { + require.NoError(t, err) + } + value := IsDefinedWarningsAsErrors() + assert.Equal(t, false, value) + }) +} diff --git a/code/go/internal/validator/folder_spec.go b/code/go/internal/validator/folder_spec.go index 2df42a84b..2023a25de 100644 --- a/code/go/internal/validator/folder_spec.go +++ b/code/go/internal/validator/folder_spec.go @@ -18,6 +18,7 @@ import ( ve "github.com/elastic/package-spec/code/go/internal/errors" "github.com/elastic/package-spec/code/go/internal/spectypes" + "github.com/elastic/package-spec/code/go/internal/validator/common" ) const ( @@ -93,7 +94,11 @@ func (s *folderSpec) validate(pkg *Package, folderPath string) ve.ValidationErro if pkg.Version.Major() > 0 && pkg.Version.Prerelease() == "" { errs = append(errs, errors.Errorf("spec for [%s] defines beta features which can't be enabled for packages with a stable semantic version", pkg.Path(folderPath))) } else { - log.Printf("Warning: package with non-stable semantic version and active beta features (enabled in [%s]) can't be released as stable version.", pkg.Path(folderPath)) + if common.IsDefinedWarningsAsErrors() { + errs = append(errs, errors.Errorf("Warning: package with non-stable semantic version and active beta features (enabled in [%s]) can't be released as stable version.", pkg.Path(folderPath))) + } else { + log.Printf("Warning: package with non-stable semantic version and active beta features (enabled in [%s]) can't be released as stable version.", pkg.Path(folderPath)) + } } default: errs = append(errs, errors.Errorf("unsupport release level, supported values: beta, ga")) diff --git a/code/go/internal/validator/semantic/validate_required_fields.go b/code/go/internal/validator/semantic/validate_required_fields.go index 756eb99dc..c1b0764e1 100644 --- a/code/go/internal/validator/semantic/validate_required_fields.go +++ b/code/go/internal/validator/semantic/validate_required_fields.go @@ -5,9 +5,10 @@ package semantic import ( + "github.com/pkg/errors" + ve "github.com/elastic/package-spec/code/go/internal/errors" "github.com/elastic/package-spec/code/go/internal/fspath" - "github.com/pkg/errors" ) // ValidateRequiredFields validates that required fields are present and have the expected diff --git a/code/go/internal/validator/semantic/validate_unique_fields.go b/code/go/internal/validator/semantic/validate_unique_fields.go index 551aa895e..c7b2249ec 100644 --- a/code/go/internal/validator/semantic/validate_unique_fields.go +++ b/code/go/internal/validator/semantic/validate_unique_fields.go @@ -8,9 +8,10 @@ import ( "sort" "strings" + "github.com/pkg/errors" + ve "github.com/elastic/package-spec/code/go/internal/errors" "github.com/elastic/package-spec/code/go/internal/fspath" - "github.com/pkg/errors" ) // ValidateUniqueFields verifies that any field is defined only once on each data stream. diff --git a/code/go/internal/validator/semantic/validate_visualizations_used_by_value.go b/code/go/internal/validator/semantic/validate_visualizations_used_by_value.go index 787e74337..8cacc2e4d 100644 --- a/code/go/internal/validator/semantic/validate_visualizations_used_by_value.go +++ b/code/go/internal/validator/semantic/validate_visualizations_used_by_value.go @@ -14,6 +14,7 @@ import ( ve "github.com/elastic/package-spec/code/go/internal/errors" "github.com/elastic/package-spec/code/go/internal/fspath" "github.com/elastic/package-spec/code/go/internal/pkgpath" + "github.com/elastic/package-spec/code/go/internal/validator/common" ) type reference struct { @@ -28,6 +29,7 @@ type reference struct { // defines some visualization using reference (containing an element of // "visualization" type inside references key). func ValidateVisualizationsUsedByValue(fsys fspath.FS) ve.ValidationErrors { + warningsAsErrors := common.IsDefinedWarningsAsErrors() var errs ve.ValidationErrors filePaths := path.Join("kibana", "dashboard", "*.json") @@ -55,7 +57,12 @@ func ValidateVisualizationsUsedByValue(fsys fspath.FS) ve.ValidationErrors { for _, ref := range references[1:] { s = fmt.Sprintf("%s, %s (%s)", s, ref.ID, ref.Type) } - log.Printf("Warning: references found in dashboard %s: %s", filePath, s) + if warningsAsErrors { + errs = append(errs, errors.Errorf("Warning: references found in dashboard %s: %s", filePath, s)) + } else { + log.Printf("Warning: references found in dashboard %s: %s", filePath, s) + } + } } diff --git a/code/go/internal/validator/spec_test.go b/code/go/internal/validator/spec_test.go index 08d61c466..6d4f4f81d 100644 --- a/code/go/internal/validator/spec_test.go +++ b/code/go/internal/validator/spec_test.go @@ -5,9 +5,10 @@ package validator import ( - "github.com/elastic/package-spec/code/go/internal/fspath" "testing" + "github.com/elastic/package-spec/code/go/internal/fspath" + "github.com/Masterminds/semver/v3" "github.com/stretchr/testify/require" ) @@ -60,6 +61,6 @@ func TestBetaFeatures_Package_GA(t *testing.T) { require.NoError(t, err) errs := s.ValidatePackage(*pkg) - require.Len(t,errs, 1) + require.Len(t, errs, 1) require.Equal(t, errs[0].Error(), "spec for [testdata/packages/features_beta/beta] defines beta features which can't be enabled for packages with a stable semantic version") -} \ No newline at end of file +} diff --git a/code/go/pkg/validator/validator_test.go b/code/go/pkg/validator/validator_test.go index 8f43c590a..d4bd4ef12 100644 --- a/code/go/pkg/validator/validator_test.go +++ b/code/go/pkg/validator/validator_test.go @@ -12,6 +12,7 @@ import ( "testing" "github.com/elastic/package-spec/code/go/internal/errors" + "github.com/elastic/package-spec/code/go/internal/validator/common" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -28,9 +29,6 @@ func TestValidateFile(t *testing.T) { "deploy_terraform": {}, "time_series": {}, "missing_data_stream": {}, - "custom_logs": {}, - "httpjson_input": {}, - "sql_input": {}, "icons_dark_mode": {}, "bad_additional_content": { "bad-bad", @@ -90,7 +88,6 @@ func TestValidateFile(t *testing.T) { "field services.docker-custom-agent: Must not validate the schema (not)", }, }, - "visualizations_by_reference": {}, } for pkgName, test := range tests { @@ -303,6 +300,58 @@ func TestValidateDuplicatedFields(t *testing.T) { } +func TestValidateWarnings(t *testing.T) { + tests := map[string][]string{ + "good": []string{}, + "custom_logs": []string{ + "package with non-stable semantic version and active beta features (enabled in [../../../../test/packages/custom_logs]) can't be released as stable version.", + }, + "httpjson_input": []string{ + "package with non-stable semantic version and active beta features (enabled in [../../../../test/packages/httpjson_input]) can't be released as stable version.", + }, + "sql_input": []string{ + "package with non-stable semantic version and active beta features (enabled in [../../../../test/packages/sql_input]) can't be released as stable version.", + }, + "visualizations_by_reference": []string{ + "references found in dashboard kibana/dashboard/visualizations_by_reference-82273ffe-6acc-4f2f-bbee-c1004abba63d.json: visualizations_by_reference-5e1a01ff-6f9a-41c1-b7ad-326472db42b6 (visualization), visualizations_by_reference-8287a5d5-1576-4f3a-83c4-444e9058439b (lens)", + }, + } + if err := common.EnableWarningsAsErrors(); err != nil { + require.NoError(t, err) + } + for pkgName, expectedWarnContains := range tests { + t.Run(pkgName, func(t *testing.T) { + warnPrefix := fmt.Sprintf("Warning: ") + + pkgRootPath := filepath.Join("..", "..", "..", "..", "test", "packages", pkgName) + errs := ValidateFromPath(pkgRootPath) + if len(expectedWarnContains) == 0 { + require.NoError(t, errs) + } else { + require.Error(t, errs) + vErrs, ok := errs.(errors.ValidationErrors) + if ok { + require.Len(t, errs, len(expectedWarnContains)) + var warnMessages []string + for _, vErr := range vErrs { + warnMessages = append(warnMessages, vErr.Error()) + } + + for _, expectedWarnMessage := range expectedWarnContains { + expectedWarn := warnPrefix + expectedWarnMessage + require.Contains(t, warnMessages, expectedWarn) + } + return + } + require.Equal(t, errs.Error(), expectedWarnContains[0]) + } + }) + } + if err := common.DisableWarningsAsErrors(); err != nil { + require.NoError(t, err) + } +} + func requireErrorMessage(t *testing.T, pkgName string, invalidItemsPerFolder map[string][]string, expectedErrorMessage string) { pkgRootPath := filepath.Join("..", "..", "..", "..", "test", "packages", pkgName) From 2f389f93fcf10ce26b7fe2a9da60b176eb21dd2a Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Date: Thu, 18 Aug 2022 10:23:56 +0200 Subject: [PATCH 13/17] Add comments and license header --- code/go/internal/validator/common/helpers.go | 9 +++++++++ code/go/internal/validator/common/helpers_test.go | 4 ++++ 2 files changed, 13 insertions(+) diff --git a/code/go/internal/validator/common/helpers.go b/code/go/internal/validator/common/helpers.go index bb9b0dbf2..dcb380cd6 100644 --- a/code/go/internal/validator/common/helpers.go +++ b/code/go/internal/validator/common/helpers.go @@ -1,3 +1,7 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + package common import ( @@ -5,8 +9,11 @@ import ( "strconv" ) +// EnvVarWarningsAsErrors is the environment variable name used to use warnings as errors const EnvVarWarningsAsErrors = "PACKAGE_SPEC_WARNINGS_AS_ERRORS" +// IsDefinedWarningsAsErrors checks whether or not warnings should be considered as errors, +// it checks the environment variable is defined and the value that it contains func IsDefinedWarningsAsErrors() bool { var err error warningsAsErrors := false @@ -20,6 +27,7 @@ func IsDefinedWarningsAsErrors() bool { return warningsAsErrors } +// EnableWarningsAsErrors is a function to enable warnings as errors, setting environment variable as true func EnableWarningsAsErrors() error { if err := os.Setenv(EnvVarWarningsAsErrors, "true"); err != nil { return err @@ -27,6 +35,7 @@ func EnableWarningsAsErrors() error { return nil } +// DisableWarningsAsErrors is a function to disable warnings as errors, unsetting environment variable func DisableWarningsAsErrors() error { if err := os.Unsetenv(EnvVarWarningsAsErrors); err != nil { return err diff --git a/code/go/internal/validator/common/helpers_test.go b/code/go/internal/validator/common/helpers_test.go index 3c5afab86..1fe238cb1 100644 --- a/code/go/internal/validator/common/helpers_test.go +++ b/code/go/internal/validator/common/helpers_test.go @@ -1,3 +1,7 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + package common import ( From 865430647647750bf13fb716de543d978a7bb10e Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Date: Thu, 18 Aug 2022 12:08:51 +0200 Subject: [PATCH 14/17] Add comment about removal depending on structured errors --- code/go/internal/validator/common/helpers.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/code/go/internal/validator/common/helpers.go b/code/go/internal/validator/common/helpers.go index dcb380cd6..420bc3499 100644 --- a/code/go/internal/validator/common/helpers.go +++ b/code/go/internal/validator/common/helpers.go @@ -9,7 +9,8 @@ import ( "strconv" ) -// EnvVarWarningsAsErrors is the environment variable name used to use warnings as errors +// EnvVarWarningsAsErrors is the environment variable name used to enable warnings as errors +// this meachinsm will be removed once structured errors are supported https://github.com/elastic/package-spec/issues/342 const EnvVarWarningsAsErrors = "PACKAGE_SPEC_WARNINGS_AS_ERRORS" // IsDefinedWarningsAsErrors checks whether or not warnings should be considered as errors, From e8a9c56e3f64e5223cc43ac2dae7f25f17323307 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Date: Thu, 18 Aug 2022 12:09:33 +0200 Subject: [PATCH 15/17] reorder imports --- code/go/internal/validator/spec_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/code/go/internal/validator/spec_test.go b/code/go/internal/validator/spec_test.go index 6d4f4f81d..a5e36df7e 100644 --- a/code/go/internal/validator/spec_test.go +++ b/code/go/internal/validator/spec_test.go @@ -7,10 +7,10 @@ package validator import ( "testing" - "github.com/elastic/package-spec/code/go/internal/fspath" - "github.com/Masterminds/semver/v3" "github.com/stretchr/testify/require" + + "github.com/elastic/package-spec/code/go/internal/fspath" ) func TestNewSpec(t *testing.T) { From d78a905fd84609be8e6a5a3948aec8848c010189 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Date: Thu, 18 Aug 2022 12:17:08 +0200 Subject: [PATCH 16/17] Use defer to disable warnings as errors in test --- code/go/pkg/validator/validator_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/code/go/pkg/validator/validator_test.go b/code/go/pkg/validator/validator_test.go index d4bd4ef12..963029bab 100644 --- a/code/go/pkg/validator/validator_test.go +++ b/code/go/pkg/validator/validator_test.go @@ -319,6 +319,8 @@ func TestValidateWarnings(t *testing.T) { if err := common.EnableWarningsAsErrors(); err != nil { require.NoError(t, err) } + defer common.DisableWarningsAsErrors() + for pkgName, expectedWarnContains := range tests { t.Run(pkgName, func(t *testing.T) { warnPrefix := fmt.Sprintf("Warning: ") @@ -347,9 +349,6 @@ func TestValidateWarnings(t *testing.T) { } }) } - if err := common.DisableWarningsAsErrors(); err != nil { - require.NoError(t, err) - } } func requireErrorMessage(t *testing.T, pkgName string, invalidItemsPerFolder map[string][]string, expectedErrorMessage string) { From 98863868636214718cf61b3fb2023a3205b0f74f Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Date: Thu, 18 Aug 2022 13:25:43 +0200 Subject: [PATCH 17/17] Use path instead of filepath for test --- code/go/pkg/validator/validator_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/code/go/pkg/validator/validator_test.go b/code/go/pkg/validator/validator_test.go index 963029bab..0d7af20e1 100644 --- a/code/go/pkg/validator/validator_test.go +++ b/code/go/pkg/validator/validator_test.go @@ -325,7 +325,7 @@ func TestValidateWarnings(t *testing.T) { t.Run(pkgName, func(t *testing.T) { warnPrefix := fmt.Sprintf("Warning: ") - pkgRootPath := filepath.Join("..", "..", "..", "..", "test", "packages", pkgName) + pkgRootPath := path.Join("..", "..", "..", "..", "test", "packages", pkgName) errs := ValidateFromPath(pkgRootPath) if len(expectedWarnContains) == 0 { require.NoError(t, errs)