From 21c7f057633d5f97c312f81f7779349aec847b3f Mon Sep 17 00:00:00 2001 From: npolshakova Date: Tue, 17 Sep 2024 11:55:43 -0400 Subject: [PATCH 1/4] add krel validation cmd --- .../validation-data/invalid-indent.yaml | 10 ++ .../validation-data/invalid-multi-line.yaml | 5 + .../validation-data/invalid-yaml-start.yaml | 3 + .../validation-data/missing-punctuation.yaml | 4 + .../cmd/testdata/validation-data/valid.yaml | 4 + cmd/krel/cmd/validate.go | 136 ++++++++++++++++++ cmd/krel/cmd/validate_test.go | 57 ++++++++ 7 files changed, 219 insertions(+) create mode 100644 cmd/krel/cmd/testdata/validation-data/invalid-indent.yaml create mode 100644 cmd/krel/cmd/testdata/validation-data/invalid-multi-line.yaml create mode 100644 cmd/krel/cmd/testdata/validation-data/invalid-yaml-start.yaml create mode 100644 cmd/krel/cmd/testdata/validation-data/missing-punctuation.yaml create mode 100644 cmd/krel/cmd/testdata/validation-data/valid.yaml create mode 100644 cmd/krel/cmd/validate.go create mode 100644 cmd/krel/cmd/validate_test.go diff --git a/cmd/krel/cmd/testdata/validation-data/invalid-indent.yaml b/cmd/krel/cmd/testdata/validation-data/invalid-indent.yaml new file mode 100644 index 00000000000..5ea7828fd2a --- /dev/null +++ b/cmd/krel/cmd/testdata/validation-data/invalid-indent.yaml @@ -0,0 +1,10 @@ +pr: 126108 +releasenote: + text: |- + Reduced state change noise when volume expansion fails. Also mark certain failures as infeasible. + + ACTION REQUIRED: If you are using the `RecoverVolumeExpansionFailure` alpha feature gate + then after upgrading to this release, you need to update some objects. + For any existing PersistentVolumeClaimss with `status.allocatedResourceStatus` set to either + "ControllerResizeFailed" or "NodeResizeFailed", clear the `status.allocatedResourceStatus`. +pr_body: "" \ No newline at end of file diff --git a/cmd/krel/cmd/testdata/validation-data/invalid-multi-line.yaml b/cmd/krel/cmd/testdata/validation-data/invalid-multi-line.yaml new file mode 100644 index 00000000000..5c5e303322b --- /dev/null +++ b/cmd/krel/cmd/testdata/validation-data/invalid-multi-line.yaml @@ -0,0 +1,5 @@ +pr: 125163 +releasenote: + text: 'ACTION REQUIRED: The Dynamic Resource Allocation (DRA) driver's DaemonSet must be deployed + with a service account that enables writing ResourceSlice and reading ResourceClaim + objects.' \ No newline at end of file diff --git a/cmd/krel/cmd/testdata/validation-data/invalid-yaml-start.yaml b/cmd/krel/cmd/testdata/validation-data/invalid-yaml-start.yaml new file mode 100644 index 00000000000..767f9775357 --- /dev/null +++ b/cmd/krel/cmd/testdata/validation-data/invalid-yaml-start.yaml @@ -0,0 +1,3 @@ +pr: 125157 +releasenote: + text: `kubeadm`: The `NodeSwap` check that kubeadm performs during preflight, has a new warning to verify if swap has been configured correctly. \ No newline at end of file diff --git a/cmd/krel/cmd/testdata/validation-data/missing-punctuation.yaml b/cmd/krel/cmd/testdata/validation-data/missing-punctuation.yaml new file mode 100644 index 00000000000..0af291b3d79 --- /dev/null +++ b/cmd/krel/cmd/testdata/validation-data/missing-punctuation.yaml @@ -0,0 +1,4 @@ +pr: 125157 +releasenote: + text: "`kubeadm`: The `NodeSwap` check that kubeadm performs during preflight, has a new warning to verify if swap has been configured correctly" +pr_body: "" diff --git a/cmd/krel/cmd/testdata/validation-data/valid.yaml b/cmd/krel/cmd/testdata/validation-data/valid.yaml new file mode 100644 index 00000000000..f179bf2f800 --- /dev/null +++ b/cmd/krel/cmd/testdata/validation-data/valid.yaml @@ -0,0 +1,4 @@ +pr: 125157 +releasenote: + text: "`kubeadm`: The `NodeSwap` check that kubeadm performs during preflight, has a new warning to verify if swap has been configured correctly." +pr_body: "" diff --git a/cmd/krel/cmd/validate.go b/cmd/krel/cmd/validate.go new file mode 100644 index 00000000000..c87f63ab8af --- /dev/null +++ b/cmd/krel/cmd/validate.go @@ -0,0 +1,136 @@ +/* +Copyright 2020 The Kubernetes Authors. + +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 cmd + +import ( + "fmt" + "os" + "path/filepath" + "regexp" + "strings" + + "github.com/spf13/cobra" + + "sigs.k8s.io/yaml" + + "k8s.io/release/pkg/notes" +) + +func init() { + // Add the validation subcommand to the root command + rootCmd.AddCommand(validateCmd) +} + +// validate represents the subcommand for `krel validate`. +var validateCmd = &cobra.Command{ + Use: "validate", + Short: "The subcommand for validating release notes for the Release Notes subteam of SIG Release", + Long: `krel validate + +The 'validate' subcommand of krel has been developed to: + +1. Check release notes maps for valid yaml. + +2. Check release notes maps for valid punctuation.`, + SilenceUsage: true, + SilenceErrors: true, + RunE: func(cmd *cobra.Command, args []string) error { + // Extract the release notes path from args + releaseNotesPath := args[0] + + // Run the PR creation function + return runValidateReleaseNotes(releaseNotesPath) + }, +} + +func runValidateReleaseNotes(releaseNotesPath string) (err error) { + // Check if the directory exists + if _, err := os.Stat(releaseNotesPath); os.IsNotExist(err) { + return fmt.Errorf("release notes path %s does not exist", releaseNotesPath) + } + + // Validate the YAML files in the directory + err = filepath.Walk(releaseNotesPath, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + // Only process YAML files + if filepath.Ext(path) == ".yaml" || filepath.Ext(path) == ".yml" { + fmt.Printf("Validating YAML file: %s\n", path) + + // Validate YAML + if err := ValidateYamlMap(path); err != nil { + return fmt.Errorf("YAML validation failed for %s: %v", path, err) + } + + // (Optional) You can add custom punctuation validation here + // For example, you could check for missing periods at the end of lines + + fmt.Printf("YAML file %s is valid.\n", path) + } + return nil + }) + if err != nil { + return fmt.Errorf("failed to validate release notes: %v", err) + } + + fmt.Println("All release notes are valid.") + return nil +} + +// ValidateYamlMap reads a YAML map file, unmarshals it into a map, and then re-marshals it +// to validate the correctness of the content. +func ValidateYamlMap(filePath string) error { + // Read the YAML file + data, err := os.ReadFile(filePath) + if err != nil { + return fmt.Errorf("failed to read file %s: %w", filePath, err) + } + + // Unmarshal the YAML data into a map for manipulation and validation + var testMap notes.ReleaseNotesMap + if err := yaml.Unmarshal(data, &testMap); err != nil { + return fmt.Errorf("YAML unmarshal failed for %s:%w", filePath, err) + } + + // Check the map for valid punctuation in the "text" field + if err := validateTextFieldPunctuation(&testMap); err != nil { + return fmt.Errorf("punctuation check failed for file %s: %w", filePath, err) + } + + // Re-marshall the YAML to check if it can be successfully serialized again + _, err = yaml.Marshal(testMap) + if err != nil { + return fmt.Errorf("while re-marshaling map for file %s:%w", filePath, err) + } + + fmt.Printf("File %s is valid YAML.\n", filePath) + return nil +} + +// validateTextFieldPunctuation checks if the "text" field in a YAML map +// ends with valid punctuation (., !, ?). +func validateTextFieldPunctuation(data *notes.ReleaseNotesMap) error { + validPunctuation := regexp.MustCompile(`[.!?]$`) + + text := *data.ReleaseNote.Text + if !validPunctuation.MatchString(strings.TrimSpace(text)) { + return fmt.Errorf("the 'text' field does not end with valid punctuation: '%s'", text) + } + + return nil +} diff --git a/cmd/krel/cmd/validate_test.go b/cmd/krel/cmd/validate_test.go new file mode 100644 index 00000000000..7aa57e048e1 --- /dev/null +++ b/cmd/krel/cmd/validate_test.go @@ -0,0 +1,57 @@ +/* +Copyright 2020 The Kubernetes Authors. + +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 cmd + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestRunValidateReleaseNotes(t *testing.T) { + testDataPath := "testdata/validation-data" + + // Valid YAML returns no error + err := runValidateReleaseNotes(filepath.Join(testDataPath, "valid.yaml")) + assert.NoError(t, err, "Expected no error for valid YAML file") + + // Try a non-existent path + err = runValidateReleaseNotes("nonexistent/path") + assert.Error(t, err, "Expected error for non-existent path") + assert.Contains(t, err.Error(), "does not exist", "Error should be about non-existent path") + + // Missing punctuation YAML returns error + err = runValidateReleaseNotes(filepath.Join(testDataPath, "missing-punctuation.yaml")) + assert.Error(t, err, "Expected error for missing punctuation YAML file") + assert.Contains(t, err.Error(), "field does not end with valid punctuation", "Error should be about missing punctuation") + + // Try invalid yaml starting with "`" + err = runValidateReleaseNotes(filepath.Join(testDataPath, "invalid-yaml-start.yaml")) + assert.Error(t, err, "Expected error for invalid yaml") + assert.Contains(t, err.Error(), "validation failed for testdata/validation-data/invalid-yaml-start", "Error should be about invalid yaml") + + // Try invalid multi line yaml + err = runValidateReleaseNotes(filepath.Join(testDataPath, "invalid-multi-line.yaml")) + assert.Error(t, err, "Expected error for invalid yaml") + assert.Contains(t, err.Error(), "YAML validation failed for testdata/validation-data/invalid-multi-line.yaml", "Error should be about invalid yaml") + + // Try invalid indent + err = runValidateReleaseNotes(filepath.Join(testDataPath, "invalid-indent.yaml")) + assert.Error(t, err, "Expected error for invalid yaml") + assert.Contains(t, err.Error(), "YAML validation failed for testdata/validation-data/invalid-indent.yaml", "Error should be about invalid yaml") +} From 59a24d0e872b785a0723ccb50ec9567facca246f Mon Sep 17 00:00:00 2001 From: npolshakova Date: Wed, 18 Sep 2024 10:18:34 -0400 Subject: [PATCH 2/4] pr feedback --- cmd/krel/cmd/validate.go | 43 ++++++++++++++++++++++++----------- cmd/krel/cmd/validate_test.go | 2 +- 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/cmd/krel/cmd/validate.go b/cmd/krel/cmd/validate.go index c87f63ab8af..344d1554826 100644 --- a/cmd/krel/cmd/validate.go +++ b/cmd/krel/cmd/validate.go @@ -1,5 +1,5 @@ /* -Copyright 2020 The Kubernetes Authors. +Copyright 2024 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -31,15 +31,15 @@ import ( ) func init() { - // Add the validation subcommand to the root command - rootCmd.AddCommand(validateCmd) + // Add the validation subcommand to the release-notes command + releaseNotesCmd.AddCommand(validateCmd) } -// validate represents the subcommand for `krel validate`. +// validate represents the subcommand for `krel release-notes validate`. var validateCmd = &cobra.Command{ Use: "validate", Short: "The subcommand for validating release notes for the Release Notes subteam of SIG Release", - Long: `krel validate + Long: `krel release-notes validate The 'validate' subcommand of krel has been developed to: @@ -49,6 +49,11 @@ The 'validate' subcommand of krel has been developed to: SilenceUsage: true, SilenceErrors: true, RunE: func(cmd *cobra.Command, args []string) error { + // Ensure exactly one argument is provided + if len(args) != 1 { + return fmt.Errorf("exactly one argument is required for release-notes validate, but got %d", len(args)) + } + // Extract the release notes path from args releaseNotesPath := args[0] @@ -58,6 +63,11 @@ The 'validate' subcommand of krel has been developed to: } func runValidateReleaseNotes(releaseNotesPath string) (err error) { + // Ensure the path is not empty + if releaseNotesPath == "" { + return fmt.Errorf("release notes path cannot be empty") + } + // Check if the directory exists if _, err := os.Stat(releaseNotesPath); os.IsNotExist(err) { return fmt.Errorf("release notes path %s does not exist", releaseNotesPath) @@ -74,18 +84,15 @@ func runValidateReleaseNotes(releaseNotesPath string) (err error) { // Validate YAML if err := ValidateYamlMap(path); err != nil { - return fmt.Errorf("YAML validation failed for %s: %v", path, err) + return fmt.Errorf("validating YAML file %s: %v", path, err) } - // (Optional) You can add custom punctuation validation here - // For example, you could check for missing periods at the end of lines - fmt.Printf("YAML file %s is valid.\n", path) } return nil }) if err != nil { - return fmt.Errorf("failed to validate release notes: %v", err) + return fmt.Errorf("validating release notes: %v", err) } fmt.Println("All release notes are valid.") @@ -98,20 +105,22 @@ func ValidateYamlMap(filePath string) error { // Read the YAML file data, err := os.ReadFile(filePath) if err != nil { - return fmt.Errorf("failed to read file %s: %w", filePath, err) + return fmt.Errorf("reading file %s: %w", filePath, err) } // Unmarshal the YAML data into a map for manipulation and validation var testMap notes.ReleaseNotesMap if err := yaml.Unmarshal(data, &testMap); err != nil { - return fmt.Errorf("YAML unmarshal failed for %s:%w", filePath, err) + return fmt.Errorf("YAML unmarshaling %s: %w", filePath, err) } // Check the map for valid punctuation in the "text" field if err := validateTextFieldPunctuation(&testMap); err != nil { - return fmt.Errorf("punctuation check failed for file %s: %w", filePath, err) + return fmt.Errorf("punctuation check for file %s: %w", filePath, err) } + // TODO: Add custom validation checks (tense, grammar, spelling, etc.) https://github.com/kubernetes/release/issues/3767 + // Re-marshall the YAML to check if it can be successfully serialized again _, err = yaml.Marshal(testMap) if err != nil { @@ -127,6 +136,14 @@ func ValidateYamlMap(filePath string) error { func validateTextFieldPunctuation(data *notes.ReleaseNotesMap) error { validPunctuation := regexp.MustCompile(`[.!?]$`) + if data == nil { + return fmt.Errorf("the release notes map is nil") + } + + if data.ReleaseNote.Text == nil { + return fmt.Errorf("the 'text' release notes map field is nil") + } + text := *data.ReleaseNote.Text if !validPunctuation.MatchString(strings.TrimSpace(text)) { return fmt.Errorf("the 'text' field does not end with valid punctuation: '%s'", text) diff --git a/cmd/krel/cmd/validate_test.go b/cmd/krel/cmd/validate_test.go index 7aa57e048e1..0e3dcacbe1e 100644 --- a/cmd/krel/cmd/validate_test.go +++ b/cmd/krel/cmd/validate_test.go @@ -1,5 +1,5 @@ /* -Copyright 2020 The Kubernetes Authors. +Copyright 2024 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. From b78255b07627c29166c47dbed3aba3dcb9c956a5 Mon Sep 17 00:00:00 2001 From: npolshakova Date: Wed, 18 Sep 2024 10:23:08 -0400 Subject: [PATCH 3/4] fmt --- cmd/krel/cmd/validate.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/cmd/krel/cmd/validate.go b/cmd/krel/cmd/validate.go index 344d1554826..b9fd64efbe5 100644 --- a/cmd/krel/cmd/validate.go +++ b/cmd/krel/cmd/validate.go @@ -17,6 +17,7 @@ limitations under the License. package cmd import ( + "errors" "fmt" "os" "path/filepath" @@ -65,7 +66,7 @@ The 'validate' subcommand of krel has been developed to: func runValidateReleaseNotes(releaseNotesPath string) (err error) { // Ensure the path is not empty if releaseNotesPath == "" { - return fmt.Errorf("release notes path cannot be empty") + return errors.New("release notes path cannot be empty") } // Check if the directory exists @@ -84,7 +85,7 @@ func runValidateReleaseNotes(releaseNotesPath string) (err error) { // Validate YAML if err := ValidateYamlMap(path); err != nil { - return fmt.Errorf("validating YAML file %s: %v", path, err) + return fmt.Errorf("validating YAML file %s: %w", path, err) } fmt.Printf("YAML file %s is valid.\n", path) @@ -92,7 +93,7 @@ func runValidateReleaseNotes(releaseNotesPath string) (err error) { return nil }) if err != nil { - return fmt.Errorf("validating release notes: %v", err) + return fmt.Errorf("validating release notes: %w", err) } fmt.Println("All release notes are valid.") @@ -137,11 +138,11 @@ func validateTextFieldPunctuation(data *notes.ReleaseNotesMap) error { validPunctuation := regexp.MustCompile(`[.!?]$`) if data == nil { - return fmt.Errorf("the release notes map is nil") + return errors.New("the release notes map is nil") } if data.ReleaseNote.Text == nil { - return fmt.Errorf("the 'text' release notes map field is nil") + return errors.New("the 'text' release notes map field is nil") } text := *data.ReleaseNote.Text From 4e62186b789d6709a957016c12c77928b88af728 Mon Sep 17 00:00:00 2001 From: npolshakova Date: Wed, 18 Sep 2024 11:55:09 -0400 Subject: [PATCH 4/4] test errors --- cmd/krel/cmd/validate_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/krel/cmd/validate_test.go b/cmd/krel/cmd/validate_test.go index 0e3dcacbe1e..5c0a603ca9f 100644 --- a/cmd/krel/cmd/validate_test.go +++ b/cmd/krel/cmd/validate_test.go @@ -43,15 +43,15 @@ func TestRunValidateReleaseNotes(t *testing.T) { // Try invalid yaml starting with "`" err = runValidateReleaseNotes(filepath.Join(testDataPath, "invalid-yaml-start.yaml")) assert.Error(t, err, "Expected error for invalid yaml") - assert.Contains(t, err.Error(), "validation failed for testdata/validation-data/invalid-yaml-start", "Error should be about invalid yaml") + assert.Contains(t, err.Error(), "YAML unmarshaling testdata/validation-data/invalid-yaml-start", "Error should be about invalid yaml") // Try invalid multi line yaml err = runValidateReleaseNotes(filepath.Join(testDataPath, "invalid-multi-line.yaml")) assert.Error(t, err, "Expected error for invalid yaml") - assert.Contains(t, err.Error(), "YAML validation failed for testdata/validation-data/invalid-multi-line.yaml", "Error should be about invalid yaml") + assert.Contains(t, err.Error(), "YAML unmarshaling testdata/validation-data/invalid-multi-line.yaml", "Error should be about invalid yaml") // Try invalid indent err = runValidateReleaseNotes(filepath.Join(testDataPath, "invalid-indent.yaml")) assert.Error(t, err, "Expected error for invalid yaml") - assert.Contains(t, err.Error(), "YAML validation failed for testdata/validation-data/invalid-indent.yaml", "Error should be about invalid yaml") + assert.Contains(t, err.Error(), "YAML unmarshaling testdata/validation-data/invalid-indent.yaml", "Error should be about invalid yaml") }