pr feedback

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 <path-to-release-notes-maps>
+	Long: `krel release-notes validate <path-to-release-notes-maps>
 
 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)

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.