From b8c6319b8724effa6e9ce3048984880c56736ce6 Mon Sep 17 00:00:00 2001 From: Brad Williams Date: Tue, 30 Jun 2026 15:18:25 -0400 Subject: [PATCH] Prevent aggregated, informing jobs rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED --- cmd/release-controller/config_validate.go | 13 ++ .../config_validate_test.go | 173 ++++++++++++++++++ 2 files changed, 186 insertions(+) diff --git a/cmd/release-controller/config_validate.go b/cmd/release-controller/config_validate.go index 72ecaeed7..f8c174253 100644 --- a/cmd/release-controller/config_validate.go +++ b/cmd/release-controller/config_validate.go @@ -52,6 +52,7 @@ func validateReleaseConfigs(configDir string) error { errors = append(errors, verifyPeriodicFields(releaseConfigs)...) errors = append(errors, findDuplicatePeriodics(releaseConfigs)...) errors = append(errors, validateQualifiers(releaseConfigs)...) + errors = append(errors, validateOptionalAggregatedJobs(releaseConfigs)...) return utilerrors.NewAggregate(errors) } @@ -169,6 +170,18 @@ func validateReleaseQualifiersConfig(configPath string) []error { return errors } +func validateOptionalAggregatedJobs(releaseConfigs []releasecontroller.ReleaseConfig) []error { + var errors []error + for _, config := range releaseConfigs { + for name, verify := range config.Verify { + if verify.Optional && verify.AggregatedProwJob != nil { + errors = append(errors, fmt.Errorf("%s: verification job %q cannot be both optional and an aggregatedProwJob", config.Name, name)) + } + } + } + return errors +} + func validateQualifier(name releasequalifiers.QualifierId, qualifier releasequalifiers.ReleaseQualifier) error { err := qualifier.Validate() if err != nil { diff --git a/cmd/release-controller/config_validate_test.go b/cmd/release-controller/config_validate_test.go index 7c41fdd0e..4fd9478f6 100644 --- a/cmd/release-controller/config_validate_test.go +++ b/cmd/release-controller/config_validate_test.go @@ -418,6 +418,179 @@ func TestValidateUpgradeJobs(t *testing.T) { } } +func TestValidateOptionalAggregatedJobs(t *testing.T) { + testCases := []struct { + name string + configs []releasecontroller.ReleaseConfig + expectedErr bool + }{ + { + name: "optional with aggregatedProwJob is invalid", + configs: []releasecontroller.ReleaseConfig{{ + Name: "4.19.0-0.nightly", + Verify: map[string]releasecontroller.ReleaseVerification{ + "analysis-job": { + Optional: true, + AggregatedProwJob: &releasecontroller.AggregatedProwJobVerification{ + AnalysisJobCount: 10, + }, + }, + }, + }}, + expectedErr: true, + }, + { + name: "non-optional with aggregatedProwJob is valid", + configs: []releasecontroller.ReleaseConfig{{ + Name: "4.19.0-0.nightly", + Verify: map[string]releasecontroller.ReleaseVerification{ + "analysis-job": { + Optional: false, + AggregatedProwJob: &releasecontroller.AggregatedProwJobVerification{ + AnalysisJobCount: 10, + }, + }, + }, + }}, + expectedErr: false, + }, + { + name: "optional without aggregatedProwJob is valid", + configs: []releasecontroller.ReleaseConfig{{ + Name: "4.19.0-0.nightly", + Verify: map[string]releasecontroller.ReleaseVerification{ + "my-optional-job": { + Optional: true, + ProwJob: &releasecontroller.ProwJobVerification{Name: "some-job"}, + }, + }, + }}, + expectedErr: false, + }, + { + name: "non-optional without aggregatedProwJob is valid", + configs: []releasecontroller.ReleaseConfig{{ + Name: "4.19.0-0.nightly", + Verify: map[string]releasecontroller.ReleaseVerification{ + "my-blocking-job": { + Optional: false, + ProwJob: &releasecontroller.ProwJobVerification{Name: "some-job"}, + }, + }, + }}, + expectedErr: false, + }, + { + name: "multiple jobs with one invalid combination", + configs: []releasecontroller.ReleaseConfig{{ + Name: "4.19.0-0.nightly", + Verify: map[string]releasecontroller.ReleaseVerification{ + "good-job": { + Optional: false, + AggregatedProwJob: &releasecontroller.AggregatedProwJobVerification{ + AnalysisJobCount: 10, + }, + }, + "bad-job": { + Optional: true, + AggregatedProwJob: &releasecontroller.AggregatedProwJobVerification{ + AnalysisJobCount: 5, + }, + }, + }, + }}, + expectedErr: true, + }, + { + name: "invalid combination across multiple configs", + configs: []releasecontroller.ReleaseConfig{ + { + Name: "4.18.0-0.nightly", + Verify: map[string]releasecontroller.ReleaseVerification{ + "good-job": { + Optional: false, + AggregatedProwJob: &releasecontroller.AggregatedProwJobVerification{ + AnalysisJobCount: 10, + }, + }, + }, + }, + { + Name: "4.19.0-0.nightly", + Verify: map[string]releasecontroller.ReleaseVerification{ + "bad-job": { + Optional: true, + AggregatedProwJob: &releasecontroller.AggregatedProwJobVerification{ + AnalysisJobCount: 5, + }, + }, + }, + }, + }, + expectedErr: true, + }, + { + name: "optional with aggregatedProwJob containing custom prowJob is invalid", + configs: []releasecontroller.ReleaseConfig{{ + Name: "4.19.0-0.nightly", + Verify: map[string]releasecontroller.ReleaseVerification{ + "analysis-job": { + Optional: true, + AggregatedProwJob: &releasecontroller.AggregatedProwJobVerification{ + ProwJob: &releasecontroller.ProwJobVerification{ + Name: "custom-aggregator", + }, + AnalysisJobCount: 10, + }, + }, + }, + }}, + expectedErr: true, + }, + { + name: "all valid jobs across multiple configs", + configs: []releasecontroller.ReleaseConfig{ + { + Name: "4.18.0-0.nightly", + Verify: map[string]releasecontroller.ReleaseVerification{ + "blocking-aggregated": { + Optional: false, + AggregatedProwJob: &releasecontroller.AggregatedProwJobVerification{ + AnalysisJobCount: 10, + }, + }, + "optional-regular": { + Optional: true, + ProwJob: &releasecontroller.ProwJobVerification{Name: "some-job"}, + }, + }, + }, + { + Name: "4.19.0-0.nightly", + Verify: map[string]releasecontroller.ReleaseVerification{ + "blocking-regular": { + Optional: false, + ProwJob: &releasecontroller.ProwJobVerification{Name: "another-job"}, + }, + }, + }, + }, + expectedErr: false, + }, + } + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + errs := validateOptionalAggregatedJobs(testCase.configs) + if len(errs) > 0 && !testCase.expectedErr { + t.Errorf("got error when error was not expected: %v", errs) + } + if len(errs) == 0 && testCase.expectedErr { + t.Errorf("did not get error when error was expected") + } + }) + } +} + func TestValidateQualifiersConfiguration(t *testing.T) { testCases := []struct { name string