From b6b17457f5edc47bf2e874aa3788fd6e546b8d8a Mon Sep 17 00:00:00 2001 From: Mikhail Scherba <41360396+miklezzzz@users.noreply.github.com> Date: Thu, 4 Apr 2024 12:41:35 +0400 Subject: [PATCH] add release requirements validation (#7869) Signed-off-by: Mikhail Scherba --- testing/validate_release_requirements.sh | 21 + tools/validation/main.go | 2 + tools/validation/release_requirements.go | 387 ++++++++++++++++++ tools/validation/release_requirements_test.go | 101 +++++ tools/validation/run_env | 2 +- .../release_requirements/correct/example.go | 35 ++ .../release_requirements/faulty/example.go | 31 ++ .../faulty/extra-check.go | 35 ++ .../faulty/function-assignment.go | 39 ++ .../release_requirements/release.yaml | 39 ++ 10 files changed, 691 insertions(+), 1 deletion(-) create mode 100755 testing/validate_release_requirements.sh create mode 100644 tools/validation/release_requirements.go create mode 100644 tools/validation/release_requirements_test.go create mode 100644 tools/validation/test_data/release_requirements/correct/example.go create mode 100644 tools/validation/test_data/release_requirements/faulty/example.go create mode 100644 tools/validation/test_data/release_requirements/faulty/extra-check.go create mode 100644 tools/validation/test_data/release_requirements/faulty/function-assignment.go create mode 100644 tools/validation/test_data/release_requirements/release.yaml diff --git a/testing/validate_release_requirements.sh b/testing/validate_release_requirements.sh new file mode 100755 index 0000000000..0d93d93e8d --- /dev/null +++ b/testing/validate_release_requirements.sh @@ -0,0 +1,21 @@ +#!/bin/bash + +# Copyright 2024 Flant JSC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +source ./tools/validation/run_env + +validation_go_run ./tools/validation \ + --type release-requirements \ + --file "$1" diff --git a/tools/validation/main.go b/tools/validation/main.go index 98726af4db..f737a74843 100644 --- a/tools/validation/main.go +++ b/tools/validation/main.go @@ -64,6 +64,8 @@ func main() { exitCode = RunDocChangesValidation(diffInfo) case "grafana-dashboard": exitCode = RunGrafanaDashboardValidation(diffInfo) + case "release-requirements": + exitCode = RunReleaseRequirementsValidation(diffInfo) case "dump": fmt.Printf("%s\n", diffInfo.Dump()) default: diff --git a/tools/validation/release_requirements.go b/tools/validation/release_requirements.go new file mode 100644 index 0000000000..0aaf853c7a --- /dev/null +++ b/tools/validation/release_requirements.go @@ -0,0 +1,387 @@ +/* +Copyright 2024 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "fmt" + "os" + "path/filepath" + "regexp" + "slices" + "strings" + + "go/ast" + "go/parser" + "go/token" + + "gopkg.in/yaml.v2" +) + +const ( + releaseFile = "release.yaml" + checkFunctionName = "RegisterCheck" + generalDecls = "general" + requirementsPackage = "requirements" +) + +var ( + moduleRequirementsRegex = regexp.MustCompile(`^(ee\/)?(be\/|fe\/)?modules\/.+\/requirements\/.+\.go$`) + modulesDirs = []string{"./modules", "./ee"} +) + +type releaseSettings struct { + Requirements map[string]string `yaml:"requirements"` +} + +func RunReleaseRequirementsValidation(info *DiffInfo) (exitCode int) { + fmt.Printf("Run 'release requirements' validation ...\n") + + exitCode = 0 + + fmt.Printf("Check new and updated lines ... ") + if len(info.Files) == 0 { + fmt.Printf("OK, diff is empty\n") + } else { + fmt.Println("") + + msgs := NewMessages() + + var ( + allRequirements map[string]struct{} + newRequirements map[string]struct{} + err error + ) + + // Checking for changes in release.yaml + for _, fileInfo := range info.Files { + if fileInfo.NewFileName == releaseFile && (fileInfo.IsAdded() || fileInfo.IsModified()) && len(fileInfo.NewLines()) != 0 { + fmt.Println("Gettings new requirements") + // Get all current requirements from release.yaml + allRequirements, newRequirements, err = getRequirements(fileInfo.NewLines(), releaseFile) + if err != nil { + fmt.Printf("Couldn't get the list of requirements to check: %s\n", err) + return 1 + } + break + } + } + + if len(newRequirements) == 0 { + fmt.Println("No new requirements were introduced") + // there were no changes in the release.yaml file but we still need all release requirements + if len(allRequirements) == 0 { + allRequirements, _, err = getRequirements([]string{}, releaseFile) + if err != nil { + fmt.Printf("Couldn't get the list of requirements to check: %s\n", err) + return 1 + } + } + } else { + fmt.Print("New requirements found: ") + for k, _ := range newRequirements { + fmt.Printf("%s ", k) + } + fmt.Println("") + + // Checking for changes in other */requirements/*.go files + for _, fileInfo := range info.Files { + if !fileInfo.HasContent() { + continue + } + // Check only added or modified files + if !(fileInfo.IsAdded() || fileInfo.IsModified()) { + continue + } + + fileName := fileInfo.NewFileName + + // Skip files unrelated to requirements + if !moduleRequirementsRegex.MatchString(fileName) { + continue + } + + // Skip tests + if strings.HasSuffix(fileName, "_test.go") { + continue + } + + // Get added or modified lines + newLines := fileInfo.NewLines() + if len(newLines) == 0 { + msgs.Add(NewSkip(fileName, "no lines added")) + continue + } + + // Check if new requirements and checks are introduced in a single PR + fmt.Println("Checking file: ", fileInfo.NewFileName) + prematureChecks, _, err := checksAndRequirements(newRequirements, fileName, requirementsPackage) + if err != nil { + msgs.Add(NewError(fileName, "coudn't linter due to some errors", err.Error())) + continue + } + + if len(prematureChecks) > 0 { + msgs.Add(NewError(fileName, "should not check release requirements introduced in the same PR", strings.Join(prematureChecks, ", "))) + continue + } + + msgs.Add(NewOK(fileName)) + } + } + + fmt.Println("Inspecting if there are any orphaned requirements (not matching any check) in the release.yaml file") + + allChecks, err := getAllChecks(modulesDirs) + if err != nil { + fmt.Printf("Couldn't inspect modules' checks: %s\n", err) + return 1 + } + fmt.Println("Following checks have been found:", allChecks) + + for _, check := range allChecks { + delete(allRequirements, check) + } + + if len(allRequirements) > 0 { + output := []string{} + for requirement, _ := range allRequirements { + output = append(output, requirement) + } + msgs.Add(NewError("release.yaml", "found requirements for non-existent module checks, please review release requirements in release.yaml", strings.Join(output, ", "))) + } + + msgs.PrintReport() + + if msgs.CountErrors() > 0 { + exitCode = 1 + } + } + + return exitCode +} + +// checksAndRequirements verifies if there are premature checks (checks that were introduced with the related requirements simultaneously). +// it also checks if there are requirements for non-existent checks (orphaned requirements) in the release.yaml file +func checksAndRequirements(newRequirements map[string]struct{}, fileName, packageName string) ( /* premature checks */ []string /* eligible checks */, []string, error) { + var ( + prematureChecks []string + eligibleChecks []string + stack []ast.Node + errMsgs []string + ) + decls := make(map[string]map[string]string) + + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, fileName, nil, 0) + if err != nil { + return nil, nil, err + } + + if file.Name.Name != packageName { + return nil, nil, nil + } + + // node is either an assignment or a definition + ast.Inspect(file, func(n ast.Node) bool { + switch ntype := n.(type) { + case *ast.AssignStmt: + if ntype.Tok == token.ASSIGN || ntype.Tok == token.DEFINE { + loop: + for i, lh := range ntype.Lhs { + var key, value string + if id, ok := lh.(*ast.Ident); ok { + key = id.Name + } + + switch v := ntype.Rhs[i].(type) { + case *ast.BasicLit: + value = v.Value + + case *ast.Ident: + var ok bool + value, ok = decls[findParentFunction(stack)][v.Name] + if !ok { + value, ok = decls[generalDecls][v.Name] + } + + default: + break loop + } + + if len(key) != 0 && len(value) != 0 { + parentNode := findParentFunction(stack) + if decls[parentNode] == nil { + decls[parentNode] = make(map[string]string) + } + decls[parentNode][key] = strings.Trim(value, "\"") + } + } + } + // node is part of general declarations + case *ast.GenDecl: + if ntype.Tok == token.CONST || ntype.Tok == token.VAR { + for _, cDecl := range ntype.Specs { + if vSpec, ok := cDecl.(*ast.ValueSpec); ok { + for i := 0; i < len(vSpec.Names); i++ { + switch v := vSpec.Values[i].(type) { + case *ast.BasicLit: + parentNode := findParentFunction(stack) + if decls[parentNode] == nil { + decls[parentNode] = make(map[string]string) + } + decls[parentNode][vSpec.Names[i].Name] = strings.Trim(v.Value, "\"") + } + } + } + } + } + + // node is a function call + case *ast.CallExpr: + if fun, ok := ntype.Fun.(*ast.SelectorExpr); ok { + // function name is what we are looking for + if fun.Sel.Name == checkFunctionName { + switch x := ntype.Args[0].(type) { + // the function's argument is a string + case *ast.BasicLit: + val := strings.Trim(x.Value, "\"") + // check for a premature check + if _, found := newRequirements[val]; found { + prematureChecks = append(prematureChecks, val) + } else { + eligibleChecks = append(eligibleChecks, val) + } + + // the function's argument is a variable + case *ast.Ident: + val, ok := decls[findParentFunction(stack)][x.Name] + if !ok { + val, ok = decls[generalDecls][x.Name] + } + + if ok { + // check for a premature check + if _, found := newRequirements[val]; found { + prematureChecks = append(prematureChecks, val) + } else { + eligibleChecks = append(eligibleChecks, val) + } + } else { + errMsgs = append(errMsgs, fmt.Sprintf("Couldn't find declaration of the '%s' variable", x.Name)) + } + } + } + } + } + + if n == nil { + stack = stack[:len(stack)-1] + } else { + stack = append(stack, n) + } + + return true + }) + + if len(errMsgs) > 0 { + err = fmt.Errorf(strings.Join(errMsgs, ", ")) + } + + return prematureChecks, eligibleChecks, err +} + +// Traverses through a "stack" of ast.Nodes to find which function current context belongs to +func findParentFunction(stack []ast.Node) string { + for i := len(stack) - 1; i >= 0; i-- { + fn, ok := stack[i].(*ast.FuncDecl) + if ok { + return fn.Name.Name + } + } + return generalDecls +} + +// Forms two maps of release requirements, all and new ones (that were indroduced by current PR) +func getRequirements(newlines []string, releaseFile string) ( /* all requirements */ map[string]struct{} /*new requirements*/, map[string]struct{}, error) { + fileContent, err := os.ReadFile(releaseFile) + if err != nil { + return nil, nil, err + } + + var releaseSettings releaseSettings + + err = yaml.Unmarshal(fileContent, &releaseSettings) + if err != nil { + return nil, nil, err + } + + allRequirements := make(map[string]struct{}) + newRequirements := make(map[string]struct{}) + + for requirement, _ := range releaseSettings.Requirements { + allRequirements[requirement] = struct{}{} + requirementRegex := regexp.MustCompile(fmt.Sprintf("^ \"%s\":", requirement)) + for _, line := range newlines { + if requirementRegex.MatchString(line) { + newRequirements[requirement] = struct{}{} + break + } + } + } + + return allRequirements, newRequirements, nil +} + +// Walks over */requirements/*.go modules' files to inspect if some release requirements have no related checks +func getAllChecks(roots []string) ([]string, error) { + allChecks := make([]string, 0) + + for _, root := range roots { + + err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + + if info.IsDir() { + return nil + } + + if filepath.Base(filepath.Dir(path)) == "requirements" && !strings.HasSuffix(info.Name(), "_test.go") { + _, checks, err := checksAndRequirements(map[string]struct{}{}, path, requirementsPackage) + if err != nil { + return err + } + + if len(checks) > 0 { + fmt.Println("Collecting checks from ", path) + allChecks = append(allChecks, checks...) + } + } + + return nil + }) + + if err != nil { + return nil, err + } + } + + slices.Sort(allChecks) + + return slices.Compact(allChecks), nil +} diff --git a/tools/validation/release_requirements_test.go b/tools/validation/release_requirements_test.go new file mode 100644 index 0000000000..e8d5891264 --- /dev/null +++ b/tools/validation/release_requirements_test.go @@ -0,0 +1,101 @@ +/* +Copyright 2024 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "fmt" + "testing" +) + +var testRequirementsPackage = "test" + +func Test_areNewRequirementsChecked(t *testing.T) { + _, _, err := checksAndRequirements(map[string]struct{}{}, "test_data/release_requirements/correct/example.go", testRequirementsPackage) + if err != nil { + t.Errorf("Should parse correct/example.go file successfully: %s", err) + } + + _, _, err = checksAndRequirements(map[string]struct{}{}, "test_data/release_requirements/faulty/function-assignment.go", testRequirementsPackage) + if err == nil { + t.Errorf("Should fail to parse faulty/function-assignment.go file: %s", err) + } + + prematureChecks, _, err := checksAndRequirements(map[string]struct{}{"testVer": struct{}{}}, "test_data/release_requirements/faulty/example.go", testRequirementsPackage) + if err != nil { + t.Errorf("Should parse faulty/example.go file successfully") + } + + if len(prematureChecks) == 0 { + t.Errorf("List of premature checks shouldn't be of 0 length") + } + + prematureChecks, eligibleChecks, err := checksAndRequirements(map[string]struct{}{"testVer": struct{}{}}, "test_data/release_requirements/faulty/extra-check.go", testRequirementsPackage) + if err != nil { + t.Errorf("Should parse faulty/extra-check.go file successfully") + } + + if len(prematureChecks) == 0 { + t.Errorf("List of premature checks shouldn't be of 0 length") + } + + if len(eligibleChecks) == 0 { + t.Errorf("List of eligible checks shouldn't be of 0 length") + } +} + +func Test_getRequirements(t *testing.T) { + lines := []string{ + ` "testVer": "1.16" # modules/110-istio/requirements/check.go`, + } + expect := map[string]struct{}{"testVer": struct{}{}} + + _, requirements, err := getRequirements(lines, "test_data/release_requirements/release.yaml") + if err != nil { + t.Errorf("Should get requirements without an error: %s", err) + } + + if fmt.Sprint(requirements) != fmt.Sprint(expect) { + t.Errorf("Expect '%s', got '%s'", expect, requirements) + } + + lines = []string{ + ` "testVer": "1.16" # modules/110-istio/requirements/check.go`, + ` "istioVer": "1.9"`, + } + expect = map[string]struct{}{"testVer": struct{}{}, "istioVer": struct{}{}} + + _, requirements, err = getRequirements(lines, "test_data/release_requirements/release.yaml") + if err != nil { + t.Errorf("Should get requirements without an error: %s", err) + } + + if fmt.Sprint(requirements) != fmt.Sprint(expect) { + t.Errorf("Expect '%s', got '%s'", expect, requirements) + } + + lines = []string{} + expect = map[string]struct{}{} + + _, requirements, err = getRequirements(lines, "test_data/release_requirements/release.yaml") + if err != nil { + t.Errorf("Should get requirements without an error: %s", err) + } + + if fmt.Sprint(requirements) != fmt.Sprint(expect) { + t.Errorf("Expect '%s', got '%s'", expect, requirements) + } +} diff --git a/tools/validation/run_env b/tools/validation/run_env index 075e578e43..24f1538b1b 100644 --- a/tools/validation/run_env +++ b/tools/validation/run_env @@ -25,5 +25,5 @@ function validation_go_run() { validationDir="$1" shift - go run ${validationDir}/{main,messages,diff,copyright,no_cyrillic,doc_changes,grafana_dashboard}.go "$@" + go run ${validationDir}/{main,messages,diff,copyright,no_cyrillic,doc_changes,grafana_dashboard,release_requirements}.go "$@" } diff --git a/tools/validation/test_data/release_requirements/correct/example.go b/tools/validation/test_data/release_requirements/correct/example.go new file mode 100644 index 0000000000..6800b1225e --- /dev/null +++ b/tools/validation/test_data/release_requirements/correct/example.go @@ -0,0 +1,35 @@ +/* +Copyright 2024 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package test + +import ( + "github.com/deckhouse/deckhouse/go_lib/dependency/requirements" +) + +var ( + someValue = "testVer" +) + +func main() { + test() +} + +func test() { + requirements.RegisterCheck(someValue, func(requirementValue string, getter requirements.ValueGetter) (bool, error) { + return false, nil + }) +} diff --git a/tools/validation/test_data/release_requirements/faulty/example.go b/tools/validation/test_data/release_requirements/faulty/example.go new file mode 100644 index 0000000000..520c2b9e54 --- /dev/null +++ b/tools/validation/test_data/release_requirements/faulty/example.go @@ -0,0 +1,31 @@ +/* +Copyright 2024 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package test + +import ( + "github.com/deckhouse/deckhouse/go_lib/dependency/requirements" +) + +var ( + someCheck = "testVer" +) + +func test3() { + requirements.RegisterCheck("testVer", func(requirementValue string, getter requirements.ValueGetter) (bool, error) { + return false, nil + }) +} diff --git a/tools/validation/test_data/release_requirements/faulty/extra-check.go b/tools/validation/test_data/release_requirements/faulty/extra-check.go new file mode 100644 index 0000000000..79b5e34e2e --- /dev/null +++ b/tools/validation/test_data/release_requirements/faulty/extra-check.go @@ -0,0 +1,35 @@ +/* +Copyright 2024 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package test + +import ( + "github.com/deckhouse/deckhouse/go_lib/dependency/requirements" +) + +var ( + whatDidYouSay = "what?" + yetAnotherCheck = "testVer" +) + +func test4() { + requirements.RegisterCheck(whatDidYouSay, func(requirementValue string, getter requirements.ValueGetter) (bool, error) { + return false, nil + }) + requirements.RegisterCheck(yetAnotherCheck, func(requirementValue string, getter requirements.ValueGetter) (bool, error) { + return false, nil + }) +} diff --git a/tools/validation/test_data/release_requirements/faulty/function-assignment.go b/tools/validation/test_data/release_requirements/faulty/function-assignment.go new file mode 100644 index 0000000000..9671c0c132 --- /dev/null +++ b/tools/validation/test_data/release_requirements/faulty/function-assignment.go @@ -0,0 +1,39 @@ +/* +Copyright 2024 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package test + +import ( + "github.com/deckhouse/deckhouse/go_lib/dependency/requirements" +) + +var ( + testValue = "testVer" +) + +func testFunction() { + value, err := exampleFunction(testValue) + if err != nil { + return + } + requirements.RegisterCheck(value, func(requirementValue string, getter requirements.ValueGetter) (bool, error) { + return false, nil + }) +} + +func exampleFunction(str string) (string, error) { + return str, nil +} diff --git a/tools/validation/test_data/release_requirements/release.yaml b/tools/validation/test_data/release_requirements/release.yaml new file mode 100644 index 0000000000..19c7dc0f36 --- /dev/null +++ b/tools/validation/test_data/release_requirements/release.yaml @@ -0,0 +1,39 @@ +# Channel canary settings +"canary": + "alpha": + enabled: true + waves: 2 + interval: "5m" + "beta": + enabled: false + waves: 1 + interval: "1m" + "early-access": + enabled: true + waves: 6 + interval: "30m" + "stable": + enabled: true + waves: 6 + interval: "30m" + "rock-solid": + enabled: false + waves: 5 + interval: "5m" + +# release requirements, don't forget to register check function in a file requirements.go +requirements: + "k8s": "1.25.0" # modules/040-control-plane-manager/requirements/check.go + "ingressNginx": "1.1" # modules/402-ingress-nginx/requirements/check.go + "nodesMinimalOSVersionUbuntu": "18.04" # modules/040-node-manager/requirements/check.go + "containerdOnAllNodes": "true" # modules/040-node-manager/requirements/check.go + "linstorMustBeDisabled": "true" # modules/041-linstor/requirements/check.go + "istioMinimalVersion": "1.16" # modules/110-istio/requirements/check.go + + # TODO: Delete in D8 1.60, migrating to istioMinimalVersion + "istioVer": "1.16" # modules/110-istio/requirements/check.go + "testVer": "1.16" # modules/110-istio/requirements/check.go + +# map of disruptions, associated with a specific release. You have to register check functions before specified release +disruptions: + "1.36": ["ingressNginx"] # modules/402-ingress-nginx/requirements/disruption.go