[DPTP-4298] validate _cluster.yaml for sanitized-prow-jobs

cmd/prow-job-dispatcher/main.go

@@ -4,6 +4,7 @@ import (
 	"flag"
 	"fmt"
 	"io/fs"
+	"log"
 	"net/http"
 	"os"
 	"os/exec"
@@ -55,10 +56,11 @@ type options struct {
 
 	prometheusDaysBefore int
 
-	createPR    bool
-	githubLogin string
-	targetDir   string
-	assign      string
+	createPR     bool
+	validateOnly bool
+	githubLogin  string
+	targetDir    string
+	assign       string
 
 	enableClusters  flagutil.Strings
 	disableClusters flagutil.Strings
@@ -87,6 +89,7 @@ func gatherOptions() options {
 	fs.IntVar(&o.prometheusDaysBefore, "prometheus-days-before", 14, "Number [1,15] of days before. Time 00-00-00 of that day will be used as time to query Prometheus. E.g., 1 means 00-00-00 of yesterday.")
 
 	fs.BoolVar(&o.createPR, "create-pr", false, "Create a pull request to the change made with this tool.")
+	fs.BoolVar(&o.validateOnly, "validate-only", false, "Only validate the cluster configuration and exit. Used for presubmit validation.")
 	fs.StringVar(&o.githubLogin, "github-login", githubLogin, "The GitHub username to use.")
 	fs.StringVar(&o.targetDir, "target-dir", "", "The directory containing the target repo.")
 	fs.StringVar(&o.assign, "assign", "ghost", "The github username or group name to assign the created pull request to.")
@@ -109,12 +112,6 @@ func gatherOptions() options {
 }
 
 func (o *options) validate() error {
-	if o.prowJobConfigDir == "" {
-		return fmt.Errorf("mandatory argument --prow-jobs-dir wasn't set")
-	}
-	if o.configPath == "" {
-		return fmt.Errorf("mandatory argument --config-path wasn't set")
-	}
 
 	if o.prometheusDaysBefore < 1 || o.prometheusDaysBefore > 15 {
 		return fmt.Errorf("--prometheus-days-before must be between 1 and 15")
@@ -124,12 +121,20 @@ func (o *options) validate() error {
 		logrus.Fatal("mandatory argument --cluster-config-path wasn't set")
 	}
 
-	if o.jobsStoragePath == "" {
-		logrus.Fatal("mandatory argument --jobs-storage-path wasn't set")
-	}
-
-	if o.slackTokenPath == "" {
-		logrus.Fatal("mandatory argument --slack-token-path wasn't set")
+	// These arguments are not required when running in validate-only mode
+	if !o.validateOnly {
+		if o.prowJobConfigDir == "" {
+			return fmt.Errorf("mandatory argument --prow-jobs-dir wasn't set")
+		}
+		if o.jobsStoragePath == "" {
+			return fmt.Errorf("mandatory argument --jobs-storage-path wasn't set")
+		}
+		if o.configPath == "" {
+			return fmt.Errorf("mandatory argument --config-path wasn't set")
+		}
+		if o.slackTokenPath == "" {
+			logrus.Fatal("mandatory argument --slack-token-path wasn't set")
+		}
 	}
 
 	enabled := o.enableClusters.StringSet()
@@ -159,7 +164,42 @@ func (o *options) validate() error {
 			return err
 		}
 	}
-	return o.PrometheusOptions.Validate()
+
+	// Prometheus validation is not required in validate-only mode
+	if !o.validateOnly {
+		return o.PrometheusOptions.Validate()
+	}
+	return nil
+}
+
+// validateConfigurations performs validation on the cluster and main config files
+func validateConfigurations(o options) error {
+	logrus.Info("Validating cluster configuration...")
+
+	// Load and validate cluster configuration
+	clusterMap, blockedClusters, err := dispatcher.LoadClusterConfig(o.clusterConfigPath)
+	if err != nil {
+		return fmt.Errorf("failed to load config from %q: %w", o.clusterConfigPath, err)
+	}
+	outFilePath := filepath.Join("/tmp", filepath.Base(o.clusterConfigPath))
+	if err := dispatcher.SaveClusterConfigPreservingFormat(clusterMap, blockedClusters, o.clusterConfigPath, outFilePath); err != nil {
+		return fmt.Errorf("failed to save config to %q: %w", outFilePath, err)
+	}
+	// now diff the roundtripped file with the original
+	output, err := exec.Command("diff", "-u", outFilePath, o.clusterConfigPath).CombinedOutput() // Execute the diff command and get combined output
+	if err != nil {
+		switch e := err.(type) {
+		case *exec.ExitError:
+			fmt.Printf("Diff found, exit code: %d\n", e.ExitCode())
+		default:
+			log.Fatalf("Error running diff command: %v\n", err)
+		}
+	}
+
+	fmt.Println(string(output)) // Print the diff output
+
+	logrus.Info("All validations passed successfully")
+	return err
 }
 
 // getCloudProvidersForE2ETests returns a set of cloud providers where a cluster is hosted for an e2e test defined in the given Prow job config.
@@ -658,6 +698,16 @@ func main() {
 		logrus.WithError(err).Fatal("Failed to complete options.")
 	}
 
+	// If validate-only mode is enabled, run validation and exit
+	if o.validateOnly {
+		if err := validateConfigurations(o); err != nil {
+			logrus.WithError(err).Fatal("Validation failed")
+			os.Exit(1)
+		}
+		logrus.Info("Validation completed successfully")
+		os.Exit(0)
+	}
+
 	if o.createPR {
 		if err := o.PRCreationOptions.Finalize(); err != nil {
 			logrus.WithError(err).Fatal("Failed to finalize PR creation options")

pkg/dispatcher/helpers.go

@@ -1,8 +1,10 @@
 package dispatcher
 
 import (
+	"fmt"
 	"os"
 	"reflect"
+	"strings"
 
 	"k8s.io/apimachinery/pkg/util/sets"
 	prowconfig "sigs.k8s.io/prow/pkg/config"
@@ -116,3 +118,194 @@ func HasCapacityOrCapabilitiesChanged(prev, next ClusterMap) bool {
 
 	return false
 }
+
+// SaveClusterConfigPreservingFormat saves the ClusterMap to YAML while preserving the original format, order, and case sensitivity
+// This method reconstructs the YAML with the exact same structure and field order as the original
+func SaveClusterConfigPreservingFormat(clusterMap ClusterMap, blockedClusters sets.Set[string], originalFilePath, outputFilePath string) error {
+	// Read and parse the original file to understand the structure and order
+	originalData, err := os.ReadFile(originalFilePath)
+	if err != nil {
+		return err
+	}
+
+	// Parse the original YAML structure
+	var originalClusters map[string][]struct {
+		Name         string   `yaml:"name"`
+		Capacity     int      `yaml:"capacity"`
+		Capabilities []string `yaml:"capabilities"`
+		Blocked      bool     `yaml:"blocked"`
+	}
+	if err := yaml.Unmarshal(originalData, &originalClusters); err != nil {
+		return err
+	}
+
+	// Analyze the original file to understand field order and explicit blocked entries
+	originalLines := strings.Split(string(originalData), "\n")
+	explicitBlockedFalse := make(map[string]bool)
+	clusterFieldOrder := make(map[string][]string) // cluster name -> field order
+
+	// Parse the original to understand field order for each cluster
+	currentCluster := ""
+	for _, line := range originalLines {
+		trimmed := strings.TrimSpace(line)
+		if strings.HasPrefix(trimmed, "- name: ") {
+			currentCluster = strings.TrimPrefix(trimmed, "- name: ")
+			clusterFieldOrder[currentCluster] = []string{"name"} // name is always first
+		} else if currentCluster != "" && strings.HasPrefix(line, "    ") {
+			// This is a field for the current cluster
+			if strings.Contains(line, "blocked: false") {
+				explicitBlockedFalse[currentCluster] = true
+				clusterFieldOrder[currentCluster] = append(clusterFieldOrder[currentCluster], "blocked")
+			} else if strings.Contains(line, "blocked: true") {
+				clusterFieldOrder[currentCluster] = append(clusterFieldOrder[currentCluster], "blocked")
+			} else if strings.Contains(line, "capacity:") {
+				clusterFieldOrder[currentCluster] = append(clusterFieldOrder[currentCluster], "capacity")
+			} else if strings.Contains(line, "capabilities:") {
+				clusterFieldOrder[currentCluster] = append(clusterFieldOrder[currentCluster], "capabilities")
+			}
+		}
+	}
+
+	var output strings.Builder
+	processedClusters := make(map[string]bool)
+
+	// Reconstruct the YAML maintaining the original provider order
+	providers := make([]string, 0, len(originalClusters))
+	for provider := range originalClusters {
+		providers = append(providers, provider)
+	}
+	// Keep the original order by parsing line by line to detect provider order
+	providerOrder := []string{}
+	for _, line := range originalLines {
+		if len(line) > 0 && line[0] != ' ' && line[0] != '-' && strings.HasSuffix(line, ":") {
+			provider := strings.TrimSuffix(line, ":")
+			// Add to order if not already present
+			found := false
+			for _, p := range providerOrder {
+				if p == provider {
+					found = true
+					break
+				}
+			}
+			if !found {
+				providerOrder = append(providerOrder, provider)
+			}
+		}
+	}
+
+	for _, provider := range providerOrder {
+		output.WriteString(fmt.Sprintf("%s:\n", provider))
+
+		clusters := originalClusters[provider]
+		for _, originalCluster := range clusters {
+			clusterName := originalCluster.Name
+			processedClusters[clusterName] = true
+
+			output.WriteString(fmt.Sprintf("  - name: %s\n", clusterName))
+
+			// Check if this cluster exists in our ClusterMap
+			if clusterInfo, exists := clusterMap[clusterName]; exists {
+				// Write fields in the original order
+				fieldOrder := clusterFieldOrder[clusterName]
+				if len(fieldOrder) == 0 {
+					// Default order if not found
+					fieldOrder = []string{"name", "capacity", "capabilities", "blocked"}
+				}
+
+				for _, field := range fieldOrder {
+					switch field {
+					case "capacity":
+						if originalCluster.Capacity > 0 || (clusterInfo.Capacity != 100 && clusterInfo.Capacity != 0) {
+							capacity := clusterInfo.Capacity
+							if capacity == 0 || capacity == 100 {
+								if originalCluster.Capacity > 0 {
+									capacity = originalCluster.Capacity
+								}
+							}
+							if capacity > 0 && capacity != 100 {
+								output.WriteString(fmt.Sprintf("    capacity: %d\n", capacity))
+							}
+						}
+					case "capabilities":
+						if len(clusterInfo.Capabilities) > 0 {
+							output.WriteString("    capabilities:\n")
+							for _, cap := range clusterInfo.Capabilities {
+								output.WriteString(fmt.Sprintf("    - %s\n", cap))
+							}
+						}
+					case "blocked":
+						if blockedClusters.Has(clusterName) {
+							output.WriteString("    blocked: true\n")
+						} else if explicitBlockedFalse[clusterName] {
+							output.WriteString("    blocked: false\n")
+						}
+					}
+				}
+			} else {
+				// Cluster not in ClusterMap, preserve original structure but mark as blocked
+				fieldOrder := clusterFieldOrder[clusterName]
+				if len(fieldOrder) == 0 {
+					fieldOrder = []string{"name", "capacity", "capabilities", "blocked"}
+				}
+
+				for _, field := range fieldOrder {
+					switch field {
+					case "capacity":
+						if originalCluster.Capacity > 0 {
+							output.WriteString(fmt.Sprintf("    capacity: %d\n", originalCluster.Capacity))
+						}
+					case "capabilities":
+						if len(originalCluster.Capabilities) > 0 {
+							output.WriteString("    capabilities:\n")
+							for _, cap := range originalCluster.Capabilities {
+								output.WriteString(fmt.Sprintf("    - %s\n", cap))
+							}
+						}
+					case "blocked":
+						output.WriteString("    blocked: true\n")
+					}
+				}
+			}
+		}
+	}
+
+	// Add any new clusters that weren't in the original file
+	newClusters := make(map[string][]string) // provider -> cluster names
+	for clusterName, clusterInfo := range clusterMap {
+		if !processedClusters[clusterName] {
+			newClusters[clusterInfo.Provider] = append(newClusters[clusterInfo.Provider], clusterName)
+		}
+	}
+
+	// Add new clusters to existing providers or create new provider sections
+	for provider, clusterNames := range newClusters {
+		// Check if provider already exists in output
+		if !strings.Contains(output.String(), provider+":") {
+			// Add new provider section
+			output.WriteString(fmt.Sprintf("\n%s:\n", provider))
+		}
+
+		for _, clusterName := range clusterNames {
+			clusterInfo := clusterMap[clusterName]
+			output.WriteString(fmt.Sprintf("  - name: %s\n", clusterName))
+
+			if clusterInfo.Capacity != 100 && clusterInfo.Capacity != 0 {
+				output.WriteString(fmt.Sprintf("    capacity: %d\n", clusterInfo.Capacity))
+			}
+
+			if len(clusterInfo.Capabilities) > 0 {
+				output.WriteString("    capabilities:\n")
+				for _, cap := range clusterInfo.Capabilities {
+					output.WriteString(fmt.Sprintf("    - %s\n", cap))
+				}
+			}
+
+			if blockedClusters.Has(clusterName) {
+				output.WriteString("    blocked: true\n")
+			}
+		}
+	}
+
+	// Write the output to file
+	return os.WriteFile(outputFilePath, []byte(output.String()), 0644)
+}