From ba83571b0b0256441678c227c02e5fa60a0028fa Mon Sep 17 00:00:00 2001 From: Evsyukov Denis Date: Thu, 12 Sep 2024 09:58:06 +0300 Subject: [PATCH] wip --- go.mod | 3 - pkg/linters/openapi/library.go | 88 ++++--------------- pkg/linters/openapi/openapi.go | 84 +++++------------- pkg/linters/openapi/openapi_testdata/crd.yaml | 32 ------- .../openapi/openapi_testdata/values.yaml | 17 ---- pkg/linters/openapi/validators/enum.go | 2 +- .../openapi/validators/ha_and_https.go | 2 +- .../openapi/validators/keys_name_check.go | 2 +- 8 files changed, 45 insertions(+), 185 deletions(-) delete mode 100644 pkg/linters/openapi/openapi_testdata/crd.yaml delete mode 100644 pkg/linters/openapi/openapi_testdata/values.yaml diff --git a/go.mod b/go.mod index f973727..4cc4bc6 100644 --- a/go.mod +++ b/go.mod @@ -9,15 +9,12 @@ require ( github.com/mitchellh/mapstructure v1.5.0 github.com/spf13/pflag v1.0.5 github.com/spf13/viper v1.19.0 - github.com/stretchr/testify v1.9.0 helm.sh/helm/v3 v3.15.4 ) require ( - github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/hashicorp/errwrap v1.1.0 // indirect github.com/pkg/errors v0.9.1 // indirect - github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect sigs.k8s.io/yaml v1.4.0 // indirect ) diff --git a/pkg/linters/openapi/library.go b/pkg/linters/openapi/library.go index 779e175..47dbde8 100644 --- a/pkg/linters/openapi/library.go +++ b/pkg/linters/openapi/library.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/deckhouse/d8-lint/pkg/linters/openapi/validators" + "github.com/hashicorp/go-multierror" "gopkg.in/yaml.v3" @@ -21,61 +22,9 @@ const ( type fileValidation struct { filePath string - validationError error -} - -type moduleVersions struct { - specVersion int - conversionsVersion int -} - -func modulesVersions(rootPath string) (map[string]*moduleVersions, error) { - result := map[string]*moduleVersions{} - - globPattern := filepath.Join(rootPath, "*", "openapi", "*.yaml") - matches, err := filepath.Glob(globPattern) - if err != nil { - return nil, err - } - - for _, path := range matches { - module := filepath.Dir(filepath.Dir(path)) - module = filepath.Base(module) - if module == "" { - continue - } - - if strings.HasSuffix(path, "config-values.yaml") { - config := getFileYAMLContent(path) - if val, ok := config["x-config-version"]; ok && val.(int) > 1 { - if mv, ok := result[module]; ok { - mv.specVersion = val.(int) - } else { - result[module] = &moduleVersions{specVersion: val.(int)} - } - } - } else if strings.Contains(path, "/conversions/") { - data, err := os.ReadFile(path) - if err != nil { - return nil, err - } - var parsed struct { - Version int - } - if err := yaml.Unmarshal(data, &parsed); err != nil { - return nil, err - } - if mv, ok := result[module]; ok { - if parsed.Version > mv.conversionsVersion { - mv.conversionsVersion = parsed.Version - } - } else { - result[module] = &moduleVersions{conversionsVersion: parsed.Version} - } - } - } + rootPath string - return result, nil + validationError error } // GetOpenAPIYAMLFiles returns all .yaml files which are placed into openapi/ | crds/ directory @@ -125,7 +74,6 @@ func GetOpenAPIYAMLFiles(rootPath string) ([]string, error) { } // RunOpenAPIValidator runs validator, get channel with file paths and returns channel with results -// nolint: revive // its a private lib, we dont need an exported struct func RunOpenAPIValidator(fileC chan fileValidation) chan fileValidation { resultC := make(chan fileValidation, 1) @@ -135,7 +83,7 @@ func RunOpenAPIValidator(fileC chan fileValidation) chan fileValidation { yamlStruct := getFileYAMLContent(vfile.filePath) - runFileParser(strings.TrimPrefix(vfile.filePath, deckhousePath), yamlStruct, parseResultC) + runFileParser(strings.TrimPrefix(vfile.filePath, vfile.rootPath), yamlStruct, parseResultC) var result *multierror.Error @@ -168,13 +116,13 @@ type fileParser struct { resultC chan error } -func getFileYAMLContent(path string) map[interface{}]interface{} { +func getFileYAMLContent(path string) map[any]any { data, err := os.ReadFile(path) if err != nil { panic(err) } - m := make(map[interface{}]interface{}) + m := make(map[any]any) err = yaml.Unmarshal(data, &m) if err != nil { @@ -184,7 +132,7 @@ func getFileYAMLContent(path string) map[interface{}]interface{} { return m } -func isCRD(data map[interface{}]interface{}) bool { +func isCRD(data map[any]any) bool { kind, ok := data["kind"].(string) if !ok { return false @@ -197,7 +145,7 @@ func isCRD(data map[interface{}]interface{}) bool { return true } -func isDeckhouseCRD(data map[interface{}]interface{}) bool { +func isDeckhouseCRD(data map[any]any) bool { kind, ok := data["kind"].(string) if !ok { return false @@ -207,7 +155,7 @@ func isDeckhouseCRD(data map[interface{}]interface{}) bool { return false } - metadata, ok := data["metadata"].(map[interface{}]interface{}) + metadata, ok := data["metadata"].(map[any]any) if !ok { return false } @@ -224,7 +172,7 @@ func isDeckhouseCRD(data map[interface{}]interface{}) bool { return false } -func (fp fileParser) parseForWrongKeys(m map[interface{}]interface{}) { +func (fp fileParser) parseForWrongKeys(m map[any]any) { keysValidator := validators.NewKeyNameValidator() err := keysValidator.Run(fp.fileName, "allfile", m) if err != nil { @@ -232,7 +180,7 @@ func (fp fileParser) parseForWrongKeys(m map[interface{}]interface{}) { } } -func runFileParser(fileName string, data map[interface{}]interface{}, resultC chan error) { +func runFileParser(fileName string, data map[any]any, resultC chan error) { // exclude external CRDs if isCRD(data) && !isDeckhouseCRD(data) { close(resultC) @@ -254,7 +202,7 @@ func runFileParser(fileName string, data map[interface{}]interface{}, resultC ch go parser.startParsing(data, resultC) } -func (fp fileParser) startParsing(m map[interface{}]interface{}, resultC chan error) { +func (fp fileParser) startParsing(m map[any]any, resultC chan error) { for k, v := range m { fp.parseValue(k.(string), v) } @@ -262,7 +210,7 @@ func (fp fileParser) startParsing(m map[interface{}]interface{}, resultC chan er close(resultC) } -func (fp fileParser) parseMap(upperKey string, m map[interface{}]interface{}) { +func (fp fileParser) parseMap(upperKey string, m map[any]any) { for k, v := range m { absKey := fmt.Sprintf("%s.%s", upperKey, k) if key, ok := k.(string); ok { @@ -277,13 +225,13 @@ func (fp fileParser) parseMap(upperKey string, m map[interface{}]interface{}) { } } -func (fp fileParser) parseSlice(upperKey string, slc []interface{}) { +func (fp fileParser) parseSlice(upperKey string, slc []any) { for k, v := range slc { fp.parseValue(fmt.Sprintf("%s[%d]", upperKey, k), v) } } -func (fp fileParser) parseValue(upperKey string, v interface{}) { +func (fp fileParser) parseValue(upperKey string, v any) { if v == nil { return } @@ -291,12 +239,12 @@ func (fp fileParser) parseValue(upperKey string, v interface{}) { switch typ { case reflect.Map: - fp.parseMap(upperKey, v.(map[interface{}]interface{})) + fp.parseMap(upperKey, v.(map[any]any)) case reflect.Slice: - fp.parseSlice(upperKey, v.([]interface{})) + fp.parseSlice(upperKey, v.([]any)) } } type validator interface { - Run(fileName, absoulteKey string, value interface{}) error + Run(fileName, absoulteKey string, value any) error } diff --git a/pkg/linters/openapi/openapi.go b/pkg/linters/openapi/openapi.go index bc82593..9fa58fa 100644 --- a/pkg/linters/openapi/openapi.go +++ b/pkg/linters/openapi/openapi.go @@ -1,31 +1,21 @@ package openapi import ( - "fmt" + "context" "strings" - "testing" - "github.com/hashicorp/go-multierror" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" + "github.com/deckhouse/d8-lint/pkg/errors" + "github.com/deckhouse/d8-lint/pkg/manager" ) -func ValidateOpenAPI(path string) error { - apiFiles, err := GetOpenAPIYAMLFiles(path) - if err != nil { - return err - } - - resultC := RunOpenAPIValidator(path, apiFiles) +// OpenAPI linter +type OpenAPI struct{} - for result := range resultC { - assert.NoError(t, result.validationError, "File '%s' has invalid spec", strings.TrimPrefix(result.filePath, deckhousePath)) +func (*OpenAPI) Run(_ context.Context, m manager.Module) (errors.LintRuleErrorsList, error) { + apiFiles, err := GetOpenAPIYAMLFiles(m.Path) + if err != nil { + return errors.LintRuleErrorsList{}, err } -} - -// TestValidators test that validation hooks are working -func TestValidators(t *testing.T) { - apiFiles := []string{deckhousePath + "testing/openapi_validation/openapi_testdata/values.yaml"} filesC := make(chan fileValidation, len(apiFiles)) resultC := RunOpenAPIValidator(filesC) @@ -33,56 +23,30 @@ func TestValidators(t *testing.T) { for _, apiFile := range apiFiles { filesC <- fileValidation{ filePath: apiFile, + rootPath: m.Path, } } close(filesC) + var result errors.LintRuleErrorsList for res := range resultC { - assert.Error(t, res.validationError) - err, ok := res.validationError.(*multierror.Error) - require.True(t, ok) - require.Len(t, err.Errors, 6) - - // we can't guarantee order here, thats why test contains - assert.Contains(t, res.validationError.Error(), "properties.https is invalid: must have no default value") - assert.Contains(t, res.validationError.Error(), "Enum 'properties.https.properties.mode.enum' is invalid: value 'disabled' must start with Capital letter") - assert.Contains(t, res.validationError.Error(), "Enum 'properties.https.properties.mode.enum' is invalid: value: 'Cert-Manager' must be in CamelCase") - assert.Contains(t, res.validationError.Error(), "Enum 'properties.https.properties.mode.enum' is invalid: value: 'Some:Thing' must be in CamelCase") - assert.Contains(t, res.validationError.Error(), "Enum 'properties.https.properties.mode.enum' is invalid: value: 'Any.Thing' must be in CamelCase") - assert.Contains(t, res.validationError.Error(), "properties.highAvailability is invalid: must have no default value") - } -} - -func TestCRDValidators(t *testing.T) { - apiFiles := []string{deckhousePath + "testing/openapi_validation/openapi_testdata/crd.yaml"} - - filesC := make(chan fileValidation, len(apiFiles)) - resultC := RunOpenAPIValidator(filesC) - - for _, apiFile := range apiFiles { - filesC <- fileValidation{ - filePath: apiFile, + if res.validationError != nil { + result.Add(errors.LintRuleError{ + Text: res.validationError.Error(), + ID: "openapi", + ObjectID: strings.TrimPrefix(res.filePath, m.Path), + Value: res.validationError, + }) } } - close(filesC) - for res := range resultC { - assert.Error(t, res.validationError) - err, ok := res.validationError.(*multierror.Error) - require.True(t, ok) - require.Len(t, err.Errors, 1) + return errors.LintRuleErrorsList{}, nil +} - // we can't guarantee order here, thats why test contains - assert.Contains(t, res.validationError.Error(), "file validation error: wrong property") - } +func (*OpenAPI) Name() string { + return "OpenAPI Linter" } -func TestModulesVersionsValidation(t *testing.T) { - mv, err := modulesVersions(deckhousePath) - require.NoError(t, err) - for m, v := range mv { - message := fmt.Sprintf("conversions version(%d) and spec version(%d) for module %s are not equal", - v.conversionsVersion, v.specVersion, m) - assert.Equal(t, true, v.conversionsVersion == v.specVersion, message) - } +func (*OpenAPI) Desc() string { + return "OpenAPI will check all openapi files in the module" } diff --git a/pkg/linters/openapi/openapi_testdata/crd.yaml b/pkg/linters/openapi/openapi_testdata/crd.yaml deleted file mode 100644 index 3aff7bc..0000000 --- a/pkg/linters/openapi/openapi_testdata/crd.yaml +++ /dev/null @@ -1,32 +0,0 @@ -apiVersion: apiextensions.k8s.io/v1 -kind: CustomResourceDefinition -metadata: - name: testcrds.deckhouse.io - labels: - heritage: deckhouse -spec: - group: deckhouse.io - scope: Cluster - names: - plural: testcrds - singular: testcrd - kind: TestCrd - preserveUnknownFields: false - versions: - - name: v1alpha1 - served: true - storage: true - schema: - openAPIV3Schema: - type: object - description: 'Test CRD' - required: - - spec - properties: - spec: - type: object - properties: - examples: - type: string - description: a - x-description: a diff --git a/pkg/linters/openapi/openapi_testdata/values.yaml b/pkg/linters/openapi/openapi_testdata/values.yaml deleted file mode 100644 index 56a9258..0000000 --- a/pkg/linters/openapi/openapi_testdata/values.yaml +++ /dev/null @@ -1,17 +0,0 @@ -properties: - https: - type: object - default: {"foo": "bar"} - properties: - mode: - type: string - default: "Disabled" - enum: - - "disabled" - - "Cert-Manager" - - "Some:Thing" - - "Any.Thing" - - "Static" - highAvailability: - type: boolean - default: false diff --git a/pkg/linters/openapi/validators/enum.go b/pkg/linters/openapi/validators/enum.go index 4ad9221..cfba434 100644 --- a/pkg/linters/openapi/validators/enum.go +++ b/pkg/linters/openapi/validators/enum.go @@ -255,7 +255,7 @@ func (en EnumValidator) validateEnumValues(enumKey string, values []string) *mul return res } -func (en EnumValidator) validateEnumValue(value string) error { +func (EnumValidator) validateEnumValue(value string) error { if value == "" { return nil } diff --git a/pkg/linters/openapi/validators/ha_and_https.go b/pkg/linters/openapi/validators/ha_and_https.go index 26e3912..991fb05 100644 --- a/pkg/linters/openapi/validators/ha_and_https.go +++ b/pkg/linters/openapi/validators/ha_and_https.go @@ -19,7 +19,7 @@ func NewHAValidator() HAValidator { return HAValidator{} } -func (en HAValidator) Run(file, absoluteKey string, value any) error { +func (HAValidator) Run(file, absoluteKey string, value any) error { values, ok := value.(map[any]any) if !ok { fmt.Printf("Possible Bug? Have to be a map. Type: %s, Value: %s, File: %s, Key: %s\n", reflect.TypeOf(value), value, file, absoluteKey) diff --git a/pkg/linters/openapi/validators/keys_name_check.go b/pkg/linters/openapi/validators/keys_name_check.go index bcb0df0..09fcf84 100644 --- a/pkg/linters/openapi/validators/keys_name_check.go +++ b/pkg/linters/openapi/validators/keys_name_check.go @@ -52,7 +52,7 @@ func checkMapForBannedKey(m map[any]any, banned []string) error { return nil } -func (knv KeyNameValidator) Run(file, _ string, value any) error { +func (KeyNameValidator) Run(file, _ string, value any) error { object, ok := value.(map[any]any) if !ok { fmt.Println("Possible Bug? Have to be a map", reflect.TypeOf(value))