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

cmd/prow-job-dispatcher/main.go

@@ -55,10 +55,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 +88,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 +111,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 +120,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 +163,45 @@ func (o *options) validate() error {
 			return err
 		}
 	}
-	return o.PrometheusOptions.Validate()
+
+	// Prometheus validation is not required in validate-only mode
+	if !o.validateOnly {
+		if err := o.PrometheusOptions.Validate(); err != nil {
+			return err
+		}
+	}
+
+	// If validate-only mode is enabled, run cluster configuration validation
+	if o.validateOnly {
+		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:
+				logrus.Fatalf("Error running diff command: %v\n", err)
+			}
+		}
+
+		fmt.Println(string(output)) // Print the diff output
+
+		logrus.Info("All validations passed successfully")
+		return err
+	}
+
+	return nil
 }
 
 // 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 +700,12 @@ func main() {
 		logrus.WithError(err).Fatal("Failed to complete options.")
 	}
 
+	// If validate-only mode is enabled, validation was already done in validate() method, just exit
+	if o.validateOnly {
+		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")

cmd/prow-job-dispatcher/main_test.go

@@ -5,6 +5,7 @@ import (
 	"path/filepath"
 	"reflect"
 	"regexp"
+	"strings"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -523,3 +524,153 @@ func TestSendSlackMessage(t *testing.T) {
 		})
 	}
 }
+
+func TestValidateOnly(t *testing.T) {
+	testCases := []struct {
+		name           string
+		clusterConfig  string
+		wantErr        bool
+		expectedErrMsg string
+		allowDiffError bool // Allow diff errors as they are informational
+	}{
+		{
+			name:           "valid cluster configuration",
+			clusterConfig:  filepath.Join("testdata", t.Name(), "valid_clusters.yaml"),
+			wantErr:        false,
+			allowDiffError: true, // Diff errors are expected and informational
+		},
+		{
+			name:           "invalid cluster configuration",
+			clusterConfig:  filepath.Join("testdata", t.Name(), "invalid_clusters.yaml"),
+			wantErr:        true,
+			expectedErrMsg: "failed to load config",
+			allowDiffError: false,
+		},
+		{
+			name:           "nonexistent cluster configuration",
+			clusterConfig:  filepath.Join("testdata", t.Name(), "nonexistent_clusters.yaml"),
+			wantErr:        true,
+			expectedErrMsg: "failed to load config",
+			allowDiffError: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			opts := options{
+				clusterConfigPath:    tc.clusterConfig,
+				validateOnly:         true,
+				prometheusDaysBefore: 7, // Valid value required for options validation
+			}
+
+			err := opts.validate()
+
+			if tc.wantErr && !tc.allowDiffError {
+				if err == nil {
+					t.Errorf("%s: expected error but got none", tc.name)
+					return
+				}
+				if tc.expectedErrMsg != "" && !strings.Contains(err.Error(), tc.expectedErrMsg) {
+					t.Errorf("%s: expected error containing %q, got %q", tc.name, tc.expectedErrMsg, err.Error())
+				}
+			} else if tc.allowDiffError {
+				// For cases where diff errors are allowed (informational only)
+				// We should either get no error or a diff-related error
+				if err != nil {
+					// Check if it's a diff-related error (exit status 1 from diff command)
+					if !strings.Contains(err.Error(), "exit status 1") {
+						t.Errorf("%s: expected diff error or no error, got: %v", tc.name, err)
+					}
+				}
+				// Diff errors are acceptable for valid configs
+			} else if !tc.wantErr {
+				if err != nil {
+					t.Errorf("%s: expected no error but got: %v", tc.name, err)
+				}
+			}
+		})
+	}
+}
+
+func TestOptionsValidateOnly(t *testing.T) {
+	testCases := []struct {
+		name           string
+		opts           options
+		wantErr        bool
+		errMsg         string
+		allowDiffError bool // Allow diff errors as they are informational
+	}{
+		{
+			name: "validate-only mode with minimal required args",
+			opts: options{
+				validateOnly:         true,
+				clusterConfigPath:    filepath.Join("testdata", "TestValidateOnly", "valid_clusters.yaml"),
+				prometheusDaysBefore: 7, // Valid value
+			},
+			wantErr:        false,
+			allowDiffError: true, // Diff errors are acceptable for valid configs
+		},
+		{
+			name: "validate-only mode with invalid prometheus days",
+			opts: options{
+				validateOnly:         true,
+				clusterConfigPath:    "test-path",
+				prometheusDaysBefore: 0, // Invalid value
+			},
+			wantErr: true,
+			errMsg:  "--prometheus-days-before must be between 1 and 15",
+		},
+		{
+			name: "validate-only mode skips prometheus validation",
+			opts: options{
+				validateOnly:         true,
+				clusterConfigPath:    filepath.Join("testdata", "TestValidateOnly", "valid_clusters.yaml"),
+				prometheusDaysBefore: 7, // Valid value
+				// PrometheusOptions not set - should not cause error in validate-only mode
+			},
+			wantErr:        false,
+			allowDiffError: true, // Diff errors are acceptable for valid configs
+		},
+		{
+			name: "validate-only mode skips prow jobs dir requirement",
+			opts: options{
+				validateOnly:         true,
+				clusterConfigPath:    filepath.Join("testdata", "TestValidateOnly", "valid_clusters.yaml"),
+				prometheusDaysBefore: 7, // Valid value
+				// prowJobConfigDir not set - should not cause error in validate-only mode
+			},
+			wantErr:        false,
+			allowDiffError: true, // Diff errors are acceptable for valid configs
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := tc.opts.validate()
+
+			if tc.wantErr && !tc.allowDiffError {
+				if err == nil {
+					t.Errorf("%s: expected error but got none", tc.name)
+					return
+				}
+				if tc.errMsg != "" && !strings.Contains(err.Error(), tc.errMsg) {
+					t.Errorf("%s: expected error containing %q, got %q", tc.name, tc.errMsg, err.Error())
+				}
+			} else if tc.allowDiffError {
+				// For cases where diff errors are allowed (informational only)
+				// We should either get no error or a diff-related error
+				if err != nil {
+					// Check if it's a diff-related error (exit status 1 from diff command)
+					if !strings.Contains(err.Error(), "exit status 1") {
+						t.Errorf("%s: expected diff error or no error, got: %v", tc.name, err)
+					}
+				}
+				// Diff errors are acceptable for valid configs
+			} else if !tc.wantErr {
+				if err != nil {
+					t.Errorf("%s: expected no error but got: %v", tc.name, err)
+				}
+			}
+		})
+	}
+}

cmd/prow-job-dispatcher/testdata/TestValidateOnly/invalid_clusters.yaml

@@ -0,0 +1,12 @@
+aws:
+  - name: build01
+    capacity: 100
+    capabilities:
+    - intranet
+  - name: build03
+    capacity: invalid_capacity
+gcp:
+  - name: build02
+    capacity: 80
+    capabilities:
+    - public

cmd/prow-job-dispatcher/testdata/TestValidateOnly/valid_clusters.yaml

@@ -0,0 +1,15 @@
+aws:
+  - name: build01
+    capacity: 100
+    capabilities:
+    - intranet
+  - name: build03
+    capacity: 50
+    disabled: true
+gcp:
+  - name: build02
+    capacity: 80
+    capabilities:
+    - public
+  - name: build04
+    blocked: true